On 11/21/2012 11:04 AM, 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); If you had an error here, you will still continue running the function, you will override the failure (by calling nested_vmx_succeed) and skip emulated instruction twice. You need to detect there was an error and return (like in vmwrite), Orit > /* > * 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); > -- 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