RE: [PATCH] x86: Prefer MWAIT over HALT on AMD processors

[Limonciello, Mario]

[Public]



> -----Original Message-----
> From: Dave Hansen <dave.hansen@xxxxxxxxx>
> Sent: Tuesday, April 5, 2022 10:47
> To: Limonciello, Mario <Mario.Limonciello@xxxxxxx>; Peter Zijlstra
> <peterz@xxxxxxxxxxxxx>; Karny, Wyes <Wyes.Karny@xxxxxxx>
> Cc: linux-kernel@xxxxxxxxxxxxxxx; Carroll, Lewis <Lewis.Carroll@xxxxxxx>;
> Shenoy, Gautham Ranjal <gautham.shenoy@xxxxxxx>; Narayan, Ananth
> <Ananth.Narayan@xxxxxxx>; Rao, Bharata Bhasker <bharata@xxxxxxx>;
> len.brown@xxxxxxxxx; x86@xxxxxxxxxx; tglx@xxxxxxxxxxxxx;
> mingo@xxxxxxxxxx; bp@xxxxxxxxx; dave.hansen@xxxxxxxxxxxxxxx;
> hpa@xxxxxxxxx; chang.seok.bae@xxxxxxxxx; keescook@xxxxxxxxxxxx;
> metze@xxxxxxxxx; zhengqi.arch@xxxxxxxxxxxxx; mark.rutland@xxxxxxx
> Subject: Re: [PATCH] x86: Prefer MWAIT over HALT on AMD processors
> 
> On 4/5/22 08:34, Limonciello, Mario wrote:
> >>>         if (!static_cpu_has(X86_FEATURE_ZEN)) {
> >>>                 msr |= ssbd_tif_to_amd_ls_cfg(tifn);
> >>>                 wrmsrl(MSR_AMD64_LS_CFG, msr);
> >>>                 return;
> >>>         }
> >> This seem _bit_ at odds with the commit message (and the AMD SSBD
> >> whitepaper):
> >>
> >>>     Add the necessary synchronization logic for AMD family 17H.
> >> So, is X86_FEATURE_ZEN for family==0x17, or family>=0x17?
> > There are Zen family CPUs and APUs from family 0x19.  Perhaps at the
> > time of the whitepaper there weren't yet though.
> 
> Is this a gap in the documentation, then?  Is there some documentation
> of the availability of SSBD mitigations on family 0x19?

It looks like a misinterpretation of the document.

Notice it mentions in Non-architectural MSRs explicitly:

"some models of family 17h have logic that allow loads to bypass older stores 
but lack the architectural SPEC_CTRL or VIRT_SPEC_CTR"

But that is not for all family 17h nor for family 19h.  You can see earlier in
the document the method to detect presence for SSBD which applies to the
rest of 17h and 19h.  That code in amd_set_core_ssb_state is only used for
one of the mitigation codepaths without MSR support, not for all Zen CPUs.

> 
> Anyway, back to MWAIT...  It would be really great to include the
> assumptions about what X86_FEATURE_ZEN means in the context of this
> patch.  Does this patch *really* mean "Zen microarchitecture" only?