* [PATCH] ARM: entry: omit FP emulation for UND exceptions taken in kernel mode
@ 2020-10-26 14:35 Ard Biesheuvel
2020-10-26 17:04 ` Dmitry Osipenko
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Ard Biesheuvel @ 2020-10-26 14:35 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Kees Cook, Linus Walleij, Nick Desaulniers,
Russell King - ARM Linux admin, Dmitry Osipenko, Ard Biesheuvel
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.
Cc: Russell King - ARM Linux admin <linux@armlinux.org.uk>
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Dmitry Osipenko <digetx@gmail.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Nick Desaulniers <ndesaulniers@google.com>
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
Related discussion here:
https://lore.kernel.org/linux-arm-kernel/20201021225737.739-1-digetx@gmail.com/
Instead of installing the undef hook just to print the additional error
message, we might decide to simply drop that entirely, and rely on the
undefined exception splat to be sufficient to figure out what is going
on.
arch/arm/kernel/entry-armv.S | 23 +---------
arch/arm/vfp/vfphw.S | 5 --
arch/arm/vfp/vfpmodule.c | 48 +++++++++++++++++++-
3 files changed, 47 insertions(+), 29 deletions(-)
diff --git a/arch/arm/kernel/entry-armv.S b/arch/arm/kernel/entry-armv.S
index 55a47df04773..1bda8b57e0bb 100644
--- a/arch/arm/kernel/entry-armv.S
+++ b/arch/arm/kernel/entry-armv.S
@@ -252,35 +252,14 @@ __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]
+ mov r1, #4 @ PC correction to apply
#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:
mov r0, sp @ struct pt_regs *regs
bl __und_fault
-__und_svc_finish:
get_thread_info tsk
ldr r5, [sp, #S_PSR] @ Get SVC cpsr
svc_exit r5 @ return from exception
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..30d1f089f890 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,50 @@ 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");
+ 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
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] ARM: entry: omit FP emulation for UND exceptions taken in kernel mode
2020-10-26 14:35 [PATCH] ARM: entry: omit FP emulation for UND exceptions taken in kernel mode Ard Biesheuvel
@ 2020-10-26 17:04 ` Dmitry Osipenko
2020-10-26 17:10 ` Ard Biesheuvel
2020-10-26 18:12 ` Nick Desaulniers
2020-11-05 10:44 ` Linus Walleij
2 siblings, 1 reply; 6+ messages in thread
From: Dmitry Osipenko @ 2020-10-26 17:04 UTC (permalink / raw)
To: Ard Biesheuvel, linux-arm-kernel
Cc: Linus Walleij, Nick Desaulniers, Russell King - ARM Linux admin,
Kees Cook
26.10.2020 17:35, Ard Biesheuvel пишет:
> 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.
>
> Cc: Russell King - ARM Linux admin <linux@armlinux.org.uk>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Dmitry Osipenko <digetx@gmail.com>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Nick Desaulniers <ndesaulniers@google.com>
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> ---
> Related discussion here:
> https://lore.kernel.org/linux-arm-kernel/20201021225737.739-1-digetx@gmail.com/
I think yours original patch with the fixes tag is still needed, hence
it should be two patches: 1) fixes the original problem 2) makes the
improvement.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ARM: entry: omit FP emulation for UND exceptions taken in kernel mode
2020-10-26 17:04 ` Dmitry Osipenko
@ 2020-10-26 17:10 ` Ard Biesheuvel
0 siblings, 0 replies; 6+ messages in thread
From: Ard Biesheuvel @ 2020-10-26 17:10 UTC (permalink / raw)
To: Dmitry Osipenko
Cc: Linus Walleij, Nick Desaulniers, Russell King - ARM Linux admin,
Linux ARM, Kees Cook
On Mon, 26 Oct 2020 at 18:04, Dmitry Osipenko <digetx@gmail.com> wrote:
>
> 26.10.2020 17:35, Ard Biesheuvel пишет:
> > 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.
> >
> > Cc: Russell King - ARM Linux admin <linux@armlinux.org.uk>
> > Cc: Linus Walleij <linus.walleij@linaro.org>
> > Cc: Dmitry Osipenko <digetx@gmail.com>
> > Cc: Kees Cook <keescook@chromium.org>
> > Cc: Nick Desaulniers <ndesaulniers@google.com>
> > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> > ---
> > Related discussion here:
> > https://lore.kernel.org/linux-arm-kernel/20201021225737.739-1-digetx@gmail.com/
>
> I think yours original patch with the fixes tag is still needed, hence
> it should be two patches: 1) fixes the original problem 2) makes the
> improvement.
Russell was pretty clear that he doesn't want the additional IT
instruction on the hot path.
This patch removes the kernel mode check entirely, which is justified,
given that there is never a need to invoke the VFP support code from
kernel mode in the first place.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ARM: entry: omit FP emulation for UND exceptions taken in kernel mode
2020-10-26 14:35 [PATCH] ARM: entry: omit FP emulation for UND exceptions taken in kernel mode Ard Biesheuvel
2020-10-26 17:04 ` Dmitry Osipenko
@ 2020-10-26 18:12 ` Nick Desaulniers
2020-10-26 18:33 ` Ard Biesheuvel
2020-11-05 10:44 ` Linus Walleij
2 siblings, 1 reply; 6+ messages in thread
From: Nick Desaulniers @ 2020-10-26 18:12 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Dmitry Osipenko, Linus Walleij, Russell King - ARM Linux admin,
Linux ARM, Kees Cook
On Mon, Oct 26, 2020 at 7:37 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
> 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.
>
> Cc: Russell King - ARM Linux admin <linux@armlinux.org.uk>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Dmitry Osipenko <digetx@gmail.com>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Nick Desaulniers <ndesaulniers@google.com>
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> ---
> Related discussion here:
> https://lore.kernel.org/linux-arm-kernel/20201021225737.739-1-digetx@gmail.com/
>
> Instead of installing the undef hook just to print the additional error
> message, we might decide to simply drop that entirely, and rely on the
> undefined exception splat to be sufficient to figure out what is going
> on.
>
> arch/arm/kernel/entry-armv.S | 23 +---------
> arch/arm/vfp/vfphw.S | 5 --
> arch/arm/vfp/vfpmodule.c | 48 +++++++++++++++++++-
> 3 files changed, 47 insertions(+), 29 deletions(-)
>
> diff --git a/arch/arm/kernel/entry-armv.S b/arch/arm/kernel/entry-armv.S
> index 55a47df04773..1bda8b57e0bb 100644
> --- a/arch/arm/kernel/entry-armv.S
> +++ b/arch/arm/kernel/entry-armv.S
> @@ -252,35 +252,14 @@ __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]
> + mov r1, #4 @ PC correction to apply
> #else
> mov r1, #2
No comment on the issue the patch is addressing, just a minor
stylistic drive by comment.
Prefer:
if x:
foo()
else:
bar()
to:
if !x:
bar()
else:
foo()
> - 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:
> mov r0, sp @ struct pt_regs *regs
> bl __und_fault
>
> -__und_svc_finish:
> get_thread_info tsk
> ldr r5, [sp, #S_PSR] @ Get SVC cpsr
> svc_exit r5 @ return from exception
> 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..30d1f089f890 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,50 @@ 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");
> + 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
> --
> 2.17.1
>
--
Thanks,
~Nick Desaulniers
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ARM: entry: omit FP emulation for UND exceptions taken in kernel mode
2020-10-26 18:12 ` Nick Desaulniers
@ 2020-10-26 18:33 ` Ard Biesheuvel
0 siblings, 0 replies; 6+ messages in thread
From: Ard Biesheuvel @ 2020-10-26 18:33 UTC (permalink / raw)
To: Nick Desaulniers
Cc: Dmitry Osipenko, Linus Walleij, Russell King - ARM Linux admin,
Linux ARM, Kees Cook
On Mon, 26 Oct 2020 at 19:13, Nick Desaulniers <ndesaulniers@google.com> wrote:
>
> On Mon, Oct 26, 2020 at 7:37 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
> > 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.
> >
> > Cc: Russell King - ARM Linux admin <linux@armlinux.org.uk>
> > Cc: Linus Walleij <linus.walleij@linaro.org>
> > Cc: Dmitry Osipenko <digetx@gmail.com>
> > Cc: Kees Cook <keescook@chromium.org>
> > Cc: Nick Desaulniers <ndesaulniers@google.com>
> > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> > ---
> > Related discussion here:
> > https://lore.kernel.org/linux-arm-kernel/20201021225737.739-1-digetx@gmail.com/
> >
> > Instead of installing the undef hook just to print the additional error
> > message, we might decide to simply drop that entirely, and rely on the
> > undefined exception splat to be sufficient to figure out what is going
> > on.
> >
> > arch/arm/kernel/entry-armv.S | 23 +---------
> > arch/arm/vfp/vfphw.S | 5 --
> > arch/arm/vfp/vfpmodule.c | 48 +++++++++++++++++++-
> > 3 files changed, 47 insertions(+), 29 deletions(-)
> >
> > diff --git a/arch/arm/kernel/entry-armv.S b/arch/arm/kernel/entry-armv.S
> > index 55a47df04773..1bda8b57e0bb 100644
> > --- a/arch/arm/kernel/entry-armv.S
> > +++ b/arch/arm/kernel/entry-armv.S
> > @@ -252,35 +252,14 @@ __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]
> > + mov r1, #4 @ PC correction to apply
> > #else
> > mov r1, #2
>
> No comment on the issue the patch is addressing, just a minor
> stylistic drive by comment.
>
> Prefer:
> if x:
> foo()
> else:
> bar()
>
> to:
> if !x:
> bar()
> else:
> foo()
>
I disagree. Either could make sense, given the context. But more
importantly, this patch does not introduce the pattern, and therefore,
changing one into the other would make the patch, which is already
non-trivial, more complicated than necessary.
So you looked at the patch and had no comments, right? Can I take that
as an ack? Or were you only looking for minor stylistic drive-by
issues? :-)
> > - 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:
> > mov r0, sp @ struct pt_regs *regs
> > bl __und_fault
> >
> > -__und_svc_finish:
> > get_thread_info tsk
> > ldr r5, [sp, #S_PSR] @ Get SVC cpsr
> > svc_exit r5 @ return from exception
> > 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..30d1f089f890 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,50 @@ 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");
> > + 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
> > --
> > 2.17.1
> >
>
>
> --
> Thanks,
> ~Nick Desaulniers
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ARM: entry: omit FP emulation for UND exceptions taken in kernel mode
2020-10-26 14:35 [PATCH] ARM: entry: omit FP emulation for UND exceptions taken in kernel mode Ard Biesheuvel
2020-10-26 17:04 ` Dmitry Osipenko
2020-10-26 18:12 ` Nick Desaulniers
@ 2020-11-05 10:44 ` Linus Walleij
2 siblings, 0 replies; 6+ messages in thread
From: Linus Walleij @ 2020-11-05 10:44 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Dmitry Osipenko, Nick Desaulniers,
Russell King - ARM Linux admin, Linux ARM, Kees Cook
On Mon, Oct 26, 2020 at 3:37 PM 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
> 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.
>
> Cc: Russell King - ARM Linux admin <linux@armlinux.org.uk>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Dmitry Osipenko <digetx@gmail.com>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Nick Desaulniers <ndesaulniers@google.com>
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> ---
> Related discussion here:
> https://lore.kernel.org/linux-arm-kernel/20201021225737.739-1-digetx@gmail.com/
I would put that with Link: in the commit message so people
can find it easily if they have a problem here.
FWIW:
Reviewed-by: Linus Walleij <iinus.walleij@linaro.org>
I can't claim to thoroughly understand it, but I roughly understand
it. Back to studies...
One sideeffect of moving to the generic hook makes this available
at core_initcall() time so later during boot IIUC, which may be worth
noting in the commit message as well? Indeed if we have
an exception like this during that early boot we will certainly
notice anyway, as long as it gives some kind of splat an not
just hang, I'm happy.
Yours,
Linus Walleij
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-11-05 10:45 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-26 14:35 [PATCH] ARM: entry: omit FP emulation for UND exceptions taken in kernel mode Ard Biesheuvel
2020-10-26 17:04 ` Dmitry Osipenko
2020-10-26 17:10 ` Ard Biesheuvel
2020-10-26 18:12 ` Nick Desaulniers
2020-10-26 18:33 ` Ard Biesheuvel
2020-11-05 10:44 ` Linus Walleij
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.