linux-kernel - Re: [PATCH v1] cpu: Add missing check to cpuhp_smt

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAJZ5v0gsiuK5iFY6cHaqEgP8R1sz_pWGoqac2orYvXqLE2xbDQ@mail.gmail.com>
Date: Fri, 5 Sep 2025 15:27:36 +0200
From: "Rafael J. Wysocki" <rafael@...nel.org>
To: Thomas Gleixner <tglx@...utronix.de>
Cc: LKML <linux-kernel@...r.kernel.org>, Linux PM <linux-pm@...r.kernel.org>, 
	Peter Zijlstra <peterz@...radead.org>, Christian Loehle <christian.loehle@....com>, 
	Dave Hansen <dave.hansen@...ux.intel.com>
Subject: Re: [PATCH v1] cpu: Add missing check to cpuhp_smt_enable()

On Fri, Sep 5, 2025 at 3:13 PM Rafael J. Wysocki <rafael@...nel.org> wrote:
>
> On Fri, Sep 5, 2025 at 9:39 AM Thomas Gleixner <tglx@...utronix.de> wrote:
> >
> > On Fri, Aug 29 2025 at 22:01, Rafael J. Wysocki wrote:
> > > --- a/kernel/cpu.c
> > > +++ b/kernel/cpu.c
> > > @@ -2710,6 +2710,12 @@
> > >       cpu_maps_update_begin();
> > >       cpu_smt_control = CPU_SMT_ENABLED;
> > >       for_each_present_cpu(cpu) {
> > > +             /*
> > > +              * Avoid accidentally onlining primary thread CPUs that have
> > > +              * been taken offline.
> > > +              */
> > > +             if (topology_is_primary_thread(cpu))
> > > +                     continue;
> >
> > Hmm. That's kinda solving the problem, but I think the proper solution
> > would be to implement topology_is_core_online() for x86.
> >
> > The above is preventing the primary thread to be onlined, but then
> > onlines the corresponding hyperthread, which does not really make sense.
>
> Well, manual online can be used for onlining the secondary thread of a
> core where the primary thread is offline, so this is technically
> possible already.
>
> > Something like the completely untested below.
>
> So given the above, shouldn't topology_is_core_online() check if any
> thread in the given core is online?

Besides, this would cause the siblings of offline SMT threads to be
skipped while enabling SMT via sysfs (using
/sys/devices/system/cpu/smt/control), but I'm not sure if this is the
expectation in the field today.  The current behavior is to online all
secondary SMT threads (and more, but that part is quite arguably
broken).

That's why I decided to use a simpler patch which doesn't go that far.