Re: [PATCH v2] KVM: fix load_guest_segment_descriptor() to return X86EMUL_*

[Takuya Yoshikawa <yoshikawa.takuya@oss.ntt.co.jp>]

A bit more explanation,

Takuya Yoshikawa wrote:
> This patch fixes load_guest_segment_descriptor() to return
> X86EMUL_PROPAGATE_FAULT when it tries to access the descriptor
> table beyond the limit of it: suggested by Marcelo.
> 
> I have checked current callers of this helper function,
>   - kvm_load_segment_descriptor()
>   - kvm_task_switch()
> and confirmed that this patch will change nothing in the
> upper layers if we do not change the handling of this
> return value from load_guest_segment_descriptor().
> 
> Next step: Although fixing the kvm_task_switch() to handle the
> propagated faults properly seems difficult, and maybe not worth
> it because TSS is not used commonly these days, we can fix
> kvm_load_segment_descriptor(). By doing so, the injected #GP
> becomes possible to be handled by the guest. The only problem
> for this is how to differentiate this fault from the page faults
> generated by kvm_read_guest_virt(). We may have to split this
> function to achive this goal.
> 

My concern is we may have to inject different types of
faults/exceptions depending on callers when kvm_read_guest_virt()
returns X86EMUL_PROPAGATE_FAULT. Actually if always injecting
page faults in the load_guest_segment_descriptor() right after
kvm_read_guest_virt() is OK, we do not have any problems.

Personally I think we'd better to inject page faults for
kvm_load_segment_descriptor().

Is it right?

> 
> Signed-off-by: Takuya Yoshikawa <yoshikawa.takuya@oss.ntt.co.jp>
> ---
>  arch/x86/kvm/x86.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index d47ceda..e5335e5 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -4662,7 +4662,7 @@ static int load_guest_segment_descriptor(struct kvm_vcpu *vcpu, u16 selector,
>  
>  	if (dtable.limit < index * 8 + 7) {
>  		kvm_queue_exception_e(vcpu, GP_VECTOR, selector & 0xfffc);
> -		return 1;
> +		return X86EMUL_PROPAGATE_FAULT;
>  	}
>  	return kvm_read_guest_virt(dtable.base + index*8, seg_desc, sizeof(*seg_desc), vcpu);
>  }