Lost IBPB, IBRS_FW support for spectre_v2 mitigation.

v4.17-rcx Jörg Otte 6 participants 16 messages Apr 30, 2018 – May 5, 2018

Subsystems (2)

x86 KVM x86

lore

Share & tools

lore

Jörg Otte

author

2018-04-30 15:59 UTC #0 lore raw

Hi,

In v4.16 I already had support for BPB, IBRS_FW for spectre_v2 mitigation. But this went away in v17-rcx.

With 4.16 I have:

jojo@fichte:~$ cd /sys/devices/system/cpu/vulnerabilities; grep ".*" *
meltdown:Mitigation: PTI
spectre_v1:Mitigation: __user pointer sanitization
spectre_v2:Mitigation: Full generic retpoline, IBPB, IBRS_FW

With 4.17-rcx I have:
meltdown:Mitigation: PTI spectre_v1:Mitigation: __user pointer sanitization spectre_v2:Mitigation: Full generic retpoline

Processor is
vendor_id : GenuineIntel
cpu family : 6
model : 60
model name : Intel(R) Core(TM) i5-4200M CPU @ 2.50GHz
stepping : 3
microcode : 0x24

The problem goes away if I revert:
d94a155 x86/cpu: Prevent cpuinfo_x86::x86_phys_bits adjustment corruption

Thanks, Jörg

Thomas Gleixner

2018-04-30 19:53 UTC #1 lore raw

Jörg,

Quoted 23 lines · Jörg Otte

In v4.16 I already had support for BPB, IBRS_FW for spectre_v2 mitigation. But this went away in v17-rcx.
With 4.16 I have:
jojo@fichte:~$ cd /sys/devices/system/cpu/vulnerabilities; grep ".*" *
meltdown:Mitigation: PTI
spectre_v1:Mitigation: __user pointer sanitization
spectre_v2:Mitigation: Full generic retpoline, IBPB, IBRS_FW
With 4.17-rcx I have:
meltdown:Mitigation: PTI spectre_v1:Mitigation: __user pointer sanitization spectre_v2:Mitigation: Full generic retpoline
Processor is
vendor_id : GenuineIntel
cpu family : 6
model : 60
model name : Intel(R) Core(TM) i5-4200M CPU @ 2.50GHz
stepping : 3
microcode : 0x24

Quoted 2 lines

The problem goes away if I revert:
d94a155 x86/cpu: Prevent cpuinfo_x86::x86_phys_bits adjustment corruption

Does the patch below fix the problem for you?

Thanks,

tglx

8<------------------
Subject: x86/cpu: Restore CPUID_8000_0008_EBX reload
From: Thomas Gleixner <[email protected]>
Date: Mon, 30 Apr 2018 21:47:46 +0200

The recent commt which addresses the x86_phys_bits corruption with encrypted memory on CPUID reload after a microcode update lost the reload of CPUID_8000_0008_EBX as well.

As a consequence IBRS and IBRS_FW are not longer detected

Restore the behaviour by bringing the reload of CPUID_8000_0008_EBX back,.

Fixes d94a155c59c9 ("x86/cpu: Prevent cpuinfo_x86::x86_phys_bits adjustment corruption")

Reported-by Jörg Otte <[email protected]>

Signed-off-by Thomas Gleixner <[email protected]>

Cc [email protected]

---
 arch/x86/kernel/cpu/common.c |    5 +++++
 1 file changed, 5 insertions(+)

arch/x86/kernel/cpu/common.c +5 -0

@@ -848,6 +848,11 @@ void get_cpu_cap(struct cpuinfo_x86 *c)
 		c->x86_power = edx;
 	}
 
+	if (c->extended_cpuid_level >= 0x80000008) {
+		cpuid(0x80000008, &eax, &ebx, &ecx, &edx);
+		c->x86_capability[CPUID_8000_0008_EBX] = ebx;
+	}
+
 	if (c->extended_cpuid_level >= 0x8000000a)
 		c->x86_capability[CPUID_8000_000A_EDX] = cpuid_edx(0x8000000a);

Thomas Gleixner

2018-05-01 12:59 UTC #3 lore raw

Quoted 7 lines · Jörg Otte

2018-04-30 21:53 GMT+02:00 Thomas Gleixner <[email protected]>:
Quoted 6 lines
arch/x86/kernel/cpu/common.c +0 -0
@@ -848,6 +848,11 @@ void get_cpu_cap(struct cpuinfo_x86 *c)
                c->x86_power = edx;
        }
 

Quoted 10 lines

arch/x86/kernel/cpu/common.c void get_cpu_cap(struct cpuinfo_x86 *c) · line 858

Quoted 8 lines

arch/x86/kernel/cpu/common.c +5 -0

+       if (c->extended_cpuid_level >= 0x80000008) {
+               cpuid(0x80000008, &eax, &ebx, &ecx, &edx);
+               c->x86_capability[CPUID_8000_0008_EBX] = ebx;
+       }
+
        if (c->extended_cpuid_level >= 0x8000000a)
                c->x86_capability[CPUID_8000_000A_EDX] = cpuid_edx(0x8000000a);

No, does not fix it.

Then I really have no idea how reverting the patch you pointed out would fix it.

Thanks,

tglx

Thomas Gleixner

2018-05-01 18:27 UTC #4 lore raw

Quoted 19 lines · Thomas Gleixner

Quoted 7 lines · Jörg Otte
2018-04-30 21:53 GMT+02:00 Thomas Gleixner <[email protected]>:
Quoted 6 lines
arch/x86/kernel/cpu/common.c +0 -0
@@ -848,6 +848,11 @@ void get_cpu_cap(struct cpuinfo_x86 *c)
                c->x86_power = edx;
        }
 
Quoted 10 lines
arch/x86/kernel/cpu/common.c void get_cpu_cap(struct cpuinfo_x86 *c) · line 858
Quoted 8 lines
arch/x86/kernel/cpu/common.c +5 -0
+       if (c->extended_cpuid_level >= 0x80000008) {
+               cpuid(0x80000008, &eax, &ebx, &ecx, &edx);
+               c->x86_capability[CPUID_8000_0008_EBX] = ebx;
+       }
+
        if (c->extended_cpuid_level >= 0x8000000a)
                c->x86_capability[CPUID_8000_000A_EDX] = cpuid_edx(0x8000000a);
 
No, does not fix it.

Quoted 2 lines

Then I really have no idea how reverting the patch you pointed out would fix it.

Can you please carefully recheck:

1) rc3 with d94a155c59c9 reverted.

2) rc3 with the above patch applied.

If #1 works then #2 must work as well.

Thanks,

tglx

Tim Chen

2018-05-01 23:07 UTC #5 lore raw

Quoted 20 lines · Thomas Gleixner

Quoted 19 lines · Thomas Gleixner
Quoted 17 lines · Jörg Otte
2018-04-30 21:53 GMT+02:00 Thomas Gleixner <[email protected]>:
Quoted 4 lines
arch/x86/kernel/cpu/common.c +0 -0
@@ -848,6 +848,11 @@ void get_cpu_cap(struct cpuinfo_x86 *c)
                c->x86_power = edx;
arch/x86/kernel/cpu/common.c void get_cpu_cap(struct cpuinfo_x86 *c) · line 858
Quoted 10 lines
arch/x86/kernel/cpu/common.c +5 -0
        }
 
+       if (c->extended_cpuid_level >= 0x80000008) {
+               cpuid(0x80000008, &eax, &ebx, &ecx, &edx);
+               c->x86_capability[CPUID_8000_0008_EBX] = ebx;
+       }
+
        if (c->extended_cpuid_level >= 0x8000000a)
                c->x86_capability[CPUID_8000_000A_EDX] = cpuid_edx(0x8000000a);
 
No, does not fix it.

Quoted 6 lines

Quoted 2 lines
Then I really have no idea how reverting the patch you pointed out would fix it.
Can you please carefully recheck:
1) rc3 with d94a155c59c9 reverted.

I saw the same issue that Jorg Otte reported on a coffee lake system with vanilla 4.17-rc3.
Reverting d94a155c59c9 did the trick for me.

Thanks.

Tim

Quoted 9 lines

2) rc3 with the above patch applied.
If #1 works then #2 must work as well.
Thanks,
tglx

Linus Torvalds

2018-05-01 20:14 UTC #6 lore raw

In reply to Thomas Gleixner

Quoted 2 lines · Thomas Gleixner

Then I really have no idea how reverting the patch you pointed out would fix it.

So I do think that the original patch is buggy.

What I think *may* be going on is:

- first we do that

                 get_cpu_cap(c);
                 get_cpu_address_sizes(c);

     but at that point, CPU levels may be masked, and that 0x80000008 leaf

isn't seen

- then we do

                 if (this_cpu->c_early_init)
                         this_cpu->c_early_init(c);

    which calls early_init_intel(), which does that

                 if (msr_clear_bit(MSR_IA32_MISC_ENABLE,
                                   MSR_IA32_MISC_ENABLE_LIMIT_CPUID_BIT) >

0) {

which now raises the cpuid_level.

- then we do

get_cpu_cap(c);

again, because the cpuid level has been raised, and _now_ it used to get
that 0x80000008 leaf information.

But with the change, that second call to get_cpu_cap() didn't do anything, because the 0x80000008 leaf handling had been moved away.

However, I agree that your patch to just do that CPUID_8000_0008_EBX in get_cpu_cap() should have fixed it, and it's possible that Jörg mis-tested it.

Jörg, are you sure you didn't somehow get the wrong microcode? Because another way for those bits to be cleared again is if bad_spectre_microcode() triggers. That should show up in dmesg as "Intel Spectre v2 broken microcode detected" though.

Linus

Jörg Otte

author

2018-05-02 07:43 UTC #7 lore raw

2018-05-01 22:14 GMT+02:00 Linus Torvalds <[email protected]>:

Quoted 44 lines · Thomas Gleixner

Quoted 3 lines
Then I really have no idea how reverting the patch you pointed out would fix it.
So I do think that the original patch is buggy.
What I think *may* be going on is:
- first we do that
                 get_cpu_cap(c);
                 get_cpu_address_sizes(c);

     but at that point, CPU levels may be masked, and that 0x80000008 leaf
isn't seen
- then we do
                 if (this_cpu->c_early_init)
                         this_cpu->c_early_init(c);

    which calls early_init_intel(), which does that

                 if (msr_clear_bit(MSR_IA32_MISC_ENABLE,
                                   MSR_IA32_MISC_ENABLE_LIMIT_CPUID_BIT) >
0) {
which now raises the cpuid_level.
- then we do
get_cpu_cap(c);
again, because the cpuid level has been raised, and _now_ it used to get
that 0x80000008 leaf information.
But with the change, that second call to get_cpu_cap() didn't do anything, because the 0x80000008 leaf handling had been moved away.
However, I agree that your patch to just do that CPUID_8000_0008_EBX in get_cpu_cap() should have fixed it, and it's possible that Jörg mis-tested it.

Quoted 6 lines

Jörg, are you sure you didn't somehow get the wrong microcode? Because another way for those bits to be cleared again is if bad_spectre_microcode() triggers. That should show up in dmesg as "Intel Spectre v2 broken microcode detected" though.
Linus

I downloaded microcode from Intel.
Here are the excerpts from dmesg:

With revert:

jojo@fichte:~$ dmesg | grep -i -e spec -e micro -e "Linux version"

[    0.000000] microcode: microcode updated early to revision 0x24,
date = 2018-01-21
[    0.000000] Linux version 4.17.0-rc3-revert-00001-gcb1069f

(jojo@fichte) (gcc version 5.4.0 20160609 (Ubuntu 5.4.0-6ubu

dmesg | grep -i -e spec -e micro -e "Linux version"

[    0.000000] microcode: microcode updated early to revision 0x24,
date = 2018-01-21
[    0.000000] Linux version 4.17.0-rc3-patch-00001-gdc10603
(jojo@fichte) (gcc version 5.4.0 20160609 (Ubuntu
5.4.0-6ubuntu1~16.04.9)) #20 SMP Wed May 2 09:08:07 CEST 2018
[    0.028417] Spectre V2 : Mitigation: Full generic retpoline
[    0.491803] microcode: sig=0x306c3, pf=0x10, revision=0x24
[    0.491831] microcode: Microcode Update Driver: v2.2.ntu1~16.04.9))
#21 SMP Wed May 2 09:14:29 CEST 2018
[    0.028414] Spectre V2 : Mitigation: Full generic retpoline
[    0.028415] Spectre V2 : Spectre v2 mitigation: Enabling Indirect
Branch Prediction Barrier
[    0.028415] Spectre V2 : Enabling Restricted Speculation for firmware calls
[    0.500157] microcode: sig=0x306c3, pf=0x10, revision=0x24
[    0.500183] microcode: Microcode Update Driver: v2.2.

With patch:

dmesg | grep -i -e spec -e micro -e "Linux version"

[    0.000000] microcode: microcode updated early to revision 0x24,
date = 2018-01-21
[    0.000000] Linux version 4.17.0-rc3-patch-00001-gdc10603
(jojo@fichte) (gcc version 5.4.0 20160609 (Ubuntu
5.4.0-6ubuntu1~16.04.9)) #20 SMP Wed May 2 09:08:07 CEST 2018
[    0.028417] Spectre V2 : Mitigation: Full generic retpoline
[    0.491803] microcode: sig=0x306c3, pf=0x10, revision=0x24
[    0.491831] microcode: Microcode Update Driver: v2.2.

Thanks, Jörg

Thomas Gleixner

2018-05-02 09:02 UTC #8 lore raw

Quoted 32 lines · Jörg Otte

With revert:

jojo@fichte:~$ dmesg | grep -i -e spec -e micro -e "Linux version"

[    0.000000] microcode: microcode updated early to revision 0x24,
date = 2018-01-21
[    0.000000] Linux version 4.17.0-rc3-revert-00001-gcb1069f

(jojo@fichte) (gcc version 5.4.0 20160609 (Ubuntu 5.4.0-6ubu

dmesg | grep -i -e spec -e micro -e "Linux version"

[    0.000000] microcode: microcode updated early to revision 0x24,
date = 2018-01-21
[    0.000000] Linux version 4.17.0-rc3-patch-00001-gdc10603
(jojo@fichte) (gcc version 5.4.0 20160609 (Ubuntu
5.4.0-6ubuntu1~16.04.9)) #20 SMP Wed May 2 09:08:07 CEST 2018
[    0.028417] Spectre V2 : Mitigation: Full generic retpoline
[    0.491803] microcode: sig=0x306c3, pf=0x10, revision=0x24
[    0.491831] microcode: Microcode Update Driver: v2.2.ntu1~16.04.9))
#21 SMP Wed May 2 09:14:29 CEST 2018
[    0.028414] Spectre V2 : Mitigation: Full generic retpoline
[    0.028415] Spectre V2 : Spectre v2 mitigation: Enabling Indirect
Branch Prediction Barrier
[    0.028415] Spectre V2 : Enabling Restricted Speculation for firmware calls
[    0.500157] microcode: sig=0x306c3, pf=0x10, revision=0x24
[    0.500183] microcode: Microcode Update Driver: v2.2.

With patch:

dmesg | grep -i -e spec -e micro -e "Linux version"

Quoted 8 lines

[    0.000000] microcode: microcode updated early to revision 0x24,
date = 2018-01-21
[    0.000000] Linux version 4.17.0-rc3-patch-00001-gdc10603
(jojo@fichte) (gcc version 5.4.0 20160609 (Ubuntu
5.4.0-6ubuntu1~16.04.9)) #20 SMP Wed May 2 09:08:07 CEST 2018
[    0.028417] Spectre V2 : Mitigation: Full generic retpoline
[    0.491803] microcode: sig=0x306c3, pf=0x10, revision=0x24
[    0.491831] microcode: Microcode Update Driver: v2.2.

Ok, I think I know what's going wrong in that steaming pile of horrors of CPUID detection. I need to analyze it down to the roots, but if you have cycles, can you please test the patch below?

It's a hack and even if it fixes the problem I'm going to do it differently.

Thanks,

tglx

8<-------------------

arch/x86/kernel/cpu/common.c +5 -1

@@ -848,6 +848,11 @@ void get_cpu_cap(struct cpuinfo_x86 *c)
 		c->x86_power = edx;
 	}
 
+	if (c->extended_cpuid_level >= 0x80000008) {
+		cpuid(0x80000008, &eax, &ebx, &ecx, &edx);
+		c->x86_capability[CPUID_8000_0008_EBX] = ebx;
+	}
+
 	if (c->extended_cpuid_level >= 0x8000000a)
 		c->x86_capability[CPUID_8000_000A_EDX] = cpuid_edx(0x8000000a);

@@ -871,7 +876,6 @@ static void get_cpu_address_sizes(struct
 
 		c->x86_virt_bits = (eax >> 8) & 0xff;
 		c->x86_phys_bits = eax & 0xff;
-		c->x86_capability[CPUID_8000_0008_EBX] = ebx;
 	}
 #ifdef CONFIG_X86_32
 	else if (cpu_has(c, X86_FEATURE_PAE) || cpu_has(c, X86_FEATURE_PSE36))

Jörg Otte

author

2018-05-02 09:25 UTC #9 lore raw

2018-05-02 11:02 GMT+02:00 Thomas Gleixner <[email protected]>:

Quoted 69 lines · Jörg Otte

Quoted 32 lines

With revert:

jojo@fichte:~$ dmesg | grep -i -e spec -e micro -e "Linux version"

[    0.000000] microcode: microcode updated early to revision 0x24,
date = 2018-01-21
[    0.000000] Linux version 4.17.0-rc3-revert-00001-gcb1069f

(jojo@fichte) (gcc version 5.4.0 20160609 (Ubuntu 5.4.0-6ubu

dmesg | grep -i -e spec -e micro -e "Linux version"

[    0.000000] microcode: microcode updated early to revision 0x24,
date = 2018-01-21
[    0.000000] Linux version 4.17.0-rc3-patch-00001-gdc10603
(jojo@fichte) (gcc version 5.4.0 20160609 (Ubuntu
5.4.0-6ubuntu1~16.04.9)) #20 SMP Wed May 2 09:08:07 CEST 2018
[    0.028417] Spectre V2 : Mitigation: Full generic retpoline
[    0.491803] microcode: sig=0x306c3, pf=0x10, revision=0x24
[    0.491831] microcode: Microcode Update Driver: v2.2.ntu1~16.04.9))
#21 SMP Wed May 2 09:14:29 CEST 2018
[    0.028414] Spectre V2 : Mitigation: Full generic retpoline
[    0.028415] Spectre V2 : Spectre v2 mitigation: Enabling Indirect
Branch Prediction Barrier
[    0.028415] Spectre V2 : Enabling Restricted Speculation for firmware calls
[    0.500157] microcode: sig=0x306c3, pf=0x10, revision=0x24
[    0.500183] microcode: Microcode Update Driver: v2.2.

With patch:

dmesg | grep -i -e spec -e micro -e "Linux version"

Quoted 8 lines

[    0.000000] microcode: microcode updated early to revision 0x24,
date = 2018-01-21
[    0.000000] Linux version 4.17.0-rc3-patch-00001-gdc10603
(jojo@fichte) (gcc version 5.4.0 20160609 (Ubuntu
5.4.0-6ubuntu1~16.04.9)) #20 SMP Wed May 2 09:08:07 CEST 2018
[    0.028417] Spectre V2 : Mitigation: Full generic retpoline
[    0.491803] microcode: sig=0x306c3, pf=0x10, revision=0x24
[    0.491831] microcode: Microcode Update Driver: v2.2.

Ok, I think I know what's going wrong in that steaming pile of horrors of CPUID detection. I need to analyze it down to the roots, but if you have cycles, can you please test the patch below?

It's a hack and even if it fixes the problem I'm going to do it differently.

Thanks,

tglx

8<-------------------

arch/x86/kernel/cpu/common.c +5 -0

@@ -848,6 +848,11 @@ void get_cpu_cap(struct cpuinfo_x86 *c)
                c->x86_power = edx;
        }
 
+       if (c->extended_cpuid_level >= 0x80000008) {
+               cpuid(0x80000008, &eax, &ebx, &ecx, &edx);
+               c->x86_capability[CPUID_8000_0008_EBX] = ebx;
+       }
+
        if (c->extended_cpuid_level >= 0x8000000a)
                c->x86_capability[CPUID_8000_000A_EDX] = cpuid_edx(0x8000000a);

arch/x86/kernel/cpu/common.c static void get_cpu_address_sizes(struct · line 881

Quoted 7 lines

arch/x86/kernel/cpu/common.c +0 -1

@@ -871,7 +876,6 @@ static void get_cpu_address_sizes(struct
 
                c->x86_virt_bits = (eax >> 8) & 0xff;
                c->x86_phys_bits = eax & 0xff;
-               c->x86_capability[CPUID_8000_0008_EBX] = ebx;
        }
 #ifdef CONFIG_X86_32
        else if (cpu_has(c, X86_FEATURE_PAE) || cpu_has(c, X86_FEATURE_PSE36))

OK, that patch works for me!

Thanks, Jörg

Thomas Gleixner

2018-05-02 12:21 UTC #10 lore raw

Quoted 28 lines · Jörg Otte

2018-05-02 11:02 GMT+02:00 Thomas Gleixner <[email protected]>:
Quoted 27 lines
Ok, I think I know what's going wrong in that steaming pile of horrors of CPUID detection. I need to analyze it down to the roots, but if you have cycles, can you please test the patch below?
It's a hack and even if it fixes the problem I'm going to do it differently.
Thanks,
tglx
8<-------------------
arch/x86/kernel/cpu/common.c +5 -0
@@ -848,6 +848,11 @@ void get_cpu_cap(struct cpuinfo_x86 *c)
                c->x86_power = edx;
        }
 
+       if (c->extended_cpuid_level >= 0x80000008) {
+               cpuid(0x80000008, &eax, &ebx, &ecx, &edx);
+               c->x86_capability[CPUID_8000_0008_EBX] = ebx;
+       }
+
        if (c->extended_cpuid_level >= 0x8000000a)
                c->x86_capability[CPUID_8000_000A_EDX] = cpuid_edx(0x8000000a);
 
@@ -871,7 +876,6 @@ static void get_cpu_address_sizes(struct
 

Quoted 9 lines

arch/x86/kernel/cpu/common.c static void get_cpu_address_sizes(struct · line 881

Quoted 7 lines

arch/x86/kernel/cpu/common.c +0 -1

                c->x86_virt_bits = (eax >> 8) & 0xff;
                c->x86_phys_bits = eax & 0xff;
-               c->x86_capability[CPUID_8000_0008_EBX] = ebx;
        }
 #ifdef CONFIG_X86_32
        else if (cpu_has(c, X86_FEATURE_PAE) || cpu_has(c, X86_FEATURE_PSE36))

OK, that patch works for me!

Thanks for confirming. Still need to find a way which is less fragile, but that's probably too much of churn for rc4....

At least I know exactly what's happening, so I can write a better changelog.

Thanks for testing!

tglx

Borislav Petkov

maintainer

2018-05-04 16:18 UTC #11 lore raw

Quoted 6 lines · Thomas Gleixner

Thanks for confirming. Still need to find a way which is less fragile, but that's probably too much of churn for rc4....
At least I know exactly what's happening, so I can write a better changelog.
Thanks for testing!

Jörg, can you pls also test this one ontop of Thomas' patch to make sure it doesn't break your box.

Thx.

---

Quoted 1 line

From 6857c2ac8e31f4f9b350cfad4f6b6eb831bf57f1 Mon Sep 17 00:00:00 2001

From: Borislav Petkov <[email protected]>
Date: Wed, 2 May 2018 18:15:14 +0200
Subject: [PATCH] x86/CPU: Use synthetic bits for IBRS/IBPB/STIBP

Intel and AMD have different CPUID bits for those so use synthetic bits which get set on the respective vendor in init_speculation_control(). So that debacles like the commit message of

c65732e4f721 ("x86/cpu: Restore CPUID_8000_0008_EBX reload")

talks about don't happen anymore.

Signed-off-by Borislav Petkov <[email protected]>

---
 arch/x86/include/asm/cpufeatures.h | 12 ++++++------
 arch/x86/kernel/cpu/common.c       | 14 ++++++++++----
 arch/x86/kvm/svm.c                 |  6 +++---
 arch/x86/kvm/vmx.c                 |  3 ---
 4 files changed, 19 insertions(+), 16 deletions(-)

arch/x86/include/asm/cpufeatures.h +6 -6

index 578793e97431..0482da6d7d6f 100644
@@ -198,7 +198,6 @@
 #define X86_FEATURE_CAT_L2		( 7*32+ 5) /* Cache Allocation Technology L2 */
 #define X86_FEATURE_CDP_L3		( 7*32+ 6) /* Code and Data Prioritization L3 */
 #define X86_FEATURE_INVPCID_SINGLE	( 7*32+ 7) /* Effectively INVPCID && CR4.PCIDE=1 */
-
 #define X86_FEATURE_HW_PSTATE		( 7*32+ 8) /* AMD HW-PState */
 #define X86_FEATURE_PROC_FEEDBACK	( 7*32+ 9) /* AMD ProcFeedbackInterface */
 #define X86_FEATURE_SME			( 7*32+10) /* AMD Secure Memory Encryption */

@@ -207,13 +206,14 @@
 #define X86_FEATURE_RETPOLINE_AMD	( 7*32+13) /* "" AMD Retpoline mitigation for Spectre variant 2 */
 #define X86_FEATURE_INTEL_PPIN		( 7*32+14) /* Intel Processor Inventory Number */
 #define X86_FEATURE_CDP_L2		( 7*32+15) /* Code and Data Prioritization L2 */
-
+#define X86_FEATURE_IBRS		( 7*32+16) /* Indirect Branch Restricted Speculation */
+#define X86_FEATURE_IBPB		( 7*32+17) /* Indirect Branch Prediction Barrier */
 #define X86_FEATURE_MBA			( 7*32+18) /* Memory Bandwidth Allocation */
 #define X86_FEATURE_RSB_CTXSW		( 7*32+19) /* "" Fill RSB on context switches */
 #define X86_FEATURE_SEV			( 7*32+20) /* AMD Secure Encrypted Virtualization */
-
 #define X86_FEATURE_USE_IBPB		( 7*32+21) /* "" Indirect Branch Prediction Barrier enabled */
 #define X86_FEATURE_USE_IBRS_FW		( 7*32+22) /* "" Use IBRS during runtime firmware calls */
+#define X86_FEATURE_STIBP		( 7*32+23) /* Single Thread Indirect Branch Predictors */
 
 /* Virtualization flags: Linux defined, word 8 */
 #define X86_FEATURE_TPR_SHADOW		( 8*32+ 0) /* Intel TPR Shadow */

@@ -274,9 +274,9 @@
 #define X86_FEATURE_CLZERO		(13*32+ 0) /* CLZERO instruction */
 #define X86_FEATURE_IRPERF		(13*32+ 1) /* Instructions Retired Count */
 #define X86_FEATURE_XSAVEERPTR		(13*32+ 2) /* Always save/restore FP error pointers */
-#define X86_FEATURE_IBPB		(13*32+12) /* Indirect Branch Prediction Barrier */
-#define X86_FEATURE_IBRS		(13*32+14) /* Indirect Branch Restricted Speculation */
-#define X86_FEATURE_STIBP		(13*32+15) /* Single Thread Indirect Branch Predictors */
+#define X86_FEATURE_AMD_IBPB		(13*32+12) /* "" Indirect Branch Prediction Barrier */
+#define X86_FEATURE_AMD_IBRS		(13*32+14) /* "" Indirect Branch Restricted Speculation */
+#define X86_FEATURE_AMD_STIBP		(13*32+15) /* "" Single Thread Indirect Branch Predictors */
 
 /* Thermal and Power Management Leaf, CPUID level 0x00000006 (EAX), word 14 */
 #define X86_FEATURE_DTHERM		(14*32+ 0) /* Digital Thermal Sensor */

arch/x86/kernel/cpu/common.c +10 -4

index ce243f7d2d4e..1831203800d3 100644
@@ -757,17 +757,23 @@ static void init_speculation_control(struct cpuinfo_x86 *c)
 	 * and they also have a different bit for STIBP support. Also,
 	 * a hypervisor might have set the individual AMD bits even on
 	 * Intel CPUs, for finer-grained selection of what's available.
-	 *
-	 * We use the AMD bits in 0x8000_0008 EBX as the generic hardware
-	 * features, which are visible in /proc/cpuinfo and used by the
-	 * kernel. So set those accordingly from the Intel bits.
 	 */
 	if (cpu_has(c, X86_FEATURE_SPEC_CTRL)) {
 		set_cpu_cap(c, X86_FEATURE_IBRS);
 		set_cpu_cap(c, X86_FEATURE_IBPB);
 	}
+
 	if (cpu_has(c, X86_FEATURE_INTEL_STIBP))
 		set_cpu_cap(c, X86_FEATURE_STIBP);
+
+	if (cpu_has(c, X86_FEATURE_AMD_IBRS))
+		set_cpu_cap(c, X86_FEATURE_IBRS);
+
+	if (cpu_has(c, X86_FEATURE_AMD_IBPB))
+		set_cpu_cap(c, X86_FEATURE_IBPB);
+
+	if (cpu_has(c, X86_FEATURE_AMD_STIBP))
+		set_cpu_cap(c, X86_FEATURE_STIBP);
 }
 
 void get_cpu_cap(struct cpuinfo_x86 *c)

arch/x86/kvm/svm.c +3 -3

index 1fc05e428aba..71e1d6334f7d 100644
@@ -4108,7 +4108,7 @@ static int svm_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
 		break;
 	case MSR_IA32_SPEC_CTRL:
 		if (!msr_info->host_initiated &&
-		    !guest_cpuid_has(vcpu, X86_FEATURE_IBRS))
+		    !guest_cpuid_has(vcpu, X86_FEATURE_AMD_IBRS))
 			return 1;
 
 		msr_info->data = svm->spec_ctrl;

@@ -4203,7 +4203,7 @@ static int svm_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr)
 		break;
 	case MSR_IA32_SPEC_CTRL:
 		if (!msr->host_initiated &&
-		    !guest_cpuid_has(vcpu, X86_FEATURE_IBRS))
+		    !guest_cpuid_has(vcpu, X86_FEATURE_AMD_IBRS))
 			return 1;
 
 		/* The STIBP bit doesn't fault even if it's not advertised */

@@ -4230,7 +4230,7 @@ static int svm_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr)
 		break;
 	case MSR_IA32_PRED_CMD:
 		if (!msr->host_initiated &&
-		    !guest_cpuid_has(vcpu, X86_FEATURE_IBPB))
+		    !guest_cpuid_has(vcpu, X86_FEATURE_AMD_IBPB))
 			return 1;
 
 		if (data & ~PRED_CMD_IBPB)

arch/x86/kvm/vmx.c +0 -3

index c7668806163f..dca7e8b8597b 100644
@@ -3523,7 +3523,6 @@ static int vmx_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
 		return kvm_get_msr_common(vcpu, msr_info);
 	case MSR_IA32_SPEC_CTRL:
 		if (!msr_info->host_initiated &&
-		    !guest_cpuid_has(vcpu, X86_FEATURE_IBRS) &&
 		    !guest_cpuid_has(vcpu, X86_FEATURE_SPEC_CTRL))
 			return 1;

@@ -3642,7 +3641,6 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
 		break;
 	case MSR_IA32_SPEC_CTRL:
 		if (!msr_info->host_initiated &&
-		    !guest_cpuid_has(vcpu, X86_FEATURE_IBRS) &&
 		    !guest_cpuid_has(vcpu, X86_FEATURE_SPEC_CTRL))
 			return 1;

@@ -3673,7 +3671,6 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
 		break;
 	case MSR_IA32_PRED_CMD:
 		if (!msr_info->host_initiated &&
-		    !guest_cpuid_has(vcpu, X86_FEATURE_IBPB) &&
 		    !guest_cpuid_has(vcpu, X86_FEATURE_SPEC_CTRL))
 			return 1;

Signature · 7 lines

2.17.0.391.g1f1cddd558b5

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

Jörg Otte

author

2018-05-05 09:47 UTC #12 lore raw

2018-05-04 18:18 GMT+02:00 Borislav Petkov <[email protected]>:

Quoted 19 lines · Thomas Gleixner

Quoted 6 lines
Thanks for confirming. Still need to find a way which is less fragile, but that's probably too much of churn for rc4....
At least I know exactly what's happening, so I can write a better changelog.
Thanks for testing!
Jörg, can you pls also test this one ontop of Thomas' patch to make sure it doesn't break your box.
Thx.
---
From 6857c2ac8e31f4f9b350cfad4f6b6eb831bf57f1 Mon Sep 17 00:00:00 2001
From: Borislav Petkov <[email protected]>
Date: Wed, 2 May 2018 18:15:14 +0200
Subject: [PATCH] x86/CPU: Use synthetic bits for IBRS/IBPB/STIBP

Quoted 6 lines

Intel and AMD have different CPUID bits for those so use synthetic bits which get set on the respective vendor in init_speculation_control(). So that debacles like the commit message of
c65732e4f721 ("x86/cpu: Restore CPUID_8000_0008_EBX reload")

Patch doesn't hurt me. For me it´s ok.

Thanks, Jörg

Borislav Petkov

maintainer

2018-05-05 09:54 UTC #13 lore raw

Quoted 3 lines · Jörg Otte

Patch doesn't hurt me. For me it´s ok.
Thanks, Jörg

Good, thanks for testing!

Signature · 4 lines

Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

tip-bot for Thomas Gleixner

merge bot

2018-05-02 14:48 UTC #14 lore raw

[tip:x86/urgent] x86/cpu: Restore CPUID_8000_0008_EBX reload

In reply to Thomas Gleixner

x86/cpu: Restore CPUID_8000_0008_EBX reload

Commit-ID:  c65732e4f72124ca5a3a0dd3bee0d3cee39c7170
Gitweb:     https://git.kernel.org/tip/c65732e4f72124ca5a3a0dd3bee0d3cee39c7170
Author:     Thomas Gleixner <[email protected]>
AuthorDate: Mon, 30 Apr 2018 21:47:46 +0200
Committer:  Thomas Gleixner <[email protected]>
CommitDate: Wed, 2 May 2018 16:44:38 +0200

x86/cpu: Restore CPUID_8000_0008_EBX reload

The recent commt which addresses the x86_phys_bits corruption with encrypted memory on CPUID reload after a microcode update lost the reload of CPUID_8000_0008_EBX as well.

As a consequence IBRS and IBRS_FW are not longer detected

Restore the behaviour by bringing the reload of CPUID_8000_0008_EBX back. This restore has a twist due to the convoluted way the cpuid analysis works:

CPUID_8000_0008_EBX is used by AMD to enumerate IBRB, IBRS, STIBP. On Intel EBX is not used. But the speculation control code sets the AMD bits when running on Intel depending on the Intel specific speculation control bits. This was done to use the same bits for alternatives.

The change which moved the 8000_0008 evaluation out of get_cpu_cap() broke this nasty scheme due to ordering. So that on Intel the store to CPUID_8000_0008_EBX clears the IBRB, IBRS, STIBP bits which had been set before by software.

So the actual CPUID_8000_0008_EBX needs to go back to the place where it was and the phys/virt address space calculation cannot touch it.

In hindsight this should have used completely synthetic bits for IBRB, IBRS, STIBP instead of reusing the AMD bits, but that's for 4.18.

/me needs to find time to cleanup that steaming pile of ...

Fixes d94a155c59c9 ("x86/cpu: Prevent cpuinfo_x86::x86_phys_bits adjustment corruption")

Reported-by Jörg Otte <[email protected]>

Reported-by Tim Chen <[email protected]>

Signed-off-by Thomas Gleixner <[email protected]>

Tested-by Jörg Otte <[email protected]>

Cc Linus Torvalds <[email protected]>

Cc [email protected]

Cc Borislav Petkov <[email protected]

Link https://lkml.kernel.org/r/[email protected] open here

---
 arch/x86/kernel/cpu/common.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

arch/x86/kernel/cpu/common.c +5 -1

index 8a5b185735e1..ce243f7d2d4e 100644
@@ -848,6 +848,11 @@ void get_cpu_cap(struct cpuinfo_x86 *c)
 		c->x86_power = edx;
 	}
 
+	if (c->extended_cpuid_level >= 0x80000008) {
+		cpuid(0x80000008, &eax, &ebx, &ecx, &edx);
+		c->x86_capability[CPUID_8000_0008_EBX] = ebx;
+	}
+
 	if (c->extended_cpuid_level >= 0x8000000a)
 		c->x86_capability[CPUID_8000_000A_EDX] = cpuid_edx(0x8000000a);

@@ -871,7 +876,6 @@ static void get_cpu_address_sizes(struct cpuinfo_x86 *c)
 
 		c->x86_virt_bits = (eax >> 8) & 0xff;
 		c->x86_phys_bits = eax & 0xff;
-		c->x86_capability[CPUID_8000_0008_EBX] = ebx;
 	}
 #ifdef CONFIG_X86_32
 	else if (cpu_has(c, X86_FEATURE_PAE) || cpu_has(c, X86_FEATURE_PSE36))

Tim Chen

Tested-by

2018-05-02 18:08 UTC #15 lore raw

Re: [tip:x86/urgent] x86/cpu: Restore CPUID_8000_0008_EBX reload

Quoted 7 lines · tip-bot for Thomas Gleixner

Commit-ID:  c65732e4f72124ca5a3a0dd3bee0d3cee39c7170
Gitweb:     https://git.kernel.org/tip/c65732e4f72124ca5a3a0dd3bee0d3cee39c7170
Author:     Thomas Gleixner <[email protected]>
AuthorDate: Mon, 30 Apr 2018 21:47:46 +0200
Committer:  Thomas Gleixner <[email protected]>
CommitDate: Wed, 2 May 2018 16:44:38 +0200

snip

Quoted 2 lines

In hindsight this should have used completely synthetic bits for IBRB,

s/IBRB/IBPB

Quoted 7 lines

IBRS, STIBP instead of reusing the AMD bits, but that's for 4.18.
/me needs to find time to cleanup that steaming pile of ...
Fixes d94a155c59c9 ("x86/cpu: Prevent cpuinfo_x86::x86_phys_bits adjustment corruption")
Reported-by Jörg Otte <[email protected]>
Reported-by Tim Chen <[email protected]>

Fix works for me too. You can also add

Tested-by Tim Chen <[email protected]>

Thanks.

Tim

Quoted 7 lines

Signed-off-by Thomas Gleixner <[email protected]>
Tested-by Jörg Otte <[email protected]>
Cc Linus Torvalds <[email protected]>
Cc [email protected]
Cc Borislav Petkov <[email protected]
Link https://lkml.kernel.org/r/[email protected] open here
---