On 17.08.2021 16:30, Andrew Cooper wrote: > The opencoded legacy Memory Disambiguation logic in init_amd() neglected > Fam19h for the Zen3 microarchitecture. > > In practice, all Zen2 based system (AMD Fam17h Model >= 0x30 and Hygon Fam18h > Model >= 0x4) have the architectural MSR_SPEC_CTRL and the SSBD bit within it. > > Implement the algorithm given in AMD's SSBD whitepaper, and leave a > printk_once() behind in the case that no controls can be found. > > This now means that a user choosing `spec-ctrl=no-ssb` will actually turn off > Memory Disambiguation on Fam19h/Zen3 systems.
Aiui you mean `spec-ctrl=no-ssbd` here? And the effect would then be to turn _on_ Memory Disambiguation, unless the original comment was the wrong way round? I'm also concerned by this behavioral change: I think opt_ssbd would want to become a tristate, such that not specifying the option at all will not also result in turning the bit off even if it was on for some reason (firmware?). Similarly "spec-ctrl=no" and "spec-ctrl=no-xen" imo shouldn't have this effect. > --- a/xen/arch/x86/cpu/amd.c > +++ b/xen/arch/x86/cpu/amd.c > @@ -681,6 +681,56 @@ void amd_init_lfence(struct cpuinfo_x86 *c) > c->x86_capability); > } > > +/* > + * Refer to the AMD Speculative Store Bypass whitepaper: > + * > https://developer.amd.com/wp-content/resources/124441_AMD64_SpeculativeStoreBypassDisable_Whitepaper_final.pdf > + */ > +void amd_init_ssbd(const struct cpuinfo_x86 *c) > +{ > + int bit = -1; > + > + if (cpu_has_ssb_no) > + return; > + > + if (cpu_has_amd_ssbd) { > + wrmsrl(MSR_SPEC_CTRL, opt_ssbd ? SPEC_CTRL_SSBD : 0); > + return; > + } > + > + if (cpu_has_virt_ssbd) { > + wrmsrl(MSR_VIRT_SPEC_CTRL, opt_ssbd ? SPEC_CTRL_SSBD : 0); > + return; > + } > + > + switch (c->x86) { > + case 0x15: bit = 54; break; > + case 0x16: bit = 33; break; > + case 0x17: > + case 0x18: bit = 10; break; > + } > + > + if (bit >= 0) { > + uint64_t val, mask = 1ull << bit; > + > + if (rdmsr_safe(MSR_AMD64_LS_CFG, val) || > + ({ > + val &= ~mask; > + if ( opt_ssbd ) Nit: No spaces inside the parentheses here. Jan
