* [U-Boot] [PATCH] powerpc/mpc85xx: set L2PE in L2CSR0 before enabling L2 cache
@ 2016-04-18 17:28 Aneesh Bansal
2016-04-19 16:32 ` York Sun
2016-05-25 3:33 ` York Sun
0 siblings, 2 replies; 7+ messages in thread
From: Aneesh Bansal @ 2016-04-18 17:28 UTC (permalink / raw)
To: u-boot
While enabling L2 cache, the value of L2PE (L2 cache parity/ECC
error checking enable) must not be changed while the L2 cache is
enabled.
So, L2PE must be set before enabling L2 cache.
Signed-off-by: Aneesh Bansal <aneesh.bansal@nxp.com>
---
arch/powerpc/cpu/mpc85xx/start.S | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)
diff --git a/arch/powerpc/cpu/mpc85xx/start.S b/arch/powerpc/cpu/mpc85xx/start.S
index 82a151a..4c51225 100644
--- a/arch/powerpc/cpu/mpc85xx/start.S
+++ b/arch/powerpc/cpu/mpc85xx/start.S
@@ -720,16 +720,39 @@ enable_l2_cluster_l2:
ori r4, r4, (L2CSR0_L2FI|L2CSR0_L2LFC)@l
sync
stw r4, 0(r3) /* invalidate L2 */
+ /* Poll till the bits are cleared */
1: sync
lwz r0, 0(r3)
twi 0, r0, 0
isync
and. r1, r0, r4
bne 1b
+
+ /* L2PE must be set before L2 cache is enabled */
+ lis r4, (L2CSR0_L2PE)@h
+ ori r4, r4, (L2CSR0_L2PE)@l
+ sync
+ stw r4, 0(r3) /* enable L2 parity/ECC error checking */
+ /* Poll till the bit is set */
+1: sync
+ lwz r0, 0(r3)
+ twi 0, r0, 0
+ isync
+ and. r1, r0, r4
+ beq 1b
+
lis r4, (L2CSR0_L2E|L2CSR0_L2PE)@h
ori r4, r4, (L2CSR0_L2REP_MODE)@l
sync
stw r4, 0(r3) /* enable L2 */
+ /* Poll till the bit is set */
+1: sync
+ lwz r0, 0(r3)
+ twi 0, r0, 0
+ isync
+ and. r1, r0, r4
+ beq 1b
+
delete_ccsr_l2_tlb:
delete_tlb0_entry 0, CONFIG_SYS_CCSRBAR + 0xC20000, MAS2_I|MAS2_G, r3
#endif
--
1.8.1.4
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [U-Boot] [PATCH] powerpc/mpc85xx: set L2PE in L2CSR0 before enabling L2 cache
2016-04-18 17:28 [U-Boot] [PATCH] powerpc/mpc85xx: set L2PE in L2CSR0 before enabling L2 cache Aneesh Bansal
@ 2016-04-19 16:32 ` York Sun
2016-04-20 11:06 ` Aneesh Bansal
2016-05-25 3:33 ` York Sun
1 sibling, 1 reply; 7+ messages in thread
From: York Sun @ 2016-04-19 16:32 UTC (permalink / raw)
To: u-boot
On 04/18/2016 05:16 AM, Aneesh Bansal wrote:
> While enabling L2 cache, the value of L2PE (L2 cache parity/ECC
> error checking enable) must not be changed while the L2 cache is
> enabled.
> So, L2PE must be set before enabling L2 cache.
Aneesh,
The original code set L2PE and L2E together. The L2PE bit doesn't change after
that. Doesn't this satisfy the requirement? Did you observe any failure before
your patch?
York
^ permalink raw reply [flat|nested] 7+ messages in thread
* [U-Boot] [PATCH] powerpc/mpc85xx: set L2PE in L2CSR0 before enabling L2 cache
2016-04-19 16:32 ` York Sun
@ 2016-04-20 11:06 ` Aneesh Bansal
2016-04-20 15:28 ` York Sun
0 siblings, 1 reply; 7+ messages in thread
From: Aneesh Bansal @ 2016-04-20 11:06 UTC (permalink / raw)
To: u-boot
> -----Original Message-----
> From: York Sun [mailto:york.sun at nxp.com]
> Sent: Tuesday, April 19, 2016 10:03 PM
> To: Aneesh Bansal <aneesh.bansal@nxp.com>; u-boot at lists.denx.de
> Cc: Ruchika Gupta <ruchika.gupta@nxp.com>; Prabhakar Kushwaha
> <prabhakar.kushwaha@nxp.com>
> Subject: Re: [PATCH] powerpc/mpc85xx: set L2PE in L2CSR0 before enabling L2
> cache
>
> On 04/18/2016 05:16 AM, Aneesh Bansal wrote:
> > While enabling L2 cache, the value of L2PE (L2 cache parity/ECC error
> > checking enable) must not be changed while the L2 cache is enabled.
> > So, L2PE must be set before enabling L2 cache.
>
> Aneesh,
>
> The original code set L2PE and L2E together. The L2PE bit doesn't change after that.
> Doesn't this satisfy the requirement? Did you observe any failure before your patch?
>
> York
e6500 block guide states that "The value of L2PE must not be changed while the L2 cache is enabled"
So, when both the bits are set together, it might lead to L2 cache getting enabled first and L2PE getting
set after that. So L2PE is getting changed from 0 to 1 while L2 is still enabled which should not be done.
In normal non-secure boot, U-Boot is the first to use L2 after reset but in case of secure boot, L2 is used
by Bot ROM before U-Boot. If L2PE and L2E are done together, ECC errors are observed on L2
(L2CAPTECC - L2 cache error capture ECC syndrome) and U-Boot crashes.
I believe this is because of ECC/Parity checking not getting enabled properly and resulting into
erroneous detection of errors
When this is changed to setting L2PE before L2E, or not setting L2PE at all i.e. disabling ECC error checks,
no ECC errors are observed and U-Boot works fine.
Aneesh
^ permalink raw reply [flat|nested] 7+ messages in thread
* [U-Boot] [PATCH] powerpc/mpc85xx: set L2PE in L2CSR0 before enabling L2 cache
2016-04-20 11:06 ` Aneesh Bansal
@ 2016-04-20 15:28 ` York Sun
2016-04-20 16:27 ` Aneesh Bansal
0 siblings, 1 reply; 7+ messages in thread
From: York Sun @ 2016-04-20 15:28 UTC (permalink / raw)
To: u-boot
On 04/20/2016 04:06 AM, Aneesh Bansal wrote:
>> -----Original Message-----
>> From: York Sun [mailto:york.sun at nxp.com]
>> Sent: Tuesday, April 19, 2016 10:03 PM
>> To: Aneesh Bansal <aneesh.bansal@nxp.com>; u-boot at lists.denx.de
>> Cc: Ruchika Gupta <ruchika.gupta@nxp.com>; Prabhakar Kushwaha
>> <prabhakar.kushwaha@nxp.com>
>> Subject: Re: [PATCH] powerpc/mpc85xx: set L2PE in L2CSR0 before enabling L2
>> cache
>>
>> On 04/18/2016 05:16 AM, Aneesh Bansal wrote:
>>> While enabling L2 cache, the value of L2PE (L2 cache parity/ECC error
>>> checking enable) must not be changed while the L2 cache is enabled.
>>> So, L2PE must be set before enabling L2 cache.
>>
>> Aneesh,
>>
>> The original code set L2PE and L2E together. The L2PE bit doesn't change after that.
>> Doesn't this satisfy the requirement? Did you observe any failure before your patch?
>>
>> York
>
> e6500 block guide states that "The value of L2PE must not be changed while the L2 cache is enabled"
> So, when both the bits are set together, it might lead to L2 cache getting enabled first and L2PE getting
> set after that. So L2PE is getting changed from 0 to 1 while L2 is still enabled which should not be done.
>
> In normal non-secure boot, U-Boot is the first to use L2 after reset but in case of secure boot, L2 is used
> by Bot ROM before U-Boot. If L2PE and L2E are done together, ECC errors are observed on L2
> (L2CAPTECC - L2 cache error capture ECC syndrome) and U-Boot crashes.
>
> I believe this is because of ECC/Parity checking not getting enabled properly and resulting into
> erroneous detection of errors
>
> When this is changed to setting L2PE before L2E, or not setting L2PE at all i.e. disabling ECC error checks,
> no ECC errors are observed and U-Boot works fine.
>
Aneesh,
You said for secure boot L2 cache was used by bootrom before U-Boot. Could the
L2 cache be left enabled when U-Boot runs? If true, that indeed sets L2PE bit
while L2E is enabled. Please confirm.
If L2E was left set by secure boot, your change actually fixes it, but the
commit message needs to be rewritten.
York
^ permalink raw reply [flat|nested] 7+ messages in thread
* [U-Boot] [PATCH] powerpc/mpc85xx: set L2PE in L2CSR0 before enabling L2 cache
2016-04-20 15:28 ` York Sun
@ 2016-04-20 16:27 ` Aneesh Bansal
2016-04-20 16:29 ` York Sun
0 siblings, 1 reply; 7+ messages in thread
From: Aneesh Bansal @ 2016-04-20 16:27 UTC (permalink / raw)
To: u-boot
> >>> While enabling L2 cache, the value of L2PE (L2 cache parity/ECC
> >>> error checking enable) must not be changed while the L2 cache is enabled.
> >>> So, L2PE must be set before enabling L2 cache.
> >>
> >> Aneesh,
> >>
> >> The original code set L2PE and L2E together. The L2PE bit doesn't change after
> that.
> >> Doesn't this satisfy the requirement? Did you observe any failure before your
> patch?
> >>
> >> York
> >
> > e6500 block guide states that "The value of L2PE must not be changed while the L2
> cache is enabled"
> > So, when both the bits are set together, it might lead to L2 cache
> > getting enabled first and L2PE getting set after that. So L2PE is getting changed from
> 0 to 1 while L2 is still enabled which should not be done.
> >
> > In normal non-secure boot, U-Boot is the first to use L2 after reset
> > but in case of secure boot, L2 is used by Bot ROM before U-Boot. If
> > L2PE and L2E are done together, ECC errors are observed on L2 (L2CAPTECC - L2
> cache error capture ECC syndrome) and U-Boot crashes.
> >
> > I believe this is because of ECC/Parity checking not getting enabled
> > properly and resulting into erroneous detection of errors
> >
> > When this is changed to setting L2PE before L2E, or not setting L2PE
> > at all i.e. disabling ECC error checks, no ECC errors are observed and U-Boot works
> fine.
> >
>
> Aneesh,
>
> You said for secure boot L2 cache was used by bootrom before U-Boot. Could the
> L2 cache be left enabled when U-Boot runs? If true, that indeed sets L2PE bit while
> L2E is enabled. Please confirm.
>
> If L2E was left set by secure boot, your change actually fixes it, but the commit
> message needs to be rewritten.
L2 is not left enabled by Boot ROM in case of Secure Boot. The IBR code enables
L2 but disables it before transferring control to U-Boot.
What I was trying to suggest is that the way we are enabling L2 in U-Boot seems to
be incorrect as we are setting L2PE and L2E simultaneously. This does not cause any
issues in case of non-secure boot as U-Boot is the first entity to enable and use L2 cache.
But in case of secure boot, L2 is enabled, used and disabled by Boot ROM first. Now in
U-Boot, if ECC/Parity checking is not enabled correctly then it starts reporting ECC errors
randomly and U-Boot crashes.
After making this change, no ECC errors are observed and U-Boot starts working correct
in case of non-secure boot.
^ permalink raw reply [flat|nested] 7+ messages in thread
* [U-Boot] [PATCH] powerpc/mpc85xx: set L2PE in L2CSR0 before enabling L2 cache
2016-04-20 16:27 ` Aneesh Bansal
@ 2016-04-20 16:29 ` York Sun
0 siblings, 0 replies; 7+ messages in thread
From: York Sun @ 2016-04-20 16:29 UTC (permalink / raw)
To: u-boot
On 04/20/2016 09:28 AM, Aneesh Bansal wrote:
>>>>> While enabling L2 cache, the value of L2PE (L2 cache parity/ECC
>>>>> error checking enable) must not be changed while the L2 cache is enabled.
>>>>> So, L2PE must be set before enabling L2 cache.
>>>>
>>>> Aneesh,
>>>>
>>>> The original code set L2PE and L2E together. The L2PE bit doesn't change after
>> that.
>>>> Doesn't this satisfy the requirement? Did you observe any failure before your
>> patch?
>>>>
>>>> York
>>>
>>> e6500 block guide states that "The value of L2PE must not be changed while the L2
>> cache is enabled"
>>> So, when both the bits are set together, it might lead to L2 cache
>>> getting enabled first and L2PE getting set after that. So L2PE is getting changed from
>> 0 to 1 while L2 is still enabled which should not be done.
>>>
>>> In normal non-secure boot, U-Boot is the first to use L2 after reset
>>> but in case of secure boot, L2 is used by Bot ROM before U-Boot. If
>>> L2PE and L2E are done together, ECC errors are observed on L2 (L2CAPTECC - L2
>> cache error capture ECC syndrome) and U-Boot crashes.
>>>
>>> I believe this is because of ECC/Parity checking not getting enabled
>>> properly and resulting into erroneous detection of errors
>>>
>>> When this is changed to setting L2PE before L2E, or not setting L2PE
>>> at all i.e. disabling ECC error checks, no ECC errors are observed and U-Boot works
>> fine.
>>>
>>
>> Aneesh,
>>
>> You said for secure boot L2 cache was used by bootrom before U-Boot. Could the
>> L2 cache be left enabled when U-Boot runs? If true, that indeed sets L2PE bit while
>> L2E is enabled. Please confirm.
>>
>> If L2E was left set by secure boot, your change actually fixes it, but the commit
>> message needs to be rewritten.
>
> L2 is not left enabled by Boot ROM in case of Secure Boot. The IBR code enables
> L2 but disables it before transferring control to U-Boot.
> What I was trying to suggest is that the way we are enabling L2 in U-Boot seems to
> be incorrect as we are setting L2PE and L2E simultaneously. This does not cause any
> issues in case of non-secure boot as U-Boot is the first entity to enable and use L2 cache.
>
> But in case of secure boot, L2 is enabled, used and disabled by Boot ROM first. Now in
> U-Boot, if ECC/Parity checking is not enabled correctly then it starts reporting ECC errors
> randomly and U-Boot crashes.
>
> After making this change, no ECC errors are observed and U-Boot starts working correct
> in case of non-secure boot.
>
All right. That clears my questions. Thanks.
York
^ permalink raw reply [flat|nested] 7+ messages in thread
* [U-Boot] [PATCH] powerpc/mpc85xx: set L2PE in L2CSR0 before enabling L2 cache
2016-04-18 17:28 [U-Boot] [PATCH] powerpc/mpc85xx: set L2PE in L2CSR0 before enabling L2 cache Aneesh Bansal
2016-04-19 16:32 ` York Sun
@ 2016-05-25 3:33 ` York Sun
1 sibling, 0 replies; 7+ messages in thread
From: York Sun @ 2016-05-25 3:33 UTC (permalink / raw)
To: u-boot
On 04/18/2016 05:16 AM, Aneesh Bansal wrote:
> While enabling L2 cache, the value of L2PE (L2 cache parity/ECC
> error checking enable) must not be changed while the L2 cache is
> enabled.
> So, L2PE must be set before enabling L2 cache.
>
> Signed-off-by: Aneesh Bansal <aneesh.bansal@nxp.com>
> ---
> arch/powerpc/cpu/mpc85xx/start.S | 23 +++++++++++++++++++++++
> 1 file changed, 23 insertions(+)
Applied to u-boot-mpc85xx master branch, awaiting upstream.
Thanks.
York
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2016-05-25 3:33 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-04-18 17:28 [U-Boot] [PATCH] powerpc/mpc85xx: set L2PE in L2CSR0 before enabling L2 cache Aneesh Bansal
2016-04-19 16:32 ` York Sun
2016-04-20 11:06 ` Aneesh Bansal
2016-04-20 15:28 ` York Sun
2016-04-20 16:27 ` Aneesh Bansal
2016-04-20 16:29 ` York Sun
2016-05-25 3:33 ` York Sun
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox