Re: [PATCH 1/1] x86/CPU/AMD: Avoid racy updates to MSR_K7_HWCR in set_cpuid_faulting()

[Jim Mattson]

()

On Tue, Jun 9, 2026 at 2:41 PM Borislav Petkov <bp@xxxxxxxxx> wrote:
>
> On Tue, Jun 09, 2026 at 02:15:47PM -0700, Jim Mattson wrote:
> > +             unsigned long flags;
> > +
> > +             local_irq_save(flags);
> >               if (on)
> >                       msr_set_bit(MSR_K7_HWCR, MSR_K7_HWCR_CPUID_USER_DIS_BIT);
> >               else
> >                       msr_clear_bit(MSR_K7_HWCR, MSR_K7_HWCR_CPUID_USER_DIS_BIT);
> > +             local_irq_restore(flags);
> >       }
>
> Can we carve out this logic that modifies bits in HWCR into a separate
> function and call it from everywhere it needs so that it is very obvious that
> synchronization needs to happen?

Yes, I'd be happy to.

Most HWCR updates are in init code, so they should be fine. Should I
call the helper from init code anyway?

The one potential HWCR update race I am quite uncertain about is:

> static int cpufreq_boost_down_prep(unsigned int cpu)
> {
>         /*
>          * Clear the boost-disable bit on the CPU_DOWN path so that
>          * this cpu cannot block the remaining ones from boosting.
>          */
>         return boost_set_msr(1);
> }

If this can race with boost_set_msr_each(), then the bigger problem is
that boost_set_msr_each() might set the boost-disable bit between the
call to cpufreq_boost_down_prep() and the CPU going offline.

I couldn't convince myself that cpufreq_boost_down_prep() was always
called on the CPU going offline. The call to boost_set_msr() on the
current CPU suggests it is, but the 'cpu' argument suggests it isn't.
If policies are shared, my reading of __cpufreq_offline() is that
cpufreq_driver->exit() [which calls cpufreq_boost_down_prep()] is only
called when the last CPU sharing the policy goes offline, potentially
leaving CPB_DIS set on the rest.

Anyway, since boost_set_msr() is called from both process context and
interrupt context, perhaps I should use the helper in boost_set_msr()
as well, just for consistency.