diff mbox

[RFC/PATCH,2/2] powerpc: 44x doesn't need G set everywhere

Message ID 20081210055139.A7FFCDDF65@ozlabs.org (mailing list archive)
State Accepted, archived
Commit 9dce3ce5c55c848f00429005a46fd6246cfabfbe
Delegated to: Josh Boyer
Headers show

Commit Message

Benjamin Herrenschmidt Dec. 10, 2008, 5:50 a.m. UTC
After discussing with chip designers, it appears that it's not
necessary to set G everywhere on 440 cores. The various core
errata related to prefetch should be sorted out by firmware by
disabling icache prefetching in CCR0. We add the workaround to
the kernel however just in case oooold firmwares don't do it.

This is valid for -all- 4xx core variants. Later ones hard wire
the absence of prefetch but it doesn't harm to clear the bits
in CCR0 (they should already be cleared anyway).

We still leave G=1 on the linear mapping for now, we need to
stop over-mapping RAM to be able to remove it.

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

 arch/powerpc/kernel/head_44x.S |   12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

Comments

Josh Boyer Dec. 10, 2008, 1:31 p.m. UTC | #1
On Wed, 10 Dec 2008 16:50:50 +1100
Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:

> After discussing with chip designers, it appears that it's not
> necessary to set G everywhere on 440 cores. The various core
> errata related to prefetch should be sorted out by firmware by
> disabling icache prefetching in CCR0. We add the workaround to
> the kernel however just in case oooold firmwares don't do it.
> 
> This is valid for -all- 4xx core variants. Later ones hard wire
> the absence of prefetch but it doesn't harm to clear the bits
> in CCR0 (they should already be cleared anyway).
> 
> We still leave G=1 on the linear mapping for now, we need to
> stop over-mapping RAM to be able to remove it.

Hm.  Over-mapping it has the nice advantage that we use as few pinned
TLB entries as possible.  For 440x6 cores with more than 256 MiB of
DRAM, you could theoretically use a single 1GiB TLB entry to map all
kernel DRAM.

Do you think the trade-offs of allowing speculative accesses are worth
the increased TLB pressure?  Large base pages will help with that in
some workloads, but others are still going to be TLB constrained.

I know, I'm probably paranoid.  But changing things like this around
without some kind of benchmark data or testcase to make sure we aren't
making it worse gives me the heebee-geebees.

josh
Benjamin Herrenschmidt Dec. 10, 2008, 8:03 p.m. UTC | #2
> > We still leave G=1 on the linear mapping for now, we need to
> > stop over-mapping RAM to be able to remove it.
> 
> Hm.  Over-mapping it has the nice advantage that we use as few pinned
> TLB entries as possible.  For 440x6 cores with more than 256 MiB of
> DRAM, you could theoretically use a single 1GiB TLB entry to map all
> kernel DRAM.

Ok well, there are several issues here.. see below

> Do you think the trade-offs of allowing speculative accesses are worth
> the increased TLB pressure?  Large base pages will help with that in
> some workloads, but others are still going to be TLB constrained.
> 
> I know, I'm probably paranoid.  But changing things like this around
> without some kind of benchmark data or testcase to make sure we aren't
> making it worse gives me the heebee-geebees.

Yup, which is why I'm not changing it yet :-) My initial thinking was
along the lines of: We can use up to 4 bolted TLB entries, that will
cover most classic memory configurations such as 256, 512 etc.... and
leave what doesn't fit to highmem.

However that fails miserably with 128M which is quite common.

Then I thought we could overmap and use G for things that don't quite
fit and remove G when we know we can do an exact mapping...

Then I though .. heh, first we know there is no speculative or
prefetched data access on 440. We also know that speculative /
prefetched instruction access is busted and must be disabled.

Thus can't we just both overmap and not have G ?

Needs testing of course :-) I'm waiting for an answer from the chip guys
here.

G=1 has some other impacts, such as preventing write combining I think,
re-ordering, and a few other things.

Cheers,
Ben.
Josh Boyer Dec. 10, 2008, 8:11 p.m. UTC | #3
On Thu, 11 Dec 2008 07:03:57 +1100
Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:

> 
> > > We still leave G=1 on the linear mapping for now, we need to
> > > stop over-mapping RAM to be able to remove it.
> > 
> > Hm.  Over-mapping it has the nice advantage that we use as few pinned
> > TLB entries as possible.  For 440x6 cores with more than 256 MiB of
> > DRAM, you could theoretically use a single 1GiB TLB entry to map all
> > kernel DRAM.
> 
> Ok well, there are several issues here.. see below
> 
> > Do you think the trade-offs of allowing speculative accesses are worth
> > the increased TLB pressure?  Large base pages will help with that in
> > some workloads, but others are still going to be TLB constrained.
> > 
> > I know, I'm probably paranoid.  But changing things like this around
> > without some kind of benchmark data or testcase to make sure we aren't
> > making it worse gives me the heebee-geebees.
> 
> Yup, which is why I'm not changing it yet :-) My initial thinking was
> along the lines of: We can use up to 4 bolted TLB entries, that will
> cover most classic memory configurations such as 256, 512 etc.... and
> leave what doesn't fit to highmem.
> 
> However that fails miserably with 128M which is quite common.
> 
> Then I thought we could overmap and use G for things that don't quite
> fit and remove G when we know we can do an exact mapping...
> 
> Then I though .. heh, first we know there is no speculative or
> prefetched data access on 440. We also know that speculative /
> prefetched instruction access is busted and must be disabled.
> 
> Thus can't we just both overmap and not have G ?
> 
> Needs testing of course :-) I'm waiting for an answer from the chip guys
> here.

Heh.  I like it.

> G=1 has some other impacts, such as preventing write combining I think,
> re-ordering, and a few other things.

Yeah.

Overall, I'm OK with changing things as long as we can sort of prove we
aren't making it worse.

josh
diff mbox

Patch

--- linux-work.orig/arch/powerpc/kernel/head_44x.S	2008-12-10 16:11:35.000000000 +1100
+++ linux-work/arch/powerpc/kernel/head_44x.S	2008-12-10 16:29:08.000000000 +1100
@@ -69,6 +69,17 @@  _ENTRY(_start);
 	li	r24,0		/* CPU number */
 
 /*
+ * In case the firmware didn't do it, we apply some workarounds
+ * that are good for all 440 core variants here
+ */
+	mfspr	r3,SPRN_CCR0
+	rlwinm	r3,r3,0,0,27	/* disable icache prefetch */
+	isync
+	mtspr	SPRN_CCR0,r3
+	isync
+	sync
+
+/*
  * Set up the initial MMU state
  *
  * We are still executing code at the virtual address
@@ -570,7 +581,6 @@  finish_tlb_load:
 	rlwimi	r10,r12,29,30,30		/* DIRTY -> SW position */
 	and	r11,r12,r10			/* Mask PTE bits to keep */
 	andi.	r10,r12,_PAGE_USER		/* User page ? */
-	ori	r11,r11,_PAGE_GUARDED		/* 440 errata, needs G set */
 	beq	1f				/* nope, leave U bits empty */
 	rlwimi	r11,r11,3,26,28			/* yes, copy S bits to U */
 1:	tlbwe	r11,r13,PPC44x_TLB_ATTRIB	/* Write ATTRIB */