RE: [PATCH v2 11/35] x86/bugs: Restructure spectre_v1 mitigation

[Kaplan, David]

[AMD Official Use Only - AMD Internal Distribution Only]

> -----Original Message-----
> From: Kaplan, David
> Sent: Thursday, November 14, 2024 9:37 AM
> To: Pawan Gupta <pawan.kumar.gupta@xxxxxxxxxxxxxxx>
> Cc: Thomas Gleixner <tglx@xxxxxxxxxxxxx>; Borislav Petkov <bp@xxxxxxxxx>; Peter
> Zijlstra <peterz@xxxxxxxxxxxxx>; Josh Poimboeuf <jpoimboe@xxxxxxxxxx>; Ingo
> Molnar <mingo@xxxxxxxxxx>; Dave Hansen <dave.hansen@xxxxxxxxxxxxxxx>;
> x86@xxxxxxxxxx; H . Peter Anvin <hpa@xxxxxxxxx>; linux-kernel@xxxxxxxxxxxxxxx
> Subject: RE: [PATCH v2 11/35] x86/bugs: Restructure spectre_v1 mitigation
>
>
>
> > -----Original Message-----
> > From: Pawan Gupta <pawan.kumar.gupta@xxxxxxxxxxxxxxx>
> > Sent: Thursday, November 14, 2024 12:57 AM
> > To: Kaplan, David <David.Kaplan@xxxxxxx>
> > Cc: Thomas Gleixner <tglx@xxxxxxxxxxxxx>; Borislav Petkov
> > <bp@xxxxxxxxx>; Peter Zijlstra <peterz@xxxxxxxxxxxxx>; Josh Poimboeuf
> > <jpoimboe@xxxxxxxxxx>; Ingo Molnar <mingo@xxxxxxxxxx>; Dave Hansen
> > <dave.hansen@xxxxxxxxxxxxxxx>; x86@xxxxxxxxxx; H . Peter Anvin
> > <hpa@xxxxxxxxx>; linux-kernel@xxxxxxxxxxxxxxx
> > Subject: Re: [PATCH v2 11/35] x86/bugs: Restructure spectre_v1
> > mitigation
> >
> > Caution: This message originated from an External Source. Use proper
> > caution when opening attachments, clicking links, or responding.
> >
> >
> > On Tue, Nov 05, 2024 at 03:54:31PM -0600, David Kaplan wrote:
> > >  static void __init spectre_v1_select_mitigation(void)
> > >  {
> > > -     if (!boot_cpu_has_bug(X86_BUG_SPECTRE_V1) || cpu_mitigations_off()) {
> > > +     if (!boot_cpu_has_bug(X86_BUG_SPECTRE_V1) ||
> > > + cpu_mitigations_off())
> > >               spectre_v1_mitigation = SPECTRE_V1_MITIGATION_NONE;
> > > +}
> > > +
> > > +static void __init spectre_v1_apply_mitigation(void) {
> > > +     if (!boot_cpu_has_bug(X86_BUG_SPECTRE_V1) ||
> > > +cpu_mitigations_off())
> >
> > We probably don't need to repeat this check, is this okay:
> >
> >         if (spectre_v1_mitigation == SPECTRE_V1_MITIGATION_NONE)
> > >               return;
> > > -     }
> > >
> > >       if (spectre_v1_mitigation == SPECTRE_V1_MITIGATION_AUTO) {
>
> I don't think so.  That would stop us from printing the message about the system
> being vulnerable at the end of the function.
>
> We should only not print the message I believe if the CPU is actually not vulnerable
> or mitigations are globally disabled.  Although now I realize my patches may not be
> suppressing the print statements always if cpu_mitigations_off(), so I need to go
> and fix that.
>

Actually looks like the existing code wasn't always consistent here.  For srbds, ssb, and gds, it would still print a message about the system being vulnerable even if mitigations=off was passed.  But for the others it would not print a message.  I think I'm going to suppress the message for all cases, but if people feel it should be the other way, let me know.

--David Kaplan