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

[Avi Kivity <avi@redhat.com>]

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