linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
* Re: [PATCH] powerpc: Fix some 6xx/7xxx CPU setup functions
       [not found] <1295591723.2148.281.camel__36384.4125684865$1295591768$gmane$org@pasglop>
@ 2011-01-21  9:48 ` Andreas Schwab
  2011-01-21 10:00   ` Benjamin Herrenschmidt
  0 siblings, 1 reply; 5+ messages in thread
From: Andreas Schwab @ 2011-01-21  9:48 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linuxppc-dev

Benjamin Herrenschmidt <benh@kernel.crashing.org> writes:

> In absence of good testing I would appreciate a close inspection of the patch
> by different pairs of eyes :-)

Looks good to me.  That broke apparently in 400d221 ("ppc32: make
cur_cpu_spec a single pointer instead of an array"), which changed the
calling convention in call_setup_cpu.

Andreas.

-- 
Andreas Schwab, schwab@redhat.com
GPG Key fingerprint = D4E8 DBE3 3813 BB5D FA84  5EC7 45C6 250E 6F00 984E
"And now for something completely different."

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] powerpc: Fix some 6xx/7xxx CPU setup functions
  2011-01-21  9:48 ` [PATCH] powerpc: Fix some 6xx/7xxx CPU setup functions Andreas Schwab
@ 2011-01-21 10:00   ` Benjamin Herrenschmidt
  0 siblings, 0 replies; 5+ messages in thread
From: Benjamin Herrenschmidt @ 2011-01-21 10:00 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: linuxppc-dev

On Fri, 2011-01-21 at 10:48 +0100, Andreas Schwab wrote:
> Benjamin Herrenschmidt <benh@kernel.crashing.org> writes:
> 
> > In absence of good testing I would appreciate a close inspection of the patch
> > by different pairs of eyes :-)
> 
> Looks good to me.  That broke apparently in 400d221 ("ppc32: make
> cur_cpu_spec a single pointer instead of an array"), which changed the
> calling convention in call_setup_cpu.

Nice, >5 years ago and it's not even my fault :-) Thanks for the
review !

Cheers,
Ben.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] powerpc: Fix some 6xx/7xxx CPU setup functions
  2011-01-21 18:15 ` kevin diggs
@ 2011-01-21 21:16   ` Benjamin Herrenschmidt
  0 siblings, 0 replies; 5+ messages in thread
From: Benjamin Herrenschmidt @ 2011-01-21 21:16 UTC (permalink / raw)
  To: kevin diggs; +Cc: linuxppc-dev

On Fri, 2011-01-21 at 12:15 -0600, kevin diggs wrote:

> I have a GigE (PowerMac 3,4?) with an upgrade card that has a pair of
> 7455s on it and an 8600 with a 750GX cpu card. I can probably test
> this on the GigE. It is running 2.6.36. Is that recent enough? The
> 8600 is not cooperating.
> 
> The GigE is running 2.6.36 compiled with 4.3.5 built from sources. It
> seems to run fine. 

The bug would have caused the kernel to allow enabling of "nap" mode
when idle on machines where this is buggy and can cause loss of cache
coherency among others. I really don't remember off hand which specific
machines may or may not have been affected. But I'd like at least people
to verify that my patches to "fix" that aren't causing a regression for
people like you who don't seem to have problems :-)

The bug was introduced a while back in 2005.

Cheers,
Ben.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] powerpc: Fix some 6xx/7xxx CPU setup functions
  2011-01-21  6:35 Benjamin Herrenschmidt
@ 2011-01-21 18:15 ` kevin diggs
  2011-01-21 21:16   ` Benjamin Herrenschmidt
  0 siblings, 1 reply; 5+ messages in thread
From: kevin diggs @ 2011-01-21 18:15 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, linuxppc-dev

On Fri, Jan 21, 2011 at 12:35 AM, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
> Some of those functions try to adjust the CPU features, for example
> to remove NAP support on some revisions. However, they seem to use
> r5 as an index into the CPU table entry, which might have been right
> a long time ago but no longer is. r4 is the right register to use.
Can you quantify 'a long time ago'? Is this compiler dependent?
>
> This probably caused some off behaviours on some PowerMac variants
> using 750cx or 7455 processor revisions.
what about a 750gx?
>
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> ---
>
> Can somebody with one of these PowerMacs test this ? I only managed to
> find a 7448 which not directly affected...
>
I have a GigE (PowerMac 3,4?) with an upgrade card that has a pair of
7455s on it and an 8600 with a 750GX cpu card. I can probably test
this on the GigE. It is running 2.6.36. Is that recent enough? The
8600 is not cooperating.

The GigE is running 2.6.36 compiled with 4.3.5 built from sources. It
seems to run fine.
_______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH] powerpc: Fix some 6xx/7xxx CPU setup functions
@ 2011-01-21  6:35 Benjamin Herrenschmidt
  2011-01-21 18:15 ` kevin diggs
  0 siblings, 1 reply; 5+ messages in thread
From: Benjamin Herrenschmidt @ 2011-01-21  6:35 UTC (permalink / raw)
  To: linuxppc-dev

Some of those functions try to adjust the CPU features, for example
to remove NAP support on some revisions. However, they seem to use
r5 as an index into the CPU table entry, which might have been right
a long time ago but no longer is. r4 is the right register to use.

This probably caused some off behaviours on some PowerMac variants
using 750cx or 7455 processor revisions.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---

Can somebody with one of these PowerMacs test this ? I only managed to
find a 7448 which not directly affected...

In absence of good testing I would appreciate a close inspection of the patch
by different pairs of eyes :-)

Cheers,
Ben.

diff --git a/arch/powerpc/kernel/cpu_setup_6xx.S b/arch/powerpc/kernel/cpu_setup_6xx.S
index 55cba4a..f8cd9fb 100644
--- a/arch/powerpc/kernel/cpu_setup_6xx.S
+++ b/arch/powerpc/kernel/cpu_setup_6xx.S
@@ -18,7 +18,7 @@
 #include <asm/mmu.h>
 
 _GLOBAL(__setup_cpu_603)
-	mflr	r4
+	mflr	r5
 BEGIN_MMU_FTR_SECTION
 	li	r10,0
 	mtspr	SPRN_SPRG_603_LRU,r10		/* init SW LRU tracking */
@@ -27,60 +27,60 @@ BEGIN_FTR_SECTION
 	bl	__init_fpu_registers
 END_FTR_SECTION_IFCLR(CPU_FTR_FPU_UNAVAILABLE)
 	bl	setup_common_caches
-	mtlr	r4
+	mtlr	r5
 	blr
 _GLOBAL(__setup_cpu_604)
-	mflr	r4
+	mflr	r5
 	bl	setup_common_caches
 	bl	setup_604_hid0
-	mtlr	r4
+	mtlr	r5
 	blr
 _GLOBAL(__setup_cpu_750)
-	mflr	r4
+	mflr	r5
 	bl	__init_fpu_registers
 	bl	setup_common_caches
 	bl	setup_750_7400_hid0
-	mtlr	r4
+	mtlr	r5
 	blr
 _GLOBAL(__setup_cpu_750cx)
-	mflr	r4
+	mflr	r5
 	bl	__init_fpu_registers
 	bl	setup_common_caches
 	bl	setup_750_7400_hid0
 	bl	setup_750cx
-	mtlr	r4
+	mtlr	r5
 	blr
 _GLOBAL(__setup_cpu_750fx)
-	mflr	r4
+	mflr	r5
 	bl	__init_fpu_registers
 	bl	setup_common_caches
 	bl	setup_750_7400_hid0
 	bl	setup_750fx
-	mtlr	r4
+	mtlr	r5
 	blr
 _GLOBAL(__setup_cpu_7400)
-	mflr	r4
+	mflr	r5
 	bl	__init_fpu_registers
 	bl	setup_7400_workarounds
 	bl	setup_common_caches
 	bl	setup_750_7400_hid0
-	mtlr	r4
+	mtlr	r5
 	blr
 _GLOBAL(__setup_cpu_7410)
-	mflr	r4
+	mflr	r5
 	bl	__init_fpu_registers
 	bl	setup_7410_workarounds
 	bl	setup_common_caches
 	bl	setup_750_7400_hid0
 	li	r3,0
 	mtspr	SPRN_L2CR2,r3
-	mtlr	r4
+	mtlr	r5
 	blr
 _GLOBAL(__setup_cpu_745x)
-	mflr	r4
+	mflr	r5
 	bl	setup_common_caches
 	bl	setup_745x_specifics
-	mtlr	r4
+	mtlr	r5
 	blr
 
 /* Enable caches for 603's, 604, 750 & 7400 */
@@ -194,10 +194,10 @@ setup_750cx:
 	cror	4*cr0+eq,4*cr0+eq,4*cr1+eq
 	cror	4*cr0+eq,4*cr0+eq,4*cr2+eq
 	bnelr
-	lwz	r6,CPU_SPEC_FEATURES(r5)
+	lwz	r6,CPU_SPEC_FEATURES(r4)
 	li	r7,CPU_FTR_CAN_NAP
 	andc	r6,r6,r7
-	stw	r6,CPU_SPEC_FEATURES(r5)
+	stw	r6,CPU_SPEC_FEATURES(r4)
 	blr
 
 /* 750fx specific
@@ -225,12 +225,12 @@ BEGIN_FTR_SECTION
 	andis.	r11,r11,L3CR_L3E@h
 	beq	1f
 END_FTR_SECTION_IFSET(CPU_FTR_L3CR)
-	lwz	r6,CPU_SPEC_FEATURES(r5)
+	lwz	r6,CPU_SPEC_FEATURES(r4)
 	andi.	r0,r6,CPU_FTR_L3_DISABLE_NAP
 	beq	1f
 	li	r7,CPU_FTR_CAN_NAP
 	andc	r6,r6,r7
-	stw	r6,CPU_SPEC_FEATURES(r5)
+	stw	r6,CPU_SPEC_FEATURES(r4)
 1:
 	mfspr	r11,SPRN_HID0
 

^ permalink raw reply related	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2011-01-21 21:16 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <1295591723.2148.281.camel__36384.4125684865$1295591768$gmane$org@pasglop>
2011-01-21  9:48 ` [PATCH] powerpc: Fix some 6xx/7xxx CPU setup functions Andreas Schwab
2011-01-21 10:00   ` Benjamin Herrenschmidt
2011-01-21  6:35 Benjamin Herrenschmidt
2011-01-21 18:15 ` kevin diggs
2011-01-21 21:16   ` Benjamin Herrenschmidt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).