Re: [PATCH 13/24] Implement VMREAD and VMWRITE

[Gleb Natapov <gleb@redhat.com>]

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.