Re: [PATCH 1/3] x86/intel: expose IPRED_CTRL to guests

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

On Tue, Jan 30, 2024 at 11:57:17AM +0100, Jan Beulich wrote:
> On 30.01.2024 10:13, Roger Pau Monne wrote:
> > The CPUID feature bit signals the presence of the IPRED_DIS_{U,S} controls 
> > in
> > SPEC_CTRL MSR.
> > 
> > Note that those controls are not used by the hypervisor.
> 
> Despite this, ...
> 
> > --- a/xen/arch/x86/msr.c
> > +++ b/xen/arch/x86/msr.c
> > @@ -324,6 +324,9 @@ uint64_t msr_spec_ctrl_valid_bits(const struct 
> > cpu_policy *cp)
> >      return (SPEC_CTRL_IBRS | SPEC_CTRL_STIBP |
> >              (ssbd       ? SPEC_CTRL_SSBD       : 0) |
> >              (psfd       ? SPEC_CTRL_PSFD       : 0) |
> > +            (cp->feat.ipred_ctrl ? (SPEC_CTRL_IPRED_DIS_U |
> > +                                    SPEC_CTRL_IPRED_DIS_S)
> > +                                 : 0) |
> >              0);
> >  }
> 
> ... if I'm not mistaken exposing SPEC_CTRL bits to guests is independent
> of whether we write SPEC_CTRL on entry to Xen. Therefore I think in the
> description it wants clarifying why it is acceptable to run Xen with the
> guest chosen settings for at least the DIS_S bit (assuming that it is
> okay to do so). Likely (didn't look there yet) also applicable to the
> further two patches.

"The added feature is made dependent on IBRSB, which ensures it will
only be exposed if X86_FEATURE_SC_MSR_{PV,HVM} is available, and that
ensures the value of SPEC_CTRL will get context switched on exit/entry
to guest."

Would adding the above to the commit message clarify the intended
implementation?

Thanks, Roger.