Geode LX boot fails after x86 microcode revision change

Geode LX boot fails after x86 microcode revision change

[Daniel Drake]

Hi,

Linux-3.1 works fine on OLPC XO-1 (Geode LX) but latest linus master
fails to boot. It hangs so early on that we don't even receive any
output over serial.

git bisect tracked it down to:


commit bcb80e53877c2045d9e52f4a71372c3fe6501f6f
Author: Borislav Petkov <borislav.petkov@amd.com>
Date:   Mon Oct 17 16:34:36 2011 +0200

    x86, microcode, AMD: Add microcode revision to /proc/cpuinfo

    Enable microcode revision output for AMD after 506ed6b53e00 ("x86,
    intel: Output microcode revision in /proc/cpuinfo") did it for Intel.

    Signed-off-by: Borislav Petkov <borislav.petkov@amd.com>


Any thoughts?

Thanks,
Daniel

Re: Geode LX boot fails after x86 microcode revision change

[Borislav Petkov]

On Tue, Nov 08, 2011 at 01:59:40PM -0600, Daniel Drake wrote:
> Hi,
> 
> Linux-3.1 works fine on OLPC XO-1 (Geode LX) but latest linus master
> fails to boot. It hangs so early on that we don't even receive any
> output over serial.
> 
> git bisect tracked it down to:
> 
> 
> commit bcb80e53877c2045d9e52f4a71372c3fe6501f6f
> Author: Borislav Petkov <borislav.petkov@amd.com>
> Date:   Mon Oct 17 16:34:36 2011 +0200
> 
>     x86, microcode, AMD: Add microcode revision to /proc/cpuinfo
> 
>     Enable microcode revision output for AMD after 506ed6b53e00 ("x86,
>     intel: Output microcode revision in /proc/cpuinfo") did it for Intel.
> 
>     Signed-off-by: Borislav Petkov <borislav.petkov@amd.com>
> 
> 
> Any thoughts?

Hmm,

it might be that the Geode doesn't have the patch level MSR.., can you
send /proc/cpuinfo from a working kernel please, say 3.1?

Although, I have rdmsr_safe in the patch there so it actually shouldn't
be an issue, theoretically.

Can you also install msr-tools and do

$ rdmsr 0x8b

as root on the working kernel? If it hangs there too, then we know why.

Also, can you boot if you comment out the rdmsr_safe line in
early_init_amd(), line 477:

// rdmsr_safe(MSR_AMD64_PATCH_LEVEL, &c->microcode, &dummy);

?

Thanks.

-- 
Regards/Gruss,
Boris.

Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
GM: Alberto Bozzo
Reg: Dornach, Landkreis Muenchen
HRB Nr. 43632 WEEE Registernr: 129 19551

Re: Geode LX boot fails after x86 microcode revision change

[H. Peter Anvin]

On 11/08/2011 12:31 PM, Borislav Petkov wrote:
> 
> it might be that the Geode doesn't have the patch level MSR.., can you
> send /proc/cpuinfo from a working kernel please, say 3.1?
> 
> Although, I have rdmsr_safe in the patch there so it actually shouldn't
> be an issue, theoretically.
> 

rdmsr_safe() requires exception table support, may not be available yet.

	-hpa

RE: Geode LX boot fails after x86 microcode revision change

[Petkov, Borislav]

> On 11/08/2011 12:31 PM, Borislav Petkov wrote:
> >
> > it might be that the Geode doesn't have the patch level MSR.., can you
> > send /proc/cpuinfo from a working kernel please, say 3.1?
> >
> > Although, I have rdmsr_safe in the patch there so it actually shouldn't
> > be an issue, theoretically.
> >
> 
> rdmsr_safe() requires exception table support, may not be available yet.
> 
>        -hpa

This could definitely explain why it hangs, the rdmsr_safe call is
 in early_init_amd(), so it could really be too early for exception tables.

If so, then a family check is unavoidable.

Thanks.

(Please forgive the formatting, I'm using the xchange crap)

Re: Geode LX boot fails after x86 microcode revision change

[Daniel Drake]

On Tue, Nov 8, 2011 at 4:13 PM, Petkov, Borislav
<Borislav.Petkov@amd.com> wrote:
> This could definitely explain why it hangs, the rdmsr_safe call is
>  in early_init_amd(), so it could really be too early for exception tables.

I think this is almost certainly the cause too. The symptoms of this
issue are exactly the same as the early boot hang once discussed in a
thread titled "Dynamic nop selection breaks boot on Geode LX" (which
you wrote to as well) which was a problem of running code before
having exception tables set up.

I haven't yet run your tests except for "rdmsr 0x8b" which returns
"pread: Input/output error" - let me know if you would still like
results from the other tests.

Thanks
Daniel

Re: Geode LX boot fails after x86 microcode revision change

[Borislav Petkov]

On Tue, Nov 08, 2011 at 04:17:21PM -0600, Daniel Drake wrote:
> On Tue, Nov 8, 2011 at 4:13 PM, Petkov, Borislav
> <Borislav.Petkov@amd.com> wrote:
> > This could definitely explain why it hangs, the rdmsr_safe call is
> >  in early_init_amd(), so it could really be too early for exception tables.
> 
> I think this is almost certainly the cause too. The symptoms of this
> issue are exactly the same as the early boot hang once discussed in a
> thread titled "Dynamic nop selection breaks boot on Geode LX" (which
> you wrote to as well) which was a problem of running code before
> having exception tables set up.

Oh yeah, that.

> I haven't yet run your tests except for "rdmsr 0x8b" which returns
> "pread: Input/output error" - let me know if you would still like
> results from the other tests.

Yep, just send me /proc/cpuinfo from a working kernel.

Thanks.

-- 
Regards/Gruss,
   Boris

Re: Geode LX boot fails after x86 microcode revision change

[Daniel Drake]

On Tue, Nov 8, 2011 at 4:37 PM, Borislav Petkov <bp@alien8.de> wrote:
> Yep, just send me /proc/cpuinfo from a working kernel.

processor	: 0
vendor_id	: AuthenticAMD
cpu family	: 5
model		: 10
model name	: Geode(TM) Integrated Processor by AMD PCS
stepping	: 2
cpu MHz		: 430.927
cache size	: 128 KB
fdiv_bug	: no
hlt_bug		: no
f00f_bug	: no
coma_bug	: no
fpu		: yes
fpu_exception	: yes
cpuid level	: 1
wp		: yes
flags		: fpu de pse tsc msr cx8 sep pge cmov clflush mmx mmxext 3dnowext 3dnow
bogomips	: 861.85
clflush size	: 32
cache_alignment	: 32
address sizes	: 32 bits physical, 32 bits virtual
power management:

Thanks,
Daniel

Re: Geode LX boot fails after x86 microcode revision change

[Borislav Petkov]

On Tue, Nov 08, 2011 at 04:59:05PM -0600, Daniel Drake wrote:
> On Tue, Nov 8, 2011 at 4:37 PM, Borislav Petkov <bp@alien8.de> wrote:
> > Yep, just send me /proc/cpuinfo from a working kernel.

Daniel, would you please test? It should work though :-)

Thanks.

--
>From daa04c94628d5686404bd6985d4545f12c2fa4fd Mon Sep 17 00:00:00 2001
From: Borislav Petkov <borislav.petkov@amd.com>
Date: Wed, 9 Nov 2011 18:08:40 +0100
Subject: [PATCH] x86, microcode, AMD: Restrict microcode reporting

bcb80e53877c ("x86, microcode, AMD: Add microcode revision to
/proc/cpuinfo") added support for dumping microcode patch level in
/proc/cpuinfo on AMD. Although we were very cautious and did rdmsr_safe
on the microcode patch level MSR, this broke booting on Geode LX because
this CPU doesn't have that MSR and at the time we do the read, exception
tables which are used by rdmsr_safe are not yet initialized, leading to
a boot hang.

This is the simple fix which enables the microcode line in /proc/cpuinfo
for K8 and newer (it is not that interesting for older boxes anyway
:-)).

Reported-and-bisected-by: Daniel Drake <dsd@laptop.org>
Link: http://lkml.kernel.org/r/CAMLZHHTSuHZoPbm_YfB8G05JA+xKCE4+Jmj4NW8mnZUHogQSBA@mail.gmail.com
Signed-off-by: Borislav Petkov <borislav.petkov@amd.com>
---
 arch/x86/kernel/cpu/amd.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
index c7e46cb..a72d183 100644
--- a/arch/x86/kernel/cpu/amd.c
+++ b/arch/x86/kernel/cpu/amd.c
@@ -474,7 +474,8 @@ static void __cpuinit early_init_amd(struct cpuinfo_x86 *c)
 	}
 #endif
 
-	rdmsr_safe(MSR_AMD64_PATCH_LEVEL, &c->microcode, &dummy);
+	if (c->x86 >= 0xf)
+		rdmsr_safe(MSR_AMD64_PATCH_LEVEL, &c->microcode, &dummy);
 }
 
 static void __cpuinit init_amd(struct cpuinfo_x86 *c)
-- 
1.7.8.rc0


-- 
Regards/Gruss,
Boris.

Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
GM: Alberto Bozzo
Reg: Dornach, Landkreis Muenchen
HRB Nr. 43632 WEEE Registernr: 129 19551

Re: Geode LX boot fails after x86 microcode revision change

[Srivatsa S. Bhat]

On 11/09/2011 11:04 PM, Borislav Petkov wrote:
> On Tue, Nov 08, 2011 at 04:59:05PM -0600, Daniel Drake wrote:
>> On Tue, Nov 8, 2011 at 4:37 PM, Borislav Petkov <bp@alien8.de> wrote:
>>> Yep, just send me /proc/cpuinfo from a working kernel.
> 
> Daniel, would you please test? It should work though :-)
> 
> Thanks.
> 
> --
> From daa04c94628d5686404bd6985d4545f12c2fa4fd Mon Sep 17 00:00:00 2001
> From: Borislav Petkov <borislav.petkov@amd.com>
> Date: Wed, 9 Nov 2011 18:08:40 +0100
> Subject: [PATCH] x86, microcode, AMD: Restrict microcode reporting
> 
> bcb80e53877c ("x86, microcode, AMD: Add microcode revision to
> /proc/cpuinfo") added support for dumping microcode patch level in
> /proc/cpuinfo on AMD. Although we were very cautious and did rdmsr_safe
> on the microcode patch level MSR, this broke booting on Geode LX because
> this CPU doesn't have that MSR and at the time we do the read, exception
> tables which are used by rdmsr_safe are not yet initialized, leading to
> a boot hang.
> 
> This is the simple fix which enables the microcode line in /proc/cpuinfo
> for K8 and newer (it is not that interesting for older boxes anyway
> :-)).
> 
> Reported-and-bisected-by: Daniel Drake <dsd@laptop.org>
> Link: http://lkml.kernel.org/r/CAMLZHHTSuHZoPbm_YfB8G05JA+xKCE4+Jmj4NW8mnZUHogQSBA@mail.gmail.com
            ^^^^^
I couldn't open this web-page!

Thanks,
Srivatsa S. Bhat

> Signed-off-by: Borislav Petkov <borislav.petkov@amd.com>
> ---
>  arch/x86/kernel/cpu/amd.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
> index c7e46cb..a72d183 100644
> --- a/arch/x86/kernel/cpu/amd.c
> +++ b/arch/x86/kernel/cpu/amd.c
> @@ -474,7 +474,8 @@ static void __cpuinit early_init_amd(struct cpuinfo_x86 *c)
>  	}
>  #endif
> 
> -	rdmsr_safe(MSR_AMD64_PATCH_LEVEL, &c->microcode, &dummy);
> +	if (c->x86 >= 0xf)
> +		rdmsr_safe(MSR_AMD64_PATCH_LEVEL, &c->microcode, &dummy);
>  }
> 
>  static void __cpuinit init_amd(struct cpuinfo_x86 *c)

Re: Geode LX boot fails after x86 microcode revision change

[Borislav Petkov]

On Thu, Nov 10, 2011 at 06:35:27PM +0530, Srivatsa S. Bhat wrote:
> > Reported-and-bisected-by: Daniel Drake <dsd@laptop.org>
> > Link: http://lkml.kernel.org/r/CAMLZHHTSuHZoPbm_YfB8G05JA+xKCE4+Jmj4NW8mnZUHogQSBA@mail.gmail.com
>             ^^^^^
> I couldn't open this web-page!

Patience my friend, patience :-)

For now, a search engine of your choice should help you.

-- 
Regards/Gruss,
Boris.

Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
GM: Alberto Bozzo
Reg: Dornach, Landkreis Muenchen
HRB Nr. 43632 WEEE Registernr: 129 19551

Re: Geode LX boot fails after x86 microcode revision change

[Srivatsa S. Bhat]

On 11/10/2011 06:58 PM, Borislav Petkov wrote:
> On Thu, Nov 10, 2011 at 06:35:27PM +0530, Srivatsa S. Bhat wrote:
>>> Reported-and-bisected-by: Daniel Drake <dsd@laptop.org>
>>> Link: http://lkml.kernel.org/r/CAMLZHHTSuHZoPbm_YfB8G05JA+xKCE4+Jmj4NW8mnZUHogQSBA@mail.gmail.com
>>             ^^^^^
>> I couldn't open this web-page!
> 
> Patience my friend, patience :-)
> 

Ha ha :-)

> For now, a search engine of your choice should help you.
> 

Anyway, I have followed the discussion over lkml of course :-)

Thanks,
Srivatsa S. Bhat

Re: Geode LX boot fails after x86 microcode revision change

[Srivatsa S. Bhat]

Apparently my mailer couldn't mail to bp@alien8.de and failed.
So resending with that address removed.

On 11/09/2011 11:04 PM, Borislav Petkov wrote:
> On Tue, Nov 08, 2011 at 04:59:05PM -0600, Daniel Drake wrote:
>> On Tue, Nov 8, 2011 at 4:37 PM, Borislav Petkov <bp@alien8.de> wrote:
>>> Yep, just send me /proc/cpuinfo from a working kernel.
> 
> Daniel, would you please test? It should work though :-)
> 
> Thanks.
> 
> --
> From daa04c94628d5686404bd6985d4545f12c2fa4fd Mon Sep 17 00:00:00 2001
> From: Borislav Petkov <borislav.petkov@amd.com>
> Date: Wed, 9 Nov 2011 18:08:40 +0100
> Subject: [PATCH] x86, microcode, AMD: Restrict microcode reporting
> 
> bcb80e53877c ("x86, microcode, AMD: Add microcode revision to
> /proc/cpuinfo") added support for dumping microcode patch level in
> /proc/cpuinfo on AMD. Although we were very cautious and did rdmsr_safe
> on the microcode patch level MSR, this broke booting on Geode LX because
> this CPU doesn't have that MSR and at the time we do the read, exception
> tables which are used by rdmsr_safe are not yet initialized, leading to
> a boot hang.
> 
> This is the simple fix which enables the microcode line in /proc/cpuinfo
> for K8 and newer (it is not that interesting for older boxes anyway
> :-)).
> 
> Reported-and-bisected-by: Daniel Drake <dsd@laptop.org>
> Link: http://lkml.kernel.org/r/CAMLZHHTSuHZoPbm_YfB8G05JA+xKCE4+Jmj4NW8mnZUHogQSBA@mail.gmail.com
                     ^^^^^^^
I couldn't open this web-page!

Thanks,
Srivatsa S. Bhat

> Signed-off-by: Borislav Petkov <borislav.petkov@amd.com>
> ---
>  arch/x86/kernel/cpu/amd.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
> index c7e46cb..a72d183 100644
> --- a/arch/x86/kernel/cpu/amd.c
> +++ b/arch/x86/kernel/cpu/amd.c
> @@ -474,7 +474,8 @@ static void __cpuinit early_init_amd(struct cpuinfo_x86 *c)
>  	}
>  #endif
> 
> -	rdmsr_safe(MSR_AMD64_PATCH_LEVEL, &c->microcode, &dummy);
> +	if (c->x86 >= 0xf)
> +		rdmsr_safe(MSR_AMD64_PATCH_LEVEL, &c->microcode, &dummy);
>  }
> 
>  static void __cpuinit init_amd(struct cpuinfo_x86 *c)

Re: Geode LX boot fails after x86 microcode revision change

[Daniel Drake]

On Wed, Nov 9, 2011 at 11:34 AM, Borislav Petkov <bp@amd64.org> wrote:
> On Tue, Nov 08, 2011 at 04:59:05PM -0600, Daniel Drake wrote:
>> On Tue, Nov 8, 2011 at 4:37 PM, Borislav Petkov <bp@alien8.de> wrote:
>> > Yep, just send me /proc/cpuinfo from a working kernel.
>
> Daniel, would you please test? It should work though :-)

Thanks, it boots now.

Daniel

Re: Geode LX boot fails after x86 microcode revision change

[Borislav Petkov]

On Fri, Nov 11, 2011 at 12:40:15PM -0600, Daniel Drake wrote:
> On Wed, Nov 9, 2011 at 11:34 AM, Borislav Petkov <bp@amd64.org> wrote:
> > On Tue, Nov 08, 2011 at 04:59:05PM -0600, Daniel Drake wrote:
> >> On Tue, Nov 8, 2011 at 4:37 PM, Borislav Petkov <bp@alien8.de> wrote:
> >> > Yep, just send me /proc/cpuinfo from a working kernel.
> >
> > Daniel, would you please test? It should work though :-)
> 
> Thanks, it boots now.

Cool, thanks, I'll add your Tested-by: and send out.

-- 
Regards/Gruss,
   Boris

Re: Geode LX boot fails after x86 microcode revision change

[H. Peter Anvin]

On 11/08/2011 02:13 PM, Petkov, Borislav wrote:
> 
> This could definitely explain why it hangs, the rdmsr_safe call is
>  in early_init_amd(), so it could really be too early for exception tables.
> 

Yes, it is.

> If so, then a family check is unavoidable.

Either that, or make changes so that we can handle exceptions earlier.
That would be useful for other reasons.

	-hpa

Re: Geode LX boot fails after x86 microcode revision change

[Borislav Petkov]

On Tue, Nov 08, 2011 at 02:19:29PM -0800, H. Peter Anvin wrote:
> Either that, or make changes so that we can handle exceptions earlier.
> That would be useful for other reasons.

I'd propose to do the trivial fix first so that Daniel can boot his box
and I'll look into the exception tables deal a bit later, when I get
unswamped.

Thanks.

-- 
Regards/Gruss,
   Boris

Re: Geode LX boot fails after x86 microcode revision change

[Borislav Petkov]

On Tue, Nov 08, 2011 at 02:19:29PM -0800, H. Peter Anvin wrote:
> > If so, then a family check is unavoidable.
> 
> Either that, or make changes so that we can handle exceptions earlier.
> That would be useful for other reasons.

Well, I did some experimenting with this by forcing a guest to #GP on a
rdmsr in kvm. What I got was the early_idt_handler's panic message:

"PANIC: early exception %02lx rip %lx:%lx error %lx cr2 %lx\n"

because of the following boot code flow:

x86_64_start_kernel
|-> ... set_intr_gate           /* register early_idt_handler */
|-> x86_64_start_reservations
    |-> start_kernel
        |-> setup_arch
        |-> early_cpu_init()    /*
				 * this is where we need the extable for
				 * rdmsr_safe
				 */
...
    |-> trap_init()             /* switch to default #GP handler */

and do_general_protection does the actual exception fixup. Now, the
whole setup_arch() is running with the initial traps and only after we
finish it and a bunch of other stuff, we switch to the #GP handler in
trap_init().

>From where I stand, maybe I could try to teach the early_idt_handler in
head_64.S to do fixup_exception() ?

Also, we might need the patches which sort extable at build time too,
which are floating around lkml currently.

Hmm...

-- 
Regards/Gruss,
Boris.

Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
GM: Alberto Bozzo
Reg: Dornach, Landkreis Muenchen
HRB Nr. 43632 WEEE Registernr: 129 19551

Re: Geode LX boot fails after x86 microcode revision change

[H. Peter Anvin]

On 11/22/2011 05:00 AM, Borislav Petkov wrote:
> 
> Also, we might need the patches which sort extable at build time too,
> which are floating around lkml currently.
> 
> Hmm...
> 

Yes, with the extable set up at build time there isn't really any strong
reason why we couldn't take exceptions very early.

I am supposed to dredge up the perfect hash code I already have for this
(I'm away from work this and next week to deal with personal stuff, so
it might be a bit slow.)

	-hpa

-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.