From mboxrd@z Thu Jan 1 00:00:00 1970 From: Takuya Yoshikawa Subject: Re: [PATCH v2] KVM: fix load_guest_segment_descriptor() to return X86EMUL_* Date: Tue, 02 Feb 2010 21:49:35 +0900 Message-ID: <4B681F5F.4090702@oss.ntt.co.jp> References: <20100201221104.b3575ac7.yoshikawa.takuya@oss.ntt.co.jp> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: kvm@vger.kernel.org To: avi@redhat.com, mtosatti@redhat.com Return-path: Received: from serv2.oss.ntt.co.jp ([222.151.198.100]:48501 "EHLO serv2.oss.ntt.co.jp" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752039Ab0BBMrf (ORCPT ); Tue, 2 Feb 2010 07:47:35 -0500 In-Reply-To: <20100201221104.b3575ac7.yoshikawa.takuya@oss.ntt.co.jp> Sender: kvm-owner@vger.kernel.org List-ID: 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 > --- > 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); > }