ARM: Add SWP/SWPB emulation for ARMv7 processors (v3)
PATCH 1/1 ARM: Add SWP/SWPB emulation for ARMv7 processors (v3)
The SWP instruction was deprecated in the ARMv6 architecture, superseded by the LDREX/STREX family of instructions for load-linked/store-conditional operations. The ARMv7 multiprocessing extensions mandate that SWP/SWPB instructions are treated as undefined from reset, with the ability to enable them through the System Control Register SW bit.
This patch adds the alternative solution to emulate the SWP and SWPB instructions using LDREX/STREX sequences, and log statistics to /proc/cpu/swp_emulation. To correctly deal with copy-on-write, it also modifies cpu_v7_set_pte_ext to change the mappings to priviliged RO when user RO.
--- arch/arm/kernel/Makefile | 1 arch/arm/kernel/swp_emulate.c | 297 +++++++++++++++++++++++++++++++++++++++++ arch/arm/mm/Kconfig | 20 +++ arch/arm/mm/proc-v7.S | 6 + 4 files changed, 324 insertions(+), 0 deletions(-) create mode 100644 arch/arm/kernel/swp_emulate.c
arch/arm/kernel/Makefile +1 -0
@@ -38,6 +38,7 @@ obj-$(CONFIG_ARM_THUMBEE) += thumbee.o obj-$(CONFIG_KGDB) += kgdb.o obj-$(CONFIG_ARM_UNWIND) += unwind.o obj-$(CONFIG_HAVE_TCM) += tcm.o +obj-$(CONFIG_SWP_EMULATE) += swp_emulate.o obj-$(CONFIG_CRUNCH) += crunch.o crunch-bits.o AFLAGS_crunch-bits.o := -Wa,-mcpu=ep9312
arch/arm/kernel/swp_emulate.c new file +297 -0
297 lines not on this page · raw message
arch/arm/mm/Kconfig +20 -0
@@ -630,6 +630,26 @@ config ARM_THUMBEE Say Y here if you have a CPU with the ThumbEE extension and code to make use of it. Say N for code that can run on CPUs without ThumbEE. +config SWP_EMULATE + bool "Emulate SWP/SWPB instructions" + depends on CPU_V7 + default y if SMP + help + ARMv6 architecture deprecates use of the SWP/SWPB instructions. + ARMv7 multiprocessing extensions introduce the ability to disable + these instructions, triggering an undefined instruction exception + when executed. Say Y here to enable software emulation of these + instructions for userspace (not kernel) using LDREX/STREX. + Also creates /proc/cpu/swp_emulation for statistics. + + NOTE: when accessing uncached shared regions, LDREX/STREX rely + on an external transaction monitoring block called a global + monitor to maintain update atomicity. If your system does not + implement a global monitor, this option can cause programs that + are map uncached memory to deadlock. + + If unsure, say Y. + config CPU_BIG_ENDIAN bool "Build big-endian kernel" depends on ARCH_SUPPORTS_BIG_ENDIAN
arch/arm/mm/proc-v7.S +6 -0
@@ -147,8 +147,10 @@ ENTRY(cpu_v7_set_pte_ext) tst r1, #L_PTE_USER orrne r3, r3, #PTE_EXT_AP1 +#ifndef CONFIG_SWP_EMULATE tstne r3, #PTE_EXT_APX bicne r3, r3, #PTE_EXT_APX | PTE_EXT_AP0 +#endif tst r1, #L_PTE_EXEC orreq r3, r3, #PTE_EXT_XN
@@ -275,6 +277,10 @@ __v7_setup: #ifdef CONFIG_CPU_ENDIAN_BE8 orr r6, r6, #1 << 25 @ big-endian page tables #endif +#ifdef CONFIG_SWP_EMULATE + orr r5, r5, #(1 << 10) @ set SW bit in "clear" + bic r6, r6, #(1 << 10) @ clear it in "mmuset" +#endif mrc p15, 0, r0, c1, c0, 0 @ read control register bic r0, r0, r5 @ clear bits them orr r0, r0, r6 @ set them
PATCH 1/1 ARM: Add SWP/SWPB emulation for ARMv7 processors (v3)
Quoted 36 lines · Leif Lindholm
arch/arm/kernel/swp_emulate.c new file +36
-0+/* +* Error-checking SWP macros implemented using ldrex{b}/strex{b} +*/ +#define __user_swp_asm (data, addr, res) \ + __asm__ __volatile__( \ + " mov r3, %1\n" \ + "0: ldrex %1, [%2]\n" \ + "1: strex %0, r3, [%2]\n" \ + " cmp %0, #0\n" \ + " movne %0, %3\n" \ + "2:\n" \ + " .section .fixup,\"ax\"\n" \ + " .align 2\n" \ + "3: mov %0, %4\n" \ + " b 2b\n" \ + " .previous\n" \ + " .section __ex_table,\"a\"\n" \ + " .align 3\n" \ + " .long 0b, 3b\n" \ + " .long 1b, 3b\n" \ + " .previous" \ + : "=&r" (res), "+r" (data) \ + : "r" (addr), "i" (-EAGAIN), "i" (-EFAULT) \ + : "cc", "r3") + +#define __user_swpb_asm (data, addr, res) \ + __asm__ __volatile__( \ + " mov r3, %1\n" \ + "0: ldrexb %1, [%2]\n" \ + "1: strexb %0, r3, [%2]\n" \ + " cmp %0, #0\n" \ + " movne %0, %3\n" \ + "2:\n" \ + " .section .fixup,\"ax\"\n" \ + " .align 2\n" \ + "3: mov %0, %4\n" \
arch/arm/kernel/swp_emulate.c line 75
Quoted 10 lines
arch/arm/kernel/swp_emulate.c new file +10
-0+" b 2b\n" \ +" .previous\n" \ +" .section __ex_table,\"a\"\n" \ +" .align 3\n" \ +" .long 0b, 3b\n" \ +" .long 1b, 3b\n" \ +" .previous" \ + : "=&r" (res), "+r" (data) \ +: " r" (addr), " i" (-EAGAIN), " i" (-EFAULT) \ + : "cc", "r3")
They are almost identical. The duplication could be removed by folding it into a single macro with another argument, like this:
#define __user_swp_asm(data, addr, res, B) \ " mov r3, %1\n" \ "0: ldrex"B" %1, [%2]\n" \ "1: strex"B" %0, r3, [%2]\n" \
Then calling it like this:
__user_swp_asm(data, address, res, ""); __user_swp_asm(data, address, res, "b");
arch/arm/kernel/swp_emulate.c line 137
Quoted 4 lines
arch/arm/kernel/swp_emulate.c new file +4
-0+if (abtcounter ==UINT_MAX ) +printk (KERN_WARNING \ +"SWP{B} emulation abort counter wrapped!\n" ); + abtcounter++;
It's not atomic therefore not precise anyway. Why not just use u64, and skip the test and printk? The code will be shorter and ironically, faster with u64 because of omitting the test.
arch/arm/kernel/swp_emulate.c line 172
Quoted 5 lines
arch/arm/kernel/swp_emulate.c new file +4
-0+static int emulate_swp(struct pt_regs *regs, unsigned int address, + unsigned int destreg, unsigned int data) +static int emulate_swpb(struct pt_regs *regs, unsigned int address, + unsigned int destreg, unsigned int data)
Two almost identical functions. I wonder if it would be better to merge them and take a flag. It would also reduce the compiled code size.
arch/arm/kernel/swp_emulate.c line 157
Quoted 7 lines
arch/arm/kernel/swp_emulate.c new file +7
-0+do { +if (res == -EAGAIN ) +cond_resched (); + +smp_mb (); +__user_swp_asm (data, address, res); + }while ((res == -EAGAIN ) && !signal_pending (current));
Why is the smp_mb() needed? I don't doubt there's a reason, but I don't see what it is.
The loop looks ok, but it could be simpler in the common path:
while (1) {
smp_mb();
__user_swp_asm(data, address, res);
if (likely(res != -EAGAIN) || signal_pending(current))
break;
cond_resched();
}Review
Quoted 2 lines
(quoted diff) +2
-0+ if (res == 0) { + smp_mb();
Why the smp_mb?
arch/arm/kernel/swp_emulate.c line 165
Quoted 5 lines
arch/arm/kernel/swp_emulate.c new file +5
-0+ regs->uregs[destreg] = data; +if (swpcounter ==UINT_MAX ) +printk (KERN_WARNING \ +"SWP emulation counter wrapped!\n" ); + swpcounter++;
As with the other counter, it's probably faster and certainly shorter code to use a u64 here and omit the test and print.
arch/arm/kernel/swp_emulate.c line 283
Quoted 9 lines
arch/arm/kernel/swp_emulate.c new file +9
-0+#ifndef CONFIG_ALIGNMENT_TRAP + res =proc_mkdir ("cpu" ,NULL ); +if (!res) +return -ENOMEM ; + + res =create_proc_entry ("swp_emulation" ,S_IRUGO , res); +#else + res =create_proc_entry ("cpu/swp_emulation" ,S_IRUGO ,NULL ); +#endif
? Is that to work with different kernel versions?
arch/arm/mm/proc-v7.S ENTRY(cpu_v7_set_pte_ext) · line 153
Quoted 12 lines
arch/arm/mm/proc-v7.S +2
-0@@ -147,8 +147,10 @@ ENTRY(cpu_v7_set_pte_ext) tst r1, #L_PTE_USER orrne r3, r3, #PTE_EXT_AP1 +#ifndef CONFIG_SWP_EMULATE tstne r3, #PTE_EXT_APX bicne r3, r3, #PTE_EXT_APX | PTE_EXT_AP0 +#endif
Is this the part which changes kernel memory access to fault when writing to user-read-only pages? (I don't know anything about the details of this, btw).
Is there any reason why this wasn't always like that?
-- Jamie
PATCH 1/1 ARM: Add SWP/SWPB emulation for ARMv7 processors (v3)
Quoted 18 lines · Jamie Lokier
arch/arm/mm/proc-v7.S ENTRY(cpu_v7_set_pte_ext) · line 153
Quoted 12 lines · Leif Lindholm
arch/arm/mm/proc-v7.S +2
-0@@ -147,8 +147,10 @@ ENTRY(cpu_v7_set_pte_ext) tst r1, #L_PTE_USER orrne r3, r3, #PTE_EXT_AP1 +#ifndef CONFIG_SWP_EMULATE tstne r3, #PTE_EXT_APX bicne r3, r3, #PTE_EXT_APX | PTE_EXT_AP0 +#endifIs this the part which changes kernel memory access to fault when writing to user-read-only pages? (I don't know anything about the details of this, btw).
Quoted 1 line
Is there any reason why this wasn't always like that?
On ARMv6 onwards (where this user RO, kernel RO is supported) we cannot easily differentiate between the vectors page and a normal kernel page unless we use another L_PTE_ bit. We need the vectors page to be writable if there is no TLS register in hardware (I guess we could use domain switching to override this though). But on ARMv7 we always have a TLS register, so no need to write to the vectors page.
Signature · 1 line
Catalin
PATCH 1/1 ARM: Add SWP/SWPB emulation for ARMv7 processors (v3)
Quoted 6 lines · Catalin Marinas
On ARMv6 onwards (where this user RO, kernel RO is supported) we cannot easily differentiate between the vectors page and a normal kernel page unless we use another
L_PTE_bit. We need the vectors page to be writable if there is no TLS register in hardware (I guess we could use domain switching to override this though). But on ARMv7 we always have a TLS register, so no need to write to the vectors page.
That's not the only reason you'd want to write to the vectors page. You'll want to write to it to install or change FIQ handlers.
PATCH 1/1 ARM: Add SWP/SWPB emulation for ARMv7 processors (v3)
Quoted 10 lines · Russell King - ARM Linux
Quoted 6 lines · Catalin Marinas
On ARMv6 onwards (where this user RO, kernel RO is supported) we cannot easily differentiate between the vectors page and a normal kernel page unless we use another
L_PTE_bit. We need the vectors page to be writable if there is no TLS register in hardware (I guess we could use domain switching to override this though). But on ARMv7 we always have a TLS register, so no need to write to the vectors page.That's not the only reason you'd want to write to the vectors page. You'll want to write to it to install or change FIQ handlers.
I suspect that's not on a critical path, so the page tables could be modified temporarily (or use set_fs but I have a patch which tries to get rid of using the domains).
Signature · 1 line
Catalin
PATCH 1/1 ARM: Add SWP/SWPB emulation for ARMv7 processors (v3)
In reply to Catalin MarinasQuoted 8 lines · Catalin Marinas
Quoted 1 line
Is there any reason why this wasn't always like that?
On ARMv6 onwards (where this user RO, kernel RO is supported) we cannot easily differentiate between the vectors page and a normal kernel page unless we use another
L_PTE_bit. We need the vectors page to be writable if there is no TLS register in hardware (I guess we could use domain switching to override this though). But on ARMv7 we always have a TLS register, so no need to write to the vectors page.
Could you map the TLS page writable (kernel access only) at another address at the same time, carefully choosing an aliasing address so that no cache flush is needed after writing?
-- Jamie
PATCH 1/1 ARM: Add SWP/SWPB emulation for ARMv7 processors (v3)
Quoted 10 lines · Jamie Lokier
Quoted 8 lines · Catalin Marinas
Quoted 1 line
Is there any reason why this wasn't always like that?
On ARMv6 onwards (where this user RO, kernel RO is supported) we cannot easily differentiate between the vectors page and a normal kernel page unless we use another
L_PTE_bit. We need the vectors page to be writable if there is no TLS register in hardware (I guess we could use domain switching to override this though). But on ARMv7 we always have a TLS register, so no need to write to the vectors page.
Quoted 3 lines
Could you map the TLS page writable (kernel access only) at another address at the same time, carefully choosing an aliasing address so that no cache flush is needed after writing?
It could be but I'm not sure it's worth. Leif's patch is intended for ARMv7 where we always have a hardware TLS register. The FIQ handler installation could probably be done by first calling set_fs(KERNEL_DS).
Signature · 1 line
Catalin
PATCH 1/1 ARM: Add SWP/SWPB emulation for ARMv7 processors (v3)
Quoted 11 lines · Catalin Marinas
Quoted 10 lines · Jamie Lokier
Quoted 8 lines · Catalin Marinas
Quoted 1 line
Is there any reason why this wasn't always like that?
On ARMv6 onwards (where this user RO, kernel RO is supported) we cannot easily differentiate between the vectors page and a normal kernel page unless we use another
L_PTE_bit. We need the vectors page to be writable if there is no TLS register in hardware (I guess we could use domain switching to override this though). But on ARMv7 we always have a TLS register, so no need to write to the vectors page.
Quoted 7 lines
Quoted 3 lines
Could you map the TLS page writable (kernel access only) at another address at the same time, carefully choosing an aliasing address so that no cache flush is needed after writing?
It could be but I'm not sure it's worth. Leif's patch is intended for ARMv7 where we always have a hardware TLS register. The FIQ handler installation could probably be done by first calling
set_fs(KERNEL_DS).
You're right.
I was confused about the meaning of (user RO == kernel RW) pages, thinking that like on a 386, it makes copy_to_user expensive.
But I've just remembered that ARM uses ldrt and strt instructions for user memory access... doh!
So user RO == kernel RW hardly matters.
-- Jamie
PATCH 1/1 ARM: Add SWP/SWPB emulation for ARMv7 processors (v3)
In reply to Jamie LokierQuoted 11 lines
From: Jamie Lokier [mailto:jamie at shareable.org]
Sent: 05 January 2010 19:43They are almost identical. The duplication could be removed by folding it into a single macro with another argument, like this:
#define __user_swp_asm(data, addr, res, B) \ " mov r3, %1\n" \ "0: ldrex"B" %1, [%2]\n" \ "1: strex"B" %0, r3, [%2]\n" \
Quoted 4 lines
Then calling it like this:
__user_swp_asm(data, address, res, ""); __user_swp_asm(data, address, res, "b");
Neat.
But how about, for clarity, keeping the calling syntax in the calling functions and add macros for the variants?:
#define __user_swp_asm_generic(data, addr, res, B) \
...
#define __user_swp_asm(data, addr, res) \
__user_swp_asm_generic(data, addr, res, "")
#define __user_swpb_asm(data, addr, res) \
__user_swp_asm_generic(data, addr, res, "b")Quoted 8 lines
arch/arm/kernel/swp_emulate.c line 137
Quoted 4 lines
arch/arm/kernel/swp_emulate.c new file +4
-0+if (abtcounter ==UINT_MAX ) +printk (KERN_WARNING \ +"SWP{B} emulation abort counter wrapped!\n" ); + abtcounter++;It's not atomic therefore not precise anyway. Why not just use u64, and skip the test and printk? The code will be shorter and ironically, faster with u64 because of omitting the test.
Fair enough. Will do.
Quoted 9 lines
arch/arm/kernel/swp_emulate.c line 172
Quoted 5 lines
arch/arm/kernel/swp_emulate.c new file +4
-0+static int emulate_swp(struct pt_regs *regs, unsigned int address, + unsigned int destreg, unsigned int data) +static int emulate_swpb(struct pt_regs *regs, unsigned int address, + unsigned int destreg, unsigned int data)Two almost identical functions. I wonder if it would be better to merge them and take a flag. It would also reduce the compiled code size.
I'm hesitant to add more than 4 arguments (adds stack overhead). Also, at least cs2009q3 gcc (4.3.3) seems to inline both of these, so not sure a codesize improvement would occur in practise.
An alternative would of course be to merge them into swp_handler(), but that would make that function a bit messy.
Quoted 2 lines
Why is the
smp_mb()needed? I don't doubt there's a reason, but I don't see what it is.
A DMB is required between acquiring a lock and accessing the protected resource, as well as between modifying a protected resource and releasing its lock. Because there is no way to tell whether the SWP performed a lock or unlock operation, inserting the barriers on either side seemed the safest way to ensure that code written for ARMv5 or earlier would work as expected.
I guess a case could be made that this is an application problem and should be resolved at that end.
Quoted 9 lines
The loop looks ok, but it could be simpler in the common path:
while (1) { smp_mb(); __user_swp_asm(data, address, res); if (likely(res != -EAGAIN) || signal_pending(current)) break; cond_resched(); }
Good point, will do that in the next version.
Quoted 4 lines
arch/arm/kernel/swp_emulate.c line 276
Quoted 2 lines
arch/arm/kernel/swp_emulate.c new file +2
-0+#ifndef CONFIG_ALIGNMENT_TRAP + res =proc_mkdir ("cpu" ,NULL );? Is that to work with different kernel versions?
It's to ensure it would work (without console warnings) even if someone decides to disable ALIGNMENT_TRAP. An alternative would be to strip the creation of /proc/cpu out from mm/alignment.c and put it somewhere else (or move the stats file somewhere else - but it seemed logical to group with /proc/alignment).
/
Leif
PATCH 1/1 ARM: Add SWP/SWPB emulation for ARMv7 processors (v3)
Quoted 7 lines · Leif Lindholm
Good point, will do that in the next version.
Quoted 4 lines
Review
Quoted 2 lines
(quoted diff) +2
-0+#ifndef CONFIG_ALIGNMENT_TRAP + res = proc_mkdir("cpu", NULL);? Is that to work with different kernel versions?
Quoted 5 lines
It's to ensure it would work (without console warnings) even if someone decides to disable
ALIGNMENT_TRAP. An alternative would be to strip the creation of/proc/cpuout frommm/alignment.cand put it somewhere else (or move the stats file somewhere else - but it seemed logical to group with/proc/alignment).
As I said when you previously posted this patch, that's definitely on the cards. In fact, I'm about to commit the following patch for doing exactly this.
arch/arm/Kconfig +4 -0
@@ -50,6 +50,9 @@ config HAVE_TCM bool select GENERIC_ALLOCATOR +config HAVE_PROC_CPU + bool + config NO_IOPORT bool
@@ -1226,6 +1229,7 @@ config ALIGNMENT_TRAP bool depends on CPU_CP15_MMU default y if !ARCH_EBSA110 + select HAVE_PROC_CPU if PROC_FS help ARM processors cannot fetch/store information which is not naturally aligned on the bus, i.e., a 4 byte fetch must start at an
arch/arm/kernel/setup.c +14 -1
@@ -24,6 +24,7 @@#include <linux/interrupt.h> #include <linux/smp.h> #include <linux/fs.h> +#include <linux/proc_fs.h> #include <asm/unified.h> #include <asm/cpu.h>
@@ -782,9 +783,21 @@ static int __init topology_init(void)return 0 ; } -subsys_initcall (topology_init); +#ifdef CONFIG_HAVE_PROC_CPU +static int __init proc_cpu_init (void ) +{ +struct proc_dir_entry *res; + + res =proc_mkdir ("cpu" ,NULL ); +if (!res) +return -ENOMEM ; +return 0 ; +} +fs_initcall (proc_cpu_init); +#endif +static const char *hwcap_str[] = {"swp" ,"half" ,
arch/arm/mm/alignment.c +1 -5
@@ -898,11 +898,7 @@ static int __init alignment_init(void)#ifdef CONFIG_PROC_FS struct proc_dir_entry *res; - res =proc_mkdir ("cpu" ,NULL ); -if (!res) -return -ENOMEM ; - - res =create_proc_entry ("alignment" ,S_IWUSR |S_IRUGO , res); + res =create_proc_entry ("cpu/alignment" ,S_IWUSR |S_IRUGO ,NULL );if (!res)return -ENOMEM ;
PATCH 1/1 ARM: Add SWP/SWPB emulation for ARMv7 processors (v3)
Quoted 9 lines
From: Russell King - ARM Linux [mailto:linux at arm.linux.org.uk]
Sent: 06 January 2010 19:37Quoted 5 lines
It's to ensure it would work (without console warnings) even if someone decides to disable
ALIGNMENT_TRAP. An alternative would be to strip the creation of/proc/cpuout frommm/alignment.cand put it somewhere else (or move the stats file somewhere else - but it seemed logical to group with/proc/alignment).
Quoted 3 lines
As I said when you previously posted this patch, that's definitely on the cards. In fact, I'm about to commit the following patch for doing exactly this.
Excellent, thanks. I'm just about to submit the next version, which relies on that patch, and no longer checks ALIGNMENT_TRAP for anything.
Other changes:
- Code restructured a bit, from Jamie's comments. It did end up quite a bit cleaner because of this.
- Comments added for memory barriers, as pointed out by Jamie.
- Added description in Documentation/arm/swp_emulation.
/
Leif