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 majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
                        Gleb.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to