* [PATCH 0/3] ARM: PJ4B: fixes for 3.10-rc
@ 2013-05-29 10:16 Gregory CLEMENT
2013-05-29 10:16 ` [PATCH 1/3] ARM PJ4B: Add support for errata 4742 Gregory CLEMENT
` (3 more replies)
0 siblings, 4 replies; 18+ messages in thread
From: Gregory CLEMENT @ 2013-05-29 10:16 UTC (permalink / raw)
To: linux-arm-kernel
Hello,
This path set allows to fix some errata for PJ4B CPus.
Without the first one, the Armada 370 based board can't boot anymore
since 3.10-rc1 and the commit "ARM: 7691/1: mm: kill unused
TLB_CAN_READ_FROM_L1_CACHE and use ALT_SMP instead" . So this one
really need to be appplied on 3.10-rc.
It should be a good thing to apply the others on the 3.10-rc, even we
didn't observe yet a problem related to these errata, they may happen.
Note that currently PJ4B is a symbol for 2 different CPU:
- the one used in Armada 370 is a PJ4B
- the ones used in Armada XP are actually PJ4B-MP
So I planed to introduce a new symbol PJ4B-MP but for 3.11 as it is
not related to a critical fix. Awaiting this new symbol I made the
errata depends on ARMADA_370 or ARMADA_XP in the Kconfig.
I will resent this series once Russell will come back next week, but I
would like to have a first feedback on this series.
Thanks,
Lior Amsalem (3):
ARM PJ4B: Add support for errata 4742
ARM PJ4B: Add support for errata 6124
ARM PJ4B: Add support for errata 4611
arch/arm/Kconfig | 36 ++++++++++++++++++++++++++++++++++++
arch/arm/include/asm/tlbflush.h | 30 ++++++++++++++++++++++++++++++
arch/arm/mm/cache-v7.S | 8 ++++++++
arch/arm/mm/copypage-v6.c | 11 +++++++++++
arch/arm/mm/proc-macros.S | 13 +++++++++++++
arch/arm/mm/proc-v7-2level.S | 4 ++++
arch/arm/mm/proc-v7-3level.S | 4 ++++
arch/arm/mm/proc-v7.S | 7 +++++++
8 files changed, 113 insertions(+)
--
1.8.1.2
^ permalink raw reply [flat|nested] 18+ messages in thread* [PATCH 1/3] ARM PJ4B: Add support for errata 4742 2013-05-29 10:16 [PATCH 0/3] ARM: PJ4B: fixes for 3.10-rc Gregory CLEMENT @ 2013-05-29 10:16 ` Gregory CLEMENT 2013-05-29 11:03 ` Will Deacon 2013-05-29 10:16 ` [PATCH 2/3] ARM PJ4B: Add support for errata 6124 Gregory CLEMENT ` (2 subsequent siblings) 3 siblings, 1 reply; 18+ messages in thread From: Gregory CLEMENT @ 2013-05-29 10:16 UTC (permalink / raw) To: linux-arm-kernel From: Lior Amsalem <alior@marvell.com> This commit fix the regression on Armada 370 (the kernal hang during boot) introduced by the commit: "ARM: 7691/1: mm: kill unused TLB_CAN_READ_FROM_L1_CACHE and use ALT_SMP instead". When coming out of either a Wait for Interrupt (WFI) or a Wait for Event (WFE) IDLE states, a specific timing sensitivity exists between the retiring WFI/WFE instructions and the newly issued subsequent instructions. This sensitivity can result in a CPU hang scenario. The workaround is to insert either a Data Synchronization Barrier (DSB) or Data Memory Barrier (DMB) command immediately after the WFI/WFE instruction [gregory.clement at free-electrons.com:add errata description in changelog] [gregory.clement at free-electrons.com:make this errata depend on Aramda 370] Signed-off-by: Lior Amsalem <alior@marvell.com> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com> --- arch/arm/Kconfig | 12 ++++++++++++ arch/arm/mm/proc-v7.S | 3 +++ 2 files changed, 15 insertions(+) diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index 49d993c..745781f 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -1087,6 +1087,18 @@ if !MMU source "arch/arm/Kconfig-nommu" endif +config PJ4B_ERRATA_4742 + bool "PJ4B Errata 4742: IDLE Wake Up Commands can Cause the CPU Core to Cease Operation" + depends on CPU_PJ4B && MACH_ARMADA_370 + help + When coming out of either a Wait for Interrupt (WFI) or a Wait for Event + (WFE) IDLE states, a specific timing sensitivity exists between the retiring + WFI/WFE instructions and the newly issued subsequent instructions. + This sensitivity can result in a CPU hang scenario. + Workaround: + The software must insert either a Data Synchronization Barrier (DSB) + or Data Memory Barrier (DMB) command immediately after the WFI/WFE instruction + config ARM_ERRATA_326103 bool "ARM errata: FSR write bit incorrect on a SWP to read-only memory" depends on CPU_V6 diff --git a/arch/arm/mm/proc-v7.S b/arch/arm/mm/proc-v7.S index 2c73a73..f872432 100644 --- a/arch/arm/mm/proc-v7.S +++ b/arch/arm/mm/proc-v7.S @@ -71,6 +71,9 @@ ENDPROC(cpu_v7_reset) ENTRY(cpu_v7_do_idle) dsb @ WFI may enter a low-power mode wfi +#ifdef CONFIG_PJ4B_ERRATA_4742 + mcr p15, 0, r0, c7, c10, 4 @barrier +#endif mov pc, lr ENDPROC(cpu_v7_do_idle) -- 1.8.1.2 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 1/3] ARM PJ4B: Add support for errata 4742 2013-05-29 10:16 ` [PATCH 1/3] ARM PJ4B: Add support for errata 4742 Gregory CLEMENT @ 2013-05-29 11:03 ` Will Deacon 2013-06-02 8:27 ` Lior Amsalem 0 siblings, 1 reply; 18+ messages in thread From: Will Deacon @ 2013-05-29 11:03 UTC (permalink / raw) To: linux-arm-kernel Hi Gregory, On Wed, May 29, 2013 at 11:16:56AM +0100, Gregory CLEMENT wrote: > From: Lior Amsalem <alior@marvell.com> > > This commit fix the regression on Armada 370 (the kernal hang during > boot) introduced by the commit: "ARM: 7691/1: mm: kill unused > TLB_CAN_READ_FROM_L1_CACHE and use ALT_SMP instead". > > When coming out of either a Wait for Interrupt (WFI) or a Wait for > Event (WFE) IDLE states, a specific timing sensitivity exists between > the retiring WFI/WFE instructions and the newly issued subsequent > instructions. This sensitivity can result in a CPU hang scenario. The > workaround is to insert either a Data Synchronization Barrier (DSB) or > Data Memory Barrier (DMB) command immediately after the WFI/WFE > instruction Thanks for chasing this up! I have no idea why the patch you mentioned triggered this though. > diff --git a/arch/arm/mm/proc-v7.S b/arch/arm/mm/proc-v7.S > index 2c73a73..f872432 100644 > --- a/arch/arm/mm/proc-v7.S > +++ b/arch/arm/mm/proc-v7.S > @@ -71,6 +71,9 @@ ENDPROC(cpu_v7_reset) > ENTRY(cpu_v7_do_idle) > dsb @ WFI may enter a low-power mode > wfi > +#ifdef CONFIG_PJ4B_ERRATA_4742 > + mcr p15, 0, r0, c7, c10, 4 @barrier > +#endif Is the cp15 encoding required here, or can you just use `dsb'? Will ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 1/3] ARM PJ4B: Add support for errata 4742 2013-05-29 11:03 ` Will Deacon @ 2013-06-02 8:27 ` Lior Amsalem 0 siblings, 0 replies; 18+ messages in thread From: Lior Amsalem @ 2013-06-02 8:27 UTC (permalink / raw) To: linux-arm-kernel Hi Will, > From: Will Deacon [mailto:will.deacon at arm.com] > Sent: Wednesday, May 29, 2013 2:04 PM > > Hi Gregory, > > On Wed, May 29, 2013 at 11:16:56AM +0100, Gregory CLEMENT wrote: > > From: Lior Amsalem <alior@marvell.com> > > > > This commit fix the regression on Armada 370 (the kernal hang during > > boot) introduced by the commit: "ARM: 7691/1: mm: kill unused > > TLB_CAN_READ_FROM_L1_CACHE and use ALT_SMP instead". > > > > When coming out of either a Wait for Interrupt (WFI) or a Wait for > > Event (WFE) IDLE states, a specific timing sensitivity exists between > > the retiring WFI/WFE instructions and the newly issued subsequent > > instructions. This sensitivity can result in a CPU hang scenario. The > > workaround is to insert either a Data Synchronization Barrier (DSB) or > > Data Memory Barrier (DMB) command immediately after the WFI/WFE > > instruction > > Thanks for chasing this up! I have no idea why the patch you mentioned > triggered this though. > > > diff --git a/arch/arm/mm/proc-v7.S b/arch/arm/mm/proc-v7.S index > > 2c73a73..f872432 100644 > > --- a/arch/arm/mm/proc-v7.S > > +++ b/arch/arm/mm/proc-v7.S > > @@ -71,6 +71,9 @@ ENDPROC(cpu_v7_reset) > > ENTRY(cpu_v7_do_idle) > > dsb @ WFI may enter a low- > power mode > > wfi > > +#ifdef CONFIG_PJ4B_ERRATA_4742 > > + mcr p15, 0, r0, c7, c10, 4 @barrier > > +#endif > > Is the cp15 encoding required here, or can you just use `dsb'? A DSB will also do the job here, we'll change that. Anyway, this CP15 command is with-in the arch commands as well (not Marvell specific). BTW, thanks for all the comments, I'll refer them all. > > Will Regards, Lior Amsalem ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 2/3] ARM PJ4B: Add support for errata 6124 2013-05-29 10:16 [PATCH 0/3] ARM: PJ4B: fixes for 3.10-rc Gregory CLEMENT 2013-05-29 10:16 ` [PATCH 1/3] ARM PJ4B: Add support for errata 4742 Gregory CLEMENT @ 2013-05-29 10:16 ` Gregory CLEMENT 2013-05-29 11:16 ` Will Deacon 2013-05-29 10:16 ` [PATCH 3/3] ARM PJ4B: Add support for errata 4611 Gregory CLEMENT 2013-05-29 11:03 ` [PATCH 0/3] ARM: PJ4B: fixes for 3.10-rc Jason Cooper 3 siblings, 1 reply; 18+ messages in thread From: Gregory CLEMENT @ 2013-05-29 10:16 UTC (permalink / raw) To: linux-arm-kernel From: Lior Amsalem <alior@marvell.com> A rare timing scenario exists where a clean operation occurs, followed by an additional store operation. If a matching snoop also occurs, it is possible for the write-back from the original clean operation and the intervention from the snoop to race, which could result in data corruption. Replacing all clean cache maintenance operations with Clean & Invalidate operation to avoid the issue [gregory.clement at free-electrons.com:add errata description in changelog] [gregory.clement at free-electrons.com: make this errata depend on Aramda XP] Signed-off-by: Lior Amsalem <alior@marvell.com> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com> --- arch/arm/Kconfig | 13 +++++++++++++ arch/arm/include/asm/tlbflush.h | 8 ++++++++ arch/arm/mm/cache-v7.S | 8 ++++++++ arch/arm/mm/proc-v7-2level.S | 4 ++++ arch/arm/mm/proc-v7-3level.S | 4 ++++ arch/arm/mm/proc-v7.S | 4 ++++ 6 files changed, 41 insertions(+) diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index 745781f..48cdbea 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -1099,6 +1099,19 @@ config PJ4B_ERRATA_4742 The software must insert either a Data Synchronization Barrier (DSB) or Data Memory Barrier (DMB) command immediately after the WFI/WFE instruction +config PJ4B_ERRATA_6124 + bool "PJ4B Errata 6124: Multiple writebacks can be issued in a rare timing scenario associated with a clean operation and an incoming snoop" + depends on CPU_PJ4B && MACH_ARMADA_XP + help + A rare timing scenario exists where a clean operation occurs, followed + by an additional store operation. If a matching snoop also occurs, it + is possible for the write-back from the original clean operation and + the intervention from the snoop to race, which could result in data + corruption. + Workaround: + Replacing all clean cache maintenance operations with + Clean & Invalidate operation will avoid the issue + config ARM_ERRATA_326103 bool "ARM errata: FSR write bit incorrect on a SWP to read-only memory" depends on CPU_V6 diff --git a/arch/arm/include/asm/tlbflush.h b/arch/arm/include/asm/tlbflush.h index a3625d1..e3a8064 100644 --- a/arch/arm/include/asm/tlbflush.h +++ b/arch/arm/include/asm/tlbflush.h @@ -475,7 +475,11 @@ static inline void flush_pmd_entry(void *pmd) { const unsigned int __tlb_flag = __cpu_tlb_flags; +#ifdef CONFIG_PJ4B_ERRATA_6124 + tlb_op(TLB_DCLEAN, "c7, c14, 1 @ flush_pmd", pmd); +#else tlb_op(TLB_DCLEAN, "c7, c10, 1 @ flush_pmd", pmd); +#endif tlb_l2_op(TLB_L2CLEAN_FR, "c15, c9, 1 @ L2 flush_pmd", pmd); if (tlb_flag(TLB_WB)) @@ -486,7 +490,11 @@ static inline void clean_pmd_entry(void *pmd) { const unsigned int __tlb_flag = __cpu_tlb_flags; +#ifdef CONFIG_PJ4B_ERRATA_6124 + tlb_op(TLB_DCLEAN, "c7, c14, 1 @ flush_pmd", pmd); +#else tlb_op(TLB_DCLEAN, "c7, c10, 1 @ flush_pmd", pmd); +#endif tlb_l2_op(TLB_L2CLEAN_FR, "c15, c9, 1 @ L2 flush_pmd", pmd); } diff --git a/arch/arm/mm/cache-v7.S b/arch/arm/mm/cache-v7.S index 15451ee..d353649 100644 --- a/arch/arm/mm/cache-v7.S +++ b/arch/arm/mm/cache-v7.S @@ -270,7 +270,11 @@ ENTRY(v7_coherent_user_range) ALT_UP(W(nop)) #endif 1: +#ifdef CONFIG_PJ4B_ERRATA_6124 + USER( mcr p15, 0, r12, c7, c14, 1 ) @ clean & invalidate D line to the point of unification +#else USER( mcr p15, 0, r12, c7, c11, 1 ) @ clean D line to the point of unification +#endif add r12, r12, r2 cmp r12, r1 blo 1b @@ -378,7 +382,11 @@ v7_dma_clean_range: ALT_UP(W(nop)) #endif 1: +#ifdef CONFIG_PJ4B_ERRATA_6124 + mcr p15, 0, r0, c7, c14, 1 @ clean & invalidate D line to the point of unification +#else mcr p15, 0, r0, c7, c10, 1 @ clean D / U line +#endif add r0, r0, r2 cmp r0, r1 blo 1b diff --git a/arch/arm/mm/proc-v7-2level.S b/arch/arm/mm/proc-v7-2level.S index 9704097..0999fd3 100644 --- a/arch/arm/mm/proc-v7-2level.S +++ b/arch/arm/mm/proc-v7-2level.S @@ -111,8 +111,12 @@ ENTRY(cpu_v7_set_pte_ext) THUMB( add r0, r0, #2048 ) THUMB( str r3, [r0] ) ALT_SMP(mov pc,lr) +#ifdef CONFIG_PJ4B_ERRATA_6124 + ALT_UP (mcr p15, 0, r0, c7, c14, 1) @ flush_pte by clean & invalidate D entry +#else ALT_UP (mcr p15, 0, r0, c7, c10, 1) @ flush_pte #endif +#endif mov pc, lr ENDPROC(cpu_v7_set_pte_ext) diff --git a/arch/arm/mm/proc-v7-3level.S b/arch/arm/mm/proc-v7-3level.S index 363027e..7a917ff 100644 --- a/arch/arm/mm/proc-v7-3level.S +++ b/arch/arm/mm/proc-v7-3level.S @@ -74,8 +74,12 @@ ENTRY(cpu_v7_set_pte_ext) orreq r2, #L_PTE_RDONLY 1: strd r2, r3, [r0] ALT_SMP(mov pc, lr) +#ifdef CONFIG_PJ4B_ERRATA_6124 + ALT_UP (mcr p15, 0, r0, c7, c14, 1) @ flush_pte by clean & invalidate D entry +#else ALT_UP (mcr p15, 0, r0, c7, c10, 1) @ flush_pte #endif +#endif mov pc, lr ENDPROC(cpu_v7_set_pte_ext) diff --git a/arch/arm/mm/proc-v7.S b/arch/arm/mm/proc-v7.S index f872432..b86d688 100644 --- a/arch/arm/mm/proc-v7.S +++ b/arch/arm/mm/proc-v7.S @@ -81,7 +81,11 @@ ENTRY(cpu_v7_dcache_clean_area) ALT_SMP(mov pc, lr) @ MP extensions imply L1 PTW ALT_UP(W(nop)) dcache_line_size r2, r3 +#ifdef CONFIG_PJ4B_ERRATA_6124 +1: mcr p15, 0, r0, c7, c14, 1 @ clean & invalidate D entry +#else 1: mcr p15, 0, r0, c7, c10, 1 @ clean D entry +#endif add r0, r0, r2 subs r1, r1, r2 bhi 1b -- 1.8.1.2 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 2/3] ARM PJ4B: Add support for errata 6124 2013-05-29 10:16 ` [PATCH 2/3] ARM PJ4B: Add support for errata 6124 Gregory CLEMENT @ 2013-05-29 11:16 ` Will Deacon 2013-06-03 7:15 ` Lior Amsalem 0 siblings, 1 reply; 18+ messages in thread From: Will Deacon @ 2013-05-29 11:16 UTC (permalink / raw) To: linux-arm-kernel On Wed, May 29, 2013 at 11:16:57AM +0100, Gregory CLEMENT wrote: > From: Lior Amsalem <alior@marvell.com> > > A rare timing scenario exists where a clean operation occurs, followed > by an additional store operation. If a matching snoop also occurs, it > is possible for the write-back from the original clean operation and > the intervention from the snoop to race, which could result in data > corruption. > Replacing all clean cache maintenance operations with Clean & > Invalidate operation to avoid the issue Curious: do you not have a chicken bit for this? Upgrading clean to clean & invalidate is a pretty common thing to do and is even architected with the virtualisation extensions. > [gregory.clement at free-electrons.com:add errata description in changelog] > [gregory.clement at free-electrons.com: make this errata depend on Aramda > XP] > Signed-off-by: Lior Amsalem <alior@marvell.com> > Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com> > --- > arch/arm/Kconfig | 13 +++++++++++++ > arch/arm/include/asm/tlbflush.h | 8 ++++++++ > arch/arm/mm/cache-v7.S | 8 ++++++++ > arch/arm/mm/proc-v7-2level.S | 4 ++++ > arch/arm/mm/proc-v7-3level.S | 4 ++++ > arch/arm/mm/proc-v7.S | 4 ++++ > 6 files changed, 41 insertions(+) > > diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig > index 745781f..48cdbea 100644 > --- a/arch/arm/Kconfig > +++ b/arch/arm/Kconfig > @@ -1099,6 +1099,19 @@ config PJ4B_ERRATA_4742 > The software must insert either a Data Synchronization Barrier (DSB) > or Data Memory Barrier (DMB) command immediately after the WFI/WFE instruction > > +config PJ4B_ERRATA_6124 > + bool "PJ4B Errata 6124: Multiple writebacks can be issued in a rare timing scenario associated with a clean operation and an incoming snoop" > + depends on CPU_PJ4B && MACH_ARMADA_XP > + help > + A rare timing scenario exists where a clean operation occurs, followed > + by an additional store operation. If a matching snoop also occurs, it > + is possible for the write-back from the original clean operation and > + the intervention from the snoop to race, which could result in data > + corruption. > + Workaround: > + Replacing all clean cache maintenance operations with > + Clean & Invalidate operation will avoid the issue > + > config ARM_ERRATA_326103 > bool "ARM errata: FSR write bit incorrect on a SWP to read-only memory" > depends on CPU_V6 > diff --git a/arch/arm/include/asm/tlbflush.h b/arch/arm/include/asm/tlbflush.h > index a3625d1..e3a8064 100644 > --- a/arch/arm/include/asm/tlbflush.h > +++ b/arch/arm/include/asm/tlbflush.h > @@ -475,7 +475,11 @@ static inline void flush_pmd_entry(void *pmd) > { > const unsigned int __tlb_flag = __cpu_tlb_flags; > > +#ifdef CONFIG_PJ4B_ERRATA_6124 > + tlb_op(TLB_DCLEAN, "c7, c14, 1 @ flush_pmd", pmd); Can you elaborate on the comment please? > +#else > tlb_op(TLB_DCLEAN, "c7, c10, 1 @ flush_pmd", pmd); > +#endif > tlb_l2_op(TLB_L2CLEAN_FR, "c15, c9, 1 @ L2 flush_pmd", pmd); > > if (tlb_flag(TLB_WB)) > @@ -486,7 +490,11 @@ static inline void clean_pmd_entry(void *pmd) > { > const unsigned int __tlb_flag = __cpu_tlb_flags; > > +#ifdef CONFIG_PJ4B_ERRATA_6124 > + tlb_op(TLB_DCLEAN, "c7, c14, 1 @ flush_pmd", pmd); and here. > +#else > tlb_op(TLB_DCLEAN, "c7, c10, 1 @ flush_pmd", pmd); > +#endif > tlb_l2_op(TLB_L2CLEAN_FR, "c15, c9, 1 @ L2 flush_pmd", pmd); > } > > diff --git a/arch/arm/mm/cache-v7.S b/arch/arm/mm/cache-v7.S > index 15451ee..d353649 100644 > --- a/arch/arm/mm/cache-v7.S > +++ b/arch/arm/mm/cache-v7.S > @@ -270,7 +270,11 @@ ENTRY(v7_coherent_user_range) > ALT_UP(W(nop)) > #endif > 1: > +#ifdef CONFIG_PJ4B_ERRATA_6124 > + USER( mcr p15, 0, r12, c7, c14, 1 ) @ clean & invalidate D line to the point of unification This flushes to the point of coherency (there is no PoU clean & inv). > +#else > USER( mcr p15, 0, r12, c7, c11, 1 ) @ clean D line to the point of unification > +#endif > add r12, r12, r2 > cmp r12, r1 > blo 1b > @@ -378,7 +382,11 @@ v7_dma_clean_range: > ALT_UP(W(nop)) > #endif > 1: > +#ifdef CONFIG_PJ4B_ERRATA_6124 > + mcr p15, 0, r0, c7, c14, 1 @ clean & invalidate D line to the point of unification Similarly, although I don't understand why you care about the erratum for dma_clean_range. There should be no pending stores to the buffer, otherwise you risk corruption anyway (from the device's point of view). > +#else > mcr p15, 0, r0, c7, c10, 1 @ clean D / U line > +#endif > add r0, r0, r2 > cmp r0, r1 > blo 1b > diff --git a/arch/arm/mm/proc-v7-2level.S b/arch/arm/mm/proc-v7-2level.S > index 9704097..0999fd3 100644 > --- a/arch/arm/mm/proc-v7-2level.S > +++ b/arch/arm/mm/proc-v7-2level.S > @@ -111,8 +111,12 @@ ENTRY(cpu_v7_set_pte_ext) > THUMB( add r0, r0, #2048 ) > THUMB( str r3, [r0] ) > ALT_SMP(mov pc,lr) > +#ifdef CONFIG_PJ4B_ERRATA_6124 > + ALT_UP (mcr p15, 0, r0, c7, c14, 1) @ flush_pte by clean & invalidate D entry The use of `by' is a bit confusing, since you can also do maintenance by set/way. > +#else > ALT_UP (mcr p15, 0, r0, c7, c10, 1) @ flush_pte > #endif > +#endif > mov pc, lr > ENDPROC(cpu_v7_set_pte_ext) > > diff --git a/arch/arm/mm/proc-v7-3level.S b/arch/arm/mm/proc-v7-3level.S > index 363027e..7a917ff 100644 > --- a/arch/arm/mm/proc-v7-3level.S > +++ b/arch/arm/mm/proc-v7-3level.S > @@ -74,8 +74,12 @@ ENTRY(cpu_v7_set_pte_ext) > orreq r2, #L_PTE_RDONLY > 1: strd r2, r3, [r0] > ALT_SMP(mov pc, lr) > +#ifdef CONFIG_PJ4B_ERRATA_6124 > + ALT_UP (mcr p15, 0, r0, c7, c14, 1) @ flush_pte by clean & invalidate D entry > +#else > ALT_UP (mcr p15, 0, r0, c7, c10, 1) @ flush_pte > #endif > +#endif > mov pc, lr > ENDPROC(cpu_v7_set_pte_ext) > > diff --git a/arch/arm/mm/proc-v7.S b/arch/arm/mm/proc-v7.S > index f872432..b86d688 100644 > --- a/arch/arm/mm/proc-v7.S > +++ b/arch/arm/mm/proc-v7.S > @@ -81,7 +81,11 @@ ENTRY(cpu_v7_dcache_clean_area) > ALT_SMP(mov pc, lr) @ MP extensions imply L1 PTW > ALT_UP(W(nop)) > dcache_line_size r2, r3 You could move the 1: label here. > +#ifdef CONFIG_PJ4B_ERRATA_6124 > +1: mcr p15, 0, r0, c7, c14, 1 @ clean & invalidate D entry > +#else > 1: mcr p15, 0, r0, c7, c10, 1 @ clean D entry > +#endif Will ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 2/3] ARM PJ4B: Add support for errata 6124 2013-05-29 11:16 ` Will Deacon @ 2013-06-03 7:15 ` Lior Amsalem 0 siblings, 0 replies; 18+ messages in thread From: Lior Amsalem @ 2013-06-03 7:15 UTC (permalink / raw) To: linux-arm-kernel > From: Will Deacon [mailto:will.deacon at arm.com] > Sent: Wednesday, May 29, 2013 2:17 PM > > On Wed, May 29, 2013 at 11:16:57AM +0100, Gregory CLEMENT wrote: > > From: Lior Amsalem <alior@marvell.com> > > > > A rare timing scenario exists where a clean operation occurs, followed > > by an additional store operation. If a matching snoop also occurs, it > > is possible for the write-back from the original clean operation and > > the intervention from the snoop to race, which could result in data > > corruption. > > Replacing all clean cache maintenance operations with Clean & > > Invalidate operation to avoid the issue > > Curious: do you not have a chicken bit for this? Upgrading clean to clean & > invalidate is a pretty common thing to do and is even architected with the > virtualisation extensions. We don't have virtualization extension. As for the chicken bit, I'm looking into that with the design team. I'll update on my findings. > > > [gregory.clement at free-electrons.com:add errata description in > > changelog] > > [gregory.clement at free-electrons.com: make this errata depend on > Aramda > > XP] > > Signed-off-by: Lior Amsalem <alior@marvell.com> > > Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com> > > --- > > arch/arm/Kconfig | 13 +++++++++++++ > > arch/arm/include/asm/tlbflush.h | 8 ++++++++ > > arch/arm/mm/cache-v7.S | 8 ++++++++ > > arch/arm/mm/proc-v7-2level.S | 4 ++++ > > arch/arm/mm/proc-v7-3level.S | 4 ++++ > > arch/arm/mm/proc-v7.S | 4 ++++ > > 6 files changed, 41 insertions(+) > > > > diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index > > 745781f..48cdbea 100644 > > --- a/arch/arm/Kconfig > > +++ b/arch/arm/Kconfig > > @@ -1099,6 +1099,19 @@ config PJ4B_ERRATA_4742 > > The software must insert either a Data Synchronization Barrier (DSB) > > or Data Memory Barrier (DMB) command immediately after the > WFI/WFE > > instruction > > > > +config PJ4B_ERRATA_6124 > > + bool "PJ4B Errata 6124: Multiple writebacks can be issued in a rare > timing scenario associated with a clean operation and an incoming snoop" > > + depends on CPU_PJ4B && MACH_ARMADA_XP > > + help > > + A rare timing scenario exists where a clean operation occurs, > followed > > + by an additional store operation. If a matching snoop also occurs, it > > + is possible for the write-back from the original clean operation and > > + the intervention from the snoop to race, which could result in data > > + corruption. > > + Workaround: > > + Replacing all clean cache maintenance operations with > > + Clean & Invalidate operation will avoid the issue > > + > > config ARM_ERRATA_326103 > > bool "ARM errata: FSR write bit incorrect on a SWP to read-only > memory" > > depends on CPU_V6 > > diff --git a/arch/arm/include/asm/tlbflush.h > > b/arch/arm/include/asm/tlbflush.h index a3625d1..e3a8064 100644 > > --- a/arch/arm/include/asm/tlbflush.h > > +++ b/arch/arm/include/asm/tlbflush.h > > @@ -475,7 +475,11 @@ static inline void flush_pmd_entry(void *pmd) { > > const unsigned int __tlb_flag = __cpu_tlb_flags; > > > > +#ifdef CONFIG_PJ4B_ERRATA_6124 > > + tlb_op(TLB_DCLEAN, "c7, c14, 1 @ flush_pmd", pmd); > > Can you elaborate on the comment please? Yes, I'll add clean & invalidate to the comment there. > > > +#else > > tlb_op(TLB_DCLEAN, "c7, c10, 1 @ flush_pmd", pmd); > > +#endif > > tlb_l2_op(TLB_L2CLEAN_FR, "c15, c9, 1 @ L2 flush_pmd", pmd); > > > > if (tlb_flag(TLB_WB)) > > @@ -486,7 +490,11 @@ static inline void clean_pmd_entry(void *pmd) { > > const unsigned int __tlb_flag = __cpu_tlb_flags; > > > > +#ifdef CONFIG_PJ4B_ERRATA_6124 > > + tlb_op(TLB_DCLEAN, "c7, c14, 1 @ flush_pmd", pmd); > > and here. Same. > > > +#else > > tlb_op(TLB_DCLEAN, "c7, c10, 1 @ flush_pmd", pmd); > > +#endif > > tlb_l2_op(TLB_L2CLEAN_FR, "c15, c9, 1 @ L2 flush_pmd", pmd); } > > > > diff --git a/arch/arm/mm/cache-v7.S b/arch/arm/mm/cache-v7.S index > > 15451ee..d353649 100644 > > --- a/arch/arm/mm/cache-v7.S > > +++ b/arch/arm/mm/cache-v7.S > > @@ -270,7 +270,11 @@ ENTRY(v7_coherent_user_range) > > ALT_UP(W(nop)) > > #endif > > 1: > > +#ifdef CONFIG_PJ4B_ERRATA_6124 > > + USER( mcr p15, 0, r12, c7, c14, 1 ) @ clean & invalidate > D line to the point of unification > > This flushes to the point of coherency (there is no PoU clean & inv). Thanks, I'll update the comment. > > > +#else > > USER( mcr p15, 0, r12, c7, c11, 1 ) @ clean D line to the > point of unification > > +#endif > > add r12, r12, r2 > > cmp r12, r1 > > blo 1b > > @@ -378,7 +382,11 @@ v7_dma_clean_range: > > ALT_UP(W(nop)) > > #endif > > 1: > > +#ifdef CONFIG_PJ4B_ERRATA_6124 > > + mcr p15, 0, r0, c7, c14, 1 @ clean & invalidate D line to > the point of unification > > Similarly, although I don't understand why you care about the erratum for > dma_clean_range. There should be no pending stores to the buffer, > otherwise you risk corruption anyway (from the device's point of view). You're raising an interesting point (and logically sound correct). I'll have to look into it and run some tests to ensure all is valid. > > > +#else > > mcr p15, 0, r0, c7, c10, 1 @ clean D / U line > > +#endif > > add r0, r0, r2 > > cmp r0, r1 > > blo 1b > > diff --git a/arch/arm/mm/proc-v7-2level.S > > b/arch/arm/mm/proc-v7-2level.S index 9704097..0999fd3 100644 > > --- a/arch/arm/mm/proc-v7-2level.S > > +++ b/arch/arm/mm/proc-v7-2level.S > > @@ -111,8 +111,12 @@ ENTRY(cpu_v7_set_pte_ext) > > THUMB( add r0, r0, #2048 ) > > THUMB( str r3, [r0] ) > > ALT_SMP(mov pc,lr) > > +#ifdef CONFIG_PJ4B_ERRATA_6124 > > + ALT_UP (mcr p15, 0, r0, c7, c14, 1) @ flush_pte by clean > & invalidate D entry > > The use of `by' is a bit confusing, since you can also do maintenance by > set/way. I'll change that to 'using' instead. > > > +#else > > ALT_UP (mcr p15, 0, r0, c7, c10, 1) @ flush_pte > > #endif > > +#endif > > mov pc, lr > > ENDPROC(cpu_v7_set_pte_ext) > > > > diff --git a/arch/arm/mm/proc-v7-3level.S > > b/arch/arm/mm/proc-v7-3level.S index 363027e..7a917ff 100644 > > --- a/arch/arm/mm/proc-v7-3level.S > > +++ b/arch/arm/mm/proc-v7-3level.S > > @@ -74,8 +74,12 @@ ENTRY(cpu_v7_set_pte_ext) > > orreq r2, #L_PTE_RDONLY > > 1: strd r2, r3, [r0] > > ALT_SMP(mov pc, lr) > > +#ifdef CONFIG_PJ4B_ERRATA_6124 > > + ALT_UP (mcr p15, 0, r0, c7, c14, 1) @ flush_pte by clean > & invalidate D entry > > +#else > > ALT_UP (mcr p15, 0, r0, c7, c10, 1) @ flush_pte > > #endif > > +#endif > > mov pc, lr > > ENDPROC(cpu_v7_set_pte_ext) > > > > diff --git a/arch/arm/mm/proc-v7.S b/arch/arm/mm/proc-v7.S index > > f872432..b86d688 100644 > > --- a/arch/arm/mm/proc-v7.S > > +++ b/arch/arm/mm/proc-v7.S > > @@ -81,7 +81,11 @@ ENTRY(cpu_v7_dcache_clean_area) > > ALT_SMP(mov pc, lr) @ MP extensions imply L1 > PTW > > ALT_UP(W(nop)) > > dcache_line_size r2, r3 > > You could move the 1: label here. Right, will do. > > > +#ifdef CONFIG_PJ4B_ERRATA_6124 > > +1: mcr p15, 0, r0, c7, c14, 1 @ clean & invalidate D entry > > +#else > > 1: mcr p15, 0, r0, c7, c10, 1 @ clean D entry > > +#endif > > Will Regards, Lior Amsalem ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 3/3] ARM PJ4B: Add support for errata 4611 2013-05-29 10:16 [PATCH 0/3] ARM: PJ4B: fixes for 3.10-rc Gregory CLEMENT 2013-05-29 10:16 ` [PATCH 1/3] ARM PJ4B: Add support for errata 4742 Gregory CLEMENT 2013-05-29 10:16 ` [PATCH 2/3] ARM PJ4B: Add support for errata 6124 Gregory CLEMENT @ 2013-05-29 10:16 ` Gregory CLEMENT 2013-05-29 11:22 ` Will Deacon 2013-05-29 11:03 ` [PATCH 0/3] ARM: PJ4B: fixes for 3.10-rc Jason Cooper 3 siblings, 1 reply; 18+ messages in thread From: Gregory CLEMENT @ 2013-05-29 10:16 UTC (permalink / raw) To: linux-arm-kernel From: Lior Amsalem <alior@marvell.com> A CP15 clean operation can result in a dead lock state if it is hit by an incoming snoop event. the fiw to this issue is the following: before any CP15 clean type operation in Cache Coherency mode, issue a Data Memory Barrier (DMB) or a Data Synchronization Barrier (DSB) instruction. [gregory.clement at free-electrons.com:add errata description in changelog] [gregory.clement at free-electrons.com:make this errata depend on Aramda 370] Signed-off-by: Lior Amsalem <alior@marvell.com> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com> --- arch/arm/Kconfig | 11 +++++++++++ arch/arm/include/asm/tlbflush.h | 22 ++++++++++++++++++++++ arch/arm/mm/copypage-v6.c | 11 +++++++++++ arch/arm/mm/proc-macros.S | 13 +++++++++++++ 4 files changed, 57 insertions(+) diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index 48cdbea..05c13f9 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -1087,6 +1087,17 @@ if !MMU source "arch/arm/Kconfig-nommu" endif +config PJ4B_ERRATA_4611 + bool "PJ4B Errata 4611: A Deadlock can Occur if a CP15 Clean/Clean and Invalidate Operation is Hit By a Snoop Event" + depends on CPU_PJ4B && MACH_ARMADA_370 + help + A CP15 clean operation can result in a dead lock state if it is hit by + an incoming snoop event. + Workaround: + Before any CP15 clean type operation in Cache Coherency mode, issue + a Data Memory Barrier (DMB) or a Data Synchronization Barrier (DSB) + instruction. + config PJ4B_ERRATA_4742 bool "PJ4B Errata 4742: IDLE Wake Up Commands can Cause the CPU Core to Cease Operation" depends on CPU_PJ4B && MACH_ARMADA_370 diff --git a/arch/arm/include/asm/tlbflush.h b/arch/arm/include/asm/tlbflush.h index e3a8064..112f778 100644 --- a/arch/arm/include/asm/tlbflush.h +++ b/arch/arm/include/asm/tlbflush.h @@ -475,11 +475,22 @@ static inline void flush_pmd_entry(void *pmd) { const unsigned int __tlb_flag = __cpu_tlb_flags; +#ifdef CONFIG_PJ4B_ERRATA_4611 + unsigned long flags; + raw_local_irq_save(flags); + dmb(); +#endif + #ifdef CONFIG_PJ4B_ERRATA_6124 tlb_op(TLB_DCLEAN, "c7, c14, 1 @ flush_pmd", pmd); #else tlb_op(TLB_DCLEAN, "c7, c10, 1 @ flush_pmd", pmd); #endif + +#ifdef CONFIG_PJ4B_ERRATA_4611 + raw_local_irq_restore(flags); +#endif + tlb_l2_op(TLB_L2CLEAN_FR, "c15, c9, 1 @ L2 flush_pmd", pmd); if (tlb_flag(TLB_WB)) @@ -490,11 +501,22 @@ static inline void clean_pmd_entry(void *pmd) { const unsigned int __tlb_flag = __cpu_tlb_flags; +#ifdef CONFIG_PJ4B_ERRATA_4611 + unsigned long flags; + raw_local_irq_save(flags); + dmb(); +#endif + #ifdef CONFIG_PJ4B_ERRATA_6124 tlb_op(TLB_DCLEAN, "c7, c14, 1 @ flush_pmd", pmd); #else tlb_op(TLB_DCLEAN, "c7, c10, 1 @ flush_pmd", pmd); #endif + +#ifdef CONFIG_PJ4B_ERRATA_4611 + raw_local_irq_restore(flags); +#endif + tlb_l2_op(TLB_L2CLEAN_FR, "c15, c9, 1 @ L2 flush_pmd", pmd); } diff --git a/arch/arm/mm/copypage-v6.c b/arch/arm/mm/copypage-v6.c index b9bcc9d..20b69ad 100644 --- a/arch/arm/mm/copypage-v6.c +++ b/arch/arm/mm/copypage-v6.c @@ -59,11 +59,22 @@ static void v6_clear_user_highpage_nonaliasing(struct page *page, unsigned long */ static void discard_old_kernel_data(void *kto) { +#ifdef CONFIG_PJ4B_ERRATA_4611 + unsigned long flags; + + raw_local_irq_save(flags); + dmb(); +#endif + __asm__("mcrr p15, 0, %1, %0, c6 @ 0xec401f06" : : "r" (kto), "r" ((unsigned long)kto + PAGE_SIZE - L1_CACHE_BYTES) : "cc"); + +#ifdef CONFIG_PJ4B_ERRATA_4611 + raw_local_irq_restore(flags); +#endif } /* diff --git a/arch/arm/mm/proc-macros.S b/arch/arm/mm/proc-macros.S index f9a0aa7..ca81291 100644 --- a/arch/arm/mm/proc-macros.S +++ b/arch/arm/mm/proc-macros.S @@ -178,7 +178,20 @@ #endif str r3, [r0] + +#ifdef CONFIG_PJ4B_ERRATA_4611 + mrs r2, cpsr + orr r3, r2, #PSR_F_BIT | PSR_I_BIT + msr cpsr_c, r3 @ Disable interrupts + dmb @ ensure ordering with previous memory accesses +#endif + mcr p15, 0, r0, c7, c10, 1 @ flush_pte + +#ifdef CONFIG_PJ4B_ERRATA_4611 + msr cpsr_c, r2 @ Restore interrupts + mcr p15, 0, r0, c7, c10, 4 @ drain write buffer +#endif .endm -- 1.8.1.2 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 3/3] ARM PJ4B: Add support for errata 4611 2013-05-29 10:16 ` [PATCH 3/3] ARM PJ4B: Add support for errata 4611 Gregory CLEMENT @ 2013-05-29 11:22 ` Will Deacon 2013-05-29 12:57 ` Gregory CLEMENT 2013-06-03 7:16 ` Lior Amsalem 0 siblings, 2 replies; 18+ messages in thread From: Will Deacon @ 2013-05-29 11:22 UTC (permalink / raw) To: linux-arm-kernel On Wed, May 29, 2013 at 11:16:58AM +0100, Gregory CLEMENT wrote: > From: Lior Amsalem <alior@marvell.com> > > A CP15 clean operation can result in a dead lock state if it is hit by > an incoming snoop event. the fiw to this issue is the following: s/fiw/fix/ > before any CP15 clean type operation in Cache Coherency mode, issue a > Data Memory Barrier (DMB) or a Data Synchronization Barrier (DSB) > instruction. Could you combine this with #6124, so we don't have back-to-back ifdefs all over the place? Also, there are more clean operations than you have described here, so why haven't you have to touch them all? > [gregory.clement at free-electrons.com:add errata description in changelog] > [gregory.clement at free-electrons.com:make this errata depend on Aramda > 370] > Signed-off-by: Lior Amsalem <alior@marvell.com> > Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com> > --- > arch/arm/Kconfig | 11 +++++++++++ > arch/arm/include/asm/tlbflush.h | 22 ++++++++++++++++++++++ > arch/arm/mm/copypage-v6.c | 11 +++++++++++ > arch/arm/mm/proc-macros.S | 13 +++++++++++++ > 4 files changed, 57 insertions(+) > > diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig > index 48cdbea..05c13f9 100644 > --- a/arch/arm/Kconfig > +++ b/arch/arm/Kconfig > @@ -1087,6 +1087,17 @@ if !MMU > source "arch/arm/Kconfig-nommu" > endif > > +config PJ4B_ERRATA_4611 > + bool "PJ4B Errata 4611: A Deadlock can Occur if a CP15 Clean/Clean and Invalidate Operation is Hit By a Snoop Event" > + depends on CPU_PJ4B && MACH_ARMADA_370 > + help > + A CP15 clean operation can result in a dead lock state if it is hit by > + an incoming snoop event. > + Workaround: > + Before any CP15 clean type operation in Cache Coherency mode, issue > + a Data Memory Barrier (DMB) or a Data Synchronization Barrier (DSB) > + instruction. > + > config PJ4B_ERRATA_4742 > bool "PJ4B Errata 4742: IDLE Wake Up Commands can Cause the CPU Core to Cease Operation" > depends on CPU_PJ4B && MACH_ARMADA_370 > diff --git a/arch/arm/include/asm/tlbflush.h b/arch/arm/include/asm/tlbflush.h > index e3a8064..112f778 100644 > --- a/arch/arm/include/asm/tlbflush.h > +++ b/arch/arm/include/asm/tlbflush.h > @@ -475,11 +475,22 @@ static inline void flush_pmd_entry(void *pmd) > { > const unsigned int __tlb_flag = __cpu_tlb_flags; > > +#ifdef CONFIG_PJ4B_ERRATA_4611 > + unsigned long flags; > + raw_local_irq_save(flags); > + dmb(); > +#endif > + > #ifdef CONFIG_PJ4B_ERRATA_6124 > tlb_op(TLB_DCLEAN, "c7, c14, 1 @ flush_pmd", pmd); > #else > tlb_op(TLB_DCLEAN, "c7, c10, 1 @ flush_pmd", pmd); > #endif > + > +#ifdef CONFIG_PJ4B_ERRATA_4611 > + raw_local_irq_restore(flags); > +#endif Eeek. Why do you have to disable interrupts during this? Again, are there no chicken bits to save you? > tlb_l2_op(TLB_L2CLEAN_FR, "c15, c9, 1 @ L2 flush_pmd", pmd); > > if (tlb_flag(TLB_WB)) > @@ -490,11 +501,22 @@ static inline void clean_pmd_entry(void *pmd) > { > const unsigned int __tlb_flag = __cpu_tlb_flags; > > +#ifdef CONFIG_PJ4B_ERRATA_4611 > + unsigned long flags; > + raw_local_irq_save(flags); > + dmb(); > +#endif > + > #ifdef CONFIG_PJ4B_ERRATA_6124 > tlb_op(TLB_DCLEAN, "c7, c14, 1 @ flush_pmd", pmd); > #else > tlb_op(TLB_DCLEAN, "c7, c10, 1 @ flush_pmd", pmd); > #endif > + > +#ifdef CONFIG_PJ4B_ERRATA_4611 > + raw_local_irq_restore(flags); > +#endif > + > tlb_l2_op(TLB_L2CLEAN_FR, "c15, c9, 1 @ L2 flush_pmd", pmd); > } > > diff --git a/arch/arm/mm/copypage-v6.c b/arch/arm/mm/copypage-v6.c > index b9bcc9d..20b69ad 100644 > --- a/arch/arm/mm/copypage-v6.c > +++ b/arch/arm/mm/copypage-v6.c > @@ -59,11 +59,22 @@ static void v6_clear_user_highpage_nonaliasing(struct page *page, unsigned long > */ > static void discard_old_kernel_data(void *kto) > { > +#ifdef CONFIG_PJ4B_ERRATA_4611 > + unsigned long flags; > + > + raw_local_irq_save(flags); > + dmb(); > +#endif > + > __asm__("mcrr p15, 0, %1, %0, c6 @ 0xec401f06" > : > : "r" (kto), > "r" ((unsigned long)kto + PAGE_SIZE - L1_CACHE_BYTES) > : "cc"); > + > +#ifdef CONFIG_PJ4B_ERRATA_4611 > + raw_local_irq_restore(flags); > +#endif Do you actually have an aliasing vipt cache? If not, you don't care about this function. > diff --git a/arch/arm/mm/proc-macros.S b/arch/arm/mm/proc-macros.S > index f9a0aa7..ca81291 100644 > --- a/arch/arm/mm/proc-macros.S > +++ b/arch/arm/mm/proc-macros.S > @@ -178,7 +178,20 @@ > #endif > > str r3, [r0] > + > +#ifdef CONFIG_PJ4B_ERRATA_4611 > + mrs r2, cpsr > + orr r3, r2, #PSR_F_BIT | PSR_I_BIT > + msr cpsr_c, r3 @ Disable interrupts > + dmb @ ensure ordering with previous memory accesses > +#endif > + > mcr p15, 0, r0, c7, c10, 1 @ flush_pte > + > +#ifdef CONFIG_PJ4B_ERRATA_4611 > + msr cpsr_c, r2 @ Restore interrupts > + mcr p15, 0, r0, c7, c10, 4 @ drain write buffer > +#endif This could be a dsb. Will ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 3/3] ARM PJ4B: Add support for errata 4611 2013-05-29 11:22 ` Will Deacon @ 2013-05-29 12:57 ` Gregory CLEMENT 2013-05-29 13:11 ` Arnd Bergmann 2013-06-03 7:16 ` Lior Amsalem 1 sibling, 1 reply; 18+ messages in thread From: Gregory CLEMENT @ 2013-05-29 12:57 UTC (permalink / raw) To: linux-arm-kernel Hi Will, On 05/29/2013 01:22 PM, Will Deacon wrote: > On Wed, May 29, 2013 at 11:16:58AM +0100, Gregory CLEMENT wrote: >> From: Lior Amsalem <alior@marvell.com> >> >> A CP15 clean operation can result in a dead lock state if it is hit by >> an incoming snoop event. the fiw to this issue is the following: > > s/fiw/fix/ > >> before any CP15 clean type operation in Cache Coherency mode, issue a >> Data Memory Barrier (DMB) or a Data Synchronization Barrier (DSB) >> instruction. > > Could you combine this with #6124, so we don't have back-to-back ifdefs all > over the place? Also, there are more clean operations than you have > described here, so why haven't you have to touch them all? For the first point, unfortunately it is not possible as they are errata for different CPUs. If finally the 2 last patches won't be merged in 3.10, then I will add a new patch before them, to introduce the PJ4B-MP which is the CPU used by the Armada XP whereas the Armada 370 uses the PJ4B. For the second point and the other CPU specific questions you raised in the other emails, I am waiting for answer from Marvell engineers. Thanks for your review > >> [gregory.clement at free-electrons.com:add errata description in changelog] >> [gregory.clement at free-electrons.com:make this errata depend on Aramda >> 370] >> Signed-off-by: Lior Amsalem <alior@marvell.com> >> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com> >> --- >> arch/arm/Kconfig | 11 +++++++++++ >> arch/arm/include/asm/tlbflush.h | 22 ++++++++++++++++++++++ >> arch/arm/mm/copypage-v6.c | 11 +++++++++++ >> arch/arm/mm/proc-macros.S | 13 +++++++++++++ >> 4 files changed, 57 insertions(+) >> >> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig >> index 48cdbea..05c13f9 100644 >> --- a/arch/arm/Kconfig >> +++ b/arch/arm/Kconfig >> @@ -1087,6 +1087,17 @@ if !MMU >> source "arch/arm/Kconfig-nommu" >> endif >> >> +config PJ4B_ERRATA_4611 >> + bool "PJ4B Errata 4611: A Deadlock can Occur if a CP15 Clean/Clean and Invalidate Operation is Hit By a Snoop Event" >> + depends on CPU_PJ4B && MACH_ARMADA_370 >> + help >> + A CP15 clean operation can result in a dead lock state if it is hit by >> + an incoming snoop event. >> + Workaround: >> + Before any CP15 clean type operation in Cache Coherency mode, issue >> + a Data Memory Barrier (DMB) or a Data Synchronization Barrier (DSB) >> + instruction. >> + >> config PJ4B_ERRATA_4742 >> bool "PJ4B Errata 4742: IDLE Wake Up Commands can Cause the CPU Core to Cease Operation" >> depends on CPU_PJ4B && MACH_ARMADA_370 >> diff --git a/arch/arm/include/asm/tlbflush.h b/arch/arm/include/asm/tlbflush.h >> index e3a8064..112f778 100644 >> --- a/arch/arm/include/asm/tlbflush.h >> +++ b/arch/arm/include/asm/tlbflush.h >> @@ -475,11 +475,22 @@ static inline void flush_pmd_entry(void *pmd) >> { >> const unsigned int __tlb_flag = __cpu_tlb_flags; >> >> +#ifdef CONFIG_PJ4B_ERRATA_4611 >> + unsigned long flags; >> + raw_local_irq_save(flags); >> + dmb(); >> +#endif >> + >> #ifdef CONFIG_PJ4B_ERRATA_6124 >> tlb_op(TLB_DCLEAN, "c7, c14, 1 @ flush_pmd", pmd); >> #else >> tlb_op(TLB_DCLEAN, "c7, c10, 1 @ flush_pmd", pmd); >> #endif >> + >> +#ifdef CONFIG_PJ4B_ERRATA_4611 >> + raw_local_irq_restore(flags); >> +#endif > > Eeek. Why do you have to disable interrupts during this? Again, are there no > chicken bits to save you? > >> tlb_l2_op(TLB_L2CLEAN_FR, "c15, c9, 1 @ L2 flush_pmd", pmd); >> >> if (tlb_flag(TLB_WB)) >> @@ -490,11 +501,22 @@ static inline void clean_pmd_entry(void *pmd) >> { >> const unsigned int __tlb_flag = __cpu_tlb_flags; >> >> +#ifdef CONFIG_PJ4B_ERRATA_4611 >> + unsigned long flags; >> + raw_local_irq_save(flags); >> + dmb(); >> +#endif >> + >> #ifdef CONFIG_PJ4B_ERRATA_6124 >> tlb_op(TLB_DCLEAN, "c7, c14, 1 @ flush_pmd", pmd); >> #else >> tlb_op(TLB_DCLEAN, "c7, c10, 1 @ flush_pmd", pmd); >> #endif >> + >> +#ifdef CONFIG_PJ4B_ERRATA_4611 >> + raw_local_irq_restore(flags); >> +#endif >> + >> tlb_l2_op(TLB_L2CLEAN_FR, "c15, c9, 1 @ L2 flush_pmd", pmd); >> } >> >> diff --git a/arch/arm/mm/copypage-v6.c b/arch/arm/mm/copypage-v6.c >> index b9bcc9d..20b69ad 100644 >> --- a/arch/arm/mm/copypage-v6.c >> +++ b/arch/arm/mm/copypage-v6.c >> @@ -59,11 +59,22 @@ static void v6_clear_user_highpage_nonaliasing(struct page *page, unsigned long >> */ >> static void discard_old_kernel_data(void *kto) >> { >> +#ifdef CONFIG_PJ4B_ERRATA_4611 >> + unsigned long flags; >> + >> + raw_local_irq_save(flags); >> + dmb(); >> +#endif >> + >> __asm__("mcrr p15, 0, %1, %0, c6 @ 0xec401f06" >> : >> : "r" (kto), >> "r" ((unsigned long)kto + PAGE_SIZE - L1_CACHE_BYTES) >> : "cc"); >> + >> +#ifdef CONFIG_PJ4B_ERRATA_4611 >> + raw_local_irq_restore(flags); >> +#endif > > Do you actually have an aliasing vipt cache? If not, you don't care about > this function. > >> diff --git a/arch/arm/mm/proc-macros.S b/arch/arm/mm/proc-macros.S >> index f9a0aa7..ca81291 100644 >> --- a/arch/arm/mm/proc-macros.S >> +++ b/arch/arm/mm/proc-macros.S >> @@ -178,7 +178,20 @@ >> #endif >> >> str r3, [r0] >> + >> +#ifdef CONFIG_PJ4B_ERRATA_4611 >> + mrs r2, cpsr >> + orr r3, r2, #PSR_F_BIT | PSR_I_BIT >> + msr cpsr_c, r3 @ Disable interrupts >> + dmb @ ensure ordering with previous memory accesses >> +#endif >> + >> mcr p15, 0, r0, c7, c10, 1 @ flush_pte >> + >> +#ifdef CONFIG_PJ4B_ERRATA_4611 >> + msr cpsr_c, r2 @ Restore interrupts >> + mcr p15, 0, r0, c7, c10, 4 @ drain write buffer >> +#endif > > This could be a dsb. > > Will > -- Gregory Clement, Free Electrons Kernel, drivers, real-time and embedded Linux development, consulting, training and support. http://free-electrons.com ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 3/3] ARM PJ4B: Add support for errata 4611 2013-05-29 12:57 ` Gregory CLEMENT @ 2013-05-29 13:11 ` Arnd Bergmann 2013-05-29 13:27 ` Gregory CLEMENT 0 siblings, 1 reply; 18+ messages in thread From: Arnd Bergmann @ 2013-05-29 13:11 UTC (permalink / raw) To: linux-arm-kernel On Wednesday 29 May 2013 14:57:10 Gregory CLEMENT wrote: > > For the first point, unfortunately it is not possible as they are > errata for different CPUs. If finally the 2 last patches won't be > merged in 3.10, then I will add a new patch before them, to introduce > the PJ4B-MP which is the CPU used by the Armada XP whereas the > Armada 370 uses the PJ4B. > > For the second point and the other CPU specific questions you raised > in the other emails, I am waiting for answer from Marvell engineers. I've been confused by the various CPU types before. If you have any information that's not yet in the excellent Documentation/arm/Marvell/README file that Thomas added, could you put it in there? In particular, I'd like to understand what variants of PJ4 there are, and maybe you can list their cpuid part numbers. Arnd ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 3/3] ARM PJ4B: Add support for errata 4611 2013-05-29 13:11 ` Arnd Bergmann @ 2013-05-29 13:27 ` Gregory CLEMENT 2013-05-29 13:33 ` Arnd Bergmann 0 siblings, 1 reply; 18+ messages in thread From: Gregory CLEMENT @ 2013-05-29 13:27 UTC (permalink / raw) To: linux-arm-kernel On 05/29/2013 03:11 PM, Arnd Bergmann wrote: > On Wednesday 29 May 2013 14:57:10 Gregory CLEMENT wrote: >> >> For the first point, unfortunately it is not possible as they are >> errata for different CPUs. If finally the 2 last patches won't be >> merged in 3.10, then I will add a new patch before them, to introduce >> the PJ4B-MP which is the CPU used by the Armada XP whereas the >> Armada 370 uses the PJ4B. >> >> For the second point and the other CPU specific questions you raised >> in the other emails, I am waiting for answer from Marvell engineers. > > I've been confused by the various CPU types before. If you have > any information that's not yet in the excellent > Documentation/arm/Marvell/README file that Thomas added, could you > put it in there? > > In particular, I'd like to understand what variants of PJ4 there > are, and maybe you can list their cpuid part numbers. OK I will see what kind of information I can gather and which ones I can add to the Documentation/arm/Marvell/README file. My current understanding is that there are at least 3 kind of CPU: PJ4, PJ4B and PJ4B-MP and all of them belongs to the Sheeva family. > > Arnd > -- Gregory Clement, Free Electrons Kernel, drivers, real-time and embedded Linux development, consulting, training and support. http://free-electrons.com ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 3/3] ARM PJ4B: Add support for errata 4611 2013-05-29 13:27 ` Gregory CLEMENT @ 2013-05-29 13:33 ` Arnd Bergmann 2013-06-04 11:34 ` Lior Amsalem 0 siblings, 1 reply; 18+ messages in thread From: Arnd Bergmann @ 2013-05-29 13:33 UTC (permalink / raw) To: linux-arm-kernel On Wednesday 29 May 2013 15:27:31 Gregory CLEMENT wrote: > OK I will see what kind of information I can gather and which ones I can > add to the Documentation/arm/Marvell/README file. > > My current understanding is that there are at least 3 kind of CPU: > PJ4, PJ4B and PJ4B-MP and all of them belongs to the Sheeva family. Ok. So I guess PJ4B adds LPAE support, and PJ4B-MP adds SMP support, right? Do either of them support hypervisor mode? Regarding the errata: are those needed for production-level chips, or just for prototypes? Arnd ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 3/3] ARM PJ4B: Add support for errata 4611 2013-05-29 13:33 ` Arnd Bergmann @ 2013-06-04 11:34 ` Lior Amsalem 2013-06-04 11:39 ` Arnd Bergmann 0 siblings, 1 reply; 18+ messages in thread From: Lior Amsalem @ 2013-06-04 11:34 UTC (permalink / raw) To: linux-arm-kernel > From: Arnd Bergmann [mailto:arnd at arndb.de] > Sent: Wednesday, May 29, 2013 4:33 PM > > On Wednesday 29 May 2013 15:27:31 Gregory CLEMENT wrote: > > OK I will see what kind of information I can gather and which ones I > > can add to the Documentation/arm/Marvell/README file. > > > > My current understanding is that there are at least 3 kind of CPU: > > PJ4, PJ4B and PJ4B-MP and all of them belongs to the Sheeva family. > > Ok. So I guess PJ4B adds LPAE support, and PJ4B-MP adds SMP support, > right? Do either of them support hypervisor mode? It goes like this: PJ4B: improved version of PJ4. PJ4B-MP: adds LPAE and SMP. No support for virtualization on any. > > Regarding the errata: are those needed for production-level chips, or just for > prototypes? These are production level chips. > > Arnd Regards, Lior Amsalem ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 3/3] ARM PJ4B: Add support for errata 4611 2013-06-04 11:34 ` Lior Amsalem @ 2013-06-04 11:39 ` Arnd Bergmann 2013-06-06 7:46 ` Lior Amsalem 0 siblings, 1 reply; 18+ messages in thread From: Arnd Bergmann @ 2013-06-04 11:39 UTC (permalink / raw) To: linux-arm-kernel On Tuesday 04 June 2013 14:34:53 Lior Amsalem wrote: > > From: Arnd Bergmann [mailto:arnd at arndb.de] > > Sent: Wednesday, May 29, 2013 4:33 PM > > > > On Wednesday 29 May 2013 15:27:31 Gregory CLEMENT wrote: > > > OK I will see what kind of information I can gather and which ones I > > > can add to the Documentation/arm/Marvell/README file. > > > > > > My current understanding is that there are at least 3 kind of CPU: > > > PJ4, PJ4B and PJ4B-MP and all of them belongs to the Sheeva family. > > > > Ok. So I guess PJ4B adds LPAE support, and PJ4B-MP adds SMP support, > > right? Do either of them support hypervisor mode? > > It goes like this: > PJ4B: improved version of PJ4. > PJ4B-MP: adds LPAE and SMP. > No support for virtualization on any. Ah, interesting. So PJ4B is just faster than PJ4 and has no extra instructions or other features? > > Regarding the errata: are those needed for production-level chips, or just for > > prototypes? > > These are production level chips. Ok, thanks for the information. To clarify, does that mean that all future PJ4B and PJ4B-MP based chips also need the errata, or is there is a chance they will be fixed in future revisions? Arnd ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 3/3] ARM PJ4B: Add support for errata 4611 2013-06-04 11:39 ` Arnd Bergmann @ 2013-06-06 7:46 ` Lior Amsalem 0 siblings, 0 replies; 18+ messages in thread From: Lior Amsalem @ 2013-06-06 7:46 UTC (permalink / raw) To: linux-arm-kernel > From: Arnd Bergmann [mailto:arnd at arndb.de] > Sent: Tuesday, June 04, 2013 2:40 PM > > On Tuesday 04 June 2013 14:34:53 Lior Amsalem wrote: > > > From: Arnd Bergmann [mailto:arnd at arndb.de] > > > Sent: Wednesday, May 29, 2013 4:33 PM > > > > > > On Wednesday 29 May 2013 15:27:31 Gregory CLEMENT wrote: > > > > OK I will see what kind of information I can gather and which ones > > > > I can add to the Documentation/arm/Marvell/README file. > > > > > > > > My current understanding is that there are at least 3 kind of CPU: > > > > PJ4, PJ4B and PJ4B-MP and all of them belongs to the Sheeva family. > > > > > > Ok. So I guess PJ4B adds LPAE support, and PJ4B-MP adds SMP support, > > > right? Do either of them support hypervisor mode? > > > > It goes like this: > > PJ4B: improved version of PJ4. > > PJ4B-MP: adds LPAE and SMP. > > No support for virtualization on any. > > Ah, interesting. So PJ4B is just faster than PJ4 and has no extra instructions or > other features? It's a different microarchitecture, pipeline, but I don't know all the details. > > > > Regarding the errata: are those needed for production-level chips, > > > or just for prototypes? > > > > These are production level chips. > > Ok, thanks for the information. To clarify, does that mean that all future PJ4B > and PJ4B-MP based chips also need the errata, or is there is a chance they > will be fixed in future revisions? No, to my knowledge they're not going to be fixed. > > Arnd Regards, Lior Amsalem ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 3/3] ARM PJ4B: Add support for errata 4611 2013-05-29 11:22 ` Will Deacon 2013-05-29 12:57 ` Gregory CLEMENT @ 2013-06-03 7:16 ` Lior Amsalem 1 sibling, 0 replies; 18+ messages in thread From: Lior Amsalem @ 2013-06-03 7:16 UTC (permalink / raw) To: linux-arm-kernel > From: Will Deacon [mailto:will.deacon at arm.com] > Sent: Wednesday, May 29, 2013 2:22 PM > > On Wed, May 29, 2013 at 11:16:58AM +0100, Gregory CLEMENT wrote: > > From: Lior Amsalem <alior@marvell.com> > > > > A CP15 clean operation can result in a dead lock state if it is hit by > > an incoming snoop event. the fiw to this issue is the following: [Snip] > > a/arch/arm/include/asm/tlbflush.h b/arch/arm/include/asm/tlbflush.h > > index e3a8064..112f778 100644 > > --- a/arch/arm/include/asm/tlbflush.h > > +++ b/arch/arm/include/asm/tlbflush.h > > @@ -475,11 +475,22 @@ static inline void flush_pmd_entry(void *pmd) { > > const unsigned int __tlb_flag = __cpu_tlb_flags; > > > > +#ifdef CONFIG_PJ4B_ERRATA_4611 > > + unsigned long flags; > > + raw_local_irq_save(flags); > > + dmb(); > > +#endif > > + > > #ifdef CONFIG_PJ4B_ERRATA_6124 > > tlb_op(TLB_DCLEAN, "c7, c14, 1 @ flush_pmd", pmd); > > #else > > tlb_op(TLB_DCLEAN, "c7, c10, 1 @ flush_pmd", pmd); > > #endif > > + > > +#ifdef CONFIG_PJ4B_ERRATA_4611 > > + raw_local_irq_restore(flags); > > +#endif > > Eeek. Why do you have to disable interrupts during this? Again, are there no > chicken bits to save you? It's there in order to avoid an exception that will cause the workaround (of adding a DMB) to be dismissed. No chicken bit for it... > > > tlb_l2_op(TLB_L2CLEAN_FR, "c15, c9, 1 @ L2 flush_pmd", pmd); > > > > if (tlb_flag(TLB_WB)) > > @@ -490,11 +501,22 @@ static inline void clean_pmd_entry(void *pmd) { > > const unsigned int __tlb_flag = __cpu_tlb_flags; > > > > +#ifdef CONFIG_PJ4B_ERRATA_4611 > > + unsigned long flags; > > + raw_local_irq_save(flags); > > + dmb(); > > +#endif > > + > > #ifdef CONFIG_PJ4B_ERRATA_6124 > > tlb_op(TLB_DCLEAN, "c7, c14, 1 @ flush_pmd", pmd); > > #else > > tlb_op(TLB_DCLEAN, "c7, c10, 1 @ flush_pmd", pmd); > > #endif > > + > > +#ifdef CONFIG_PJ4B_ERRATA_4611 > > + raw_local_irq_restore(flags); > > +#endif > > + > > tlb_l2_op(TLB_L2CLEAN_FR, "c15, c9, 1 @ L2 flush_pmd", pmd); } > > > > diff --git a/arch/arm/mm/copypage-v6.c b/arch/arm/mm/copypage-v6.c > > index b9bcc9d..20b69ad 100644 > > --- a/arch/arm/mm/copypage-v6.c > > +++ b/arch/arm/mm/copypage-v6.c > > @@ -59,11 +59,22 @@ static void > v6_clear_user_highpage_nonaliasing(struct page *page, unsigned long > > */ > > static void discard_old_kernel_data(void *kto) { > > +#ifdef CONFIG_PJ4B_ERRATA_4611 > > + unsigned long flags; > > + > > + raw_local_irq_save(flags); > > + dmb(); > > +#endif > > + > > __asm__("mcrr p15, 0, %1, %0, c6 @ 0xec401f06" > > : > > : "r" (kto), > > "r" ((unsigned long)kto + PAGE_SIZE - L1_CACHE_BYTES) > > : "cc"); > > + > > +#ifdef CONFIG_PJ4B_ERRATA_4611 > > + raw_local_irq_restore(flags); #endif > > Do you actually have an aliasing vipt cache? If not, you don't care about this > function. No, It's a mistake, will be removed. 10x for pointing that out. > > > diff --git a/arch/arm/mm/proc-macros.S b/arch/arm/mm/proc-macros.S > > index f9a0aa7..ca81291 100644 > > --- a/arch/arm/mm/proc-macros.S > > +++ b/arch/arm/mm/proc-macros.S > > @@ -178,7 +178,20 @@ > > #endif > > > > str r3, [r0] > > + > > +#ifdef CONFIG_PJ4B_ERRATA_4611 > > + mrs r2, cpsr > > + orr r3, r2, #PSR_F_BIT | PSR_I_BIT > > + msr cpsr_c, r3 @ Disable interrupts > > + dmb @ ensure ordering with previous memory > accesses > > +#endif > > + > > mcr p15, 0, r0, c7, c10, 1 @ flush_pte > > + > > +#ifdef CONFIG_PJ4B_ERRATA_4611 > > + msr cpsr_c, r2 @ Restore interrupts > > + mcr p15, 0, r0, c7, c10, 4 @ drain write buffer > > +#endif > > This could be a dsb. Yes it can. > > Will Regards, Lior Amsalem ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 0/3] ARM: PJ4B: fixes for 3.10-rc 2013-05-29 10:16 [PATCH 0/3] ARM: PJ4B: fixes for 3.10-rc Gregory CLEMENT ` (2 preceding siblings ...) 2013-05-29 10:16 ` [PATCH 3/3] ARM PJ4B: Add support for errata 4611 Gregory CLEMENT @ 2013-05-29 11:03 ` Jason Cooper 3 siblings, 0 replies; 18+ messages in thread From: Jason Cooper @ 2013-05-29 11:03 UTC (permalink / raw) To: linux-arm-kernel Gregory, On Wed, May 29, 2013 at 12:16:55PM +0200, Gregory CLEMENT wrote: > Without the first one, the Armada 370 based board can't boot anymore > since 3.10-rc1 and the commit "ARM: 7691/1: mm: kill unused > TLB_CAN_READ_FROM_L1_CACHE and use ALT_SMP instead" . So this one > really need to be appplied on 3.10-rc. > > It should be a good thing to apply the others on the 3.10-rc, even we > didn't observe yet a problem related to these errata, they may happen. Since Linus in encouraging smaller -rc's, we should probably just submit the first patch to rmk's patch tracker for v3.10. The other two should be considered 'fixes-non-critical' for v3.11. thx, Jason. ^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2013-06-06 7:46 UTC | newest] Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2013-05-29 10:16 [PATCH 0/3] ARM: PJ4B: fixes for 3.10-rc Gregory CLEMENT 2013-05-29 10:16 ` [PATCH 1/3] ARM PJ4B: Add support for errata 4742 Gregory CLEMENT 2013-05-29 11:03 ` Will Deacon 2013-06-02 8:27 ` Lior Amsalem 2013-05-29 10:16 ` [PATCH 2/3] ARM PJ4B: Add support for errata 6124 Gregory CLEMENT 2013-05-29 11:16 ` Will Deacon 2013-06-03 7:15 ` Lior Amsalem 2013-05-29 10:16 ` [PATCH 3/3] ARM PJ4B: Add support for errata 4611 Gregory CLEMENT 2013-05-29 11:22 ` Will Deacon 2013-05-29 12:57 ` Gregory CLEMENT 2013-05-29 13:11 ` Arnd Bergmann 2013-05-29 13:27 ` Gregory CLEMENT 2013-05-29 13:33 ` Arnd Bergmann 2013-06-04 11:34 ` Lior Amsalem 2013-06-04 11:39 ` Arnd Bergmann 2013-06-06 7:46 ` Lior Amsalem 2013-06-03 7:16 ` Lior Amsalem 2013-05-29 11:03 ` [PATCH 0/3] ARM: PJ4B: fixes for 3.10-rc Jason Cooper
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.