Re: [PATCH v4 07/30] x86/spec-ctrl: introduce Address Space Isolation command line option

[George Dunlap <gwd@xxxxxxxxxxxxxx>]

On Fri, Oct 23, 2026 at 8:45 AM Roger Pau Monné <roger@xxxxxxxxxxxxxx> wrote:
> > ---
> > Changes in v4:
> > - Docs: "domains" throughout, not "guests" (Roger).
> > - Group the per-domain settings as one-bit fields of d->arch.asi
> >   (Roger).
> > - Rename opt_vcpu_pt_hwdom to opt_pvh_vcpu_pt_hwdom: it acts on a PVH
> >   hardware domain only (Jan).
>
> I would leave opt_vcpu_pt_hwdom as-is.  The fact it only acts on a PVH
> hardware domain is a transient implementation short-coming.  I don't
> think it's worth introducing a separate opt_pv_vcpu_pt_hwdom when PV
> dom0 support is added, or having to rename opt_pvh_vcpu_pt_hwdom ->
> opt_vcpu_pt_hwdom.

Yes, I think you're right; in the end an option which says, "Turn on
vcpu_pt if dom0 a PVH domain, but not if it's a PV domain" is terribly
useful.

> > diff --git a/docs/misc/xen-command-line.pandoc 
> > b/docs/misc/xen-command-line.pandoc
> > index a6f08463d1..839d06da26 100644
> > --- a/docs/misc/xen-command-line.pandoc
> > +++ b/docs/misc/xen-command-line.pandoc
> > @@ -202,6 +202,35 @@ to appropriate auditing by Xen.  Argo is disabled by 
> > default.
> >      This option is disabled by default, to protect domains from a DoS by a
> >      buggy or malicious other domain spamming the ring.
> >
> > +### asi (x86)
> > +> `= List of [ <bool>, {pv,hvm,dom0}=<bool>,
> > +>              vcpu-pt=<bool> | vcpu-pt=hvm=<bool> ]`
> > +
> > +> Default: `false`
> > +
> > +Offers control over whether the hypervisor will engage in Address Space
> > +Isolation, by not having potentially sensitive information permanently 
> > mapped
> > +in the VMM page-tables.  Using this option might avoid the need to apply
> > +mitigations for certain speculative related attacks, at the cost of mapping
> > +sensitive information on-demand.  It might also offer some protection 
> > against
> > +unmitigated speculation-related attacks.
> > +
> > +The mechanisms currently implemented apply to HVM domains, including a PVH
> > +hardware domain.  PV domains, including a PV hardware domain, are 
> > unaffected.
>
> I think we might want to drop this.  It's an incomplete in-development
> feature: we don't need to mention explicitly what is and what is not
> implemented in the command line documentation for the time being.

This seems to me a kind of difference in taste.  I prefer the
documentation to be always accurate according to that state in the
tree; having the documentation change as the code changes helps you
see at a big picture how things are progressing.  In any case, I doubt
we'll get vcpu_pt implemented by the next release, so we'll want a
statement like this in there anyway, won't we?

> > +* `pv=`, `hvm=` and `dom0=` enable the features for PV domains or HVM 
> > domains
> > +  other than the hardware domain, or for the hardware domain, PV or PVH.  
> > The
> > +  plain boolean does all three: `asi=1,dom0=0` leaves the hardware domain
> > +  out.  The forms naming no domain type (an un-suffixed `vcpu-pt=<bool>`)
> > +  cover the hardware domain too.
> > +
> > +**WARNING: manual de-selection of enabled options will invalidate any
> > +protection offered by the feature.  The fine grained options provided below
> > +are meant to be used for debugging purposes only.**
> > +
> > +* `vcpu-pt` gives each vCPU its own per-domain area: the per-domain slot of
> > +  each vCPU's monitor table maps a region private to that vCPU.
> > +
> >  ### asid (x86)
> >  > `= <boolean>`
> >
> > diff --git a/xen/arch/x86/include/asm/domain.h 
> > b/xen/arch/x86/include/asm/domain.h
> > index 212daae1f5..12f79feb42 100644
> > --- a/xen/arch/x86/include/asm/domain.h
> > +++ b/xen/arch/x86/include/asm/domain.h
> > @@ -474,6 +474,12 @@ struct arch_domain
> >      /* Don't unconditionally inject #GP for unhandled MSRs. */
> >      bool msr_relaxed;
> >
> > +    /* Fine grained ASI related options. */
> > +    struct {
> > +        /* Give each vCPU its own per-domain area (HVM only). */
>
> I would drop the HVM only part, it's a transient implementation
> limitation, not a design one.

Same idea: It's true as of this patch, and it's likely to be true in
the next release.  Better to document it rather than have someone
search down the implementation and determine that it's ignored for PV.

> > @@ -487,6 +491,69 @@ static int __init cf_check parse_pv_l1tf(const char *s)
> >  }
> >  custom_param("pv-l1tf", parse_pv_l1tf);
> >
> > +static int __init cf_check parse_asi(const char *s)
> > +{
> > +    const char *ss;
> > +    int val, rc = 0;
> > +
> > +    /* Interpret 'asi' alone in its positive boolean form. */
> > +    if ( *s == '\0' )
> > +        opt_pvh_vcpu_pt_hwdom = opt_vcpu_pt_hvm = true;
> > +
> > +    do {
> > +        ss = strchr(s, ',');
> > +        if ( !ss )
> > +            ss = strchr(s, '\0');
> > +
> > +        val = parse_bool(s, ss);
> > +        switch ( val )
> > +        {
> > +        case 0:
> > +        case 1:
> > +            opt_pvh_vcpu_pt_hwdom = opt_vcpu_pt_hvm = val;
> > +            break;
> > +
> > +        default:
> > +            if ( (val = parse_boolean("pv", s, ss)) >= 0 )
> > +                /* PV domains are unaffected. */;
>
> You might want to print a message saying that the option is ignored
> for PV?  It will go into the console buffer at least.

Yes, I think so.

> > @@ -680,6 +747,13 @@ static void __init print_details(enum ind_thunk thunk)
> >             opt_pv_l1tf_hwdom ? "enabled"  : "disabled",
> >             opt_pv_l1tf_domu  ? "enabled"  : "disabled");
> >  #endif
> > +
> > +    printk("  ASI features for Dom0:%s\n",
> > +           opt_dom0_pvh && opt_pvh_vcpu_pt_hwdom ? " vCPU-PT" : " None");
> > +#ifdef CONFIG_HVM
> > +    printk("  ASI features for HVM VMs:%s\n",
> > +           opt_vcpu_pt_hvm                       ? " vCPU-PT" : " None");
> > +#endif
>
> opt_vcpu_pt_hvm is defined unconditionally, so you might as well use:
>
> if ( IS_ENABLED(CONFIG_HVM) )
>     printk("  ASI features for HVM VMs:%s\n", ...
>
> Which is a bit nicer than the ifdefs?

Ack

> > @@ -2158,6 +2235,11 @@ void __init init_speculation_mitigations(void)
> >           hw_smt_enabled && default_xen_spec_ctrl )
> >          setup_force_cpu_cap(X86_FEATURE_SC_MSR_IDLE);
> >
> > +    if ( opt_pvh_vcpu_pt_hwdom || opt_vcpu_pt_hvm )
> > +        warning_add(
> > +            "Address Space Isolation is not functional, this option is\n"
> > +            "intended to be used only for development purposes.\n");
>
> I wonder, could we add the warning in parse_asi() itself?  I know if
> would then also trigger for asi=0, but would avoid having to remember
> to expand the condition here every time a new option is added?

I'm OK with that.

 -George