On Wed, Nov 21, 2012 at 05:04:34PM +0800, Dongxiao Xu wrote:
> abstract vmcs12_read and vmcs12_write functions to do the vmcs12
> read/write operations.
>
> Signed-off-by: Dongxiao Xu <dongxiao.xu@xxxxxxxxx>
> ---
> arch/x86/kvm/vmx.c | 86 +++++++++++++++++++++++++++-------------------------
> 1 files changed, 45 insertions(+), 41 deletions(-)
>
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index f858159..d8670e4 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -5407,32 +5407,67 @@ static inline int vmcs_field_readonly(unsigned long field)
> * some of the bits we return here (e.g., on 32-bit guests, only 32 bits of
> * 64-bit fields are to be returned).
> */
> -static inline bool vmcs12_read_any(struct kvm_vcpu *vcpu,
> - unsigned long field, u64 *ret)
> +static inline u64 vmcs12_read(struct kvm_vcpu *vcpu, unsigned long field)
> {
> short offset = vmcs_field_to_offset(field);
> char *p;
>
> - if (offset < 0)
> + if (offset < 0) {
> + nested_vmx_failValid(vcpu, VMXERR_UNSUPPORTED_VMCS_COMPONENT);
> + skip_emulated_instruction(vcpu);
> return 0;
> + }
>
> p = ((char *)(get_vmcs12(vcpu))) + offset;
>
> switch (vmcs_field_type(field)) {
> case VMCS_FIELD_TYPE_NATURAL_WIDTH:
> - *ret = *((natural_width *)p);
> + return *((natural_width *)p);
> + case VMCS_FIELD_TYPE_U16:
> + return *((u16 *)p);
> + case VMCS_FIELD_TYPE_U32:
> + return *((u32 *)p);
> + case VMCS_FIELD_TYPE_U64:
> + return *((u64 *)p);
> + default:
> + nested_vmx_failValid(vcpu, VMXERR_UNSUPPORTED_VMCS_COMPONENT);
> + skip_emulated_instruction(vcpu);
> + return 0; /* can never happen. */
> + }
> +}
> +
> +static inline int vmcs12_write(struct kvm_vcpu *vcpu,
> + unsigned long field,
> + u64 value)
> +{
> + short offset = vmcs_field_to_offset(field);
> + char *p;
> +
> + if (offset < 0) {
> + nested_vmx_failValid(vcpu, VMXERR_UNSUPPORTED_VMCS_COMPONENT);
> + skip_emulated_instruction(vcpu);
> + return 0;
> + }
> +
> + p = ((char *)(get_vmcs12(vcpu))) + offset;
> +
> + switch (vmcs_field_type(field)) {
> + case VMCS_FIELD_TYPE_NATURAL_WIDTH:
> + *(natural_width *)p = value;
> return 1;
> case VMCS_FIELD_TYPE_U16:
> - *ret = *((u16 *)p);
> + *(u16 *)p = value;
> return 1;
> case VMCS_FIELD_TYPE_U32:
> - *ret = *((u32 *)p);
> + *(u32 *)p = value;
> return 1;
> case VMCS_FIELD_TYPE_U64:
> - *ret = *((u64 *)p);
> + *(u64 *)p = value;
> return 1;
> default:
> - return 0; /* can never happen. */
> + nested_vmx_failValid(vcpu, VMXERR_UNSUPPORTED_VMCS_COMPONENT);
> + skip_emulated_instruction(vcpu);
> + return 0;
> }
> }
>
> @@ -5466,11 +5501,7 @@ static int handle_vmread(struct kvm_vcpu *vcpu)
> /* Decode instruction info and find the field to read */
> field = kvm_register_read(vcpu, (((vmx_instruction_info) >> 28) & 0xf));
> /* Read the field, zero-extended to a u64 field_value */
> - if (!vmcs12_read_any(vcpu, field, &field_value)) {
> - nested_vmx_failValid(vcpu, VMXERR_UNSUPPORTED_VMCS_COMPONENT);
> - skip_emulated_instruction(vcpu);
> - return 1;
> - }
> + field_value = vmcs12_read(vcpu, field);
You do not handle failure here and always write back field_value even if
vmcs12_read() failed. Actually now it is impossible to detect a
failure. Call to nested_vmx_failValid() in vmcs12_read() will be
overwritten by call to nested_vmx_succeed() at the end of
handle_vmread() and skip_emulated_instruction() will be called twice.
> + skip_emulated_instruction(vcpu);
> /*
> * Now copy part of this value to register or memory, as requested.
> * Note that the number of bits actually copied is 32 or 64 depending
> @@ -5500,8 +5531,6 @@ static int handle_vmwrite(struct kvm_vcpu *vcpu)
> gva_t gva;
> unsigned long exit_qualification = vmcs_readl(EXIT_QUALIFICATION);
> u32 vmx_instruction_info = vmcs_read32(VMX_INSTRUCTION_INFO);
> - char *p;
> - short offset;
> /* The value to write might be 32 or 64 bits, depending on L1's long
> * mode, and eventually we need to write that into a field of several
> * possible lengths. The code below first zero-extends the value to 64
> @@ -5537,33 +5566,8 @@ static int handle_vmwrite(struct kvm_vcpu *vcpu)
> skip_emulated_instruction(vcpu);
> return 1;
> }
> -
> - offset = vmcs_field_to_offset(field);
> - if (offset < 0) {
> - nested_vmx_failValid(vcpu, VMXERR_UNSUPPORTED_VMCS_COMPONENT);
> - skip_emulated_instruction(vcpu);
> - return 1;
> - }
> - p = ((char *) get_vmcs12(vcpu)) + offset;
> -
> - switch (vmcs_field_type(field)) {
> - case VMCS_FIELD_TYPE_U16:
> - *(u16 *)p = field_value;
> - break;
> - case VMCS_FIELD_TYPE_U32:
> - *(u32 *)p = field_value;
> - break;
> - case VMCS_FIELD_TYPE_U64:
> - *(u64 *)p = field_value;
> - break;
> - case VMCS_FIELD_TYPE_NATURAL_WIDTH:
> - *(natural_width *)p = field_value;
> - break;
> - default:
> - nested_vmx_failValid(vcpu, VMXERR_UNSUPPORTED_VMCS_COMPONENT);
> - skip_emulated_instruction(vcpu);
> + if (!vmcs12_write(vcpu, field, field_value))
> return 1;
> - }
>
> nested_vmx_succeed(vcpu);
> skip_emulated_instruction(vcpu);
> --
> 1.7.1
--
Gleb.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
