From mboxrd@z Thu Jan 1 00:00:00 1970 From: Gleb Natapov Subject: Re: [PATCH 13/24] Implement VMREAD and VMWRITE Date: Wed, 16 Jun 2010 17:48:21 +0300 Message-ID: <20100616144821.GD523@redhat.com> References: <1276431753-nyh@il.ibm.com> <201006131229.o5DCTDhk013030@rice.haifa.ibm.com> <4C15F802.7060106@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]:27857 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1759023Ab0FPOsZ (ORCPT ); Wed, 16 Jun 2010 10:48:25 -0400 Content-Disposition: inline In-Reply-To: <4C15F802.7060106@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: On Mon, Jun 14, 2010 at 12:36:02PM +0300, Avi Kivity wrote: > On 06/13/2010 03:29 PM, Nadav Har'El wrote: > >Implement the VMREAD and VMWRITE instructions. With these instructions, L1 > >can read and write to the VMCS it is holding. The values are read or written > >to the fields of the shadow_vmcs structure introduced in the previous patch. > > > > > >+ > >+static inline int vmcs_field_size(int field_type, struct kvm_vcpu *vcpu) > >+{ > >+ switch (field_type) { > >+ case VMCS_FIELD_TYPE_U16: > >+ return 2; > >+ case VMCS_FIELD_TYPE_U32: > >+ return 4; > >+ case VMCS_FIELD_TYPE_U64: > >+ return 8; > >+ case VMCS_FIELD_TYPE_ULONG: > >+#ifdef CONFIG_X86_64 > >+ if (is_long_mode(vcpu)) > >+ return 8; > >+#endif > >+ return 4; > > No need for the ifdef, is_long_mode() works everywhere. > > >+ } > >+ return 0; /* should never happen */ > > Then BUG()? > > >+} > >+ > > struct vcpu_vmx { > > struct kvm_vcpu vcpu; > > struct list_head local_vcpus_link; > >@@ -4184,6 +4220,189 @@ static int handle_vmclear(struct kvm_vcp > > return 1; > > } > > > > > >+static int handle_vmread_reg(struct kvm_vcpu *vcpu, int reg, > >+ unsigned long field) > >+{ > >+ u64 field_value; > >+ if (!nested_vmcs_read_any(vcpu, field,&field_value)) > >+ return 0; > >+ > >+#ifdef CONFIG_X86_64 > >+ switch (vmcs_field_type(field)) { > >+ case VMCS_FIELD_TYPE_U64: case VMCS_FIELD_TYPE_ULONG: > >+ if (!is_long_mode(vcpu)) { > >+ kvm_register_write(vcpu, reg+1, field_value>> 32); > > What's this reg+1 thing? I thought vmread simply ignores the upper half. > > >+ field_value = (u32)field_value; > >+ } > >+ } > >+#endif > >+ kvm_register_write(vcpu, reg, field_value); > >+ return 1; > >+} > >+ > >+static int handle_vmread_mem(struct kvm_vcpu *vcpu, gva_t gva, > >+ unsigned long field) > >+{ > >+ u64 field_value; > >+ if (!nested_vmcs_read_any(vcpu, field,&field_value)) > >+ return 0; > >+ > >+ /* It's ok to use *_system, because handle_vmread verifies cpl=0 */ > > >+ kvm_write_guest_virt_system(gva,&field_value, > >+ vmcs_field_size(vmcs_field_type(field), vcpu), > >+ vcpu, NULL); > > vmread doesn't support 64-bit writes to memory outside long mode, so > you'll have to truncate the write. > > I think you'll be better off returning a 32-bit size in > vmcs_field_size() in these cases. > Actually write should be always 32bit long outside IA-32e mode and 64bit long in 64 bit mode. Unused bits should be set to zero. > >+ return 1; > >+} > >+ > >+static int handle_vmread(struct kvm_vcpu *vcpu) > >+{ > >+ unsigned long field; > >+ unsigned long exit_qualification = vmcs_readl(EXIT_QUALIFICATION); > >+ u32 vmx_instruction_info = vmcs_read32(VMX_INSTRUCTION_INFO); > >+ gva_t gva = 0; > >+ int read_succeed; > >+ > >+ if (!nested_vmx_check_permission(vcpu)) > >+ return 1; > >+ > >+ if (!nested_map_current(vcpu)) { > >+ printk(KERN_INFO "%s invalid shadow vmcs\n", __func__); > >+ set_rflags_to_vmx_fail_invalid(vcpu); > >+ return 1; > >+ } > > Can do the read_any() here. > > >+ > >+ /* decode instruction info to get the field to read and where to store > >+ * its value */ > >+ field = kvm_register_read(vcpu, VMX_OPERAND_REG2(vmx_instruction_info)); > >+ if (VMX_OPERAND_IS_REG(vmx_instruction_info)) { > >+ read_succeed = handle_vmread_reg(vcpu, > >+ VMX_OPERAND_REG(vmx_instruction_info), field); > >+ } else { > >+ gva = get_vmx_mem_address(vcpu, exit_qualification, > >+ vmx_instruction_info); > >+ if (gva == 0) > >+ return 1; > >+ read_succeed = handle_vmread_mem(vcpu, gva, field); > >+ } > >+ > >+ if (read_succeed) { > >+ clear_rflags_cf_zf(vcpu); > >+ skip_emulated_instruction(vcpu); > >+ } else { > >+ set_rflags_to_vmx_fail_valid(vcpu); > >+ vmcs_write32(VM_INSTRUCTION_ERROR, 12); > > s_e_i() in any case but an exception. > > >+ } > >+ > >+ nested_unmap_current(vcpu); > >+ return 1; > >+} > >+ > >+ > > > >+ if (VMX_OPERAND_IS_REG(vmx_instruction_info)) > >+ field_value = kvm_register_read(vcpu, > >+ VMX_OPERAND_REG(vmx_instruction_info)); > >+ else { > >+ gva = get_vmx_mem_address(vcpu, exit_qualification, > >+ vmx_instruction_info); > >+ if (gva == 0) > >+ return 1; > >+ kvm_read_guest_virt(gva,&field_value, > >+ vmcs_field_size(field_type, vcpu), vcpu, NULL); > > Check for exception. > > > -- > 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.