On Thu, Jan 09, 2025 at 02:58:29PM +0000, Alejandro Vallejo wrote:
> On Wed Jan 8, 2025 at 2:26 PM GMT, Roger Pau Monne wrote:
> > No functional change, as the option is not used.
> >
> > Introduced new so newly added functionality is keyed on the option being
> > enabled, even if the feature is non-functional.
> >
> > When ASI is enabled for PV domains, printing the usage of XPTI might be
> > omitted if it must be uniformly disabled given the usage of ASI.
> >
> > Signed-off-by: Roger Pau Monné <roger....@citrix.com>
> > ---
> > Changes since v1:
> > - Improve comments and documentation about what ASI provides.
> > - Do not print the XPTI information if ASI is used for pv domUs and dom0 is
> > PVH, or if ASI is used for both domU and dom0.
> >
> > FWIW, I would print the state of XPTI uniformly, as otherwise I find the
> > output
> > might be confusing for user expecting to assert the state of XPTI.
> > ---
> > docs/misc/xen-command-line.pandoc | 19 +++++
> > xen/arch/x86/include/asm/domain.h | 3 +
> > xen/arch/x86/include/asm/spec_ctrl.h | 2 +
> > xen/arch/x86/spec_ctrl.c | 115 +++++++++++++++++++++++++--
> > 4 files changed, 133 insertions(+), 6 deletions(-)
> >
> > diff --git a/docs/misc/xen-command-line.pandoc
> > b/docs/misc/xen-command-line.pandoc
> > index 08b0053f9ced..3c1ad7b5fe7d 100644
> > --- a/docs/misc/xen-command-line.pandoc
> > +++ b/docs/misc/xen-command-line.pandoc
> > @@ -202,6 +202,25 @@ 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}=<bool>,
> > + {vcpu-pt}=<bool>|{pv,hvm}=<bool> ]`
>
> nit: While this grows later, the braces around vcpu-pt aren't strictly needed
> here.
Since I have to modify the whole line I can indeed add the braces
later.
> > +
> > +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.
>
> Might be worth mentioning that this provides some defense in depth against
> unmitigated attacks too.
It's IMO a bit too vague to make such promises, but I can add:
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.
> > +
> > +* `pv=` and `hvm=` sub-options allow enabling for specific guest types.
> > +
> > +**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` ensure each vCPU uses a unique top-level page-table and setup a
> > + virtual address space region to map memory on a per-vCPU basis.
> > +
> > ### asid (x86)
> > > `= <boolean>`
> >
> > diff --git a/xen/arch/x86/spec_ctrl.c b/xen/arch/x86/spec_ctrl.c
> > index ced84750015c..9463a8624701 100644
> > --- a/xen/arch/x86/spec_ctrl.c
> > +++ b/xen/arch/x86/spec_ctrl.c
> > @@ -2075,6 +2165,19 @@ void __init init_speculation_mitigations(void)
> > hw_smt_enabled && default_xen_spec_ctrl )
> > setup_force_cpu_cap(X86_FEATURE_SC_MSR_IDLE);
> >
> > + /* Disable all ASI options by default until feature is finished. */
> > + if ( opt_vcpu_pt_pv == -1 )
> > + opt_vcpu_pt_pv = 0;
> > + if ( opt_vcpu_pt_hwdom == -1 )
> > + opt_vcpu_pt_hwdom = 0;
> > + if ( opt_vcpu_pt_hvm == -1 )
> > + opt_vcpu_pt_hvm = 0;
>
> Why not preinitialise them to zero instead in the static declarations?
Hm, indeed. I can probably make them booleans then. I wrongly
recall that checking whether they haven't been initialized was needed
somewhere, but that doesn't seem to be the case.
Thanks, Roger.
