PPC440EPx/sequoia TLB question...

U-Boot-Users New Dave Littell 3 participants 10 messages Apr 23, 2008 – Apr 29, 2008

lore

Share & tools

lore

Dave Littell

author

2008-04-23 03:11 UTC #0 lore raw

Hi all,

From ?/board/amcc/sequoia/init.S:

/* TLB-entry for Internal Registers & OCM */
tlbentry( 0xe0000000, SZ_16M, 0xe0000000, 0, AC_R|AC_W|AC_X|SA_I )

Why is this memory region not marked Guarded? It would seem to meet the definition of ?non-well-behaved?.

Also the TLB entry for SDRAM marks it Guarded, but that?s one area I would think wouldn't need to be Guarded.

Thanks,
Dave

Stefan Roese

2008-04-23 12:42 UTC #1 lore raw

Quoted 7 lines · Dave Littell

Quoted 1 line
From ?/board/amcc/sequoia/init.S:
/* TLB-entry for Internal Registers & OCM */
tlbentry( 0xe0000000, SZ_16M, 0xe0000000, 0, AC_R|AC_W|AC_X|SA_I )
Why is this memory region not marked Guarded? It would seem to meet the definition of ?non-well-behaved?.

Why do you think this is the case?

Quoted 2 lines

Also the TLB entry for SDRAM marks it Guarded, but that?s one area I would think wouldn't need to be Guarded.

This could be a mistake. Should work without G bis set too. Please give it a try and send a patch to fix it, if it works fine.

Thanks.

Best regards,
Stefan

=====================================================================
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office at denx.de
=====================================================================

Dave Littell

author

2008-04-24 02:27 UTC #2 lore raw

Quoted 11 lines · Stefan Roese

Quoted 7 lines · Dave Littell
Quoted 1 line
From ?/board/amcc/sequoia/init.S:
/* TLB-entry for Internal Registers & OCM */
tlbentry( 0xe0000000, SZ_16M, 0xe0000000, 0, AC_R|AC_W|AC_X|SA_I )
Why is this memory region not marked Guarded? It would seem to meet the definition of ?non-well-behaved?.
Why do you think this is the case?

Hi again, Stefan!

Well, there's registers in that address space, not unlike other registers in other TLB entries (PCI, BCSR, etc.) that are marked Guarded. I would think the same rationale would apply to the internal registers.

I need to go back and check the register settings for speculative accesses. I seem to remember that there's a 440EPx Errata (actually, more than one) that has a workaround that turns off speculative instruction fetches. Data speculative accesses may have gotten squashed in there as well, so it wouldn't matter what the TLB said if that's the case.

Quoted 6 lines

Quoted 2 lines
Also the TLB entry for SDRAM marks it Guarded, but that?s one area I would think wouldn't need to be Guarded.
This could be a mistake. Should work without G bis set too. Please give it a try and send a patch to fix it, if it works fine.

Hard to define "works fine" - this is the same 440EPx platform I'm asking about over in the embedded Linux mailing list. I'm pretty sure the kernel doesn't flag SDRAM as Guarded, but I'll give it a try to see how it goes.

Thanks,
Dave

Stefan Roese

2008-04-24 05:28 UTC #3 lore raw

Quoted 4 lines · Dave Littell

Well, there's registers in that address space, not unlike other registers in other TLB entries (PCI, BCSR, etc.) that are marked Guarded. I would think the same rationale would apply to the internal registers.

We should probably split this up in multiple TLB entries then. SRAM without G set and registers with G set.

Quoted 10 lines

I need to go back and check the register settings for speculative accesses. I seem to remember that there's a 440EPx Errata (actually, more than one) that has a workaround that turns off speculative instruction fetches. Data speculative accesses may have gotten squashed in there as well, so it wouldn't matter what the TLB said if that's the case.
Quoted 3 lines
Quoted 2 lines
Also the TLB entry for SDRAM marks it Guarded, but that?s one area I would think wouldn't need to be Guarded.

Quoted 6 lines

Quoted 2 lines
This could be a mistake. Should work without G bis set too. Please give it a try and send a patch to fix it, if it works fine.
Hard to define "works fine" - this is the same 440EPx platform I'm asking about over in the embedded Linux mailing list. I'm pretty sure the kernel doesn't flag SDRAM as Guarded,

Yes, and the 4xx code to dynamically set the SDRAM TLB's in the SPD code doesn't set it either. So it really isn't needed.

Quoted 2 lines

but I'll give it a try to see
how it goes.

Thanks.

Best regards,
Stefan

Josh Boyer

2008-04-24 11:58 UTC #4 lore raw

Quoted 7 lines · Stefan Roese

Quoted 6 lines · Dave Littell
Quoted 5 lines
Quoted 2 lines
Also the TLB entry for SDRAM marks it Guarded, but that?s one area I would think wouldn't need to be Guarded.
This could be a mistake. Should work without G bis set too. Please give it a try and send a patch to fix it, if it works fine.

Quoted 6 lines

Quoted 3 lines
Hard to define "works fine" - this is the same 440EPx platform I'm asking about over in the embedded Linux mailing list. I'm pretty sure the kernel doesn't flag SDRAM as Guarded,
Yes, and the 4xx code to dynamically set the SDRAM TLB's in the SPD code doesn't set it either. So it really isn't needed.

Actually, that's not true for kernel memory. We pin 256MiB TLBs to cover lowmem in AS0, and the Guarded bit is set. We set it to prevent speculative access to memory holes where the 256MiB TLB covers more address space than there is physical DRAM.

josh

Stefan Roese

2008-04-24 13:07 UTC #5 lore raw

Quoted 11 lines · Josh Boyer

Quoted 6 lines
Quoted 3 lines
Hard to define "works fine" - this is the same 440EPx platform I'm asking about over in the embedded Linux mailing list. I'm pretty sure the kernel doesn't flag SDRAM as Guarded,
Yes, and the 4xx code to dynamically set the SDRAM TLB's in the SPD code doesn't set it either. So it really isn't needed.
Actually, that's not true for kernel memory. We pin 256MiB TLBs to cover lowmem in AS0, and the Guarded bit is set. We set it to prevent speculative access to memory holes where the 256MiB TLB covers more address space than there is physical DRAM.

I wasn't aware of this. Thanks for the explanation.

Best regards,
Stefan

Dave Littell

author

2008-04-25 02:34 UTC #6 lore raw

PATCH 1/1 PPC440EPx/sequoia TLB question...

In reply to Stefan Roese

Quoted 9 lines · Stefan Roese

Quoted 7 lines · Dave Littell
Quoted 1 line
From ?/board/amcc/sequoia/init.S:
/* TLB-entry for Internal Registers & OCM */ tlbentry( 0xe0000000, SZ_16M, 0xe0000000, 0, AC_R|AC_W|AC_X|SA_I )
Why is this memory region not marked Guarded? It would seem to meet the definition of ?non-well-behaved?.

Quoted 8 lines

Why do you think this is the case?
Quoted 2 lines
Also the TLB entry for SDRAM marks it Guarded, but that?s one area I would think wouldn't need to be Guarded.
This could be a mistake. Should work without G bis set too. Please give it a try and send a patch to fix it, if it works fine.

Here goes:

Patch for AMCC Sequoia to remove the TLB Guarded attribute for SDRAM, and add the Guarded attribute for Internal Registers & OCM.

(Sorry if the wrap monster mangles this - I can't seem to convince Thunderbird to play nice.)

diff -purN u-boot-1.3.2_base/board/amcc/sequoia/init.S u-boot-1.3.2/board/amcc/sequoia/init.S
--- u-boot-1.3.2_base/board/amcc/sequoia/init.S 2008-03-09 10:20:02.000000000 -0500
+++ u-boot-1.3.2/board/amcc/sequoia/init.S 2008-04-24
21:24:17.542281994 -0500
@@ -45,9 +45,9 @@ tlbtab:

        /* TLB-entry for DDR SDRAM (Up to 2GB) */

#ifdef CONFIG_4xx_DCACHE
- tlbentry( CFG_SDRAM_BASE, SZ_256M, CFG_SDRAM_BASE, 0,
AC_R|AC_W|AC_X|SA_G)
+ tlbentry( CFG_SDRAM_BASE, SZ_256M, CFG_SDRAM_BASE, 0,
AC_R|AC_W|AC_X)
#else
- tlbentry( CFG_SDRAM_BASE, SZ_256M, CFG_SDRAM_BASE, 0,
AC_R|AC_W|AC_X|SA_G|SA_I )
+ tlbentry( CFG_SDRAM_BASE, SZ_256M, CFG_SDRAM_BASE, 0,
AC_R|AC_W|AC_X|SA_I )
#endif

        /* TLB-entry for EBC */

@@ -77,7 +77,7 @@ tlbtab:
tlbentry( CFG_NAND_ADDR, SZ_1K, CFG_NAND_ADDR, 1,
AC_R|AC_W|AC_X|SA_G|SA_I )

        /* TLB-entry for Internal Registers & OCM */

- tlbentry( 0xe0000000, SZ_16M, 0xe0000000, 0, AC_R|AC_W|AC_X|SA_I )
+ tlbentry( 0xe0000000, SZ_16M, 0xe0000000, 0,
AC_R|AC_W|AC_X|SA_G|SA_I )

        /*TLB-entry PCI registers*/
        tlbentry( 0xEEC00000, SZ_1K, 0xEEC00000, 1,

AC_R|AC_W|AC_X|SA_G|SA_I )

Stefan Roese

2008-04-25 05:31 UTC #7 lore raw

PATCH 1/1 PPC440EPx/sequoia TLB question...

Quoted 5 lines · Dave Littell

Patch for AMCC Sequoia to remove the TLB Guarded attribute for SDRAM, and add the Guarded attribute for Internal Registers & OCM.
(Sorry if the wrap monster mangles this - I can't seem to convince Thunderbird to play nice.)

Yes, it's wrapped. Please take a look at this page:

http://kerneltrap.org/Linux/Email_Clients_and_Patches

Hope this helps.

Best regards,
Stefan

Dave Littell

author

2008-04-26 01:54 UTC #8 lore raw

In reply to Stefan Roese

Quoted 9 lines · Stefan Roese

Quoted 7 lines · Dave Littell
Quoted 1 line
From ?/board/amcc/sequoia/init.S:
/* TLB-entry for Internal Registers & OCM */
tlbentry( 0xe0000000, SZ_16M, 0xe0000000, 0, AC_R|AC_W|AC_X|SA_I )
Why is this memory region not marked Guarded? It would seem to meet the definition of ?non-well-behaved?.

Quoted 8 lines

Why do you think this is the case?
Quoted 2 lines
Also the TLB entry for SDRAM marks it Guarded, but that?s one area I would think wouldn't need to be Guarded.
This could be a mistake. Should work without G bis set too. Please give it a try and send a patch to fix it, if it works fine.

Here goes (again):

Patch for AMCC Sequoia to remove the TLB Guarded attribute for SDRAM, and add the Guarded attribute for Internal Registers & OCM.

board/amcc/sequoia/init.S 2008-04-24 21:24:17.542281994 -0500 renamed from board/amcc/sequoia/init.S 2008-03-09 10:20:02.000000000 -0500 +3 -3

diff -purN u-boot-1.3.2_base/board/amcc/sequoia/init.S u-boot-1.3.2/board/amcc/sequoia/init.S
@@ -45,9 +45,9 @@ tlbtab:
 
        /* TLB-entry for DDR SDRAM (Up to 2GB) */
 #ifdef CONFIG_4xx_DCACHE
-       tlbentry( CFG_SDRAM_BASE, SZ_256M, CFG_SDRAM_BASE, 0, AC_R|AC_W|AC_X|SA_G)
+       tlbentry( CFG_SDRAM_BASE, SZ_256M, CFG_SDRAM_BASE, 0, AC_R|AC_W|AC_X)
 #else
-       tlbentry( CFG_SDRAM_BASE, SZ_256M, CFG_SDRAM_BASE, 0, AC_R|AC_W|AC_X|SA_G|SA_I )
+       tlbentry( CFG_SDRAM_BASE, SZ_256M, CFG_SDRAM_BASE, 0, AC_R|AC_W|AC_X|SA_I )
 #endif
 
        /* TLB-entry for EBC */

@@ -77,7 +77,7 @@ tlbtab:
        tlbentry( CFG_NAND_ADDR, SZ_1K, CFG_NAND_ADDR, 1, AC_R|AC_W|AC_X|SA_G|SA_I )
 
        /* TLB-entry for Internal Registers & OCM */
-       tlbentry( 0xe0000000, SZ_16M, 0xe0000000, 0,  AC_R|AC_W|AC_X|SA_I )
+       tlbentry( 0xe0000000, SZ_16M, 0xe0000000, 0,  AC_R|AC_W|AC_X|SA_G|SA_I )
 
        /*TLB-entry PCI registers*/
        tlbentry( 0xEEC00000, SZ_1K, 0xEEC00000, 1,  AC_R|AC_W|AC_X|SA_G|SA_I )

I sure hope this is better (from a line-wrapping perspective).

Dave

Stefan Roese

2008-04-29 05:54 UTC #9 lore raw

Quoted 4 lines · Dave Littell

Here goes (again):
Patch for AMCC Sequoia to remove the TLB Guarded attribute for SDRAM, and add the Guarded attribute for Internal Registers & OCM.

I still have a few issues with this patch:

- Signed-off-by line missing
- Please use a descriptive subject for this patch, something like:
"[PATCH] ppc4xx: Fix guarded bit in some Sequoia TLB entries"
- Comments that should not be part of the commit text should go below the "---" line in the email. Please take a look at other patches sent to the list on how exactly this should be done.
- And best would be to create two separate TLB entries, one for internal SRAM (OCM) with G cleared and one for internal registers with G set

I strongly suggest to work with the git tools to create and send patches. This makes life a lot easier for all of us.

Thanks.

Best regards,
Stefan