Re: [PATCH v3 1/5] x86: suppress XPTI-related TLB flushes when possible

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Wed, Sep 25, 2019 at 05:23:11PM +0200, Jan Beulich wrote:
> When there's no XPTI-enabled PV domain at all, there's no need to issue
> respective TLB flushes. Hardwire opt_xpti_* to false when !PV, and
> record the creation of PV domains by bumping opt_xpti_* accordingly.
> 
> As to the sticky opt_xpti_domu vs increment/decrement of opt_xpti_hwdom,
> this is done this way to avoid
> (a) widening the former variable,
> (b) any risk of a missed flush, which would result in an XSA if a DomU
>     was able to exercise it, and
> (c) any races updating the variable.
> Fundamentally the TLB flush done when context switching out the domain's
> vCPU-s the last time before destroying the domain ought to be
> sufficient, so in principle DomU handling could be made match hwdom's.
> 
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> ---
> v3: Re-base.
> v2: Add comment to spec_ctrl.h. Explain difference in accounting of DomU
>     and hwdom.
> ---
> TBD: The hardwiring to false could be extended to opt_pv_l1tf_* and (for
>      !HVM) opt_l1d_flush as well.
> 
> ---
>  xen/arch/x86/flushtlb.c         |    2 +-
>  xen/arch/x86/pv/domain.c        |   14 +++++++++++++-
>  xen/arch/x86/spec_ctrl.c        |    6 ++++++
>  xen/include/asm-x86/spec_ctrl.h |   11 +++++++++++
>  4 files changed, 31 insertions(+), 2 deletions(-)
> 
> --- a/xen/arch/x86/flushtlb.c
> +++ b/xen/arch/x86/flushtlb.c
> @@ -207,7 +207,7 @@ unsigned int flush_area_local(const void
>                   */
>                  invpcid_flush_one(PCID_PV_PRIV, addr);
>                  invpcid_flush_one(PCID_PV_USER, addr);
> -                if ( opt_xpti_hwdom || opt_xpti_domu )
> +                if ( opt_xpti_hwdom > 1 || opt_xpti_domu > 1 )
>                  {
>                      invpcid_flush_one(PCID_PV_PRIV | PCID_PV_XPTI, addr);
>                      invpcid_flush_one(PCID_PV_USER | PCID_PV_XPTI, addr);
> --- a/xen/arch/x86/pv/domain.c
> +++ b/xen/arch/x86/pv/domain.c
> @@ -272,6 +272,9 @@ void pv_domain_destroy(struct domain *d)
>      destroy_perdomain_mapping(d, GDT_LDT_VIRT_START,
>                                GDT_LDT_MBYTES << (20 - PAGE_SHIFT));
>  
> +    opt_xpti_hwdom -= IS_ENABLED(CONFIG_LATE_HWDOM) &&
> +                      !d->domain_id && opt_xpti_hwdom;
> +
>      XFREE(d->arch.pv.cpuidmasks);
>  
>      FREE_XENHEAP_PAGE(d->arch.pv.gdt_ldt_l1tab);
> @@ -310,7 +313,16 @@ int pv_domain_initialise(struct domain *
>      /* 64-bit PV guest by default. */
>      d->arch.is_32bit_pv = d->arch.has_32bit_shinfo = 0;
>  
> -    d->arch.pv.xpti = is_hardware_domain(d) ? opt_xpti_hwdom : opt_xpti_domu;
> +    if ( is_hardware_domain(d) && opt_xpti_hwdom )
> +    {
> +        d->arch.pv.xpti = true;
> +        ++opt_xpti_hwdom;
> +    }
> +    if ( !is_hardware_domain(d) && opt_xpti_domu )
> +    {
> +        d->arch.pv.xpti = true;
> +        opt_xpti_domu = 2;

I wonder whether a store fence is needed here in order to guarantee
that opt_xpti_domu is visible to flush_area_local before proceeding
any further with domain creation.

Thanks, Roger.