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
Jörg,
On Mon, 30 Apr 2018, Jörg Otte wrote:
> 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
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(+)
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -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);
2018-04-30 21:53 GMT+02:00 Thomas Gleixner <[email protected]>:
> Jörg,
>
> On Mon, 30 Apr 2018, Jörg Otte wrote:
>
>> 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
>
> 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(+)
>
> --- a/arch/x86/kernel/cpu/common.c
> +++ b/arch/x86/kernel/cpu/common.c
> @@ -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);
>
No, does not fix it.
Thanks, Jörg
On Tue, 1 May 2018, Jörg Otte wrote:
> 2018-04-30 21:53 GMT+02:00 Thomas Gleixner <[email protected]>:
> > --- a/arch/x86/kernel/cpu/common.c
> > +++ b/arch/x86/kernel/cpu/common.c
> > @@ -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);
> >
>
> No, does not fix it.
Then I really have no idea how reverting the patch you pointed out would
fix it.
Thanks,
tglx
On Tue, 1 May 2018, Thomas Gleixner wrote:
> On Tue, 1 May 2018, J?rg Otte wrote:
> > 2018-04-30 21:53 GMT+02:00 Thomas Gleixner <[email protected]>:
> > > --- a/arch/x86/kernel/cpu/common.c
> > > +++ b/arch/x86/kernel/cpu/common.c
> > > @@ -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);
> > >
> >
> > No, does not fix it.
>
> 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
On Tue, May 1, 2018 at 5:59 AM Thomas Gleixner <[email protected]> wrote:
> 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
On 05/01/2018 11:27 AM, Thomas Gleixner wrote:
> On Tue, 1 May 2018, Thomas Gleixner wrote:
>> On Tue, 1 May 2018, J?rg Otte wrote:
>>> 2018-04-30 21:53 GMT+02:00 Thomas Gleixner <[email protected]>:
>>>> --- a/arch/x86/kernel/cpu/common.c
>>>> +++ b/arch/x86/kernel/cpu/common.c
>>>> @@ -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);
>>>>
>>>
>>> No, does not fix it.
>>
>> 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
>
> 2) rc3 with the above patch applied.
>
> If #1 works then #2 must work as well.
>
> Thanks,
>
> tglx
>
2018-05-01 22:14 GMT+02:00 Linus Torvalds <[email protected]>:
> On Tue, May 1, 2018 at 5:59 AM Thomas Gleixner <[email protected]> wrote:
>
>> 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
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
On Wed, 2 May 2018, Jörg Otte wrote:
> 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.
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<-------------------
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -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))
2018-05-02 11:02 GMT+02:00 Thomas Gleixner <[email protected]>:
> On Wed, 2 May 2018, Jörg Otte wrote:
>> 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.
>
> 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<-------------------
> --- a/arch/x86/kernel/cpu/common.c
> +++ b/arch/x86/kernel/cpu/common.c
> @@ -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))
>
OK, that patch works for me!
Thanks, Jörg
On Wed, 2 May 2018, Jörg Otte wrote:
> 2018-05-02 11:02 GMT+02:00 Thomas Gleixner <[email protected]>:
> > 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<-------------------
> > --- a/arch/x86/kernel/cpu/common.c
> > +++ b/arch/x86/kernel/cpu/common.c
> > @@ -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))
> >
>
> 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
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]
---
arch/x86/kernel/cpu/common.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 8a5b185735e1..ce243f7d2d4e 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -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))
On 05/02/2018 07:48 AM, tip-bot for Thomas Gleixner wrote:
> 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
>
> In hindsight this should have used completely synthetic bits for IBRB,
s/IBRB/IBPB
> 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
> 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]
> ---
On Wed, May 02, 2018 at 02:20:52PM +0200, Thomas Gleixner wrote:
> 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
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(-)
diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
index 578793e97431..0482da6d7d6f 100644
--- a/arch/x86/include/asm/cpufeatures.h
+++ b/arch/x86/include/asm/cpufeatures.h
@@ -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 */
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index ce243f7d2d4e..1831203800d3 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -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)
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index 1fc05e428aba..71e1d6334f7d 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -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)
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index c7668806163f..dca7e8b8597b 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -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;
--
2.17.0.391.g1f1cddd558b5
--
Regards/Gruss,
Boris.
Good mailing practices for 400: avoid top-posting and trim the reply.
2018-05-04 18:18 GMT+02:00 Borislav Petkov <[email protected]>:
> On Wed, May 02, 2018 at 02:20:52PM +0200, Thomas Gleixner wrote:
>> 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
>
> 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
On Sat, May 05, 2018 at 11:47:53AM +0200, Jörg Otte wrote:
> Patch doesn't hurt me. For me it´s ok.
>
> Thanks, Jörg
Good, thanks for testing!
--
Regards/Gruss,
Boris.
Good mailing practices for 400: avoid top-posting and trim the reply.