2022-11-28 04:04:42

by Benjamin Gray

[permalink] [raw]
Subject: [RFC PATCH 00/13] Add DEXCR support

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/[email protected]/

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


2022-11-28 04:09:01

by Russell Currey

[permalink] [raw]
Subject: Re: [RFC PATCH 00/13] Add DEXCR support

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.

>
> 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/[email protected]/
>
> 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

2022-11-28 04:17:31

by Benjamin Gray

[permalink] [raw]
Subject: [RFC PATCH 02/13] powerpc: Add initial Dynamic Execution Control Register (DEXCR) support

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 <[email protected]>
---
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

diff --git a/arch/powerpc/include/asm/book3s/64/kexec.h b/arch/powerpc/include/asm/book3s/64/kexec.h
index d4b9d476ecba..563baf94a962 100644
--- a/arch/powerpc/include/asm/book3s/64/kexec.h
+++ b/arch/powerpc/include/asm/book3s/64/kexec.h
@@ -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();
}
diff --git a/arch/powerpc/include/asm/cputable.h b/arch/powerpc/include/asm/cputable.h
index 757dbded11dc..03bc192f2d8b 100644
--- a/arch/powerpc/include/asm/cputable.h
+++ b/arch/powerpc/include/asm/cputable.h
@@ -192,6 +192,10 @@ static inline void cpu_feature_keys_init(void) { }
#define CPU_FTR_P9_RADIX_PREFETCH_BUG LONG_ASM_CONST(0x0002000000000000)
#define CPU_FTR_ARCH_31 LONG_ASM_CONST(0x0004000000000000)
#define CPU_FTR_DAWR1 LONG_ASM_CONST(0x0008000000000000)
+#define CPU_FTR_DEXCR_SBHE LONG_ASM_CONST(0x0010000000000000)
+#define CPU_FTR_DEXCR_IBRTPD LONG_ASM_CONST(0x0020000000000000)
+#define CPU_FTR_DEXCR_SRAPD LONG_ASM_CONST(0x0040000000000000)
+#define CPU_FTR_DEXCR_NPHIE LONG_ASM_CONST(0x0080000000000000)

#ifndef __ASSEMBLY__

@@ -451,7 +455,9 @@ static inline void cpu_feature_keys_init(void) { }
CPU_FTR_CFAR | CPU_FTR_HVMODE | CPU_FTR_VMX_COPY | \
CPU_FTR_DBELL | CPU_FTR_HAS_PPR | CPU_FTR_ARCH_207S | \
CPU_FTR_ARCH_300 | CPU_FTR_ARCH_31 | \
- CPU_FTR_DAWR | CPU_FTR_DAWR1)
+ CPU_FTR_DAWR | CPU_FTR_DAWR1 | \
+ CPU_FTR_DEXCR_SBHE | CPU_FTR_DEXCR_IBRTPD | CPU_FTR_DEXCR_SRAPD | \
+ CPU_FTR_DEXCR_NPHIE)
#define CPU_FTRS_CELL (CPU_FTR_LWSYNC | \
CPU_FTR_PPCAS_ARCH_V2 | CPU_FTR_CTRL | \
CPU_FTR_ALTIVEC_COMP | CPU_FTR_MMCRA | CPU_FTR_SMT | \
diff --git a/arch/powerpc/include/asm/processor.h b/arch/powerpc/include/asm/processor.h
index 631802999d59..0a8a793b8b8b 100644
--- a/arch/powerpc/include/asm/processor.h
+++ b/arch/powerpc/include/asm/processor.h
@@ -446,6 +446,19 @@ int exit_vmx_usercopy(void);
int enter_vmx_ops(void);
void *exit_vmx_ops(void *dest);

+#ifdef CONFIG_PPC_BOOK3S_64
+
+unsigned long get_thread_dexcr(struct thread_struct const *t);
+
+#else
+
+static inline unsigned long get_thread_dexcr(struct thread_struct const *t)
+{
+ return 0;
+}
+
+#endif /* CONFIG_PPC_BOOK3S_64 */
+
#endif /* __KERNEL__ */
#endif /* __ASSEMBLY__ */
#endif /* _ASM_POWERPC_PROCESSOR_H */
diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
index 1e8b2e04e626..cdd1f174c399 100644
--- a/arch/powerpc/include/asm/reg.h
+++ b/arch/powerpc/include/asm/reg.h
@@ -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 */
+#define DEXCR_PRO_SBHE DEXCR_PRO_MASK(0) /* Speculative Branch Hint Enable */
+#define DEXCR_PRO_IBRTPD DEXCR_PRO_MASK(3) /* Indirect Branch Recurrent Target Prediction Disable */
+#define DEXCR_PRO_SRAPD DEXCR_PRO_MASK(4) /* Subroutine Return Address Prediction Disable */
+#define DEXCR_PRO_NPHIE DEXCR_PRO_MASK(5) /* Non-Privileged Hash Instruction Enable */
#define SPRN_IC 0x350 /* Virtual Instruction Count */
#define SPRN_VTB 0x351 /* Virtual Time Base */
#define SPRN_LDBAR 0x352 /* LD Base Address Register */
diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
index 9b6146056e48..b112315cfdc2 100644
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -79,6 +79,7 @@ obj-$(CONFIG_VDSO32) += vdso32_wrapper.o
obj-$(CONFIG_PPC_WATCHDOG) += watchdog.o
obj-$(CONFIG_HAVE_HW_BREAKPOINT) += hw_breakpoint.o
obj-$(CONFIG_PPC_DAWR) += dawr.o
+obj-$(CONFIG_PPC_BOOK3S_64) += dexcr.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) += mce.o mce_power.o
diff --git a/arch/powerpc/kernel/dexcr.c b/arch/powerpc/kernel/dexcr.c
new file mode 100644
index 000000000000..32a0a69ff638
--- /dev/null
+++ b/arch/powerpc/kernel/dexcr.c
@@ -0,0 +1,25 @@
+#include <linux/cache.h>
+#include <linux/init.h>
+
+#include <asm/cpu_has_feature.h>
+#include <asm/cputable.h>
+#include <asm/processor.h>
+#include <asm/reg.h>
+
+#define DEFAULT_DEXCR 0
+
+static int __init dexcr_init(void)
+{
+ if (!early_cpu_has_feature(CPU_FTR_ARCH_31))
+ return 0;
+
+ mtspr(SPRN_DEXCR, DEFAULT_DEXCR);
+
+ return 0;
+}
+early_initcall(dexcr_init);
+
+unsigned long get_thread_dexcr(struct thread_struct const *t)
+{
+ return DEFAULT_DEXCR;
+}
diff --git a/arch/powerpc/kernel/dt_cpu_ftrs.c b/arch/powerpc/kernel/dt_cpu_ftrs.c
index c3fb9fdf5bd7..896a48211a37 100644
--- a/arch/powerpc/kernel/dt_cpu_ftrs.c
+++ b/arch/powerpc/kernel/dt_cpu_ftrs.c
@@ -661,6 +661,10 @@ static struct dt_cpu_feature_match __initdata
{"prefix-instructions", feat_enable, 0},
{"matrix-multiply-assist", feat_enable_mma, 0},
{"debug-facilities-v31", feat_enable, CPU_FTR_DAWR1},
+ {"dexcr-speculative-branch-hint-enable", feat_enable, CPU_FTR_DEXCR_SBHE},
+ {"dexcr-indirect-branch-recurrent-target-prediction-disable", feat_enable, CPU_FTR_DEXCR_IBRTPD},
+ {"dexcr-subroutine-return-address-prediction-disable", feat_enable, CPU_FTR_DEXCR_SRAPD},
+ {"dexcr-non-privileged-hash-instruction-enable", feat_enable, CPU_FTR_DEXCR_NPHIE},
};

static bool __initdata using_dt_cpu_ftrs;
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index 67da147fe34d..17d26f652b80 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -1228,6 +1228,13 @@ static inline void restore_sprs(struct thread_struct *old_thread,
if (cpu_has_feature(CPU_FTR_P9_TIDR) &&
old_thread->tidr != new_thread->tidr)
mtspr(SPRN_TIDR, new_thread->tidr);
+
+ if (cpu_has_feature(CPU_FTR_ARCH_31)) {
+ unsigned long new_dexcr = get_thread_dexcr(new_thread);
+
+ if (new_dexcr != get_thread_dexcr(old_thread))
+ mtspr(SPRN_DEXCR, new_dexcr);
+ }
#endif

}
@@ -1802,7 +1809,7 @@ int copy_thread(struct task_struct *p, const struct kernel_clone_args *args)

setup_ksp_vsid(p, sp);

-#ifdef CONFIG_PPC64
+#ifdef CONFIG_PPC64
if (cpu_has_feature(CPU_FTR_DSCR)) {
p->thread.dscr_inherit = current->thread.dscr_inherit;
p->thread.dscr = mfspr(SPRN_DSCR);
@@ -1939,6 +1946,10 @@ void start_thread(struct pt_regs *regs, 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(&current->thread));
+#endif /* CONFIG_PPC_BOOK3S_64 */
}
EXPORT_SYMBOL(start_thread);

diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
index 1eed87d954ba..eff250e1ae9a 100644
--- a/arch/powerpc/kernel/prom.c
+++ b/arch/powerpc/kernel/prom.c
@@ -180,6 +180,10 @@ static struct ibm_feature ibm_pa_features[] __initdata = {
.cpu_user_ftrs2 = PPC_FEATURE2_HTM_COMP | PPC_FEATURE2_HTM_NOSC_COMP },

{ .pabyte = 64, .pabit = 0, .cpu_features = CPU_FTR_DAWR1 },
+ { .pabyte = 68, .pabit = 0, .cpu_features = CPU_FTR_DEXCR_SBHE },
+ { .pabyte = 68, .pabit = 3, .cpu_features = CPU_FTR_DEXCR_IBRTPD },
+ { .pabyte = 68, .pabit = 4, .cpu_features = CPU_FTR_DEXCR_SRAPD },
+ { .pabyte = 68, .pabit = 5, .cpu_features = CPU_FTR_DEXCR_NPHIE },
};

/*
--
2.38.1

2022-11-28 04:21:58

by Benjamin Gray

[permalink] [raw]
Subject: [RFC PATCH 11/13] selftests/powerpc: Add DEXCR prctl, sysctl interface test

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 <[email protected]>
---
.../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

diff --git a/tools/testing/selftests/powerpc/dexcr/.gitignore b/tools/testing/selftests/powerpc/dexcr/.gitignore
index 37adb7f47832..035a1fcd8fb3 100644
--- a/tools/testing/selftests/powerpc/dexcr/.gitignore
+++ b/tools/testing/selftests/powerpc/dexcr/.gitignore
@@ -1 +1,2 @@
+dexcr_test
hashchk_user
diff --git a/tools/testing/selftests/powerpc/dexcr/Makefile b/tools/testing/selftests/powerpc/dexcr/Makefile
index 4b4380d4d986..9814e72a4afa 100644
--- a/tools/testing/selftests/powerpc/dexcr/Makefile
+++ b/tools/testing/selftests/powerpc/dexcr/Makefile
@@ -1,4 +1,4 @@
-TEST_GEN_PROGS := hashchk_test
+TEST_GEN_PROGS := dexcr_test hashchk_test

TEST_FILES := settings
top_srcdir = ../../../../..
@@ -6,4 +6,4 @@ include ../../lib.mk

HASHCHK_TEST_CFLAGS = -no-pie $(call cc-option,-mno-rop-protect)

-$(TEST_GEN_PROGS): ../harness.c ../utils.c ./dexcr.c
+$(TEST_GEN_PROGS): ../harness.c ../utils.c ./dexcr.c ./cap.c
diff --git a/tools/testing/selftests/powerpc/dexcr/cap.c b/tools/testing/selftests/powerpc/dexcr/cap.c
new file mode 100644
index 000000000000..3c9b1f27345d
--- /dev/null
+++ b/tools/testing/selftests/powerpc/dexcr/cap.c
@@ -0,0 +1,72 @@
+#include <linux/capability.h>
+#include <string.h>
+#include <sys/syscall.h>
+
+#include "cap.h"
+#include "utils.h"
+
+struct kernel_capabilities {
+ struct __user_cap_header_struct header;
+
+ struct __user_cap_data_struct data[_LINUX_CAPABILITY_U32S_3];
+};
+
+static void get_caps(struct kernel_capabilities *caps)
+{
+ FAIL_IF_EXIT_MSG(syscall(SYS_capget, &caps->header, &caps->data),
+ "cannot get capabilities");
+}
+
+static void set_caps(struct kernel_capabilities *caps)
+{
+ FAIL_IF_EXIT_MSG(syscall(SYS_capset, &caps->header, &caps->data),
+ "cannot set capabilities");
+}
+
+static void init_caps(struct kernel_capabilities *caps, pid_t pid)
+{
+ memset(caps, 0, sizeof(*caps));
+
+ caps->header.version = _LINUX_CAPABILITY_VERSION_3;
+ caps->header.pid = pid;
+
+ get_caps(caps);
+}
+
+static bool has_cap(struct kernel_capabilities *caps, size_t cap)
+{
+ size_t data_index = cap / 32;
+ size_t offset = cap % 32;
+
+ FAIL_IF_EXIT_MSG(data_index >= ARRAY_SIZE(caps->data), "cap out of range");
+
+ return caps->data[data_index].effective & (1 << offset);
+}
+
+static void drop_cap(struct kernel_capabilities *caps, size_t cap)
+{
+ size_t data_index = cap / 32;
+ size_t offset = cap % 32;
+
+ FAIL_IF_EXIT_MSG(data_index >= ARRAY_SIZE(caps->data), "cap out of range");
+
+ caps->data[data_index].effective &= ~(1 << offset);
+}
+
+bool check_cap_sysadmin(void)
+{
+ struct kernel_capabilities caps;
+
+ init_caps(&caps, 0);
+
+ return has_cap(&caps, CAP_SYS_ADMIN);
+}
+
+void drop_cap_sysadmin(void)
+{
+ struct kernel_capabilities caps;
+
+ init_caps(&caps, 0);
+ drop_cap(&caps, CAP_SYS_ADMIN);
+ set_caps(&caps);
+}
diff --git a/tools/testing/selftests/powerpc/dexcr/cap.h b/tools/testing/selftests/powerpc/dexcr/cap.h
new file mode 100644
index 000000000000..41f41dda9862
--- /dev/null
+++ b/tools/testing/selftests/powerpc/dexcr/cap.h
@@ -0,0 +1,18 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Simple capabilities getter/setter
+ *
+ * This header file contains helper functions and macros
+ * required to get and set capabilities(7). Introduced so
+ * we aren't the first to rely on libcap.
+ */
+#ifndef _SELFTESTS_POWERPC_DEXCR_CAP_H
+#define _SELFTESTS_POWERPC_DEXCR_CAP_H
+
+#include <stdbool.h>
+
+bool check_cap_sysadmin(void);
+
+void drop_cap_sysadmin(void);
+
+#endif /* _SELFTESTS_POWERPC_DEXCR_CAP_H */
diff --git a/tools/testing/selftests/powerpc/dexcr/dexcr.h b/tools/testing/selftests/powerpc/dexcr/dexcr.h
index fb8007bf19f8..b90633ae49e9 100644
--- a/tools/testing/selftests/powerpc/dexcr/dexcr.h
+++ b/tools/testing/selftests/powerpc/dexcr/dexcr.h
@@ -21,6 +21,8 @@
#define DEXCR_PRO_SRAPD DEXCR_PRO_MASK(4)
#define DEXCR_PRO_NPHIE DEXCR_PRO_MASK(5)

+#define SYSCTL_DEXCR_SBHE "/proc/sys/kernel/speculative_branch_hint_enable"
+
enum DexcrSource {
UDEXCR, /* Userspace DEXCR value */
ENFORCED, /* Enforced by hypervisor */
diff --git a/tools/testing/selftests/powerpc/dexcr/dexcr_test.c b/tools/testing/selftests/powerpc/dexcr/dexcr_test.c
new file mode 100644
index 000000000000..5446cb350c84
--- /dev/null
+++ b/tools/testing/selftests/powerpc/dexcr/dexcr_test.c
@@ -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"
+
+/*
+ * Test that an editable aspect
+ * - Current prctl state reported by the getter
+ * - Can be toggled on and off when process has CAP_SYS_ADMIN
+ * - Can't be edited if CAP_SYS_ADMIN not present
+ * - Can't be modified after force set
+ */
+static int dexcr_prctl_editable_aspect_test(unsigned long which)
+{
+ pid_t pid;
+
+ 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");
+
+ return 0;
+}
+
+static int dexcr_prctl_sbhe_test(void)
+{
+ sysctl_set_sbhe(-1);
+ return dexcr_prctl_editable_aspect_test(PR_PPC_DEXCR_SBHE);
+}
+
+static int dexcr_prctl_ibrtpd_test(void)
+{
+ return dexcr_prctl_editable_aspect_test(PR_PPC_DEXCR_IBRTPD);
+}
+
+static int dexcr_prctl_srapd_test(void)
+{
+ return dexcr_prctl_editable_aspect_test(PR_PPC_DEXCR_SRAPD);
+}
+
+static int dexcr_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");
+
+ return 0;
+}
+
+static int dexcr_test_inherit_execve(char expected_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;
+ }
+
+ return 0;
+}
+
+/*
+ * Check that a child process inherits the DEXCR over fork and execve
+ */
+static int dexcr_inherit_test(void)
+{
+ pid_t pid;
+
+ 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);
+
+ return 0;
+}
+
+int main(int argc, char *argv[])
+{
+ int err = 0;
+
+ if (argc >= 2 && strcmp(argv[0], "dexcr_test_inherit_execve") == 0)
+ return dexcr_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");
+
+ return err;
+}
--
2.38.1

2023-03-07 04:46:57

by Nicholas Piggin

[permalink] [raw]
Subject: Re: [RFC PATCH 02/13] powerpc: Add initial Dynamic Execution Control Register (DEXCR) support

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 <[email protected]>

> Signed-off-by: Benjamin Gray <[email protected]>
> ---
> 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
>
> diff --git a/arch/powerpc/include/asm/book3s/64/kexec.h b/arch/powerpc/include/asm/book3s/64/kexec.h
> index d4b9d476ecba..563baf94a962 100644
> --- a/arch/powerpc/include/asm/book3s/64/kexec.h
> +++ b/arch/powerpc/include/asm/book3s/64/kexec.h
> @@ -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();
> }
> diff --git a/arch/powerpc/include/asm/cputable.h b/arch/powerpc/include/asm/cputable.h
> index 757dbded11dc..03bc192f2d8b 100644
> --- a/arch/powerpc/include/asm/cputable.h
> +++ b/arch/powerpc/include/asm/cputable.h
> @@ -192,6 +192,10 @@ static inline void cpu_feature_keys_init(void) { }
> #define CPU_FTR_P9_RADIX_PREFETCH_BUG LONG_ASM_CONST(0x0002000000000000)
> #define CPU_FTR_ARCH_31 LONG_ASM_CONST(0x0004000000000000)
> #define CPU_FTR_DAWR1 LONG_ASM_CONST(0x0008000000000000)
> +#define CPU_FTR_DEXCR_SBHE LONG_ASM_CONST(0x0010000000000000)
> +#define CPU_FTR_DEXCR_IBRTPD LONG_ASM_CONST(0x0020000000000000)
> +#define CPU_FTR_DEXCR_SRAPD LONG_ASM_CONST(0x0040000000000000)
> +#define CPU_FTR_DEXCR_NPHIE LONG_ASM_CONST(0x0080000000000000)

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.

>
> #ifndef __ASSEMBLY__
>
> @@ -451,7 +455,9 @@ static inline void cpu_feature_keys_init(void) { }
> CPU_FTR_CFAR | CPU_FTR_HVMODE | CPU_FTR_VMX_COPY | \
> CPU_FTR_DBELL | CPU_FTR_HAS_PPR | CPU_FTR_ARCH_207S | \
> CPU_FTR_ARCH_300 | CPU_FTR_ARCH_31 | \
> - CPU_FTR_DAWR | CPU_FTR_DAWR1)
> + CPU_FTR_DAWR | CPU_FTR_DAWR1 | \
> + CPU_FTR_DEXCR_SBHE | CPU_FTR_DEXCR_IBRTPD | CPU_FTR_DEXCR_SRAPD | \
> + CPU_FTR_DEXCR_NPHIE)
> #define CPU_FTRS_CELL (CPU_FTR_LWSYNC | \
> CPU_FTR_PPCAS_ARCH_V2 | CPU_FTR_CTRL | \
> CPU_FTR_ALTIVEC_COMP | CPU_FTR_MMCRA | CPU_FTR_SMT | \
> diff --git a/arch/powerpc/include/asm/processor.h b/arch/powerpc/include/asm/processor.h
> index 631802999d59..0a8a793b8b8b 100644
> --- a/arch/powerpc/include/asm/processor.h
> +++ b/arch/powerpc/include/asm/processor.h
> @@ -446,6 +446,19 @@ int exit_vmx_usercopy(void);
> int enter_vmx_ops(void);
> void *exit_vmx_ops(void *dest);
>
> +#ifdef CONFIG_PPC_BOOK3S_64
> +
> +unsigned long get_thread_dexcr(struct thread_struct const *t);
> +
> +#else
> +
> +static inline unsigned long get_thread_dexcr(struct thread_struct const *t)
> +{
> + return 0;
> +}
> +
> +#endif /* CONFIG_PPC_BOOK3S_64 */
> +
> #endif /* __KERNEL__ */
> #endif /* __ASSEMBLY__ */
> #endif /* _ASM_POWERPC_PROCESSOR_H */
> diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
> index 1e8b2e04e626..cdd1f174c399 100644
> --- a/arch/powerpc/include/asm/reg.h
> +++ b/arch/powerpc/include/asm/reg.h
> @@ -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.

> +#define DEXCR_PRO_SBHE DEXCR_PRO_MASK(0) /* Speculative Branch Hint Enable */
> +#define DEXCR_PRO_IBRTPD DEXCR_PRO_MASK(3) /* Indirect Branch Recurrent Target Prediction Disable */
> +#define DEXCR_PRO_SRAPD DEXCR_PRO_MASK(4) /* Subroutine Return Address Prediction Disable */
> +#define DEXCR_PRO_NPHIE DEXCR_PRO_MASK(5) /* Non-Privileged Hash Instruction Enable */
> #define SPRN_IC 0x350 /* Virtual Instruction Count */
> #define SPRN_VTB 0x351 /* Virtual Time Base */
> #define SPRN_LDBAR 0x352 /* LD Base Address Register */
> diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
> index 9b6146056e48..b112315cfdc2 100644
> --- a/arch/powerpc/kernel/Makefile
> +++ b/arch/powerpc/kernel/Makefile
> @@ -79,6 +79,7 @@ obj-$(CONFIG_VDSO32) += vdso32_wrapper.o
> obj-$(CONFIG_PPC_WATCHDOG) += watchdog.o
> obj-$(CONFIG_HAVE_HW_BREAKPOINT) += hw_breakpoint.o
> obj-$(CONFIG_PPC_DAWR) += dawr.o
> +obj-$(CONFIG_PPC_BOOK3S_64) += dexcr.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) += mce.o mce_power.o
> diff --git a/arch/powerpc/kernel/dexcr.c b/arch/powerpc/kernel/dexcr.c
> new file mode 100644
> index 000000000000..32a0a69ff638
> --- /dev/null
> +++ b/arch/powerpc/kernel/dexcr.c
> @@ -0,0 +1,25 @@
> +#include <linux/cache.h>
> +#include <linux/init.h>
> +
> +#include <asm/cpu_has_feature.h>
> +#include <asm/cputable.h>
> +#include <asm/processor.h>
> +#include <asm/reg.h>
> +
> +#define DEFAULT_DEXCR 0
> +
> +static int __init dexcr_init(void)
> +{
> + if (!early_cpu_has_feature(CPU_FTR_ARCH_31))
> + return 0;
> +
> + mtspr(SPRN_DEXCR, DEFAULT_DEXCR);
> +
> + return 0;
> +}
> +early_initcall(dexcr_init);
> +
> +unsigned long get_thread_dexcr(struct thread_struct const *t)
> +{
> + return DEFAULT_DEXCR;
> +}
> diff --git a/arch/powerpc/kernel/dt_cpu_ftrs.c b/arch/powerpc/kernel/dt_cpu_ftrs.c
> index c3fb9fdf5bd7..896a48211a37 100644
> --- a/arch/powerpc/kernel/dt_cpu_ftrs.c
> +++ b/arch/powerpc/kernel/dt_cpu_ftrs.c
> @@ -661,6 +661,10 @@ static struct dt_cpu_feature_match __initdata
> {"prefix-instructions", feat_enable, 0},
> {"matrix-multiply-assist", feat_enable_mma, 0},
> {"debug-facilities-v31", feat_enable, CPU_FTR_DAWR1},
> + {"dexcr-speculative-branch-hint-enable", feat_enable, CPU_FTR_DEXCR_SBHE},
> + {"dexcr-indirect-branch-recurrent-target-prediction-disable", feat_enable, CPU_FTR_DEXCR_IBRTPD},
> + {"dexcr-subroutine-return-address-prediction-disable", feat_enable, CPU_FTR_DEXCR_SRAPD},
> + {"dexcr-non-privileged-hash-instruction-enable", feat_enable, CPU_FTR_DEXCR_NPHIE},
> };
>
> static bool __initdata using_dt_cpu_ftrs;
> diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
> index 67da147fe34d..17d26f652b80 100644
> --- a/arch/powerpc/kernel/process.c
> +++ b/arch/powerpc/kernel/process.c
> @@ -1228,6 +1228,13 @@ static inline void restore_sprs(struct thread_struct *old_thread,
> if (cpu_has_feature(CPU_FTR_P9_TIDR) &&
> old_thread->tidr != new_thread->tidr)
> mtspr(SPRN_TIDR, new_thread->tidr);
> +
> + if (cpu_has_feature(CPU_FTR_ARCH_31)) {
> + unsigned long new_dexcr = get_thread_dexcr(new_thread);
> +
> + if (new_dexcr != get_thread_dexcr(old_thread))
> + mtspr(SPRN_DEXCR, new_dexcr);
> + }
> #endif
>
> }
> @@ -1802,7 +1809,7 @@ int copy_thread(struct task_struct *p, const struct kernel_clone_args *args)
>
> setup_ksp_vsid(p, sp);
>
> -#ifdef CONFIG_PPC64
> +#ifdef CONFIG_PPC64
> if (cpu_has_feature(CPU_FTR_DSCR)) {
> p->thread.dscr_inherit = current->thread.dscr_inherit;
> p->thread.dscr = mfspr(SPRN_DSCR);
> @@ -1939,6 +1946,10 @@ void start_thread(struct pt_regs *regs, 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(&current->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.

Thanks,
Nick

> }
> EXPORT_SYMBOL(start_thread);
>
> diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
> index 1eed87d954ba..eff250e1ae9a 100644
> --- a/arch/powerpc/kernel/prom.c
> +++ b/arch/powerpc/kernel/prom.c
> @@ -180,6 +180,10 @@ static struct ibm_feature ibm_pa_features[] __initdata = {
> .cpu_user_ftrs2 = PPC_FEATURE2_HTM_COMP | PPC_FEATURE2_HTM_NOSC_COMP },
>
> { .pabyte = 64, .pabit = 0, .cpu_features = CPU_FTR_DAWR1 },
> + { .pabyte = 68, .pabit = 0, .cpu_features = CPU_FTR_DEXCR_SBHE },
> + { .pabyte = 68, .pabit = 3, .cpu_features = CPU_FTR_DEXCR_IBRTPD },
> + { .pabyte = 68, .pabit = 4, .cpu_features = CPU_FTR_DEXCR_SRAPD },
> + { .pabyte = 68, .pabit = 5, .cpu_features = CPU_FTR_DEXCR_NPHIE },
> };
>
> /*
> --
> 2.38.1


2023-03-09 23:47:26

by Benjamin Gray

[permalink] [raw]
Subject: Re: [RFC PATCH 02/13] powerpc: Add initial Dynamic Execution Control Register (DEXCR) support

On Tue, 2023-03-07 at 14:45 +1000, Nicholas Piggin wrote:
> On Mon Nov 28, 2022 at 12:44 PM AEST, Benjamin Gray wrote:
> > diff --git a/arch/powerpc/include/asm/cputable.h
> > b/arch/powerpc/include/asm/cputable.h
> > index 757dbded11dc..03bc192f2d8b 100644
> > --- a/arch/powerpc/include/asm/cputable.h
> > +++ b/arch/powerpc/include/asm/cputable.h
> > @@ -192,6 +192,10 @@ static inline void cpu_feature_keys_init(void)
> > { }
> >  #define
> > CPU_FTR_P9_RADIX_PREFETCH_BUG  LONG_ASM_CONST(0x0002000000000000)
> >  #define
> > CPU_FTR_ARCH_31                        LONG_ASM_CONST(0x00040000000
> > 00000)
> >  #define
> > CPU_FTR_DAWR1                  LONG_ASM_CONST(0x0008000000000000)
> > +#define
> > CPU_FTR_DEXCR_SBHE             LONG_ASM_CONST(0x0010000000000000)
> > +#define
> > CPU_FTR_DEXCR_IBRTPD           LONG_ASM_CONST(0x0020000000000000)
> > +#define
> > CPU_FTR_DEXCR_SRAPD            LONG_ASM_CONST(0x0040000000000000)
> > +#define
> > CPU_FTR_DEXCR_NPHIE            LONG_ASM_CONST(0x0080000000000000)
>
> 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
>
>

> > @@ -1802,7 +1809,7 @@ int copy_thread(struct task_struct *p, const
> > struct kernel_clone_args *args)
> >  
> >         setup_ksp_vsid(p, sp);
> >  
> > -#ifdef CONFIG_PPC64
> > +#ifdef CONFIG_PPC64
> >         if (cpu_has_feature(CPU_FTR_DSCR)) {
> >                 p->thread.dscr_inherit = current-
> > >thread.dscr_inherit;
> >                 p->thread.dscr = mfspr(SPRN_DSCR);
> > @@ -1939,6 +1946,10 @@ void start_thread(struct pt_regs *regs,
> > 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(&current-
> > >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.