[U-Boot-Users] PPC440EPx/sequoia TLB question...

[U-Boot-Users] PPC440EPx/sequoia TLB question...

[Dave Littell]

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

[U-Boot-Users] PPC440EPx/sequoia TLB question...

[Stefan Roese]

On Wednesday 23 April 2008, Dave Littell wrote:
> >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?

> 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
=====================================================================

[U-Boot-Users] PPC440EPx/sequoia TLB question...

[Dave Littell]

Stefan Roese wrote:
> On Wednesday 23 April 2008, Dave Littell wrote:
>> >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.

>> 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

[U-Boot-Users] PPC440EPx/sequoia TLB question...

[Stefan Roese]

On Thursday 24 April 2008, Dave Littell wrote:
> 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.

> 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.
>
> >> 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,

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.

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

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
=====================================================================

[U-Boot-Users] PPC440EPx/sequoia TLB question...

[Josh Boyer]

On Thu, 2008-04-24 at 07:28 +0200, Stefan Roese wrote:
> On Thursday 24 April 2008, Dave Littell wrote:
> > >> 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,
> 
> 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

[U-Boot-Users] PPC440EPx/sequoia TLB question...

[Stefan Roese]

On Thursday 24 April 2008, Josh Boyer wrote:
> > > 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

[U-Boot-Users] [PATCH] PPC440EPx/sequoia TLB question...

[Dave Littell]

Stefan Roese wrote:
> On Wednesday 23 April 2008, Dave Littell wrote:
>>> 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?
> 
>> 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 )

[U-Boot-Users] [PATCH] PPC440EPx/sequoia TLB question...

[Stefan Roese]

On Friday 25 April 2008, Dave Littell wrote:
> 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

=====================================================================
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
=====================================================================

[U-Boot-Users] PPC440EPx/sequoia TLB question...

[Dave Littell]

Stefan Roese wrote:
> On Wednesday 23 April 2008, Dave Littell wrote:
>> >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?
> 
>> 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.

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 )


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


Dave

[U-Boot-Users] PPC440EPx/sequoia TLB question...

[Stefan Roese]

On Saturday 26 April 2008, Dave Littell wrote:
> 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

=====================================================================
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
=====================================================================