Skip to contentSkip to conversation

Fix Thumb-2 undef handling for mixed-arch kernels

RFC PATCH New Dave Martin 4 participants 17 messages Aug 10, 2011 – Sep 12, 2011

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

cover Fix Thumb-2 undef handling for mixed-arch kernels

In kernels which support v6 and v7 platforms in a single binary such as omap2plus_defconfig for example, undefined instruction exceptions taken on Thumb instructions are not processed correctly, because the kernel assumes at build-time that Thumb-2 won't be supported in userspace.

This series implements a workaround, by allowing the __und_usr handler to check the CPU architecture at runtime, in affected kernels.

As a side-effect, this also changes cpu_architecture from a function into a global variable initialised at boot-time, which is probably a sensible idea anyway.

While I think the approach in this series is roughly correct, it's not very pretty -- in anyone was a cleaner solution to suggest, I'm open to ideas.

Note that much of the actual diffstat comes from changing references to cpu_architecture().

Dave Martin (5):
ARM: Make cpu_alignment into a global variable
ARM: s3c24xx: Reference cpu_architecture as a global variable
ARM: kprobes: Reference cpu_architecture as a global variable
ARM: entry: Remove unnecessary masking when decoding Thumb-2
instructions
ARM: entry: Fix Thumb-2 undef handling for multi-CPU kernels

 arch/arm/include/asm/system.h    |    2 +-
 arch/arm/kernel/elf.c            |    4 ++-
 arch/arm/kernel/entry-armv.S     |   44 +++++++++++++++++++++++++++++++++----
 arch/arm/kernel/kprobes-common.c |   11 ++++-----
 arch/arm/kernel/setup.c          |   12 ++++++----
 arch/arm/kernel/thumbee.c        |    4 +-
 arch/arm/mm/alignment.c          |    9 ++++---
 arch/arm/mm/fault.c              |    4 +-
 arch/arm/mm/idmap.c              |    3 +-
 arch/arm/mm/ioremap.c            |    3 +-
 arch/arm/mm/mmu.c                |   31 ++++++++++++++------------
 arch/arm/plat-s3c24xx/cpu.c      |    3 +-
 arch/arm/vfp/vfpmodule.c         |    4 +-
 13 files changed, 89 insertions(+), 45 deletions(-)
Signature · 1 line
1.7.4.1
#1 lore raw

RFC PATCH 1/5 ARM: Make cpu_alignment into a global variable

The CPU architecture really should not be changing at runtime, so make it a global variable instead of a function.

---
 arch/arm/include/asm/system.h |    2 +-
 arch/arm/kernel/elf.c         |    4 +++-
 arch/arm/kernel/setup.c       |   12 +++++++-----
 arch/arm/kernel/thumbee.c     |    4 ++--
 arch/arm/mm/alignment.c       |    9 +++++----
 arch/arm/mm/fault.c           |    4 ++--
 arch/arm/mm/idmap.c           |    3 ++-
 arch/arm/mm/ioremap.c         |    3 ++-
 arch/arm/mm/mmu.c             |   31 +++++++++++++++++--------------
 arch/arm/vfp/vfpmodule.c      |    4 ++--
 10 files changed, 43 insertions(+), 33 deletions(-)
arch/arm/include/asm/system.h +1 -1
index 832888d..e7acb6e 100644
@@ -104,7 +104,7 @@ struct mm_struct;
 extern void show_pte(struct mm_struct *mm, unsigned long addr);
 extern void __show_regs(struct pt_regs *);
 
-extern int cpu_architecture(void);
+extern int cpu_architecture;
 extern void cpu_init(void);
 
 void arm_machine_restart(char mode, const char *cmd);
arch/arm/kernel/elf.c +3 -1
index 9b05c6a..174e90d 100644
@@ -4,6 +4,8 @@
 #include <linux/binfmts.h>
 #include <linux/elf.h>
 
+#include <asm/system.h>
+
 int elf_check_arch(const struct elf32_hdr *x)
 {
 	unsigned int eflags;
@@ -83,7 +85,7 @@ int arm_elf_read_implies_exec(const struct elf32_hdr *x, int executable_stack)
 {
 	if (executable_stack != EXSTACK_DISABLE_X)
 		return 1;
-	if (cpu_architecture() < CPU_ARCH_ARMv6)
+	if (cpu_architecture < CPU_ARCH_ARMv6)
 		return 1;
 	return 0;
 }
arch/arm/kernel/setup.c +7 -5
index 70bca64..db67274 100644
@@ -42,6 +42,7 @@
 #include <asm/cacheflush.h>
 #include <asm/cachetype.h>
 #include <asm/tlbflush.h>
+#include <asm/system.h>
 
 #include <asm/prom.h>
 #include <asm/mach/arch.h>
@@ -114,6 +115,7 @@ struct cpu_cache_fns cpu_cache __read_mostly;
 struct outer_cache_fns outer_cache __read_mostly;
 EXPORT_SYMBOL(outer_cache);
 #endif
+int cpu_architecture __read_mostly;
 
 struct stack {
 	u32 irq[3];
@@ -210,7 +212,7 @@ static const char *proc_arch[] = {
 	"?(17)",
 };
 
-int cpu_architecture(void)
+static int __init __cpu_architecture(void)
 {
 	int cpu_arch;
 
@@ -275,9 +277,8 @@ static int cpu_has_aliasing_icache(unsigned int arch)
 static void __init cacheid_init(void)
 {
 	unsigned int cachetype = read_cpuid_cachetype();
-	unsigned int arch = cpu_architecture();
 
-	if (arch >= CPU_ARCH_ARMv6) {
+	if (cpu_architecture >= CPU_ARCH_ARMv6) {
 		if ((cachetype & (7 << 29)) == 4 << 29) {
 			/* ARMv7 register format */
 			cacheid = CACHEID_VIPT_NONALIASING;
@@ -413,6 +414,7 @@ static void __init setup_processor(void)
 	}
 
 	cpu_name = list->cpu_name;
+	cpu_architecture = __cpu_architecture();
 
 #ifdef MULTI_CPU
 	processor = *list->proc;
@@ -429,7 +431,7 @@ static void __init setup_processor(void)
 
 	printk("CPU: %s [%08x] revision %d (ARMv%s), cr=%08lx\n",
 	       cpu_name, read_cpuid_id(), read_cpuid_id() & 15,
-	       proc_arch[cpu_architecture()], cr_alignment);
+	       proc_arch[cpu_architecture], cr_alignment);
 
 	sprintf(init_utsname()->machine, "%s%c", list->arch_name, ENDIANNESS);
 	sprintf(elf_platform, "%s%c", list->elf_name, ENDIANNESS);
@@ -1026,7 +1028,7 @@ static int c_show(struct seq_file *m, void *v)
 			seq_printf(m, "%s ", hwcap_str[i]);
 
 	seq_printf(m, "\nCPU implementer\t: 0x%02x\n", read_cpuid_id() >> 24);
-	seq_printf(m, "CPU architecture: %s\n", proc_arch[cpu_architecture()]);
+	seq_printf(m, "CPU architecture: %s\n", proc_arch[cpu_architecture]);
 
 	if ((read_cpuid_id() & 0x0008f000) == 0x00000000) {
 		/* pre-ARM7 */
arch/arm/kernel/thumbee.c +2 -2
index 9cb7aac..12ac868 100644
@@ -20,6 +20,7 @@
 #include <linux/kernel.h>
 #include <linux/init.h>
 
+#include <asm/system.h>
 #include <asm/thread_notify.h>
 
 /*
@@ -61,9 +62,8 @@ static struct notifier_block thumbee_notifier_block = {
 static int __init thumbee_init(void)
 {
 	unsigned long pfr0;
-	unsigned int cpu_arch = cpu_architecture();
 
-	if (cpu_arch < CPU_ARCH_ARMv7)
+	if (cpu_architecture < CPU_ARCH_ARMv7)
 		return 0;
 
 	/* processor feature register 0 */
arch/arm/mm/alignment.c +5 -4
index be7c638..7188c81 100644
@@ -23,6 +23,7 @@
 #include <linux/uaccess.h>
 
 #include <asm/unaligned.h>
+#include <asm/system.h>
 
 #include "fault.h"
 
@@ -102,7 +103,7 @@ static int alignment_proc_show(struct seq_file *m, void *v)
 	seq_printf(m, "Skipped:\t%lu\n", ai_skipped);
 	seq_printf(m, "Half:\t\t%lu\n", ai_half);
 	seq_printf(m, "Word:\t\t%lu\n", ai_word);
-	if (cpu_architecture() >= CPU_ARCH_ARMv5TE)
+	if (cpu_architecture >= CPU_ARCH_ARMv5TE)
 		seq_printf(m, "DWord:\t\t%lu\n", ai_dword);
 	seq_printf(m, "Multi:\t\t%lu\n", ai_multi);
 	seq_printf(m, "User faults:\t%i (%s)\n", ai_usermode,
@@ -737,7 +738,7 @@ do_alignment(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
 	if (thumb_mode(regs)) {
 		fault = __get_user(tinstr, (u16 *)(instrptr & ~1));
 		if (!fault) {
-			if (cpu_architecture() >= CPU_ARCH_ARMv7 &&
+			if (cpu_architecture >= CPU_ARCH_ARMv7 &&
 			    IS_T32(tinstr)) {
 				/* Thumb-2 32-bit */
 				u16 tinst2 = 0;
@@ -935,7 +936,7 @@ static int __init alignment_init(void)
 	 * CPUs since we spin re-faulting the instruction without
 	 * making any progress.
 	 */
-	if (cpu_architecture() >= CPU_ARCH_ARMv6 && (cr_alignment & CR_U)) {
+	if (cpu_architecture >= CPU_ARCH_ARMv6 && (cr_alignment & CR_U)) {
 		cr_alignment &= ~CR_A;
 		cr_no_alignment &= ~CR_A;
 		set_cr(cr_alignment);
@@ -952,7 +953,7 @@ static int __init alignment_init(void)
 	 * TODO: handle ARMv6K properly. Runtime check for 'K' extension is
 	 * needed.
 	 */
-	if (cpu_architecture() <= CPU_ARCH_ARMv6) {
+	if (cpu_architecture <= CPU_ARCH_ARMv6) {
 		hook_fault_code(3, do_alignment, SIGBUS, BUS_ADRALN,
 				"alignment exception");
 	}
arch/arm/mm/fault.c +2 -2
index 3b5ea68..72ab66b 100644
@@ -642,12 +642,12 @@ do_PrefetchAbort(unsigned long addr, unsigned int ifsr, struct pt_regs *regs)
 
 static int __init exceptions_init(void)
 {
-	if (cpu_architecture() >= CPU_ARCH_ARMv6) {
+	if (cpu_architecture >= CPU_ARCH_ARMv6) {
 		hook_fault_code(4, do_translation_fault, SIGSEGV, SEGV_MAPERR,
 				"I-cache maintenance fault");
 	}
 
-	if (cpu_architecture() >= CPU_ARCH_ARMv7) {
+	if (cpu_architecture >= CPU_ARCH_ARMv7) {
 		/*
 		 * TODO: Access flag faults introduced in ARMv6K.
 		 * Runtime check for 'K' extension is needed
arch/arm/mm/idmap.c +2 -1
index 2be9139..78cfebf 100644
@@ -3,6 +3,7 @@
 #include <asm/cputype.h>
 #include <asm/pgalloc.h>
 #include <asm/pgtable.h>
+#include <asm/system.h>
 
 static void idmap_add_pmd(pud_t *pud, unsigned long addr, unsigned long end,
 	unsigned long prot)
@@ -33,7 +34,7 @@ void identity_mapping_add(pgd_t *pgd, unsigned long addr, unsigned long end)
 	unsigned long prot, next;
 
 	prot = PMD_TYPE_SECT | PMD_SECT_AP_WRITE;
-	if (cpu_architecture() <= CPU_ARCH_ARMv5TEJ && !cpu_is_xscale())
+	if (cpu_architecture <= CPU_ARCH_ARMv5TEJ && !cpu_is_xscale())
 		prot |= PMD_BIT4;
 
 	pgd += pgd_index(addr);
arch/arm/mm/ioremap.c +2 -1
index ab50627..1ee99a2 100644
@@ -32,6 +32,7 @@
 #include <asm/pgalloc.h>
 #include <asm/tlbflush.h>
 #include <asm/sizes.h>
+#include <asm/system.h>
 
 #include <asm/mach/map.h>
 #include "mm.h"
@@ -223,7 +224,7 @@ void __iomem * __arm_ioremap_pfn_caller(unsigned long pfn,
 
 #ifndef CONFIG_SMP
 	if (DOMAIN_IO == 0 &&
-	    (((cpu_architecture() >= CPU_ARCH_ARMv6) && (get_cr() & CR_XP)) ||
+	    (((cpu_architecture >= CPU_ARCH_ARMv6) && (get_cr() & CR_XP)) ||
 	       cpu_is_xsc3()) && pfn >= 0x100000 &&
 	       !((__pfn_to_phys(pfn) | size | addr) & ~SUPERSECTION_MASK)) {
 		area->flags |= VM_ARM_SECTION_MAPPING;
arch/arm/mm/mmu.c +17 -14
index 594d677..f969675 100644
@@ -25,6 +25,7 @@
 #include <asm/tlb.h>
 #include <asm/highmem.h>
 #include <asm/traps.h>
+#include <asm/system.h>
 
 #include <asm/mach/arch.h>
 #include <asm/mach/map.h>
@@ -122,7 +123,7 @@ static int __init early_cachepolicy(char *p)
 	 * change these attributes once the initial assembly has setup the
 	 * page tables.
 	 */
-	if (cpu_architecture() >= CPU_ARCH_ARMv6) {
+	if (cpu_architecture >= CPU_ARCH_ARMv6) {
 		printk(KERN_WARNING "Only cachepolicy=writeback supported on ARMv6 and later\n");
 		cachepolicy = CPOLICY_WRITEBACK;
 	}
@@ -289,10 +290,9 @@ static void __init build_mem_type_table(void)
 	struct cachepolicy *cp;
 	unsigned int cr = get_cr();
 	unsigned int user_pgprot, kern_pgprot, vecs_pgprot;
-	int cpu_arch = cpu_architecture();
 	int i;
 
-	if (cpu_arch < CPU_ARCH_ARMv6) {
+	if (cpu_architecture < CPU_ARCH_ARMv6) {
 #if defined(CONFIG_CPU_DCACHE_DISABLE)
 		if (cachepolicy > CPOLICY_BUFFERED)
 			cachepolicy = CPOLICY_BUFFERED;
@@ -301,7 +301,7 @@ static void __init build_mem_type_table(void)
 			cachepolicy = CPOLICY_WRITETHROUGH;
 #endif
 	}
-	if (cpu_arch < CPU_ARCH_ARMv5) {
+	if (cpu_architecture < CPU_ARCH_ARMv5) {
 		if (cachepolicy >= CPOLICY_WRITEALLOC)
 			cachepolicy = CPOLICY_WRITEBACK;
 		ecc_mask = 0;
@@ -314,10 +314,11 @@ static void __init build_mem_type_table(void)
 	 * Pre-ARMv5 CPUs don't have TEX bits.  Pre-ARMv6 CPUs or those
 	 * without extended page tables don't have the 'Shared' bit.
 	 */
-	if (cpu_arch < CPU_ARCH_ARMv5)
+	if (cpu_architecture < CPU_ARCH_ARMv5)
 		for (i = 0; i < ARRAY_SIZE(mem_types); i++)
 			mem_types[i].prot_sect &= ~PMD_SECT_TEX(7);
-	if ((cpu_arch < CPU_ARCH_ARMv6 || !(cr & CR_XP)) && !cpu_is_xsc3())
+	if ((cpu_architecture < CPU_ARCH_ARMv6 || !(cr & CR_XP)) &&
+	    !cpu_is_xsc3())
 		for (i = 0; i < ARRAY_SIZE(mem_types); i++)
 			mem_types[i].prot_sect &= ~PMD_SECT_S;
 
@@ -331,7 +332,7 @@ static void __init build_mem_type_table(void)
 			mem_types[i].prot_sect &= ~PMD_BIT4;
 			mem_types[i].prot_l1 &= ~PMD_BIT4;
 		}
-	} else if (cpu_arch < CPU_ARCH_ARMv6) {
+	} else if (cpu_architecture < CPU_ARCH_ARMv6) {
 		for (i = 0; i < ARRAY_SIZE(mem_types); i++) {
 			if (mem_types[i].prot_l1)
 				mem_types[i].prot_l1 |= PMD_BIT4;
@@ -343,7 +344,8 @@ static void __init build_mem_type_table(void)
 	/*
 	 * Mark the device areas according to the CPU/architecture.
 	 */
-	if (cpu_is_xsc3() || (cpu_arch >= CPU_ARCH_ARMv6 && (cr & CR_XP))) {
+	if (cpu_is_xsc3() ||
+	    (cpu_architecture >= CPU_ARCH_ARMv6 && (cr & CR_XP))) {
 		if (!cpu_is_xsc3()) {
 			/*
 			 * Mark device regions on ARMv6+ as execute-never
@@ -354,7 +356,7 @@ static void __init build_mem_type_table(void)
 			mem_types[MT_DEVICE_CACHED].prot_sect |= PMD_SECT_XN;
 			mem_types[MT_DEVICE_WC].prot_sect |= PMD_SECT_XN;
 		}
-		if (cpu_arch >= CPU_ARCH_ARMv7 && (cr & CR_TRE)) {
+		if (cpu_architecture >= CPU_ARCH_ARMv7 && (cr & CR_TRE)) {
 			/*
 			 * For ARMv7 with TEX remapping,
 			 * - shared device is SXCB=1100
@@ -404,7 +406,8 @@ static void __init build_mem_type_table(void)
 	/*
 	 * Only use write-through for non-SMP systems
 	 */
-	if (!is_smp() && cpu_arch >= CPU_ARCH_ARMv5 && cachepolicy > CPOLICY_WRITETHROUGH)
+	if (!is_smp() && cpu_architecture >= CPU_ARCH_ARMv5 &&
+	    cachepolicy > CPOLICY_WRITETHROUGH)
 		vecs_pgprot = cache_policies[CPOLICY_WRITETHROUGH].pte;
 
 	/*
@@ -420,7 +423,7 @@ static void __init build_mem_type_table(void)
 	/*
 	 * ARMv6 and above have extended page tables.
 	 */
-	if (cpu_arch >= CPU_ARCH_ARMv6 && (cr & CR_XP)) {
+	if (cpu_architecture >= CPU_ARCH_ARMv6 && (cr & CR_XP)) {
 		/*
 		 * Mark cache clean areas and XIP ROM read only
 		 * from SVC mode and no access from userspace.
@@ -452,8 +455,8 @@ static void __init build_mem_type_table(void)
 	 * Non-cacheable Normal - intended for memory areas that must
 	 * not cause dirty cache line writebacks when used
 	 */
-	if (cpu_arch >= CPU_ARCH_ARMv6) {
-		if (cpu_arch >= CPU_ARCH_ARMv7 && (cr & CR_TRE)) {
+	if (cpu_architecture >= CPU_ARCH_ARMv6) {
+		if (cpu_architecture >= CPU_ARCH_ARMv7 && (cr & CR_TRE)) {
 			/* Non-cacheable Normal is XCB = 001 */
 			mem_types[MT_MEMORY_NONCACHED].prot_sect |=
 				PMD_SECT_BUFFERED;
@@ -606,7 +609,7 @@ static void __init create_36bit_mapping(struct map_desc *md,
 	phys = __pfn_to_phys(md->pfn);
 	length = PAGE_ALIGN(md->length);
 
-	if (!(cpu_architecture() >= CPU_ARCH_ARMv6 || cpu_is_xsc3())) {
+	if (!(cpu_architecture >= CPU_ARCH_ARMv6 || cpu_is_xsc3())) {
 		printk(KERN_ERR "MM: CPU does not support supersection "
 		       "mapping for 0x%08llx at 0x%08lx\n",
 		       (long long)__pfn_to_phys((u64)md->pfn), addr);
arch/arm/vfp/vfpmodule.c +2 -2
index 79bcb43..128ce18 100644
@@ -20,6 +20,7 @@
 
 #include <asm/cputype.h>
 #include <asm/thread_notify.h>
+#include <asm/system.h>
 #include <asm/vfp.h>
 
 #include "vfpinstr.h"
@@ -541,9 +542,8 @@ static int vfp_hotplug(struct notifier_block *b, unsigned long action,
 static int __init vfp_init(void)
 {
 	unsigned int vfpsid;
-	unsigned int cpu_arch = cpu_architecture();
 
-	if (cpu_arch >= CPU_ARCH_ARMv6)
+	if (cpu_architecture >= CPU_ARCH_ARMv6)
 		vfp_enable(NULL);
 
 	/*
Signature · 1 line
1.7.4.1
#2 lore raw

RFC PATCH 1/5 ARM: Make cpu_alignment into a global variable

Shouldn't the title of this patch no have 'cpu_architecture' rather than 'cpu_alignment'?

arch/arm/kernel/setup.c struct cpu_cache_fns cpu_cache __read_mostly; · line 118

Quoted 5 lines · Dave Martin
arch/arm/kernel/setup.c +1 -0
@@ -114,6 +115,7 @@ struct cpu_cache_fns cpu_cache __read_mostly;
 struct outer_cache_fns outer_cache __read_mostly;
 EXPORT_SYMBOL(outer_cache);
 #endif
+int cpu_architecture __read_mostly;

I would have been tempted to initialise the variable to CPU_ARCH_UNKNOWN, though as this is defined as zero it's a bit moot.

Signature · 1 line
Tixy
#3 lore raw

RFC PATCH 2/5 ARM: s3c24xx: Reference cpu_architecture as a global variable

In reply to Dave Martin

This patch is needed for compatibility with the change of cpu_architecture from a function to a global variable.

Since cpu_architecture is ARM-specific, also add an explicit include for <asm/system.h> rather than relying on this being included as a side-effect.

---
 arch/arm/plat-s3c24xx/cpu.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)
arch/arm/plat-s3c24xx/cpu.c +2 -1
index c1fc6c6..bc118dc 100644
@@ -34,6 +34,7 @@
 #include <mach/hardware.h>
 #include <asm/irq.h>
 #include <asm/cacheflush.h>
+#include <asm/system.h>
 
 #include <asm/mach/arch.h>
 #include <asm/mach/map.h>
@@ -221,7 +222,7 @@ void __init s3c24xx_init_io(struct map_desc *mach_desc, int size)
 	iotable_init(mach_desc, size);
 	iotable_init(s3c_iodesc, ARRAY_SIZE(s3c_iodesc));
 
-	if (cpu_architecture() >= CPU_ARCH_ARMv5) {
+	if (cpu_architecture >= CPU_ARCH_ARMv5) {
 		idcode = s3c24xx_read_idcode_v5();
 	} else {
 		idcode = s3c24xx_read_idcode_v4();
Signature · 1 line
1.7.4.1
#4 lore raw

RFC PATCH 3/5 ARM: kprobes: Reference cpu_architecture as a global variable

In reply to Dave Martin

This patch is needed for compatibility with the change of cpu_architecture from a function to a global variable.

Since cpu_architecture is ARM-specific, also add an explicit include for <asm/system.h> rather than relying on this being included as a side-effect.

---
 arch/arm/kernel/kprobes-common.c |   11 +++++------
 1 files changed, 5 insertions(+), 6 deletions(-)
arch/arm/kernel/kprobes-common.c +5 -6
index a5394fb4..bbf248c 100644
@@ -13,6 +13,7 @@
 
 #include <linux/kernel.h>
 #include <linux/kprobes.h>
+#include <asm/system.h>
 
 #include "kprobes.h"
 
@@ -51,9 +52,8 @@ bool load_write_pc_interworks;
 
 void __init test_load_write_pc_interworking(void)
 {
-	int arch = cpu_architecture();
-	BUG_ON(arch == CPU_ARCH_UNKNOWN);
-	load_write_pc_interworks = arch >= CPU_ARCH_ARMv5T;
+	BUG_ON(cpu_architecture == CPU_ARCH_UNKNOWN);
+	load_write_pc_interworks = cpu_architecture >= CPU_ARCH_ARMv5T;
 }
 
 #endif /* !test_load_write_pc_interworking */
@@ -65,9 +65,8 @@ bool alu_write_pc_interworks;
 
 void __init test_alu_write_pc_interworking(void)
 {
-	int arch = cpu_architecture();
-	BUG_ON(arch == CPU_ARCH_UNKNOWN);
-	alu_write_pc_interworks = arch >= CPU_ARCH_ARMv7;
+	BUG_ON(cpu_architecture == CPU_ARCH_UNKNOWN);
+	alu_write_pc_interworks = cpu_architecture >= CPU_ARCH_ARMv7;
 }
 
 #endif /* !test_alu_write_pc_interworking */
Signature · 1 line
1.7.4.1
#5 lore raw

RFC PATCH 3/5 ARM: kprobes: Reference cpu_architecture as a global variable

Quoted 11 lines · Dave Martin

This patch is needed for compatibility with the change of cpu_architecture from a function to a global variable.

Since cpu_architecture is ARM-specific, also add an explicit include for <asm/system.h> rather than relying on this being included as a side-effect.

---
 arch/arm/kernel/kprobes-common.c |   11 +++++------
 1 files changed, 5 insertions(+), 6 deletions(-)

Is there any reason that this patch isn't part of the other ARM platform changes in patch one?

Signature · 1 line
Tixy
#6 lore raw

RFC PATCH 4/5 ARM: entry: Remove unnecessary masking when decoding Thumb-2 instructions

In reply to Dave Martin

When testing whether a Thumb-2 instruction is 32 bits long or not, the masking done in order to test bits 11-15 of the first instruction halfword won't affect the result of the comparison, so remove it.

---
 arch/arm/kernel/entry-armv.S |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)
arch/arm/kernel/entry-armv.S +2 -4
index a87cbf8..b7236d4 100644
@@ -262,8 +262,7 @@ __und_svc:
 	ldr	r0, [r4, #-4]
 #else
 	ldrh	r0, [r4, #-2]			@ Thumb instruction at LR - 2
-	and	r9, r0, #0xf800
-	cmp	r9, #0xe800			@ 32-bit instruction if xx >= 0
+	cmp	r0, #0xe800			@ 32-bit instruction if xx >= 0
 	ldrhhs	r9, [r4]			@ bottom 16 bits
 	orrhs	r0, r9, r0, lsl #16
 #endif
@@ -445,8 +444,7 @@ __und_usr:
  ARM(	ldrht	r5, [r4], #2	)
  THUMB(	ldrht	r5, [r4]	)
  THUMB(	add	r4, r4, #2	)
-	and	r0, r5, #0xf800			@ mask bits 111x x... .... ....
-	cmp	r0, #0xe800			@ 32bit instruction if xx != 0
+	cmp	r5, #0xe800			@ 32bit instruction if xx != 0
 	blo	__und_usr_unknown
 3:	ldrht	r0, [r4]
 	add	r2, r2, #2			@ r2 is PC + 2, make it PC + 4
Signature · 1 line
1.7.4.1
#7 lore raw

RFC PATCH 5/5 ARM: entry: Fix Thumb-2 undef handling for multi-CPU kernels

In reply to Dave Martin

When v6 and >=v7 boards are supported in the same kernel, the __und_usr code currently makes a build-time assumption that Thumb-2 instructions occurring in userspace don't need to be supported. Strictly speaking this is incorrect.

This patch fixes the above case by doing a run-time check on the CPU architecture in these cases. This only affects kernels which support v6 and >=v7 CPUs together: plain v6 and plain v7 kernels are unaffected.

---
 arch/arm/kernel/entry-armv.S |   38 +++++++++++++++++++++++++++++++++++++-
 1 files changed, 37 insertions(+), 1 deletions(-)
arch/arm/kernel/entry-armv.S +37 -1
index b7236d4..bc41ebd 100644
@@ -24,6 +24,7 @@
 #include <asm/unwind.h>
 #include <asm/unistd.h>
 #include <asm/tls.h>
+#include <asm/system.h>
 
 #include "entry-header.S"
 #include <asm/entry-macro-multi.S>
@@ -439,7 +440,27 @@ __und_usr:
 #endif
 	beq	call_fpe
 	@ Thumb instruction
-#if __LINUX_ARM_ARCH__ >= 7
+#if CONFIG_ARM_THUMB && __LINUX_ARM_ARCH__ >= 6 && CONFIG_CPU_V7
+/*
+ * Thumb-2 instruction handling.  Note that because pre-v6 and >= v6 platforms
+ * can never be supported in a single kernel, this code is not applicable at
+ * all when __LINUX_ARM_ARCH__ < 6.  This allows simplifying assumptions to be
+ * made about .arch directives.
+ */
+#if __LINUX_ARM_ARCH__ < 7
+/* If the target CPU may not be Thumb-2-capable, a run-time check is needed: */
+#define NEED_CPU_ARCHITECTURE
+	ldr	r5, .LCcpu_architecture
+	ldr	r5, [r5]
+	cmp	r5, #CPU_ARCH_ARMv7
+	blo	__und_usr_unknown
+/*
+ * The following code won't get run unless the running CPU really is v7, so
+ * coding round the lack of ldrht on older arches is pointless.  Temporarily
+ * override the assembler target arch with the minimum required instead:
+ */
+	.arch	armv6t2
+#endif
 2:
  ARM(	ldrht	r5, [r4], #2	)
  THUMB(	ldrht	r5, [r4]	)
@@ -449,7 +470,16 @@ __und_usr:
 3:	ldrht	r0, [r4]
 	add	r2, r2, #2			@ r2 is PC + 2, make it PC + 4
 	orr	r0, r0, r5, lsl #16
+
+#if __LINUX_ARM_ARCH__ < 7
+/* If the target arch was overridden, change it back: */
+#ifdef CONFIG_CPU_32v6K
+	.arch	armv6k
 #else
+	.arch	armv6
+#endif
+#endif /* __LINUX_ARM_ARCH__ < 7 */
+#else /* !(CONFIG_ARM_THUMB && __LINUX_ARM_ARCH__ >= 6 && CONFIG_CPU_V7) */
 	b	__und_usr_unknown
 #endif
  UNWIND(.fnend		)
@@ -576,6 +606,12 @@ call_fpe:
 	movw_pc	lr				@ CP#14 (Debug)
 	movw_pc	lr				@ CP#15 (Control)
 
+#ifdef NEED_CPU_ARCHITECTURE
+	.align	2
+.LCcpu_architecture:
+	.word	cpu_architecture
+#endif
+
 #ifdef CONFIG_NEON
 	.align	6
 
Signature · 1 line
1.7.4.1
#8 lore raw

RFC PATCH 5/5 ARM: entry: Fix Thumb-2 undef handling for multi-CPU kernels

Quoted 19 lines · Dave Martin
arch/arm/kernel/entry-armv.S +14 -0
+#if __LINUX_ARM_ARCH__ < 7
+/* If the target CPU may not be Thumb-2-capable, a run-time check is needed: */
+#define NEED_CPU_ARCHITECTURE
+	ldr	r5, .LCcpu_architecture
+	ldr	r5, [r5]
+	cmp	r5, #CPU_ARCH_ARMv7
+	blo	__und_usr_unknown
+/*
+ * The following code won't get run unless the running CPU really is v7, so
+ * coding round the lack of ldrht on older arches is pointless.  Temporarily
+ * override the assembler target arch with the minimum required instead:
+ */
+	.arch	armv6t2
+#endif
 2:
  ARM(	ldrht	r5, [r4], #2	)
  THUMB(	ldrht	r5, [r4]	)
@@ -449,7 +470,16 @@ __und_usr:
 3:	ldrht	r0, [r4]

arch/arm/kernel/entry-armv.S __und_usr: · line 480

Quoted 10 lines
arch/arm/kernel/entry-armv.S +7 -0
 	add	r2, r2, #2			@ r2 is PC + 2, make it PC + 4
 	orr	r0, r0, r5, lsl #16
+
+#if __LINUX_ARM_ARCH__ < 7
+/* If the target arch was overridden, change it back: */
+#ifdef CONFIG_CPU_32v6K
+	.arch	armv6k
 #else
+	.arch	armv6
+#endif

Are we confident that there will be no other v6 architectures supported in future? Looks like a bit of a hazard otherwise.

Looking at the assembler manual, MIPS and PowerPC have push/pop operations for changing things like arch settings, pity ARM doesn't.

Signature · 1 line
Tixy
#9 lore raw

RFC PATCH 5/5 ARM: entry: Fix Thumb-2 undef handling for multi-CPU kernels

Quoted 25 lines · Tixy
Quoted 24 lines · Dave Martin
arch/arm/kernel/entry-armv.S +17 -0
+#if __LINUX_ARM_ARCH__ < 7
+/* If the target CPU may not be Thumb-2-capable, a run-time check is needed: */
+#define NEED_CPU_ARCHITECTURE
+	ldr	r5, .LCcpu_architecture
+	ldr	r5, [r5]
+	cmp	r5, #CPU_ARCH_ARMv7
+	blo	__und_usr_unknown
+/*
+ * The following code won't get run unless the running CPU really is v7, so
+ * coding round the lack of ldrht on older arches is pointless.  Temporarily
+ * override the assembler target arch with the minimum required instead:
+ */
+	.arch	armv6t2
+#endif
 2:
  ARM(	ldrht	r5, [r4], #2	)
  THUMB(	ldrht	r5, [r4]	)
@@ -449,7 +470,16 @@ __und_usr:
 3:	ldrht	r0, [r4]
 	add	r2, r2, #2			@ r2 is PC + 2, make it PC + 4
 	orr	r0, r0, r5, lsl #16
+
+#if __LINUX_ARM_ARCH__ < 7
+/* If the target arch was overridden, change it back: */
Quoted 8 lines

arch/arm/kernel/entry-armv.S __und_usr: · line 480

Quoted 5 lines
arch/arm/kernel/entry-armv.S +4 -0
+#ifdef CONFIG_CPU_32v6K
+	.arch	armv6k
 #else
+	.arch	armv6
+#endif

Are we confident that there will be no other v6 architectures supported in future? Looks like a bit of a hazard otherwise.

More or less. armv6Z is theoretically possible, but highly unlikely to affect this file. armv6T2 is only rarely deployed and has not gone upstream. Neither __LINUX_ARM_ARCH__ nor cpu_architecture has a value corresponding to this architecture. Without CONFIG_THUMB2_KERNEL, I think assembling entry-armv.S will just work as-is; with CONFIG_THUMB2_KERNEL there would be build failures, giving someone an opportunity to fix it. Since v6T2 would only happen in some specialised niches anyway, it's less likely that someone would be building this into a combined kernel, even if it were supported.

An alternative route is to change arch/arm/Makefile to pass a command-line #define for the "real" architecture so we can restore to it. My attempts to do that seemed at least as fragile, though...

Quoted 3 lines

Looking at the assembler manual, MIPS and PowerPC have push/pop operations for changing things like arch settings, pity ARM doesn't.

There have been discussions with people (i.e., me) advocating a similar option for the assembler on ARM. I wasn't aware that it actually existed for some arches... interesting. My searches didn't find these.

Unfortunately, those save/restore features seem have been done in a somewhat gross, target-specific way (maybe inherited from those targets' native assemblers) where it could profitably have been made generic... Either way, even if it gets implemented it will be some time before everyone using gas has that feature, so we can't really rely on it in ARM-land.

Cheers
---Dave

#10 lore raw

RFC PATCH 5/5 ARM: entry: Fix Thumb-2 undef handling for multi-CPU kernels

In reply to Dave Martin
Quoted 9 lines · Dave Martin

When v6 and >=v7 boards are supported in the same kernel, the __und_usr code currently makes a build-time assumption that Thumb-2 instructions occurring in userspace don't need to be supported. Strictly speaking this is incorrect.

This patch fixes the above case by doing a run-time check on the CPU architecture in these cases. This only affects kernels which support v6 and >=v7 CPUs together: plain v6 and plain v7 kernels are unaffected.

I think this patch broke random configurations (!THUMB2 && v7) for me:

Quoted 18 lines
(quoted diff) +13 -1
@@ -439,7 +440,27 @@ __und_usr:
 #endif
 	beq	call_fpe
 	@ Thumb instruction
-#if __LINUX_ARM_ARCH__ >= 7
+#if CONFIG_ARM_THUMB && __LINUX_ARM_ARCH__ >= 6 && CONFIG_CPU_V7
+/*
+ * Thumb-2 instruction handling.  Note that because pre-v6 and >= v6 platforms
+ * can never be supported in a single kernel, this code is not applicable at
+ * all when __LINUX_ARM_ARCH__ < 6.  This allows simplifying assumptions to be
+ * made about .arch directives.
+ */
+#if __LINUX_ARM_ARCH__ < 7
+/* If the target CPU may not be Thumb-2-capable, a run-time check is needed: */
+#define NEED_CPU_ARCHITECTURE
+	ldr	r5, .LCcpu_architecture
+	ldr	r5, [r5]
+	cmp	r5, #CPU_ARCH_ARMv7

Review __und_usr: · line 465

Quoted 10 lines
(quoted diff) +8 -0
+	blo	__und_usr_unknown
+/*
+ * The following code won't get run unless the running CPU really is v7, so
+ * coding round the lack of ldrht on older arches is pointless.  Temporarily
+ * override the assembler target arch with the minimum required instead:
+ */
+	.arch	armv6t2
+#endif
 2:
  ARM(	ldrht	r5, [r4], #2	)

This fixes it, please fold into the patch or apply on top.

Arnd

8<---
ARM: entry: refix Fix Thumb-2 undef handling for multi-CPU kernels

In some configurations labels become obsolete, so don't generate fixups for them.

arch/arm/kernel/entry-armv.S +1 -1
index 9ad50c4..b145f16 100644
@@ -497,7 +497,7 @@ ENDPROC(__und_usr)
 	.popsection
 	.pushsection __ex_table,"a"
 	.long	1b, 4b
-#if __LINUX_ARM_ARCH__ >= 7
+#if CONFIG_ARM_THUMB && __LINUX_ARM_ARCH__ >= 6 && CONFIG_CPU_V7
 	.long	2b, 4b
 	.long	3b, 4b
 #endif
#11 lore raw

RFC PATCH 5/5 ARM: entry: Fix Thumb-2 undef handling for multi-CPU kernels

Quoted 59 lines · Arnd Bergmann
Quoted 9 lines · Dave Martin

When v6 and >=v7 boards are supported in the same kernel, the __und_usr code currently makes a build-time assumption that Thumb-2 instructions occurring in userspace don't need to be supported. Strictly speaking this is incorrect.

This patch fixes the above case by doing a run-time check on the CPU architecture in these cases. This only affects kernels which support v6 and >=v7 CPUs together: plain v6 and plain v7 kernels are unaffected.

I think this patch broke random configurations (!THUMB2 && v7) for me:

Quoted 18 lines
(quoted diff) +13 -1
@@ -439,7 +440,27 @@ __und_usr:
 #endif
 	beq	call_fpe
 	@ Thumb instruction
-#if __LINUX_ARM_ARCH__ >= 7
+#if CONFIG_ARM_THUMB && __LINUX_ARM_ARCH__ >= 6 && CONFIG_CPU_V7
+/*
+ * Thumb-2 instruction handling.  Note that because pre-v6 and >= v6 platforms
+ * can never be supported in a single kernel, this code is not applicable at
+ * all when __LINUX_ARM_ARCH__ < 6.  This allows simplifying assumptions to be
+ * made about .arch directives.
+ */
+#if __LINUX_ARM_ARCH__ < 7
+/* If the target CPU may not be Thumb-2-capable, a run-time check is needed: */
+#define NEED_CPU_ARCHITECTURE
+	ldr	r5, .LCcpu_architecture
+	ldr	r5, [r5]
+	cmp	r5, #CPU_ARCH_ARMv7

Review __und_usr: · line 465

Quoted 10 lines
(quoted diff) +8 -0
+	blo	__und_usr_unknown
+/*
+ * The following code won't get run unless the running CPU really is v7, so
+ * coding round the lack of ldrht on older arches is pointless.  Temporarily
+ * override the assembler target arch with the minimum required instead:
+ */
+	.arch	armv6t2
+#endif
 2:
  ARM(	ldrht	r5, [r4], #2	)

This fixes it, please fold into the patch or apply on top.

Arnd

8<---
ARM: entry: refix Fix Thumb-2 undef handling for multi-CPU kernels

In some configurations labels become obsolete, so don't generate fixups for them.

arch/arm/kernel/entry-armv.S +0 -0
index 9ad50c4..b145f16 100644

arch/arm/kernel/entry-armv.S ENDPROC(__und_usr) · line 503

Quoted 8 lines
arch/arm/kernel/entry-armv.S +1 -1
@@ -497,7 +497,7 @@ ENDPROC(__und_usr)
 	.popsection
 	.pushsection __ex_table,"a"
 	.long	1b, 4b
-#if __LINUX_ARM_ARCH__ >= 7
+#if CONFIG_ARM_THUMB && __LINUX_ARM_ARCH__ >= 6 && CONFIG_CPU_V7
 	.long	2b, 4b
 	.long	3b, 4b
 #endif

This patch looks correct.

We could tidy things up so this is easier to maintain, by avoiding the fixups being declared in a separate place from the labels they reference, but I'm not sure if it's worth it just for this case.

Russell, would you prefer to fold this in with the previous patch, or apply it on top?

Cheers
---Dave

#12 lore raw

RFC PATCH 5/5 ARM: entry: Fix Thumb-2 undef handling for multi-CPU kernels

In reply to Arnd Bergmann
Quoted 59 lines · Arnd Bergmann
Quoted 9 lines · Dave Martin

When v6 and >=v7 boards are supported in the same kernel, the __und_usr code currently makes a build-time assumption that Thumb-2 instructions occurring in userspace don't need to be supported. Strictly speaking this is incorrect.

This patch fixes the above case by doing a run-time check on the CPU architecture in these cases. This only affects kernels which support v6 and >=v7 CPUs together: plain v6 and plain v7 kernels are unaffected.

I think this patch broke random configurations (!THUMB2 && v7) for me:

Quoted 18 lines
(quoted diff) +13 -1
@@ -439,7 +440,27 @@ __und_usr:
 #endif
 	beq	call_fpe
 	@ Thumb instruction
-#if __LINUX_ARM_ARCH__ >= 7
+#if CONFIG_ARM_THUMB && __LINUX_ARM_ARCH__ >= 6 && CONFIG_CPU_V7
+/*
+ * Thumb-2 instruction handling.  Note that because pre-v6 and >= v6 platforms
+ * can never be supported in a single kernel, this code is not applicable at
+ * all when __LINUX_ARM_ARCH__ < 6.  This allows simplifying assumptions to be
+ * made about .arch directives.
+ */
+#if __LINUX_ARM_ARCH__ < 7
+/* If the target CPU may not be Thumb-2-capable, a run-time check is needed: */
+#define NEED_CPU_ARCHITECTURE
+	ldr	r5, .LCcpu_architecture
+	ldr	r5, [r5]
+	cmp	r5, #CPU_ARCH_ARMv7

Review __und_usr: · line 465

Quoted 10 lines
(quoted diff) +8 -0
+	blo	__und_usr_unknown
+/*
+ * The following code won't get run unless the running CPU really is v7, so
+ * coding round the lack of ldrht on older arches is pointless.  Temporarily
+ * override the assembler target arch with the minimum required instead:
+ */
+	.arch	armv6t2
+#endif
 2:
  ARM(	ldrht	r5, [r4], #2	)

This fixes it, please fold into the patch or apply on top.

Arnd

8<---
ARM: entry: refix Fix Thumb-2 undef handling for multi-CPU kernels

In some configurations labels become obsolete, so don't generate fixups for them.

arch/arm/kernel/entry-armv.S +0 -0
index 9ad50c4..b145f16 100644

arch/arm/kernel/entry-armv.S ENDPROC(__und_usr) · line 503

Quoted 8 lines
arch/arm/kernel/entry-armv.S +1 -1
@@ -497,7 +497,7 @@ ENDPROC(__und_usr)
 	.popsection
 	.pushsection __ex_table,"a"
 	.long	1b, 4b
-#if __LINUX_ARM_ARCH__ >= 7
+#if CONFIG_ARM_THUMB && __LINUX_ARM_ARCH__ >= 6 && CONFIG_CPU_V7
 	.long	2b, 4b
 	.long	3b, 4b
 #endif

Arnd, do you want to go ahead and send this patch to Russell's patch system?

Feel free to add my Reviewed-by.

Cheers
---Dave

#13 lore raw

cover Fix Thumb-2 undef handling for mixed-arch kernels

In reply to Dave Martin
Quoted 3 lines · Dave Martin

As a side-effect, this also changes cpu_architecture from a function into a global variable initialised at boot-time, which is probably a sensible idea anyway.

One possible pitfall of this is if the variable didn't get set up before it's first use. I have looked at all the uses and this doesn't seem to be a problem though.

An alternative to defend against this is to make cpu_architecture() an inline function returning the value of the global variable like:

inline int cpu_architecture(void)

{
	BUG_ON(the_cpu_architecture == CPU_ARCH_UNKNOWN);
	return the_cpu_architecture;
}

This has the bonus of not needing to change users of the the function.

Signature · 1 line
Tixy
#14 lore raw

cover Fix Thumb-2 undef handling for mixed-arch kernels

Quoted 8 lines · Tixy
Quoted 3 lines · Dave Martin

As a side-effect, this also changes cpu_architecture from a function into a global variable initialised at boot-time, which is probably a sensible idea anyway.

One possible pitfall of this is if the variable didn't get set up before it's first use. I have looked at all the uses and this doesn't seem to be a problem though.

I thought of that. I believe the code is safe as-is, but it's not too maintenance-friendly, so...

Quoted 11 lines

An alternative to defend against this is to make cpu_architecture() an inline function returning the value of the global variable like:

inline int cpu_architecture(void)

{
	BUG_ON(the_cpu_architecture == CPU_ARCH_UNKNOWN);
	return the_cpu_architecture;
}

This has the bonus of not needing to change users of the the function.

Sounds like a good idea. I got rid of the function because I didn't like calling a function from the undef handler entry code, but an inline function which just reads the variable seems like the best of both worlds. I would continue to read the variable directly from the undef handler, but this will not get called until userspace starts -- by which time the inline C function will have been called a few times from C code.

If cpu_architecture is unexpectedly read as zero from the undef handler, we will get some unexpected SIGILLs in userspace -- so this will get noticed (rather than causing silent errors). However, I think the kernel should always hit BUG() before that point is reached, if using your suggested inline function.

As you say, it takes away a lot of the patch churn too.

I'll follow up with a revision based on this suggestion.

This also gets rid of the kprobes- and s3c24xx-specific patches.

Cheers
---Dave

#15 lore raw

cover Fix Thumb-2 undef handling for mixed-arch kernels

Quoted 10 lines · Dave Martin
Quoted 9 lines · Tixy

An alternative to defend against this is to make cpu_architecture() an inline function returning the value of the global variable like:

inline int cpu_architecture(void)

{
	BUG_ON(the_cpu_architecture == CPU_ARCH_UNKNOWN);
	return the_cpu_architecture;
}
Quoted 5 lines
Quoted 1 line

This has the bonus of not needing to change users of the the function.

Sounds like a good idea. I got rid of the function because I didn't like calling a function from the undef handler entry code, but an inline function which just reads the variable seems like the best of both worlds.

While at it, you could mark the function with __attribute__((pure)).

Nicolas

#16 lore raw

cover Fix Thumb-2 undef handling for mixed-arch kernels

Quoted 12 lines · Nicolas Pitre
Quoted 10 lines · Dave Martin
Quoted 9 lines · Tixy

An alternative to defend against this is to make cpu_architecture() an inline function returning the value of the global variable like:

inline int cpu_architecture(void)

{
	BUG_ON(the_cpu_architecture == CPU_ARCH_UNKNOWN);
	return the_cpu_architecture;
}
Quoted 7 lines
Quoted 5 lines
Quoted 1 line

This has the bonus of not needing to change users of the the function.

Sounds like a good idea. I got rid of the function because I didn't like calling a function from the undef handler entry code, but an inline function which just reads the variable seems like the best of both worlds.

While at it, you could mark the function with __attribute__((pure)).

Hmmm, I don't know what exact impact that has for an inline function, but it would certainly do no harm. Maybe it will eliminate some calls to BUG_ON(). I will add it anyway -- I don't see a reason not to.

Cheers
---Dave