Re: 2.6.22-rcX Transmeta/APM regression

Re: 2.6.22-rcX Transmeta/APM regression

[linux]

Okay, after a ridiculous amount of bisecting and recompiling and
rebooting...

First I had to find out that the kernel stops booting as of
bf50467204: "i386: Use per-cpu GDT immediately on boot"
(With theis commit, it silently stops booting.  The GP fault I
posted earlier comes a little later, but I didn't bother finding it.)

and starts again as of
b0b73cb41d: "i386: msr.h: be paranoid about types and parentheses"

However, one commit before the former suspends properly, and the latter
fails to suspend (exactly the same problem at get_fixed_ranges+0x9/0x60),
so I had to bisect further between the two, backporting the msr.h changes
across the msr-index.h splitoff.

Anyway, the patch which introduces the problem is the aptly named 3ebad:
3ebad59056: [PATCH] x86: Save and restore the fixed-range MTRRs of the BSP when suspending

2.6.22-rc6 plus that one commit reverted successfully does APM suspend
(and resume) for me.

Re: 2.6.22-rcX Transmeta/APM regression

[H. Peter Anvin]

linux@horizon.com wrote:
> 
> Anyway, the patch which introduces the problem is the aptly named 3ebad:
> 3ebad59056: [PATCH] x86: Save and restore the fixed-range MTRRs of the BSP when suspending
> 
> 2.6.22-rc6 plus that one commit reverted successfully does APM suspend
> (and resume) for me.

Okay, I would guess that that patch probably touches MTRRs without 
actually verify that the CPU *has* MTRRs -- the Transmeta Crusoe CPU 
doesn't have MTRRs.

	-hpa

Re: 2.6.22-rcX Transmeta/APM regression

[Andrew Morton]

On Sat, 30 Jun 2007 00:06:36 -0400 "H. Peter Anvin" <hpa@zytor.com> wrote:

> linux@horizon.com wrote:
> > 
> > Anyway, the patch which introduces the problem is the aptly named 3ebad:
> > 3ebad59056: [PATCH] x86: Save and restore the fixed-range MTRRs of the BSP when suspending
> > 
> > 2.6.22-rc6 plus that one commit reverted successfully does APM suspend
> > (and resume) for me.
> 
> Okay, I would guess that that patch probably touches MTRRs without 
> actually verify that the CPU *has* MTRRs -- the Transmeta Crusoe CPU 
> doesn't have MTRRs.
> 

This?

--- a/arch/i386/kernel/cpu/mtrr/generic.c~i386-mtrr-crash-fix
+++ a/arch/i386/kernel/cpu/mtrr/generic.c
@@ -65,7 +65,8 @@ get_fixed_ranges(mtrr_type * frs)
 
 void mtrr_save_fixed_ranges(void *info)
 {
-	get_fixed_ranges(mtrr_state.fixed_ranges);
+	if (cpu_has_mtrr)
+		get_fixed_ranges(mtrr_state.fixed_ranges);
 }
 
 static void print_fixed(unsigned base, unsigned step, const mtrr_type*types)
_

Re: 2.6.22-rcX Transmeta/APM regression

[Andi Kleen]

On Saturday 30 June 2007 05:03, linux@horizon.com wrote:

> Anyway, the patch which introduces the problem is the aptly named 3ebad:
> 3ebad59056: [PATCH] x86: Save and restore the fixed-range MTRRs of the BSP
> when suspending
>
> 2.6.22-rc6 plus that one commit reverted successfully does APM suspend
> (and resume) for me.

Mr.Linux, Does that patch fix it? 

-Andi

i386: Check if CPU has MTRRs before trying to save them

Signed-off-by: Andi Kleen <ak@suse.de>

Index: linux/arch/i386/kernel/cpu/mtrr/main.c
===================================================================
--- linux.orig/arch/i386/kernel/cpu/mtrr/main.c
+++ linux/arch/i386/kernel/cpu/mtrr/main.c
@@ -734,8 +734,11 @@ void mtrr_ap_init(void)
  */
 void mtrr_save_state(void)
 {
-	int cpu = get_cpu();
+	int cpu;
 
+	if (!cpu_has_mtrr)
+		return;
+	cpu  = get_cpu();
 	if (cpu == 0)
 		mtrr_save_fixed_ranges(NULL);
 	else

Re: 2.6.22-rcX Transmeta/APM regression

[linux]

Responding to various proposed fixes:

> Index: linux/arch/i386/kernel/cpu/mtrr/main.c
> ===================================================================
> --- linux.orig/arch/i386/kernel/cpu/mtrr/main.c
> +++ linux/arch/i386/kernel/cpu/mtrr/main.c
> @@ -734,8 +734,11 @@ void mtrr_ap_init(void)
>   */
>  void mtrr_save_state(void)
>  {
> -	int cpu = get_cpu();
> +	int cpu;
>  
> +	if (!cpu_has_mtrr)
> +		return;
> +	cpu  = get_cpu();
>  	if (cpu == 0)
>  		mtrr_save_fixed_ranges(NULL);
>  	else

This does not change the symptoms in any way.

> --- a/arch/i386/kernel/cpu/mtrr/generic.c~i386-mtrr-crash-fix
> +++ a/arch/i386/kernel/cpu/mtrr/generic.c
> @@ -65,7 +65,8 @@ get_fixed_ranges(mtrr_type * frs)
>  
>  void mtrr_save_fixed_ranges(void *info)
>  {
> -	get_fixed_ranges(mtrr_state.fixed_ranges);
> +	if (cpu_has_mtrr)
> +		get_fixed_ranges(mtrr_state.fixed_ranges);
>  }
>  
>  static void print_fixed(unsigned base, unsigned step, const mtrr_type*types)

This works great, thanks!  Please consider the regression diagnosed and fixed.

Re: 2.6.22-rcX Transmeta/APM regression

[Andrew Morton]

On 30 Jun 2007 14:57:06 -0400 linux@horizon.com wrote:

> > --- a/arch/i386/kernel/cpu/mtrr/generic.c~i386-mtrr-crash-fix
> > +++ a/arch/i386/kernel/cpu/mtrr/generic.c
> > @@ -65,7 +65,8 @@ get_fixed_ranges(mtrr_type * frs)
> >  
> >  void mtrr_save_fixed_ranges(void *info)
> >  {
> > -	get_fixed_ranges(mtrr_state.fixed_ranges);
> > +	if (cpu_has_mtrr)
> > +		get_fixed_ranges(mtrr_state.fixed_ranges);
> >  }
> >  
> >  static void print_fixed(unsigned base, unsigned step, const mtrr_type*types)
> 
> This works great, thanks!  Please consider the regression diagnosed and fixed.

OK, thanks.  I'll scoot that linuswards later today.