* [FIX v2] v3.8-rc: fix for MM ASID code in BE mode @ 2013-02-10 14:57 Ben Dooks 2013-02-10 14:57 ` [PATCH 1/2] ARM: mm: replace direct access to mm->context.id with new macro Ben Dooks 2013-02-10 14:57 ` [PATCH 2/2] ARM: mm: mm->context.id fix for big-endian Ben Dooks 0 siblings, 2 replies; 11+ messages in thread From: Ben Dooks @ 2013-02-10 14:57 UTC (permalink / raw) To: linux-arm-kernel This is a pair of patches to fix the ASID allocation in BE mode, and probably should be merged as soon as possible. This is a n Also available at: git://git.baserock.org/delta/linux baserock/bjdooks/arm-fix-asid [PATCH 1/2] ARM: mm: replace direct access to mm->context.id with [PATCH 2/2] ARM: mm: mm->context.id fix for big-endian ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/2] ARM: mm: replace direct access to mm->context.id with new macro 2013-02-10 14:57 [FIX v2] v3.8-rc: fix for MM ASID code in BE mode Ben Dooks @ 2013-02-10 14:57 ` Ben Dooks 2013-02-10 14:57 ` [PATCH 2/2] ARM: mm: mm->context.id fix for big-endian Ben Dooks 1 sibling, 0 replies; 11+ messages in thread From: Ben Dooks @ 2013-02-10 14:57 UTC (permalink / raw) To: linux-arm-kernel The mmid macro is meant to be used to get the mm->context.id data from the mm structure, but it seems to have been missed in a cuple of files. Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk> --- arch/arm/mm/proc-v6.S | 2 +- arch/arm/mm/proc-v7-2level.S | 2 +- arch/arm/mm/proc-v7-3level.S | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/arch/arm/mm/proc-v6.S b/arch/arm/mm/proc-v6.S index 09c5233..bcaaa8d 100644 --- a/arch/arm/mm/proc-v6.S +++ b/arch/arm/mm/proc-v6.S @@ -101,7 +101,7 @@ ENTRY(cpu_v6_dcache_clean_area) ENTRY(cpu_v6_switch_mm) #ifdef CONFIG_MMU mov r2, #0 - ldr r1, [r1, #MM_CONTEXT_ID] @ get mm->context.id + mmid r1, r1 @ get mm->context.id ALT_SMP(orr r0, r0, #TTB_FLAGS_SMP) ALT_UP(orr r0, r0, #TTB_FLAGS_UP) mcr p15, 0, r2, c7, c5, 6 @ flush BTAC/BTB diff --git a/arch/arm/mm/proc-v7-2level.S b/arch/arm/mm/proc-v7-2level.S index 6d98c13..78f520b 100644 --- a/arch/arm/mm/proc-v7-2level.S +++ b/arch/arm/mm/proc-v7-2level.S @@ -40,7 +40,7 @@ ENTRY(cpu_v7_switch_mm) #ifdef CONFIG_MMU mov r2, #0 - ldr r1, [r1, #MM_CONTEXT_ID] @ get mm->context.id + mmid r1, r1 @ get mm->context.id ALT_SMP(orr r0, r0, #TTB_FLAGS_SMP) ALT_UP(orr r0, r0, #TTB_FLAGS_UP) #ifdef CONFIG_ARM_ERRATA_430973 diff --git a/arch/arm/mm/proc-v7-3level.S b/arch/arm/mm/proc-v7-3level.S index 7b56386..50bf1da 100644 --- a/arch/arm/mm/proc-v7-3level.S +++ b/arch/arm/mm/proc-v7-3level.S @@ -47,7 +47,7 @@ */ ENTRY(cpu_v7_switch_mm) #ifdef CONFIG_MMU - ldr r1, [r1, #MM_CONTEXT_ID] @ get mm->context.id + mmid r1, r1 @ get mm->context.id and r3, r1, #0xff mov r3, r3, lsl #(48 - 32) @ ASID mcrr p15, 0, r0, r3, c2 @ set TTB 0 -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 2/2] ARM: mm: mm->context.id fix for big-endian 2013-02-10 14:57 [FIX v2] v3.8-rc: fix for MM ASID code in BE mode Ben Dooks 2013-02-10 14:57 ` [PATCH 1/2] ARM: mm: replace direct access to mm->context.id with new macro Ben Dooks @ 2013-02-10 14:57 ` Ben Dooks 2013-02-10 15:05 ` Ben Dooks 1 sibling, 1 reply; 11+ messages in thread From: Ben Dooks @ 2013-02-10 14:57 UTC (permalink / raw) To: linux-arm-kernel Since the new ASID code in [b5466f8728527a05a493cc4abe9e6f034a1bbaab] was changed to use 64bit operations it has broken the BE operation due to an issue with the MM code accessing sub-fields of mm->context.id. When running in BE mode we see the values in mm->context.id are stored with the highest value first, so the LDR in the arch/arm/mm/proc-macros.S reads the wrong part of this field. To resolve this, change the LDR in the mmid macro to load from +4. Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk> --- arch/arm/mm/context.c | 3 +++ arch/arm/mm/proc-macros.S | 5 +++++ 2 files changed, 8 insertions(+) diff --git a/arch/arm/mm/context.c b/arch/arm/mm/context.c index bc4a5e9..7a05111 100644 --- a/arch/arm/mm/context.c +++ b/arch/arm/mm/context.c @@ -34,6 +34,9 @@ * The ASID is used to tag entries in the CPU caches and TLBs. * The context ID is used by debuggers and trace logic, and * should be unique within all running processes. + * + * In big endian operation, the two 32 bit words are swapped if accesed by + * non 64-bit operations. */ #define ASID_FIRST_VERSION (1ULL << ASID_BITS) #define NUM_USER_ASIDS (ASID_FIRST_VERSION - 1) diff --git a/arch/arm/mm/proc-macros.S b/arch/arm/mm/proc-macros.S index eb6aa73..f9a0aa7 100644 --- a/arch/arm/mm/proc-macros.S +++ b/arch/arm/mm/proc-macros.S @@ -38,9 +38,14 @@ /* * mmid - get context id from mm pointer (mm->context.id) + * note, this field is 64bit, so in big-endian the two words are swapped too. */ .macro mmid, rd, rn +#ifdef __ARMEB__ + ldr \rd, [\rn, #MM_CONTEXT_ID + 4 ] +#else ldr \rd, [\rn, #MM_CONTEXT_ID] +#endif .endm /* -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 2/2] ARM: mm: mm->context.id fix for big-endian 2013-02-10 14:57 ` [PATCH 2/2] ARM: mm: mm->context.id fix for big-endian Ben Dooks @ 2013-02-10 15:05 ` Ben Dooks 2013-02-10 23:29 ` Russell King - ARM Linux 0 siblings, 1 reply; 11+ messages in thread From: Ben Dooks @ 2013-02-10 15:05 UTC (permalink / raw) To: linux-arm-kernel On 10/02/2013 14:57, Ben Dooks wrote: > Since the new ASID code in [b5466f8728527a05a493cc4abe9e6f034a1bbaab] > was changed to use 64bit operations it has broken the BE operation > due > to an issue with the MM code accessing sub-fields of mm->context.id. Fixed the ref, new patch pushed to git. ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 2/2] ARM: mm: mm->context.id fix for big-endian 2013-02-10 15:05 ` Ben Dooks @ 2013-02-10 23:29 ` Russell King - ARM Linux 2013-02-11 10:38 ` Ben Dooks 0 siblings, 1 reply; 11+ messages in thread From: Russell King - ARM Linux @ 2013-02-10 23:29 UTC (permalink / raw) To: linux-arm-kernel On Sun, Feb 10, 2013 at 03:05:11PM +0000, Ben Dooks wrote: > On 10/02/2013 14:57, Ben Dooks wrote: >> Since the new ASID code in [b5466f8728527a05a493cc4abe9e6f034a1bbaab] >> was changed to use 64bit operations it has broken the BE operation due >> to an issue with the MM code accessing sub-fields of mm->context.id. > > Fixed the ref, new patch pushed to git. No you haven't. "Since the new ASID code in b5466f872 (ARM: mm: remove IPI broadcasting on ASID rollover)..." is the correct way to refer to another patch by commit ID - both the ID _and_ its summary line. You can use a short ID but it's a good idea to add a few additional characters to avoid future hash clashes. ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 2/2] ARM: mm: mm->context.id fix for big-endian 2013-02-10 23:29 ` Russell King - ARM Linux @ 2013-02-11 10:38 ` Ben Dooks 2013-02-11 10:51 ` Will Deacon 0 siblings, 1 reply; 11+ messages in thread From: Ben Dooks @ 2013-02-11 10:38 UTC (permalink / raw) To: linux-arm-kernel On 10/02/13 23:29, Russell King - ARM Linux wrote: > On Sun, Feb 10, 2013 at 03:05:11PM +0000, Ben Dooks wrote: >> On 10/02/2013 14:57, Ben Dooks wrote: >>> Since the new ASID code in [b5466f8728527a05a493cc4abe9e6f034a1bbaab] >>> was changed to use 64bit operations it has broken the BE operation due >>> to an issue with the MM code accessing sub-fields of mm->context.id. >> >> Fixed the ref, new patch pushed to git. > > No you haven't. > > "Since the new ASID code in b5466f872 (ARM: mm: remove IPI broadcasting > on ASID rollover)..." > > is the correct way to refer to another patch by commit ID - both the ID > _and_ its summary line. You can use a short ID but it's a good idea to > add a few additional characters to avoid future hash clashes. My repo is showing: Since the new ASID code in b5466f8728527a05a493cc4abe9e6f034a1bbaab ("ARM: mm: remove IPI broadcasting on ASID rollover") was changed to use 64bit operations it has broken the BE operation due to an issue with the MM code accessing sub-fields of mm->context.id. Would you like the commit-id shortened? Is there anything else before this pair can be pulled? -- Ben Dooks http://www.codethink.co.uk/ Senior Engineer Codethink - Providing Genius ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 2/2] ARM: mm: mm->context.id fix for big-endian 2013-02-11 10:38 ` Ben Dooks @ 2013-02-11 10:51 ` Will Deacon 0 siblings, 0 replies; 11+ messages in thread From: Will Deacon @ 2013-02-11 10:51 UTC (permalink / raw) To: linux-arm-kernel On Mon, Feb 11, 2013 at 10:38:29AM +0000, Ben Dooks wrote: > On 10/02/13 23:29, Russell King - ARM Linux wrote: > > On Sun, Feb 10, 2013 at 03:05:11PM +0000, Ben Dooks wrote: > >> On 10/02/2013 14:57, Ben Dooks wrote: > >>> Since the new ASID code in [b5466f8728527a05a493cc4abe9e6f034a1bbaab] > >>> was changed to use 64bit operations it has broken the BE operation due > >>> to an issue with the MM code accessing sub-fields of mm->context.id. > >> > >> Fixed the ref, new patch pushed to git. > > > > No you haven't. > > > > "Since the new ASID code in b5466f872 (ARM: mm: remove IPI broadcasting > > on ASID rollover)..." > > > > is the correct way to refer to another patch by commit ID - both the ID > > _and_ its summary line. You can use a short ID but it's a good idea to > > add a few additional characters to avoid future hash clashes. > > My repo is showing: > > Since the new ASID code in b5466f8728527a05a493cc4abe9e6f034a1bbaab > ("ARM: mm: remove IPI broadcasting on ASID rollover") was changed to > use 64bit operations it has broken the BE operation due to an issue > with the MM code accessing sub-fields of mm->context.id. > > Would you like the commit-id shortened? > Is there anything else before this pair can be pulled? You can add my acked-by if you like: Acked-by: Will Deacon <will.deacon@arm.com> Given that these are two small patches without any dependencies, it's probably easier to stick them in the patch system (which will also resolve any confusion about the form of the final commits). Cheers, Will ^ permalink raw reply [flat|nested] 11+ messages in thread
* v3.8-rc issues with big endian @ 2013-02-09 23:15 Ben Dooks 2013-02-09 23:15 ` [PATCH 2/2] ARM: mm: mm->context.id fix for big-endian Ben Dooks 0 siblings, 1 reply; 11+ messages in thread From: Ben Dooks @ 2013-02-09 23:15 UTC (permalink / raw) To: linux-arm-kernel Whilst debugging with big endian ARM on v3.8-rc I came across a problem with the new ASID code (commit b5466f8728527a05a493cc4abe9e6f034a1bbaab) which is causing issues with user-land failing to work. Since the new ASID code is using 64-bit accesses, the ASID ID is now being placed in context->id + 4, which means that the assembly mm code no-longer works. I also noticed and fixed a couple of places where the new mmid macro was not being used, so fixed those too. Tested on a highbank system. [PATCH 1/2] ARM: mm: replace direct access to mm->context.id with [PATCH 2/2] ARM: mm: mm->context.id fix for big-endian ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 2/2] ARM: mm: mm->context.id fix for big-endian 2013-02-09 23:15 v3.8-rc issues with big endian Ben Dooks @ 2013-02-09 23:15 ` Ben Dooks 2013-02-10 1:10 ` Ben Dooks 2013-02-10 14:14 ` Sergei Shtylyov 0 siblings, 2 replies; 11+ messages in thread From: Ben Dooks @ 2013-02-09 23:15 UTC (permalink / raw) To: linux-arm-kernel Since the new ASID code introduced in b5466f8728527a05a493cc4abe9e6f034a1bbaab uses 64bit operations, when running in BE mode we see the values in mm->context.id swapped around as the lowest word is being stored in mm->context.id + 4, instead of + 0. Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk> --- arch/arm/mm/proc-macros.S | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/arch/arm/mm/proc-macros.S b/arch/arm/mm/proc-macros.S index eb6aa73..5a6a347 100644 --- a/arch/arm/mm/proc-macros.S +++ b/arch/arm/mm/proc-macros.S @@ -40,7 +40,11 @@ * mmid - get context id from mm pointer (mm->context.id) */ .macro mmid, rd, rn +#ifdef __ARMEB__ + ldr \rd, [\rn, #MM_CONTEXT_ID + 4 ] +#else ldr \rd, [\rn, #MM_CONTEXT_ID] +#endif .endm /* -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 2/2] ARM: mm: mm->context.id fix for big-endian 2013-02-09 23:15 ` [PATCH 2/2] ARM: mm: mm->context.id fix for big-endian Ben Dooks @ 2013-02-10 1:10 ` Ben Dooks 2013-02-10 14:14 ` Sergei Shtylyov 1 sibling, 0 replies; 11+ messages in thread From: Ben Dooks @ 2013-02-10 1:10 UTC (permalink / raw) To: linux-arm-kernel On 09/02/2013 23:15, Ben Dooks wrote: > Since the new ASID code introduced in > b5466f8728527a05a493cc4abe9e6f034a1bbaab > uses 64bit operations, when running in BE mode we see the values in > mm->context.id swapped around as the lowest word is being stored in > mm->context.id + 4, instead of + 0. The one thing I have just realised with this is that we are no-longer going to be storing the mm->context.id data in the sat was a pre the ASID changes, which could confuse debuggers which assume they can read the data. Should we fix this the easy way below, or go and change the code in arch/arm/mm/context.c to deal with re-arranging the atomic64 accesses when in big-endian mode? > Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk> > --- > arch/arm/mm/proc-macros.S | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/arch/arm/mm/proc-macros.S b/arch/arm/mm/proc-macros.S > index eb6aa73..5a6a347 100644 > --- a/arch/arm/mm/proc-macros.S > +++ b/arch/arm/mm/proc-macros.S > @@ -40,7 +40,11 @@ > * mmid - get context id from mm pointer (mm->context.id) > */ > .macro mmid, rd, rn > +#ifdef __ARMEB__ > + ldr \rd, [\rn, #MM_CONTEXT_ID + 4 ] > +#else > ldr \rd, [\rn, #MM_CONTEXT_ID] > +#endif > .endm > > /* ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 2/2] ARM: mm: mm->context.id fix for big-endian 2013-02-09 23:15 ` [PATCH 2/2] ARM: mm: mm->context.id fix for big-endian Ben Dooks 2013-02-10 1:10 ` Ben Dooks @ 2013-02-10 14:14 ` Sergei Shtylyov 2013-02-10 14:17 ` Ben Dooks 1 sibling, 1 reply; 11+ messages in thread From: Sergei Shtylyov @ 2013-02-10 14:14 UTC (permalink / raw) To: linux-arm-kernel Hello. On 10-02-2013 3:15, Ben Dooks wrote: > Since the new ASID code introduced in b5466f8728527a05a493cc4abe9e6f034a1bbaab Plase also specify that commit's summary in parens (or however you like). > uses 64bit operations, when running in BE mode we see the values in > mm->context.id swapped around as the lowest word is being stored in > mm->context.id + 4, instead of + 0. > Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk> WBR, Sergei ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 2/2] ARM: mm: mm->context.id fix for big-endian 2013-02-10 14:14 ` Sergei Shtylyov @ 2013-02-10 14:17 ` Ben Dooks 0 siblings, 0 replies; 11+ messages in thread From: Ben Dooks @ 2013-02-10 14:17 UTC (permalink / raw) To: linux-arm-kernel On 10/02/2013 14:14, Sergei Shtylyov wrote: > Hello. > > On 10-02-2013 3:15, Ben Dooks wrote: > >> Since the new ASID code introduced in >> b5466f8728527a05a493cc4abe9e6f034a1bbaab > > Plase also specify that commit's summary in parens (or however you > like). > >> uses 64bit operations, when running in BE mode we see the values in >> mm->context.id swapped around as the lowest word is being stored in >> mm->context.id + 4, instead of + 0. > >> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk> > > WBR, Sergei Thanks, will sort that out and also take into account other feedback and put new patches out today. -- Ben ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2013-02-11 10:51 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-02-10 14:57 [FIX v2] v3.8-rc: fix for MM ASID code in BE mode Ben Dooks 2013-02-10 14:57 ` [PATCH 1/2] ARM: mm: replace direct access to mm->context.id with new macro Ben Dooks 2013-02-10 14:57 ` [PATCH 2/2] ARM: mm: mm->context.id fix for big-endian Ben Dooks 2013-02-10 15:05 ` Ben Dooks 2013-02-10 23:29 ` Russell King - ARM Linux 2013-02-11 10:38 ` Ben Dooks 2013-02-11 10:51 ` Will Deacon -- strict thread matches above, loose matches on Subject: below -- 2013-02-09 23:15 v3.8-rc issues with big endian Ben Dooks 2013-02-09 23:15 ` [PATCH 2/2] ARM: mm: mm->context.id fix for big-endian Ben Dooks 2013-02-10 1:10 ` Ben Dooks 2013-02-10 14:14 ` Sergei Shtylyov 2013-02-10 14:17 ` Ben Dooks
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).