Re: [Xen-devel] [PATCH 1/3] vvmx: set vmxon_region_pa of vcpu out of VMX operation to an invalid address

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 13/12/16 12:16, Haozhong Zhang wrote:
> nvmx_handle_vmxon() previously checks whether a vcpu is in VMX
> operation by comparing its vmxon_region_pa with GPA 0. However, 0 is
> also a valid VMXON region address. If L1 hypervisor had set the VMXON
> region address to 0, the check in nvmx_handle_vmxon() will be skipped.
> Fix this problem by using an invalid VMXON region address for vcpu
> out of VMX operation.
>
> Signed-off-by: Haozhong Zhang <haozhong.zhang@xxxxxxxxx>
> ---
>  xen/arch/x86/hvm/vmx/vvmx.c | 20 ++++++++++++++++----
>  1 file changed, 16 insertions(+), 4 deletions(-)
>
> diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c
> index e6e9ebd..f5637eb 100644
> --- a/xen/arch/x86/hvm/vmx/vvmx.c
> +++ b/xen/arch/x86/hvm/vmx/vvmx.c
> @@ -32,6 +32,18 @@ static DEFINE_PER_CPU(u64 *, vvmcs_buf);
>  
>  static void nvmx_purge_vvmcs(struct vcpu *v);
>  
> +/*
> + * When a vcpu is out of VMXON region, set its vmxon_region_pa to
> + * INVALID_VMXON_REGION_PA. We cannot use 0, because 0 is also a valid
> + * VMXON region address.
> + */
> +#define INVALID_VMXON_REGION_PA (~0UL)
> +
> +static bool nvmx_vcpu_in_vmx(struct vcpu *v)

const struct vcpu *v.

Otherwise, Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

I can fix this up on commit if no other issues are found by the other
reviewers.

> +{
> +    return vcpu_2_nvmx(v).vmxon_region_pa != INVALID_VMXON_REGION_PA;
> +}
> +
>  #define VMCS_BUF_SIZE 100
>  
>  int nvmx_cpu_up_prepare(unsigned int cpu)
> @@ -107,7 +119,7 @@ int nvmx_vcpu_initialise(struct vcpu *v)
>  
>      nvmx->ept.enabled = 0;
>      nvmx->guest_vpid = 0;
> -    nvmx->vmxon_region_pa = 0;
> +    nvmx->vmxon_region_pa = INVALID_VMXON_REGION_PA;
>      nvcpu->nv_vvmcx = NULL;
>      nvcpu->nv_vvmcxaddr = VMCX_EADDR;
>      nvmx->intr.intr_info = 0;
> @@ -357,7 +369,7 @@ static int vmx_inst_check_privilege(struct cpu_user_regs 
> *regs, int vmxop_check)
>               !(v->arch.hvm_vcpu.guest_cr[4] & X86_CR4_VMXE) )
>              goto invalid_op;
>      }
> -    else if ( !vcpu_2_nvmx(v).vmxon_region_pa )
> +    else if ( !nvmx_vcpu_in_vmx(v) )
>          goto invalid_op;
>  
>      hvm_get_segment_register(v, x86_seg_cs, &cs);
> @@ -1384,7 +1396,7 @@ int nvmx_handle_vmxon(struct cpu_user_regs *regs)
>      if ( rc != X86EMUL_OKAY )
>          return rc;
>  
> -    if ( nvmx->vmxon_region_pa )
> +    if ( nvmx_vcpu_in_vmx(v) )
>          gdprintk(XENLOG_WARNING, 
>                   "vmxon again: orig %"PRIpaddr" new %lx\n",
>                   nvmx->vmxon_region_pa, gpa);
> @@ -1417,7 +1429,7 @@ int nvmx_handle_vmxoff(struct cpu_user_regs *regs)
>          return rc;
>  
>      nvmx_purge_vvmcs(v);
> -    nvmx->vmxon_region_pa = 0;
> +    nvmx->vmxon_region_pa = INVALID_VMXON_REGION_PA;
>  
>      vmreturn(regs, VMSUCCEED);
>      return X86EMUL_OKAY;


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel