x86: Fix override new_cpu_data.x86 with 486
PATCH 1/1 x86: Fix override new_cpu_data.x86 with 486
We should set X86 to 486 before use cpuid to detect the cpu type, if we set X86 to 486 after cpuid, then we will get 486 for ever.
Yes, we will correct this in early_cpu_init, but it is still a wrong behavior, right? So just fix it.
I also find maybe we can delete the new_cpu_data, because we will over write all the information in early_cpu_init, and before early_cpu_init, there is no user of new_cpu_data, but this is another patch in the future.
--- arch/x86/kernel/head_32.S | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
arch/x86/kernel/head_32.S +1 -1
@@ -410,6 +410,7 @@ enable_paging: /* * Check if it is 486 */ + movb $4,X86 # at least 486 cmpl $-1,X86_CPUID je is486
@@ -437,7 +438,6 @@ enable_paging: movl %edx,X86_CAPABILITY is486: - movb $4,X86 movl $0x50022,%ecx # set AM, WP, NE and MP movl %cr0,%eax andl $0x80000011,%eax # Save PG,PE,ET
Signature · 1 line
1.7.12.4.dirty
Borislav Petkov
maintainerQuoted 4 lines · Wang YanQing
We should set X86 to 486 before use cpuid to detect the cpu type, if we set X86 to 486 after cpuid, then we will get 486 for ever.
Yep, that should be fixed, AFAICT. But it is too late for me in the day to actually stare at asm and not miss anything so I'd need a doublecheck from hpa.
Quoted 9 lines
Yes, we will correct this in
early_cpu_init, but it is still a wrong behavior, right? So just fix it.I also find maybe we can delete the
new_cpu_data, because we will over write all the information inearly_cpu_init, and beforeearly_cpu_init, there is no user ofnew_cpu_data, but this is another patch in the future.
That won't be that simple because reportedly there are cpus which report different CPUID levels due to wrong MSR programming.
And, even if you went and removed it, you'd need to test your patch on everything so that you can make sure you don't break anything. Which is not worth the trouble, IMHO.
Thanks.
Signature · 5 lines
Regards/Gruss,
Boris.
Sent from a fat crate under my desk. Formatting is fine.
--Borislav Petkov
maintainerQuoted 4 lines · Wang YanQing
We should set X86 to 486 before use cpuid to detect the cpu type, if we set X86 to 486 after cpuid, then we will get 486 for ever.
So not "for ever" but until cpu_detect runs.
Quoted 4 lines
Yes, we will correct this in
early_cpu_init, but it is still a wrong behavior, right? So just fix it.
Right, so this patch is correct and it fixes the small window where we run with family == 4 before cpu_detect but the commit message needs a bit massaging before it gets applied.
Quoted 6 lines
I also find maybe we can delete the
new_cpu_data, because we will over write all the information inearly_cpu_init, and beforeearly_cpu_init, there is no user ofnew_cpu_data, but this is another patch in the future.
You can drop that part.
Thanks.
Signature · 5 lines
Regards/Gruss,
Boris.
Sent from a fat crate under my desk. Formatting is fine.
--Quoted 9 lines · Borislav Petkov
Quoted 4 lines · Wang YanQing
We should set X86 to 486 before use cpuid to detect the cpu type, if we set X86 to 486 after cpuid, then we will get 486 for ever.
So not "for ever" but until
cpu_detectruns.Quoted 1 line
Quoted 7 lines
Quoted 3 lines
Yes, we will correct this in
early_cpu_init, but it is still a wrong behavior, right? So just fix it.Right, so this patch is correct and it fixes the small window where we run with family == 4 before
cpu_detectbut the commit message needs a bit massaging before it gets applied.
I think it fixes the window where we run with family > 4 before cpu_detect :)
Quoted 9 lines
Quoted 7 lines
I also find maybe we can delete the
new_cpu_data, because we will over write all the information inearly_cpu_init, and beforeearly_cpu_init, there is no user ofnew_cpu_data, but this is another patch in the future.You can drop that part.
I can't catch the exactly meaning:
We can drop the new_cpu_data or
I need to drop this part commit
message.
Thanks.
Borislav Petkov
maintainerQuoted 5 lines · Wang YanQing
Quoted 3 lines
Right, so this patch is correct and it fixes the small window where we run with family == 4 before
cpu_detectbut the commit message needs a bit massaging before it gets applied.I think it fixes the window where we run with family > 4 before
cpu_detect:)
Yes, before cpu_detect, the family is set to 4. And with "family" I mean
#define X86 new_cpu_data+CPUINFO_x86
and not the hardware family of the CPU and what you mean is that when we boot 32-bit on a hardware family != 4, we run with X86 == 4 until cpu_detect runs.
So basically we're saying the same thing. :)
Quoted 5 lines
I can't catch the exactly meaning:
We can drop the
new_cpu_dataor
I need to drop this part commit
message.
Yes, this part of the commit message.
Thanks.
Signature · 5 lines
Regards/Gruss,
Boris.
Sent from a fat crate under my desk. Formatting is fine.
--