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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 22.04.2022 11:55, Roger Pau Monné wrote:
> 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."

Thanks.

> 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.

Right, better be on the safe side.

Jan