* [PATCH 0/2] add interworking support to Thumb2 kernel @ 2016-08-20 7:45 Ard Biesheuvel 2016-08-20 7:45 ` [PATCH 1/2] ARM: Thumb-2: infer function annotation for external ksyms Ard Biesheuvel ` (2 more replies) 0 siblings, 3 replies; 8+ messages in thread From: Ard Biesheuvel @ 2016-08-20 7:45 UTC (permalink / raw) To: linux-arm-kernel In the core kernel, interworking is supported since vmlinux is a static binary, and the linker takes care of fixing up the bl instructions and emitting veneers if necessary. For modules, however, all function objects must currently be in the same mode, since the kernel's module loader does not handle interworking switches at all. However, now that we have optimized PLT support, we can simply enable it unconditionally for Thumb2 builds, which by itself is reasonable since the module area is pretty small, especially with multiplatform kernels (current multi_v7_defconfig results in a .text section that exceeds 8 MB, leaving less than 8 MB for modules when not using PLTs). PLT support also allows us to reuse the veneer emitting routines to emit veneers for interworking branches that cannot be fixed up by tweaking the instruction encoding. Patch #1 fixes an issue where ARM functions exported from other modules will be mistaken for Thumb2 functions due to the missing STT_FUNC annotation on SHN_UNDEF symbols. Patch #2 implements the interworking support, by tweaking the instruction encoding for unconditional bl instructions, and emitting a PLT veneer in all other cases. Ard Biesheuvel (2): ARM: Thumb-2: infer function annotation for external ksyms ARM: add interworking support to Thumb-2 kernel arch/arm/Kconfig | 1 + arch/arm/include/asm/module.h | 3 +- arch/arm/kernel/module-plts.c | 21 +++++-- arch/arm/kernel/module.c | 64 ++++++++++++++++---- 4 files changed, 69 insertions(+), 20 deletions(-) -- 2.7.4 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/2] ARM: Thumb-2: infer function annotation for external ksyms 2016-08-20 7:45 [PATCH 0/2] add interworking support to Thumb2 kernel Ard Biesheuvel @ 2016-08-20 7:45 ` Ard Biesheuvel 2016-08-20 7:45 ` [PATCH 2/2] ARM: add interworking support to Thumb-2 kernel Ard Biesheuvel 2016-08-20 16:23 ` [PATCH 0/2] add interworking support to Thumb2 kernel Russell King - ARM Linux 2 siblings, 0 replies; 8+ messages in thread From: Ard Biesheuvel @ 2016-08-20 7:45 UTC (permalink / raw) To: linux-arm-kernel Commit 9a00318eadbb ("Thumb-2: Relax relocation requirements for non-function symbols") updated the Thumb-2 jump and call relocation handling so that non-function symbols with the Thumb bit (bit 0) cleared are not treated as A32 symbols requiring an interworking mode switch. However, since ksyms are stripped of their function annotation by EXPORT_SYMBOL(), A32 symbols that do require an interworking mode switch will be called in the wrong mode if the relocation is resolved across a module boundary. This patch enhances the function symbol check by including untyped symbols that resolve to external ksyms. Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> --- arch/arm/kernel/module.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/arch/arm/kernel/module.c b/arch/arm/kernel/module.c index 4f14b5ce6535..6c22b13cbd12 100644 --- a/arch/arm/kernel/module.c +++ b/arch/arm/kernel/module.c @@ -182,13 +182,20 @@ apply_relocate(Elf32_Shdr *sechdrs, const char *strtab, unsigned int symindex, /* * For function symbols, only Thumb addresses are * allowed (no interworking). + * This applies equally to untyped symbols that + * resolve to external ksyms: EXPORT_SYMBOL() + * strips the function annotation, but we can + * infer from the relocation type that the target + * must be a function. * * For non-function symbols, the destination * has no specific ARM/Thumb disposition, so * the branch is resolved under the assumption * that interworking is not required. */ - if (ELF32_ST_TYPE(sym->st_info) == STT_FUNC && + if ((ELF32_ST_TYPE(sym->st_info) == STT_FUNC || + (ELF32_ST_TYPE(sym->st_info) == STT_NOTYPE && + sym->st_shndx == SHN_UNDEF)) && !(sym->st_value & 1)) { pr_err("%s: section %u reloc %u sym '%s': unsupported interworking call (Thumb -> ARM)\n", module->name, relindex, i, symname); -- 2.7.4 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/2] ARM: add interworking support to Thumb-2 kernel 2016-08-20 7:45 [PATCH 0/2] add interworking support to Thumb2 kernel Ard Biesheuvel 2016-08-20 7:45 ` [PATCH 1/2] ARM: Thumb-2: infer function annotation for external ksyms Ard Biesheuvel @ 2016-08-20 7:45 ` Ard Biesheuvel 2016-08-20 16:23 ` [PATCH 0/2] add interworking support to Thumb2 kernel Russell King - ARM Linux 2 siblings, 0 replies; 8+ messages in thread From: Ard Biesheuvel @ 2016-08-20 7:45 UTC (permalink / raw) To: linux-arm-kernel ARM/Thumb interworking is currently not allowed in modules at all: external module dependencies can only be fulfilled by symbols of the same flavour, but even inside a module, jump and call relocations between objects are rejected if they would incur a mode switch. This patch relaxes that restriction, by allowing function calls ('bl') from T32 into A32 code, and vice versa, by fixing up the bl instruction to a blx instruction, with the appropriate rounding applied to the offset. For jump ('b') instructions, this is not possible (the ISA does not provide jump instructions that switch mode) and so a PLT entry emitted instead, which is inherently interworking-aware since it uses a 'ldr pc, =xxx' instruction. Since this requires module PLT support, add the 'select ARM_MODULE_PLTS ' to config THUMB2_KERNEL. Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> --- arch/arm/Kconfig | 1 + arch/arm/include/asm/module.h | 3 +- arch/arm/kernel/module-plts.c | 21 +++++--- arch/arm/kernel/module.c | 55 +++++++++++++++----- 4 files changed, 61 insertions(+), 19 deletions(-) diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index a9c4e48bb7ec..c786337f3c8b 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -1544,6 +1544,7 @@ config THUMB2_KERNEL select AEABI select ARM_ASM_UNIFIED select ARM_UNWIND + select ARM_MODULE_PLTS help By enabling this option, the kernel will be compiled in Thumb-2 mode. A compiler/assembler that understand the unified diff --git a/arch/arm/include/asm/module.h b/arch/arm/include/asm/module.h index 464748b9fd7d..82ba2a82f9a3 100644 --- a/arch/arm/include/asm/module.h +++ b/arch/arm/include/asm/module.h @@ -28,7 +28,8 @@ struct mod_arch_specific { #endif }; -u32 get_module_plt(struct module *mod, unsigned long loc, Elf32_Addr val); +u32 get_module_plt(struct module *mod, unsigned long loc, Elf32_Addr val, + bool from_thumb); /* * Add the ARM architecture version to the version magic string diff --git a/arch/arm/kernel/module-plts.c b/arch/arm/kernel/module-plts.c index a910f1db0c14..c239bc92f81b 100644 --- a/arch/arm/kernel/module-plts.c +++ b/arch/arm/kernel/module-plts.c @@ -18,12 +18,15 @@ #define PLT_ENT_COUNT (PLT_ENT_STRIDE / sizeof(u32)) #define PLT_ENT_SIZE (sizeof(struct plt_entries) / PLT_ENT_COUNT) -#ifdef CONFIG_THUMB2_KERNEL -#define PLT_ENT_LDR __opcode_to_mem_thumb32(0xf8dff000 | \ +#define PLT_ENT_LDR_ARM __opcode_to_mem_arm(0xe59ff000 | \ + (PLT_ENT_STRIDE - 8)) +#define PLT_ENT_LDR_THUMB __opcode_to_mem_thumb32(0xf8dff000 | \ (PLT_ENT_STRIDE - 4)) + +#ifdef CONFIG_THUMB2_KERNEL +#define PLT_ENT_LDR PLT_ENT_LDR_THUMB #else -#define PLT_ENT_LDR __opcode_to_mem_arm(0xe59ff000 | \ - (PLT_ENT_STRIDE - 8)) +#define PLT_ENT_LDR PLT_ENT_LDR_ARM #endif struct plt_entries { @@ -31,7 +34,8 @@ struct plt_entries { u32 lit[PLT_ENT_COUNT]; }; -u32 get_module_plt(struct module *mod, unsigned long loc, Elf32_Addr val) +u32 get_module_plt(struct module *mod, unsigned long loc, Elf32_Addr val, + bool from_thumb) { struct plt_entries *plt = (struct plt_entries *)mod->arch.plt->sh_addr; int idx = 0; @@ -45,7 +49,10 @@ u32 get_module_plt(struct module *mod, unsigned long loc, Elf32_Addr val) plt += (mod->arch.plt_count - 1) / PLT_ENT_COUNT; idx = (mod->arch.plt_count - 1) % PLT_ENT_COUNT; - if (plt->lit[idx] == val) + if (plt->lit[idx] == val && + (!IS_ENABLED(CONFIG_THUMB2_KERNEL) || + plt->ldr[idx] == from_thumb ? PLT_ENT_LDR_THUMB : + PLT_ENT_LDR_ARM)) return (u32)&plt->ldr[idx]; idx = (idx + 1) % PLT_ENT_COUNT; @@ -65,6 +72,8 @@ u32 get_module_plt(struct module *mod, unsigned long loc, Elf32_Addr val) else plt->lit[idx] = val; + if (IS_ENABLED(CONFIG_THUMB2_KERNEL) && !from_thumb) + plt->ldr[idx] = PLT_ENT_LDR_ARM; return (u32)&plt->ldr[idx]; } diff --git a/arch/arm/kernel/module.c b/arch/arm/kernel/module.c index 6c22b13cbd12..f6c4059d0006 100644 --- a/arch/arm/kernel/module.c +++ b/arch/arm/kernel/module.c @@ -103,7 +103,8 @@ apply_relocate(Elf32_Shdr *sechdrs, const char *strtab, unsigned int symindex, case R_ARM_PC24: case R_ARM_CALL: case R_ARM_JUMP24: - if (sym->st_value & 3) { + if (!IS_ENABLED(CONFIG_THUMB2_KERNEL) && + sym->st_value & 3) { pr_err("%s: section %u reloc %u sym '%s': unsupported interworking call (ARM -> Thumb)\n", module->name, relindex, i, symname); return -ENOEXEC; @@ -121,12 +122,17 @@ apply_relocate(Elf32_Shdr *sechdrs, const char *strtab, unsigned int symindex, * supported range. Note that 'offset + loc + 8' * contains the absolute jump target, i.e., * @sym + addend, corrected for the +8 PC bias. + * Also emit PLT entries for interworking jumps, + * and for conditional interworking call instructions. */ if (IS_ENABLED(CONFIG_ARM_MODULE_PLTS) && (offset <= (s32)0xfe000000 || - offset >= (s32)0x02000000)) + offset >= (s32)0x02000000 || + ((sym->st_value & 1) != 0 && + (ELF32_R_TYPE(rel->r_info) != R_ARM_CALL || + __mem_to_opcode_arm(*(u32 *)loc) >> 28 != 0xe)))) offset = get_module_plt(module, loc, - offset + loc + 8) + offset + loc + 8, false) - loc - 8; if (offset <= (s32)0xfe000000 || @@ -138,11 +144,19 @@ apply_relocate(Elf32_Shdr *sechdrs, const char *strtab, unsigned int symindex, return -ENOEXEC; } + if (offset & 1) { + /* convert bl instruction to blx */ + tmp = 0xf0000000 | (offset & 2) << 23; + *(u32 *)loc &= __opcode_to_mem_arm(~BIT(24)); + *(u32 *)loc |= __opcode_to_mem_arm(tmp); + } + offset >>= 2; offset &= 0x00ffffff; *(u32 *)loc &= __opcode_to_mem_arm(0xff000000); *(u32 *)loc |= __opcode_to_mem_arm(offset); + break; case R_ARM_V4BX: @@ -180,8 +194,9 @@ apply_relocate(Elf32_Shdr *sechdrs, const char *strtab, unsigned int symindex, case R_ARM_THM_CALL: case R_ARM_THM_JUMP24: /* - * For function symbols, only Thumb addresses are - * allowed (no interworking). + * For function symbols, we need to force a + * Thumb -> ARM mode switch if the destination + * address has its Thumb bit (bit 0) cleared. * This applies equally to untyped symbols that * resolve to external ksyms: EXPORT_SYMBOL() * strips the function annotation, but we can @@ -197,9 +212,9 @@ apply_relocate(Elf32_Shdr *sechdrs, const char *strtab, unsigned int symindex, (ELF32_ST_TYPE(sym->st_info) == STT_NOTYPE && sym->st_shndx == SHN_UNDEF)) && !(sym->st_value & 1)) { - pr_err("%s: section %u reloc %u sym '%s': unsupported interworking call (Thumb -> ARM)\n", - module->name, relindex, i, symname); - return -ENOEXEC; + tmp = sym->st_value; + } else { + tmp = sym->st_value | 1; } upper = __mem_to_opcode_thumb16(*(u16 *)loc); @@ -227,18 +242,34 @@ apply_relocate(Elf32_Shdr *sechdrs, const char *strtab, unsigned int symindex, ((lower & 0x07ff) << 1); if (offset & 0x01000000) offset -= 0x02000000; - offset += sym->st_value - loc; + offset += tmp; + + /* + * When fixing up a bl instruction to blx, the address + * of the call site must be rounded down in the + * calculation of 'offset'. As this could potentially + * cause 'offset' to go out of range, we need to do + * this before performing the range check. + */ + tmp = offset & 1 ? loc : loc & ~2; + offset -= tmp; /* * Route through a PLT entry if 'offset' exceeds the - * supported range. + * supported range. Also emit PLT entries for + * interworking jump instructions. */ if (IS_ENABLED(CONFIG_ARM_MODULE_PLTS) && (offset <= (s32)0xff000000 || - offset >= (s32)0x01000000)) + offset >= (s32)0x01000000 || + (ELF32_R_TYPE(rel->r_info) != R_ARM_THM_CALL && + !(sym->st_value & 1)))) offset = get_module_plt(module, loc, - offset + loc + 4) + offset + tmp + 4, true) - loc - 4; + else if (!(offset & 1)) + /* fix up bl -> blx */ + lower &= ~(1 << 12); if (offset <= (s32)0xff000000 || offset >= (s32)0x01000000) { -- 2.7.4 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 0/2] add interworking support to Thumb2 kernel 2016-08-20 7:45 [PATCH 0/2] add interworking support to Thumb2 kernel Ard Biesheuvel 2016-08-20 7:45 ` [PATCH 1/2] ARM: Thumb-2: infer function annotation for external ksyms Ard Biesheuvel 2016-08-20 7:45 ` [PATCH 2/2] ARM: add interworking support to Thumb-2 kernel Ard Biesheuvel @ 2016-08-20 16:23 ` Russell King - ARM Linux 2016-08-20 16:36 ` Ard Biesheuvel 2 siblings, 1 reply; 8+ messages in thread From: Russell King - ARM Linux @ 2016-08-20 16:23 UTC (permalink / raw) To: linux-arm-kernel On Sat, Aug 20, 2016 at 09:45:29AM +0200, Ard Biesheuvel wrote: > In the core kernel, interworking is supported since vmlinux is a static > binary, and the linker takes care of fixing up the bl instructions and > emitting veneers if necessary. For modules, however, all function objects > must currently be in the same mode, since the kernel's module loader does > not handle interworking switches at all. I'm not sure what the usefulness of this is - you're effectively encouraging people to build modules with a different configuration from the main kernel, which is bad news. Since it's the configuration which dictates whether the kernel (and modules) are built as ARM or as Thumb2, you'd have to use two different configurations, one for the main kernel and one for the modules. That's really something we should not be encouraging. -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net. ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 0/2] add interworking support to Thumb2 kernel 2016-08-20 16:23 ` [PATCH 0/2] add interworking support to Thumb2 kernel Russell King - ARM Linux @ 2016-08-20 16:36 ` Ard Biesheuvel 2016-08-20 16:55 ` Russell King - ARM Linux 0 siblings, 1 reply; 8+ messages in thread From: Ard Biesheuvel @ 2016-08-20 16:36 UTC (permalink / raw) To: linux-arm-kernel On 20 August 2016 at 18:23, Russell King - ARM Linux <linux@armlinux.org.uk> wrote: > On Sat, Aug 20, 2016 at 09:45:29AM +0200, Ard Biesheuvel wrote: >> In the core kernel, interworking is supported since vmlinux is a static >> binary, and the linker takes care of fixing up the bl instructions and >> emitting veneers if necessary. For modules, however, all function objects >> must currently be in the same mode, since the kernel's module loader does >> not handle interworking switches at all. > > I'm not sure what the usefulness of this is - you're effectively > encouraging people to build modules with a different configuration > from the main kernel, which is bad news. Since it's the configuration > which dictates whether the kernel (and modules) are built as ARM or > as Thumb2, you'd have to use two different configurations, one for the > main kernel and one for the modules. > No, that is not the point. The point is to lift the requirement that a Thumb2 kernel must implement all its exported or modularised functions in Thumb2, not only in the core kernel but also in modules themselves. For example, the crypto modules I contributed contain .S core implementations, and building those in Thumb2 requires tweaking the implementations so that the assembler accepts it as Thumb2 code. There are examples in the tree (sha1-v4 and aes-v4) where we had to add ugly workarounds so that the code would build (and it was still broken for a while). So in general, it would be preferable to have a single ARM implementation for both modes. Also, it will become less likely for the multi_v7_defconfig to break inadvertently due to the addition of low level platform code with asm components that turn out not to build in Thumb2. So with these patches, it is no longer necessary for core helper routines coded in assembler (or built in ARM mode for other reasons) to go out of their way so that the assembler can turn them in both ARM and Thumb2 code. I think there may be a performance benefit in some cases as well, although this is anecdotal hearsay. The primary benefit of Thumb2 is code size, and multi_v7_defconfig is a couple of MBs smaller when built in Thumb2. In any case, lifting the restriction only affects configurations that set CONFIG_THUMB2_KERNEL globally, and THUMB2 modules can still only be loaded into a THUMB2 kernel (due to the THUMB2 tag in the version string). Oh, and the first patch is arguably a bugfix. An exported ARM function will not be rejected atm, but called in Thumb2 mode. -- Ard. ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 0/2] add interworking support to Thumb2 kernel 2016-08-20 16:36 ` Ard Biesheuvel @ 2016-08-20 16:55 ` Russell King - ARM Linux 2016-08-20 17:38 ` Ard Biesheuvel 0 siblings, 1 reply; 8+ messages in thread From: Russell King - ARM Linux @ 2016-08-20 16:55 UTC (permalink / raw) To: linux-arm-kernel On Sat, Aug 20, 2016 at 06:36:44PM +0200, Ard Biesheuvel wrote: > No, that is not the point. The point is to lift the requirement that a > Thumb2 kernel must implement all its exported or modularised functions > in Thumb2, not only in the core kernel but also in modules themselves. > For example, the crypto modules I contributed contain .S core > implementations, and building those in Thumb2 requires tweaking the > implementations so that the assembler accepts it as Thumb2 code. There > are examples in the tree (sha1-v4 and aes-v4) where we had to add ugly > workarounds so that the code would build (and it was still broken for > a while). So in general, it would be preferable to have a single ARM > implementation for both modes. Also, it will become less likely for > the multi_v7_defconfig to break inadvertently due to the addition of > low level platform code with asm components that turn out not to build > in Thumb2. Right, so we're talking _purely_ about assembly level stuff here. However, what I would point out is that some people will want to build their kernel as definitely ARM-only or definitely Thumb2-only. While A-class accepts either, other class CPUs do not. If we allow interworking, then we start breaking that. We do already support CPUs that only enter the kernel in Thumb2 mode or only enter in ARM mode, because the CPU has no support for the other instruction set. > So with these patches, it is no longer necessary for core helper > routines coded in assembler (or built in ARM mode for other reasons) > to go out of their way so that the assembler can turn them in both ARM > and Thumb2 code. I'm not sure what "core helper routines" you're referring to - all C functions will be built according to whether we want ARM mode or Thumb2 mode. We aren't going to end up with a mixture for C functions within the kernel. > I think there may be a performance benefit in some cases as well, > although this is anecdotal hearsay. The primary benefit of Thumb2 is > code size, and multi_v7_defconfig is a couple of MBs smaller when > built in Thumb2. In any case, lifting the restriction only affects > configurations that set CONFIG_THUMB2_KERNEL globally, and THUMB2 > modules can still only be loaded into a THUMB2 kernel (due to the > THUMB2 tag in the version string). > > Oh, and the first patch is arguably a bugfix. An exported ARM function > will not be rejected atm, but called in Thumb2 mode. Sorry, I don't see it as a bug fix at all - I see it as a regression causing change for those classes of CPU which only accept one or other of the ISAs. -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net. ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 0/2] add interworking support to Thumb2 kernel 2016-08-20 16:55 ` Russell King - ARM Linux @ 2016-08-20 17:38 ` Ard Biesheuvel 0 siblings, 0 replies; 8+ messages in thread From: Ard Biesheuvel @ 2016-08-20 17:38 UTC (permalink / raw) To: linux-arm-kernel On 20 August 2016 at 18:55, Russell King - ARM Linux <linux@armlinux.org.uk> wrote: > On Sat, Aug 20, 2016 at 06:36:44PM +0200, Ard Biesheuvel wrote: >> No, that is not the point. The point is to lift the requirement that a >> Thumb2 kernel must implement all its exported or modularised functions >> in Thumb2, not only in the core kernel but also in modules themselves. >> For example, the crypto modules I contributed contain .S core >> implementations, and building those in Thumb2 requires tweaking the >> implementations so that the assembler accepts it as Thumb2 code. There >> are examples in the tree (sha1-v4 and aes-v4) where we had to add ugly >> workarounds so that the code would build (and it was still broken for >> a while). So in general, it would be preferable to have a single ARM >> implementation for both modes. Also, it will become less likely for >> the multi_v7_defconfig to break inadvertently due to the addition of >> low level platform code with asm components that turn out not to build >> in Thumb2. > > Right, so we're talking _purely_ about assembly level stuff here. > Primarily, yes. > However, what I would point out is that some people will want to build > their kernel as definitely ARM-only or definitely Thumb2-only. While > A-class accepts either, other class CPUs do not. If we allow > interworking, then we start breaking that. > > We do already support CPUs that only enter the kernel in Thumb2 mode or > only enter in ARM mode, because the CPU has no support for the other > instruction set. > >> So with these patches, it is no longer necessary for core helper >> routines coded in assembler (or built in ARM mode for other reasons) >> to go out of their way so that the assembler can turn them in both ARM >> and Thumb2 code. > > I'm not sure what "core helper routines" you're referring to - all C > functions will be built according to whether we want ARM mode or Thumb2 > mode. We aren't going to end up with a mixture for C functions within > the kernel. > Yes, but taking the NEON code in arch/arm/crypto as an example, this can only execute on A cores anyway. When it is built into the kernel, we can happily use the ARM version on a Thumb2 kernel, but as a module it suddenly has to be adapted so that it is emitted as Thumb2 (while upstream OpenSSL builds it in ARM mode) >> I think there may be a performance benefit in some cases as well, >> although this is anecdotal hearsay. The primary benefit of Thumb2 is >> code size, and multi_v7_defconfig is a couple of MBs smaller when >> built in Thumb2. In any case, lifting the restriction only affects >> configurations that set CONFIG_THUMB2_KERNEL globally, and THUMB2 >> modules can still only be loaded into a THUMB2 kernel (due to the >> THUMB2 tag in the version string). >> >> Oh, and the first patch is arguably a bugfix. An exported ARM function >> will not be rejected atm, but called in Thumb2 mode. > > Sorry, I don't see it as a bug fix at all - I see it as a regression > causing change for those classes of CPU which only accept one or > other of the ISAs. > At the moment, the module loader allows R_ARM_THM_CALL relocations against imported ARM symbols, under the assumption that an untyped symbol is a Thumb2 symbol that simply has its Thumb bit cleared due to the fact that it lacks a function annotation. However, if the relocation is resolved across a module boundary, this annotation is missing as well, and so imported ARM symbols are mistaken for Thumb2 symbols, causing a crash when called. Patch #1 fixes just that. So do you (or anyone else) have any comments on the performance delta between Thumb2 and ARM? I can imagine fixed size instructions make for more efficient pipelines, but as I said, this is only a hunch. ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/2] ARM: Thumb-2: infer function annotation for external ksyms @ 2014-11-26 16:10 Ard Biesheuvel 0 siblings, 0 replies; 8+ messages in thread From: Ard Biesheuvel @ 2014-11-26 16:10 UTC (permalink / raw) To: linux-arm-kernel Commit 9a00318eadbb ("Thumb-2: Relax relocation requirements for non-function symbols") updated the Thumb-2 jump and call relocation handling so that non-function symbols with the Thumb bit (bit 0) cleared are not treated as A32 symbols requiring an interworking mode switch. However, since ksyms are stripped of their function annotation by EXPORT_SYMBOL(), this will result in A32 symbols that do require an interworking mode switch to be called in the wrong mode if the relocation is resolved across a module boundary. This patch enhances the function symbol check by including untyped symbols that resolve to external ksyms. Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> --- arch/arm/kernel/module.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/arch/arm/kernel/module.c b/arch/arm/kernel/module.c index a68040989143..6c08b2188992 100644 --- a/arch/arm/kernel/module.c +++ b/arch/arm/kernel/module.c @@ -182,13 +182,20 @@ apply_relocate(Elf32_Shdr *sechdrs, const char *strtab, unsigned int symindex, /* * For function symbols, only Thumb addresses are * allowed (no interworking). + * This applies equally to untyped symbols that + * resolve to external ksyms: EXPORT_SYMBOL() + * strips the function annotation, but we can + * infer from the relocation type that the target + * must be a function. * * For non-function symbols, the destination * has no specific ARM/Thumb disposition, so * the branch is resolved under the assumption * that interworking is not required. */ - if (ELF32_ST_TYPE(sym->st_info) == STT_FUNC && + if ((ELF32_ST_TYPE(sym->st_info) == STT_FUNC || + (ELF32_ST_TYPE(sym->st_info) == STT_NOTYPE && + sym->st_shndx == SHN_UNDEF)) && !(sym->st_value & 1)) { pr_err("%s: section %u reloc %u sym '%s': unsupported interworking call (Thumb -> ARM)\n", module->name, relindex, i, symname); -- 1.8.3.2 ^ permalink raw reply related [flat|nested] 8+ messages in thread
end of thread, other threads:[~2016-08-20 17:38 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-08-20 7:45 [PATCH 0/2] add interworking support to Thumb2 kernel Ard Biesheuvel 2016-08-20 7:45 ` [PATCH 1/2] ARM: Thumb-2: infer function annotation for external ksyms Ard Biesheuvel 2016-08-20 7:45 ` [PATCH 2/2] ARM: add interworking support to Thumb-2 kernel Ard Biesheuvel 2016-08-20 16:23 ` [PATCH 0/2] add interworking support to Thumb2 kernel Russell King - ARM Linux 2016-08-20 16:36 ` Ard Biesheuvel 2016-08-20 16:55 ` Russell King - ARM Linux 2016-08-20 17:38 ` Ard Biesheuvel -- strict thread matches above, loose matches on Subject: below -- 2014-11-26 16:10 [PATCH 1/2] ARM: Thumb-2: infer function annotation for external ksyms Ard Biesheuvel
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).