Re: [PATCH] VMX: Eliminate cr3 store/load vmexit when UG enabled

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 23/10/13 08:39, Yang Zhang wrote:
> From: Yang Zhang <yang.z.zhang@Intel.com>
>
> With the feature of unrestricted guest, there should no vmexit
> be triggered when guest accesses the cr3 in non-paging mode.
>
> Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com>

You English here confused me for a bit.  I presume you mean "Xen should
not cause vmexits for cr3 accesses in unrestricted guests", whereas the
current meaning implies that hardware wont generate a vmexit for cr3
accesses for unrestricted guests (which is not correct according to the
SDM).

> ---
>  xen/arch/x86/hvm/vmx/vmx.c |    6 +++---
>  1 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
> index 9ca8632..b9b5e74 100644
> --- a/xen/arch/x86/hvm/vmx/vmx.c
> +++ b/xen/arch/x86/hvm/vmx/vmx.c
> @@ -1086,7 +1086,7 @@ static void vmx_update_guest_cr(struct vcpu *v, unsigned int cr)
>              uint32_t cr3_ctls = (CPU_BASED_CR3_LOAD_EXITING |
>                                   CPU_BASED_CR3_STORE_EXITING);
>              v->arch.hvm_vmx.exec_control &= ~cr3_ctls;
> -            if ( !hvm_paging_enabled(v) )
> +            if ( !hvm_paging_enabled(v) && !vmx_unrestricted_guest(v) )

>From my (brief, so please correct me if I am wrong) reading of the spec,
guest_CR0.PG is enforced if SECONDARY_EXEC_UNRESTRICTED_GUEST is clear. 
Therefore, !hvm_paging_enable() implies
SECONDARY_EXEC_UNRESTRICTED_GUEST is set (or we will incur a vmentry
failure)

>                  v->arch.hvm_vmx.exec_control |= cr3_ctls;
>  
>              /* Trap CR3 updates if CR3 memory events are enabled. */
> @@ -1156,7 +1156,7 @@ static void vmx_update_guest_cr(struct vcpu *v, unsigned int cr)
>      case 3:
>          if ( paging_mode_hap(v->domain) )
>          {
> -            if ( !hvm_paging_enabled(v) )
> +            if ( !hvm_paging_enabled(v) && !vmx_unrestricted_guest(v) )
>                  v->arch.hvm_vcpu.hw_cr[3] =
>                      v->domain->arch.hvm_domain.params[HVM_PARAM_IDENT_PT];
>              vmx_load_pdptrs(v);

Why does unrestricted mode affect which pagetables we use when moving
back into Xen?

> @@ -2408,7 +2408,7 @@ void vmx_vmexit_handler(struct cpu_user_regs *regs)
>  
>      hvm_invalidate_regs_fields(regs);
>  
> -    if ( paging_mode_hap(v->domain) && hvm_paging_enabled(v) )
> +    if ( paging_mode_hap(v->domain) )
>      {
>          __vmread(GUEST_CR3, &v->arch.hvm_vcpu.hw_cr[3]);
>          v->arch.hvm_vcpu.guest_cr[3] = v->arch.hvm_vcpu.hw_cr[3];

~Andrew