From mboxrd@z Thu Jan 1 00:00:00 1970 From: Gleb Natapov Subject: Re: [PATCH 11/24] Implement VMPTRST Date: Wed, 16 Jun 2010 16:53:35 +0300 Message-ID: <20100616135335.GB523@redhat.com> References: <1276431753-nyh@il.ibm.com> <201006131228.o5DCSCAW012994@rice.haifa.ibm.com> <4C15F31E.9000905@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: "Nadav Har'El" , kvm@vger.kernel.org To: Avi Kivity Return-path: Received: from mx1.redhat.com ([209.132.183.28]:26450 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755962Ab0FPNxi (ORCPT ); Wed, 16 Jun 2010 09:53:38 -0400 Content-Disposition: inline In-Reply-To: <4C15F31E.9000905@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: On Mon, Jun 14, 2010 at 12:15:10PM +0300, Avi Kivity wrote: > On 06/13/2010 03:28 PM, Nadav Har'El wrote: > >This patch implements the VMPTRST instruction. > > > >Signed-off-by: Nadav Har'El > >--- > >--- .before/arch/x86/kvm/x86.c 2010-06-13 15:01:29.000000000 +0300 > >+++ .after/arch/x86/kvm/x86.c 2010-06-13 15:01:29.000000000 +0300 > >@@ -3301,7 +3301,7 @@ static int kvm_read_guest_virt_system(gv > > return kvm_read_guest_virt_helper(addr, val, bytes, vcpu, 0, error); > > } > > > >-static int kvm_write_guest_virt_system(gva_t addr, void *val, > >+int kvm_write_guest_virt_system(gva_t addr, void *val, > > unsigned int bytes, > > struct kvm_vcpu *vcpu, > > u32 *error) > > write_guest_virt_system() is used by writes which need to ignore the > cpl, for example when a cpl 3 instruction loads a segment, the > processor needs to update the accessed flag even though it is only > accessible to cpl 0. That's not your case, you need the ordinary > write_guest_virt(). > > Um, I see there is no kvm_write_guest_virt(), you'll have to introduce it. > the code uses this function after checking cpl to be zero, so may be it is ok, not to pretty though. I was actually hoping to get rid of all kvm_(read|write)_guest_virt* and replace existing uses with emulator_(read|write)_emulated, but this patch series adds more users that will be hard to replace :( > > > >+/* Emulate the VMPTRST instruction */ > >+static int handle_vmptrst(struct kvm_vcpu *vcpu) > >+{ > >+ int r = 0; > >+ unsigned long exit_qualification = vmcs_readl(EXIT_QUALIFICATION); > >+ u32 vmx_instruction_info = vmcs_read32(VMX_INSTRUCTION_INFO); > >+ gva_t vmcs_gva; > >+ > >+ if (!nested_vmx_check_permission(vcpu)) > >+ return 1; > >+ > >+ vmcs_gva = get_vmx_mem_address(vcpu, exit_qualification, > >+ vmx_instruction_info); > >+ if (vmcs_gva == 0) > >+ return 1; > > What's wrong with gva 0? It's favoured by exploiters everywhere. > > >+ r = kvm_write_guest_virt_system(vmcs_gva, > >+ (void *)&to_vmx(vcpu)->nested.current_vmptr, > >+ sizeof(u64), vcpu, NULL); > >+ if (r) { > > Check against the X86EMUL return codes. You'll need to inject a > page fault on failure. > > >+ printk(KERN_INFO "%s failed to write vmptr\n", __func__); > >+ return 1; > >+ } > >+ clear_rflags_cf_zf(vcpu); > >+ skip_emulated_instruction(vcpu); > >+ return 1; > >+} > >+ > > -- > error compiling committee.c: too many arguments to function > > -- > To unsubscribe from this list: send the line "unsubscribe kvm" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- Gleb.