Re: [PATCH v3 3/3] amd/msr: implement VIRT_SPEC_CTRL for HVM guests using legacy SSBD

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

On Thu, Apr 21, 2022 at 05:27:18PM +0200, Jan Beulich wrote:
> On 21.04.2022 17:21, Roger Pau Monné wrote:
> > On Thu, Apr 21, 2022 at 11:50:16AM +0200, Jan Beulich wrote:
> >> On 31.03.2022 11:27, Roger Pau Monne wrote:
> >>> Expose VIRT_SSBD to guests if the hardware supports setting SSBD in
> >>> the LS_CFG MSR (a.k.a. non-architectural way). Different AMD CPU
> >>> families use different bits in LS_CFG, so exposing VIRT_SPEC_CTRL.SSBD
> >>> allows for an unified way of exposing SSBD support to guests on AMD
> >>> hardware that's compatible migration wise, regardless of what
> >>> underlying mechanism is used to set SSBD.
> >>>
> >>> Note that on AMD Family 17h (Zen 1) the value of SSBD in LS_CFG is
> >>> shared between threads on the same core, so there's extra logic in
> >>> order to synchronize the value and have SSBD set as long as one of the
> >>> threads in the core requires it to be set. Such logic also requires
> >>> extra storage for each thread state, which is allocated at
> >>> initialization time.
> >>
> >> So where exactly is the boundary? If I'm not mistaken Zen2 is also
> >> Fam17 (and only Zen3 is Fam19), yet here and elsewhere you look to
> >> take Zen1 == Fam17.
> > 
> > Right, but Zen2 already has AMD_SSBD (ie: SPEC_CTRL), so it's not
> > using this logic.
> > 
> > The AMD whitepaper is more clear about this: any Fam17h processor that
> > is using the non-architectural MSRs to set SSBD and has more than 1
> > logical processor for each logical core must synchronize the setting
> > of SSBD.
> > 
> > I think just dropping the mention of Zen 1 in the commit message
> > should remove your concerns?
> 
> Or keep it, but qualify it by saying that Zen2 isn't expected to take
> this path because of having SSBD. But iirc SSBD was introduced to
> Zen2 only by a ucode update, so such a description should not be wrong
> wrt such not-up-to-date systems.

FTAOD I've worded this as:

"Note that on AMD Family 17h and Hygon Family 18h processors the value
of SSBD in LS_CFG is shared between threads on the same core, so
there's extra logic in order to synchronize the value and have SSBD
set as long as one of the threads in the core requires it to be set.
Such logic also requires extra storage for each thread state, which is
allocated at initialization time."

Which I think is correct in all cases.  Iff Zen2 was to resort to
using the non-architectural way of setting SSBD (if that's even
possible) it should synchronize it between threads according to my
read of the AMD whitepaper.

I've also added handling for Hygon Fam18h, seeing as those also make
use of the non-architectural way of setting SSBD.

Thanks, Roger.