From: Benjamin Gray <hidden> Date: 2022-11-28 02:46:26
This series is based on initial work by Chris Riedl that was not sent
to the list.
Adds a kernel interface for userspace to interact with the DEXCR.
The DEXCR is a SPR that allows control over various execution
'aspects', such as indirect branch prediction and enabling the
hashst/hashchk instructions. Further details are in ISA 3.1B
Book 3 chapter 12.
This RFC proposes an interface for users to interact with the DEXCR.
It aims to support
* Querying supported aspects
* Getting/setting aspects on a per-process level
* Allowing global overrides across all processes
There are some parts that I'm not sure on the best way to approach (hence RFC):
* The feature names in arch/powerpc/kernel/dt_cpu_ftrs.c appear to be unimplemented
in skiboot, so are being defined by this series. Is being so verbose fine?
* What aspects should be editable by a process? E.g., SBHE has
effects that potentially bleed into other processes. Should
it only be system wide configurable?
* Should configuring certain aspects for the process be non-privileged? E.g.,
Is there harm in always allowing configuration of IBRTPD, SRAPD? The *FORCE_SET*
action prevents further process local changes regardless of privilege.
* The tests fail Patchwork CI because of the new prctl macros, and the CI
doesn't run headers_install and add -isystem <buildpath>/usr/include to
the make command.
* On handling an exception, I don't check if the NPHIE bit is enabled in the DEXCR.
To do so would require reading both the DEXCR and HDEXCR, for little gain (it
should only matter that the current instruction was a hashchk. If so, the only
reason it would cause an exception is the failed check. If the instruction is
rewritten between exception and check we'd be wrong anyway).
The series is based on the earlier selftest utils series[1], so the tests won't build
at all without applying that first. The kernel side should build fine on ppc/next
247f34f7b80357943234f93f247a1ae6b6c3a740 though.
[1]: https://patchwork.ozlabs.org/project/linuxppc-dev/cover/20221122231103.15829-1-bgray@linux.ibm.com/
Benjamin Gray (13):
powerpc/book3s: Add missing <linux/sched.h> include
powerpc: Add initial Dynamic Execution Control Register (DEXCR)
support
powerpc/dexcr: Handle hashchk exception
powerpc/dexcr: Support userspace ROP protection
prctl: Define PowerPC DEXCR interface
powerpc/dexcr: Add prctl implementation
powerpc/dexcr: Add sysctl entry for SBHE system override
powerpc/dexcr: Add enforced userspace ROP protection config
selftests/powerpc: Add more utility macros
selftests/powerpc: Add hashst/hashchk test
selftests/powerpc: Add DEXCR prctl, sysctl interface test
selftests/powerpc: Add DEXCR status utility lsdexcr
Documentation: Document PowerPC kernel DEXCR interface
Documentation/powerpc/dexcr.rst | 183 +++++++++++
Documentation/powerpc/index.rst | 1 +
arch/powerpc/Kconfig | 5 +
arch/powerpc/include/asm/book3s/64/kexec.h | 6 +
arch/powerpc/include/asm/book3s/64/kup.h | 1 +
arch/powerpc/include/asm/cputable.h | 8 +-
arch/powerpc/include/asm/ppc-opcode.h | 1 +
arch/powerpc/include/asm/processor.h | 33 ++
arch/powerpc/include/asm/reg.h | 7 +
arch/powerpc/kernel/Makefile | 1 +
arch/powerpc/kernel/dexcr.c | 310 ++++++++++++++++++
arch/powerpc/kernel/dt_cpu_ftrs.c | 4 +
arch/powerpc/kernel/process.c | 31 +-
arch/powerpc/kernel/prom.c | 4 +
arch/powerpc/kernel/traps.c | 6 +
include/uapi/linux/prctl.h | 14 +
kernel/sys.c | 16 +
tools/testing/selftests/powerpc/Makefile | 1 +
.../selftests/powerpc/dexcr/.gitignore | 3 +
.../testing/selftests/powerpc/dexcr/Makefile | 11 +
tools/testing/selftests/powerpc/dexcr/cap.c | 72 ++++
tools/testing/selftests/powerpc/dexcr/cap.h | 18 +
tools/testing/selftests/powerpc/dexcr/dexcr.c | 118 +++++++
tools/testing/selftests/powerpc/dexcr/dexcr.h | 54 +++
.../selftests/powerpc/dexcr/dexcr_test.c | 241 ++++++++++++++
.../selftests/powerpc/dexcr/hashchk_test.c | 229 +++++++++++++
.../testing/selftests/powerpc/dexcr/lsdexcr.c | 178 ++++++++++
tools/testing/selftests/powerpc/include/reg.h | 4 +
.../testing/selftests/powerpc/include/utils.h | 44 +++
29 files changed, 1602 insertions(+), 2 deletions(-)
create mode 100644 Documentation/powerpc/dexcr.rst
create mode 100644 arch/powerpc/kernel/dexcr.c
create mode 100644 tools/testing/selftests/powerpc/dexcr/.gitignore
create mode 100644 tools/testing/selftests/powerpc/dexcr/Makefile
create mode 100644 tools/testing/selftests/powerpc/dexcr/cap.c
create mode 100644 tools/testing/selftests/powerpc/dexcr/cap.h
create mode 100644 tools/testing/selftests/powerpc/dexcr/dexcr.c
create mode 100644 tools/testing/selftests/powerpc/dexcr/dexcr.h
create mode 100644 tools/testing/selftests/powerpc/dexcr/dexcr_test.c
create mode 100644 tools/testing/selftests/powerpc/dexcr/hashchk_test.c
create mode 100644 tools/testing/selftests/powerpc/dexcr/lsdexcr.c
base-commit: 9dc58a6040662faaf24c8932861f485670fce7ff
--
2.38.1
From: Benjamin Gray <hidden> Date: 2022-11-28 02:46:05
The ISA 3.1B hashst and hashchk instructions use a per-cpu SPR HASHKEYR
to hold a key used in the hash calculation. This key should be different
for each process to make it harder for a malicious process to recreate
valid hash values for a victim process.
Add support for storing a per-thread hash key, and setting/clearing
HASHKEYR appropriately.
Signed-off-by: Benjamin Gray <redacted>
---
arch/powerpc/include/asm/book3s/64/kexec.h | 3 +++
arch/powerpc/include/asm/processor.h | 1 +
arch/powerpc/include/asm/reg.h | 1 +
arch/powerpc/kernel/process.c | 12 ++++++++++++
4 files changed, 17 insertions(+)
@@ -24,6 +24,9 @@ static inline void reset_sprs(void)if(cpu_has_feature(CPU_FTR_ARCH_31))mtspr(SPRN_DEXCR,0);+if(cpu_has_feature(CPU_FTR_DEXCR_NPHIE))+mtspr(SPRN_HASHKEYR,0);+/* Do we need isync()? We are going via a kexec reset */isync();}
From: Benjamin Gray <hidden> Date: 2022-11-28 02:46:07
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.
Signed-off-by: Benjamin Gray <redacted>
---
arch/powerpc/include/asm/ppc-opcode.h | 1 +
arch/powerpc/include/asm/processor.h | 6 ++++++
arch/powerpc/kernel/dexcr.c | 22 ++++++++++++++++++++++
arch/powerpc/kernel/traps.c | 6 ++++++
4 files changed, 35 insertions(+)
From: Benjamin Gray <hidden> Date: 2022-11-28 02:46:12
The functions here use struct thread_struct fields, so need to import
the full definition from <linux/sched.h>. The <asm/current.h> header
that defines current only forward declares struct thread_struct.
Failing to include this <linux/sched.h> header leads to a compilation
error when a translation unit does not also include <linux/sched.h>
indirectly.
Signed-off-by: Benjamin Gray <redacted>
---
arch/powerpc/include/asm/book3s/64/kup.h | 1 +
1 file changed, 1 insertion(+)
From: Benjamin Gray <hidden> Date: 2022-11-28 02:46:15
The DEXCR Non-Privileged Hash Instruction Enable (NPHIE) aspect controls
whether the hashst and hashchk instructions are treated as no-ops by the
CPU.
NPHIE behaviour per ISA 3.1B:
0: hashst and hashchk instructions are executed as no-ops
(even when allowed by PCR)
1: hashst and hashchk instructions are executed normally
(if allowed by PCR)
Currently this aspect may be set per-process by prctl() or enforced
globally by the hypervisor.
Add a kernel config option PPC_USER_ROP_PROTECT to enforce DEXCR[NPHIE]
globally regardless of prctl() or hypervisor. If set, don't report
NPHIE as editable via prctl(), as the prctl() value can never take
effect.
Signed-off-by: Benjamin Gray <redacted>
---
arch/powerpc/Kconfig | 5 +++++
arch/powerpc/kernel/dexcr.c | 15 +++++++++++----
2 files changed, 16 insertions(+), 4 deletions(-)
@@ -18,8 +19,8 @@#define DEFAULT_DEXCR 0/* Allow process configuration of these by default */-#define DEXCR_PRCTL_EDITABLE (DEXCR_PRO_SBHE | DEXCR_PRO_IBRTPD | \-DEXCR_PRO_SRAPD|DEXCR_PRO_NPHIE)+staticunsignedlongdexcr_prctl_editable__ro_after_init=+DEXCR_PRO_SBHE|DEXCR_PRO_IBRTPD|DEXCR_PRO_SRAPD|DEXCR_PRO_NPHIE;/**LocktoprotectsystemDEXCRoverridefromconcurrentupdates.
@@ -83,6 +84,12 @@ static int __init dexcr_init(void)if(early_cpu_has_feature(CPU_FTR_DEXCR_SBHE))update_userspace_system_dexcr(DEXCR_PRO_SBHE,spec_branch_hint_enable);+if(early_cpu_has_feature(CPU_FTR_DEXCR_NPHIE)&&+IS_ENABLED(CONFIG_PPC_USER_ROP_PROTECT)){+update_userspace_system_dexcr(DEXCR_PRO_NPHIE,1);+dexcr_prctl_editable&=~DEXCR_PRO_NPHIE;+}+return0;}early_initcall(dexcr_init);
@@ -131,7 +138,7 @@ static int dexcr_aspect_get(struct task_struct *task, unsigned int aspect){intret=0;-if(aspect&DEXCR_PRCTL_EDITABLE)+if(aspect&dexcr_prctl_editable)ret|=PR_PPC_DEXCR_PRCTL;if(aspect&task->thread.dexcr_mask){
@@ -174,7 +181,7 @@ int dexcr_prctl_get(struct task_struct *task, unsigned long which)staticintdexcr_aspect_set(structtask_struct*task,unsignedintaspect,unsignedlongctrl){-if(!(aspect&DEXCR_PRCTL_EDITABLE))+if(!(aspect&dexcr_prctl_editable))return-ENXIO;/* Aspect is not allowed to be changed by prctl */if(aspect&task->thread.dexcr_forced)
From: Benjamin Gray <hidden> Date: 2022-11-28 02:46:21
Adds more assertion variants to provide more context behind why a
failure occurred.
The SIGSAFE_FAIL_* variants are to allow safely asserting conditions
in a signal handler (though we are about to exit, so it's unlikely to
run into an issue with regular FAIL_IF_EXIT).
Also adds an ARRAY_SIZE macro.
These will be used by the following DEXCR selftests.
Signed-off-by: Benjamin Gray <redacted>
---
.../testing/selftests/powerpc/include/utils.h | 44 +++++++++++++++++++
1 file changed, 44 insertions(+)
@@ -9,12 +9,19 @@#define __cacheline_aligned __attribute__((aligned(128)))#include<stdint.h>+#include<stdio.h>#include<stdbool.h>+#include<string.h>+#include<unistd.h>#include<linux/auxvec.h>#include<linux/perf_event.h>#include<asm/cputable.h>#include"reg.h"+#ifndef ARRAY_SIZE+# define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))+#endif+/* Avoid headaches with PRI?64 - just use %ll? always */typedefunsignedlonglongu64;typedefsignedlonglongs64;
@@ -111,6 +118,16 @@ do { \}\}while(0)+#define FAIL_IF_MSG(x, msg) \+do{\+if((x)){\+fprintf(stderr,\+"[FAIL] Test FAILED on line %d: %s\n",\+__LINE__,msg);\+return1;\+}\+}while(0)+#define FAIL_IF_EXIT(x) \do{\if((x)){\
@@ -120,6 +137,16 @@ do { \}\}while(0)+#define FAIL_IF_EXIT_MSG(x, msg) \+do{\+if((x)){\+fprintf(stderr,\+"[FAIL] Test FAILED on line %d: %s\n",\+__LINE__,msg);\+_exit(1);\+}\+}while(0)+/* The test harness uses this, yes it's gross */#define MAGIC_SKIP_RETURN_VALUE 99
@@ -149,6 +176,23 @@ do { \ssize_tnbytes__attribute__((unused));\nbytes=write(STDERR_FILENO,msg,strlen(msg));})+#define SIGSAFE_FAIL_IF_EXIT(x) \+do{\+if((x)){\+sigsafe_err("[FAIL] Test FAILED on line "str(__LINE__)"\n");\+_exit(1);\+}\+}while(0)++#define SIGSAFE_FAIL_IF_EXIT_MSG(x, msg) \+do{\+if((x)){\+sigsafe_err("[FAIL] Test FAILED on line "\+str(__LINE__)": "msg"\n");\+_exit(1);\+}\+}while(0)+/* POWER9 feature */#ifndef PPC_FEATURE2_ARCH_3_00#define PPC_FEATURE2_ARCH_3_00 0x00800000
From: Benjamin Gray <hidden> Date: 2022-11-28 02:46:23
Adds an initial prctl interface implementation. Unprivileged processes
can query the current prctl setting, including whether an aspect is
implemented by the hardware or is permitted to be modified by a setter
prctl. Editable aspects can be changed by a CAP_SYS_ADMIN privileged
process.
The prctl setting represents what the process itself has requested, and
does not account for any overrides. Either the kernel or a hypervisor
may enforce a different setting for an aspect.
Userspace can access a readonly view of the current DEXCR via SPR 812,
and a readonly view of the aspects enforced by the hypervisor via
SPR 455. A bitwise OR of these two SPRs will give the effective
DEXCR aspect state of the process.
Signed-off-by: Benjamin Gray <redacted>
---
arch/powerpc/include/asm/processor.h | 13 +++
arch/powerpc/kernel/dexcr.c | 133 ++++++++++++++++++++++++++-
arch/powerpc/kernel/process.c | 6 ++
3 files changed, 151 insertions(+), 1 deletion(-)
@@ -11,6 +14,10 @@#define DEFAULT_DEXCR 0+/* Allow process configuration of these by default */+#define DEXCR_PRCTL_EDITABLE (DEXCR_PRO_SBHE | DEXCR_PRO_IBRTPD | \+DEXCR_PRO_SRAPD|DEXCR_PRO_NPHIE)+staticint__initdexcr_init(void){if(!early_cpu_has_feature(CPU_FTR_ARCH_31))
@@ -43,5 +50,129 @@ bool is_hashchk_trap(struct pt_regs const *regs)unsignedlongget_thread_dexcr(structthread_structconst*t){-returnDEFAULT_DEXCR;+unsignedlongdexcr=DEFAULT_DEXCR;++/* Apply prctl overrides */+dexcr=(dexcr&~t->dexcr_mask)|t->dexcr_override;++returndexcr;+}++staticvoidupdate_dexcr_on_cpu(void*info)+{+mtspr(SPRN_DEXCR,get_thread_dexcr(¤t->thread));+}++staticintdexcr_aspect_get(structtask_struct*task,unsignedintaspect)+{+intret=0;++if(aspect&DEXCR_PRCTL_EDITABLE)+ret|=PR_PPC_DEXCR_PRCTL;++if(aspect&task->thread.dexcr_mask){+if(aspect&task->thread.dexcr_override){+if(aspect&task->thread.dexcr_forced)+ret|=PR_PPC_DEXCR_FORCE_SET_ASPECT;+else+ret|=PR_PPC_DEXCR_SET_ASPECT;+}else{+ret|=PR_PPC_DEXCR_CLEAR_ASPECT;+}+}++returnret;+}++intdexcr_prctl_get(structtask_struct*task,unsignedlongwhich)+{+switch(which){+casePR_PPC_DEXCR_SBHE:+if(!cpu_has_feature(CPU_FTR_DEXCR_SBHE))+return-ENODEV;+returndexcr_aspect_get(task,DEXCR_PRO_SBHE);+casePR_PPC_DEXCR_IBRTPD:+if(!cpu_has_feature(CPU_FTR_DEXCR_IBRTPD))+return-ENODEV;+returndexcr_aspect_get(task,DEXCR_PRO_IBRTPD);+casePR_PPC_DEXCR_SRAPD:+if(!cpu_has_feature(CPU_FTR_DEXCR_SRAPD))+return-ENODEV;+returndexcr_aspect_get(task,DEXCR_PRO_SRAPD);+casePR_PPC_DEXCR_NPHIE:+if(!cpu_has_feature(CPU_FTR_DEXCR_NPHIE))+return-ENODEV;+returndexcr_aspect_get(task,DEXCR_PRO_NPHIE);+default:+return-ENODEV;+}+}++staticintdexcr_aspect_set(structtask_struct*task,unsignedintaspect,unsignedlongctrl)+{+if(!(aspect&DEXCR_PRCTL_EDITABLE))+return-ENXIO;/* Aspect is not allowed to be changed by prctl */++if(aspect&task->thread.dexcr_forced)+return-EPERM;/* Aspect has been forced to current state */++switch(ctrl){+casePR_PPC_DEXCR_SET_ASPECT:+task->thread.dexcr_mask|=aspect;+task->thread.dexcr_override|=aspect;+break;+casePR_PPC_DEXCR_FORCE_SET_ASPECT:+task->thread.dexcr_mask|=aspect;+task->thread.dexcr_override|=aspect;+task->thread.dexcr_forced|=aspect;+break;+casePR_PPC_DEXCR_CLEAR_ASPECT:+task->thread.dexcr_mask|=aspect;+task->thread.dexcr_override&=~aspect;+break;+default:+return-ERANGE;+}++return0;+}++intdexcr_prctl_set(structtask_struct*task,unsignedlongwhich,unsignedlongctrl)+{+interr=0;++if(!capable(CAP_SYS_ADMIN))+return-EPERM;++switch(which){+casePR_PPC_DEXCR_SBHE:+if(!cpu_has_feature(CPU_FTR_DEXCR_SBHE))+return-ENODEV;+err=dexcr_aspect_set(task,DEXCR_PRO_SBHE,ctrl);+break;+casePR_PPC_DEXCR_IBRTPD:+if(!cpu_has_feature(CPU_FTR_DEXCR_IBRTPD))+return-ENODEV;+err=dexcr_aspect_set(task,DEXCR_PRO_IBRTPD,ctrl);+break;+casePR_PPC_DEXCR_SRAPD:+if(!cpu_has_feature(CPU_FTR_DEXCR_SRAPD))+return-ENODEV;+err=dexcr_aspect_set(task,DEXCR_PRO_SRAPD,ctrl);+break;+casePR_PPC_DEXCR_NPHIE:+if(!cpu_has_feature(CPU_FTR_DEXCR_NPHIE))+return-ENODEV;+err=dexcr_aspect_set(task,DEXCR_PRO_NPHIE,ctrl);+break;+default:+return-ENODEV;+}++if(err)+returnerr;++update_dexcr_on_cpu(NULL);++return0;}
From: Benjamin Gray <hidden> Date: 2022-11-28 02:46:33
Test the prctl and sysctl interfaces of the DEXCR.
This adds a new capabilities util for getting and setting CAP_SYS_ADMIN.
Adding this avoids depending on an external libcap package. There is a
similar implementation (and reason) in the tools/testing/selftests/bpf
subtree but there's no obvious place to move it for sharing.
Signed-off-by: Benjamin Gray <redacted>
---
.../selftests/powerpc/dexcr/.gitignore | 1 +
.../testing/selftests/powerpc/dexcr/Makefile | 4 +-
tools/testing/selftests/powerpc/dexcr/cap.c | 72 ++++++
tools/testing/selftests/powerpc/dexcr/cap.h | 18 ++
tools/testing/selftests/powerpc/dexcr/dexcr.h | 2 +
.../selftests/powerpc/dexcr/dexcr_test.c | 241 ++++++++++++++++++
6 files changed, 336 insertions(+), 2 deletions(-)
create mode 100644 tools/testing/selftests/powerpc/dexcr/cap.c
create mode 100644 tools/testing/selftests/powerpc/dexcr/cap.h
create mode 100644 tools/testing/selftests/powerpc/dexcr/dexcr_test.c
@@ -0,0 +1,72 @@+#include<linux/capability.h>+#include<string.h>+#include<sys/syscall.h>++#include"cap.h"+#include"utils.h"++structkernel_capabilities{+struct__user_cap_header_structheader;++struct__user_cap_data_structdata[_LINUX_CAPABILITY_U32S_3];+};++staticvoidget_caps(structkernel_capabilities*caps)+{+FAIL_IF_EXIT_MSG(syscall(SYS_capget,&caps->header,&caps->data),+"cannot get capabilities");+}++staticvoidset_caps(structkernel_capabilities*caps)+{+FAIL_IF_EXIT_MSG(syscall(SYS_capset,&caps->header,&caps->data),+"cannot set capabilities");+}++staticvoidinit_caps(structkernel_capabilities*caps,pid_tpid)+{+memset(caps,0,sizeof(*caps));++caps->header.version=_LINUX_CAPABILITY_VERSION_3;+caps->header.pid=pid;++get_caps(caps);+}++staticboolhas_cap(structkernel_capabilities*caps,size_tcap)+{+size_tdata_index=cap/32;+size_toffset=cap%32;++FAIL_IF_EXIT_MSG(data_index>=ARRAY_SIZE(caps->data),"cap out of range");++returncaps->data[data_index].effective&(1<<offset);+}++staticvoiddrop_cap(structkernel_capabilities*caps,size_tcap)+{+size_tdata_index=cap/32;+size_toffset=cap%32;++FAIL_IF_EXIT_MSG(data_index>=ARRAY_SIZE(caps->data),"cap out of range");++caps->data[data_index].effective&=~(1<<offset);+}++boolcheck_cap_sysadmin(void)+{+structkernel_capabilitiescaps;++init_caps(&caps,0);++returnhas_cap(&caps,CAP_SYS_ADMIN);+}++voiddrop_cap_sysadmin(void)+{+structkernel_capabilitiescaps;++init_caps(&caps,0);+drop_cap(&caps,CAP_SYS_ADMIN);+set_caps(&caps);+}
@@ -0,0 +1,241 @@+#include<errno.h>+#include<stdlib.h>+#include<string.h>+#include<sys/prctl.h>+#include<unistd.h>++#include"cap.h"+#include"dexcr.h"+#include"utils.h"++/*+*Testthataneditableaspect+*-Currentprctlstatereportedbythegetter+*-CanbetoggledonandoffwhenprocesshasCAP_SYS_ADMIN+*-Can'tbeeditedifCAP_SYS_ADMINnotpresent+*-Can'tbemodifiedafterforceset+*/+staticintdexcr_prctl_editable_aspect_test(unsignedlongwhich)+{+pid_tpid;++SKIP_IF_MSG(!check_cap_sysadmin(),"must have capability CAP_SYS_ADMIN");+SKIP_IF_MSG(!pr_aspect_supported(which),"aspect not supported");++FAIL_IF_MSG(!(pr_aspect_get(which)&PR_PPC_DEXCR_PRCTL),"aspect not editable");++FAIL_IF_MSG(!pr_aspect_edit(which,PR_PPC_DEXCR_CLEAR_ASPECT),"prctl failed");+FAIL_IF_MSG(pr_aspect_check(which,UDEXCR),+"resetting aspect did not take effect");++FAIL_IF_MSG(pr_aspect_get(which)!=(PR_PPC_DEXCR_CLEAR_ASPECT|PR_PPC_DEXCR_PRCTL),+"prctl getter not reporting aspect state");++FAIL_IF_MSG(!pr_aspect_edit(which,PR_PPC_DEXCR_SET_ASPECT),"prctl failed");+FAIL_IF_MSG(!pr_aspect_check(which,UDEXCR),+"setting aspect did not take effect");++FAIL_IF_MSG(pr_aspect_get(which)!=(PR_PPC_DEXCR_SET_ASPECT|PR_PPC_DEXCR_PRCTL),+"prctl getter not reporting aspect state");++FAIL_IF_MSG(!pr_aspect_edit(which,PR_PPC_DEXCR_CLEAR_ASPECT),"prctl failed");+FAIL_IF_MSG(pr_aspect_check(which,UDEXCR),+"clearing aspect did not take effect");++FAIL_IF_MSG(pr_aspect_get(which)!=(PR_PPC_DEXCR_CLEAR_ASPECT|PR_PPC_DEXCR_PRCTL),+"prctl getter not reporting aspect state");++pid=fork();+if(pid==0){+drop_cap_sysadmin();+FAIL_IF_EXIT_MSG(pr_aspect_edit(which,PR_PPC_DEXCR_SET_ASPECT),+"prctl success when nonprivileged");+FAIL_IF_EXIT_MSG(pr_aspect_check(which,UDEXCR),+"edited aspect when nonprivileged");+_exit(0);+}+await_child_success(pid);++FAIL_IF_MSG(!pr_aspect_edit(which,PR_PPC_DEXCR_FORCE_SET_ASPECT),"prctl force set failed");+FAIL_IF_MSG(!pr_aspect_check(which,UDEXCR),+"force setting aspect did not take effect");++FAIL_IF_MSG(pr_aspect_get(which)!=(PR_PPC_DEXCR_FORCE_SET_ASPECT|PR_PPC_DEXCR_PRCTL),+"prctl getter not reporting aspect state");++FAIL_IF_MSG(pr_aspect_edit(which,PR_PPC_DEXCR_CLEAR_ASPECT),"prctl success when forced");+FAIL_IF_MSG(!pr_aspect_check(which,UDEXCR),+"edited aspect when forced");++return0;+}++staticintdexcr_prctl_sbhe_test(void)+{+sysctl_set_sbhe(-1);+returndexcr_prctl_editable_aspect_test(PR_PPC_DEXCR_SBHE);+}++staticintdexcr_prctl_ibrtpd_test(void)+{+returndexcr_prctl_editable_aspect_test(PR_PPC_DEXCR_IBRTPD);+}++staticintdexcr_prctl_srapd_test(void)+{+returndexcr_prctl_editable_aspect_test(PR_PPC_DEXCR_SRAPD);+}++staticintdexcr_sysctl_sbhe_test(void)+{+SKIP_IF_MSG(!check_cap_sysadmin(),"must have capability CAP_SYS_ADMIN");+SKIP_IF_MSG(!pr_aspect_supported(PR_PPC_DEXCR_SBHE),"aspect not supported");++sysctl_set_sbhe(0);+FAIL_IF_MSG(sysctl_get_sbhe()!=0,"failed to clear sysctl SBHE");+FAIL_IF_MSG(pr_aspect_check(PR_PPC_DEXCR_SBHE,UDEXCR),+"SBHE failed to clear");++sysctl_set_sbhe(1);+FAIL_IF_MSG(sysctl_get_sbhe()!=1,"failed to set sysctl SBHE");+FAIL_IF_MSG(!pr_aspect_check(PR_PPC_DEXCR_SBHE,UDEXCR),+"SBHE failed to set");++sysctl_set_sbhe(-1);+FAIL_IF_MSG(sysctl_get_sbhe()!=-1,"failed to default sysctl SBHE");+FAIL_IF_MSG(!pr_aspect_edit(PR_PPC_DEXCR_SBHE,PR_PPC_DEXCR_CLEAR_ASPECT),"prctl failed");+FAIL_IF_MSG(pr_aspect_check(PR_PPC_DEXCR_SBHE,UDEXCR),+"SBHE failed to default to prctl clear setting");++FAIL_IF_MSG(!pr_aspect_edit(PR_PPC_DEXCR_SBHE,PR_PPC_DEXCR_SET_ASPECT),"prctl failed");+FAIL_IF_MSG(!pr_aspect_check(PR_PPC_DEXCR_SBHE,UDEXCR),+"SBHE failed to default to prctl set setting");++sysctl_set_sbhe(0);+FAIL_IF_MSG(sysctl_get_sbhe()!=0,"failed to clear sysctl SBHE");+FAIL_IF_MSG(pr_aspect_check(PR_PPC_DEXCR_SBHE,UDEXCR),+"SBHE failed to override prctl setting");++return0;+}++staticintdexcr_test_inherit_execve(charexpected_dexcr)+{+switch(expected_dexcr){+case'0':+FAIL_IF_EXIT_MSG(pr_aspect_get(PR_PPC_DEXCR_IBRTPD)!=+(PR_PPC_DEXCR_CLEAR_ASPECT|PR_PPC_DEXCR_PRCTL),+"clearing IBRTPD across exec not inherited");++FAIL_IF_EXIT_MSG(pr_aspect_check(PR_PPC_DEXCR_IBRTPD,UDEXCR),+"clearing IBRTPD across exec not applied");+break;+case'1':+FAIL_IF_EXIT_MSG(pr_aspect_get(PR_PPC_DEXCR_IBRTPD)!=+(PR_PPC_DEXCR_SET_ASPECT|PR_PPC_DEXCR_PRCTL),+"setting IBRTPD across exec not inherited");++FAIL_IF_EXIT_MSG(!pr_aspect_check(PR_PPC_DEXCR_IBRTPD,UDEXCR),+"setting IBRTPD across exec not applied");+break;+case'2':+FAIL_IF_EXIT_MSG(pr_aspect_get(PR_PPC_DEXCR_IBRTPD)!=+(PR_PPC_DEXCR_FORCE_SET_ASPECT|PR_PPC_DEXCR_PRCTL),+"force setting IBRTPD across exec not inherited");++FAIL_IF_EXIT_MSG(!pr_aspect_check(PR_PPC_DEXCR_IBRTPD,UDEXCR),+"force setting IBRTPD across exec not applied");+break;+}++return0;+}++/*+*CheckthatachildprocessinheritstheDEXCRoverforkandexecve+*/+staticintdexcr_inherit_test(void)+{+pid_tpid;++SKIP_IF_MSG(!check_cap_sysadmin(),"must have capability CAP_SYS_ADMIN");+SKIP_IF_MSG(!pr_aspect_supported(PR_PPC_DEXCR_IBRTPD),"IBRTPD not supported");++pr_aspect_edit(PR_PPC_DEXCR_IBRTPD,PR_PPC_DEXCR_CLEAR_ASPECT);+FAIL_IF_MSG(pr_aspect_check(PR_PPC_DEXCR_IBRTPD,UDEXCR),+"IBRTPD failed to clear");++pid=fork();+if(pid==0){+char*args[]={"dexcr_test_inherit_execve","0",NULL};++FAIL_IF_EXIT_MSG(pr_aspect_get(PR_PPC_DEXCR_IBRTPD)!=+(PR_PPC_DEXCR_CLEAR_ASPECT|PR_PPC_DEXCR_PRCTL),+"clearing IBRTPD not inherited");++FAIL_IF_EXIT_MSG(pr_aspect_check(PR_PPC_DEXCR_IBRTPD,UDEXCR),+"clearing IBRTPD not applied");++execve("/proc/self/exe",args,NULL);+_exit(errno);+}+await_child_success(pid);++pr_aspect_edit(PR_PPC_DEXCR_IBRTPD,PR_PPC_DEXCR_SET_ASPECT);+FAIL_IF_MSG(!pr_aspect_check(PR_PPC_DEXCR_IBRTPD,UDEXCR),+"IBRTPD failed to set");++pid=fork();+if(pid==0){+char*args[]={"dexcr_test_inherit_execve","1",NULL};++FAIL_IF_EXIT_MSG(pr_aspect_get(PR_PPC_DEXCR_IBRTPD)!=+(PR_PPC_DEXCR_SET_ASPECT|PR_PPC_DEXCR_PRCTL),+"setting IBRTPD not inherited");++FAIL_IF_EXIT_MSG(!pr_aspect_check(PR_PPC_DEXCR_IBRTPD,UDEXCR),+"setting IBRTPD not applied");++execve("/proc/self/exe",args,NULL);+_exit(errno);+}+await_child_success(pid);++pr_aspect_edit(PR_PPC_DEXCR_IBRTPD,PR_PPC_DEXCR_FORCE_SET_ASPECT);+FAIL_IF_MSG(!pr_aspect_check(PR_PPC_DEXCR_IBRTPD,UDEXCR),+"IBRTPD failed to force set");++pid=fork();+if(pid==0){+char*args[]={"dexcr_test_inherit_execve","2",NULL};++FAIL_IF_EXIT_MSG(pr_aspect_get(PR_PPC_DEXCR_IBRTPD)!=+(PR_PPC_DEXCR_FORCE_SET_ASPECT|PR_PPC_DEXCR_PRCTL),+"force setting IBRTPD not inherited");++FAIL_IF_EXIT_MSG(!pr_aspect_check(PR_PPC_DEXCR_IBRTPD,UDEXCR),+"force setting IBRTPD not applied");++execve("/proc/self/exe",args,NULL);+_exit(errno);+}+await_child_success(pid);++return0;+}++intmain(intargc,char*argv[])+{+interr=0;++if(argc>=2&&strcmp(argv[0],"dexcr_test_inherit_execve")==0)+returndexcr_test_inherit_execve(argv[1][0]);++err|=test_harness(dexcr_prctl_sbhe_test,"dexcr_prctl_sbhe");+err|=test_harness(dexcr_prctl_ibrtpd_test,"dexcr_prctl_ibrtpd");+err|=test_harness(dexcr_prctl_srapd_test,"dexcr_prctl_srapd");+err|=test_harness(dexcr_sysctl_sbhe_test,"dexcr_sysctl_sbhe");+err|=test_harness(dexcr_inherit_test,"dexcr_inherit");++returnerr;+}
From: Benjamin Gray <hidden> Date: 2022-11-28 02:46:36
ISA 3.1B introduces the Dynamic Execution Control Register (DEXCR). It
is a per-cpu register that allows control over various CPU behaviours
including branch hint usage, indirect branch speculation, and
hashst/hashchk support.
Though introduced in 3.1B, no CPUs using 3.1 were released, so
CPU_FTR_ARCH_31 is used to determine support for the register itself.
Support for each DEXCR bit (aspect) is reported separately by the
firmware.
Add various definitions and basic support for the DEXCR in the kernel.
Right now it just initialises and maintains the DEXCR on process
creation/swap, and clears it in reset_sprs().
Signed-off-by: Benjamin Gray <redacted>
---
arch/powerpc/include/asm/book3s/64/kexec.h | 3 +++
arch/powerpc/include/asm/cputable.h | 8 ++++++-
arch/powerpc/include/asm/processor.h | 13 +++++++++++
arch/powerpc/include/asm/reg.h | 6 ++++++
arch/powerpc/kernel/Makefile | 1 +
arch/powerpc/kernel/dexcr.c | 25 ++++++++++++++++++++++
arch/powerpc/kernel/dt_cpu_ftrs.c | 4 ++++
arch/powerpc/kernel/process.c | 13 ++++++++++-
arch/powerpc/kernel/prom.c | 4 ++++
9 files changed, 75 insertions(+), 2 deletions(-)
create mode 100644 arch/powerpc/kernel/dexcr.c
@@ -21,6 +21,9 @@ static inline void reset_sprs(void)plpar_set_ciabr(0);}+if(cpu_has_feature(CPU_FTR_ARCH_31))+mtspr(SPRN_DEXCR,0);+/* Do we need isync()? We are going via a kexec reset */isync();}
From: Benjamin Gray <hidden> Date: 2022-11-28 02:46:45
Describe the DEXCR and document how to interact with it via the
prctl and sysctl interfaces.
Signed-off-by: Benjamin Gray <redacted>
---
Documentation/powerpc/dexcr.rst | 183 ++++++++++++++++++++++++++++++++
Documentation/powerpc/index.rst | 1 +
2 files changed, 184 insertions(+)
create mode 100644 Documentation/powerpc/dexcr.rst
@@ -0,0 +1,183 @@+==========================================+DEXCR (Dynamic Execution Control Register)+==========================================++Overview+========++The DEXCR is a privileged special purpose register (SPR) introduced in+PowerPC ISA 3.1B (Power10) that allows per-cpu control over several dynamic+execution behaviours. These behaviours include speculation (e.g., indirect+branch target prediction) and enabling return-oriented programming (ROP)+protection instructions.++The execution control is exposed in hardware as up to 32 bits ('aspects') in+the DEXCR. Each aspect controls a certain behaviour, and can be set or cleared+to enable/disable the aspect. There are several variants of the DEXCR for+different purposes:++DEXCR+ A priviliged SPR that can control aspects for userspace and kernel space+HDEXCR+ A hypervisor-privileged SPR that can control aspects for the hypervisor and+ enforce aspects for the kernel and userspace.+UDEXCR+ An optional ultravisor-privileged SPR that can control aspects for the ultravisor.++Userspace can examine the current DEXCR state using a dedicated SPR that+provides a non-privileged read-only view of the userspace DEXCR aspects.+There is also an SPR that provides a read-only view of the hypervisor enforced+aspects, which ORed with the userspace DEXCR view gives the effective DEXCR+state for a process.+++User API+========++prctl()+-------++A process can control its own userspace DEXCR value using the+``PR_PPC_GET_DEXCR`` and ``PR_PPC_SET_DEXCR`` pair of+:manpage:`prctl(2)` commands. These calls have the form::++ prctl(PR_PPC_GET_DEXCR, unsigned long aspect, 0, 0, 0);+ prctl(PR_PPC_SET_DEXCR, unsigned long aspect, unsigned long flags, 0, 0);++Where ``aspect`` (``arg1``) is a constant and ``flags`` (``arg2``) is a bifield.+The possible aspect and flag values are as follows. Note there is no relation+between aspect value and ``prctl()`` constant value.++..flat-table::+:header-rows: 1+:widths: 2 7 1++* - ``prctl()`` constant+- Aspect name+- Aspect bit++* - ``PR_PPC_DEXCR_SBHE``+- Speculative Branch Hint Enable (SBHE)+- 0++* - ``PR_PPC_DEXCR_IBRTPD``+- Indirect Branch Recurrent Target Prediction Disable (IBRTPD)+- 3++* - ``PR_PPC_DEXCR_SRAPD``+- Subroutine Return Address Prediction Disable (SRAPD)+- 4++* - ``PR_PPC_DEXCR_NPHIE``+- Non-Privileged Hash Instruction Enable (NPHIE)+- 5++..flat-table::+:header-rows: 1+:widths: 2 8++* - ``prctl()`` flag+- Meaning++* - ``PR_PPC_DEXCR_PRCTL``+- This aspect can be configured with ``prctl(PR_PPC_SET_DEXCR, ...)``++* - ``PR_PPC_DEXCR_SET_ASPECT``+- This aspect is set++* - ``PR_PPC_DEXCR_FORCE_SET_ASPECT``+- This aspect is set and cannot be undone. A subsequent+``prctl(..., PR_PPC_DEXCR_CLEAR_ASPECT)`` will fail.++* - ``PR_PPC_DEXCR_CLEAR_ASPECT``+- This aspect is clear++Note that++* The ``*_SET_ASPECT`` / ``*_CLEAR_ASPECT`` refers to setting/clearing the bit in the DEXCR.+ For example::++ prctl(PR_PPC_SET_DEXCR, PR_PPC_DEXCR_IBRTPD, PR_PPC_DEXCR_SET_ASPECT, 0, 0);++ will set the IBRTPD aspect bit in the DEXCR, causing indirect branch prediction+ to be disabled.++* The status returned by ``PR_PPC_GET_DEXCR`` does not include any alternative+ config overrides. To see the true DEXCR state software should read the appropriate+ SPRs directly.++* A forced aspect will still report ``PR_PPC_DEXCR_PRCTL`` if it would+ otherwise be editable.++* The aspect state when starting a process is copied from the parent's+ state on :manpage:`fork(2)` and :manpage:`execve(2)`. Aspects may also be set+ or cleared by the kernel on process creation.++Use ``PR_PPC_SET_DEXCR`` with one of ``PR_PPC_DEXCR_SET_ASPECT``,+``PR_PPC_DEXCR_FORCE_SET_ASPECT``, or ``PR_PPC_DEXCR_CLEAR_ASPECT`` to edit a+ given aspect.++Common error codes for both getting and setting the DEXCR are as follows:++..flat-table::+:header-rows: 1+:widths: 2 8++* - Error+- Meaning++* - ``EINVAL``+- The DEXCR is not supported by the kernel.++* - ``ENODEV``+- The aspect is not recognised by the kernel or not supported by the hardware.++``PR_PPC_SET_DEXCR`` may also report the following error codes:++..flat-table::+:header-rows: 1+:widths: 2 8++* - Error+- Meaning++* - ``ERANGE``+-``arg2`` is incorrect. E.g., it does not select an action (set/clear),+ or the flags are not recognised by the kernel.++* - ``ENXIO``+- The aspect is not editable via ``prctl()``.++* - ``EPERM``+- The process does not have sufficient privilege to modify this aspect,+ or the aspect has been force set and cannot be modified.+++sysctl+------++Some aspects can be modified globally via :manpage:`sysctl(8)` entries. Such global+modifications are applied after any process modifications. Any ``prctl()`` call to+an overridden aspect this aspect may still report it as editable. The prctl setting+will take effect again if the global override is restored to its default state.++A global SBHE config is exposed in ``/proc/sys/kernel/speculative_branch_hint_enable``.+Any process can read the current config value from it. Privileged processes can+write to it to change the config. The new config is applied to all current and future+processes (though note the kernel cannot override any hypervisor enforced aspects).++..flat-table::+:header-rows: 1+:widths: 2 8++* - Value+- Meaning++* - ``-1``+- Do not change from default or ``prctl()`` config.++* - ``0``+- Force clear aspect.++* - ``1``+- Force set aspect.
@@ -0,0 +1,118 @@+#include<errno.h>+#include<fcntl.h>+#include<limits.h>+#include<stdlib.h>+#include<string.h>+#include<sys/capability.h>+#include<sys/prctl.h>+#include<sys/wait.h>++#include"dexcr.h"+#include"reg.h"+#include"utils.h"++longsysctl_get_sbhe(void)+{+longvalue;++FAIL_IF_EXIT_MSG(read_long(SYSCTL_DEXCR_SBHE,&value,10),+"failed to read "SYSCTL_DEXCR_SBHE);++returnvalue;+}++voidsysctl_set_sbhe(longvalue)+{+FAIL_IF_EXIT_MSG(write_long(SYSCTL_DEXCR_SBHE,value,10),+"failed to write to "SYSCTL_DEXCR_SBHE);+}++unsignedintpr_aspect_to_dexcr_mask(unsignedlongwhich)+{+switch(which){+casePR_PPC_DEXCR_SBHE:+returnDEXCR_PRO_SBHE;+casePR_PPC_DEXCR_IBRTPD:+returnDEXCR_PRO_IBRTPD;+casePR_PPC_DEXCR_SRAPD:+returnDEXCR_PRO_SRAPD;+casePR_PPC_DEXCR_NPHIE:+returnDEXCR_PRO_NPHIE;+default:+FAIL_IF_EXIT_MSG(true,"unknown PR aspect");+}+}++staticinlineunsignedintget_dexcr_pro(void)+{+returnmfspr(SPRN_DEXCR);+}++staticinlineunsignedintget_dexcr_enf(void)+{+returnmfspr(SPRN_HDEXCR);+}++staticinlineunsignedintget_dexcr_eff(void)+{+returnget_dexcr_pro()|get_dexcr_enf();+}++unsignedintget_dexcr(enumDexcrSourcesource)+{+switch(source){+caseUDEXCR:+returnget_dexcr_pro();+caseENFORCED:+returnget_dexcr_enf();+caseEFFECTIVE:+returnget_dexcr_eff();+default:+FAIL_IF_EXIT_MSG(true,"bad DEXCR source");+}+}++boolpr_aspect_supported(unsignedlongwhich)+{+returnprctl(PR_PPC_GET_DEXCR,which,0,0,0)>=0;+}++boolpr_aspect_editable(unsignedlongwhich)+{+intret=prctl(PR_PPC_GET_DEXCR,which,0,0,0);+returnret>0&&(ret&PR_PPC_DEXCR_PRCTL)>0;+}++boolpr_aspect_edit(unsignedlongwhich,unsignedlongctrl)+{+returnprctl(PR_PPC_SET_DEXCR,which,ctrl,0,0)==0;+}++boolpr_aspect_check(unsignedlongwhich,enumDexcrSourcesource)+{+unsignedintdexcr=get_dexcr(source);+unsignedintaspect=pr_aspect_to_dexcr_mask(which);+return(dexcr&aspect)!=0;+}++intpr_aspect_get(unsignedlongpr_aspect)+{+intret=prctl(PR_PPC_GET_DEXCR,pr_aspect,0,0,0);+FAIL_IF_EXIT_MSG(ret<0,"prctl failed");+returnret;+}++booldexcr_pro_check(unsignedintpro,enumDexcrSourcesource)+{+return(get_dexcr(source)&pro)!=0;+}++voidawait_child_success(pid_tpid)+{+intwstatus;++FAIL_IF_EXIT_MSG(pid==-1,"fork failed");+FAIL_IF_EXIT_MSG(waitpid(pid,&wstatus,0)==-1,"wait failed");+FAIL_IF_EXIT_MSG(!WIFEXITED(wstatus),"child did not exit cleanly");+FAIL_IF_EXIT_MSG(WEXITSTATUS(wstatus)!=0,"child exit error");+}
@@ -0,0 +1,229 @@+#define _GNU_SOURCE++#include<errno.h>+#include<fcntl.h>+#include<limits.h>+#include<sched.h>+#include<signal.h>+#include<stdio.h>+#include<stdlib.h>+#include<string.h>+#include<sys/mman.h>+#include<sys/prctl.h>+#include<unistd.h>++#include"dexcr.h"+#include"utils.h"++staticintrequire_nphie(void)+{+SKIP_IF_MSG(!pr_aspect_supported(PR_PPC_DEXCR_NPHIE),+"DEXCR[NPHIE] not supported");++if(dexcr_pro_check(DEXCR_PRO_NPHIE,EFFECTIVE))+return0;++pr_aspect_edit(PR_PPC_DEXCR_NPHIE,PR_PPC_DEXCR_FORCE_SET_ASPECT);+FAIL_IF_EXIT_MSG(!dexcr_pro_check(DEXCR_PRO_NPHIE,EFFECTIVE),+"failed to enable DEXCR[NPIHE]");++return0;+}++staticvoidsigill_handler_enabled(intsignum,siginfo_t*info,void*context)+{+SIGSAFE_FAIL_IF_EXIT_MSG(signum!=SIGILL,"wrong signal received");+SIGSAFE_FAIL_IF_EXIT_MSG(info->si_code!=ILL_ILLOPN,"wrong signal-code received");+exit(0);+}++staticvoiddo_bad_hashchk(void)+{+unsignedlonghash=0;+void*hash_p=((void*)&hash)+8;/* hash* offset must be at least -8 */++asm("li 3, 0;"/* set r3 (pretend LR) to known value */+"hashst 3, -8(%1);"/* compute good hash */+"addi 3, 3, 1;"/* modify hash */+"hashchk 3, -8(%1);"/* check bad hash */+:"+m"(hash):"r"(hash_p):"r3");+}++/*+*CheckthathashchktriggerswhenDEXCR[NPHIE]isenabled+*andisdetectedassuchbythekernelexceptionhandler+*/+staticinthashchk_enabled_test(void)+{+interr;+structsigactionsa;++if((err=require_nphie()))+returnerr;++sa.sa_sigaction=sigill_handler_enabled;+sigemptyset(&sa.sa_mask);+sa.sa_flags=SA_SIGINFO;+FAIL_IF_MSG(sigaction(SIGILL,&sa,NULL),"cannot install signal handler");++do_bad_hashchk();++FAIL_IF_MSG(true,"hashchk failed to trigger");+}++#define HASH_COUNT 8++staticunsignedlonghash_values[HASH_COUNT+1];++staticvoidfill_hash_values(void)+{+for(unsignedlongi=0;i<HASH_COUNT;i++){+void*hash_addr=((void*)&hash_values[i])+8;++asmvolatile("hashst %2, -8(%1);"+:"+m"(hash_values[i]):"r"(hash_addr),"r"(i));+}++hash_values[HASH_COUNT]=(unsignedlong)&hash_values;+}++staticunsignedintcount_hash_values_matches(void)+{+unsignedlongmatches=0;++FAIL_IF_EXIT_MSG(hash_values[HASH_COUNT]!=(unsignedlong)hash_values,+"bad address check");++for(unsignedlongi=0;i<HASH_COUNT;i++){+unsignedlongorig_hash=hash_values[i];+void*hash_addr=((void*)&hash_values[i])+8;++asmvolatile("hashst %2, -8(%1);"+:"+m"(hash_values[i]):"r"(hash_addr),"r"(i));++if(hash_values[i]==orig_hash)+matches++;+}++returnmatches;+}++staticinthashchk_exec_child(void)+{+ssize_tcount;++fill_hash_values();++count=write(STDOUT_FILENO,hash_values,sizeof(hash_values));+returncount==sizeof(hash_values)?0:EOVERFLOW;+}++/*+*Checkthatnewprogramsgetdifferentkeyssoamaliciousprocess+*can'trecreateavictim'shashvalues.+*/+staticinthashchk_exec_random_key_test(void)+{+pid_tpid;+interr;+intpipefd[2];++if((err=require_nphie()))+returnerr;++FAIL_IF_MSG(pipe(pipefd),"failed to create pipe");++pid=fork();+if(pid==0){+char*args[]={"hashchk_exec_child",NULL};++if(dup2(pipefd[1],STDOUT_FILENO)==-1)+_exit(errno);++execve("/proc/self/exe",args,NULL);+_exit(errno);+}++await_child_success(pid);+FAIL_IF_MSG(read(pipefd[0],hash_values,sizeof(hash_values))!=sizeof(hash_values),+"missing expected child output");++/* If all hashes are the same it means (most likely) same key */+FAIL_IF_MSG(count_hash_values_matches()==HASH_COUNT,"shared key detected");++return0;+}++/*+*Checkthatforkssharethesamekeysothatexistinghashvalues+*remainvalid.+*/+staticinthashchk_fork_share_key_test(void)+{+pid_tpid;+interr;++if((err=require_nphie()))+returnerr;++fill_hash_values();++pid=fork();+if(pid==0){+if(count_hash_values_matches()!=HASH_COUNT)+_exit(1);+_exit(0);+}++await_child_success(pid);+return0;+}++#define STACK_SIZE (1024 * 1024)++staticinthashchk_clone_child_fn(void*args)+{+fill_hash_values();+return0;+}++/*+*Checkthatthreadssharethesamekeysothatexistinghashvalues+*remainvalid.+*/+staticinthashchk_clone_share_key_test(void)+{+void*child_stack;+pid_tpid;+interr;++if((err=require_nphie()))+returnerr;++child_stack=mmap(NULL,STACK_SIZE,PROT_READ|PROT_WRITE,+MAP_PRIVATE|MAP_ANONYMOUS|MAP_STACK,-1,0);++FAIL_IF_MSG(child_stack==MAP_FAILED,"failed to map child stack");++pid=clone(hashchk_clone_child_fn,child_stack+STACK_SIZE,CLONE_VM|SIGCHLD,NULL);++await_child_success(pid);+FAIL_IF_MSG(count_hash_values_matches()!=HASH_COUNT,"different key detected");++return0;+}++intmain(intargc,char*argv[])+{+interr=0;++if(argc>=1&&!strcmp(argv[0],"hashchk_exec_child"))+returnhashchk_exec_child();++err|=test_harness(hashchk_enabled_test,"hashchk_enabled");+err|=test_harness(hashchk_exec_random_key_test,"hashchk_exec_random_key");+err|=test_harness(hashchk_fork_share_key_test,"hashchk_fork_share_key");+err|=test_harness(hashchk_clone_share_key_test,"hashchk_clone_share_key");++returnerr;+}
From: Benjamin Gray <hidden> Date: 2022-11-28 02:46:51
Adds the definitions and generic handler for prctl control of the
PowerPC Dynamic Execution Control Register (DEXCR).
Signed-off-by: Benjamin Gray <redacted>
---
include/uapi/linux/prctl.h | 14 ++++++++++++++
kernel/sys.c | 16 ++++++++++++++++
2 files changed, 30 insertions(+)
From: Benjamin Gray <hidden> Date: 2022-11-28 02:47:00
The DEXCR Speculative Branch Hint Enable (SBHE) aspect controls whether
the hints provided by BO field of Branch instructions are obeyed during
speculative execution.
SBHE behaviour per ISA 3.1B:
0: The hints provided by BO field of Branch instructions may be
ignored during speculative execution
1: The hints provided by BO field of Branch instructions are obeyed
during speculative execution
Add a sysctl entry to allow changing this aspect globally in the system
at runtime:
/proc/sys/kernel/speculative_branch_hint_enable
Three values are supported:
-1: Disable DEXCR SBHE sysctl override
0: Override and set DEXCR[SBHE] aspect to 0
1: Override and set DEXCR[SBHE] aspect to 1
Internally, introduces a mechanism to apply arbitrary system wide
overrides on top of the prctl() config.
Signed-off-by: Benjamin Gray <redacted>
---
arch/powerpc/kernel/dexcr.c | 125 ++++++++++++++++++++++++++++++++++++
1 file changed, 125 insertions(+)
@@ -18,6 +21,58 @@#define DEXCR_PRCTL_EDITABLE (DEXCR_PRO_SBHE | DEXCR_PRO_IBRTPD | \DEXCR_PRO_SRAPD|DEXCR_PRO_NPHIE)+/*+*LocktoprotectsystemDEXCRoverridefromconcurrentupdates.+*RCUsemantics:writerstakelock,readersareunlocked.+*Writersensurethememoryupdateisatomic,readersread+*atomically.+*/+staticDEFINE_SPINLOCK(dexcr_sys_enforced_write_lock);++structmask_override{+union{+struct{+unsignedintmask;+unsignedintoverride;+};++/* Raw access for atomic read/write */+unsignedlongall;+};+};++staticstructmask_overridedexcr_sys_enforced;++staticintspec_branch_hint_enable=-1;++staticvoidupdate_userspace_system_dexcr(unsignedintpro_mask,intvalue)+{+structmask_overrideupdate={.all=0};++switch(value){+case-1:/* Clear the mask bit, clear the override bit */+break;+case0:/* Set the mask bit, clear the override bit */+update.mask|=pro_mask;+break;+case1:/* Set the mask bit, set the override bit */+update.mask|=pro_mask;+update.override|=pro_mask;+break;+}++spin_lock(&dexcr_sys_enforced_write_lock);++/* Use the existing values for the non-updated bits */+update.mask|=dexcr_sys_enforced.mask&~pro_mask;+update.override|=dexcr_sys_enforced.override&~pro_mask;++/* Atomically update system enforced aspects */+WRITE_ONCE(dexcr_sys_enforced.all,update.all);++spin_unlock(&dexcr_sys_enforced_write_lock);+}+staticint__initdexcr_init(void){if(!early_cpu_has_feature(CPU_FTR_ARCH_31))
@@ -25,6 +80,9 @@ static int __init dexcr_init(void)mtspr(SPRN_DEXCR,DEFAULT_DEXCR);+if(early_cpu_has_feature(CPU_FTR_DEXCR_SBHE))+update_userspace_system_dexcr(DEXCR_PRO_SBHE,spec_branch_hint_enable);+return0;}early_initcall(dexcr_init);
From: Russell Currey <hidden> Date: 2022-11-28 04:06:00
On Mon, 2022-11-28 at 13:44 +1100, Benjamin Gray wrote:
This series is based on initial work by Chris Riedl that was not sent
to the list.
Adds a kernel interface for userspace to interact with the DEXCR.
The DEXCR is a SPR that allows control over various execution
'aspects', such as indirect branch prediction and enabling the
hashst/hashchk instructions. Further details are in ISA 3.1B
Book 3 chapter 12.
This RFC proposes an interface for users to interact with the DEXCR.
It aims to support
* Querying supported aspects
* Getting/setting aspects on a per-process level
* Allowing global overrides across all processes
There are some parts that I'm not sure on the best way to approach
(hence RFC):
* The feature names in arch/powerpc/kernel/dt_cpu_ftrs.c appear to be
unimplemented
in skiboot, so are being defined by this series. Is being so
verbose fine?
These are going to need to be added to skiboot before they can be
referenced in the kernel. Inclusion in skiboot makes them ABI, the
kernel is just a consumer.
* What aspects should be editable by a process? E.g., SBHE has
effects that potentially bleed into other processes. Should
it only be system wide configurable?
For context, ISA 3.1B p1358 says:
In some micro-architectures, the execution behav-
ior controlled by aspect 0 is difficult to change with
any degree of timing precision. The change may
also bleed over into other threads on the same pro-
cessor. Any environment that has a dependence on
the more secure setting of aspect 0 should not
change the value, and ideally should share a pro-
cessor only with similar threads. For other environ-
ments, changes to the effective value of aspect 0
represent a relative risk tolerance for its aspect of
execution behavior, with the understanding that
there will be significant hysteresis in the execution
behavior.
If a process sets SBHE for itself and all it takes is context switching
from a process with SBHE unset to cause exposure, then yeah I think it
should just be global. I doubt branch hints have enough impact for
process granularity to be especially desirable anyway.
* Should configuring certain aspects for the process be non-
privileged? E.g.,
Is there harm in always allowing configuration of IBRTPD, SRAPD?
The *FORCE_SET*
action prevents further process local changes regardless of
privilege.
I'm not aware of a reason why it would be a problem to allow
unprivileged configuration as long as there's a way to prevent further
changes. The concerning case is if a mitigation is set by a trusted
process context, and then untrusted code is executed that manages to
turn the mitigation off again.
* The tests fail Patchwork CI because of the new prctl macros, and
the CI
doesn't run headers_install and add -isystem
<buildpath>/usr/include to
the make command.
The CI runs on x86 and cross compiles the kernel and selftests, and
boots are done in qemu tcg. Maybe we can skip the build if the symbols
are undefined or do something like
#ifndef PR_PPC_DEXCR_...
return KSFT_SKIP;
#endif
in the test itself?
* On handling an exception, I don't check if the NPHIE bit is enabled
in the DEXCR.
To do so would require reading both the DEXCR and HDEXCR, for
little gain (it
should only matter that the current instruction was a hashchk. If
so, the only
reason it would cause an exception is the failed check. If the
instruction is
rewritten between exception and check we'd be wrong anyway).
For context, the hashst and hashchk instructions are implemented using
previously reserved nops. I'm not aware of any reason a nop could trap
(i.e. we could check for a trap that came from hashchk even if NPHIE is
not set), but afaik that'd be the only reason we would have to check.
On Mon Nov 28, 2022 at 12:44 PM AEST, Benjamin Gray wrote:
quoted hunk
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.
Signed-off-by: Benjamin Gray <redacted>
---
arch/powerpc/include/asm/ppc-opcode.h | 1 +
arch/powerpc/include/asm/processor.h | 6 ++++++
arch/powerpc/kernel/dexcr.c | 22 ++++++++++++++++++++++
arch/powerpc/kernel/traps.c | 6 ++++++
4 files changed, 35 insertions(+)
@@ -19,6 +22,25 @@ static int __init dexcr_init(void)}early_initcall(dexcr_init);+boolis_hashchk_trap(structpt_regsconst*regs)+{+ppc_inst_tinsn;++if(!cpu_has_feature(CPU_FTR_DEXCR_NPHIE))+returnfalse;++if(get_user_instr(insn,(void__user*)regs->nip)){+WARN_ON(1);+returnfalse;+}
Nice series, just starting to have a look at it.
You probably don't want a WARN_ON() here because it's user triggerable
and isn't necessarily even indiciating a problem or attack if the app
is doing code unmapping in order to get faults.
Check some of the other instruction emulation for what to do in case of
an EFAULT.
From: Benjamin Gray <hidden> Date: 2022-11-29 22:04:31
On Tue, 2022-11-29 at 20:39 +1000, Nicholas Piggin wrote:
On Mon Nov 28, 2022 at 12:44 PM AEST, Benjamin Gray wrote:
quoted
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.
Signed-off-by: Benjamin Gray <redacted>
---
arch/powerpc/include/asm/ppc-opcode.h | 1 +
arch/powerpc/include/asm/processor.h | 6 ++++++
arch/powerpc/kernel/dexcr.c | 22 ++++++++++++++++++++++
arch/powerpc/kernel/traps.c | 6 ++++++
4 files changed, 35 insertions(+)
Nice series, just starting to have a look at it.
You probably don't want a WARN_ON() here because it's user
triggerable
and isn't necessarily even indiciating a problem or attack if the app
is doing code unmapping in order to get faults.
Check some of the other instruction emulation for what to do in case
of
an EFAULT.
I guess ILLOPN makes sense. Do you know if any other archs do
similar?
Ah sorry, when refactoring Chris' patches I forgot to put back in the
commit message that this is how ARM reports their similar check
failure. For example, their FPAC handler in
arch/arm64/kernel/traps.c:518 does this.
On Mon Nov 28, 2022 at 12:44 PM AEST, Benjamin Gray wrote:
The functions here use struct thread_struct fields, so need to import
the full definition from <linux/sched.h>. The <asm/current.h> header
that defines current only forward declares struct thread_struct.
Failing to include this <linux/sched.h> header leads to a compilation
error when a translation unit does not also include <linux/sched.h>
indirectly.
Signed-off-by: Benjamin Gray <redacted>
On Mon Nov 28, 2022 at 12:44 PM AEST, Benjamin Gray wrote:
ISA 3.1B introduces the Dynamic Execution Control Register (DEXCR). It
is a per-cpu register that allows control over various CPU behaviours
including branch hint usage, indirect branch speculation, and
hashst/hashchk support.
Though introduced in 3.1B, no CPUs using 3.1 were released, so
CPU_FTR_ARCH_31 is used to determine support for the register itself.
Support for each DEXCR bit (aspect) is reported separately by the
firmware.
Add various definitions and basic support for the DEXCR in the kernel.
Right now it just initialises and maintains the DEXCR on process
creation/swap, and clears it in reset_sprs().
A couple of comments below, but it looks good:
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
@@ -21,6 +21,9 @@ static inline void reset_sprs(void)plpar_set_ciabr(0);}+if(cpu_has_feature(CPU_FTR_ARCH_31))+mtspr(SPRN_DEXCR,0);+/* Do we need isync()? We are going via a kexec reset */isync();}
We potentially don't need to use CPU_FTR bits for each of these. We
only really want them to use instruction patching and make feature
tests fast. But we have been a bit liberal with using them and they
are kind of tied into cpu feature parsing code so maybe it's easier
to go with them for now.
@@ -385,6 +385,12 @@#define SPRN_HSRR0 0x13A /* Hypervisor Save/Restore 0 */#define SPRN_HSRR1 0x13B /* Hypervisor Save/Restore 1 */#define SPRN_ASDR 0x330 /* Access segment descriptor register */+#define SPRN_DEXCR 0x33C /* Dynamic execution control register */+#define DEXCR_PRO_MASK(aspect) __MASK(63 - (32 + (aspect))) /* Aspect number to problem state aspect mask */
I think PR is a better shorthand for problem state than PRO. It's just
more commonly used.
We also have PPC_BIT and PPC_BITMASK, _BIT being used for single-bit
mask. So this could be -
#define DEXCR_PR_BIT(aspect) PPC_BIT(32 + (aspect))
Or maybe DEXCR_PR_ASPECT_BIT.
You possibly don't need the ifdef here because CPU_FTR_ARCH_31 should
fold away. Some of the others do because they're using open-coded
access to struct members, but if you're using accessor functions to
get and set such things, there may be no need to.
I think my preference is for your style.
Thanks,
Nick
On Mon Nov 28, 2022 at 12:44 PM AEST, Benjamin Gray wrote:
quoted hunk
The ISA 3.1B hashst and hashchk instructions use a per-cpu SPR HASHKEYR
to hold a key used in the hash calculation. This key should be different
for each process to make it harder for a malicious process to recreate
valid hash values for a victim process.
Add support for storing a per-thread hash key, and setting/clearing
HASHKEYR appropriately.
Signed-off-by: Benjamin Gray <redacted>
---
arch/powerpc/include/asm/book3s/64/kexec.h | 3 +++
arch/powerpc/include/asm/processor.h | 1 +
arch/powerpc/include/asm/reg.h | 1 +
arch/powerpc/kernel/process.c | 12 ++++++++++++
4 files changed, 17 insertions(+)
@@ -24,6 +24,9 @@ static inline void reset_sprs(void)if(cpu_has_feature(CPU_FTR_ARCH_31))mtspr(SPRN_DEXCR,0);+if(cpu_has_feature(CPU_FTR_DEXCR_NPHIE))+mtspr(SPRN_HASHKEYR,0);+/* Do we need isync()? We are going via a kexec reset */isync();}
I wonder if we'd want to avoid switching it when switching to kernel
threads, and from kernel thread back to the same user thread. Might
want to optimise it to do that in future but for an initial enablement
patch this is okay.
quoted hunk
+
if (cpu_has_feature(CPU_FTR_ARCH_31)) {
unsigned long new_dexcr = get_thread_dexcr(new_thread);
Similar comment about your accessor style, if we had get/set_thread_hashkeyr()
functions then no ifdef required.
I think it is not quite per-process? I don't actually know how the user
toolchain side is put together, but I'm thinking we can not give it a new
salt on fork(), but we could on exec(). I think we could actually give
each thread their own salt within a process too, right?
I don't know off the top of my head whether that can be translated into
a simple test at the copy_thread level. For now you're giving out a new
salt on exec I think, which should be fine at least to start with.
Thanks,
Nick
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
quoted hunk
/*
* Run with the current AMR value of the kernel
@@ -1947,6 +1954,11 @@ void start_thread(struct pt_regs *regs, unsigned long start, unsigned long sp) current->thread.load_tm = 0; #endif /* CONFIG_PPC_TRANSACTIONAL_MEM */ #ifdef CONFIG_PPC_BOOK3S_64+ if (cpu_has_feature(CPU_FTR_DEXCR_NPHIE)) {+ current->thread.hashkeyr = get_random_long();+ mtspr(SPRN_HASHKEYR, current->thread.hashkeyr);+ }+ if (cpu_has_feature(CPU_FTR_ARCH_31)) mtspr(SPRN_DEXCR, get_thread_dexcr(¤t->thread)); #endif /* CONFIG_PPC_BOOK3S_64 */
On Mon Nov 28, 2022 at 12:44 PM AEST, Benjamin Gray wrote:
Adds the definitions and generic handler for prctl control of the
PowerPC Dynamic Execution Control Register (DEXCR).
Assuming we'd go with the later prctl patches, this prep patch
is nice way to split out some of the mechanism.
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
On Mon Nov 28, 2022 at 12:44 PM AEST, Benjamin Gray wrote:
Adds an initial prctl interface implementation. Unprivileged processes
can query the current prctl setting, including whether an aspect is
implemented by the hardware or is permitted to be modified by a setter
prctl. Editable aspects can be changed by a CAP_SYS_ADMIN privileged
process.
The prctl setting represents what the process itself has requested, and
does not account for any overrides. Either the kernel or a hypervisor
may enforce a different setting for an aspect.
Userspace can access a readonly view of the current DEXCR via SPR 812,
and a readonly view of the aspects enforced by the hypervisor via
SPR 455. A bitwise OR of these two SPRs will give the effective
DEXCR aspect state of the process.
You said (offline) that you were looking at the PR_SPEC_* speculation
control APIs but that this was different enough that you needed a
different one.
It would be good to know what some of those issues were in the
changelog, would be nice to have some docs (could we add something
to spec_ctrl.rst maybe?). I assume at least one difference is that
some of our bits are not speculative but architectural (e.g., the
stack hash check).
I also wonder if we could implement some of the PR_SPEC controls
APIs by mapping relevant DEXCR aspects to them instead of (or as well
as) the DEXCR controls? Or would the PR_SPEC users be amenable to
extensions that make our usage fit a bit better?
I'm just thinking if we can reduce reliance on arch specific APIs a
bit would be nice.
Hmm, what's the mask doing here? It only gets bits set and never
cleared AFAIKS. What is different between an initial state and a
SET then CLEAR state?
Thanks,
Nick
On Mon Nov 28, 2022 at 12:44 PM AEST, Benjamin Gray wrote:
The DEXCR Speculative Branch Hint Enable (SBHE) aspect controls whether
the hints provided by BO field of Branch instructions are obeyed during
speculative execution.
SBHE behaviour per ISA 3.1B:
0: The hints provided by BO field of Branch instructions may be
ignored during speculative execution
1: The hints provided by BO field of Branch instructions are obeyed
during speculative execution
Add a sysctl entry to allow changing this aspect globally in the system
at runtime:
/proc/sys/kernel/speculative_branch_hint_enable
Three values are supported:
-1: Disable DEXCR SBHE sysctl override
0: Override and set DEXCR[SBHE] aspect to 0
1: Override and set DEXCR[SBHE] aspect to 1
Internally, introduces a mechanism to apply arbitrary system wide
overrides on top of the prctl() config.
Why have an override for this, and not others?
Thanks,
Nick
From: Benjamin Gray <hidden> Date: 2023-03-07 05:38:17
On Tue, 2023-03-07 at 15:05 +1000, Nicholas Piggin wrote:
I think it is not quite per-process? I don't actually know how the
user
toolchain side is put together, but I'm thinking we can not give it a
new
salt on fork(), but we could on exec(). I think we could actually
give
each thread their own salt within a process too, right?
Yeah, the error case is we return further than we called in a given
execution context. A forked child may return after the fork, meaning it
needs the same key as the parent for the hashchk to work. Exec can get
a new key because we can't return with any existing hashes. I haven't
seen enough of kernel thread support to know if/how we can give threads
their own key. I believe they go through the fork() call that copies
the parent key currently.
On Mon Nov 28, 2022 at 12:44 PM AEST, Benjamin Gray wrote:
Describe the DEXCR and document how to interact with it via the
prctl and sysctl interfaces.
Oh you've got the docs here, sorry. Thanks for that. I don't know enough
yet to give much useful feedback on the API. I think at least all the
mechanism stuff up to the prctl API looks pretty straightfoward so would
like to get that merged if we can.
Might need a bit more time and discussion on the API. Interestingly
because the hashchk aspect is architectural, we may not be able to
necessarily sanely enable that, because if it was disabled to start
out with, our callchain up to the prctl call I think would have no
return hashes set so we'd immediately fail on our first return.
Thanks,
Nick
From: Benjamin Gray <hidden> Date: 2023-03-07 05:52:25
On Tue, 2023-03-07 at 15:40 +1000, Nicholas Piggin wrote:
Might need a bit more time and discussion on the API. Interestingly
because the hashchk aspect is architectural, we may not be able to
necessarily sanely enable that, because if it was disabled to start
out with, our callchain up to the prctl call I think would have no
return hashes set so we'd immediately fail on our first return.
I assumed it could eventually be supported in whatever startup wrapper
programs are built with, so either as one of the first things before
any calls, or the compiler could skip putting hash instructions in the
wrapper altogether. The ELF file itself might even be able to request
bits be enabled, so the kernel would start the process correctly.
As it is inherited, it's also possible for a wrapper program to set a
specific DEXCR before a child runs. E.g.,
fork();
prctl(...NPHIE...);
exec();
I hadn't thought of the prctl call itself causing an unbalanced hashchk
when it returns, but that should be solvable with an inline syscall.
From: Benjamin Gray <hidden> Date: 2023-03-07 05:58:44
On Tue, 2023-03-07 at 15:30 +1000, Nicholas Piggin wrote:
On Mon Nov 28, 2022 at 12:44 PM AEST, Benjamin Gray wrote:
quoted
The DEXCR Speculative Branch Hint Enable (SBHE) aspect controls
whether
the hints provided by BO field of Branch instructions are obeyed
during
speculative execution.
SBHE behaviour per ISA 3.1B:
0: The hints provided by BO field of Branch instructions may
be
ignored during speculative execution
1: The hints provided by BO field of Branch instructions are
obeyed
during speculative execution
Add a sysctl entry to allow changing this aspect globally in the
system
at runtime:
/proc/sys/kernel/speculative_branch_hint_enable
Three values are supported:
-1: Disable DEXCR SBHE sysctl override
0: Override and set DEXCR[SBHE] aspect to 0
1: Override and set DEXCR[SBHE] aspect to 1
Internally, introduces a mechanism to apply arbitrary system wide
overrides on top of the prctl() config.
Why have an override for this, and not others?
Should be in the commit message of course, but this aspect bleeds over
to other processes, so a user may wish to prevent all processes from
changing the value. The other aspects are probably only relevant to
their own process, though the implementation here should support
arbitrary system wide overrides.
We potentially don't need to use CPU_FTR bits for each of these. We
only really want them to use instruction patching and make feature
tests fast. But we have been a bit liberal with using them and they
are kind of tied into cpu feature parsing code so maybe it's easier
to go with them for now.
For the static only DEXCR series I've only got CPU_FTR_DEXCR_NPHIE
because that's needed for hashkey updates. The others don't really
matter; they are only interesting for masking out unsupported bits.
Masking itself seems to be unnecessary; the DEXCR will just ignore
unsupported bits. Attempting to set all bits on a P10 showed the first
8 were set and the remainder stayed 0'd, and the kernel worked fine.
It's definitely easier to use CPU_FTR_* for feature detection from the
PAPR specified blob though. Maybe it would be possible to support a
callback on a match instead of setting a feature flag.
@@ -1802,7 +1809,7 @@ int copy_thread(struct task_struct *p, const
quoted
@@ -1802,7 +1809,7 @@ int copy_thread(struct task_struct *p, const
unsigned long start, unsigned long sp)
current->thread.tm_tfiar = 0;
current->thread.load_tm = 0;
#endif /* CONFIG_PPC_TRANSACTIONAL_MEM */
+#ifdef CONFIG_PPC_BOOK3S_64
+ if (cpu_has_feature(CPU_FTR_ARCH_31))
+ mtspr(SPRN_DEXCR, get_thread_dexcr(¤t-
quoted
thread));
+#endif /* CONFIG_PPC_BOOK3S_64 */
You possibly don't need the ifdef here because CPU_FTR_ARCH_31 should
fold away. Some of the others do because they're using open-coded
access to struct members, but if you're using accessor functions to
get and set such things, there may be no need to.
I think my preference is for your style.
I've been revisiting where the DEXCR is initialised and updated. With
the static DEXCR, the thread value is just a field on the task struct
like the others.
On Tue Mar 7, 2023 at 3:37 PM AEST, Benjamin Gray wrote:
On Tue, 2023-03-07 at 15:05 +1000, Nicholas Piggin wrote:
quoted
I think it is not quite per-process? I don't actually know how the
user
toolchain side is put together, but I'm thinking we can not give it a
new
salt on fork(), but we could on exec(). I think we could actually
give
each thread their own salt within a process too, right?
Yeah, the error case is we return further than we called in a given
execution context. A forked child may return after the fork, meaning it
needs the same key as the parent for the hashchk to work. Exec can get
a new key because we can't return with any existing hashes. I haven't
seen enough of kernel thread support to know if/how we can give threads
their own key. I believe they go through the fork() call that copies
the parent key currently.
Could look at possibly doing per-thread keys afterward but what you're
doing makes sense so no problem.
Thanks,
Nick