From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Nadav Har'El" Subject: Re: [PATCH 7/24] Understanding guest pointers to vmcs12 structures Date: Mon, 2 Aug 2010 15:25:50 +0300 Message-ID: <20100802122550.GA3779@fermat.math.technion.ac.il> References: <1276431753-nyh@il.ibm.com> <201006131226.o5DCQ95O012945@rice.haifa.ibm.com> <4C15ECD1.1070108@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: kvm@vger.kernel.org To: Avi Kivity Return-path: Received: from mailgw13.technion.ac.il ([132.68.225.13]:53605 "EHLO mailgw13.technion.ac.il" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752913Ab0HBMZy (ORCPT ); Mon, 2 Aug 2010 08:25:54 -0400 Content-Disposition: inline In-Reply-To: <4C15ECD1.1070108@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: Hi, On Mon, Jun 14, 2010, Avi Kivity wrote about "Re: [PATCH 7/24] Understanding guest pointers to vmcs12 structures": > On 06/13/2010 03:26 PM, Nadav Har'El wrote: > >This patch includes a couple of utility functions for extracting pointer > >operands of VMX instructions issued by L1 (a guest hypervisor), and > >translating guest-given vmcs12 virtual addresses to guest-physical >>addresses. >... > >+#define VMX_OPERAND_IS_REG(vii) ((vii)& (1u<< 10)) >... > Since those defines are used just ones, you can fold them into their > uses. It doesn't add much to repeat the variable name. Actually a few of these macros were used several times, but you're right, it didn't make anything clearer and just made the code uglier. So I folded them. > >+ /* offfset = Base + [Index * Scale] + Displacement */ > >+ addr = vmx_get_segment_base(vcpu, seg_reg); > >+ if (base_is_valid) > >+ addr += kvm_register_read(vcpu, base_reg); > >+ if (index_is_valid) > >+ addr += kvm_register_read(vcpu, index_reg)< >+ addr += exit_qualification; /* holds the displacement */ > > > > Do we need a segment limit and access rights check? You are absolutely right. The instructions we're emulating (VMREAD, VMWRITE, VMPTRLD, etc.) should throw a #GP in a bunch of segmentation errors, including segment limit, execute-only segments, non-canonical 64-bit addresses, and a bunch of other unlikely error cases. To achieve 100% accurate emulation in the error path, it will require quite a bit new code (here, and in many other places throughout the nested VMX code) that isn't necessary for running a correctly-written guest hypervisor (such as KVM or VMware). At worst, not accurately emulating the error path correctly might allow a broken L1 to do bad things to itself, but it doesn't allow it to do anything bad to L0 or other L1's. Would you accept that I'll add a TODO in the code here (and in similar cases) and leave perfecting the error path to a later path? Thanks, Nadav. -- Nadav Har'El | Monday, Aug 2 2010, 22 Av 5770 nyh@math.technion.ac.il |----------------------------------------- Phone +972-523-790466, ICQ 13349191 |Wear short sleeves! Support your right to http://nadav.harel.org.il |bare arms!