| Message ID | 20230330055040.434133-5-bgray@linux.ibm.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add static DEXCR support | expand |
On Thu, 2023-03-30 at 16:50 +1100, Benjamin Gray wrote: > Recognise and pass the appropriate signal to the user program when a > hashchk instruction triggers. This is independent of allowing > configuration of DEXCR[NPHIE], as a hypervisor can enforce this > aspect > regardless of the kernel. > > The signal mirrors how ARM reports their similar check failure. For > example, their FPAC handler in arch/arm64/kernel/traps.c > do_el0_fpac() > does this. When we fail to read the instruction that caused the fault > we send a segfault, similar to how emulate_math() does it. > > Signed-off-by: Benjamin Gray <bgray@linux.ibm.com> > > --- > > v1: * Refactor the hashchk check to return 0 on success, an error > code on failure. Determine what to do based on specific > error > code. 0 on success makes sense, but it's not exactly obvious what "success" is in this case. > * Motivate signal and code > --- > arch/powerpc/include/asm/ppc-opcode.h | 1 + > arch/powerpc/include/asm/processor.h | 9 +++++++ > arch/powerpc/kernel/Makefile | 1 + > arch/powerpc/kernel/dexcr.c | 36 > +++++++++++++++++++++++++++ > arch/powerpc/kernel/traps.c | 10 ++++++++ > 5 files changed, 57 insertions(+) > create mode 100644 arch/powerpc/kernel/dexcr.c > > diff --git a/arch/powerpc/include/asm/ppc-opcode.h > b/arch/powerpc/include/asm/ppc-opcode.h > index 21e33e46f4b8..89b316466ed1 100644 > --- a/arch/powerpc/include/asm/ppc-opcode.h > +++ b/arch/powerpc/include/asm/ppc-opcode.h > @@ -215,6 +215,7 @@ > #define OP_31_XOP_STFSX 663 > #define OP_31_XOP_STFSUX 695 > #define OP_31_XOP_STFDX 727 > +#define OP_31_XOP_HASHCHK 754 > #define OP_31_XOP_STFDUX 759 > #define OP_31_XOP_LHBRX 790 > #define OP_31_XOP_LFIWAX 855 > diff --git a/arch/powerpc/include/asm/processor.h > b/arch/powerpc/include/asm/processor.h > index e96c9b8c2a60..bad64d6a5d36 100644 > --- a/arch/powerpc/include/asm/processor.h > +++ b/arch/powerpc/include/asm/processor.h > @@ -455,6 +455,15 @@ int exit_vmx_usercopy(void); > int enter_vmx_ops(void); > void *exit_vmx_ops(void *dest); > > +#ifdef CONFIG_PPC_BOOK3S_64 > +int check_hashchk_trap(struct pt_regs const *regs); > +#else > +static inline int check_hashchk_trap(struct pt_regs const *regs) > +{ > + return -EINVAL; > +} > +#endif /* CONFIG_PPC_BOOK3S_64 */ > + > #endif /* __KERNEL__ */ > #endif /* __ASSEMBLY__ */ > #endif /* _ASM_POWERPC_PROCESSOR_H */ > diff --git a/arch/powerpc/kernel/Makefile > b/arch/powerpc/kernel/Makefile > index 9bf2be123093..07181e508754 100644 > --- a/arch/powerpc/kernel/Makefile > +++ b/arch/powerpc/kernel/Makefile > @@ -88,6 +88,7 @@ obj-$(CONFIG_HAVE_HW_BREAKPOINT) += > hw_breakpoint.o > obj-$(CONFIG_PPC_DAWR) += dawr.o > obj-$(CONFIG_PPC_BOOK3S_64) += cpu_setup_ppc970.o > cpu_setup_pa6t.o > obj-$(CONFIG_PPC_BOOK3S_64) += cpu_setup_power.o > +obj-$(CONFIG_PPC_BOOK3S_64) += dexcr.o > obj-$(CONFIG_PPC_BOOK3S_64) += mce.o mce_power.o > obj-$(CONFIG_PPC_BOOK3E_64) += exceptions-64e.o idle_64e.o > obj-$(CONFIG_PPC_BARRIER_NOSPEC) += security.o > diff --git a/arch/powerpc/kernel/dexcr.c > b/arch/powerpc/kernel/dexcr.c > new file mode 100644 > index 000000000000..f263e5439cc6 > --- /dev/null > +++ b/arch/powerpc/kernel/dexcr.c > @@ -0,0 +1,36 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * DEXCR infrastructure May as well spell out DEXCR here > + * > + * Copyright 2023, Benjamin Gray, IBM Corporation. > + */ > +#include <linux/compiler_types.h> > + > +#include <asm/cpu_has_feature.h> > +#include <asm/cputable.h> > +#include <asm/disassemble.h> > +#include <asm/errno.h> > +#include <asm/inst.h> > +#include <asm/ppc-opcode.h> > +#include <asm/ptrace.h> > +#include <asm/reg.h> > + > +int check_hashchk_trap(struct pt_regs const *regs) > +{ > + ppc_inst_t insn; > + > + if (!cpu_has_feature(CPU_FTR_DEXCR_NPHIE)) > + return -EINVAL; > + > + if (!user_mode(regs)) > + return -EINVAL; > + > + if (get_user_instr(insn, (void __user *)regs->nip)) > + return -EFAULT; > + > + if (ppc_inst_primary_opcode(insn) != 31 || > + get_xop(ppc_inst_val(insn)) != OP_31_XOP_HASHCHK) > + return -EINVAL; > + > + return 0; > +} The return values here are quite confusing and only start to make sense in the context of the calling function, which isn't great for something living in a different file. So if we return 0 we SIGILL because of hashchk, return -EFAULT to SEGV_MAPERR and return -EINVAL to fall through to SIGTRAP. I would like a comment for that since it's not very intuitive. > diff --git a/arch/powerpc/kernel/traps.c > b/arch/powerpc/kernel/traps.c > index 9bdd79aa51cf..ade67e23b974 100644 > --- a/arch/powerpc/kernel/traps.c > +++ b/arch/powerpc/kernel/traps.c > @@ -1516,6 +1516,16 @@ static void do_program_check(struct pt_regs > *regs) > return; > } > } > + > + switch (check_hashchk_trap(regs)) { > + case 0: > + _exception(SIGILL, regs, ILL_ILLOPN, regs- > >nip); > + return; > + case -EFAULT: > + _exception(SIGSEGV, regs, SEGV_MAPERR, regs- > >nip); > + return; > + } > + I offer no constructive feedback on how to rearrange it, but this just looks gross. > _exception(SIGTRAP, regs, TRAP_BRKPT, regs->nip); > return; > }
diff --git a/arch/powerpc/include/asm/ppc-opcode.h b/arch/powerpc/include/asm/ppc-opcode.h index 21e33e46f4b8..89b316466ed1 100644 --- a/arch/powerpc/include/asm/ppc-opcode.h +++ b/arch/powerpc/include/asm/ppc-opcode.h @@ -215,6 +215,7 @@ #define OP_31_XOP_STFSX 663 #define OP_31_XOP_STFSUX 695 #define OP_31_XOP_STFDX 727 +#define OP_31_XOP_HASHCHK 754 #define OP_31_XOP_STFDUX 759 #define OP_31_XOP_LHBRX 790 #define OP_31_XOP_LFIWAX 855 diff --git a/arch/powerpc/include/asm/processor.h b/arch/powerpc/include/asm/processor.h index e96c9b8c2a60..bad64d6a5d36 100644 --- a/arch/powerpc/include/asm/processor.h +++ b/arch/powerpc/include/asm/processor.h @@ -455,6 +455,15 @@ int exit_vmx_usercopy(void); int enter_vmx_ops(void); void *exit_vmx_ops(void *dest); +#ifdef CONFIG_PPC_BOOK3S_64 +int check_hashchk_trap(struct pt_regs const *regs); +#else +static inline int check_hashchk_trap(struct pt_regs const *regs) +{ + return -EINVAL; +} +#endif /* CONFIG_PPC_BOOK3S_64 */ + #endif /* __KERNEL__ */ #endif /* __ASSEMBLY__ */ #endif /* _ASM_POWERPC_PROCESSOR_H */ diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile index 9bf2be123093..07181e508754 100644 --- a/arch/powerpc/kernel/Makefile +++ b/arch/powerpc/kernel/Makefile @@ -88,6 +88,7 @@ obj-$(CONFIG_HAVE_HW_BREAKPOINT) += hw_breakpoint.o obj-$(CONFIG_PPC_DAWR) += dawr.o obj-$(CONFIG_PPC_BOOK3S_64) += cpu_setup_ppc970.o cpu_setup_pa6t.o obj-$(CONFIG_PPC_BOOK3S_64) += cpu_setup_power.o +obj-$(CONFIG_PPC_BOOK3S_64) += dexcr.o obj-$(CONFIG_PPC_BOOK3S_64) += mce.o mce_power.o obj-$(CONFIG_PPC_BOOK3E_64) += exceptions-64e.o idle_64e.o obj-$(CONFIG_PPC_BARRIER_NOSPEC) += security.o diff --git a/arch/powerpc/kernel/dexcr.c b/arch/powerpc/kernel/dexcr.c new file mode 100644 index 000000000000..f263e5439cc6 --- /dev/null +++ b/arch/powerpc/kernel/dexcr.c @@ -0,0 +1,36 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * DEXCR infrastructure + * + * Copyright 2023, Benjamin Gray, IBM Corporation. + */ +#include <linux/compiler_types.h> + +#include <asm/cpu_has_feature.h> +#include <asm/cputable.h> +#include <asm/disassemble.h> +#include <asm/errno.h> +#include <asm/inst.h> +#include <asm/ppc-opcode.h> +#include <asm/ptrace.h> +#include <asm/reg.h> + +int check_hashchk_trap(struct pt_regs const *regs) +{ + ppc_inst_t insn; + + if (!cpu_has_feature(CPU_FTR_DEXCR_NPHIE)) + return -EINVAL; + + if (!user_mode(regs)) + return -EINVAL; + + if (get_user_instr(insn, (void __user *)regs->nip)) + return -EFAULT; + + if (ppc_inst_primary_opcode(insn) != 31 || + get_xop(ppc_inst_val(insn)) != OP_31_XOP_HASHCHK) + return -EINVAL; + + return 0; +} diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c index 9bdd79aa51cf..ade67e23b974 100644 --- a/arch/powerpc/kernel/traps.c +++ b/arch/powerpc/kernel/traps.c @@ -1516,6 +1516,16 @@ static void do_program_check(struct pt_regs *regs) return; } } + + switch (check_hashchk_trap(regs)) { + case 0: + _exception(SIGILL, regs, ILL_ILLOPN, regs->nip); + return; + case -EFAULT: + _exception(SIGSEGV, regs, SEGV_MAPERR, regs->nip); + return; + } + _exception(SIGTRAP, regs, TRAP_BRKPT, regs->nip); return; }
Recognise and pass the appropriate signal to the user program when a hashchk instruction triggers. This is independent of allowing configuration of DEXCR[NPHIE], as a hypervisor can enforce this aspect regardless of the kernel. The signal mirrors how ARM reports their similar check failure. For example, their FPAC handler in arch/arm64/kernel/traps.c do_el0_fpac() does this. When we fail to read the instruction that caused the fault we send a segfault, similar to how emulate_math() does it. Signed-off-by: Benjamin Gray <bgray@linux.ibm.com> --- v1: * Refactor the hashchk check to return 0 on success, an error code on failure. Determine what to do based on specific error code. * Motivate signal and code --- arch/powerpc/include/asm/ppc-opcode.h | 1 + arch/powerpc/include/asm/processor.h | 9 +++++++ arch/powerpc/kernel/Makefile | 1 + arch/powerpc/kernel/dexcr.c | 36 +++++++++++++++++++++++++++ arch/powerpc/kernel/traps.c | 10 ++++++++ 5 files changed, 57 insertions(+) create mode 100644 arch/powerpc/kernel/dexcr.c