linux-kernel - Re: [PATCH] cpu/hotplug: Create SMT sysfs interface for all arches

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: <20190326180348.fzerpltlf62dnurd@treble>
Date:   Tue, 26 Mar 2019 13:03:48 -0500
From:   Josh Poimboeuf <jpoimboe@...hat.com>
To:     Thomas Gleixner <tglx@...utronix.de>
Cc:     linux-kernel@...r.kernel.org, x86@...nel.org,
        Andrea Arcangeli <aarcange@...hat.com>,
        Waiman Long <longman@...hat.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Jiri Kosina <jikos@...nel.org>
Subject: Re: [PATCH] cpu/hotplug: Create SMT sysfs interface for all arches

On Tue, Mar 26, 2019 at 02:53:04PM +0100, Thomas Gleixner wrote:
> On Tue, 26 Mar 2019, Josh Poimboeuf wrote:
> > On Sun, Mar 24, 2019 at 09:13:18PM +0100, Thomas Gleixner wrote:
> > > Second thoughts. I'm not really convinced that changing the meaning of
> > > notsupported and in fact overloading it, is the right thing to do.
> > > notsupported means now:
> > > 
> > >   CPU does not support it - OR - architecture does not support it
> > > 
> > > That's not pretty and we are surely not short of state space. There are
> > > several options for handling this:
> > > 
> > >  1) Do not expose the state file, just expose the active file
> > > 
> > >  2) Expose the state file, but return -ENOTSUPP or some other sensible error
> > >     code
> > > 
> > >  3) Expose the state file and let show return 'notimplemented' which is
> > >     more accurate. That wouldn't even require to expand the state space
> > >     enum. It just can be returned unconditionally.
> > 
> > Makes sense.  I like #3.  I can post another version.
> 
> Yes, please.

Something like so (on top of the original patch)?

diff --git a/Documentation/ABI/testing/sysfs-devices-system-cpu b/Documentation/ABI/testing/sysfs-devices-system-cpu
index 4a11cba73085..5eea46fefcb2 100644
--- a/Documentation/ABI/testing/sysfs-devices-system-cpu
+++ b/Documentation/ABI/testing/sysfs-devices-system-cpu
@@ -511,11 +511,12 @@ Description:	Control Symetric Multi Threading (SMT)
 		control: Read/write interface to control SMT. Possible
 			 values:
 
-			 "on"		SMT is enabled
-			 "off"		SMT is disabled
-			 "forceoff"	SMT is force disabled. Cannot be changed.
-			 "notsupported" Runtime SMT toggling is not currently
-					supported for the architecture
+			 "on"		  SMT is enabled
+			 "off"		  SMT is disabled
+			 "forceoff"	  SMT is force disabled. Cannot be changed.
+			 "notsupported"   SMT is not supported by the CPU
+			 "notimplemented" SMT runtime toggling is not
+					  implemented for the architecture
 
 			 If control status is "forceoff" or "notsupported" writes
 			 are rejected.
diff --git a/kernel/cpu.c b/kernel/cpu.c
index 517ab1803a22..05a71ee98440 100644
--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -2145,7 +2145,11 @@ static const char *smt_states[] = {
 static ssize_t
 show_smt_control(struct device *dev, struct device_attribute *attr, char *buf)
 {
-	return snprintf(buf, PAGE_SIZE - 2, "%s\n", smt_states[cpu_smt_control]);
+	const char *state = IS_ENABLED(CONFIG_HOTPLUG_SMT) ?
+				smt_states[cpu_smt_control] :
+				"notimplemented";
+
+	return snprintf(buf, PAGE_SIZE - 2, "%s\n", state);
 }
 
 static ssize_t