* Re: [PATCH] pseries: Fix 64 bit logical memory block panic
From: Michael Ellerman @ 2020-07-16 12:55 UTC (permalink / raw)
To: paulus, mpe, Anton Blanchard, nathanl, benh; +Cc: linuxppc-dev
In-Reply-To: <20200715000820.1255764-1-anton@ozlabs.org>
On Wed, 15 Jul 2020 10:08:20 +1000, Anton Blanchard wrote:
> Booting with a 4GB LMB size causes us to panic:
>
> qemu-system-ppc64: OS terminated: OS panic:
> Memory block size not suitable: 0x0
>
> Fix pseries_memory_block_size() to handle 64 bit LMBs.
Applied to powerpc/next.
[1/1] pseries: Fix 64 bit logical memory block panic
https://git.kernel.org/powerpc/c/89c140bbaeee7a55ed0360a88f294ead2b95201b
cheers
^ permalink raw reply
* Re: [PATCH] xmon: Reset RCU and soft lockup watchdogs
From: Michael Ellerman @ 2020-07-16 12:55 UTC (permalink / raw)
To: Anton Blanchard, linuxppc-dev; +Cc: Paul Mackerras, Nicholas Piggin
In-Reply-To: <20200630100218.62a3c3fb@kryten.localdomain>
On Tue, 30 Jun 2020 10:02:18 +1000, Anton Blanchard wrote:
> I'm seeing RCU warnings when exiting xmon. xmon resets the NMI watchdog,
> but does nothing with the RCU stall or soft lockup watchdogs. Add a
> helper function that handles all three.
Applied to powerpc/next.
[1/1] powerpc/xmon: Reset RCU and soft lockup watchdogs
https://git.kernel.org/powerpc/c/5c699396f5f6cf6d67055af7b82c270d31fd831a
cheers
^ permalink raw reply
* Re: [PATCH] powerpc/vdso: Fix vdso cpu truncation
From: Michael Ellerman @ 2020-07-16 12:55 UTC (permalink / raw)
To: paulus, mpe, benh, miltonm, Anton Blanchard; +Cc: linuxppc-dev
In-Reply-To: <20200715233704.1352257-1-anton@ozlabs.org>
On Thu, 16 Jul 2020 09:37:04 +1000, Anton Blanchard wrote:
> The code in vdso_cpu_init that exposes the cpu and numa node to
> userspace via SPRG_VDSO incorrctly masks the cpu to 12 bits. This means
> that any kernel running on a box with more than 4096 threads (NR_CPUS
> advertises a limit of of 8192 cpus) would expose userspace to two cpu
> contexts running at the same time with the same cpu number.
>
> Note: I'm not aware of any distro shipping a kernel with support for more
> than 4096 threads today, nor of any system image that currently exceeds
> 4096 threads. Found via code browsing.
Applied to powerpc/next.
[1/1] powerpc/vdso: Fix vdso cpu truncation
https://git.kernel.org/powerpc/c/a9f675f950a07d5c1dbcbb97aabac56f5ed085e3
cheers
^ permalink raw reply
* Re: [PATCH] powerpc: Add cputime_to_nsecs()
From: Michael Ellerman @ 2020-07-16 12:55 UTC (permalink / raw)
To: npiggin, paulus, mpe, Anton Blanchard, benh; +Cc: linuxppc-dev
In-Reply-To: <20200713083601.1103978-1-anton@ozlabs.org>
On Mon, 13 Jul 2020 18:36:01 +1000, Anton Blanchard wrote:
> Generic code has a wrapper to implement cputime_to_nsecs() on top of
> cputime_to_usecs() but we can easily return the full nanosecond
> resolution directly.
Applied to powerpc/next.
[1/1] powerpc: Add cputime_to_nsecs()
https://git.kernel.org/powerpc/c/ade7667a981be49af9310f7c682c226283ec833d
cheers
^ permalink raw reply
* Re: [PATCH v7 0/7] Support new pmem flush and sync instructions for POWER
From: Michael Ellerman @ 2020-07-16 12:55 UTC (permalink / raw)
To: mpe, linux-nvdimm, Aneesh Kumar K.V, dan.j.williams, linuxppc-dev
Cc: msuchanek, Jan Kara
In-Reply-To: <20200701072235.223558-1-aneesh.kumar@linux.ibm.com>
On Wed, 1 Jul 2020 12:52:28 +0530, Aneesh Kumar K.V wrote:
> This patch series enables the usage os new pmem flush and sync instructions on POWER
> architecture. POWER10 introduces two new variants of dcbf instructions (dcbstps and dcbfps)
> that can be used to write modified locations back to persistent storage. Additionally,
> POWER10 also introduce phwsync and plwsync which can be used to establish order of these
> writes to persistent storage.
>
> This series exposes these instructions to the rest of the kernel. The existing
> dcbf and hwsync instructions in P8 and P9 are adequate to enable appropriate
> synchronization with OpenCAPI-hosted persistent storage. Hence the new instructions
> are added as a variant of the old ones that old hardware won't differentiate.
>
> [...]
Applied to powerpc/next.
[1/7] powerpc/pmem: Restrict papr_scm to P8 and above.
https://git.kernel.org/powerpc/c/c83040192f3763b243ece26073d61a895b4a230f
[2/7] powerpc/pmem: Add new instructions for persistent storage and sync
https://git.kernel.org/powerpc/c/32db09d992ddc7d145595cff49cccfe14e018266
[3/7] powerpc/pmem: Add flush routines using new pmem store and sync instruction
https://git.kernel.org/powerpc/c/d358042793183a57094dac45a44116e1165ac593
[4/7] libnvdimm/nvdimm/flush: Allow architecture to override the flush barrier
https://git.kernel.org/powerpc/c/3e79f082ebfc130360bcee23e4dd74729dcafdf4
[5/7] powerpc/pmem: Update ppc64 to use the new barrier instruction.
https://git.kernel.org/powerpc/c/76e6c73f33d4e1cc4de4f25c0bf66d59e42113c4
[6/7] powerpc/pmem: Avoid the barrier in flush routines
https://git.kernel.org/powerpc/c/436499ab868f1a9e497cfdbf641affe8a122c571
[7/7] powerpc/pmem: Initialize pmem device on newer hardware
https://git.kernel.org/powerpc/c/8c26ab72663b4affc31e47cdf77d61d0172d1033
cheers
^ permalink raw reply
* Re: [PATCH V2] powerpc/pseries/svm: Remove unwanted check for shared_lppaca_size
From: Michael Ellerman @ 2020-07-16 12:47 UTC (permalink / raw)
To: Satheesh Rajendran, linuxppc-dev
Cc: Sukadev Bhattiprolu, Laurent Dufour, Ram Pai, linux-kernel,
Thiago Jung Bauermann
In-Reply-To: <20200619070113.16696-1-sathnaga@linux.vnet.ibm.com>
On Fri, 19 Jun 2020 12:31:13 +0530, Satheesh Rajendran wrote:
> Early secure guest boot hits the below crash while booting with
> vcpus numbers aligned with page boundary for PAGE size of 64k
> and LPPACA size of 1k i.e 64, 128 etc, due to the BUG_ON assert
> for shared_lppaca_total_size equal to shared_lppaca_size,
>
> [ 0.000000] Partition configured for 64 cpus.
> [ 0.000000] CPU maps initialized for 1 thread per core
> [ 0.000000] ------------[ cut here ]------------
> [ 0.000000] kernel BUG at arch/powerpc/kernel/paca.c:89!
> [ 0.000000] Oops: Exception in kernel mode, sig: 5 [#1]
> [ 0.000000] LE PAGE_SIZE=64K MMU=Radix SMP NR_CPUS=2048 NUMA pSeries
>
> [...]
Applied to powerpc/fixes.
[1/1] powerpc/pseries/svm: Fix incorrect check for shared_lppaca_size
https://git.kernel.org/powerpc/c/b710d27bf72068b15b2f0305d825988183e2ff28
cheers
^ permalink raw reply
* Re: [PATCH V2 1/2] powerpc/vas: Report proper error code for address translation failure
From: Michael Ellerman @ 2020-07-16 12:47 UTC (permalink / raw)
To: Haren Myneni, mpe; +Cc: tulioqm, linuxppc-dev, abali, rzinsly
In-Reply-To: <019fd53e7538c6f8f332d175df74b1815ef5aa8c.camel@linux.ibm.com>
On Fri, 10 Jul 2020 16:47:19 -0700, Haren Myneni wrote:
> P9 DD2 NX workbook (Table 4-36) says DMA controller uses CC=5
> internally for translation fault handling. NX reserves CC=250 for
> OS to notify user space when NX encounters address translation
> failure on the request buffer. Not an issue in earlier releases
> as NX does not get faults on kernel addresses.
>
> This patch defines CSB_CC_FAULT_ADDRESS(250) and updates CSB.CC with
> this proper error code for user space.
>
> [...]
Applied to powerpc/fixes.
[1/2] powerpc/vas: Report proper error code for address translation failure
https://git.kernel.org/powerpc/c/6068e1a4427e88f5cc62f238d1baf94a8b824ef4
[2/2] selftests/powerpc: Use proper error code to check fault address
https://git.kernel.org/powerpc/c/f0479c4bcbd92d1a457d4a43bcab79f29d11334a
cheers
^ permalink raw reply
* Re: [PATCH] powerpc/book3s64/pkeys: Fix pkey_access_permitted w.r.t execute disable pkey
From: Michael Ellerman @ 2020-07-16 12:47 UTC (permalink / raw)
To: Aneesh Kumar K.V, mpe, linuxppc-dev; +Cc: linuxram, Sandipan Das
In-Reply-To: <20200712132047.1038594-1-aneesh.kumar@linux.ibm.com>
On Sun, 12 Jul 2020 18:50:47 +0530, Aneesh Kumar K.V wrote:
> Even if the IAMR value deny the execute access, current kernel return true
> w.r.t pkey_access_permitted() for execute permission check if the AMR
> read pkey bit is cleared.
>
> This results in repeated page fault loop with a test like below.
>
> #define _GNU_SOURCE
> #include <errno.h>
> #include <stdio.h>
> #include <stdlib.h>
> #include <signal.h>
> #include <inttypes.h>
>
> [...]
Applied to powerpc/fixes.
[1/1] powerpc/book3s64/pkeys: Fix pkey_access_permitted() for execute disable pkey
https://git.kernel.org/powerpc/c/192b6a780598976feb7321ff007754f8511a4129
cheers
^ permalink raw reply
* [PATCH] selftests/powerpc: Run per_event_excludes test on Power8 or later
From: Michael Ellerman @ 2020-07-16 12:21 UTC (permalink / raw)
To: linuxppc-dev
The per_event_excludes test wants to run on Power8 or later. But
currently it checks that AT_BASE_PLATFORM *equals* power8, which means
it only runs on Power8.
Fix it to check for the ISA 2.07 feature, which will be set on Power8
and later CPUs.
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
tools/testing/selftests/powerpc/pmu/per_event_excludes.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/tools/testing/selftests/powerpc/pmu/per_event_excludes.c b/tools/testing/selftests/powerpc/pmu/per_event_excludes.c
index 2756fe2efdc5..2d37942bf72b 100644
--- a/tools/testing/selftests/powerpc/pmu/per_event_excludes.c
+++ b/tools/testing/selftests/powerpc/pmu/per_event_excludes.c
@@ -12,6 +12,8 @@
#include <string.h>
#include <sys/prctl.h>
+#include <asm/cputable.h>
+
#include "event.h"
#include "lib.h"
#include "utils.h"
@@ -23,12 +25,9 @@
static int per_event_excludes(void)
{
struct event *e, events[4];
- char *platform;
int i;
- platform = (char *)get_auxv_entry(AT_BASE_PLATFORM);
- FAIL_IF(!platform);
- SKIP_IF(strcmp(platform, "power8") != 0);
+ SKIP_IF(!have_hwcap2(PPC_FEATURE2_ARCH_2_07));
/*
* We need to create the events disabled, otherwise the running/enabled
--
2.25.1
^ permalink raw reply related
* [PATCH 5/5] selftests/powerpc: Add test for pkey siginfo verification
From: Sandipan Das @ 2020-07-16 11:03 UTC (permalink / raw)
To: mpe; +Cc: fweimer, aneesh.kumar, linuxram, linuxppc-dev, bauerman
In-Reply-To: <cover.1594897099.git.sandipan@linux.ibm.com>
Commit c46241a370a61 ("powerpc/pkeys: Check vma before
returning key fault error to the user") fixes a bug which
causes the kernel to set the wrong pkey in siginfo when a
pkey fault occurs after two competing threads that have
allocated different pkeys, one fully permissive and the
other restrictive, attempt to protect a common page at the
same time. This adds a test to detect the bug.
Signed-off-by: Sandipan Das <sandipan@linux.ibm.com>
---
tools/testing/selftests/powerpc/mm/.gitignore | 1 +
tools/testing/selftests/powerpc/mm/Makefile | 5 +-
.../selftests/powerpc/mm/pkey_siginfo.c | 332 ++++++++++++++++++
3 files changed, 337 insertions(+), 1 deletion(-)
create mode 100644 tools/testing/selftests/powerpc/mm/pkey_siginfo.c
diff --git a/tools/testing/selftests/powerpc/mm/.gitignore b/tools/testing/selftests/powerpc/mm/.gitignore
index 8f841f925baa5..36ec2c4ccdea4 100644
--- a/tools/testing/selftests/powerpc/mm/.gitignore
+++ b/tools/testing/selftests/powerpc/mm/.gitignore
@@ -9,3 +9,4 @@ large_vm_fork_separation
bad_accesses
tlbie_test
pkey_exec_prot
+pkey_siginfo
diff --git a/tools/testing/selftests/powerpc/mm/Makefile b/tools/testing/selftests/powerpc/mm/Makefile
index f9fa0ba7435c4..558b7ccc93932 100644
--- a/tools/testing/selftests/powerpc/mm/Makefile
+++ b/tools/testing/selftests/powerpc/mm/Makefile
@@ -3,7 +3,8 @@ noarg:
$(MAKE) -C ../
TEST_GEN_PROGS := hugetlb_vs_thp_test subpage_prot prot_sao segv_errors wild_bctr \
- large_vm_fork_separation bad_accesses pkey_exec_prot
+ large_vm_fork_separation bad_accesses pkey_exec_prot \
+ pkey_siginfo
TEST_GEN_PROGS_EXTENDED := tlbie_test
TEST_GEN_FILES := tempfile
@@ -18,8 +19,10 @@ $(OUTPUT)/wild_bctr: CFLAGS += -m64
$(OUTPUT)/large_vm_fork_separation: CFLAGS += -m64
$(OUTPUT)/bad_accesses: CFLAGS += -m64
$(OUTPUT)/pkey_exec_prot: CFLAGS += -m64
+$(OUTPUT)/pkey_siginfo: CFLAGS += -m64
$(OUTPUT)/tempfile:
dd if=/dev/zero of=$@ bs=64k count=1
$(OUTPUT)/tlbie_test: LDLIBS += -lpthread
+$(OUTPUT)/pkey_siginfo: LDLIBS += -lpthread
diff --git a/tools/testing/selftests/powerpc/mm/pkey_siginfo.c b/tools/testing/selftests/powerpc/mm/pkey_siginfo.c
new file mode 100644
index 0000000000000..58605c53d495d
--- /dev/null
+++ b/tools/testing/selftests/powerpc/mm/pkey_siginfo.c
@@ -0,0 +1,332 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright 2020, Sandipan Das, IBM Corp.
+ *
+ * Test if the signal information reports the correct memory protection
+ * key upon getting a key access violation fault for a page that was
+ * attempted to be protected by two different keys from two competing
+ * threads at the same time.
+ */
+
+#define _GNU_SOURCE
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <signal.h>
+
+#include <unistd.h>
+#include <pthread.h>
+#include <sys/mman.h>
+
+#include "pkeys.h"
+
+#define PPC_INST_NOP 0x60000000
+#define PPC_INST_BLR 0x4e800020
+#define PROT_RWX (PROT_READ | PROT_WRITE | PROT_EXEC)
+
+#define NUM_ITERATIONS 1000000
+
+static volatile sig_atomic_t perm_pkey, rest_pkey;
+static volatile sig_atomic_t rights, fault_count;
+static volatile unsigned int *volatile fault_addr;
+static pthread_barrier_t iteration_barrier;
+
+static void segv_handler(int signum, siginfo_t *sinfo, void *ctx)
+{
+ void *pgstart;
+ size_t pgsize;
+ int pkey;
+
+ pkey = siginfo_pkey(sinfo);
+
+ /* Check if this fault originated from a pkey access violation */
+ if (sinfo->si_code != SEGV_PKUERR) {
+ sigsafe_err("got a fault for an unexpected reason\n");
+ _exit(1);
+ }
+
+ /* Check if this fault originated from the expected address */
+ if (sinfo->si_addr != (void *) fault_addr) {
+ sigsafe_err("got a fault for an unexpected address\n");
+ _exit(1);
+ }
+
+ /* Check if this fault originated from the restrictive pkey */
+ if (pkey != rest_pkey) {
+ sigsafe_err("got a fault for an unexpected pkey\n");
+ _exit(1);
+ }
+
+ /* Check if too many faults have occurred for the same iteration */
+ if (fault_count > 0) {
+ sigsafe_err("got too many faults for the same address\n");
+ _exit(1);
+ }
+
+ pgsize = getpagesize();
+ pgstart = (void *) ((unsigned long) fault_addr & ~(pgsize - 1));
+
+ /*
+ * If the current fault occurred due to lack of execute rights,
+ * reassociate the page with the exec-only pkey since execute
+ * rights cannot be changed directly for the faulting pkey as
+ * IAMR is inaccessible from userspace.
+ *
+ * Otherwise, if the current fault occurred due to lack of
+ * read-write rights, change the AMR permission bits for the
+ * pkey.
+ *
+ * This will let the test continue.
+ */
+ if (rights == PKEY_DISABLE_EXECUTE &&
+ mprotect(pgstart, pgsize, PROT_EXEC))
+ _exit(1);
+ else
+ pkey_set_rights(pkey, 0);
+
+ fault_count++;
+}
+
+struct region {
+ unsigned long rights;
+ unsigned int *base;
+ size_t size;
+};
+
+static void *protect(void *p)
+{
+ unsigned long rights;
+ unsigned int *base;
+ size_t size;
+ int tid, i;
+
+ tid = gettid();
+ base = ((struct region *) p)->base;
+ size = ((struct region *) p)->size;
+ FAIL_IF_EXIT(!base);
+
+ /* No read, write and execute restrictions */
+ rights = 0;
+
+ printf("tid %d, pkey permissions are %s\n", tid, pkey_rights(rights));
+
+ /* Allocate the permissive pkey */
+ perm_pkey = sys_pkey_alloc(0, rights);
+ FAIL_IF_EXIT(perm_pkey < 0);
+
+ /*
+ * Repeatedly try to protect the common region with a permissive
+ * pkey
+ */
+ for (i = 0; i < NUM_ITERATIONS; i++) {
+ /*
+ * Wait until the other thread has finished allocating the
+ * restrictive pkey or until the next iteration has begun
+ */
+ pthread_barrier_wait(&iteration_barrier);
+
+ /* Try to associate the permissive pkey with the region */
+ FAIL_IF_EXIT(sys_pkey_mprotect(base, size, PROT_RWX,
+ perm_pkey));
+ }
+
+ /* Free the permissive pkey */
+ sys_pkey_free(perm_pkey);
+
+ return NULL;
+}
+
+static void *protect_access(void *p)
+{
+ size_t size, numinsns;
+ unsigned int *base;
+ int tid, i;
+
+ tid = gettid();
+ base = ((struct region *) p)->base;
+ size = ((struct region *) p)->size;
+ rights = ((struct region *) p)->rights;
+ numinsns = size / sizeof(base[0]);
+ FAIL_IF_EXIT(!base);
+
+ /* Allocate the restrictive pkey */
+ rest_pkey = sys_pkey_alloc(0, rights);
+ FAIL_IF_EXIT(rest_pkey < 0);
+
+ printf("tid %d, pkey permissions are %s\n", tid, pkey_rights(rights));
+ printf("tid %d, %s randomly in range [%p, %p]\n", tid,
+ (rights == PKEY_DISABLE_EXECUTE) ? "execute" :
+ (rights == PKEY_DISABLE_WRITE) ? "write" : "read",
+ base, base + numinsns);
+
+ /*
+ * Repeatedly try to protect the common region with a restrictive
+ * pkey and read from it
+ */
+ for (i = 0; i < NUM_ITERATIONS; i++) {
+ /*
+ * Wait until the other thread has finished allocating the
+ * permissive pkey or until the next iteration has begun
+ */
+ pthread_barrier_wait(&iteration_barrier);
+
+ /* Try to associate the restrictive pkey with the region */
+ FAIL_IF_EXIT(sys_pkey_mprotect(base, size, PROT_RWX,
+ rest_pkey));
+
+ /* Choose a random instruction word address from the region */
+ fault_addr = base + (rand() % numinsns);
+ fault_count = 0;
+
+ switch (rights) {
+ /* Read protection test */
+ case PKEY_DISABLE_ACCESS:
+ /*
+ * Read an instruction word from the region and
+ * verify if it has not been overwritten to
+ * something unexpected
+ */
+ FAIL_IF_EXIT(*fault_addr != PPC_INST_NOP &&
+ *fault_addr != PPC_INST_BLR);
+ break;
+
+ /* Write protection test */
+ case PKEY_DISABLE_WRITE:
+ /*
+ * Write an instruction word to the region and
+ * verify if the overwrite has succeeded
+ */
+ *fault_addr = PPC_INST_BLR;
+ FAIL_IF_EXIT(*fault_addr != PPC_INST_BLR);
+ break;
+
+ /* Execute protection test */
+ case PKEY_DISABLE_EXECUTE:
+ /* Jump to the region and execute instructions */
+ asm volatile(
+ "mtctr %0; bctrl"
+ : : "r"(fault_addr) : "ctr", "lr");
+ break;
+ }
+
+ /*
+ * Restore the restrictions originally imposed by the
+ * restrictive pkey as the signal handler would have
+ * cleared out the corresponding AMR bits
+ */
+ pkey_set_rights(rest_pkey, rights);
+ }
+
+ /* Free restrictive pkey */
+ sys_pkey_free(rest_pkey);
+
+ return NULL;
+}
+
+static void reset_pkeys(unsigned long rights)
+{
+ int pkeys[NR_PKEYS], i;
+
+ /* Exhaustively allocate all available pkeys */
+ for (i = 0; i < NR_PKEYS; i++)
+ pkeys[i] = sys_pkey_alloc(0, rights);
+
+ /* Free all allocated pkeys */
+ for (i = 0; i < NR_PKEYS; i++)
+ sys_pkey_free(pkeys[i]);
+}
+
+static int test(void)
+{
+ pthread_t prot_thread, pacc_thread;
+ struct sigaction act;
+ pthread_attr_t attr;
+ size_t numinsns;
+ struct region r;
+ int ret, i;
+
+ srand(time(NULL));
+ ret = pkeys_unsupported();
+ if (ret)
+ return ret;
+
+ /* Allocate the region */
+ r.size = getpagesize();
+ r.base = mmap(NULL, r.size, PROT_RWX,
+ MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+ FAIL_IF(r.base == MAP_FAILED);
+
+ /*
+ * Fill the region with no-ops with a branch at the end
+ * for returning to the caller
+ */
+ numinsns = r.size / sizeof(r.base[0]);
+ for (i = 0; i < numinsns - 1; i++)
+ r.base[i] = PPC_INST_NOP;
+ r.base[i] = PPC_INST_BLR;
+
+ /* Setup SIGSEGV handler */
+ act.sa_handler = 0;
+ act.sa_sigaction = segv_handler;
+ FAIL_IF(sigprocmask(SIG_SETMASK, 0, &act.sa_mask) != 0);
+ act.sa_flags = SA_SIGINFO;
+ act.sa_restorer = 0;
+ FAIL_IF(sigaction(SIGSEGV, &act, NULL) != 0);
+
+ /*
+ * For these tests, the parent process should clear all bits of
+ * AMR and IAMR, i.e. impose no restrictions, for all available
+ * pkeys. This will be the base for the initial AMR and IAMR
+ * values for all the test thread pairs.
+ *
+ * If the AMR and IAMR bits of all available pkeys are cleared
+ * before running the tests and a fault is generated when
+ * attempting to read, write or execute instructions from a
+ * pkey protected region, the pkey responsible for this must be
+ * the one from the protect-and-access thread since the other
+ * one is fully permissive. Despite that, if the pkey reported
+ * by siginfo is not the restrictive pkey, then there must be a
+ * kernel bug.
+ */
+ reset_pkeys(0);
+
+ /* Setup barrier for protect and protect-and-access threads */
+ FAIL_IF(pthread_attr_init(&attr) != 0);
+ FAIL_IF(pthread_barrier_init(&iteration_barrier, NULL, 2) != 0);
+
+ /* Setup and start protect and protect-and-read threads */
+ puts("starting thread pair (protect, protect-and-read)");
+ r.rights = PKEY_DISABLE_ACCESS;
+ FAIL_IF(pthread_create(&prot_thread, &attr, &protect, &r) != 0);
+ FAIL_IF(pthread_create(&pacc_thread, &attr, &protect_access, &r) != 0);
+ FAIL_IF(pthread_join(prot_thread, NULL) != 0);
+ FAIL_IF(pthread_join(pacc_thread, NULL) != 0);
+
+ /* Setup and start protect and protect-and-write threads */
+ puts("starting thread pair (protect, protect-and-write)");
+ r.rights = PKEY_DISABLE_WRITE;
+ FAIL_IF(pthread_create(&prot_thread, &attr, &protect, &r) != 0);
+ FAIL_IF(pthread_create(&pacc_thread, &attr, &protect_access, &r) != 0);
+ FAIL_IF(pthread_join(prot_thread, NULL) != 0);
+ FAIL_IF(pthread_join(pacc_thread, NULL) != 0);
+
+ /* Setup and start protect and protect-and-execute threads */
+ puts("starting thread pair (protect, protect-and-execute)");
+ r.rights = PKEY_DISABLE_EXECUTE;
+ FAIL_IF(pthread_create(&prot_thread, &attr, &protect, &r) != 0);
+ FAIL_IF(pthread_create(&pacc_thread, &attr, &protect_access, &r) != 0);
+ FAIL_IF(pthread_join(prot_thread, NULL) != 0);
+ FAIL_IF(pthread_join(pacc_thread, NULL) != 0);
+
+ /* Cleanup */
+ FAIL_IF(pthread_attr_destroy(&attr) != 0);
+ FAIL_IF(pthread_barrier_destroy(&iteration_barrier) != 0);
+ munmap(r.base, r.size);
+
+ return 0;
+}
+
+int main(void)
+{
+ test_harness(test, "pkey_siginfo");
+}
--
2.25.1
^ permalink raw reply related
* [PATCH 4/5] selftests/powerpc: Add helper to exit on failure
From: Sandipan Das @ 2020-07-16 11:03 UTC (permalink / raw)
To: mpe; +Cc: fweimer, aneesh.kumar, linuxram, linuxppc-dev, bauerman
In-Reply-To: <cover.1594897099.git.sandipan@linux.ibm.com>
This adds a helper similar to FAIL_IF() which lets a
program exit with code 1 (to indicate failure) when
the given condition is true.
Signed-off-by: Sandipan Das <sandipan@linux.ibm.com>
---
tools/testing/selftests/powerpc/include/utils.h | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/tools/testing/selftests/powerpc/include/utils.h b/tools/testing/selftests/powerpc/include/utils.h
index 7f259f36e23bc..69d16875802da 100644
--- a/tools/testing/selftests/powerpc/include/utils.h
+++ b/tools/testing/selftests/powerpc/include/utils.h
@@ -72,6 +72,15 @@ do { \
} \
} while (0)
+#define FAIL_IF_EXIT(x) \
+do { \
+ if ((x)) { \
+ fprintf(stderr, \
+ "[FAIL] Test FAILED on line %d\n", __LINE__); \
+ _exit(1); \
+ } \
+} while (0)
+
/* The test harness uses this, yes it's gross */
#define MAGIC_SKIP_RETURN_VALUE 99
--
2.25.1
^ permalink raw reply related
* [PATCH 3/5] selftests/powerpc: Harden test for execute-disabled pkeys
From: Sandipan Das @ 2020-07-16 11:03 UTC (permalink / raw)
To: mpe; +Cc: fweimer, aneesh.kumar, linuxram, linuxppc-dev, bauerman
In-Reply-To: <cover.1594897099.git.sandipan@linux.ibm.com>
Commit 192b6a7805989 ("powerpc/book3s64/pkeys: Fix
pkey_access_permitted() for execute disable pkey") fixed a
bug that caused repetitive faults for pkeys with no execute
rights alongside some combination of read and write rights.
This removes the last two cases of the test, which check
the behaviour of pkeys with read, write but no execute
rights and all the rights, in favour of checking all the
possible combinations of read, write and execute rights
to be able to detect bugs like the one mentioned above.
Signed-off-by: Sandipan Das <sandipan@linux.ibm.com>
---
.../selftests/powerpc/mm/pkey_exec_prot.c | 84 +++++++++----------
1 file changed, 41 insertions(+), 43 deletions(-)
diff --git a/tools/testing/selftests/powerpc/mm/pkey_exec_prot.c b/tools/testing/selftests/powerpc/mm/pkey_exec_prot.c
index 18ebfe6bae1c9..9e5c7f3f498a7 100644
--- a/tools/testing/selftests/powerpc/mm/pkey_exec_prot.c
+++ b/tools/testing/selftests/powerpc/mm/pkey_exec_prot.c
@@ -237,55 +237,53 @@ static int test(void)
*fault_addr = PPC_INST_NOP;
FAIL_IF(remaining_faults != 0 || fault_code != SEGV_ACCERR);
- /*
- * Jump to the executable region when AMR bits are set i.e.
- * the pkey permits neither read nor write access.
- *
- * This should generate a pkey fault based on IAMR bits which
- * are set to not permit execution. AMR bits should not affect
- * execution.
- *
- * This also checks if the overwrite of the first instruction
- * word from a trap to a no-op succeeded.
- */
- fault_addr = insns;
- fault_type = PKEY_DISABLE_EXECUTE;
- fault_pkey = pkey;
- remaining_faults = 1;
- FAIL_IF(sys_pkey_mprotect(insns, pgsize, PROT_EXEC, pkey) != 0);
- pkey_set_rights(pkey, PKEY_DISABLE_ACCESS);
- printf("execute at %p, pkey permissions are %s\n", fault_addr,
- pkey_rights(rights));
- asm volatile("mtctr %0; bctrl" : : "r"(insns));
- FAIL_IF(remaining_faults != 0 || fault_code != SEGV_PKUERR);
-
- /*
- * Free the current pkey and allocate a new one that is
- * fully permissive.
- */
+ /* Free the current pkey */
sys_pkey_free(pkey);
+
rights = 0;
- pkey = sys_pkey_alloc(0, rights);
+ do {
+ /*
+ * Allocate pkeys with all valid combinations of read,
+ * write and execute restrictions.
+ */
+ pkey = sys_pkey_alloc(0, rights);
+ FAIL_IF(pkey < 0);
+
+ /*
+ * Jump to the executable region. AMR bits may or may not
+ * be set but they should not affect execution.
+ *
+ * This should generate pkey faults based on IAMR bits which
+ * may be set to restrict execution.
+ *
+ * The first iteration also checks if the overwrite of the
+ * first instruction word from a trap to a no-op succeeded.
+ */
+ fault_pkey = pkey;
+ fault_type = -1;
+ remaining_faults = 0;
+ if (rights & PKEY_DISABLE_EXECUTE) {
+ fault_type = PKEY_DISABLE_EXECUTE;
+ remaining_faults = 1;
+ }
- /*
- * Jump to the executable region when AMR bits are not set
- * i.e. the pkey permits read and write access.
- *
- * This should not generate any faults as the IAMR bits are
- * also not set and hence will the pkey will not restrict
- * execution.
- */
- fault_pkey = pkey;
- remaining_faults = 0;
- FAIL_IF(sys_pkey_mprotect(insns, pgsize, PROT_EXEC, pkey) != 0);
- printf("execute at %p, pkey permissions are %s\n", fault_addr,
- pkey_rights(rights));
- asm volatile("mtctr %0; bctrl" : : "r"(insns));
- FAIL_IF(remaining_faults != 0);
+ FAIL_IF(sys_pkey_mprotect(insns, pgsize, PROT_EXEC, pkey) != 0);
+ printf("execute at %p, pkey permissions are %s\n", fault_addr,
+ pkey_rights(rights));
+ asm volatile("mtctr %0; bctrl" : : "r"(insns));
+ FAIL_IF(remaining_faults != 0);
+ if (rights & PKEY_DISABLE_EXECUTE)
+ FAIL_IF(fault_code != SEGV_PKUERR);
+
+ /* Free the current pkey */
+ sys_pkey_free(pkey);
+
+ /* Find next valid combination of pkey rights */
+ rights = next_pkey_rights(rights);
+ } while (rights);
/* Cleanup */
munmap((void *) insns, pgsize);
- sys_pkey_free(pkey);
return 0;
}
--
2.25.1
^ permalink raw reply related
* [PATCH 2/5] selftests/powerpc: Add pkey helpers for rights
From: Sandipan Das @ 2020-07-16 11:03 UTC (permalink / raw)
To: mpe; +Cc: fweimer, aneesh.kumar, linuxram, linuxppc-dev, bauerman
In-Reply-To: <cover.1594897099.git.sandipan@linux.ibm.com>
This adds some new pkey-related helper to print
access rights of a pkey in the "rwx" format and
to generate different valid combinations of pkey
rights starting from a given combination.
Signed-off-by: Sandipan Das <sandipan@linux.ibm.com>
---
.../testing/selftests/powerpc/include/pkeys.h | 28 +++++++++++++++
.../selftests/powerpc/mm/pkey_exec_prot.c | 36 ++++++++++---------
2 files changed, 48 insertions(+), 16 deletions(-)
diff --git a/tools/testing/selftests/powerpc/include/pkeys.h b/tools/testing/selftests/powerpc/include/pkeys.h
index 9b53a97e664ea..6ba95039a0343 100644
--- a/tools/testing/selftests/powerpc/include/pkeys.h
+++ b/tools/testing/selftests/powerpc/include/pkeys.h
@@ -105,4 +105,32 @@ int siginfo_pkey(siginfo_t *si)
#endif
}
+#define pkey_rights(r) ({ \
+ static char buf[4] = "rwx"; \
+ unsigned int amr_bits; \
+ if ((r) & PKEY_DISABLE_EXECUTE) \
+ buf[2] = '-'; \
+ amr_bits = (r) & PKEY_BITS_MASK; \
+ if (amr_bits & PKEY_DISABLE_WRITE) \
+ buf[1] = '-'; \
+ if (amr_bits & PKEY_DISABLE_ACCESS & ~PKEY_DISABLE_WRITE) \
+ buf[0] = '-'; \
+ buf; \
+})
+
+unsigned long next_pkey_rights(unsigned long rights)
+{
+ if (rights == PKEY_DISABLE_ACCESS)
+ return PKEY_DISABLE_EXECUTE;
+ else if (rights == (PKEY_DISABLE_ACCESS | PKEY_DISABLE_EXECUTE))
+ return 0;
+
+ if ((rights & PKEY_BITS_MASK) == 0)
+ rights |= PKEY_DISABLE_WRITE;
+ else if ((rights & PKEY_BITS_MASK) == PKEY_DISABLE_WRITE)
+ rights |= PKEY_DISABLE_ACCESS;
+
+ return rights;
+}
+
#endif /* _SELFTESTS_POWERPC_PKEYS_H */
diff --git a/tools/testing/selftests/powerpc/mm/pkey_exec_prot.c b/tools/testing/selftests/powerpc/mm/pkey_exec_prot.c
index 1253ad6afba24..18ebfe6bae1c9 100644
--- a/tools/testing/selftests/powerpc/mm/pkey_exec_prot.c
+++ b/tools/testing/selftests/powerpc/mm/pkey_exec_prot.c
@@ -102,6 +102,7 @@ static void segv_handler(int signum, siginfo_t *sinfo, void *ctx)
static int test(void)
{
struct sigaction segv_act, trap_act;
+ unsigned long rights;
int pkey, ret, i;
ret = pkeys_unsupported();
@@ -150,7 +151,8 @@ static int test(void)
insns[numinsns - 1] = PPC_INST_BLR;
/* Allocate a pkey that restricts execution */
- pkey = sys_pkey_alloc(0, PKEY_DISABLE_EXECUTE);
+ rights = PKEY_DISABLE_EXECUTE;
+ pkey = sys_pkey_alloc(0, rights);
FAIL_IF(pkey < 0);
/*
@@ -175,8 +177,8 @@ static int test(void)
*/
remaining_faults = 0;
FAIL_IF(sys_pkey_mprotect(insns, pgsize, PROT_EXEC, pkey) != 0);
- printf("read from %p, pkey is execute-disabled, access-enabled\n",
- (void *) fault_addr);
+ printf("read from %p, pkey permissions are %s\n", fault_addr,
+ pkey_rights(rights));
i = *fault_addr;
FAIL_IF(remaining_faults != 0);
@@ -192,12 +194,13 @@ static int test(void)
*/
remaining_faults = 1;
FAIL_IF(sys_pkey_mprotect(insns, pgsize, PROT_EXEC, pkey) != 0);
- printf("write to %p, pkey is execute-disabled, access-enabled\n",
- (void *) fault_addr);
+ printf("write to %p, pkey permissions are %s\n", fault_addr,
+ pkey_rights(rights));
*fault_addr = PPC_INST_TRAP;
FAIL_IF(remaining_faults != 0 || fault_code != SEGV_ACCERR);
/* The following three cases will generate SEGV_PKUERR */
+ rights |= PKEY_DISABLE_ACCESS;
fault_type = PKEY_DISABLE_ACCESS;
fault_pkey = pkey;
@@ -211,9 +214,9 @@ static int test(void)
*/
remaining_faults = 1;
FAIL_IF(sys_pkey_mprotect(insns, pgsize, PROT_EXEC, pkey) != 0);
- printf("read from %p, pkey is execute-disabled, access-disabled\n",
- (void *) fault_addr);
- pkey_set_rights(pkey, PKEY_DISABLE_ACCESS);
+ pkey_set_rights(pkey, rights);
+ printf("read from %p, pkey permissions are %s\n", fault_addr,
+ pkey_rights(rights));
i = *fault_addr;
FAIL_IF(remaining_faults != 0 || fault_code != SEGV_PKUERR);
@@ -228,9 +231,9 @@ static int test(void)
*/
remaining_faults = 2;
FAIL_IF(sys_pkey_mprotect(insns, pgsize, PROT_EXEC, pkey) != 0);
- printf("write to %p, pkey is execute-disabled, access-disabled\n",
- (void *) fault_addr);
- pkey_set_rights(pkey, PKEY_DISABLE_ACCESS);
+ pkey_set_rights(pkey, rights);
+ printf("write to %p, pkey permissions are %s\n", fault_addr,
+ pkey_rights(rights));
*fault_addr = PPC_INST_NOP;
FAIL_IF(remaining_faults != 0 || fault_code != SEGV_ACCERR);
@@ -251,8 +254,8 @@ static int test(void)
remaining_faults = 1;
FAIL_IF(sys_pkey_mprotect(insns, pgsize, PROT_EXEC, pkey) != 0);
pkey_set_rights(pkey, PKEY_DISABLE_ACCESS);
- printf("execute at %p, pkey is execute-disabled, access-disabled\n",
- (void *) fault_addr);
+ printf("execute at %p, pkey permissions are %s\n", fault_addr,
+ pkey_rights(rights));
asm volatile("mtctr %0; bctrl" : : "r"(insns));
FAIL_IF(remaining_faults != 0 || fault_code != SEGV_PKUERR);
@@ -261,7 +264,8 @@ static int test(void)
* fully permissive.
*/
sys_pkey_free(pkey);
- pkey = sys_pkey_alloc(0, 0);
+ rights = 0;
+ pkey = sys_pkey_alloc(0, rights);
/*
* Jump to the executable region when AMR bits are not set
@@ -274,8 +278,8 @@ static int test(void)
fault_pkey = pkey;
remaining_faults = 0;
FAIL_IF(sys_pkey_mprotect(insns, pgsize, PROT_EXEC, pkey) != 0);
- printf("execute at %p, pkey is execute-enabled, access-enabled\n",
- (void *) fault_addr);
+ printf("execute at %p, pkey permissions are %s\n", fault_addr,
+ pkey_rights(rights));
asm volatile("mtctr %0; bctrl" : : "r"(insns));
FAIL_IF(remaining_faults != 0);
--
2.25.1
^ permalink raw reply related
* [PATCH 1/5] selftests/powerpc: Move pkey helpers to headers
From: Sandipan Das @ 2020-07-16 11:03 UTC (permalink / raw)
To: mpe; +Cc: fweimer, aneesh.kumar, linuxram, linuxppc-dev, bauerman
In-Reply-To: <cover.1594897099.git.sandipan@linux.ibm.com>
This moves all the pkey-related helpers to a new header
file and also a helper to print error messages in signal
handlers to the existing utils header file.
Signed-off-by: Sandipan Das <sandipan@linux.ibm.com>
---
.../testing/selftests/powerpc/include/pkeys.h | 108 ++++++++++++++++++
.../testing/selftests/powerpc/include/utils.h | 4 +
.../selftests/powerpc/mm/pkey_exec_prot.c | 100 +---------------
3 files changed, 114 insertions(+), 98 deletions(-)
create mode 100644 tools/testing/selftests/powerpc/include/pkeys.h
diff --git a/tools/testing/selftests/powerpc/include/pkeys.h b/tools/testing/selftests/powerpc/include/pkeys.h
new file mode 100644
index 0000000000000..9b53a97e664ea
--- /dev/null
+++ b/tools/testing/selftests/powerpc/include/pkeys.h
@@ -0,0 +1,108 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright 2020, Sandipan Das, IBM Corp.
+ */
+
+#ifndef _SELFTESTS_POWERPC_PKEYS_H
+#define _SELFTESTS_POWERPC_PKEYS_H
+
+#include <sys/mman.h>
+
+#include "reg.h"
+#include "utils.h"
+
+/*
+ * Older versions of libc use the Intel-specific access rights.
+ * Hence, override the definitions as they might be incorrect.
+ */
+#undef PKEY_DISABLE_ACCESS
+#define PKEY_DISABLE_ACCESS 0x3
+
+#undef PKEY_DISABLE_WRITE
+#define PKEY_DISABLE_WRITE 0x2
+
+#undef PKEY_DISABLE_EXECUTE
+#define PKEY_DISABLE_EXECUTE 0x4
+
+/* Older versions of libc do not not define this */
+#ifndef SEGV_PKUERR
+#define SEGV_PKUERR 4
+#endif
+
+#define SI_PKEY_OFFSET 0x20
+
+#define SYS_pkey_mprotect 386
+#define SYS_pkey_alloc 384
+#define SYS_pkey_free 385
+
+#define PKEY_BITS_PER_PKEY 2
+#define NR_PKEYS 32
+#define PKEY_BITS_MASK ((1UL << PKEY_BITS_PER_PKEY) - 1)
+
+inline unsigned long pkeyreg_get(void)
+{
+ return mfspr(SPRN_AMR);
+}
+
+inline void pkeyreg_set(unsigned long amr)
+{
+ set_amr(amr);
+}
+
+void pkey_set_rights(int pkey, unsigned long rights)
+{
+ unsigned long amr, shift;
+
+ shift = (NR_PKEYS - pkey - 1) * PKEY_BITS_PER_PKEY;
+ amr = pkeyreg_get();
+ amr &= ~(PKEY_BITS_MASK << shift);
+ amr |= (rights & PKEY_BITS_MASK) << shift;
+ pkeyreg_set(amr);
+}
+
+int sys_pkey_mprotect(void *addr, size_t len, int prot, int pkey)
+{
+ return syscall(SYS_pkey_mprotect, addr, len, prot, pkey);
+}
+
+int sys_pkey_alloc(unsigned long flags, unsigned long rights)
+{
+ return syscall(SYS_pkey_alloc, flags, rights);
+}
+
+int sys_pkey_free(int pkey)
+{
+ return syscall(SYS_pkey_free, pkey);
+}
+
+int pkeys_unsupported(void)
+{
+ bool hash_mmu = false;
+ int pkey;
+
+ /* Protection keys are currently supported on Hash MMU only */
+ FAIL_IF(using_hash_mmu(&hash_mmu));
+ SKIP_IF(!hash_mmu);
+
+ /* Check if the system call is supported */
+ pkey = sys_pkey_alloc(0, 0);
+ SKIP_IF(pkey < 0);
+ sys_pkey_free(pkey);
+
+ return 0;
+}
+
+int siginfo_pkey(siginfo_t *si)
+{
+ /*
+ * In older versions of libc, siginfo_t does not have si_pkey as
+ * a member.
+ */
+#ifdef si_pkey
+ return si->si_pkey;
+#else
+ return *((int *)(((char *) si) + SI_PKEY_OFFSET));
+#endif
+}
+
+#endif /* _SELFTESTS_POWERPC_PKEYS_H */
diff --git a/tools/testing/selftests/powerpc/include/utils.h b/tools/testing/selftests/powerpc/include/utils.h
index 9dbe607cc5ec3..7f259f36e23bc 100644
--- a/tools/testing/selftests/powerpc/include/utils.h
+++ b/tools/testing/selftests/powerpc/include/utils.h
@@ -97,6 +97,10 @@ do { \
#define _str(s) #s
#define str(s) _str(s)
+#define sigsafe_err(msg) ({ \
+ ssize_t nbytes __attribute__((unused)); \
+ nbytes = write(STDERR_FILENO, msg, strlen(msg)); })
+
/* POWER9 feature */
#ifndef PPC_FEATURE2_ARCH_3_00
#define PPC_FEATURE2_ARCH_3_00 0x00800000
diff --git a/tools/testing/selftests/powerpc/mm/pkey_exec_prot.c b/tools/testing/selftests/powerpc/mm/pkey_exec_prot.c
index 7c7c93425c5e9..1253ad6afba24 100644
--- a/tools/testing/selftests/powerpc/mm/pkey_exec_prot.c
+++ b/tools/testing/selftests/powerpc/mm/pkey_exec_prot.c
@@ -14,83 +14,13 @@
#include <signal.h>
#include <unistd.h>
-#include <sys/mman.h>
-#include "reg.h"
-#include "utils.h"
-
-/*
- * Older versions of libc use the Intel-specific access rights.
- * Hence, override the definitions as they might be incorrect.
- */
-#undef PKEY_DISABLE_ACCESS
-#define PKEY_DISABLE_ACCESS 0x3
-
-#undef PKEY_DISABLE_WRITE
-#define PKEY_DISABLE_WRITE 0x2
-
-#undef PKEY_DISABLE_EXECUTE
-#define PKEY_DISABLE_EXECUTE 0x4
-
-/* Older versions of libc do not not define this */
-#ifndef SEGV_PKUERR
-#define SEGV_PKUERR 4
-#endif
-
-#define SI_PKEY_OFFSET 0x20
-
-#define SYS_pkey_mprotect 386
-#define SYS_pkey_alloc 384
-#define SYS_pkey_free 385
-
-#define PKEY_BITS_PER_PKEY 2
-#define NR_PKEYS 32
-#define PKEY_BITS_MASK ((1UL << PKEY_BITS_PER_PKEY) - 1)
+#include "pkeys.h"
#define PPC_INST_NOP 0x60000000
#define PPC_INST_TRAP 0x7fe00008
#define PPC_INST_BLR 0x4e800020
-#define sigsafe_err(msg) ({ \
- ssize_t nbytes __attribute__((unused)); \
- nbytes = write(STDERR_FILENO, msg, strlen(msg)); })
-
-static inline unsigned long pkeyreg_get(void)
-{
- return mfspr(SPRN_AMR);
-}
-
-static inline void pkeyreg_set(unsigned long amr)
-{
- set_amr(amr);
-}
-
-static void pkey_set_rights(int pkey, unsigned long rights)
-{
- unsigned long amr, shift;
-
- shift = (NR_PKEYS - pkey - 1) * PKEY_BITS_PER_PKEY;
- amr = pkeyreg_get();
- amr &= ~(PKEY_BITS_MASK << shift);
- amr |= (rights & PKEY_BITS_MASK) << shift;
- pkeyreg_set(amr);
-}
-
-static int sys_pkey_mprotect(void *addr, size_t len, int prot, int pkey)
-{
- return syscall(SYS_pkey_mprotect, addr, len, prot, pkey);
-}
-
-static int sys_pkey_alloc(unsigned long flags, unsigned long rights)
-{
- return syscall(SYS_pkey_alloc, flags, rights);
-}
-
-static int sys_pkey_free(int pkey)
-{
- return syscall(SYS_pkey_free, pkey);
-}
-
static volatile sig_atomic_t fault_pkey, fault_code, fault_type;
static volatile sig_atomic_t remaining_faults;
static volatile unsigned int *fault_addr;
@@ -110,16 +40,7 @@ static void segv_handler(int signum, siginfo_t *sinfo, void *ctx)
{
int signal_pkey;
- /*
- * In older versions of libc, siginfo_t does not have si_pkey as
- * a member.
- */
-#ifdef si_pkey
- signal_pkey = sinfo->si_pkey;
-#else
- signal_pkey = *((int *)(((char *) sinfo) + SI_PKEY_OFFSET));
-#endif
-
+ signal_pkey = siginfo_pkey(sinfo);
fault_code = sinfo->si_code;
/* Check if this fault originated from the expected address */
@@ -178,23 +99,6 @@ static void segv_handler(int signum, siginfo_t *sinfo, void *ctx)
remaining_faults--;
}
-static int pkeys_unsupported(void)
-{
- bool hash_mmu = false;
- int pkey;
-
- /* Protection keys are currently supported on Hash MMU only */
- FAIL_IF(using_hash_mmu(&hash_mmu));
- SKIP_IF(!hash_mmu);
-
- /* Check if the system call is supported */
- pkey = sys_pkey_alloc(0, 0);
- SKIP_IF(pkey < 0);
- sys_pkey_free(pkey);
-
- return 0;
-}
-
static int test(void)
{
struct sigaction segv_act, trap_act;
--
2.25.1
^ permalink raw reply related
* [PATCH 0/5] Improvements to pkey tests
From: Sandipan Das @ 2020-07-16 11:03 UTC (permalink / raw)
To: mpe; +Cc: fweimer, aneesh.kumar, linuxram, linuxppc-dev, bauerman
Based on recent bugs found in the pkey infrastructure, this
improves the test for execute-disabled pkeys and adds a new
test for detecting inconsistencies with the pkey reported by
the signal information upon getting a fault.
Sandipan Das (5):
selftests/powerpc: Move pkey helpers to headers
selftests/powerpc: Add pkey helpers for rights
selftests/powerpc: Harden test for execute-disabled pkeys
selftests/powerpc: Add helper to exit on failure
selftests/powerpc: Add test for pkey siginfo verification
.../testing/selftests/powerpc/include/pkeys.h | 136 +++++++
.../testing/selftests/powerpc/include/utils.h | 13 +
tools/testing/selftests/powerpc/mm/.gitignore | 1 +
tools/testing/selftests/powerpc/mm/Makefile | 5 +-
.../selftests/powerpc/mm/pkey_exec_prot.c | 210 +++--------
.../selftests/powerpc/mm/pkey_siginfo.c | 332 ++++++++++++++++++
6 files changed, 544 insertions(+), 153 deletions(-)
create mode 100644 tools/testing/selftests/powerpc/include/pkeys.h
create mode 100644 tools/testing/selftests/powerpc/mm/pkey_siginfo.c
--
2.25.1
^ permalink raw reply
* Re: [RFC PATCH 4/7] x86: use exit_lazy_tlb rather than membarrier_mm_sync_core_before_usermode
From: peterz @ 2020-07-16 11:00 UTC (permalink / raw)
To: Nicholas Piggin
Cc: linux-arch, Arnd Bergmann, x86, linux-kernel, Andy Lutomirski,
linux-mm, Mathieu Desnoyers, Andy Lutomirski, linuxppc-dev
In-Reply-To: <1594892300.mxnq3b9a77.astroid@bobo.none>
On Thu, Jul 16, 2020 at 08:03:36PM +1000, Nicholas Piggin wrote:
> Excerpts from Peter Zijlstra's message of July 16, 2020 6:50 pm:
> > On Wed, Jul 15, 2020 at 10:18:20PM -0700, Andy Lutomirski wrote:
> >> > On Jul 15, 2020, at 9:15 PM, Nicholas Piggin <npiggin@gmail.com> wrote:
> >> But I’m wondering if all this deferred sync stuff is wrong. In the
> >> brave new world of io_uring and such, perhaps kernel access matter
> >> too. Heck, even:
> >
> > IIRC the membarrier SYNC_CORE use-case is about user-space
> > self-modifying code.
> >
> > Userspace re-uses a text address and needs to SYNC_CORE before it can be
> > sure the old text is forgotten. Nothing the kernel does matters there.
> >
> > I suppose the manpage could be more clear there.
>
> True, but memory ordering of kernel stores from kernel threads for
> regular mem barrier is the concern here.
>
> Does io_uring update completion queue from kernel thread or interrupt,
> for example? If it does, then membarrier will not order such stores
> with user memory accesses.
So we're talking about regular membarrier() then? Not the SYNC_CORE
variant per-se.
Even there, I'll argue we don't care, but perhaps Mathieu has a
different opinion. All we care about is that all other threads (or CPUs
for GLOBAL) observe an smp_mb() before it returns.
Any serialization against whatever those other threads/CPUs are running
at the instant of the syscall is external to the syscall, we make no
gauarantees about that. That is, we can fundamentally not say what
another CPU is executing concurrently. Nor should we want to.
So if you feel that your membarrier() ought to serialize against remote
execution, you need to arrange a quiecent state on the remote side
yourself.
Now, normally membarrier() is used to implement userspace RCU like
things, and there all that matters is that the remote CPUs observe the
beginngin of the new grace-period, ie counter flip, and we observe their
read-side critical sections, or smething like that, it's been a while
since I looked at all that.
It's always been the case that concurrent syscalls could change user
memory, io_uring doesn't change that, it just makes it even less well
defined when that would happen. If you want to serialize against that,
you need to arrange that externally.
^ permalink raw reply
* [PATCH -next] powerpc: Convert to DEFINE_SHOW_ATTRIBUTE
From: Qinglang Miao @ 2020-07-16 9:07 UTC (permalink / raw)
To: Greg Kroah-Hartman, Paul Mackerras, Michael Ellerman
Cc: linuxppc-dev, linux-kernel, kvm-ppc
From: Chen Huang <chenhuang5@huawei.com>
Use DEFINE_SHOW_ATTRIBUTE macro to simplify the code.
Signed-off-by: Chen Huang <chenhuang5@huawei.com>
---
arch/powerpc/kvm/book3s_xive_native.c | 12 +-----------
arch/powerpc/mm/ptdump/bats.c | 24 +++++++-----------------
arch/powerpc/mm/ptdump/hashpagetable.c | 12 +-----------
arch/powerpc/mm/ptdump/ptdump.c | 13 +------------
arch/powerpc/mm/ptdump/segment_regs.c | 12 +-----------
5 files changed, 11 insertions(+), 62 deletions(-)
diff --git a/arch/powerpc/kvm/book3s_xive_native.c b/arch/powerpc/kvm/book3s_xive_native.c
index 02e3cbbea..d0c2db0e0 100644
--- a/arch/powerpc/kvm/book3s_xive_native.c
+++ b/arch/powerpc/kvm/book3s_xive_native.c
@@ -1227,17 +1227,7 @@ static int xive_native_debug_show(struct seq_file *m, void *private)
return 0;
}
-static int xive_native_debug_open(struct inode *inode, struct file *file)
-{
- return single_open(file, xive_native_debug_show, inode->i_private);
-}
-
-static const struct file_operations xive_native_debug_fops = {
- .open = xive_native_debug_open,
- .read_iter = seq_read_iter,
- .llseek = seq_lseek,
- .release = single_release,
-};
+DEFINE_SHOW_ATTRIBUTE(xive_native_debug);
static void xive_native_debugfs_init(struct kvmppc_xive *xive)
{
diff --git a/arch/powerpc/mm/ptdump/bats.c b/arch/powerpc/mm/ptdump/bats.c
index 7afcdac48..93771af72 100644
--- a/arch/powerpc/mm/ptdump/bats.c
+++ b/arch/powerpc/mm/ptdump/bats.c
@@ -56,7 +56,7 @@ static void bat_show_601(struct seq_file *m, int idx, u32 lower, u32 upper)
#define BAT_SHOW_601(_m, _n, _l, _u) bat_show_601(_m, _n, mfspr(_l), mfspr(_u))
-static int bats_show_601(struct seq_file *m, void *v)
+static int bats_601_show(struct seq_file *m, void *v)
{
seq_puts(m, "---[ Block Address Translation ]---\n");
@@ -113,7 +113,7 @@ static void bat_show_603(struct seq_file *m, int idx, u32 lower, u32 upper, bool
#define BAT_SHOW_603(_m, _n, _l, _u, _d) bat_show_603(_m, _n, mfspr(_l), mfspr(_u), _d)
-static int bats_show_603(struct seq_file *m, void *v)
+static int bats_603_show(struct seq_file *m, void *v)
{
seq_puts(m, "---[ Instruction Block Address Translation ]---\n");
@@ -144,25 +144,15 @@ static int bats_show_603(struct seq_file *m, void *v)
return 0;
}
-static int bats_open(struct inode *inode, struct file *file)
-{
- if (IS_ENABLED(CONFIG_PPC_BOOK3S_601))
- return single_open(file, bats_show_601, NULL);
-
- return single_open(file, bats_show_603, NULL);
-}
-
-static const struct file_operations bats_fops = {
- .open = bats_open,
- .read_iter = seq_read_iter,
- .llseek = seq_lseek,
- .release = single_release,
-};
+DEFINE_SHOW_ATTRIBUTE(bats_601);
+DEFINE_SHOW_ATTRIBUTE(bats_603);
static int __init bats_init(void)
{
debugfs_create_file("block_address_translation", 0400,
- powerpc_debugfs_root, NULL, &bats_fops);
+ powerpc_debugfs_root, NULL,
+ IS_ENABLED(CONFIG_PPC_BOOK3S_601) ?
+ &bats_601_fops : &bats_603_fops);
return 0;
}
device_initcall(bats_init);
diff --git a/arch/powerpc/mm/ptdump/hashpagetable.c b/arch/powerpc/mm/ptdump/hashpagetable.c
index 457fcee7e..c7f824d29 100644
--- a/arch/powerpc/mm/ptdump/hashpagetable.c
+++ b/arch/powerpc/mm/ptdump/hashpagetable.c
@@ -526,17 +526,7 @@ static int ptdump_show(struct seq_file *m, void *v)
return 0;
}
-static int ptdump_open(struct inode *inode, struct file *file)
-{
- return single_open(file, ptdump_show, NULL);
-}
-
-static const struct file_operations ptdump_fops = {
- .open = ptdump_open,
- .read_iter = seq_read_iter,
- .llseek = seq_lseek,
- .release = single_release,
-};
+DEFINE_SHOW_ATTRIBUTE(ptdump);
static int ptdump_init(void)
{
diff --git a/arch/powerpc/mm/ptdump/ptdump.c b/arch/powerpc/mm/ptdump/ptdump.c
index db17e84b5..58b062f1b 100644
--- a/arch/powerpc/mm/ptdump/ptdump.c
+++ b/arch/powerpc/mm/ptdump/ptdump.c
@@ -398,18 +398,7 @@ static int ptdump_show(struct seq_file *m, void *v)
return 0;
}
-
-static int ptdump_open(struct inode *inode, struct file *file)
-{
- return single_open(file, ptdump_show, NULL);
-}
-
-static const struct file_operations ptdump_fops = {
- .open = ptdump_open,
- .read_iter = seq_read_iter,
- .llseek = seq_lseek,
- .release = single_release,
-};
+DEFINE_SHOW_ATTRIBUTE(ptdump);
static void build_pgtable_complete_mask(void)
{
diff --git a/arch/powerpc/mm/ptdump/segment_regs.c b/arch/powerpc/mm/ptdump/segment_regs.c
index 8b15bad5a..9e870d44c 100644
--- a/arch/powerpc/mm/ptdump/segment_regs.c
+++ b/arch/powerpc/mm/ptdump/segment_regs.c
@@ -41,17 +41,7 @@ static int sr_show(struct seq_file *m, void *v)
return 0;
}
-static int sr_open(struct inode *inode, struct file *file)
-{
- return single_open(file, sr_show, NULL);
-}
-
-static const struct file_operations sr_fops = {
- .open = sr_open,
- .read_iter = seq_read_iter,
- .llseek = seq_lseek,
- .release = single_release,
-};
+DEFINE_SHOW_ATTRIBUTE(sr);
static int __init sr_init(void)
{
--
2.17.1
^ permalink raw reply related
* Re: [RFC PATCH 4/7] x86: use exit_lazy_tlb rather than membarrier_mm_sync_core_before_usermode
From: Nicholas Piggin @ 2020-07-16 10:03 UTC (permalink / raw)
To: Andy Lutomirski, Peter Zijlstra
Cc: linux-arch, Arnd Bergmann, x86, linux-kernel, linux-mm,
Mathieu Desnoyers, Andy Lutomirski, linuxppc-dev
In-Reply-To: <20200716085032.GO10769@hirez.programming.kicks-ass.net>
Excerpts from Peter Zijlstra's message of July 16, 2020 6:50 pm:
> On Wed, Jul 15, 2020 at 10:18:20PM -0700, Andy Lutomirski wrote:
>> > On Jul 15, 2020, at 9:15 PM, Nicholas Piggin <npiggin@gmail.com> wrote:
>
>> > CPU0 CPU1
>> > 1. user stuff
>> > a. membarrier() 2. enter kernel
>> > b. read rq->curr 3. rq->curr switched to kthread
>> > c. is kthread, skip IPI 4. switch_to kthread
>> > d. return to user 5. rq->curr switched to user thread
>> > 6. switch_to user thread
>> > 7. exit kernel
>> > 8. more user stuff
>
>> I find it hard to believe that this is x86 only. Why would thread
>> switch imply core sync on any architecture? Is x86 unique in having a
>> stupid expensive core sync that is heavier than smp_mb()?
>
> smp_mb() is nowhere near the most expensive barrier we have in Linux,
> mb() might qualify, since that has some completion requirements since it
> needs to serialize against external actors.
>
> On x86_64 things are rather murky, we have:
>
> LOCK prefix -- which implies smp_mb() before and after RmW
> LFENCE -- which used to be rmb like, until Spectre, and now it
> is ISYNC like. Since ISYNC ensures an empty pipeline,
> it also implies all loads are retired (and therefore
> complete) it implies rmb.
> MFENCE -- which is a memop completion barrier like, it makes
> sure all previously issued memops are complete.
>
> if you read that carefully, you'll note you'll have to use LFENCE +
> MFENCE to order against non-memops instructions.
>
> But none of them imply dumping the instruction decoder caches, that only
> happens on core serializing instructions like CR3 writes, IRET, CPUID
> and a few others, I think we recently got a SERIALIZE instruction to add
> to this list.
>
>
> On ARM64 there's something a whole different set of barriers, and again
> smp_mb() isn't nowhere near the top of the list. They have roughly 3
> classes:
>
> ISB -- instruction sync barrier
> DMB(x) -- memory ordering in domain x
> DSB(x) -- memory completion in domain x
>
> And they have at least 3 domains (IIRC), system, outer, inner.
>
> The ARM64 __switch_to() includes a dsb(sy), just like PowerPC used to
> have a SYNC, but since PowerPC is rare for only having one rediculously
> heavy serializing instruction, we got to re-use the smp_mb() early in
> __schedule() instead, but ARM64 can't do that.
>
>
> So rather than say that x86 is special here, I'd say that PowerPC is
> special here.
PowerPC is "special", I'll agree with you there :)
It does have a SYNC (HWSYNC) instruction that is mb(). It does not
serialize the core.
ISYNC is a nop. ICBI ; ISYNC does serialize the core.
Difference between them is probably much the same as difference between
MFENCE and CPUID on x86 CPUs. Serializing the core is almost always
pretty expensive. HWSYNC/MFENCE can be expensive if you have a lot of
or difficult (not exclusive in cache) outstanding with critical reads
after the barrier, but it can also be somewhat cheap if there are few
writes, and executed past, it only needs to hold up subsequent reads.
That said... implementation details. powerpc CPUs have traditionally
had fairly costly HWSYNC.
>> But I’m wondering if all this deferred sync stuff is wrong. In the
>> brave new world of io_uring and such, perhaps kernel access matter
>> too. Heck, even:
>
> IIRC the membarrier SYNC_CORE use-case is about user-space
> self-modifying code.
>
> Userspace re-uses a text address and needs to SYNC_CORE before it can be
> sure the old text is forgotten. Nothing the kernel does matters there.
>
> I suppose the manpage could be more clear there.
True, but memory ordering of kernel stores from kernel threads for
regular mem barrier is the concern here.
Does io_uring update completion queue from kernel thread or interrupt,
for example? If it does, then membarrier will not order such stores
with user memory accesses.
Thanks,
Nick
^ permalink raw reply
* Re: [PATCH] pseries: Fix 64 bit logical memory block panic
From: Paul Mackerras @ 2020-07-16 1:30 UTC (permalink / raw)
To: Aneesh Kumar K.V; +Cc: nathanl, linuxppc-dev
In-Reply-To: <87d04x3q6m.fsf@linux.ibm.com>
On Wed, Jul 15, 2020 at 06:12:25PM +0530, Aneesh Kumar K.V wrote:
> Anton Blanchard <anton@ozlabs.org> writes:
>
> > Booting with a 4GB LMB size causes us to panic:
> >
> > qemu-system-ppc64: OS terminated: OS panic:
> > Memory block size not suitable: 0x0
> >
> > Fix pseries_memory_block_size() to handle 64 bit LMBs.
> >
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Anton Blanchard <anton@ozlabs.org>
> > ---
> > arch/powerpc/platforms/pseries/hotplug-memory.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c b/arch/powerpc/platforms/pseries/hotplug-memory.c
> > index 5ace2f9a277e..6574ac33e887 100644
> > --- a/arch/powerpc/platforms/pseries/hotplug-memory.c
> > +++ b/arch/powerpc/platforms/pseries/hotplug-memory.c
> > @@ -27,7 +27,7 @@ static bool rtas_hp_event;
> > unsigned long pseries_memory_block_size(void)
> > {
> > struct device_node *np;
> > - unsigned int memblock_size = MIN_MEMORY_BLOCK_SIZE;
> > + uint64_t memblock_size = MIN_MEMORY_BLOCK_SIZE;
> > struct resource r;
> >
> > np = of_find_node_by_path("/ibm,dynamic-reconfiguration-memory");
>
> We need similar changes at more places?
>
> modified arch/powerpc/include/asm/book3s/64/mmu.h
> @@ -85,7 +85,7 @@ extern unsigned int mmu_base_pid;
> /*
> * memory block size used with radix translation.
> */
> -extern unsigned int __ro_after_init radix_mem_block_size;
> +extern unsigned long __ro_after_init radix_mem_block_size;
>
> #define PRTB_SIZE_SHIFT (mmu_pid_bits + 4)
> #define PRTB_ENTRIES (1ul << mmu_pid_bits)
> modified arch/powerpc/include/asm/drmem.h
> @@ -21,7 +21,7 @@ struct drmem_lmb {
> struct drmem_lmb_info {
> struct drmem_lmb *lmbs;
> int n_lmbs;
> - u32 lmb_size;
> + u64 lmb_size;
> };
>
> extern struct drmem_lmb_info *drmem_info;
> modified arch/powerpc/mm/book3s64/radix_pgtable.c
> @@ -34,7 +34,7 @@
>
> unsigned int mmu_pid_bits;
> unsigned int mmu_base_pid;
> -unsigned int radix_mem_block_size __ro_after_init;
> +unsigned long radix_mem_block_size __ro_after_init;
These changes look fine.
> static __ref void *early_alloc_pgtable(unsigned long size, int nid,
> unsigned long region_start, unsigned long region_end)
> modified arch/powerpc/mm/drmem.c
> @@ -268,14 +268,15 @@ static void __init __walk_drmem_v2_lmbs(const __be32 *prop, const __be32 *usm,
> void __init walk_drmem_lmbs_early(unsigned long node,
> void (*func)(struct drmem_lmb *, const __be32 **))
> {
> + const __be64 *lmb_prop;
> const __be32 *prop, *usm;
> int len;
>
> - prop = of_get_flat_dt_prop(node, "ibm,lmb-size", &len);
> - if (!prop || len < dt_root_size_cells * sizeof(__be32))
> + lmb_prop = of_get_flat_dt_prop(node, "ibm,lmb-size", &len);
> + if (!lmb_prop || len < sizeof(__be64))
> return;
>
> - drmem_info->lmb_size = dt_mem_next_cell(dt_root_size_cells, &prop);
> + drmem_info->lmb_size = be64_to_cpup(lmb_prop);
This particular change shouldn't be necessary. We already have
dt_mem_next_cell() returning u64, and it knows how to combine two
cells to give a u64 (for dt_root_size_cells == 2).
> usm = of_get_flat_dt_prop(node, "linux,drconf-usable-memory", &len);
>
> @@ -296,19 +297,19 @@ void __init walk_drmem_lmbs_early(unsigned long node,
>
> static int __init init_drmem_lmb_size(struct device_node *dn)
> {
> - const __be32 *prop;
> + const __be64 *prop;
> int len;
>
> if (drmem_info->lmb_size)
> return 0;
>
> prop = of_get_property(dn, "ibm,lmb-size", &len);
> - if (!prop || len < dt_root_size_cells * sizeof(__be32)) {
> + if (!prop || len < sizeof(__be64)) {
> pr_info("Could not determine LMB size\n");
> return -1;
> }
>
> - drmem_info->lmb_size = dt_mem_next_cell(dt_root_size_cells, &prop);
> + drmem_info->lmb_size = be64_to_cpup(prop);
Same comment here.
Paul.
^ permalink raw reply
* Re: [RFC PATCH 4/7] x86: use exit_lazy_tlb rather than membarrier_mm_sync_core_before_usermode
From: Peter Zijlstra @ 2020-07-16 8:50 UTC (permalink / raw)
To: Andy Lutomirski
Cc: linux-arch, Arnd Bergmann, x86, linux-kernel, Nicholas Piggin,
linux-mm, Mathieu Desnoyers, Andy Lutomirski, linuxppc-dev
In-Reply-To: <EFAD6E2F-EC08-4EB3-9ECC-2A963C023FC5@amacapital.net>
On Wed, Jul 15, 2020 at 10:18:20PM -0700, Andy Lutomirski wrote:
> > On Jul 15, 2020, at 9:15 PM, Nicholas Piggin <npiggin@gmail.com> wrote:
> > CPU0 CPU1
> > 1. user stuff
> > a. membarrier() 2. enter kernel
> > b. read rq->curr 3. rq->curr switched to kthread
> > c. is kthread, skip IPI 4. switch_to kthread
> > d. return to user 5. rq->curr switched to user thread
> > 6. switch_to user thread
> > 7. exit kernel
> > 8. more user stuff
> I find it hard to believe that this is x86 only. Why would thread
> switch imply core sync on any architecture? Is x86 unique in having a
> stupid expensive core sync that is heavier than smp_mb()?
smp_mb() is nowhere near the most expensive barrier we have in Linux,
mb() might qualify, since that has some completion requirements since it
needs to serialize against external actors.
On x86_64 things are rather murky, we have:
LOCK prefix -- which implies smp_mb() before and after RmW
LFENCE -- which used to be rmb like, until Spectre, and now it
is ISYNC like. Since ISYNC ensures an empty pipeline,
it also implies all loads are retired (and therefore
complete) it implies rmb.
MFENCE -- which is a memop completion barrier like, it makes
sure all previously issued memops are complete.
if you read that carefully, you'll note you'll have to use LFENCE +
MFENCE to order against non-memops instructions.
But none of them imply dumping the instruction decoder caches, that only
happens on core serializing instructions like CR3 writes, IRET, CPUID
and a few others, I think we recently got a SERIALIZE instruction to add
to this list.
On ARM64 there's something a whole different set of barriers, and again
smp_mb() isn't nowhere near the top of the list. They have roughly 3
classes:
ISB -- instruction sync barrier
DMB(x) -- memory ordering in domain x
DSB(x) -- memory completion in domain x
And they have at least 3 domains (IIRC), system, outer, inner.
The ARM64 __switch_to() includes a dsb(sy), just like PowerPC used to
have a SYNC, but since PowerPC is rare for only having one rediculously
heavy serializing instruction, we got to re-use the smp_mb() early in
__schedule() instead, but ARM64 can't do that.
So rather than say that x86 is special here, I'd say that PowerPC is
special here.
> But I’m wondering if all this deferred sync stuff is wrong. In the
> brave new world of io_uring and such, perhaps kernel access matter
> too. Heck, even:
IIRC the membarrier SYNC_CORE use-case is about user-space
self-modifying code.
Userspace re-uses a text address and needs to SYNC_CORE before it can be
sure the old text is forgotten. Nothing the kernel does matters there.
I suppose the manpage could be more clear there.
^ permalink raw reply
* [RFC PATCH] powerpc/pseries/svm: capture instruction faulting on MMIO access, in sprg0 register
From: Ram Pai @ 2020-07-16 8:32 UTC (permalink / raw)
To: kvm-ppc, linuxppc-dev
Cc: sukadev, aik, linuxram, bharata, sathnaga, ldufour, bauerman,
david
An instruction accessing a mmio address, generates a HDSI fault. This fault is
appropriately handled by the Hypervisor. However in the case of secureVMs, the
fault is delivered to the ultravisor.
Unfortunately the Ultravisor has no correct-way to fetch the faulting
instruction. The PEF architecture does not allow Ultravisor to enable MMU
translation. Walking the two level page table to read the instruction can race
with other vcpus modifying the SVM's process scoped page table.
This problem can be correctly solved with some help from the kernel.
Capture the faulting instruction in SPRG0 register, before executing the
faulting instruction. This enables the ultravisor to easily procure the
faulting instruction and emulate it.
Signed-off-by: Ram Pai <linuxram@us.ibm.com>
---
arch/powerpc/include/asm/io.h | 85 ++++++++++++++++++++++++++++++++++++++-----
1 file changed, 75 insertions(+), 10 deletions(-)
diff --git a/arch/powerpc/include/asm/io.h b/arch/powerpc/include/asm/io.h
index 635969b..7ef663d 100644
--- a/arch/powerpc/include/asm/io.h
+++ b/arch/powerpc/include/asm/io.h
@@ -35,6 +35,7 @@
#include <asm/mmu.h>
#include <asm/ppc_asm.h>
#include <asm/pgtable.h>
+#include <asm/svm.h>
#define SIO_CONFIG_RA 0x398
#define SIO_CONFIG_RD 0x399
@@ -105,34 +106,98 @@
static inline u##size name(const volatile u##size __iomem *addr) \
{ \
u##size ret; \
- __asm__ __volatile__("sync;"#insn" %0,%y1;twi 0,%0,0;isync" \
- : "=r" (ret) : "Z" (*addr) : "memory"); \
+ if (is_secure_guest()) { \
+ __asm__ __volatile__("mfsprg0 %3;" \
+ "lnia %2;" \
+ "ld %2,12(%2);" \
+ "mtsprg0 %2;" \
+ "sync;" \
+ #insn" %0,%y1;" \
+ "twi 0,%0,0;" \
+ "isync;" \
+ "mtsprg0 %3" \
+ : "=r" (ret) \
+ : "Z" (*addr), "r" (0), "r" (0) \
+ : "memory"); \
+ } else { \
+ __asm__ __volatile__("sync;" \
+ #insn" %0,%y1;" \
+ "twi 0,%0,0;" \
+ "isync" \
+ : "=r" (ret) : "Z" (*addr) : "memory"); \
+ } \
return ret; \
}
#define DEF_MMIO_OUT_X(name, size, insn) \
static inline void name(volatile u##size __iomem *addr, u##size val) \
{ \
- __asm__ __volatile__("sync;"#insn" %1,%y0" \
- : "=Z" (*addr) : "r" (val) : "memory"); \
- mmiowb_set_pending(); \
+ if (is_secure_guest()) { \
+ __asm__ __volatile__("mfsprg0 %3;" \
+ "lnia %2;" \
+ "ld %2,12(%2);" \
+ "mtsprg0 %2;" \
+ "sync;" \
+ #insn" %1,%y0;" \
+ "mtsprg0 %3" \
+ : "=Z" (*addr) \
+ : "r" (val), "r" (0), "r" (0) \
+ : "memory"); \
+ } else { \
+ __asm__ __volatile__("sync;" \
+ #insn" %1,%y0" \
+ : "=Z" (*addr) : "r" (val) : "memory"); \
+ mmiowb_set_pending(); \
+ } \
}
#define DEF_MMIO_IN_D(name, size, insn) \
static inline u##size name(const volatile u##size __iomem *addr) \
{ \
u##size ret; \
- __asm__ __volatile__("sync;"#insn"%U1%X1 %0,%1;twi 0,%0,0;isync"\
- : "=r" (ret) : "m" (*addr) : "memory"); \
+ if (is_secure_guest()) { \
+ __asm__ __volatile__("mfsprg0 %3;" \
+ "lnia %2;" \
+ "ld %2,12(%2);" \
+ "mtsprg0 %2;" \
+ "sync;" \
+ #insn"%U1%X1 %0,%1;" \
+ "twi 0,%0,0;" \
+ "isync;" \
+ "mtsprg0 %3" \
+ : "=r" (ret) \
+ : "m" (*addr), "r" (0), "r" (0) \
+ : "memory"); \
+ } else { \
+ __asm__ __volatile__("sync;" \
+ #insn"%U1%X1 %0,%1;" \
+ "twi 0,%0,0;" \
+ "isync" \
+ : "=r" (ret) : "m" (*addr) : "memory"); \
+ } \
return ret; \
}
#define DEF_MMIO_OUT_D(name, size, insn) \
static inline void name(volatile u##size __iomem *addr, u##size val) \
{ \
- __asm__ __volatile__("sync;"#insn"%U0%X0 %1,%0" \
- : "=m" (*addr) : "r" (val) : "memory"); \
- mmiowb_set_pending(); \
+ if (is_secure_guest()) { \
+ __asm__ __volatile__("mfsprg0 %3;" \
+ "lnia %2;" \
+ "ld %2,12(%2);" \
+ "mtsprg0 %2;" \
+ "sync;" \
+ #insn"%U0%X0 %1,%0;" \
+ "mtsprg0 %3" \
+ : "=m" (*addr) \
+ : "r" (val), "r" (0), "r" (0) \
+ : "memory"); \
+ } else { \
+ __asm__ __volatile__("sync;" \
+ #insn"%U0%X0 %1,%0" \
+ : "=m" (*addr) : "r" (val) : "memory"); \
+ mmiowb_set_pending(); \
+ } \
}
DEF_MMIO_IN_D(in_8, 8, lbz);
--
1.8.3.1
^ permalink raw reply related
* RE: [RFC PATCH 00/35] Move all PCIBIOS* definitions into arch/x86
From: David Laight @ 2020-07-16 8:18 UTC (permalink / raw)
To: 'Bjorn Helgaas'
Cc: Keith Busch, Paul Mackerras, sparclinux, Toan Le, Kjetil Oftedal,
Greg Ungerer, Marek Vasut, Rob Herring, Lorenzo Pieralisi,
Sagi Grimberg, Russell King, Ley Foon Tan, Christoph Hellwig,
Geert Uytterhoeven, Kevin Hilman, linux-pci, Jakub Kicinski,
Matt Turner, linux-kernel-mentees@lists.linuxfoundation.org,
Guenter Roeck, 'Arnd Bergmann', Ray Jui, Jens Axboe,
Ivan Kokshaysky, Shuah Khan, bjorn@helgaas.com, Boris Ostrovsky,
Richard Henderson, Juergen Gross, Bjorn Helgaas,
Thomas Bogendoerfer, Scott Branden, Jingoo Han,
Saheed O. Bolarinwa, linux-kernel@vger.kernel.org, Philipp Zabel,
Greg Kroah-Hartman, Gustavo Pimentel, linuxppc-dev,
David S. Miller, Heiner Kallweit
In-Reply-To: <20200715220135.GA563272@bjorn-Precision-5520>
From: Bjorn Helgaas
> Sent: 15 July 2020 23:02
>
> On Wed, Jul 15, 2020 at 02:24:21PM +0000, David Laight wrote:
> > From: Arnd Bergmann
> > > Sent: 15 July 2020 07:47
> > > On Wed, Jul 15, 2020 at 1:46 AM Bjorn Helgaas <helgaas@kernel.org> wrote:
> > >
> > > So something like:
> > > >
> > > > void pci_read_config_word(struct pci_dev *dev, int where, u16 *val)
> > > >
> > > > and where we used to return anything non-zero, we just set *val = ~0
> > > > instead? I think we do that already in most, maybe all, cases.
> > >
> > > Right, this is what I had in mind. If we start by removing the handling
> > > of the return code in all files that clearly don't need it, looking at
> > > whatever remains will give a much better idea of what a good interface
> > > should be.
> >
> > It would be best to get rid of that nasty 'u16 *' parameter.
>
> Do you mean nasty because it's basically a return value, but not
> returned as the *function's* return value? I agree that if we were
> starting from scratch it would nicer to have:
>
> u16 pci_read_config_word(struct pci_dev *dev, int where)
>
> but I don't think it's worth changing the thousands of callers just
> for that.
It'll shrink the kernel text size somewhat.
It could also be 'fixed' with a static inline.
Actually you don't even want the result to be u16.
Even though the domain of the value is 0..65535 keeping
the type as int (or unsigned int) will save the compiler
having to generate lots of masking instructions.
Code performance here will be overwhelmed by the time taken
for the config space access.
But more generally all local variables should really be
the size of cpu registers.
On x86-64 you need to use 'unsigned int' for anything used
as array subscripts to avoid the 'sign extend' instructions.
In some code paths it may matter...
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
^ permalink raw reply
* Re: [PATCH v3] powerpc/pseries: detect secure and trusted boot state of the system.
From: Michal Suchánek @ 2020-07-16 8:13 UTC (permalink / raw)
To: Nayna Jain; +Cc: linuxppc-dev, linux-kernel, Mimi Zohar, Daniel Axtens
In-Reply-To: <1594813921-12425-1-git-send-email-nayna@linux.ibm.com>
On Wed, Jul 15, 2020 at 07:52:01AM -0400, Nayna Jain wrote:
> The device-tree property to check secure and trusted boot state is
> different for guests(pseries) compared to baremetal(powernv).
>
> This patch updates the existing is_ppc_secureboot_enabled() and
> is_ppc_trustedboot_enabled() functions to add support for pseries.
>
> The secureboot and trustedboot state are exposed via device-tree property:
> /proc/device-tree/ibm,secure-boot and /proc/device-tree/ibm,trusted-boot
>
> The values of ibm,secure-boot under pseries are interpreted as:
^^^
>
> 0 - Disabled
> 1 - Enabled in Log-only mode. This patch interprets this value as
> disabled, since audit mode is currently not supported for Linux.
> 2 - Enabled and enforced.
> 3-9 - Enabled and enforcing; requirements are at the discretion of the
> operating system.
>
> The values of ibm,trusted-boot under pseries are interpreted as:
^^^
These two should be different I suppose?
Thanks
Michal
> 0 - Disabled
> 1 - Enabled
>
> Signed-off-by: Nayna Jain <nayna@linux.ibm.com>
> Reviewed-by: Daniel Axtens <dja@axtens.net>
> ---
> v3:
> * fixed double check. Thanks Daniel for noticing it.
> * updated patch description.
>
> v2:
> * included Michael Ellerman's feedback.
> * added Daniel Axtens's Reviewed-by.
>
> arch/powerpc/kernel/secure_boot.c | 19 +++++++++++++++++--
> 1 file changed, 17 insertions(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/kernel/secure_boot.c b/arch/powerpc/kernel/secure_boot.c
> index 4b982324d368..118bcb5f79c4 100644
> --- a/arch/powerpc/kernel/secure_boot.c
> +++ b/arch/powerpc/kernel/secure_boot.c
> @@ -6,6 +6,7 @@
> #include <linux/types.h>
> #include <linux/of.h>
> #include <asm/secure_boot.h>
> +#include <asm/machdep.h>
>
> static struct device_node *get_ppc_fw_sb_node(void)
> {
> @@ -23,12 +24,19 @@ bool is_ppc_secureboot_enabled(void)
> {
> struct device_node *node;
> bool enabled = false;
> + u32 secureboot;
>
> node = get_ppc_fw_sb_node();
> enabled = of_property_read_bool(node, "os-secureboot-enforcing");
> -
> of_node_put(node);
>
> + if (enabled)
> + goto out;
> +
> + if (!of_property_read_u32(of_root, "ibm,secure-boot", &secureboot))
> + enabled = (secureboot > 1);
> +
> +out:
> pr_info("Secure boot mode %s\n", enabled ? "enabled" : "disabled");
>
> return enabled;
> @@ -38,12 +46,19 @@ bool is_ppc_trustedboot_enabled(void)
> {
> struct device_node *node;
> bool enabled = false;
> + u32 trustedboot;
>
> node = get_ppc_fw_sb_node();
> enabled = of_property_read_bool(node, "trusted-enabled");
> -
> of_node_put(node);
>
> + if (enabled)
> + goto out;
> +
> + if (!of_property_read_u32(of_root, "ibm,trusted-boot", &trustedboot))
> + enabled = (trustedboot > 0);
> +
> +out:
> pr_info("Trusted boot mode %s\n", enabled ? "enabled" : "disabled");
>
> return enabled;
> --
> 2.26.2
>
^ permalink raw reply
* RE: [RFC PATCH 00/35] Move all PCIBIOS* definitions into arch/x86
From: David Laight @ 2020-07-16 8:07 UTC (permalink / raw)
To: 'Benjamin Herrenschmidt', Bjorn Helgaas
Cc: Greg Kroah-Hartman, linux-pci, bjorn@helgaas.com, Paul Mackerras,
sparclinux, Toan Le, Christoph Hellwig, Marek Vasut, Rob Herring,
Lorenzo Pieralisi, Sagi Grimberg, Kevin Hilman, Russell King,
Ley Foon Tan, Greg Ungerer, Geert Uytterhoeven, Jakub Kicinski,
Matt Turner, linux-kernel-mentees@lists.linuxfoundation.org,
Guenter Roeck, Arnd Bergmann, Ray Jui, linuxppc-dev, Jens Axboe,
Ivan Kokshaysky, Shuah Khan, Keith Busch, Boris Ostrovsky,
Richard Henderson, Juergen Gross, Thomas Bogendoerfer,
Scott Branden, Jingoo Han, linux-kernel@vger.kernel.org,
Philipp Zabel, Saheed O. Bolarinwa,
'Oliver O'Halloran', Gustavo Pimentel, Bjorn Helgaas,
David S. Miller, Heiner Kallweit
In-Reply-To: <5d4b3a716f85017c17c52a85915fba9e19509e81.camel@kernel.crashing.org>
From: Benjamin Herrenschmidt
> Sent: 15 July 2020 23:49
> On Wed, 2020-07-15 at 17:12 -0500, Bjorn Helgaas wrote:
> > > I've 'played' with PCIe error handling - without much success.
> > > What might be useful is for a driver that has just read ~0u to
> > > be able to ask 'has there been an error signalled for this device?'.
> >
> > In many cases a driver will know that ~0 is not a valid value for the
> > register it's reading. But if ~0 *could* be valid, an interface like
> > you suggest could be useful. I don't think we have anything like that
> > today, but maybe we could. It would certainly be nice if the PCI core
> > noticed, logged, and cleared errors. We have some of that for AER,
> > but that's an optional feature, and support for the error bits in the
> > garden-variety PCI_STATUS register is pretty haphazard. As you note
> > below, this sort of SERR/PERR reporting is frequently hard-wired in
> > ways that takes it out of our purview.
>
> We do have pci_channel_state (via pci_channel_offline()) which covers
> the cases where the underlying error handling (such as EEH or unplug)
> results in the device being offlined though this tend to be
> asynchronous so it might take a few ~0's before you get it.
On one of my systems I don't think the error TLP from the target
made its way past the first bridge - I could see the error in it's
status registers.
But I couldn't find any of the AER status registers in the root bridge.
So I think you'd need a software poll of the bridge registers to
find out (and clear) the error.
The NMI on the dell system (which is supposed to meet some special
NEBS? server requirements) is just stupid.
Too late to be synchronous and impossible for the OS to handle.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
^ permalink raw reply
* [powerpc:merge] BUILD SUCCESS 58a4eb09c4aebaaffa8b4517c71543a41539c096
From: kernel test robot @ 2020-07-16 7:47 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git merge
branch HEAD: 58a4eb09c4aebaaffa8b4517c71543a41539c096 Automatic merge of 'master', 'next' and 'fixes' (2020-07-15 23:12)
elapsed time: 1031m
configs tested: 94
configs skipped: 4
The following configs have been built successfully.
More configs may be tested in the coming days.
arm defconfig
arm allyesconfig
arm allmodconfig
arm allnoconfig
arm64 allyesconfig
arm64 defconfig
arm64 allmodconfig
arm64 allnoconfig
arc axs101_defconfig
c6x evmc6457_defconfig
sh kfr2r09-romimage_defconfig
powerpc gamecube_defconfig
arm lpd270_defconfig
mips malta_defconfig
riscv defconfig
c6x evmc6474_defconfig
arm clps711x_defconfig
arm corgi_defconfig
riscv allyesconfig
arm orion5x_defconfig
arm moxart_defconfig
powerpc amigaone_defconfig
m68k apollo_defconfig
sh edosk7705_defconfig
i386 allnoconfig
i386 allyesconfig
i386 defconfig
i386 debian-10.3
ia64 allmodconfig
ia64 defconfig
ia64 allnoconfig
ia64 allyesconfig
m68k allmodconfig
m68k allnoconfig
m68k sun3_defconfig
m68k defconfig
m68k allyesconfig
nios2 defconfig
nios2 allyesconfig
openrisc defconfig
c6x allyesconfig
c6x allnoconfig
openrisc allyesconfig
nds32 defconfig
nds32 allnoconfig
csky allyesconfig
csky defconfig
alpha defconfig
alpha allyesconfig
xtensa allyesconfig
h8300 allyesconfig
h8300 allmodconfig
xtensa defconfig
arc defconfig
arc allyesconfig
sh allmodconfig
sh allnoconfig
microblaze allnoconfig
mips allyesconfig
mips allnoconfig
mips allmodconfig
parisc allnoconfig
parisc defconfig
parisc allyesconfig
parisc allmodconfig
powerpc allyesconfig
powerpc rhel-kconfig
powerpc allmodconfig
powerpc allnoconfig
powerpc defconfig
i386 randconfig-a016-20200715
i386 randconfig-a011-20200715
i386 randconfig-a015-20200715
i386 randconfig-a012-20200715
i386 randconfig-a013-20200715
i386 randconfig-a014-20200715
riscv allnoconfig
riscv allmodconfig
s390 allyesconfig
s390 allnoconfig
s390 allmodconfig
s390 defconfig
sparc allyesconfig
sparc defconfig
sparc64 defconfig
sparc64 allnoconfig
sparc64 allyesconfig
sparc64 allmodconfig
x86_64 rhel-7.6-kselftests
x86_64 rhel-8.3
x86_64 kexec
x86_64 rhel
x86_64 lkp
x86_64 fedora-25
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox