| Message ID | 20201118130849.7445-1-ardb@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] ARM: entry: omit FP emulation for UND exceptions taken in kernel mode | expand |
On Wed, Nov 18, 2020 at 5:09 AM Ard Biesheuvel <ardb@kernel.org> wrote: > > There are a couple of problems with the exception entry code that deals > with FP exceptions (which are reported as UND exceptions) when building > the kernel in Thumb2 mode: > - the conditional branch to vfp_kmode_exception in vfp_support_entry() > may be out of range for its target, depending on how the linker decides > to arrange the sections; > - when the UND exception is taken in kernel mode, the emulation handling > logic is entered via the 'call_fpe' label, which means we end up using > the wrong value/mask pairs to match and detect the NEON opcodes. > > Since UND exceptions in kernel mode are unlikely to occur on a hot path > (as opposed to the user mode version which is invoked for VFP support > code and lazy restore), we can use the existing undef hook machinery for Right, I'd expect these maybe from userspace, but within the kernel? > any kernel mode instruction emulation that is needed, including calling > the existing vfp_kmode_exception() routine for unexpected cases. So drop > the call to call_fpe, and instead, install an undef hook that will get > called for NEON and VFP instructions that trigger an UND exception in > kernel mode. > > While at it, make sure that the PC correction is accurate for the > execution mode where the exception was taken, by checking the PSR > Thumb bit. > > Cc: Dmitry Osipenko <digetx@gmail.com> > Cc: Kees Cook <keescook@chromium.org> > Cc: Nick Desaulniers <ndesaulniers@google.com> > Fixes: eff8728fe698 ("vmlinux.lds.h: Add PGO and AutoFDO input sections") > Signed-off-by: Ard Biesheuvel <ardb@kernel.org> > Reviewed-by: Linus Walleij <linus.walleij@linaro.org> > --- > NOTE: this supersedes 9018/2 "vfp: force non-conditional encoding for external > Thumb2" which is currently queued in the patch system - the out-of-range > branch to vfp_kmode_exception() is dropped entirely in this patch > > v2: - use the PSR T bit to select the right PC correction > - add Linus's ack > > arch/arm/kernel/entry-armv.S | 25 +--------- > arch/arm/vfp/vfphw.S | 5 -- > arch/arm/vfp/vfpmodule.c | 49 +++++++++++++++++++- > 3 files changed, 49 insertions(+), 30 deletions(-) > > diff --git a/arch/arm/kernel/entry-armv.S b/arch/arm/kernel/entry-armv.S > index c4220f51fcf3..0ea8529a4872 100644 > --- a/arch/arm/kernel/entry-armv.S > +++ b/arch/arm/kernel/entry-armv.S > @@ -252,31 +252,10 @@ __und_svc: > #else > svc_entry > #endif > - @ > - @ call emulation code, which returns using r9 if it has emulated > - @ the instruction, or the more conventional lr if we are to treat > - @ this as a real undefined instruction > - @ > - @ r0 - instruction > - @ > -#ifndef CONFIG_THUMB2_KERNEL > - ldr r0, [r4, #-4] > -#else > - mov r1, #2 > - ldrh r0, [r4, #-2] @ Thumb instruction at LR - 2 > - cmp r0, #0xe800 @ 32-bit instruction if xx >= 0 > - blo __und_svc_fault > - ldrh r9, [r4] @ bottom 16 bits > - add r4, r4, #2 > - str r4, [sp, #S_PC] > - orr r0, r9, r0, lsl #16 > -#endif > - badr r9, __und_svc_finish > - mov r2, r4 > - bl call_fpe > > mov r1, #4 @ PC correction to apply > -__und_svc_fault: > + THUMB( tst r5, #PSR_T_BIT ) @ exception taken in Thumb mode? Question: what's in r5 at this point? > + THUMB( movne r1, #2 ) @ if so, fix up PC correction > mov r0, sp @ struct pt_regs *regs > bl __und_fault > > diff --git a/arch/arm/vfp/vfphw.S b/arch/arm/vfp/vfphw.S > index 4fcff9f59947..d5837bf05a9a 100644 > --- a/arch/arm/vfp/vfphw.S > +++ b/arch/arm/vfp/vfphw.S > @@ -79,11 +79,6 @@ ENTRY(vfp_support_entry) > DBGSTR3 "instr %08x pc %08x state %p", r0, r2, r10 > > .fpu vfpv2 > - ldr r3, [sp, #S_PSR] @ Neither lazy restore nor FP exceptions > - and r3, r3, #MODE_MASK @ are supported in kernel mode > - teq r3, #USR_MODE > - bne vfp_kmode_exception @ Returns through lr > - > VFPFMRX r1, FPEXC @ Is the VFP enabled? > DBGSTR1 "fpexc %08x", r1 > tst r1, #FPEXC_EN > diff --git a/arch/arm/vfp/vfpmodule.c b/arch/arm/vfp/vfpmodule.c > index 8c9e7f9f0277..c3b6451c18bd 100644 > --- a/arch/arm/vfp/vfpmodule.c > +++ b/arch/arm/vfp/vfpmodule.c > @@ -23,6 +23,7 @@ > #include <asm/cputype.h> > #include <asm/system_info.h> > #include <asm/thread_notify.h> > +#include <asm/traps.h> > #include <asm/vfp.h> > > #include "vfpinstr.h" > @@ -642,7 +643,9 @@ static int vfp_starting_cpu(unsigned int unused) > return 0; > } > > -void vfp_kmode_exception(void) > +#ifdef CONFIG_KERNEL_MODE_NEON > + > +static int vfp_kmode_exception(struct pt_regs *regs, unsigned int instr) > { > /* > * If we reach this point, a floating point exception has been raised > @@ -660,9 +663,51 @@ void vfp_kmode_exception(void) > pr_crit("BUG: unsupported FP instruction in kernel mode\n"); > else > pr_crit("BUG: FP instruction issued in kernel mode with FP unit disabled\n"); > + pr_crit("FPEXC == 0x%08x\n", fmrx(FPEXC)); > + return 1; > } > > -#ifdef CONFIG_KERNEL_MODE_NEON > +static struct undef_hook vfp_kmode_exception_hook[] = {{ > + .instr_mask = 0xfe000000, > + .instr_val = 0xf2000000, > + .cpsr_mask = MODE_MASK | PSR_T_BIT, > + .cpsr_val = SVC_MODE, > + .fn = vfp_kmode_exception, > +}, { > + .instr_mask = 0xff100000, > + .instr_val = 0xf4000000, > + .cpsr_mask = MODE_MASK | PSR_T_BIT, > + .cpsr_val = SVC_MODE, > + .fn = vfp_kmode_exception, > +}, { > + .instr_mask = 0xef000000, > + .instr_val = 0xef000000, > + .cpsr_mask = MODE_MASK | PSR_T_BIT, > + .cpsr_val = SVC_MODE | PSR_T_BIT, > + .fn = vfp_kmode_exception, > +}, { > + .instr_mask = 0xff100000, > + .instr_val = 0xf9000000, > + .cpsr_mask = MODE_MASK | PSR_T_BIT, > + .cpsr_val = SVC_MODE | PSR_T_BIT, > + .fn = vfp_kmode_exception, > +}, { > + .instr_mask = 0x0c000e00, > + .instr_val = 0x0c000a00, > + .cpsr_mask = MODE_MASK, > + .cpsr_val = SVC_MODE, > + .fn = vfp_kmode_exception, > +}}; I don't plan on verifying these instruction masks, but I wanted to check that the first two should not be bitwise OR'ing PSR_T_BIT for the .cpsr_val like the next two structs do? Patch looks reasonable to me otherwise, just some naive questions in case these differences were unintentional. Would comments be helpful for each mask for what kind of opcode they're handling? > + > +static int __init vfp_kmode_exception_hook_init(void) > +{ > + int i; > + > + for (i = 0; i < ARRAY_SIZE(vfp_kmode_exception_hook); i++) > + register_undef_hook(&vfp_kmode_exception_hook[i]); > + return 0; > +} > +core_initcall(vfp_kmode_exception_hook_init); > > /* > * Kernel-side NEON support functions > -- > 2.17.1 >
On Wed, 18 Nov 2020 at 17:42, Nick Desaulniers <ndesaulniers@google.com> wrote: > > On Wed, Nov 18, 2020 at 5:09 AM Ard Biesheuvel <ardb@kernel.org> wrote: > > > > There are a couple of problems with the exception entry code that deals > > with FP exceptions (which are reported as UND exceptions) when building > > the kernel in Thumb2 mode: > > - the conditional branch to vfp_kmode_exception in vfp_support_entry() > > may be out of range for its target, depending on how the linker decides > > to arrange the sections; > > - when the UND exception is taken in kernel mode, the emulation handling > > logic is entered via the 'call_fpe' label, which means we end up using > > the wrong value/mask pairs to match and detect the NEON opcodes. > > > > Since UND exceptions in kernel mode are unlikely to occur on a hot path > > (as opposed to the user mode version which is invoked for VFP support > > code and lazy restore), we can use the existing undef hook machinery for > > Right, I'd expect these maybe from userspace, but within the kernel? > Russell explained off-list that there used to be a case in the pre-VFP era, but this is no longer relevant. > > any kernel mode instruction emulation that is needed, including calling > > the existing vfp_kmode_exception() routine for unexpected cases. So drop > > the call to call_fpe, and instead, install an undef hook that will get > > called for NEON and VFP instructions that trigger an UND exception in > > kernel mode. > > > > While at it, make sure that the PC correction is accurate for the > > execution mode where the exception was taken, by checking the PSR > > Thumb bit. > > > > Cc: Dmitry Osipenko <digetx@gmail.com> > > Cc: Kees Cook <keescook@chromium.org> > > Cc: Nick Desaulniers <ndesaulniers@google.com> > > Fixes: eff8728fe698 ("vmlinux.lds.h: Add PGO and AutoFDO input sections") > > Signed-off-by: Ard Biesheuvel <ardb@kernel.org> > > Reviewed-by: Linus Walleij <linus.walleij@linaro.org> > > --- > > NOTE: this supersedes 9018/2 "vfp: force non-conditional encoding for external > > Thumb2" which is currently queued in the patch system - the out-of-range > > branch to vfp_kmode_exception() is dropped entirely in this patch > > > > v2: - use the PSR T bit to select the right PC correction > > - add Linus's ack > > > > arch/arm/kernel/entry-armv.S | 25 +--------- > > arch/arm/vfp/vfphw.S | 5 -- > > arch/arm/vfp/vfpmodule.c | 49 +++++++++++++++++++- > > 3 files changed, 49 insertions(+), 30 deletions(-) > > > > diff --git a/arch/arm/kernel/entry-armv.S b/arch/arm/kernel/entry-armv.S > > index c4220f51fcf3..0ea8529a4872 100644 > > --- a/arch/arm/kernel/entry-armv.S > > +++ b/arch/arm/kernel/entry-armv.S > > @@ -252,31 +252,10 @@ __und_svc: > > #else > > svc_entry > > #endif > > - @ > > - @ call emulation code, which returns using r9 if it has emulated > > - @ the instruction, or the more conventional lr if we are to treat > > - @ this as a real undefined instruction > > - @ > > - @ r0 - instruction > > - @ > > -#ifndef CONFIG_THUMB2_KERNEL > > - ldr r0, [r4, #-4] > > -#else > > - mov r1, #2 > > - ldrh r0, [r4, #-2] @ Thumb instruction at LR - 2 > > - cmp r0, #0xe800 @ 32-bit instruction if xx >= 0 > > - blo __und_svc_fault > > - ldrh r9, [r4] @ bottom 16 bits > > - add r4, r4, #2 > > - str r4, [sp, #S_PC] > > - orr r0, r9, r0, lsl #16 > > -#endif > > - badr r9, __und_svc_finish > > - mov r2, r4 > > - bl call_fpe > > > > mov r1, #4 @ PC correction to apply > > -__und_svc_fault: > > + THUMB( tst r5, #PSR_T_BIT ) @ exception taken in Thumb mode? > > Question: what's in r5 at this point? > The PSR of the interrupted execution context. > > + THUMB( movne r1, #2 ) @ if so, fix up PC correction > > mov r0, sp @ struct pt_regs *regs > > bl __und_fault > > > > diff --git a/arch/arm/vfp/vfphw.S b/arch/arm/vfp/vfphw.S > > index 4fcff9f59947..d5837bf05a9a 100644 > > --- a/arch/arm/vfp/vfphw.S > > +++ b/arch/arm/vfp/vfphw.S > > @@ -79,11 +79,6 @@ ENTRY(vfp_support_entry) > > DBGSTR3 "instr %08x pc %08x state %p", r0, r2, r10 > > > > .fpu vfpv2 > > - ldr r3, [sp, #S_PSR] @ Neither lazy restore nor FP exceptions > > - and r3, r3, #MODE_MASK @ are supported in kernel mode > > - teq r3, #USR_MODE > > - bne vfp_kmode_exception @ Returns through lr > > - > > VFPFMRX r1, FPEXC @ Is the VFP enabled? > > DBGSTR1 "fpexc %08x", r1 > > tst r1, #FPEXC_EN > > diff --git a/arch/arm/vfp/vfpmodule.c b/arch/arm/vfp/vfpmodule.c > > index 8c9e7f9f0277..c3b6451c18bd 100644 > > --- a/arch/arm/vfp/vfpmodule.c > > +++ b/arch/arm/vfp/vfpmodule.c > > @@ -23,6 +23,7 @@ > > #include <asm/cputype.h> > > #include <asm/system_info.h> > > #include <asm/thread_notify.h> > > +#include <asm/traps.h> > > #include <asm/vfp.h> > > > > #include "vfpinstr.h" > > @@ -642,7 +643,9 @@ static int vfp_starting_cpu(unsigned int unused) > > return 0; > > } > > > > -void vfp_kmode_exception(void) > > +#ifdef CONFIG_KERNEL_MODE_NEON > > + > > +static int vfp_kmode_exception(struct pt_regs *regs, unsigned int instr) > > { > > /* > > * If we reach this point, a floating point exception has been raised > > @@ -660,9 +663,51 @@ void vfp_kmode_exception(void) > > pr_crit("BUG: unsupported FP instruction in kernel mode\n"); > > else > > pr_crit("BUG: FP instruction issued in kernel mode with FP unit disabled\n"); > > + pr_crit("FPEXC == 0x%08x\n", fmrx(FPEXC)); > > + return 1; > > } > > > > -#ifdef CONFIG_KERNEL_MODE_NEON > > +static struct undef_hook vfp_kmode_exception_hook[] = {{ > > + .instr_mask = 0xfe000000, > > + .instr_val = 0xf2000000, > > + .cpsr_mask = MODE_MASK | PSR_T_BIT, > > + .cpsr_val = SVC_MODE, > > + .fn = vfp_kmode_exception, > > +}, { > > + .instr_mask = 0xff100000, > > + .instr_val = 0xf4000000, > > + .cpsr_mask = MODE_MASK | PSR_T_BIT, > > + .cpsr_val = SVC_MODE, > > + .fn = vfp_kmode_exception, > > +}, { > > + .instr_mask = 0xef000000, > > + .instr_val = 0xef000000, > > + .cpsr_mask = MODE_MASK | PSR_T_BIT, > > + .cpsr_val = SVC_MODE | PSR_T_BIT, > > + .fn = vfp_kmode_exception, > > +}, { > > + .instr_mask = 0xff100000, > > + .instr_val = 0xf9000000, > > + .cpsr_mask = MODE_MASK | PSR_T_BIT, > > + .cpsr_val = SVC_MODE | PSR_T_BIT, > > + .fn = vfp_kmode_exception, > > +}, { > > + .instr_mask = 0x0c000e00, > > + .instr_val = 0x0c000a00, > > + .cpsr_mask = MODE_MASK, > > + .cpsr_val = SVC_MODE, > > + .fn = vfp_kmode_exception, > > +}}; > > I don't plan on verifying these instruction masks, but I wanted to > check that the first two should not be bitwise OR'ing PSR_T_BIT for > the .cpsr_val like the next two structs do? The first pair describes ARM opcodes, and the second pair describes Thumb2 opcodes. So in the former case, the T bit should be clear, and in the latter, the T bit should be set (and in the final case, the T bit is D/C so it is omitted from both the mask and the val fields) > Patch looks reasonable to > me otherwise, just some naive questions in case these differences were > unintentional. Would comments be helpful for each mask for what kind > of opcode they're handling? > I don't mind adding those, although it is fairly self explanatory if you are familiar with how these undef hooks work. > > + > > +static int __init vfp_kmode_exception_hook_init(void) > > +{ > > + int i; > > + > > + for (i = 0; i < ARRAY_SIZE(vfp_kmode_exception_hook); i++) > > + register_undef_hook(&vfp_kmode_exception_hook[i]); > > + return 0; > > +} > > +core_initcall(vfp_kmode_exception_hook_init); > > > > /* > > * Kernel-side NEON support functions > > -- > > 2.17.1 > > > > > -- > Thanks, > ~Nick Desaulniers
On Wed, Nov 18, 2020 at 8:48 AM Ard Biesheuvel <ardb@kernel.org> wrote: > > On Wed, 18 Nov 2020 at 17:42, Nick Desaulniers <ndesaulniers@google.com> wrote: > > > > On Wed, Nov 18, 2020 at 5:09 AM Ard Biesheuvel <ardb@kernel.org> wrote: > > > > > > There are a couple of problems with the exception entry code that deals > > > with FP exceptions (which are reported as UND exceptions) when building > > > the kernel in Thumb2 mode: > > > - the conditional branch to vfp_kmode_exception in vfp_support_entry() > > > may be out of range for its target, depending on how the linker decides > > > to arrange the sections; > > > - when the UND exception is taken in kernel mode, the emulation handling > > > logic is entered via the 'call_fpe' label, which means we end up using > > > the wrong value/mask pairs to match and detect the NEON opcodes. > > > > > > Since UND exceptions in kernel mode are unlikely to occur on a hot path > > > (as opposed to the user mode version which is invoked for VFP support > > > code and lazy restore), we can use the existing undef hook machinery for > > > > Right, I'd expect these maybe from userspace, but within the kernel? > > > > Russell explained off-list that there used to be a case in the pre-VFP > era, but this is no longer relevant. If the use case is no longer relevant, consider dropping support. Dead code is technical debt. > > > > any kernel mode instruction emulation that is needed, including calling > > > the existing vfp_kmode_exception() routine for unexpected cases. So drop > > > the call to call_fpe, and instead, install an undef hook that will get > > > called for NEON and VFP instructions that trigger an UND exception in > > > kernel mode. > > > > > > While at it, make sure that the PC correction is accurate for the > > > execution mode where the exception was taken, by checking the PSR > > > Thumb bit. > > > > > > Cc: Dmitry Osipenko <digetx@gmail.com> > > > Cc: Kees Cook <keescook@chromium.org> > > > Cc: Nick Desaulniers <ndesaulniers@google.com> > > > Fixes: eff8728fe698 ("vmlinux.lds.h: Add PGO and AutoFDO input sections") > > > Signed-off-by: Ard Biesheuvel <ardb@kernel.org> > > > Reviewed-by: Linus Walleij <linus.walleij@linaro.org> > > > --- > > > NOTE: this supersedes 9018/2 "vfp: force non-conditional encoding for external > > > Thumb2" which is currently queued in the patch system - the out-of-range > > > branch to vfp_kmode_exception() is dropped entirely in this patch > > > > > > v2: - use the PSR T bit to select the right PC correction > > > - add Linus's ack > > > > > > arch/arm/kernel/entry-armv.S | 25 +--------- > > > arch/arm/vfp/vfphw.S | 5 -- > > > arch/arm/vfp/vfpmodule.c | 49 +++++++++++++++++++- > > > 3 files changed, 49 insertions(+), 30 deletions(-) > > > > > > diff --git a/arch/arm/kernel/entry-armv.S b/arch/arm/kernel/entry-armv.S > > > index c4220f51fcf3..0ea8529a4872 100644 > > > --- a/arch/arm/kernel/entry-armv.S > > > +++ b/arch/arm/kernel/entry-armv.S > > > @@ -252,31 +252,10 @@ __und_svc: > > > #else > > > svc_entry > > > #endif > > > - @ > > > - @ call emulation code, which returns using r9 if it has emulated > > > - @ the instruction, or the more conventional lr if we are to treat > > > - @ this as a real undefined instruction > > > - @ > > > - @ r0 - instruction > > > - @ > > > -#ifndef CONFIG_THUMB2_KERNEL > > > - ldr r0, [r4, #-4] > > > -#else > > > - mov r1, #2 > > > - ldrh r0, [r4, #-2] @ Thumb instruction at LR - 2 > > > - cmp r0, #0xe800 @ 32-bit instruction if xx >= 0 > > > - blo __und_svc_fault > > > - ldrh r9, [r4] @ bottom 16 bits > > > - add r4, r4, #2 > > > - str r4, [sp, #S_PC] > > > - orr r0, r9, r0, lsl #16 > > > -#endif > > > - badr r9, __und_svc_finish > > > - mov r2, r4 > > > - bl call_fpe > > > > > > mov r1, #4 @ PC correction to apply > > > -__und_svc_fault: > > > + THUMB( tst r5, #PSR_T_BIT ) @ exception taken in Thumb mode? > > > > Question: what's in r5 at this point? > > > > The PSR of the interrupted execution context. ah the svc_entry assembler macro has a comment related to a store multiple increment after that sets it. > > > > + THUMB( movne r1, #2 ) @ if so, fix up PC correction > > > mov r0, sp @ struct pt_regs *regs > > > bl __und_fault > > > > > > diff --git a/arch/arm/vfp/vfphw.S b/arch/arm/vfp/vfphw.S > > > index 4fcff9f59947..d5837bf05a9a 100644 > > > --- a/arch/arm/vfp/vfphw.S > > > +++ b/arch/arm/vfp/vfphw.S > > > @@ -79,11 +79,6 @@ ENTRY(vfp_support_entry) > > > DBGSTR3 "instr %08x pc %08x state %p", r0, r2, r10 > > > > > > .fpu vfpv2 > > > - ldr r3, [sp, #S_PSR] @ Neither lazy restore nor FP exceptions > > > - and r3, r3, #MODE_MASK @ are supported in kernel mode > > > - teq r3, #USR_MODE > > > - bne vfp_kmode_exception @ Returns through lr > > > - > > > VFPFMRX r1, FPEXC @ Is the VFP enabled? > > > DBGSTR1 "fpexc %08x", r1 > > > tst r1, #FPEXC_EN > > > diff --git a/arch/arm/vfp/vfpmodule.c b/arch/arm/vfp/vfpmodule.c > > > index 8c9e7f9f0277..c3b6451c18bd 100644 > > > --- a/arch/arm/vfp/vfpmodule.c > > > +++ b/arch/arm/vfp/vfpmodule.c > > > @@ -23,6 +23,7 @@ > > > #include <asm/cputype.h> > > > #include <asm/system_info.h> > > > #include <asm/thread_notify.h> > > > +#include <asm/traps.h> > > > #include <asm/vfp.h> > > > > > > #include "vfpinstr.h" > > > @@ -642,7 +643,9 @@ static int vfp_starting_cpu(unsigned int unused) > > > return 0; > > > } > > > > > > -void vfp_kmode_exception(void) > > > +#ifdef CONFIG_KERNEL_MODE_NEON > > > + > > > +static int vfp_kmode_exception(struct pt_regs *regs, unsigned int instr) > > > { > > > /* > > > * If we reach this point, a floating point exception has been raised > > > @@ -660,9 +663,51 @@ void vfp_kmode_exception(void) > > > pr_crit("BUG: unsupported FP instruction in kernel mode\n"); > > > else > > > pr_crit("BUG: FP instruction issued in kernel mode with FP unit disabled\n"); > > > + pr_crit("FPEXC == 0x%08x\n", fmrx(FPEXC)); > > > + return 1; > > > } > > > > > > -#ifdef CONFIG_KERNEL_MODE_NEON > > > +static struct undef_hook vfp_kmode_exception_hook[] = {{ > > > + .instr_mask = 0xfe000000, > > > + .instr_val = 0xf2000000, > > > + .cpsr_mask = MODE_MASK | PSR_T_BIT, > > > + .cpsr_val = SVC_MODE, > > > + .fn = vfp_kmode_exception, > > > +}, { > > > + .instr_mask = 0xff100000, > > > + .instr_val = 0xf4000000, > > > + .cpsr_mask = MODE_MASK | PSR_T_BIT, > > > + .cpsr_val = SVC_MODE, > > > + .fn = vfp_kmode_exception, > > > +}, { > > > + .instr_mask = 0xef000000, > > > + .instr_val = 0xef000000, > > > + .cpsr_mask = MODE_MASK | PSR_T_BIT, > > > + .cpsr_val = SVC_MODE | PSR_T_BIT, > > > + .fn = vfp_kmode_exception, > > > +}, { > > > + .instr_mask = 0xff100000, > > > + .instr_val = 0xf9000000, > > > + .cpsr_mask = MODE_MASK | PSR_T_BIT, > > > + .cpsr_val = SVC_MODE | PSR_T_BIT, > > > + .fn = vfp_kmode_exception, > > > +}, { > > > + .instr_mask = 0x0c000e00, > > > + .instr_val = 0x0c000a00, > > > + .cpsr_mask = MODE_MASK, > > > + .cpsr_val = SVC_MODE, > > > + .fn = vfp_kmode_exception, > > > +}}; > > > > I don't plan on verifying these instruction masks, but I wanted to > > check that the first two should not be bitwise OR'ing PSR_T_BIT for > > the .cpsr_val like the next two structs do? > > The first pair describes ARM opcodes, and the second pair describes > Thumb2 opcodes. So in the former case, the T bit should be clear, and > in the latter, the T bit should be set (and in the final case, the T > bit is D/C so it is omitted from both the mask and the val fields) > > > Patch looks reasonable to > > me otherwise, just some naive questions in case these differences were > > unintentional. Would comments be helpful for each mask for what kind > > of opcode they're handling? > > > > I don't mind adding those, although it is fairly self explanatory if > you are familiar with how these undef hooks work. Whichever, just a thought. Thanks for the patch. Reviewed-by: Nick Desaulniers <ndesaulniers@google.com> > > > > + > > > +static int __init vfp_kmode_exception_hook_init(void) > > > +{ > > > + int i; > > > + > > > + for (i = 0; i < ARRAY_SIZE(vfp_kmode_exception_hook); i++) > > > + register_undef_hook(&vfp_kmode_exception_hook[i]); > > > + return 0; > > > +} > > > +core_initcall(vfp_kmode_exception_hook_init); > > > > > > /* > > > * Kernel-side NEON support functions > > > -- > > > 2.17.1 > > > > > > > > > -- > > Thanks, > > ~Nick Desaulniers
On Wed, 18 Nov 2020 at 17:59, Nick Desaulniers <ndesaulniers@google.com> wrote: > > On Wed, Nov 18, 2020 at 8:48 AM Ard Biesheuvel <ardb@kernel.org> wrote: > > > > On Wed, 18 Nov 2020 at 17:42, Nick Desaulniers <ndesaulniers@google.com> wrote: > > > > > > On Wed, Nov 18, 2020 at 5:09 AM Ard Biesheuvel <ardb@kernel.org> wrote: > > > > > > > > There are a couple of problems with the exception entry code that deals > > > > with FP exceptions (which are reported as UND exceptions) when building > > > > the kernel in Thumb2 mode: > > > > - the conditional branch to vfp_kmode_exception in vfp_support_entry() > > > > may be out of range for its target, depending on how the linker decides > > > > to arrange the sections; > > > > - when the UND exception is taken in kernel mode, the emulation handling > > > > logic is entered via the 'call_fpe' label, which means we end up using > > > > the wrong value/mask pairs to match and detect the NEON opcodes. > > > > > > > > Since UND exceptions in kernel mode are unlikely to occur on a hot path > > > > (as opposed to the user mode version which is invoked for VFP support > > > > code and lazy restore), we can use the existing undef hook machinery for > > > > > > Right, I'd expect these maybe from userspace, but within the kernel? > > > > > > > Russell explained off-list that there used to be a case in the pre-VFP > > era, but this is no longer relevant. > > If the use case is no longer relevant, consider dropping support. > Dead code is technical debt. > Dropping support for what? > > > > > > any kernel mode instruction emulation that is needed, including calling > > > > the existing vfp_kmode_exception() routine for unexpected cases. So drop > > > > the call to call_fpe, and instead, install an undef hook that will get > > > > called for NEON and VFP instructions that trigger an UND exception in > > > > kernel mode. > > > > > > > > While at it, make sure that the PC correction is accurate for the > > > > execution mode where the exception was taken, by checking the PSR > > > > Thumb bit. > > > > > > > > Cc: Dmitry Osipenko <digetx@gmail.com> > > > > Cc: Kees Cook <keescook@chromium.org> > > > > Cc: Nick Desaulniers <ndesaulniers@google.com> > > > > Fixes: eff8728fe698 ("vmlinux.lds.h: Add PGO and AutoFDO input sections") > > > > Signed-off-by: Ard Biesheuvel <ardb@kernel.org> > > > > Reviewed-by: Linus Walleij <linus.walleij@linaro.org> > > > > --- > > > > NOTE: this supersedes 9018/2 "vfp: force non-conditional encoding for external > > > > Thumb2" which is currently queued in the patch system - the out-of-range > > > > branch to vfp_kmode_exception() is dropped entirely in this patch > > > > > > > > v2: - use the PSR T bit to select the right PC correction > > > > - add Linus's ack > > > > > > > > arch/arm/kernel/entry-armv.S | 25 +--------- > > > > arch/arm/vfp/vfphw.S | 5 -- > > > > arch/arm/vfp/vfpmodule.c | 49 +++++++++++++++++++- > > > > 3 files changed, 49 insertions(+), 30 deletions(-) > > > > > > > > diff --git a/arch/arm/kernel/entry-armv.S b/arch/arm/kernel/entry-armv.S > > > > index c4220f51fcf3..0ea8529a4872 100644 > > > > --- a/arch/arm/kernel/entry-armv.S > > > > +++ b/arch/arm/kernel/entry-armv.S > > > > @@ -252,31 +252,10 @@ __und_svc: > > > > #else > > > > svc_entry > > > > #endif > > > > - @ > > > > - @ call emulation code, which returns using r9 if it has emulated > > > > - @ the instruction, or the more conventional lr if we are to treat > > > > - @ this as a real undefined instruction > > > > - @ > > > > - @ r0 - instruction > > > > - @ > > > > -#ifndef CONFIG_THUMB2_KERNEL > > > > - ldr r0, [r4, #-4] > > > > -#else > > > > - mov r1, #2 > > > > - ldrh r0, [r4, #-2] @ Thumb instruction at LR - 2 > > > > - cmp r0, #0xe800 @ 32-bit instruction if xx >= 0 > > > > - blo __und_svc_fault > > > > - ldrh r9, [r4] @ bottom 16 bits > > > > - add r4, r4, #2 > > > > - str r4, [sp, #S_PC] > > > > - orr r0, r9, r0, lsl #16 > > > > -#endif > > > > - badr r9, __und_svc_finish > > > > - mov r2, r4 > > > > - bl call_fpe > > > > > > > > mov r1, #4 @ PC correction to apply > > > > -__und_svc_fault: > > > > + THUMB( tst r5, #PSR_T_BIT ) @ exception taken in Thumb mode? > > > > > > Question: what's in r5 at this point? > > > > > > > The PSR of the interrupted execution context. > > ah the svc_entry assembler macro has a comment related to a store > multiple increment after that sets it. > > > > > > > + THUMB( movne r1, #2 ) @ if so, fix up PC correction > > > > mov r0, sp @ struct pt_regs *regs > > > > bl __und_fault > > > > > > > > diff --git a/arch/arm/vfp/vfphw.S b/arch/arm/vfp/vfphw.S > > > > index 4fcff9f59947..d5837bf05a9a 100644 > > > > --- a/arch/arm/vfp/vfphw.S > > > > +++ b/arch/arm/vfp/vfphw.S > > > > @@ -79,11 +79,6 @@ ENTRY(vfp_support_entry) > > > > DBGSTR3 "instr %08x pc %08x state %p", r0, r2, r10 > > > > > > > > .fpu vfpv2 > > > > - ldr r3, [sp, #S_PSR] @ Neither lazy restore nor FP exceptions > > > > - and r3, r3, #MODE_MASK @ are supported in kernel mode > > > > - teq r3, #USR_MODE > > > > - bne vfp_kmode_exception @ Returns through lr > > > > - > > > > VFPFMRX r1, FPEXC @ Is the VFP enabled? > > > > DBGSTR1 "fpexc %08x", r1 > > > > tst r1, #FPEXC_EN > > > > diff --git a/arch/arm/vfp/vfpmodule.c b/arch/arm/vfp/vfpmodule.c > > > > index 8c9e7f9f0277..c3b6451c18bd 100644 > > > > --- a/arch/arm/vfp/vfpmodule.c > > > > +++ b/arch/arm/vfp/vfpmodule.c > > > > @@ -23,6 +23,7 @@ > > > > #include <asm/cputype.h> > > > > #include <asm/system_info.h> > > > > #include <asm/thread_notify.h> > > > > +#include <asm/traps.h> > > > > #include <asm/vfp.h> > > > > > > > > #include "vfpinstr.h" > > > > @@ -642,7 +643,9 @@ static int vfp_starting_cpu(unsigned int unused) > > > > return 0; > > > > } > > > > > > > > -void vfp_kmode_exception(void) > > > > +#ifdef CONFIG_KERNEL_MODE_NEON > > > > + > > > > +static int vfp_kmode_exception(struct pt_regs *regs, unsigned int instr) > > > > { > > > > /* > > > > * If we reach this point, a floating point exception has been raised > > > > @@ -660,9 +663,51 @@ void vfp_kmode_exception(void) > > > > pr_crit("BUG: unsupported FP instruction in kernel mode\n"); > > > > else > > > > pr_crit("BUG: FP instruction issued in kernel mode with FP unit disabled\n"); > > > > + pr_crit("FPEXC == 0x%08x\n", fmrx(FPEXC)); > > > > + return 1; > > > > } > > > > > > > > -#ifdef CONFIG_KERNEL_MODE_NEON > > > > +static struct undef_hook vfp_kmode_exception_hook[] = {{ > > > > + .instr_mask = 0xfe000000, > > > > + .instr_val = 0xf2000000, > > > > + .cpsr_mask = MODE_MASK | PSR_T_BIT, > > > > + .cpsr_val = SVC_MODE, > > > > + .fn = vfp_kmode_exception, > > > > +}, { > > > > + .instr_mask = 0xff100000, > > > > + .instr_val = 0xf4000000, > > > > + .cpsr_mask = MODE_MASK | PSR_T_BIT, > > > > + .cpsr_val = SVC_MODE, > > > > + .fn = vfp_kmode_exception, > > > > +}, { > > > > + .instr_mask = 0xef000000, > > > > + .instr_val = 0xef000000, > > > > + .cpsr_mask = MODE_MASK | PSR_T_BIT, > > > > + .cpsr_val = SVC_MODE | PSR_T_BIT, > > > > + .fn = vfp_kmode_exception, > > > > +}, { > > > > + .instr_mask = 0xff100000, > > > > + .instr_val = 0xf9000000, > > > > + .cpsr_mask = MODE_MASK | PSR_T_BIT, > > > > + .cpsr_val = SVC_MODE | PSR_T_BIT, > > > > + .fn = vfp_kmode_exception, > > > > +}, { > > > > + .instr_mask = 0x0c000e00, > > > > + .instr_val = 0x0c000a00, > > > > + .cpsr_mask = MODE_MASK, > > > > + .cpsr_val = SVC_MODE, > > > > + .fn = vfp_kmode_exception, > > > > +}}; > > > > > > I don't plan on verifying these instruction masks, but I wanted to > > > check that the first two should not be bitwise OR'ing PSR_T_BIT for > > > the .cpsr_val like the next two structs do? > > > > The first pair describes ARM opcodes, and the second pair describes > > Thumb2 opcodes. So in the former case, the T bit should be clear, and > > in the latter, the T bit should be set (and in the final case, the T > > bit is D/C so it is omitted from both the mask and the val fields) > > > > > Patch looks reasonable to > > > me otherwise, just some naive questions in case these differences were > > > unintentional. Would comments be helpful for each mask for what kind > > > of opcode they're handling? > > > > > > > I don't mind adding those, although it is fairly self explanatory if > > you are familiar with how these undef hooks work. > > Whichever, just a thought. Thanks for the patch. > Reviewed-by: Nick Desaulniers <ndesaulniers@google.com> > Thanks!
On Wed, Nov 18, 2020 at 9:00 AM Ard Biesheuvel <ardb@kernel.org> wrote: > > On Wed, 18 Nov 2020 at 17:59, Nick Desaulniers <ndesaulniers@google.com> wrote: > > > > On Wed, Nov 18, 2020 at 8:48 AM Ard Biesheuvel <ardb@kernel.org> wrote: > > > > > > On Wed, 18 Nov 2020 at 17:42, Nick Desaulniers <ndesaulniers@google.com> wrote: > > > > > > > > On Wed, Nov 18, 2020 at 5:09 AM Ard Biesheuvel <ardb@kernel.org> wrote: > > > > > > > > > > There are a couple of problems with the exception entry code that deals > > > > > with FP exceptions (which are reported as UND exceptions) when building > > > > > the kernel in Thumb2 mode: > > > > > - the conditional branch to vfp_kmode_exception in vfp_support_entry() > > > > > may be out of range for its target, depending on how the linker decides > > > > > to arrange the sections; > > > > > - when the UND exception is taken in kernel mode, the emulation handling > > > > > logic is entered via the 'call_fpe' label, which means we end up using > > > > > the wrong value/mask pairs to match and detect the NEON opcodes. > > > > > > > > > > Since UND exceptions in kernel mode are unlikely to occur on a hot path > > > > > (as opposed to the user mode version which is invoked for VFP support > > > > > code and lazy restore), we can use the existing undef hook machinery for > > > > > > > > Right, I'd expect these maybe from userspace, but within the kernel? > > > > > > > > > > Russell explained off-list that there used to be a case in the pre-VFP > > > era, but this is no longer relevant. > > > > If the use case is no longer relevant, consider dropping support. > > Dead code is technical debt. > > > > Dropping support for what? By `there used to be a case`, I interpret "a case" to mean one case, singular. "but this is no longer relevant" seems to imply that singular case is not an issue anymore. If what you're referring to is "UND exceptions in kernel mode," then I guess we need exception handling support for those, even if they occur less frequently than the "pre-VFP era" alluded to. So nvm
On Wed, 18 Nov 2020 at 18:08, Nick Desaulniers <ndesaulniers@google.com> wrote: > > On Wed, Nov 18, 2020 at 9:00 AM Ard Biesheuvel <ardb@kernel.org> wrote: > > > > On Wed, 18 Nov 2020 at 17:59, Nick Desaulniers <ndesaulniers@google.com> wrote: > > > > > > On Wed, Nov 18, 2020 at 8:48 AM Ard Biesheuvel <ardb@kernel.org> wrote: > > > > > > > > On Wed, 18 Nov 2020 at 17:42, Nick Desaulniers <ndesaulniers@google.com> wrote: > > > > > > > > > > On Wed, Nov 18, 2020 at 5:09 AM Ard Biesheuvel <ardb@kernel.org> wrote: > > > > > > > > > > > > There are a couple of problems with the exception entry code that deals > > > > > > with FP exceptions (which are reported as UND exceptions) when building > > > > > > the kernel in Thumb2 mode: > > > > > > - the conditional branch to vfp_kmode_exception in vfp_support_entry() > > > > > > may be out of range for its target, depending on how the linker decides > > > > > > to arrange the sections; > > > > > > - when the UND exception is taken in kernel mode, the emulation handling > > > > > > logic is entered via the 'call_fpe' label, which means we end up using > > > > > > the wrong value/mask pairs to match and detect the NEON opcodes. > > > > > > > > > > > > Since UND exceptions in kernel mode are unlikely to occur on a hot path > > > > > > (as opposed to the user mode version which is invoked for VFP support > > > > > > code and lazy restore), we can use the existing undef hook machinery for > > > > > > > > > > Right, I'd expect these maybe from userspace, but within the kernel? > > > > > > > > > > > > > Russell explained off-list that there used to be a case in the pre-VFP > > > > era, but this is no longer relevant. > > > > > > If the use case is no longer relevant, consider dropping support. > > > Dead code is technical debt. > > > > > > > Dropping support for what? > > By `there used to be a case`, I interpret "a case" to mean one case, > singular. "but this is no longer relevant" seems to imply that > singular case is not an issue anymore. > Indeed. Dropping support for the special FP case is precisely what I am proposing here. The only remaining option to handle an undef exception in kernel mode is via undef hooks, and those are not going away. > If what you're referring to is "UND exceptions in kernel mode," then I > guess we need exception handling support for those, even if they occur > less frequently than the "pre-VFP era" alluded to. So nvm No worries. Thanks for the review.
18.11.2020 16:08, Ard Biesheuvel пишет: > diff --git a/arch/arm/kernel/entry-armv.S b/arch/arm/kernel/entry-armv.S > index c4220f51fcf3..0ea8529a4872 100644 > --- a/arch/arm/kernel/entry-armv.S > +++ b/arch/arm/kernel/entry-armv.S > @@ -252,31 +252,10 @@ __und_svc: > #else > svc_entry > #endif > - @ > - @ call emulation code, which returns using r9 if it has emulated > - @ the instruction, or the more conventional lr if we are to treat > - @ this as a real undefined instruction > - @ > - @ r0 - instruction > - @ > -#ifndef CONFIG_THUMB2_KERNEL > - ldr r0, [r4, #-4] > -#else > - mov r1, #2 > - ldrh r0, [r4, #-2] @ Thumb instruction at LR - 2 > - cmp r0, #0xe800 @ 32-bit instruction if xx >= 0 > - blo __und_svc_fault > - ldrh r9, [r4] @ bottom 16 bits > - add r4, r4, #2 > - str r4, [sp, #S_PC] > - orr r0, r9, r0, lsl #16 > -#endif > - badr r9, __und_svc_finish > - mov r2, r4 > - bl call_fpe > > mov r1, #4 @ PC correction to apply > -__und_svc_fault: > + THUMB( tst r5, #PSR_T_BIT ) @ exception taken in Thumb mode? > + THUMB( movne r1, #2 ) @ if so, fix up PC correction > mov r0, sp @ struct pt_regs *regs > bl __und_fault Am I understanding correctly that when call_fpe was invoked previously, it was supposed to print extra debug info about the VFP state? But it didn't work properly for thumb mode, correct?
On Tue, 24 Nov 2020 at 23:59, Dmitry Osipenko <digetx@gmail.com> wrote: > > 18.11.2020 16:08, Ard Biesheuvel пишет: > > diff --git a/arch/arm/kernel/entry-armv.S b/arch/arm/kernel/entry-armv.S > > index c4220f51fcf3..0ea8529a4872 100644 > > --- a/arch/arm/kernel/entry-armv.S > > +++ b/arch/arm/kernel/entry-armv.S > > @@ -252,31 +252,10 @@ __und_svc: > > #else > > svc_entry > > #endif > > - @ > > - @ call emulation code, which returns using r9 if it has emulated > > - @ the instruction, or the more conventional lr if we are to treat > > - @ this as a real undefined instruction > > - @ > > - @ r0 - instruction > > - @ > > -#ifndef CONFIG_THUMB2_KERNEL > > - ldr r0, [r4, #-4] > > -#else > > - mov r1, #2 > > - ldrh r0, [r4, #-2] @ Thumb instruction at LR - 2 > > - cmp r0, #0xe800 @ 32-bit instruction if xx >= 0 > > - blo __und_svc_fault > > - ldrh r9, [r4] @ bottom 16 bits > > - add r4, r4, #2 > > - str r4, [sp, #S_PC] > > - orr r0, r9, r0, lsl #16 > > -#endif > > - badr r9, __und_svc_finish > > - mov r2, r4 > > - bl call_fpe > > > > mov r1, #4 @ PC correction to apply > > -__und_svc_fault: > > + THUMB( tst r5, #PSR_T_BIT ) @ exception taken in Thumb mode? > > + THUMB( movne r1, #2 ) @ if so, fix up PC correction > > mov r0, sp @ struct pt_regs *regs > > bl __und_fault > > Am I understanding correctly that when call_fpe was invoked previously, > it was supposed to print extra debug info about the VFP state? But it > didn't work properly for thumb mode, correct? call_fpe was originally called to perform emulation of any UNDEF instruction that matched the same constraints that also apply when the UNDEF is taken in user mode. If the UNDEF was triggered by a VFP/NEON instruction, the VFP handler would check for kernel or user mode, and trigger an error if the exception was taken in kernel mode. The Thumb mode opcode matching was wrong in this case, so if a Thumb2 NEON exception triggered an UNDEF exception in kernel mode (which only happens if there are bugs in the kernel) we would fail to identify it as a NEON instruction. This code removes the call into the emulation code from kernel mode entirely, as it no longer has valid users, and the invalid ones can simply be served by undef hooks.
25.11.2020 10:00, Ard Biesheuvel пишет: > On Tue, 24 Nov 2020 at 23:59, Dmitry Osipenko <digetx@gmail.com> wrote: >> >> 18.11.2020 16:08, Ard Biesheuvel пишет: >>> diff --git a/arch/arm/kernel/entry-armv.S b/arch/arm/kernel/entry-armv.S >>> index c4220f51fcf3..0ea8529a4872 100644 >>> --- a/arch/arm/kernel/entry-armv.S >>> +++ b/arch/arm/kernel/entry-armv.S >>> @@ -252,31 +252,10 @@ __und_svc: >>> #else >>> svc_entry >>> #endif >>> - @ >>> - @ call emulation code, which returns using r9 if it has emulated >>> - @ the instruction, or the more conventional lr if we are to treat >>> - @ this as a real undefined instruction >>> - @ >>> - @ r0 - instruction >>> - @ >>> -#ifndef CONFIG_THUMB2_KERNEL >>> - ldr r0, [r4, #-4] >>> -#else >>> - mov r1, #2 >>> - ldrh r0, [r4, #-2] @ Thumb instruction at LR - 2 >>> - cmp r0, #0xe800 @ 32-bit instruction if xx >= 0 >>> - blo __und_svc_fault >>> - ldrh r9, [r4] @ bottom 16 bits >>> - add r4, r4, #2 >>> - str r4, [sp, #S_PC] >>> - orr r0, r9, r0, lsl #16 >>> -#endif >>> - badr r9, __und_svc_finish >>> - mov r2, r4 >>> - bl call_fpe >>> >>> mov r1, #4 @ PC correction to apply >>> -__und_svc_fault: >>> + THUMB( tst r5, #PSR_T_BIT ) @ exception taken in Thumb mode? >>> + THUMB( movne r1, #2 ) @ if so, fix up PC correction >>> mov r0, sp @ struct pt_regs *regs >>> bl __und_fault >> >> Am I understanding correctly that when call_fpe was invoked previously, >> it was supposed to print extra debug info about the VFP state? But it >> didn't work properly for thumb mode, correct? > > call_fpe was originally called to perform emulation of any UNDEF > instruction that matched the same constraints that also apply when the > UNDEF is taken in user mode. If the UNDEF was triggered by a VFP/NEON > instruction, the VFP handler would check for kernel or user mode, and > trigger an error if the exception was taken in kernel mode. > > The Thumb mode opcode matching was wrong in this case, so if a Thumb2 > NEON exception triggered an UNDEF exception in kernel mode (which only > happens if there are bugs in the kernel) we would fail to identify it > as a NEON instruction. > > This code removes the call into the emulation code from kernel mode > entirely, as it no longer has valid users, and the invalid ones can > simply be served by undef hooks. > Thanks, that's a cleaner explanation in comparison to the commit message. Would be nice to have an improved commit message, although maybe not really worth the v3, either way: Reviwed-by: Dmitry Osipenko <digetx@gmail.com> Tested-by: Dmitry Osipenko <digetx@gmail.com>
diff --git a/arch/arm/kernel/entry-armv.S b/arch/arm/kernel/entry-armv.S index c4220f51fcf3..0ea8529a4872 100644 --- a/arch/arm/kernel/entry-armv.S +++ b/arch/arm/kernel/entry-armv.S @@ -252,31 +252,10 @@ __und_svc: #else svc_entry #endif - @ - @ call emulation code, which returns using r9 if it has emulated - @ the instruction, or the more conventional lr if we are to treat - @ this as a real undefined instruction - @ - @ r0 - instruction - @ -#ifndef CONFIG_THUMB2_KERNEL - ldr r0, [r4, #-4] -#else - mov r1, #2 - ldrh r0, [r4, #-2] @ Thumb instruction at LR - 2 - cmp r0, #0xe800 @ 32-bit instruction if xx >= 0 - blo __und_svc_fault - ldrh r9, [r4] @ bottom 16 bits - add r4, r4, #2 - str r4, [sp, #S_PC] - orr r0, r9, r0, lsl #16 -#endif - badr r9, __und_svc_finish - mov r2, r4 - bl call_fpe mov r1, #4 @ PC correction to apply -__und_svc_fault: + THUMB( tst r5, #PSR_T_BIT ) @ exception taken in Thumb mode? + THUMB( movne r1, #2 ) @ if so, fix up PC correction mov r0, sp @ struct pt_regs *regs bl __und_fault diff --git a/arch/arm/vfp/vfphw.S b/arch/arm/vfp/vfphw.S index 4fcff9f59947..d5837bf05a9a 100644 --- a/arch/arm/vfp/vfphw.S +++ b/arch/arm/vfp/vfphw.S @@ -79,11 +79,6 @@ ENTRY(vfp_support_entry) DBGSTR3 "instr %08x pc %08x state %p", r0, r2, r10 .fpu vfpv2 - ldr r3, [sp, #S_PSR] @ Neither lazy restore nor FP exceptions - and r3, r3, #MODE_MASK @ are supported in kernel mode - teq r3, #USR_MODE - bne vfp_kmode_exception @ Returns through lr - VFPFMRX r1, FPEXC @ Is the VFP enabled? DBGSTR1 "fpexc %08x", r1 tst r1, #FPEXC_EN diff --git a/arch/arm/vfp/vfpmodule.c b/arch/arm/vfp/vfpmodule.c index 8c9e7f9f0277..c3b6451c18bd 100644 --- a/arch/arm/vfp/vfpmodule.c +++ b/arch/arm/vfp/vfpmodule.c @@ -23,6 +23,7 @@ #include <asm/cputype.h> #include <asm/system_info.h> #include <asm/thread_notify.h> +#include <asm/traps.h> #include <asm/vfp.h> #include "vfpinstr.h" @@ -642,7 +643,9 @@ static int vfp_starting_cpu(unsigned int unused) return 0; } -void vfp_kmode_exception(void) +#ifdef CONFIG_KERNEL_MODE_NEON + +static int vfp_kmode_exception(struct pt_regs *regs, unsigned int instr) { /* * If we reach this point, a floating point exception has been raised @@ -660,9 +663,51 @@ void vfp_kmode_exception(void) pr_crit("BUG: unsupported FP instruction in kernel mode\n"); else pr_crit("BUG: FP instruction issued in kernel mode with FP unit disabled\n"); + pr_crit("FPEXC == 0x%08x\n", fmrx(FPEXC)); + return 1; } -#ifdef CONFIG_KERNEL_MODE_NEON +static struct undef_hook vfp_kmode_exception_hook[] = {{ + .instr_mask = 0xfe000000, + .instr_val = 0xf2000000, + .cpsr_mask = MODE_MASK | PSR_T_BIT, + .cpsr_val = SVC_MODE, + .fn = vfp_kmode_exception, +}, { + .instr_mask = 0xff100000, + .instr_val = 0xf4000000, + .cpsr_mask = MODE_MASK | PSR_T_BIT, + .cpsr_val = SVC_MODE, + .fn = vfp_kmode_exception, +}, { + .instr_mask = 0xef000000, + .instr_val = 0xef000000, + .cpsr_mask = MODE_MASK | PSR_T_BIT, + .cpsr_val = SVC_MODE | PSR_T_BIT, + .fn = vfp_kmode_exception, +}, { + .instr_mask = 0xff100000, + .instr_val = 0xf9000000, + .cpsr_mask = MODE_MASK | PSR_T_BIT, + .cpsr_val = SVC_MODE | PSR_T_BIT, + .fn = vfp_kmode_exception, +}, { + .instr_mask = 0x0c000e00, + .instr_val = 0x0c000a00, + .cpsr_mask = MODE_MASK, + .cpsr_val = SVC_MODE, + .fn = vfp_kmode_exception, +}}; + +static int __init vfp_kmode_exception_hook_init(void) +{ + int i; + + for (i = 0; i < ARRAY_SIZE(vfp_kmode_exception_hook); i++) + register_undef_hook(&vfp_kmode_exception_hook[i]); + return 0; +} +core_initcall(vfp_kmode_exception_hook_init); /* * Kernel-side NEON support functions
There are a couple of problems with the exception entry code that deals with FP exceptions (which are reported as UND exceptions) when building the kernel in Thumb2 mode: - the conditional branch to vfp_kmode_exception in vfp_support_entry() may be out of range for its target, depending on how the linker decides to arrange the sections; - when the UND exception is taken in kernel mode, the emulation handling logic is entered via the 'call_fpe' label, which means we end up using the wrong value/mask pairs to match and detect the NEON opcodes. Since UND exceptions in kernel mode are unlikely to occur on a hot path (as opposed to the user mode version which is invoked for VFP support code and lazy restore), we can use the existing undef hook machinery for any kernel mode instruction emulation that is needed, including calling the existing vfp_kmode_exception() routine for unexpected cases. So drop the call to call_fpe, and instead, install an undef hook that will get called for NEON and VFP instructions that trigger an UND exception in kernel mode. While at it, make sure that the PC correction is accurate for the execution mode where the exception was taken, by checking the PSR Thumb bit. Cc: Dmitry Osipenko <digetx@gmail.com> Cc: Kees Cook <keescook@chromium.org> Cc: Nick Desaulniers <ndesaulniers@google.com> Fixes: eff8728fe698 ("vmlinux.lds.h: Add PGO and AutoFDO input sections") Signed-off-by: Ard Biesheuvel <ardb@kernel.org> Reviewed-by: Linus Walleij <linus.walleij@linaro.org> --- NOTE: this supersedes 9018/2 "vfp: force non-conditional encoding for external Thumb2" which is currently queued in the patch system - the out-of-range branch to vfp_kmode_exception() is dropped entirely in this patch v2: - use the PSR T bit to select the right PC correction - add Linus's ack arch/arm/kernel/entry-armv.S | 25 +--------- arch/arm/vfp/vfphw.S | 5 -- arch/arm/vfp/vfpmodule.c | 49 +++++++++++++++++++- 3 files changed, 49 insertions(+), 30 deletions(-)