Re: [Xen-devel] [PATCH] vmx/hap: optimize CR4 trapping

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 15/02/18 16:25, Roger Pau Monne wrote:
> There a bunch of bits in CR4 that should be allowed to be set directly
> by the guest without requiring Xen intervention, currently this is
> already done by passing through guest writes into the CR4 used when
> running in non-root mode, but taking an expensive vmexit in order to
> do so.
>
> xenalyze reports the following when running a PV guest in shim mode:
>
>  CR_ACCESS             3885950  6.41s 17.04%  3957 cyc { 2361| 3378| 7920}
>    cr4  3885940  6.41s 17.04%  3957 cyc { 2361| 3378| 7920}
>    cr3        1  0.00s  0.00%  3480 cyc { 3480| 3480| 3480}
>      *[  0]        1  0.00s  0.00%  3480 cyc { 3480| 3480| 3480}
>    cr0        7  0.00s  0.00%  7112 cyc { 3248| 5960|17480}
>    clts        2  0.00s  0.00%  4588 cyc { 3456| 5720| 5720}
>
> After this change this turns into:
>
>  CR_ACCESS                  12  0.00s  0.00%  9972 cyc { 3680|11024|24032}
>    cr4        2  0.00s  0.00% 17528 cyc {11024|24032|24032}
>    cr3        1  0.00s  0.00%  3680 cyc { 3680| 3680| 3680}
>      *[  0]        1  0.00s  0.00%  3680 cyc { 3680| 3680| 3680}
>    cr0        7  0.00s  0.00%  9209 cyc { 4184| 7848|17488}
>    clts        2  0.00s  0.00%  8232 cyc { 5352|11112|11112}
>
> Note that this optimized trapping is currently only applied to guests
> running with HAP on Intel hardware. If using shadow paging more CR4
> bits need to be unconditionally trapped, which makes this approach
> unlikely to yield any important performance improvements.
>
> Reported-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> ---
> Cc: Jun Nakajima <jun.nakajima@xxxxxxxxx>
> Cc: Kevin Tian <kevin.tian@xxxxxxxxx>
> Cc: Jan Beulich <jbeulich@xxxxxxxx>
> Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> Cc: Razvan Cojocaru <rcojocaru@xxxxxxxxxxxxxxx>
> Cc: Tamas K Lengyel <tamas@xxxxxxxxxxxxx>
> ---
>  xen/arch/x86/hvm/vmx/vmx.c  | 41 +++++++++++++++++++++++++++++++++++++++++
>  xen/arch/x86/hvm/vmx/vvmx.c |  5 ++++-
>  xen/arch/x86/monitor.c      |  5 +++--
>  3 files changed, 48 insertions(+), 3 deletions(-)
>
> diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
> index d35cf55982..9747b2a398 100644
> --- a/xen/arch/x86/hvm/vmx/vmx.c
> +++ b/xen/arch/x86/hvm/vmx/vmx.c
> @@ -1684,6 +1684,35 @@ static void vmx_update_guest_cr(struct vcpu *v, 
> unsigned int cr)
>          }
>  
>          __vmwrite(GUEST_CR4, v->arch.hvm_vcpu.hw_cr[4]);
> +
> +        if ( (v->domain->arch.monitor.write_ctrlreg_enabled &
> +              monitor_ctrlreg_bitmask(VM_EVENT_X86_CR4)) ||
> +             !paging_mode_hap(v->domain) )
> +            /*
> +             * If requested by introspection or running in shadow mode trap 
> all
> +             * accesses to CR4.

The monitor write_ctrlreg_onchangeonly feature was purposefully
introduced to avoid sending PGE updates to the introspection agent.  It
would be ideal to include that in the mask calculation so introspection
cases don't vmexit for PGE changes.

Also, AMD has similar capabilities, and (as of today) has gained CR
monitoring.

> +             *
> +             * NB: shadow path has not been optimized because it requires
> +             * unconditionally trapping more CR4 bits, at which point the
> +             * performance benefit of doing this is quite dubious.
> +             */
> +            __vmwrite(CR4_GUEST_HOST_MASK, ~0UL);

It would be better to stash the calculated mask, rather than reading it
back in the vmexit handler.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel