[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH] Nested VMX: CR emulation fix up



Dong, Eddie wrote on 2013-10-08:
> Acked-by Eddie.dong@xxxxxxxxx
> 
> -----Original Message-----
> From: Zhang, Yang Z
> Sent: Tuesday, October 08, 2013 3:30 PM
> To: xen-devel@xxxxxxxxxxxxxxxxxxx
> Cc: JBeulich@xxxxxxxx; Dong, Eddie; Zhang, Yang Z
> Subject: [PATCH] Nested VMX: CR emulation fix up
> 
> From: Yang Zhang <yang.z.zhang@xxxxxxxxx>
> 
> This patch fixs two issues:
> 1. The CR_READ_SHADOW should only cover the value that L2 wirtes to CR
> when L2 is running. But currently, L0 wirtes wrong value to it during
> virtual vmentry and L2's CR access emualtion.
> 2. L2 changed cr0 in a way that did not change any of L1's shadowed
> bits, but did change L0 shadowed bits. In this case, the effective cr0
> value that L1 would like to write into the hardware is consist of the
> L2-owned bits from the new value combined with the L1-owned bits from L1's 
> guest cr0.
The same issue in cr4 emulation which also fixed by this patch.

> Signed-off-by: Yang Zhang <yang.z.zhang@xxxxxxxxx>
> ---
>  xen/arch/x86/hvm/hvm.c         |    6 ++++ xen/arch/x86/hvm/vmx/vmx.c  
>    |   51 ++++++++++++++++++++++++++++++++++++++-
>  xen/arch/x86/hvm/vmx/vvmx.c    |    2 + xen/include/asm-x86/hvm/vcpu.h
>  |    3 ++ 4 files changed, 60 insertions(+), 2 deletions(-)
> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c index
> de81e45..77ff40e 100644
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -1817,6 +1817,9 @@ int hvm_set_cr0(unsigned long value)
>      }
>      
>      v->arch.hvm_vcpu.guest_cr[0] = value;
> +    if ( !nestedhvm_vmswitch_in_progress(v) &&
> +         nestedhvm_vcpu_in_guestmode(v) )
> +        v->arch.hvm_vcpu.nvcpu.guest_cr[0] = value;
>      hvm_update_guest_cr(v, 0);
>      
>      if ( (value ^ old_value) & X86_CR0_PG ) { @@ -1899,6 +1902,9 @@ int
>      hvm_set_cr4(unsigned long value) }
>      
>      v->arch.hvm_vcpu.guest_cr[4] = value;
> +    if ( !nestedhvm_vmswitch_in_progress(v) &&
> +         nestedhvm_vcpu_in_guestmode(v) )
> +        v->arch.hvm_vcpu.nvcpu.guest_cr[4] = value;
>      hvm_update_guest_cr(v, 4);
>      hvm_memory_event_cr4(value, old_cr);
> diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
> index 9ca8632..279a00a 100644
> --- a/xen/arch/x86/hvm/vmx/vmx.c
> +++ b/xen/arch/x86/hvm/vmx/vmx.c
> @@ -1096,6 +1096,30 @@ static void vmx_update_guest_cr(struct vcpu *v,
> unsigned int cr)
>              vmx_update_cpu_exec_control(v);
>          }
> +        if ( nestedhvm_vcpu_in_guestmode(v) ) +        { +           
> if ( !nestedhvm_vmswitch_in_progress(v) ) +            { +              
>  /* +                 * We get here when L2 changed cr0 in a way that
> did + not change +                 * any of L1's shadowed bits (see
> nvmx_n2_vmexit_handler), +                 * but did change L0 shadowed
> bits. So we first + calculate the +                 * effective cr0
> value that L1 would like to write into the +                 * hardware.
> It consists of the L2-owned bits from + the new +                 *
> value combined with the L1-owned bits from L1's + guest cr0. +          
>       */ +                v->arch.hvm_vcpu.guest_cr[0] &= +             
>       ~__get_vvmcs(vcpu_nestedhvm(v).nv_vvmcx, CR0_GUEST_HOST_MASK); +  
>              v->arch.hvm_vcpu.guest_cr[0] |= +                   
> __get_vvmcs(vcpu_nestedhvm(v).nv_vvmcx, GUEST_CR0) & +                  
>  __get_vvmcs(vcpu_nestedhvm(v).nv_vvmcx, CR0_GUEST_HOST_MASK); +        
>    } +             /* nvcpu.guest_cr[0] is what L2 write to cr0
> actually. */ +            __vmwrite(CR0_READ_SHADOW,
> v->arch.hvm_vcpu.nvcpu.guest_cr[0]); +        } +        else +         
>   __vmwrite(CR0_READ_SHADOW, v->arch.hvm_vcpu.guest_cr[0]); +
>          if ( !(v->arch.hvm_vcpu.guest_cr[0] & X86_CR0_TS) )
>          {
>              if ( v != current )
> @@ -1144,7 +1168,6 @@ static void vmx_update_guest_cr(struct vcpu *v,
> unsigned int cr)
>          v->arch.hvm_vcpu.hw_cr[0] =
>              v->arch.hvm_vcpu.guest_cr[0] | hw_cr0_mask;
>          __vmwrite(GUEST_CR0, v->arch.hvm_vcpu.hw_cr[0]);
> -        __vmwrite(CR0_READ_SHADOW, v->arch.hvm_vcpu.guest_cr[0]);
> 
>          /* Changing CR0 can change some bits in real CR4. */
>          vmx_update_guest_cr(v, 4);
> @@ -1169,6 +1192,31 @@ static void vmx_update_guest_cr(struct vcpu *v,
> unsigned int cr)
>          v->arch.hvm_vcpu.hw_cr[4] = HVM_CR4_HOST_MASK;
>          if ( paging_mode_hap(v->domain) )
>              v->arch.hvm_vcpu.hw_cr[4] &= ~X86_CR4_PAE;
> + +        if ( nestedhvm_vcpu_in_guestmode(v) ) +        { +           
> if ( !nestedhvm_vmswitch_in_progress(v) ) +            { +              
>  /* +                 * We get here when L2 changed cr4 in a way that
> did + not change +                 * any of L1's shadowed bits (see
> nvmx_n2_vmexit_handler), +                 * but did change L0 shadowed
> bits. So we first + calculate the +                 * effective cr4
> value that L1 would like to write into the +                 * hardware.
> It consists of the L2-owned bits from + the new +                 *
> value combined with the L1-owned bits from L1's + guest cr4. +          
>       */ +                v->arch.hvm_vcpu.guest_cr[4] &= +             
>       ~__get_vvmcs(vcpu_nestedhvm(v).nv_vvmcx, CR4_GUEST_HOST_MASK); +  
>              v->arch.hvm_vcpu.guest_cr[4] |= +                   
> __get_vvmcs(vcpu_nestedhvm(v).nv_vvmcx, GUEST_CR4) & +                  
>  __get_vvmcs(vcpu_nestedhvm(v).nv_vvmcx, CR4_GUEST_HOST_MASK); +        
>    } +             /* nvcpu.guest_cr[4] is what L2 write to cr4
> actually. */ +            __vmwrite(CR4_READ_SHADOW,
> v->arch.hvm_vcpu.nvcpu.guest_cr[4]); +        } +        else +         
>   __vmwrite(CR4_READ_SHADOW, v->arch.hvm_vcpu.guest_cr[4]); +
>          v->arch.hvm_vcpu.hw_cr[4] |= v->arch.hvm_vcpu.guest_cr[4];
>          if ( v->arch.hvm_vmx.vmx_realmode )
>              v->arch.hvm_vcpu.hw_cr[4] |= X86_CR4_VME; @@ -1188,7
> +1236,6 @@ static void vmx_update_guest_cr(struct vcpu *v, unsigned
> +int cr)
>              v->arch.hvm_vcpu.hw_cr[4] &= ~X86_CR4_SMEP;
>          } __vmwrite(GUEST_CR4, v->arch.hvm_vcpu.hw_cr[4]); -       
>          __vmwrite(CR4_READ_SHADOW, v->arch.hvm_vcpu.guest_cr[4]);
>          break; default: BUG();
> diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c
> index 2b2de77..6b2d51e 100644
> --- a/xen/arch/x86/hvm/vmx/vvmx.c
> +++ b/xen/arch/x86/hvm/vmx/vvmx.c
> @@ -1042,6 +1042,8 @@ static void load_shadow_guest_state(struct vcpu
> *v)
>      vvmcs_to_shadow_bulk(v, ARRAY_SIZE(vmcs_gstate_field),
>                           vmcs_gstate_field);
> +    nvcpu->guest_cr[0] = __get_vvmcs(vvmcs, CR0_READ_SHADOW);
> +    nvcpu->guest_cr[4] = __get_vvmcs(vvmcs, CR4_READ_SHADOW);
>      hvm_set_cr0(__get_vvmcs(vvmcs, GUEST_CR0));
>      hvm_set_cr4(__get_vvmcs(vvmcs, GUEST_CR4));
>      hvm_set_cr3(__get_vvmcs(vvmcs, GUEST_CR3)); diff --git
> a/xen/include/asm-x86/hvm/vcpu.h b/xen/include/asm-x86/hvm/vcpu.h
> index e8b8cd7..ab5349a 100644
> --- a/xen/include/asm-x86/hvm/vcpu.h
> +++ b/xen/include/asm-x86/hvm/vcpu.h
> @@ -100,6 +100,9 @@ struct nestedvcpu {
>       */
>      bool_t nv_ioport80;
>      bool_t nv_ioportED;
> +
> +    /* L2's control-resgister, just as the L2 sees them. */
> +    unsigned long       guest_cr[5];
>  };
>  
>  #define vcpu_nestedhvm(v) ((v)->arch.hvm_vcpu.nvcpu)


Best regards,
Yang



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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.