On 26.03.2024 12:33, Andrew Cooper wrote:
> On 26/03/2024 9:13 am, Jan Beulich wrote:
>> On 25.03.2024 19:18, Andrew Cooper wrote:
>>> + /* Avoid reading BP_CFG if we don't intend to change anything. */
>>> + if (!new)
>>> return;
>>>
>>> rdmsrl(MSR_AMD64_BP_CFG, val);
>>>
>>> - if (val & chickenbit)
>>> + if ((val & new) == new)
>>> return;
>> Since bits may also need turning off:
>>
>> if (!((val ^ new) & (BP_CFG_SPEC_REDUCE | (1 << 5))))
>> return;
>>
>> and the !new early-out dropped, too? Looks like this wasn't quite right
>> before, either.
>
> That's adding unnecessary complexity. It's unlikely that we'll ever
> need to clear bits like this.
Okay. Half a sentence in a comment would be nice, to make clear the
behavior is intended to only ever set bits.
>>> @@ -1078,22 +1082,41 @@ static void __init ibpb_calculations(void)
>>> * Confusion. Mitigate with IBPB-on-entry.
>>> */
>>> if ( !boot_cpu_has(X86_FEATURE_BTC_NO) )
>>> - def_ibpb_entry = true;
>>> + def_ibpb_entry_pv = def_ibpb_entry_hvm = true;
>>>
>>> /*
>>> - * Further to BTC, Zen3/4 CPUs suffer from Speculative Return Stack
>>> - * Overflow in most configurations. Mitigate with IBPB-on-entry
>>> if we
>>> - * have the microcode that makes this an effective option.
>>> + * Further to BTC, Zen3 and later CPUs suffer from Speculative
>>> Return
>>> + * Stack Overflow in most configurations. Mitigate with
>>> IBPB-on-entry
>>> + * if we have the microcode that makes this an effective option,
>>> + * except where there are other mitigating factors available.
>>> */
>> Hmm, is "Zen3 and later" really appropriate?
>
> Yes.
>
> SRSO isn't fixed until SRSO_NO is enumerated.
IOW even on Zen5 that's going to be only by ucode update?
>>> --- a/xen/include/public/arch-x86/cpufeatureset.h
>>> +++ b/xen/include/public/arch-x86/cpufeatureset.h
>>> @@ -304,7 +304,9 @@ XEN_CPUFEATURE(FSRSC, 11*32+19) /*A Fast
>>> Short REP SCASB */
>>> XEN_CPUFEATURE(AMD_PREFETCHI, 11*32+20) /*A PREFETCHIT{0,1}
>>> Instructions */
>>> XEN_CPUFEATURE(SBPB, 11*32+27) /*A Selective Branch
>>> Predictor Barrier */
>>> XEN_CPUFEATURE(IBPB_BRTYPE, 11*32+28) /*A IBPB flushes Branch Type
>>> predictions too */
>>> -XEN_CPUFEATURE(SRSO_NO, 11*32+29) /*A Hardware not vulenrable
>>> to Speculative Return Stack Overflow */
>>> +XEN_CPUFEATURE(SRSO_NO, 11*32+29) /*A Hardware not vulnerable
>>> to Speculative Return Stack Overflow */
>>> +XEN_CPUFEATURE(SRSO_US_NO, 11*32+30) /*A Hardware not vulnerable
>>> to SRSO across the User/Supervisor boundary */
>> Can we validly expose this to 64-bit PV guests, where there's no CPL
>> boundary? Or else isn't my "x86/PV: issue branch prediction barrier
>> when switching 64-bit guest to kernel mode" needed as a prereq?
>
> Based on uarch details, if BP-SPEC-REDUCE is active, we can advertise
> SRSO_US_NO to PV guests.
Which would make it at best !A, for the PV exposure then depending on
the HVM side choice.
Jan
