[MODERATED] BUG: disabling SMT from BIOS

[MODERATED] BUG: disabling SMT from BIOS

[Josh Poimboeuf]

When SMT is disabled from BIOS, sysfs confusingly reports that SMT is
enabled:

  $ cat /sys/devices/system/cpu/smt/control 
  on

And it allows turning it 'off' and 'on', though those don't seem to have
any effect.  (Though at least the 'active' file always shows '0'.)

And the 'l1tf' file reports that SMT is vulnerable:

  $ cat /sys/devices/system/cpu/vulnerabilities/l1tf 
  Mitigation: PTE Inversion; VMX: SMT vulnerable, L1D conditional cache flushes

which is obviously incorrect.

-- 
Josh

[MODERATED] Re: BUG: disabling SMT from BIOS

[Jiri Kosina]

On Sun, 22 Jul 2018, speck for Josh Poimboeuf wrote:

> When SMT is disabled from BIOS, sysfs confusingly reports that SMT is
> enabled:
> 
>   $ cat /sys/devices/system/cpu/smt/control 
>   on
> 
> And it allows turning it 'off' and 'on', though those don't seem to have
> any effect.  (Though at least the 'active' file always shows '0'.)
> 
> And the 'l1tf' file reports that SMT is vulnerable:
> 
>   $ cat /sys/devices/system/cpu/vulnerabilities/l1tf 
>   Mitigation: PTE Inversion; VMX: SMT vulnerable, L1D conditional cache flushes
> 
> which is obviously incorrect.

I might not have time to test this properly today, but I think this should 
cure it, right?



From: Jiri Kosina <jkosina@suse.cz>
Subject: [PATCH] x86, cpu/hotplug: fix topology_smt_supported()

Make sure topology_smt_supported() takes into account the detection 
performed by set_cpu_sibling_map() in order to properly report whether the 
current platform really does support SMT.

Signed-off-by: Jiri Kosina <jkosina@suse.cz>
---
 arch/x86/kernel/smpboot.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index 7f7def989fb0..afe4d1ee8c87 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -279,7 +279,7 @@ bool topology_is_primary_thread(unsigned int cpu)
  */
 bool topology_smt_supported(void)
 {
-	return smp_num_siblings > 1;
+	return topology_max_smt_threads() > 1;
 }
 
 /**

-- 
Jiri Kosina
SUSE Labs

[MODERATED] Re: BUG: disabling SMT from BIOS

[Peter Zijlstra]

On Mon, Jul 23, 2018 at 10:06:45AM +0200, speck for Jiri Kosina wrote:
> On Sun, 22 Jul 2018, speck for Josh Poimboeuf wrote:
> 
> > When SMT is disabled from BIOS, sysfs confusingly reports that SMT is
> > enabled:
> > 
> >   $ cat /sys/devices/system/cpu/smt/control 
> >   on
> > 
> > And it allows turning it 'off' and 'on', though those don't seem to have
> > any effect.  (Though at least the 'active' file always shows '0'.)
> > 
> > And the 'l1tf' file reports that SMT is vulnerable:
> > 
> >   $ cat /sys/devices/system/cpu/vulnerabilities/l1tf 
> >   Mitigation: PTE Inversion; VMX: SMT vulnerable, L1D conditional cache flushes
> > 
> > which is obviously incorrect.
> 
> I might not have time to test this properly today, but I think this should 
> cure it, right?

Yes that looks about right, smp_num_siblings is the max number of
siblings the CPUID topology information _can_ tell is about.
topology_max_smt_threads() is howver many we've actually seen during
enumeration.

If someone could confirm, I'll see if I can work this magic Thomas left
me :-)

[MODERATED] Re: BUG: disabling SMT from BIOS

[Jiri Kosina]

On Mon, 23 Jul 2018, speck for Peter Zijlstra wrote:

> Yes that looks about right, smp_num_siblings is the max number of 
> siblings the CPUID topology information _can_ tell is about. 

Exactly. And I think that's pretty much constant on a given system no 
matter the BIOS settings, contrary to the dynamically performed topology 
enumeration.

> topology_max_smt_threads() is howver many we've actually seen during 
> enumeration.
> 
> If someone could confirm, I'll see if I can work this magic Thomas left
> me :-)

Thanks; please also add

	Fixes: f048c399e0f74 ("x86/topology: Provide topology_smt_supported()")

once/if applying, I forgot to include that in a rush.

-- 
Jiri Kosina
SUSE Labs

[MODERATED] Re: BUG: disabling SMT from BIOS

[Josh Poimboeuf]

On Mon, Jul 23, 2018 at 10:44:10AM +0200, speck for Peter Zijlstra wrote:
> On Mon, Jul 23, 2018 at 10:06:45AM +0200, speck for Jiri Kosina wrote:
> > On Sun, 22 Jul 2018, speck for Josh Poimboeuf wrote:
> > 
> > > When SMT is disabled from BIOS, sysfs confusingly reports that SMT is
> > > enabled:
> > > 
> > >   $ cat /sys/devices/system/cpu/smt/control 
> > >   on
> > > 
> > > And it allows turning it 'off' and 'on', though those don't seem to have
> > > any effect.  (Though at least the 'active' file always shows '0'.)
> > > 
> > > And the 'l1tf' file reports that SMT is vulnerable:
> > > 
> > >   $ cat /sys/devices/system/cpu/vulnerabilities/l1tf 
> > >   Mitigation: PTE Inversion; VMX: SMT vulnerable, L1D conditional cache flushes
> > > 
> > > which is obviously incorrect.
> > 
> > I might not have time to test this properly today, but I think this should 
> > cure it, right?
> 
> Yes that looks about right, smp_num_siblings is the max number of
> siblings the CPUID topology information _can_ tell is about.
> topology_max_smt_threads() is howver many we've actually seen during
> enumeration.
> 
> If someone could confirm, I'll see if I can work this magic Thomas left
> me :-)

NACK, this permanently disables SMT on my system, regardless of the BIOS
settings.  I suspect it needs to be called later, after the other CPUs
have been brought online.

-- 
Josh

[MODERATED] Re: BUG: disabling SMT from BIOS

[Jiri Kosina]

On Mon, 23 Jul 2018, speck for Josh Poimboeuf wrote:

> This permanently disables SMT on my system, regardless of the BIOS 
> settings.

I see, we're setting _NOT_SUPPORTED before the sibling map has been 
established.

> I suspect it needs to be called later, after the other CPUs have been 
> brought online.

Either that, but that will basically mean redefining the CPU_SMT_ENABLED 
meaning, and I actually think the current semantics makes more sense.

How about this instead? (still untested)



From: Jiri Kosina <jkosina@suse.cz>
Subject: [PATCH] x86/speculation/l1tf: properly report SMT vulnerable status

Even if SMT has not been disabled from the OS and CPUID indicates
it's supported, we should report as SMT-vulnerable iff there really
is more than one SMT sibling (consider "HT disabled in BIOS" case).

Signed-off-by: Jiri Kosina <jkosina@suse.cz>
---
 arch/x86/kernel/cpu/bugs.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
index fa6123bdd032..023bcb4b8244 100644
--- a/arch/x86/kernel/cpu/bugs.c
+++ b/arch/x86/kernel/cpu/bugs.c
@@ -760,7 +760,8 @@ static ssize_t l1tf_show_state(char *buf)
 		return sprintf(buf, "%s\n", L1TF_DEFAULT_MSG);
 
 	return sprintf(buf, "%s; VMX: SMT %s, L1D %s\n", L1TF_DEFAULT_MSG,
-		       cpu_smt_control == CPU_SMT_ENABLED ? "vulnerable" : "disabled",
+		       (cpu_smt_control == CPU_SMT_ENABLED && topology_max_smt_threads() > 1)
+			? "vulnerable" : "disabled",
 		       l1tf_vmx_states[l1tf_vmx_mitigation]);
 }
 #else

-- 
Jiri Kosina
SUSE Labs

[MODERATED] Re: BUG: disabling SMT from BIOS

[Josh Poimboeuf]

On Mon, Jul 23, 2018 at 04:03:50PM +0200, speck for Jiri Kosina wrote:
> On Mon, 23 Jul 2018, speck for Josh Poimboeuf wrote:
> 
> > This permanently disables SMT on my system, regardless of the BIOS 
> > settings.
> 
> I see, we're setting _NOT_SUPPORTED before the sibling map has been 
> established.
> 
> > I suspect it needs to be called later, after the other CPUs have been 
> > brought online.
> 
> Either that, but that will basically mean redefining the CPU_SMT_ENABLED 
> meaning, and I actually think the current semantics makes more sense.
> 
> How about this instead? (still untested)

That seems like only half the fix, because the 'control' file still
allows you to pretend to turn SMT off and on.

How about this one?

-----

From: Josh Poimboeuf <jpoimboe@redhat.com>
Subject: [PATCH] cpu/hotplug: detect SMT disabled by BIOS

If SMT is disabled in BIOS, the CPU code doesn't properly detect it.
The /sys/devices/system/cpu/smt/control file shows 'on', and the 'l1tf'
vulnerabilities file shows SMT as vulnerable.

Fix it by forcing 'cpu_smt_control' to CPU_SMT_NOT_SUPPORTED in such a
case.  Unfortunately the detection can only be done after bringing all
the CPUs online, so we have to overwrite any previous writes to the
variable.

Reported-by: Joe Mario <jmario@redhat.com>
Fixes: f048c399e0f7 ("x86/topology: Provide topology_smt_supported()")
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
---
 kernel/cpu.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/kernel/cpu.c b/kernel/cpu.c
index 37eec872042b..2bc3d2f5b2a5 100644
--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -2125,6 +2125,15 @@ static const struct attribute_group cpuhp_smt_attr_group = {
 
 static int __init cpu_smt_state_init(void)
 {
+	/*
+	 * If SMT was disabled by BIOS, detect it here, after the CPUs have
+	 * been brought online.  This ensures the smt/l1tf sysfs entries are
+	 * consistent with reality.  Note this may overwrite cpu_smt_control's
+	 * previous setting.
+	 */
+	if (topology_max_smt_threads() == 1)
+		cpu_smt_control = CPU_SMT_NOT_SUPPORTED;
+
 	return sysfs_create_group(&cpu_subsys.dev_root->kobj,
 				  &cpuhp_smt_attr_group);
 }
-- 
2.17.0

[MODERATED] Re: BUG: disabling SMT from BIOS

[Jiri Kosina]

On Mon, 23 Jul 2018, speck for Josh Poimboeuf wrote:

> From: Josh Poimboeuf <jpoimboe@redhat.com>
> Subject: [PATCH] cpu/hotplug: detect SMT disabled by BIOS
> 
> If SMT is disabled in BIOS, the CPU code doesn't properly detect it.
> The /sys/devices/system/cpu/smt/control file shows 'on', and the 'l1tf'
> vulnerabilities file shows SMT as vulnerable.
> 
> Fix it by forcing 'cpu_smt_control' to CPU_SMT_NOT_SUPPORTED in such a
> case.  Unfortunately the detection can only be done after bringing all
> the CPUs online, so we have to overwrite any previous writes to the
> variable.
> 
> Reported-by: Joe Mario <jmario@redhat.com>
> Fixes: f048c399e0f7 ("x86/topology: Provide topology_smt_supported()")
> Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>

	Tested-by: Jiri Kosina <jkosina@suse.cz>

Thanks,

-- 
Jiri Kosina
SUSE Labs

[MODERATED] Re: BUG: disabling SMT from BIOS

[Peter Zijlstra]

On Tue, Jul 24, 2018 at 09:30:37AM +0200, speck for Jiri Kosina wrote:
> On Mon, 23 Jul 2018, speck for Josh Poimboeuf wrote:
> 
> > From: Josh Poimboeuf <jpoimboe@redhat.com>
> > Subject: [PATCH] cpu/hotplug: detect SMT disabled by BIOS
> > 
> > If SMT is disabled in BIOS, the CPU code doesn't properly detect it.
> > The /sys/devices/system/cpu/smt/control file shows 'on', and the 'l1tf'
> > vulnerabilities file shows SMT as vulnerable.
> > 
> > Fix it by forcing 'cpu_smt_control' to CPU_SMT_NOT_SUPPORTED in such a
> > case.  Unfortunately the detection can only be done after bringing all
> > the CPUs online, so we have to overwrite any previous writes to the
> > variable.
> > 
> > Reported-by: Joe Mario <jmario@redhat.com>
> > Fixes: f048c399e0f7 ("x86/topology: Provide topology_smt_supported()")
> > Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
> 
> 	Tested-by: Jiri Kosina <jkosina@suse.cz>

Thanks guys!

I've pushed this out to speck/{master,linux-4.1[764].y}. In specific,
I've not touched the 4.9 branch since that is dwmw2's baby and I didn't
know if I was supposed to touch that.

If there's anything funny, that's because I'm basically useless at this
git thing, but let me know and I'll try and fix it ;-)

[MODERATED] Re: BUG: disabling SMT from BIOS

[Peter Zijlstra]

On Tue, Jul 24, 2018 at 05:36:27PM +0200, Peter Zijlstra wrote:
> On Tue, Jul 24, 2018 at 09:30:37AM +0200, speck for Jiri Kosina wrote:
> > On Mon, 23 Jul 2018, speck for Josh Poimboeuf wrote:
> > 
> > > From: Josh Poimboeuf <jpoimboe@redhat.com>
> > > Subject: [PATCH] cpu/hotplug: detect SMT disabled by BIOS
> > > 
> > > If SMT is disabled in BIOS, the CPU code doesn't properly detect it.
> > > The /sys/devices/system/cpu/smt/control file shows 'on', and the 'l1tf'
> > > vulnerabilities file shows SMT as vulnerable.
> > > 
> > > Fix it by forcing 'cpu_smt_control' to CPU_SMT_NOT_SUPPORTED in such a
> > > case.  Unfortunately the detection can only be done after bringing all
> > > the CPUs online, so we have to overwrite any previous writes to the
> > > variable.
> > > 
> > > Reported-by: Joe Mario <jmario@redhat.com>
> > > Fixes: f048c399e0f7 ("x86/topology: Provide topology_smt_supported()")
> > > Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
> > 
> > 	Tested-by: Jiri Kosina <jkosina@suse.cz>
> 
> Thanks guys!
> 
> I've pushed this out to speck/{master,linux-4.1[764].y}. In specific,
> I've not touched the 4.9 branch since that is dwmw2's baby and I didn't
> know if I was supposed to touch that.
> 
> If there's anything funny, that's because I'm basically useless at this
> git thing, but let me know and I'll try and fix it ;-)

As Josh noted, I messed up and created new branches instead of updating
the old. I've since deleted these new branches, but every time I try and
push into the existing branches the git thing complains about
non-fast-forward crud and I've given up trying to figure this thing out
for the day.

I'll try again tomorrow..

git and me just ain't gonna be friends it seems.

[MODERATED] Re: BUG: disabling SMT from BIOS

[Peter Zijlstra]

On Tue, Jul 24, 2018 at 06:36:58PM +0200, Peter Zijlstra wrote:
> I'll try again tomorrow..
> 
> git and me just ain't gonna be friends it seems.

Jikos enlightened me and figured out the push command I should've used.

The result is that speck/master is now one commit ahead of the rest.
I will update the stable branches in the morning for fear of messing
things up (again) this late.

[MODERATED] Re: BUG: disabling SMT from BIOS

[Peter Zijlstra]

On Wed, Jul 25, 2018 at 12:34:16AM +0200, Peter Zijlstra wrote:
> On Tue, Jul 24, 2018 at 06:36:58PM +0200, Peter Zijlstra wrote:
> > I'll try again tomorrow..
> > 
> > git and me just ain't gonna be friends it seems.
> 
> Jikos enlightened me and figured out the push command I should've used.
> 
> The result is that speck/master is now one commit ahead of the rest.
> I will update the stable branches in the morning for fear of messing
> things up (again) this late.

With exception of the linux-4.9 branch (still waiting for dwmw2 to get
back -- apparently he's on holidays too), they're now all in-sync.

[MODERATED] Re: BUG: disabling SMT from BIOS

[Peter Zijlstra]

On Wed, Jul 25, 2018 at 10:45:55AM +0200, speck for Peter Zijlstra wrote:
> On Wed, Jul 25, 2018 at 12:34:16AM +0200, Peter Zijlstra wrote:
> > On Tue, Jul 24, 2018 at 06:36:58PM +0200, Peter Zijlstra wrote:
> > > I'll try again tomorrow..
> > > 
> > > git and me just ain't gonna be friends it seems.
> > 
> > Jikos enlightened me and figured out the push command I should've used.
> > 
> > The result is that speck/master is now one commit ahead of the rest.
> > I will update the stable branches in the morning for fear of messing
> > things up (again) this late.
> 
> With exception of the linux-4.9 branch (still waiting for dwmw2 to get
> back -- apparently he's on holidays too), they're now all in-sync.

At David's reqest I've also updated the 4.9 branch, so they should all
be in-sync now.

[MODERATED] Re: BUG: disabling SMT from BIOS

[Greg KH]

On Wed, Jul 25, 2018 at 12:05:16PM +0200, speck for Peter Zijlstra wrote:
> On Wed, Jul 25, 2018 at 10:45:55AM +0200, speck for Peter Zijlstra wrote:
> > On Wed, Jul 25, 2018 at 12:34:16AM +0200, Peter Zijlstra wrote:
> > > On Tue, Jul 24, 2018 at 06:36:58PM +0200, Peter Zijlstra wrote:
> > > > I'll try again tomorrow..
> > > > 
> > > > git and me just ain't gonna be friends it seems.
> > > 
> > > Jikos enlightened me and figured out the push command I should've used.
> > > 
> > > The result is that speck/master is now one commit ahead of the rest.
> > > I will update the stable branches in the morning for fear of messing
> > > things up (again) this late.
> > 
> > With exception of the linux-4.9 branch (still waiting for dwmw2 to get
> > back -- apparently he's on holidays too), they're now all in-sync.
> 
> At David's reqest I've also updated the 4.9 branch, so they should all
> be in-sync now.

I just checked and they all look good to me, thanks for doing the
backports.

greg k-h

Re: BUG: disabling SMT from BIOS

[Thomas Gleixner]

On Mon, 23 Jul 2018, speck for Josh Poimboeuf wrote:
> From: Josh Poimboeuf <jpoimboe@redhat.com>
> Subject: [PATCH] cpu/hotplug: detect SMT disabled by BIOS
> 
> If SMT is disabled in BIOS, the CPU code doesn't properly detect it.
> The /sys/devices/system/cpu/smt/control file shows 'on', and the 'l1tf'
> vulnerabilities file shows SMT as vulnerable.
> 
> Fix it by forcing 'cpu_smt_control' to CPU_SMT_NOT_SUPPORTED in such a
> case.  Unfortunately the detection can only be done after bringing all
> the CPUs online, so we have to overwrite any previous writes to the
> variable.

Bah, this whole CPU enumeration on x86 sucks.

> Reported-by: Joe Mario <jmario@redhat.com>
> Fixes: f048c399e0f7 ("x86/topology: Provide topology_smt_supported()")
> Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>

Belated Acked-by-me.