[PATCH v2] ARM: entry: omit FP emulation for UND exceptions taken in kernel mode
Ard Biesheuvel
ardb at kernel.org
Wed Nov 18 11:47:56 EST 2020
On Wed, 18 Nov 2020 at 17:42, Nick Desaulniers <ndesaulniers at google.com> wrote:
>
> On Wed, Nov 18, 2020 at 5:09 AM Ard Biesheuvel <ardb at 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 at gmail.com>
> > Cc: Kees Cook <keescook at chromium.org>
> > Cc: Nick Desaulniers <ndesaulniers at google.com>
> > Fixes: eff8728fe698 ("vmlinux.lds.h: Add PGO and AutoFDO input sections")
> > Signed-off-by: Ard Biesheuvel <ardb at kernel.org>
> > Reviewed-by: Linus Walleij <linus.walleij at 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
More information about the linux-arm-kernel
mailing list