Re: [Xen-devel] [PATCH v3 5/9] x86/vvmx: make updating shadow EPTP value more efficient

[George Dunlap <george.dunlap@xxxxxxxxxx>]

On 10/03/2017 04:21 PM, Sergey Dyasli wrote:
> At the moment, the shadow EPTP value is written unconditionally in
> ept_handle_violation().
> 
> Instead, write the value on vmentry to the guest; but only write it if
> the value needs updating.
> 
> To detect this, add a flag to the nestedvcpu struct, stale_np2m, to
> indicate when such an action is necessary.  Set it when the nested p2m
> changes or when the np2m is flushed by an IPI, and clear it when we
> write the new value.
> 
> Since an IPI invalidating the p2m may happen between
> nvmx_switch_guest() and vmx_vmenter, but we can't perform the vmwrite
> with interrupts disabled, check the flag just before entering the
> guest and restart the vmentry if it's set.
> 
> Signed-off-by: Sergey Dyasli <sergey.dyasli@xxxxxxxxxx>
> Signed-off-by: George Dunlap <george.dunlap@xxxxxxxxxx>

Looks good to me; just one question...

> ---
> v2 --> v3:
> - current pointer is now calculated only once in nvmx_eptp_update()
> ---
>  xen/arch/x86/hvm/nestedhvm.c   |  2 ++
>  xen/arch/x86/hvm/vmx/entry.S   |  6 ++++++
>  xen/arch/x86/hvm/vmx/vmx.c     | 14 +++++++-------
>  xen/arch/x86/hvm/vmx/vvmx.c    | 22 ++++++++++++++++++++++
>  xen/arch/x86/mm/p2m.c          | 10 ++++++++--
>  xen/include/asm-x86/hvm/vcpu.h |  1 +
>  6 files changed, 46 insertions(+), 9 deletions(-)
> 
> diff --git a/xen/arch/x86/hvm/nestedhvm.c b/xen/arch/x86/hvm/nestedhvm.c
> index f2f7469d86..74a464d162 100644
> --- a/xen/arch/x86/hvm/nestedhvm.c
> +++ b/xen/arch/x86/hvm/nestedhvm.c
> @@ -56,6 +56,7 @@ nestedhvm_vcpu_reset(struct vcpu *v)
>      nv->nv_vvmcxaddr = INVALID_PADDR;
>      nv->nv_flushp2m = 0;
>      nv->nv_p2m = NULL;
> +    nv->stale_np2m = false;
>  
>      hvm_asid_flush_vcpu_asid(&nv->nv_n2asid);
>  
> @@ -107,6 +108,7 @@ nestedhvm_flushtlb_ipi(void *info)
>       */
>      hvm_asid_flush_core();
>      vcpu_nestedhvm(v).nv_p2m = NULL;
> +    vcpu_nestedhvm(v).stale_np2m = true;
>  }
>  
>  void
> diff --git a/xen/arch/x86/hvm/vmx/entry.S b/xen/arch/x86/hvm/vmx/entry.S
> index 53eedc6363..9fb8f89220 100644
> --- a/xen/arch/x86/hvm/vmx/entry.S
> +++ b/xen/arch/x86/hvm/vmx/entry.S
> @@ -79,6 +79,8 @@ UNLIKELY_END(realmode)
>  
>          mov  %rsp,%rdi
>          call vmx_vmenter_helper
> +        cmp  $0,%eax
> +        jne .Lvmx_vmentry_restart
>          mov  VCPU_hvm_guest_cr2(%rbx),%rax
>  
>          pop  %r15
> @@ -117,6 +119,10 @@ ENTRY(vmx_asm_do_vmentry)
>          GET_CURRENT(bx)
>          jmp  .Lvmx_do_vmentry
>  
> +.Lvmx_vmentry_restart:
> +        sti
> +        jmp  .Lvmx_do_vmentry
> +
>  .Lvmx_goto_emulator:
>          sti
>          mov  %rsp,%rdi
> diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
> index 9cfa9b6965..c9a4111267 100644
> --- a/xen/arch/x86/hvm/vmx/vmx.c
> +++ b/xen/arch/x86/hvm/vmx/vmx.c
> @@ -3249,12 +3249,6 @@ static void ept_handle_violation(ept_qual_t q, paddr_t 
> gpa)
>      case 0:         // Unhandled L1 EPT violation
>          break;
>      case 1:         // This violation is handled completly
> -        /*Current nested EPT maybe flushed by other vcpus, so need
> -         * to re-set its shadow EPTP pointer.
> -         */
> -        if ( nestedhvm_vcpu_in_guestmode(current) &&
> -                        nestedhvm_paging_mode_hap(current ) )
> -            __vmwrite(EPT_POINTER, get_shadow_eptp(current));
>          return;
>      case -1:        // This vioaltion should be injected to L1 VMM
>          vcpu_nestedhvm(current).nv_vmexit_pending = 1;
> @@ -4203,13 +4197,17 @@ static void lbr_fixup(void)
>          bdw_erratum_bdf14_fixup();
>  }
>  
> -void vmx_vmenter_helper(const struct cpu_user_regs *regs)
> +int vmx_vmenter_helper(const struct cpu_user_regs *regs)

...Andy, did you want a comment here explaining what the return value is
supposed to mean? (And/or changing this to a bool?)

 -George

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