[PATCH] x86: Fix override new_cpu_data.x86 with 486

[PATCH] x86: Fix override new_cpu_data.x86 with 486

[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.

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.

Signed-off-by: Wang YanQing <udknight@gmail.com>
---
 arch/x86/kernel/head_32.S | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kernel/head_32.S b/arch/x86/kernel/head_32.S
index 73afd11..733a8ef 100644
--- a/arch/x86/kernel/head_32.S
+++ b/arch/x86/kernel/head_32.S
@@ -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
-- 
1.7.12.4.dirty

Re: [PATCH] x86: Fix override new_cpu_data.x86 with 486

[Borislav Petkov]

On Thu, Jun 27, 2013 at 12:26:40AM +0800, Wang YanQing wrote:
> 
> 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.

> 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.

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.

-- 
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

Re: [PATCH] x86: Fix override new_cpu_data.x86 with 486

[Borislav Petkov]

On Thu, Jun 27, 2013 at 12:26:40AM +0800, Wang YanQing wrote:
> 
> 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.

> 
> 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.

> 
> 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.

You can drop that part.

Thanks.

-- 
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

Re: [PATCH] x86: Fix override new_cpu_data.x86 with 486

[Wang YanQing]

On Thu, Jun 27, 2013 at 12:54:35PM +0200, Borislav Petkov wrote:
> On Thu, Jun 27, 2013 at 12:26:40AM +0800, Wang YanQing wrote:
> > 
> > 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.
> 
> > 
> > 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.

I think it fixes the window where we run with family > 4 before cpu_detect :)

> 
> > 
> > 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.
> 
> 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.

Re: [PATCH] x86: Fix override new_cpu_data.x86 with 486

[Borislav Petkov]

On Fri, Jun 28, 2013 at 08:56:42AM +0800, Wang YanQing wrote:
> > 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.
> 
> 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. :)

> I can't catch the exactly meaning:
> 
> We can drop the new_cpu_data or 
> I need to drop this part commit
> message.

Yes, this part of the commit message.

Thanks.

-- 
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--