From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: [PATCH 13/24] Implement VMREAD and VMWRITE Date: Mon, 14 Jun 2010 12:36:02 +0300 Message-ID: <4C15F802.7060106@redhat.com> References: <1276431753-nyh@il.ibm.com> <201006131229.o5DCTDhk013030@rice.haifa.ibm.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: kvm@vger.kernel.org To: "Nadav Har'El" Return-path: Received: from mx1.redhat.com ([209.132.183.28]:55565 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755464Ab0FNJgH (ORCPT ); Mon, 14 Jun 2010 05:36:07 -0400 In-Reply-To: <201006131229.o5DCTDhk013030@rice.haifa.ibm.com> Sender: kvm-owner@vger.kernel.org List-ID: 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. > + 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