* [PATCH 2/5] powerpc: Allow 4096 bytes of stack expansion for the signal frame
From: Michael Ellerman @ 2020-07-03 14:13 UTC (permalink / raw)
To: linuxppc-dev; +Cc: linux-arch, hughd, linux-kernel
In-Reply-To: <20200703141327.1732550-1-mpe@ellerman.id.au>
We have powerpc specific logic in our page fault handling to decide if
an access to an unmapped address below the stack pointer should expand
the stack VMA.
The code was originally added in 2004 "ported from 2.4". The rough
logic is that the stack is allowed to grow to 1MB with no extra
checking. Over 1MB the access must be within 2048 bytes of the stack
pointer, or be from a user instruction that updates the stack pointer.
The 2048 byte allowance below the stack pointer is there to cover the
288 "red zone" as well as the "about 1.5kB" needed by the signal
delivery code.
Unfortunately since then the signal frame has expanded, and is now
4096 bytes on 64-bit kernels with transactional memory enabled. This
means if a process has consumed more than 1MB of stack, and its stack
pointer lies less than 4096 bytes from the next page boundary, signal
delivery will fault when trying to expand the stack and the process
will see a SEGV.
The 2048 allowance was sufficient until 2008 as the signal frame was:
struct rt_sigframe {
struct ucontext uc; /* 0 1440 */
/* --- cacheline 11 boundary (1408 bytes) was 32 bytes ago --- */
long unsigned int _unused[2]; /* 1440 16 */
unsigned int tramp[6]; /* 1456 24 */
struct siginfo * pinfo; /* 1480 8 */
void * puc; /* 1488 8 */
struct siginfo info; /* 1496 128 */
/* --- cacheline 12 boundary (1536 bytes) was 88 bytes ago --- */
char abigap[288]; /* 1624 288 */
/* size: 1920, cachelines: 15, members: 7 */
/* padding: 8 */
};
Then in commit ce48b2100785 ("powerpc: Add VSX context save/restore,
ptrace and signal support") (Jul 2008) the signal frame expanded to
2176 bytes:
struct rt_sigframe {
struct ucontext uc; /* 0 1696 */ <--
/* --- cacheline 13 boundary (1664 bytes) was 32 bytes ago --- */
long unsigned int _unused[2]; /* 1696 16 */
unsigned int tramp[6]; /* 1712 24 */
struct siginfo * pinfo; /* 1736 8 */
void * puc; /* 1744 8 */
struct siginfo info; /* 1752 128 */
/* --- cacheline 14 boundary (1792 bytes) was 88 bytes ago --- */
char abigap[288]; /* 1880 288 */
/* size: 2176, cachelines: 17, members: 7 */
/* padding: 8 */
};
At this point we should have been exposed to the bug, though as far as
I know it was never reported. I no longer have a system old enough to
easily test on.
Then in 2010 commit 320b2b8de126 ("mm: keep a guard page below a
grow-down stack segment") caused our stack expansion code to never
trigger, as there was always a VMA found for a write up to PAGE_SIZE
below r1.
That meant the bug was hidden as we continued to expand the signal
frame in commit 2b0a576d15e0 ("powerpc: Add new transactional memory
state to the signal context") (Feb 2013):
struct rt_sigframe {
struct ucontext uc; /* 0 1696 */
/* --- cacheline 13 boundary (1664 bytes) was 32 bytes ago --- */
struct ucontext uc_transact; /* 1696 1696 */ <--
/* --- cacheline 26 boundary (3328 bytes) was 64 bytes ago --- */
long unsigned int _unused[2]; /* 3392 16 */
unsigned int tramp[6]; /* 3408 24 */
struct siginfo * pinfo; /* 3432 8 */
void * puc; /* 3440 8 */
struct siginfo info; /* 3448 128 */
/* --- cacheline 27 boundary (3456 bytes) was 120 bytes ago --- */
char abigap[288]; /* 3576 288 */
/* size: 3872, cachelines: 31, members: 8 */
/* padding: 8 */
/* last cacheline: 32 bytes */
};
And commit 573ebfa6601f ("powerpc: Increase stack redzone for 64-bit
userspace to 512 bytes") (Feb 2014):
struct rt_sigframe {
struct ucontext uc; /* 0 1696 */
/* --- cacheline 13 boundary (1664 bytes) was 32 bytes ago --- */
struct ucontext uc_transact; /* 1696 1696 */
/* --- cacheline 26 boundary (3328 bytes) was 64 bytes ago --- */
long unsigned int _unused[2]; /* 3392 16 */
unsigned int tramp[6]; /* 3408 24 */
struct siginfo * pinfo; /* 3432 8 */
void * puc; /* 3440 8 */
struct siginfo info; /* 3448 128 */
/* --- cacheline 27 boundary (3456 bytes) was 120 bytes ago --- */
char abigap[512]; /* 3576 512 */ <--
/* size: 4096, cachelines: 32, members: 8 */
/* padding: 8 */
};
Then finally in 2017 commit 1be7107fbe18 ("mm: larger stack guard gap,
between vmas") exposed us to the existing bug, because it changed the
stack VMA to be the correct/real size, meaning our stack expansion
code is now triggered.
Fix it by increasing the allowance to 4096 bytes.
Hard-coding 4096 is obviously unsafe against future expansions of the
signal frame in the same way as the existing code. We can't easily use
sizeof() because the signal frame structure is not in a header. We
will either fix that, or rip out all the custom stack expansion
checking logic entirely.
Reported-by: Tom Lane <tgl@sss.pgh.pa.us>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
arch/powerpc/mm/fault.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c
index 641fc5f3d7dd..ed01329dd12b 100644
--- a/arch/powerpc/mm/fault.c
+++ b/arch/powerpc/mm/fault.c
@@ -274,7 +274,7 @@ static bool bad_stack_expansion(struct pt_regs *regs, unsigned long address,
/*
* N.B. The POWER/Open ABI allows programs to access up to
* 288 bytes below the stack pointer.
- * The kernel signal delivery code writes up to about 1.5kB
+ * The kernel signal delivery code writes up to 4KB
* below the stack pointer (r1) before decrementing it.
* The exec code can write slightly over 640kB to the stack
* before setting the user r1. Thus we allow the stack to
@@ -299,7 +299,7 @@ static bool bad_stack_expansion(struct pt_regs *regs, unsigned long address,
* between the last mapped region and the stack will
* expand the stack rather than segfaulting.
*/
- if (address + 2048 >= uregs->gpr[1])
+ if (address + 4096 >= uregs->gpr[1])
return false;
if ((flags & FAULT_FLAG_WRITE) && (flags & FAULT_FLAG_USER) &&
--
2.25.1
^ permalink raw reply related
* [PATCH 3/5] selftests/powerpc: Update the stack expansion test
From: Michael Ellerman @ 2020-07-03 14:13 UTC (permalink / raw)
To: linuxppc-dev; +Cc: linux-arch, hughd, linux-kernel
In-Reply-To: <20200703141327.1732550-1-mpe@ellerman.id.au>
Update the stack expansion load/store test to take into account the
new allowance of 4096 bytes below the stack pointer.
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
.../selftests/powerpc/mm/stack_expansion_ldst.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/tools/testing/selftests/powerpc/mm/stack_expansion_ldst.c b/tools/testing/selftests/powerpc/mm/stack_expansion_ldst.c
index 0587e11437f5..95c3f3de16a1 100644
--- a/tools/testing/selftests/powerpc/mm/stack_expansion_ldst.c
+++ b/tools/testing/selftests/powerpc/mm/stack_expansion_ldst.c
@@ -186,17 +186,17 @@ static void test_one_type(enum access_type type, unsigned long page_size, unsign
// But if we go past the rlimit it should fail
assert(test_one(DEFAULT_SIZE, rlim_cur + 1, type) != 0);
- // Above 1MB powerpc only allows accesses within 2048 bytes of
+ // Above 1MB powerpc only allows accesses within 4096 bytes of
// r1 for accesses that aren't stdu
- assert(test_one(1 * _MB + page_size - 128, -2048, type) == 0);
+ assert(test_one(1 * _MB + page_size - 128, -4096, type) == 0);
#ifdef __powerpc__
- assert(test_one(1 * _MB + page_size - 128, -2049, type) != 0);
+ assert(test_one(1 * _MB + page_size - 128, -4097, type) != 0);
#else
- assert(test_one(1 * _MB + page_size - 128, -2049, type) == 0);
+ assert(test_one(1 * _MB + page_size - 128, -4097, type) == 0);
#endif
// By consuming 2MB of stack we test the stdu case
- assert(test_one(2 * _MB + page_size - 128, -2048, type) == 0);
+ assert(test_one(2 * _MB + page_size - 128, -4096, type) == 0);
}
static int test(void)
--
2.25.1
^ permalink raw reply related
* [PATCH 1/5] selftests/powerpc: Add test of stack expansion logic
From: Michael Ellerman @ 2020-07-03 14:13 UTC (permalink / raw)
To: linuxppc-dev; +Cc: linux-arch, hughd, linux-kernel
We have custom stack expansion checks that it turns out are extremely
badly tested and contain bugs, surprise. So add some tests that
exercise the code and capture the current boundary conditions.
The signal test currently fails on 64-bit kernels because the 2048
byte allowance for the signal frame is too small, we will fix that in
a subsequent patch.
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
tools/testing/selftests/powerpc/mm/.gitignore | 2 +
tools/testing/selftests/powerpc/mm/Makefile | 8 +-
.../powerpc/mm/stack_expansion_ldst.c | 233 ++++++++++++++++++
.../powerpc/mm/stack_expansion_signal.c | 114 +++++++++
tools/testing/selftests/powerpc/pmu/lib.h | 1 +
5 files changed, 357 insertions(+), 1 deletion(-)
create mode 100644 tools/testing/selftests/powerpc/mm/stack_expansion_ldst.c
create mode 100644 tools/testing/selftests/powerpc/mm/stack_expansion_signal.c
diff --git a/tools/testing/selftests/powerpc/mm/.gitignore b/tools/testing/selftests/powerpc/mm/.gitignore
index 2ca523255b1b..8bfa3b39f628 100644
--- a/tools/testing/selftests/powerpc/mm/.gitignore
+++ b/tools/testing/selftests/powerpc/mm/.gitignore
@@ -8,3 +8,5 @@ wild_bctr
large_vm_fork_separation
bad_accesses
tlbie_test
+stack_expansion_ldst
+stack_expansion_signal
diff --git a/tools/testing/selftests/powerpc/mm/Makefile b/tools/testing/selftests/powerpc/mm/Makefile
index b9103c4bb414..3937b277c288 100644
--- a/tools/testing/selftests/powerpc/mm/Makefile
+++ b/tools/testing/selftests/powerpc/mm/Makefile
@@ -3,7 +3,8 @@
$(MAKE) -C ../
TEST_GEN_PROGS := hugetlb_vs_thp_test subpage_prot prot_sao segv_errors wild_bctr \
- large_vm_fork_separation bad_accesses
+ large_vm_fork_separation bad_accesses stack_expansion_signal \
+ stack_expansion_ldst
TEST_GEN_PROGS_EXTENDED := tlbie_test
TEST_GEN_FILES := tempfile
@@ -18,6 +19,11 @@ $(OUTPUT)/wild_bctr: CFLAGS += -m64
$(OUTPUT)/large_vm_fork_separation: CFLAGS += -m64
$(OUTPUT)/bad_accesses: CFLAGS += -m64
+$(OUTPUT)/stack_expansion_signal: ../utils.c ../pmu/lib.c
+
+$(OUTPUT)/stack_expansion_ldst: CFLAGS += -fno-stack-protector
+$(OUTPUT)/stack_expansion_ldst: ../utils.c
+
$(OUTPUT)/tempfile:
dd if=/dev/zero of=$@ bs=64k count=1
diff --git a/tools/testing/selftests/powerpc/mm/stack_expansion_ldst.c b/tools/testing/selftests/powerpc/mm/stack_expansion_ldst.c
new file mode 100644
index 000000000000..0587e11437f5
--- /dev/null
+++ b/tools/testing/selftests/powerpc/mm/stack_expansion_ldst.c
@@ -0,0 +1,233 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Test that loads/stores expand the stack segment, or trigger a SEGV, in
+ * various conditions.
+ *
+ * Based on test code by Tom Lane.
+ */
+
+#undef NDEBUG
+#include <assert.h>
+
+#include <err.h>
+#include <errno.h>
+#include <stdio.h>
+#include <signal.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/resource.h>
+#include <sys/time.h>
+#include <sys/types.h>
+#include <sys/wait.h>
+#include <unistd.h>
+
+#define _KB (1024)
+#define _MB (1024 * 1024)
+
+volatile char *stack_top_ptr;
+volatile unsigned long stack_top_sp;
+volatile char c;
+
+enum access_type {
+ LOAD,
+ STORE,
+};
+
+/*
+ * Consume stack until the stack pointer is below @target_sp, then do an access
+ * (load or store) at offset @delta from either the base of the stack or the
+ * current stack pointer.
+ */
+__attribute__ ((noinline))
+int consume_stack(unsigned long target_sp, unsigned long stack_high, int delta, enum access_type type)
+{
+ unsigned long target;
+ char stack_cur;
+
+ if ((unsigned long)&stack_cur > target_sp)
+ return consume_stack(target_sp, stack_high, delta, type);
+ else {
+ // We don't really need this, but without it GCC might not
+ // generate a recursive call above.
+ stack_top_ptr = &stack_cur;
+
+#ifdef __powerpc__
+ asm volatile ("mr %[sp], %%r1" : [sp] "=r" (stack_top_sp));
+#else
+ asm volatile ("mov %%rsp, %[sp]" : [sp] "=r" (stack_top_sp));
+#endif
+
+ // Kludge, delta < 0 indicates relative to SP
+ if (delta < 0)
+ target = stack_top_sp + delta;
+ else
+ target = stack_high - delta + 1;
+
+ volatile char *p = (char *)target;
+
+ if (type == STORE)
+ *p = c;
+ else
+ c = *p;
+
+ // Do something to prevent the stack frame being popped prior to
+ // our access above.
+ getpid();
+ }
+
+ return 0;
+}
+
+static int search_proc_maps(char *needle, unsigned long *low, unsigned long *high)
+{
+ unsigned long start, end;
+ static char buf[4096];
+ char name[128];
+ FILE *f;
+ int rc;
+
+ f = fopen("/proc/self/maps", "r");
+ if (!f) {
+ perror("fopen");
+ return -1;
+ }
+
+ while (fgets(buf, sizeof(buf), f)) {
+ rc = sscanf(buf, "%lx-%lx %*c%*c%*c%*c %*x %*d:%*d %*d %127s\n",
+ &start, &end, name);
+ if (rc == 2)
+ continue;
+
+ if (rc != 3) {
+ printf("sscanf errored\n");
+ rc = -1;
+ break;
+ }
+
+ if (strstr(name, needle)) {
+ *low = start;
+ *high = end - 1;
+ rc = 0;
+ break;
+ }
+ }
+
+ fclose(f);
+
+ return rc;
+}
+
+int child(unsigned int stack_used, int delta, enum access_type type)
+{
+ unsigned long low, stack_high;
+
+ assert(search_proc_maps("[stack]", &low, &stack_high) == 0);
+
+ assert(consume_stack(stack_high - stack_used, stack_high, delta, type) == 0);
+
+ printf("Access OK: %s delta %-7d used size 0x%06x stack high 0x%lx top_ptr %p top sp 0x%lx actual used 0x%lx\n",
+ type == LOAD ? "load" : "store", delta, stack_used, stack_high,
+ stack_top_ptr, stack_top_sp, stack_high - stack_top_sp + 1);
+
+ return 0;
+}
+
+static int test_one(unsigned int stack_used, int delta, enum access_type type)
+{
+ pid_t pid;
+ int rc;
+
+ pid = fork();
+ if (pid == 0)
+ exit(child(stack_used, delta, type));
+
+ assert(waitpid(pid, &rc, 0) != -1);
+
+ if (WIFEXITED(rc) && WEXITSTATUS(rc) == 0)
+ return 0;
+
+ // We don't expect a non-zero exit that's not a signal
+ assert(!WIFEXITED(rc));
+
+ printf("Faulted: %s delta %-7d used size 0x%06x signal %d\n",
+ type == LOAD ? "load" : "store", delta, stack_used,
+ WTERMSIG(rc));
+
+ return 1;
+}
+
+// This is fairly arbitrary but is well below any of the targets below,
+// so that the delta between the stack pointer and the target is large.
+#define DEFAULT_SIZE (32 * _KB)
+
+static void test_one_type(enum access_type type, unsigned long page_size, unsigned long rlim_cur)
+{
+ assert(test_one(DEFAULT_SIZE, 512 * _KB, type) == 0);
+
+ // powerpc has a special case to allow up to 1MB
+ assert(test_one(DEFAULT_SIZE, 1 * _MB, type) == 0);
+
+#ifdef __powerpc__
+ // This fails on powerpc because it's > 1MB and is not a stdu &
+ // not close to r1
+ assert(test_one(DEFAULT_SIZE, 1 * _MB + 8, type) != 0);
+#else
+ assert(test_one(DEFAULT_SIZE, 1 * _MB + 8, type) == 0);
+#endif
+
+#ifdef __powerpc__
+ // Accessing way past the stack pointer is not allowed on powerpc
+ assert(test_one(DEFAULT_SIZE, rlim_cur, type) != 0);
+#else
+ // We should be able to access anywhere within the rlimit
+ assert(test_one(DEFAULT_SIZE, rlim_cur, type) == 0);
+#endif
+
+ // But if we go past the rlimit it should fail
+ assert(test_one(DEFAULT_SIZE, rlim_cur + 1, type) != 0);
+
+ // Above 1MB powerpc only allows accesses within 2048 bytes of
+ // r1 for accesses that aren't stdu
+ assert(test_one(1 * _MB + page_size - 128, -2048, type) == 0);
+#ifdef __powerpc__
+ assert(test_one(1 * _MB + page_size - 128, -2049, type) != 0);
+#else
+ assert(test_one(1 * _MB + page_size - 128, -2049, type) == 0);
+#endif
+
+ // By consuming 2MB of stack we test the stdu case
+ assert(test_one(2 * _MB + page_size - 128, -2048, type) == 0);
+}
+
+static int test(void)
+{
+ unsigned long page_size;
+ struct rlimit rlimit;
+
+ page_size = getpagesize();
+ getrlimit(RLIMIT_STACK, &rlimit);
+ printf("Stack rlimit is 0x%lx\n", rlimit.rlim_cur);
+
+ printf("Testing loads ...\n");
+ test_one_type(LOAD, page_size, rlimit.rlim_cur);
+ printf("Testing stores ...\n");
+ test_one_type(STORE, page_size, rlimit.rlim_cur);
+
+ printf("All OK\n");
+
+ return 0;
+}
+
+#ifdef __powerpc__
+#include "utils.h"
+
+int main(void)
+{
+ return test_harness(test, "stack_expansion_ldst");
+}
+#else
+int main(void)
+{
+ return test();
+}
+#endif
diff --git a/tools/testing/selftests/powerpc/mm/stack_expansion_signal.c b/tools/testing/selftests/powerpc/mm/stack_expansion_signal.c
new file mode 100644
index 000000000000..0632deb00679
--- /dev/null
+++ b/tools/testing/selftests/powerpc/mm/stack_expansion_signal.c
@@ -0,0 +1,114 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Test that signal delivery is able to expand the stack segment without
+ * triggering a SEGV.
+ *
+ * Based on test code by Tom Lane.
+ */
+
+#include <err.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <signal.h>
+#include <sys/types.h>
+#include <unistd.h>
+
+#include "../pmu/lib.h"
+#include "utils.h"
+
+#define _KB (1024)
+#define _MB (1024 * 1024)
+
+static char *stack_base_ptr;
+static char *stack_top_ptr;
+
+static volatile sig_atomic_t sig_occurred = 0;
+
+static void sigusr1_handler(int signal_arg)
+{
+ sig_occurred = 1;
+}
+
+static int consume_stack(unsigned int stack_size, union pipe write_pipe)
+{
+ char stack_cur;
+
+ if ((stack_base_ptr - &stack_cur) < stack_size)
+ return consume_stack(stack_size, write_pipe);
+ else {
+ stack_top_ptr = &stack_cur;
+
+ FAIL_IF(notify_parent(write_pipe));
+
+ while (!sig_occurred)
+ barrier();
+ }
+
+ return 0;
+}
+
+static int child(unsigned int stack_size, union pipe write_pipe)
+{
+ struct sigaction act;
+ char stack_base;
+
+ act.sa_handler = sigusr1_handler;
+ sigemptyset(&act.sa_mask);
+ act.sa_flags = 0;
+ if (sigaction(SIGUSR1, &act, NULL) < 0)
+ err(1, "sigaction");
+
+ stack_base_ptr = (char *) (((size_t) &stack_base + 65535) & ~65535UL);
+
+ FAIL_IF(consume_stack(stack_size, write_pipe));
+
+ printf("size 0x%06x: OK, stack base %p top %p (%zx used)\n",
+ stack_size, stack_base_ptr, stack_top_ptr,
+ stack_base_ptr - stack_top_ptr);
+
+ return 0;
+}
+
+static int test_one_size(unsigned int stack_size)
+{
+ union pipe read_pipe, write_pipe;
+ pid_t pid;
+
+ FAIL_IF(pipe(read_pipe.fds) == -1);
+ FAIL_IF(pipe(write_pipe.fds) == -1);
+
+ pid = fork();
+ if (pid == 0) {
+ close(read_pipe.read_fd);
+ close(write_pipe.write_fd);
+ exit(child(stack_size, read_pipe));
+ }
+
+ close(read_pipe.write_fd);
+ close(write_pipe.read_fd);
+ FAIL_IF(sync_with_child(read_pipe, write_pipe));
+
+ kill(pid, SIGUSR1);
+
+ FAIL_IF(wait_for_child(pid));
+
+ close(read_pipe.read_fd);
+ close(write_pipe.write_fd);
+
+ return 0;
+}
+
+int test(void)
+{
+ unsigned int size;
+
+ for (size = (512 * _KB); size < (3 * _MB); size += (4 * _KB))
+ FAIL_IF(test_one_size(size));
+
+ return 0;
+}
+
+int main(void)
+{
+ return test_harness(test, "stack_expansion_signal");
+}
diff --git a/tools/testing/selftests/powerpc/pmu/lib.h b/tools/testing/selftests/powerpc/pmu/lib.h
index fa12e7d0b4d3..bf1bec013bbb 100644
--- a/tools/testing/selftests/powerpc/pmu/lib.h
+++ b/tools/testing/selftests/powerpc/pmu/lib.h
@@ -6,6 +6,7 @@
#ifndef __SELFTESTS_POWERPC_PMU_LIB_H
#define __SELFTESTS_POWERPC_PMU_LIB_H
+#include <stdbool.h>
#include <stdio.h>
#include <stdint.h>
#include <string.h>
--
2.25.1
^ permalink raw reply related
* Re: [v2 PATCH] crypto: af_alg - Fix regression on empty requests
From: Luis Chamberlain @ 2020-07-03 13:35 UTC (permalink / raw)
To: Herbert Xu
Cc: Sachin Sant, David Howells, David S. Miller, Naresh Kamboju,
Jarkko Sakkinen, open list, lkft-triage, James Morris,
Eric Biggers, Linux Next Mailing List, linux-security-module,
keyrings, Linux Crypto Mailing List, chrubis, linux- stable,
linuxppc-dev, Jan Stancek, LTP List, Serge E. Hallyn
In-Reply-To: <20200702033221.GA19367@gondor.apana.org.au>
On Thu, Jul 02, 2020 at 01:32:21PM +1000, Herbert Xu wrote:
> On Tue, Jun 30, 2020 at 02:18:11PM +0530, Naresh Kamboju wrote:
> >
> > Since we are on this subject,
> > LTP af_alg02 test case fails on stable 4.9 and stable 4.4
> > This is not a regression because the test case has been failing from
> > the beginning.
> >
> > Is this test case expected to fail on stable 4.9 and 4.4 ?
> > or any chance to fix this on these older branches ?
> >
> > Test output:
> > af_alg02.c:52: BROK: Timed out while reading from request socket.
> >
> > ref:
> > https://qa-reports.linaro.org/lkft/linux-stable-rc-4.9-oe/build/v4.9.228-191-g082e807235d7/testrun/2884917/suite/ltp-crypto-tests/test/af_alg02/history/
> > https://qa-reports.linaro.org/lkft/linux-stable-rc-4.9-oe/build/v4.9.228-191-g082e807235d7/testrun/2884606/suite/ltp-crypto-tests/test/af_alg02/log
>
> Actually this test really is broken.
FWIW the patch "umh: fix processed error when UMH_WAIT_PROC is used" was
dropped from linux-next for now as it was missing checking for signals.
I'll be open coding iall checks for each UMH_WAIT_PROC callers next. Its
not clear if this was the issue with this test case, but figured I'd let
you know.
Luis
^ permalink raw reply
* Re: [PATCH v5 3/3] mm/page_alloc: Keep memoryless cpuless node 0 offline
From: Srikar Dronamraju @ 2020-07-03 12:58 UTC (permalink / raw)
To: Michal Hocko
Cc: Gautham R Shenoy, Andi Kleen, David Hildenbrand, linuxppc-dev,
linux-kernel, linux-mm, Satheesh Rajendran, Mel Gorman,
Kirill A. Shutemov, Andrew Morton, Michal Such?nek,
Linus Torvalds, Christopher Lameter, Vlastimil Babka
In-Reply-To: <20200703105944.GS18446@dhcp22.suse.cz>
* Michal Hocko <mhocko@kernel.org> [2020-07-03 12:59:44]:
> > Honestly, I do not have any idea. I've traced it down to
> > Author: Andi Kleen <ak@suse.de>
> > Date: Tue Jan 11 15:35:48 2005 -0800
> >
> > [PATCH] x86_64: Fix ACPI SRAT NUMA parsing
> >
> > Fix fallout from the recent nodemask_t changes. The node ids assigned
> > in the SRAT parser were off by one.
> >
> > I added a new first_unset_node() function to nodemask.h to allocate
> > IDs sanely.
> >
> > Signed-off-by: Andi Kleen <ak@suse.de>
> > Signed-off-by: Linus Torvalds <torvalds@osdl.org>
> >
> > which doesn't really tell all that much. The historical baggage and a
> > long term behavior which is not really trivial to fix I suspect.
>
> Thinking about this some more, this logic makes some sense afterall.
> Especially in the world without memory hotplug which was very likely the
> case back then. It is much better to have compact node mask rather than
> sparse one. After all node numbers shouldn't really matter as long as
> you have a clear mapping to the HW. I am not sure we export that
> information (except for the kernel ring buffer) though.
>
> The memory hotplug changes that somehow because you can hotremove numa
> nodes and therefore make the nodemask sparse but that is not a common
> case. I am not sure what would happen if a completely new node was added
> and its corresponding node was already used by the renumbered one
> though. It would likely conflate the two I am afraid. But I am not sure
> this is really possible with x86 and a lack of a bug report would
> suggest that nobody is doing that at least.
>
JFYI,
Satheesh copied in this mailchain had opened a bug a year on crash with vcpu
hotplug on memoryless node.
https://bugzilla.kernel.org/show_bug.cgi?id=202187
--
Thanks and Regards
Srikar Dronamraju
^ permalink raw reply
* [PATCH 2/2] powerpc/powernv/idle: save-restore DAWR0,DAWRX0 for P10
From: Pratik Rajesh Sampat @ 2020-07-03 12:46 UTC (permalink / raw)
To: mpe, linuxppc-dev, linux-kernel, benh, paulus, ego, svaidy,
psampat, pratik.r.sampat
In-Reply-To: <20200703124640.42820-1-psampat@linux.ibm.com>
Additional registers DAWR0, DAWRX0 may be lost on Power 10 for
stop levels < 4.
Therefore save the values of these SPRs before entering a "stop"
state and restore their values on wakeup.
Signed-off-by: Pratik Rajesh Sampat <psampat@linux.ibm.com>
---
arch/powerpc/platforms/powernv/idle.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/arch/powerpc/platforms/powernv/idle.c b/arch/powerpc/platforms/powernv/idle.c
index 19d94d021357..471d4a65b1fa 100644
--- a/arch/powerpc/platforms/powernv/idle.c
+++ b/arch/powerpc/platforms/powernv/idle.c
@@ -600,6 +600,8 @@ struct p9_sprs {
u64 iamr;
u64 amor;
u64 uamor;
+ u64 dawr0;
+ u64 dawrx0;
};
static unsigned long power9_idle_stop(unsigned long psscr, bool mmu_on)
@@ -677,6 +679,10 @@ static unsigned long power9_idle_stop(unsigned long psscr, bool mmu_on)
sprs.tscr = mfspr(SPRN_TSCR);
if (!firmware_has_feature(FW_FEATURE_ULTRAVISOR))
sprs.ldbar = mfspr(SPRN_LDBAR);
+ if (cpu_has_feature(CPU_FTR_ARCH_31)) {
+ sprs.dawr0 = mfspr(SPRN_DAWR0);
+ sprs.dawrx0 = mfspr(SPRN_DAWRX0);
+ }
sprs_saved = true;
@@ -792,6 +798,10 @@ static unsigned long power9_idle_stop(unsigned long psscr, bool mmu_on)
mtspr(SPRN_MMCR2, sprs.mmcr2);
if (!firmware_has_feature(FW_FEATURE_ULTRAVISOR))
mtspr(SPRN_LDBAR, sprs.ldbar);
+ if (cpu_has_feature(CPU_FTR_ARCH_31)) {
+ mtspr(SPRN_DAWR0, sprs.dawr0);
+ mtspr(SPRN_DAWRX0, sprs.dawrx0);
+ }
mtspr(SPRN_SPRG3, local_paca->sprg_vdso);
--
2.25.4
^ permalink raw reply related
* [PATCH 1/2] powerpc/powernv/idle: Exclude mfspr on HID1, 4, 5 on P9 and above
From: Pratik Rajesh Sampat @ 2020-07-03 12:46 UTC (permalink / raw)
To: mpe, linuxppc-dev, linux-kernel, benh, paulus, ego, svaidy,
psampat, pratik.r.sampat
POWER9 onwards the support for the registers HID1, HID4, HID5 has been
receded.
Although mfspr on the above registers worked in Power9, In Power10
simulator is unrecognized. Moving their assignment under the
check for machines lower than Power9
Signed-off-by: Pratik Rajesh Sampat <psampat@linux.ibm.com>
---
arch/powerpc/platforms/powernv/idle.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/platforms/powernv/idle.c b/arch/powerpc/platforms/powernv/idle.c
index 2dd467383a88..19d94d021357 100644
--- a/arch/powerpc/platforms/powernv/idle.c
+++ b/arch/powerpc/platforms/powernv/idle.c
@@ -73,9 +73,6 @@ static int pnv_save_sprs_for_deep_states(void)
*/
uint64_t lpcr_val = mfspr(SPRN_LPCR);
uint64_t hid0_val = mfspr(SPRN_HID0);
- uint64_t hid1_val = mfspr(SPRN_HID1);
- uint64_t hid4_val = mfspr(SPRN_HID4);
- uint64_t hid5_val = mfspr(SPRN_HID5);
uint64_t hmeer_val = mfspr(SPRN_HMEER);
uint64_t msr_val = MSR_IDLE;
uint64_t psscr_val = pnv_deepest_stop_psscr_val;
@@ -117,6 +114,9 @@ static int pnv_save_sprs_for_deep_states(void)
/* Only p8 needs to set extra HID regiters */
if (!cpu_has_feature(CPU_FTR_ARCH_300)) {
+ uint64_t hid1_val = mfspr(SPRN_HID1);
+ uint64_t hid4_val = mfspr(SPRN_HID4);
+ uint64_t hid5_val = mfspr(SPRN_HID5);
rc = opal_slw_set_reg(pir, SPRN_HID1, hid1_val);
if (rc != 0)
--
2.25.4
^ permalink raw reply related
* Re: [PATCH v5 3/3] mm/page_alloc: Keep memoryless cpuless node 0 offline
From: Michal Hocko @ 2020-07-03 11:46 UTC (permalink / raw)
To: David Hildenbrand
Cc: Gautham R Shenoy, Andi Kleen, Srikar Dronamraju, linuxppc-dev,
linux-kernel, linux-mm, Satheesh Rajendran, Mel Gorman,
Kirill A. Shutemov, Andrew Morton, Michal Suchánek,
Linus Torvalds, Christopher Lameter, Vlastimil Babka
In-Reply-To: <3f926058-cabc-94d0-0f92-4e966ea4cdc3@redhat.com>
On Fri 03-07-20 13:32:21, David Hildenbrand wrote:
> On 03.07.20 12:59, Michal Hocko wrote:
> > On Fri 03-07-20 11:24:17, Michal Hocko wrote:
> >> [Cc Andi]
> >>
> >> On Fri 03-07-20 11:10:01, Michal Suchanek wrote:
> >>> On Wed, Jul 01, 2020 at 02:21:10PM +0200, Michal Hocko wrote:
> >>>> On Wed 01-07-20 13:30:57, David Hildenbrand wrote:
> >> [...]
> >>>>> Yep, looks like it.
> >>>>>
> >>>>> [ 0.009726] SRAT: PXM 1 -> APIC 0x00 -> Node 0
> >>>>> [ 0.009727] SRAT: PXM 1 -> APIC 0x01 -> Node 0
> >>>>> [ 0.009727] SRAT: PXM 1 -> APIC 0x02 -> Node 0
> >>>>> [ 0.009728] SRAT: PXM 1 -> APIC 0x03 -> Node 0
> >>>>> [ 0.009731] ACPI: SRAT: Node 0 PXM 1 [mem 0x00000000-0x0009ffff]
> >>>>> [ 0.009732] ACPI: SRAT: Node 0 PXM 1 [mem 0x00100000-0xbfffffff]
> >>>>> [ 0.009733] ACPI: SRAT: Node 0 PXM 1 [mem 0x100000000-0x13fffffff]
> >>>>
> >>>> This begs a question whether ppc can do the same thing?
> >>> Or x86 stop doing it so that you can see on what node you are running?
> >>>
> >>> What's the point of this indirection other than another way of avoiding
> >>> empty node 0?
> >>
> >> Honestly, I do not have any idea. I've traced it down to
> >> Author: Andi Kleen <ak@suse.de>
> >> Date: Tue Jan 11 15:35:48 2005 -0800
> >>
> >> [PATCH] x86_64: Fix ACPI SRAT NUMA parsing
> >>
> >> Fix fallout from the recent nodemask_t changes. The node ids assigned
> >> in the SRAT parser were off by one.
> >>
> >> I added a new first_unset_node() function to nodemask.h to allocate
> >> IDs sanely.
> >>
> >> Signed-off-by: Andi Kleen <ak@suse.de>
> >> Signed-off-by: Linus Torvalds <torvalds@osdl.org>
> >>
> >> which doesn't really tell all that much. The historical baggage and a
> >> long term behavior which is not really trivial to fix I suspect.
> >
> > Thinking about this some more, this logic makes some sense afterall.
> > Especially in the world without memory hotplug which was very likely the
> > case back then. It is much better to have compact node mask rather than
> > sparse one. After all node numbers shouldn't really matter as long as
> > you have a clear mapping to the HW. I am not sure we export that
> > information (except for the kernel ring buffer) though.
> >
> > The memory hotplug changes that somehow because you can hotremove numa
> > nodes and therefore make the nodemask sparse but that is not a common
> > case. I am not sure what would happen if a completely new node was added
> > and its corresponding node was already used by the renumbered one
> > though. It would likely conflate the two I am afraid. But I am not sure
> > this is really possible with x86 and a lack of a bug report would
> > suggest that nobody is doing that at least.
> >
>
> I think the ACPI code takes care of properly mapping PXM to nodes.
>
> So if I start with PXM 0 empty and PXM 1 populated, I will get
> PXM 1 == node 0 as described. Once I hotplug something to PXM 0 in QEMU
>
> $ echo "object_add memory-backend-ram,id=mem0,size=1G" | sudo nc -U /var/tmp/monitor
> $ echo "device_add pc-dimm,id=dimm0,memdev=mem0,node=0" | sudo nc -U /var/tmp/monitor
>
> $ echo "info numa" | sudo nc -U /var/tmp/monitor
> QEMU 5.0.50 monitor - type 'help' for more information
> (qemu) info numa
> 2 nodes
> node 0 cpus:
> node 0 size: 1024 MB
> node 0 plugged: 1024 MB
> node 1 cpus: 0 1 2 3
> node 1 size: 4096 MB
> node 1 plugged: 0 MB
Thanks for double checking.
> I get in the guest:
>
> [ 50.174435] ------------[ cut here ]------------
> [ 50.175436] node 1 was absent from the node_possible_map
> [ 50.176844] WARNING: CPU: 0 PID: 7 at mm/memory_hotplug.c:1021 add_memory_resource+0x8c/0x290
This would mean that the ACPI code or whoever does the remaping is not
adding the new node into possible nodes.
[...]
> I remember that we added that check just recently (due to powerpc if I am not wrong).
> Not sure why that triggers here.
This was a misbehaving Qemu IIRC providing a garbage map.
--
Michal Hocko
SUSE Labs
^ permalink raw reply
* Re: [PATCH v5 3/3] mm/page_alloc: Keep memoryless cpuless node 0 offline
From: David Hildenbrand @ 2020-07-03 11:32 UTC (permalink / raw)
To: Michal Hocko, Michal Suchánek
Cc: Gautham R Shenoy, Andi Kleen, Srikar Dronamraju, linuxppc-dev,
linux-kernel, linux-mm, Satheesh Rajendran, Mel Gorman,
Kirill A. Shutemov, Andrew Morton, Linus Torvalds,
Christopher Lameter, Vlastimil Babka
In-Reply-To: <20200703105944.GS18446@dhcp22.suse.cz>
On 03.07.20 12:59, Michal Hocko wrote:
> On Fri 03-07-20 11:24:17, Michal Hocko wrote:
>> [Cc Andi]
>>
>> On Fri 03-07-20 11:10:01, Michal Suchanek wrote:
>>> On Wed, Jul 01, 2020 at 02:21:10PM +0200, Michal Hocko wrote:
>>>> On Wed 01-07-20 13:30:57, David Hildenbrand wrote:
>> [...]
>>>>> Yep, looks like it.
>>>>>
>>>>> [ 0.009726] SRAT: PXM 1 -> APIC 0x00 -> Node 0
>>>>> [ 0.009727] SRAT: PXM 1 -> APIC 0x01 -> Node 0
>>>>> [ 0.009727] SRAT: PXM 1 -> APIC 0x02 -> Node 0
>>>>> [ 0.009728] SRAT: PXM 1 -> APIC 0x03 -> Node 0
>>>>> [ 0.009731] ACPI: SRAT: Node 0 PXM 1 [mem 0x00000000-0x0009ffff]
>>>>> [ 0.009732] ACPI: SRAT: Node 0 PXM 1 [mem 0x00100000-0xbfffffff]
>>>>> [ 0.009733] ACPI: SRAT: Node 0 PXM 1 [mem 0x100000000-0x13fffffff]
>>>>
>>>> This begs a question whether ppc can do the same thing?
>>> Or x86 stop doing it so that you can see on what node you are running?
>>>
>>> What's the point of this indirection other than another way of avoiding
>>> empty node 0?
>>
>> Honestly, I do not have any idea. I've traced it down to
>> Author: Andi Kleen <ak@suse.de>
>> Date: Tue Jan 11 15:35:48 2005 -0800
>>
>> [PATCH] x86_64: Fix ACPI SRAT NUMA parsing
>>
>> Fix fallout from the recent nodemask_t changes. The node ids assigned
>> in the SRAT parser were off by one.
>>
>> I added a new first_unset_node() function to nodemask.h to allocate
>> IDs sanely.
>>
>> Signed-off-by: Andi Kleen <ak@suse.de>
>> Signed-off-by: Linus Torvalds <torvalds@osdl.org>
>>
>> which doesn't really tell all that much. The historical baggage and a
>> long term behavior which is not really trivial to fix I suspect.
>
> Thinking about this some more, this logic makes some sense afterall.
> Especially in the world without memory hotplug which was very likely the
> case back then. It is much better to have compact node mask rather than
> sparse one. After all node numbers shouldn't really matter as long as
> you have a clear mapping to the HW. I am not sure we export that
> information (except for the kernel ring buffer) though.
>
> The memory hotplug changes that somehow because you can hotremove numa
> nodes and therefore make the nodemask sparse but that is not a common
> case. I am not sure what would happen if a completely new node was added
> and its corresponding node was already used by the renumbered one
> though. It would likely conflate the two I am afraid. But I am not sure
> this is really possible with x86 and a lack of a bug report would
> suggest that nobody is doing that at least.
>
I think the ACPI code takes care of properly mapping PXM to nodes.
So if I start with PXM 0 empty and PXM 1 populated, I will get
PXM 1 == node 0 as described. Once I hotplug something to PXM 0 in QEMU
$ echo "object_add memory-backend-ram,id=mem0,size=1G" | sudo nc -U /var/tmp/monitor
$ echo "device_add pc-dimm,id=dimm0,memdev=mem0,node=0" | sudo nc -U /var/tmp/monitor
$ echo "info numa" | sudo nc -U /var/tmp/monitor
QEMU 5.0.50 monitor - type 'help' for more information
(qemu) info numa
2 nodes
node 0 cpus:
node 0 size: 1024 MB
node 0 plugged: 1024 MB
node 1 cpus: 0 1 2 3
node 1 size: 4096 MB
node 1 plugged: 0 MB
I get in the guest:
[ 50.174435] ------------[ cut here ]------------
[ 50.175436] node 1 was absent from the node_possible_map
[ 50.176844] WARNING: CPU: 0 PID: 7 at mm/memory_hotplug.c:1021 add_memory_resource+0x8c/0x290
[ 50.176844] Modules linked in:
[ 50.176845] CPU: 0 PID: 7 Comm: kworker/u8:0 Not tainted 5.8.0-rc2+ #4
[ 50.176846] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.13.0-0-gf21b5a4aeb02-prebuilt.qemu.4
[ 50.176846] Workqueue: kacpi_hotplug acpi_hotplug_work_fn
[ 50.176847] RIP: 0010:add_memory_resource+0x8c/0x290
[ 50.176849] Code: 5b 5d 41 5c 41 5d 41 5e 41 5f c3 48 63 c5 48 89 04 24 48 0f a3 05 94 6c 1c 01 72 17 89 ee 48 c78
[ 50.176849] RSP: 0018:ffffa7a1c0043d48 EFLAGS: 00010296
[ 50.176850] RAX: 000000000000002c RBX: ffff8bc633e63b80 RCX: 0000000000000000
[ 50.176851] RDX: ffff8bc63bc27060 RSI: ffff8bc63bc18d00 RDI: ffff8bc63bc18d00
[ 50.176851] RBP: 0000000000000001 R08: 00000000000001e1 R09: ffffa7a1c0043bd8
[ 50.176852] R10: 0000000000000005 R11: 0000000000000000 R12: 0000000140000000
[ 50.176852] R13: 000000017fffffff R14: 0000000040000000 R15: 0000000180000000
[ 50.176853] FS: 0000000000000000(0000) GS:ffff8bc63bc00000(0000) knlGS:0000000000000000
[ 50.176853] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 50.176855] CR2: 000055dfcbfc5ee8 CR3: 00000000aca0a000 CR4: 00000000000006f0
[ 50.176855] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[ 50.176856] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[ 50.176856] Call Trace:
[ 50.176856] __add_memory+0x33/0x70
[ 50.176857] acpi_memory_device_add+0x132/0x2f2
[ 50.176857] acpi_bus_attach+0xd2/0x200
[ 50.176858] acpi_bus_scan+0x33/0x70
[ 50.176858] acpi_device_hotplug+0x298/0x390
[ 50.176858] acpi_hotplug_work_fn+0x3d/0x50
[ 50.176859] process_one_work+0x1b4/0x370
[ 50.176859] worker_thread+0x53/0x3e0
[ 50.176860] ? process_one_work+0x370/0x370
[ 50.176860] kthread+0x119/0x140
[ 50.176860] ? __kthread_bind_mask+0x60/0x60
[ 50.176861] ret_from_fork+0x22/0x30
[ 50.176861] ---[ end trace 9a2a837c1e0164f1 ]---
[ 50.209816] acpi PNP0C80:00: add_memory failed
[ 50.210510] acpi PNP0C80:00: acpi_memory_enable_device() error
[ 50.211445] acpi PNP0C80:00: Enumeration failure
I remember that we added that check just recently (due to powerpc if I am not wrong).
Not sure why that triggers here.
But it properly maps PXM 0 to node 1.
--
Thanks,
David / dhildenb
^ permalink raw reply
* Re: [PATCH 16/26] mm/powerpc: Use general page fault accounting
From: Michael Ellerman @ 2020-07-03 11:08 UTC (permalink / raw)
To: Peter Xu, linux-mm, linux-kernel
Cc: Andrea Arcangeli, Linus Torvalds, linuxppc-dev, Peter Xu,
Paul Mackerras, Andrew Morton, Will Deacon, Gerald Schaefer
In-Reply-To: <20200626223622.199765-1-peterx@redhat.com>
Peter Xu <peterx@redhat.com> writes:
> Use the general page fault accounting by passing regs into handle_mm_fault().
>
> CC: Michael Ellerman <mpe@ellerman.id.au>
> CC: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> CC: Paul Mackerras <paulus@samba.org>
> CC: linuxppc-dev@lists.ozlabs.org
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
> arch/powerpc/mm/fault.c | 11 +++--------
> 1 file changed, 3 insertions(+), 8 deletions(-)
>
> diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c
> index 992b10c3761c..e325d13efaf5 100644
> --- a/arch/powerpc/mm/fault.c
> +++ b/arch/powerpc/mm/fault.c
> @@ -563,7 +563,7 @@ static int __do_page_fault(struct pt_regs *regs, unsigned long address,
> * make sure we exit gracefully rather than endlessly redo
> * the fault.
> */
> - fault = handle_mm_fault(vma, address, flags, NULL);
> + fault = handle_mm_fault(vma, address, flags, regs);
>
> #ifdef CONFIG_PPC_MEM_KEYS
> /*
> @@ -604,14 +604,9 @@ static int __do_page_fault(struct pt_regs *regs, unsigned long address,
> /*
> * Major/minor page fault accounting.
> */
> - if (major) {
> - current->maj_flt++;
> - perf_sw_event(PERF_COUNT_SW_PAGE_FAULTS_MAJ, 1, regs, address);
> + if (major)
> cmo_account_page_fault();
> - } else {
> - current->min_flt++;
> - perf_sw_event(PERF_COUNT_SW_PAGE_FAULTS_MIN, 1, regs, address);
> - }
> +
> return 0;
> }
> NOKPROBE_SYMBOL(__do_page_fault);
You do change the logic a bit if regs is NULL (in mm_account_fault()),
but regs can never be NULL in this path, so it looks OK to me.
Acked-by: Michael Ellerman <mpe@ellerman.id.au>
cheers
^ permalink raw reply
* Re: [PATCH v5 3/3] mm/page_alloc: Keep memoryless cpuless node 0 offline
From: Michal Hocko @ 2020-07-03 10:59 UTC (permalink / raw)
To: Michal Suchánek
Cc: Gautham R Shenoy, Andi Kleen, Srikar Dronamraju,
David Hildenbrand, linuxppc-dev, linux-kernel, linux-mm,
Satheesh Rajendran, Mel Gorman, Kirill A. Shutemov, Andrew Morton,
Linus Torvalds, Christopher Lameter, Vlastimil Babka
In-Reply-To: <20200703092414.GR18446@dhcp22.suse.cz>
On Fri 03-07-20 11:24:17, Michal Hocko wrote:
> [Cc Andi]
>
> On Fri 03-07-20 11:10:01, Michal Suchanek wrote:
> > On Wed, Jul 01, 2020 at 02:21:10PM +0200, Michal Hocko wrote:
> > > On Wed 01-07-20 13:30:57, David Hildenbrand wrote:
> [...]
> > > > Yep, looks like it.
> > > >
> > > > [ 0.009726] SRAT: PXM 1 -> APIC 0x00 -> Node 0
> > > > [ 0.009727] SRAT: PXM 1 -> APIC 0x01 -> Node 0
> > > > [ 0.009727] SRAT: PXM 1 -> APIC 0x02 -> Node 0
> > > > [ 0.009728] SRAT: PXM 1 -> APIC 0x03 -> Node 0
> > > > [ 0.009731] ACPI: SRAT: Node 0 PXM 1 [mem 0x00000000-0x0009ffff]
> > > > [ 0.009732] ACPI: SRAT: Node 0 PXM 1 [mem 0x00100000-0xbfffffff]
> > > > [ 0.009733] ACPI: SRAT: Node 0 PXM 1 [mem 0x100000000-0x13fffffff]
> > >
> > > This begs a question whether ppc can do the same thing?
> > Or x86 stop doing it so that you can see on what node you are running?
> >
> > What's the point of this indirection other than another way of avoiding
> > empty node 0?
>
> Honestly, I do not have any idea. I've traced it down to
> Author: Andi Kleen <ak@suse.de>
> Date: Tue Jan 11 15:35:48 2005 -0800
>
> [PATCH] x86_64: Fix ACPI SRAT NUMA parsing
>
> Fix fallout from the recent nodemask_t changes. The node ids assigned
> in the SRAT parser were off by one.
>
> I added a new first_unset_node() function to nodemask.h to allocate
> IDs sanely.
>
> Signed-off-by: Andi Kleen <ak@suse.de>
> Signed-off-by: Linus Torvalds <torvalds@osdl.org>
>
> which doesn't really tell all that much. The historical baggage and a
> long term behavior which is not really trivial to fix I suspect.
Thinking about this some more, this logic makes some sense afterall.
Especially in the world without memory hotplug which was very likely the
case back then. It is much better to have compact node mask rather than
sparse one. After all node numbers shouldn't really matter as long as
you have a clear mapping to the HW. I am not sure we export that
information (except for the kernel ring buffer) though.
The memory hotplug changes that somehow because you can hotremove numa
nodes and therefore make the nodemask sparse but that is not a common
case. I am not sure what would happen if a completely new node was added
and its corresponding node was already used by the renumbered one
though. It would likely conflate the two I am afraid. But I am not sure
this is really possible with x86 and a lack of a bug report would
suggest that nobody is doing that at least.
--
Michal Hocko
SUSE Labs
^ permalink raw reply
* Re: [PATCH 5/8] powerpc/64s: implement queued spinlocks and rwlocks
From: Michael Ellerman @ 2020-07-03 10:52 UTC (permalink / raw)
To: Nicholas Piggin, Will Deacon
Cc: linux-arch, Peter Zijlstra, Boqun Feng, linux-kernel, kvm-ppc,
virtualization, Ingo Molnar, Waiman Long, linuxppc-dev
In-Reply-To: <1593686722.w9psaqk7yp.astroid@bobo.none>
Nicholas Piggin <npiggin@gmail.com> writes:
> Excerpts from Will Deacon's message of July 2, 2020 8:35 pm:
>> On Thu, Jul 02, 2020 at 08:25:43PM +1000, Nicholas Piggin wrote:
>>> Excerpts from Will Deacon's message of July 2, 2020 6:02 pm:
>>> > On Thu, Jul 02, 2020 at 05:48:36PM +1000, Nicholas Piggin wrote:
>>> >> diff --git a/arch/powerpc/include/asm/qspinlock.h b/arch/powerpc/include/asm/qspinlock.h
>>> >> new file mode 100644
>>> >> index 000000000000..f84da77b6bb7
>>> >> --- /dev/null
>>> >> +++ b/arch/powerpc/include/asm/qspinlock.h
>>> >> @@ -0,0 +1,20 @@
>>> >> +/* SPDX-License-Identifier: GPL-2.0 */
>>> >> +#ifndef _ASM_POWERPC_QSPINLOCK_H
>>> >> +#define _ASM_POWERPC_QSPINLOCK_H
>>> >> +
>>> >> +#include <asm-generic/qspinlock_types.h>
>>> >> +
>>> >> +#define _Q_PENDING_LOOPS (1 << 9) /* not tuned */
>>> >> +
>>> >> +#define smp_mb__after_spinlock() smp_mb()
>>> >> +
>>> >> +static __always_inline int queued_spin_is_locked(struct qspinlock *lock)
>>> >> +{
>>> >> + smp_mb();
>>> >> + return atomic_read(&lock->val);
>>> >> +}
>>> >
>>> > Why do you need the smp_mb() here?
>>>
>>> A long and sad tale that ends here 51d7d5205d338
>>>
>>> Should probably at least refer to that commit from here, since this one
>>> is not going to git blame back there. I'll add something.
>>
>> Is this still an issue, though?
>>
>> See 38b850a73034 (where we added a similar barrier on arm64) and then
>> c6f5d02b6a0f (where we removed it).
>>
>
> Oh nice, I didn't know that went away. Thanks for the heads up.
Argh! I spent so much time chasing that damn bug in the ipc code.
> I'm going to say I'm too scared to remove it while changing the
> spinlock algorithm, but I'll open an issue and we should look at
> removing it.
Sounds good.
cheers
^ permalink raw reply
* [RFC PATCH v0 2/2] KVM: PPC: Book3S HV: Use H_RPT_INVALIDATE in nested KVM
From: Bharata B Rao @ 2020-07-03 10:44 UTC (permalink / raw)
To: kvm-ppc, linuxppc-dev; +Cc: aneesh.kumar, Bharata B Rao, npiggin
In-Reply-To: <20200703104420.21349-1-bharata@linux.ibm.com>
In the nested KVM case, replace H_TLB_INVALIDATE by the new hcall
H_RPT_INVALIDATE if available. The availability of this hcall
is determined from "hcall-rpt-invalidate" string in ibm,hypertas-functions
DT property.
Signed-off-by: Bharata B Rao <bharata@linux.ibm.com>
---
arch/powerpc/include/asm/firmware.h | 4 +++-
arch/powerpc/kvm/book3s_64_mmu_radix.c | 26 ++++++++++++++++++-----
arch/powerpc/kvm/book3s_hv_nested.c | 13 ++++++++++--
arch/powerpc/platforms/pseries/firmware.c | 1 +
4 files changed, 36 insertions(+), 8 deletions(-)
diff --git a/arch/powerpc/include/asm/firmware.h b/arch/powerpc/include/asm/firmware.h
index 6003c2e533a0..aa6a5ef5d483 100644
--- a/arch/powerpc/include/asm/firmware.h
+++ b/arch/powerpc/include/asm/firmware.h
@@ -52,6 +52,7 @@
#define FW_FEATURE_PAPR_SCM ASM_CONST(0x0000002000000000)
#define FW_FEATURE_ULTRAVISOR ASM_CONST(0x0000004000000000)
#define FW_FEATURE_STUFF_TCE ASM_CONST(0x0000008000000000)
+#define FW_FEATURE_RPT_INVALIDATE ASM_CONST(0x0000010000000000)
#ifndef __ASSEMBLY__
@@ -71,7 +72,8 @@ enum {
FW_FEATURE_TYPE1_AFFINITY | FW_FEATURE_PRRN |
FW_FEATURE_HPT_RESIZE | FW_FEATURE_DRMEM_V2 |
FW_FEATURE_DRC_INFO | FW_FEATURE_BLOCK_REMOVE |
- FW_FEATURE_PAPR_SCM | FW_FEATURE_ULTRAVISOR,
+ FW_FEATURE_PAPR_SCM | FW_FEATURE_ULTRAVISOR |
+ FW_FEATURE_RPT_INVALIDATE,
FW_FEATURE_PSERIES_ALWAYS = 0,
FW_FEATURE_POWERNV_POSSIBLE = FW_FEATURE_OPAL | FW_FEATURE_ULTRAVISOR,
FW_FEATURE_POWERNV_ALWAYS = 0,
diff --git a/arch/powerpc/kvm/book3s_64_mmu_radix.c b/arch/powerpc/kvm/book3s_64_mmu_radix.c
index e738ea652192..8411e42eedbd 100644
--- a/arch/powerpc/kvm/book3s_64_mmu_radix.c
+++ b/arch/powerpc/kvm/book3s_64_mmu_radix.c
@@ -21,6 +21,7 @@
#include <asm/pte-walk.h>
#include <asm/ultravisor.h>
#include <asm/kvm_book3s_uvmem.h>
+#include <asm/plpar_wrappers.h>
/*
* Supported radix tree geometry.
@@ -313,9 +314,17 @@ void kvmppc_radix_tlbie_page(struct kvm *kvm, unsigned long addr,
}
psi = shift_to_mmu_psize(pshift);
- rb = addr | (mmu_get_ap(psi) << PPC_BITLSHIFT(58));
- rc = plpar_hcall_norets(H_TLB_INVALIDATE, H_TLBIE_P1_ENC(0, 0, 1),
- lpid, rb);
+ if (!firmware_has_feature(FW_FEATURE_RPT_INVALIDATE)) {
+ rb = addr | (mmu_get_ap(psi) << PPC_BITLSHIFT(58));
+ rc = plpar_hcall_norets(H_TLB_INVALIDATE,
+ H_TLBIE_P1_ENC(0, 0, 1), lpid, rb);
+ } else {
+ rc = pseries_rpt_invalidate(lpid, H_RPTI_TARGET_CMMU,
+ H_RPTI_TYPE_NESTED |
+ H_RPTI_TYPE_TLB,
+ psize_to_rpti_pgsize(psi),
+ addr, addr + psize);
+ }
if (rc)
pr_err("KVM: TLB page invalidation hcall failed, rc=%ld\n", rc);
}
@@ -329,8 +338,15 @@ static void kvmppc_radix_flush_pwc(struct kvm *kvm, unsigned int lpid)
return;
}
- rc = plpar_hcall_norets(H_TLB_INVALIDATE, H_TLBIE_P1_ENC(1, 0, 1),
- lpid, TLBIEL_INVAL_SET_LPID);
+ if (!firmware_has_feature(FW_FEATURE_RPT_INVALIDATE))
+ rc = plpar_hcall_norets(H_TLB_INVALIDATE,
+ H_TLBIE_P1_ENC(1, 0, 1),
+ lpid, TLBIEL_INVAL_SET_LPID);
+ else
+ rc = pseries_rpt_invalidate(lpid, H_RPTI_TARGET_CMMU,
+ H_RPTI_TYPE_NESTED |
+ H_RPTI_TYPE_PWC, H_RPTI_PAGE_ALL,
+ 0, -1UL);
if (rc)
pr_err("KVM: TLB PWC invalidation hcall failed, rc=%ld\n", rc);
}
diff --git a/arch/powerpc/kvm/book3s_hv_nested.c b/arch/powerpc/kvm/book3s_hv_nested.c
index efb78d37f29a..4d023c451be4 100644
--- a/arch/powerpc/kvm/book3s_hv_nested.c
+++ b/arch/powerpc/kvm/book3s_hv_nested.c
@@ -19,6 +19,7 @@
#include <asm/pgalloc.h>
#include <asm/pte-walk.h>
#include <asm/reg.h>
+#include <asm/plpar_wrappers.h>
static struct patb_entry *pseries_partition_tb;
@@ -401,8 +402,16 @@ static void kvmhv_flush_lpid(unsigned int lpid)
return;
}
- rc = plpar_hcall_norets(H_TLB_INVALIDATE, H_TLBIE_P1_ENC(2, 0, 1),
- lpid, TLBIEL_INVAL_SET_LPID);
+ if (!firmware_has_feature(FW_FEATURE_RPT_INVALIDATE))
+ rc = plpar_hcall_norets(H_TLB_INVALIDATE,
+ H_TLBIE_P1_ENC(2, 0, 1),
+ lpid, TLBIEL_INVAL_SET_LPID);
+ else
+ rc = pseries_rpt_invalidate(lpid, H_RPTI_TARGET_CMMU,
+ H_RPTI_TYPE_NESTED |
+ H_RPTI_TYPE_TLB | H_RPTI_TYPE_PWC |
+ H_RPTI_TYPE_PAT,
+ H_RPTI_PAGE_ALL, 0, -1UL);
if (rc)
pr_err("KVM: TLB LPID invalidation hcall failed, rc=%ld\n", rc);
}
diff --git a/arch/powerpc/platforms/pseries/firmware.c b/arch/powerpc/platforms/pseries/firmware.c
index 3e49cc23a97a..4c7b7f5a2ebc 100644
--- a/arch/powerpc/platforms/pseries/firmware.c
+++ b/arch/powerpc/platforms/pseries/firmware.c
@@ -65,6 +65,7 @@ hypertas_fw_features_table[] = {
{FW_FEATURE_HPT_RESIZE, "hcall-hpt-resize"},
{FW_FEATURE_BLOCK_REMOVE, "hcall-block-remove"},
{FW_FEATURE_PAPR_SCM, "hcall-scm"},
+ {FW_FEATURE_RPT_INVALIDATE, "hcall-rpt-invalidate"},
};
/* Build up the firmware features bitmask using the contents of
--
2.21.3
^ permalink raw reply related
* [RFC PATCH v0 1/2] KVM: PPC: Book3S HV: Add support for H_RPT_INVALIDATE (nested case only)
From: Bharata B Rao @ 2020-07-03 10:44 UTC (permalink / raw)
To: kvm-ppc, linuxppc-dev; +Cc: aneesh.kumar, Bharata B Rao, npiggin
In-Reply-To: <20200703104420.21349-1-bharata@linux.ibm.com>
Implements H_RPT_INVALIDATE hcall and supports only nested case
currently.
A KVM capability KVM_CAP_RPT_INVALIDATE is added to indicate the
support for this hcall.
Signed-off-by: Bharata B Rao <bharata@linux.ibm.com>
---
Documentation/virt/kvm/api.rst | 17 ++++
.../include/asm/book3s/64/tlbflush-radix.h | 18 ++++
arch/powerpc/include/asm/kvm_book3s.h | 3 +
arch/powerpc/kvm/book3s_hv.c | 32 +++++++
arch/powerpc/kvm/book3s_hv_nested.c | 94 +++++++++++++++++++
arch/powerpc/kvm/powerpc.c | 3 +
arch/powerpc/mm/book3s64/radix_tlb.c | 4 -
include/uapi/linux/kvm.h | 1 +
8 files changed, 168 insertions(+), 4 deletions(-)
diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
index 426f94582b7a..d235d16a4bf0 100644
--- a/Documentation/virt/kvm/api.rst
+++ b/Documentation/virt/kvm/api.rst
@@ -5843,6 +5843,23 @@ controlled by the kvm module parameter halt_poll_ns. This capability allows
the maximum halt time to specified on a per-VM basis, effectively overriding
the module parameter for the target VM.
+7.21 KVM_CAP_RPT_INVALIDATE
+--------------------------
+
+:Capability: KVM_CAP_RPT_INVALIDATE
+:Architectures: ppc
+:Type: vm
+
+This capability indicates that the kernel is capable of handling
+H_RPT_INVALIDATE hcall.
+
+In order to enable the use of H_RPT_INVALIDATE in the guest,
+user space might have to advertise it for the guest. For example,
+IBM pSeries (sPAPR) guest starts using it if "hcall-rpt-invalidate" is
+present in the "ibm,hypertas-functions" device-tree property.
+
+This capability is always enabled.
+
8. Other capabilities.
======================
diff --git a/arch/powerpc/include/asm/book3s/64/tlbflush-radix.h b/arch/powerpc/include/asm/book3s/64/tlbflush-radix.h
index 94439e0cefc9..aace7e9b2397 100644
--- a/arch/powerpc/include/asm/book3s/64/tlbflush-radix.h
+++ b/arch/powerpc/include/asm/book3s/64/tlbflush-radix.h
@@ -4,6 +4,10 @@
#include <asm/hvcall.h>
+#define RIC_FLUSH_TLB 0
+#define RIC_FLUSH_PWC 1
+#define RIC_FLUSH_ALL 2
+
struct vm_area_struct;
struct mm_struct;
struct mmu_gather;
@@ -21,6 +25,20 @@ static inline u64 psize_to_rpti_pgsize(unsigned long psize)
return H_RPTI_PAGE_ALL;
}
+static inline int rpti_pgsize_to_psize(unsigned long page_size)
+{
+ if (page_size == H_RPTI_PAGE_4K)
+ return MMU_PAGE_4K;
+ if (page_size == H_RPTI_PAGE_64K)
+ return MMU_PAGE_64K;
+ if (page_size == H_RPTI_PAGE_2M)
+ return MMU_PAGE_2M;
+ if (page_size == H_RPTI_PAGE_1G)
+ return MMU_PAGE_1G;
+ else
+ return MMU_PAGE_64K; /* Default */
+}
+
static inline int mmu_get_ap(int psize)
{
return mmu_psize_defs[psize].ap;
diff --git a/arch/powerpc/include/asm/kvm_book3s.h b/arch/powerpc/include/asm/kvm_book3s.h
index d32ec9ae73bd..0f1c5fa6e8ce 100644
--- a/arch/powerpc/include/asm/kvm_book3s.h
+++ b/arch/powerpc/include/asm/kvm_book3s.h
@@ -298,6 +298,9 @@ void kvmhv_set_ptbl_entry(unsigned int lpid, u64 dw0, u64 dw1);
void kvmhv_release_all_nested(struct kvm *kvm);
long kvmhv_enter_nested_guest(struct kvm_vcpu *vcpu);
long kvmhv_do_nested_tlbie(struct kvm_vcpu *vcpu);
+long kvmhv_h_rpti_nested(struct kvm_vcpu *vcpu, unsigned long lpid,
+ unsigned long type, unsigned long pg_sizes,
+ unsigned long start, unsigned long end);
int kvmhv_run_single_vcpu(struct kvm_vcpu *vcpu,
u64 time_limit, unsigned long lpcr);
void kvmhv_save_hv_regs(struct kvm_vcpu *vcpu, struct hv_guest_state *hr);
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index 6bf66649ab92..2f772183f249 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -895,6 +895,28 @@ static int kvmppc_get_yield_count(struct kvm_vcpu *vcpu)
return yield_count;
}
+static long kvmppc_h_rpt_invalidate(struct kvm_vcpu *vcpu,
+ unsigned long pid, unsigned long target,
+ unsigned long type, unsigned long pg_sizes,
+ unsigned long start, unsigned long end)
+{
+ if (end < start)
+ return H_P5;
+
+ if ((!type & H_RPTI_TYPE_NESTED))
+ return H_P3;
+
+ if (!nesting_enabled(vcpu->kvm))
+ return H_FUNCTION;
+
+ /* Support only cores as target */
+ if (target != H_RPTI_TARGET_CMMU)
+ return H_P2;
+
+ return kvmhv_h_rpti_nested(vcpu, pid, (type & ~H_RPTI_TYPE_NESTED),
+ pg_sizes, start, end);
+}
+
int kvmppc_pseries_do_hcall(struct kvm_vcpu *vcpu)
{
unsigned long req = kvmppc_get_gpr(vcpu, 3);
@@ -1103,6 +1125,14 @@ int kvmppc_pseries_do_hcall(struct kvm_vcpu *vcpu)
*/
ret = kvmppc_h_svm_init_abort(vcpu->kvm);
break;
+ case H_RPT_INVALIDATE:
+ ret = kvmppc_h_rpt_invalidate(vcpu, kvmppc_get_gpr(vcpu, 4),
+ kvmppc_get_gpr(vcpu, 5),
+ kvmppc_get_gpr(vcpu, 6),
+ kvmppc_get_gpr(vcpu, 7),
+ kvmppc_get_gpr(vcpu, 8),
+ kvmppc_get_gpr(vcpu, 9));
+ break;
default:
return RESUME_HOST;
@@ -1149,6 +1179,7 @@ static int kvmppc_hcall_impl_hv(unsigned long cmd)
case H_XIRR_X:
#endif
case H_PAGE_INIT:
+ case H_RPT_INVALIDATE:
return 1;
}
@@ -5284,6 +5315,7 @@ static unsigned int default_hcall_list[] = {
H_XIRR,
H_XIRR_X,
#endif
+ H_RPT_INVALIDATE,
0
};
diff --git a/arch/powerpc/kvm/book3s_hv_nested.c b/arch/powerpc/kvm/book3s_hv_nested.c
index 2c849a65db77..efb78d37f29a 100644
--- a/arch/powerpc/kvm/book3s_hv_nested.c
+++ b/arch/powerpc/kvm/book3s_hv_nested.c
@@ -1141,6 +1141,100 @@ long kvmhv_do_nested_tlbie(struct kvm_vcpu *vcpu)
return H_SUCCESS;
}
+static long do_tlb_invalidate_nested_tlb(struct kvm_vcpu *vcpu,
+ unsigned long lpid,
+ unsigned long page_size,
+ unsigned long ap,
+ unsigned long start,
+ unsigned long end)
+{
+ unsigned long addr = start;
+ int ret;
+
+ do {
+ ret = kvmhv_emulate_tlbie_tlb_addr(vcpu, lpid, ap,
+ get_epn(addr));
+ if (ret)
+ return ret;
+ addr += page_size;
+ } while (addr < end);
+
+ return ret;
+}
+
+static long do_tlb_invalidate_nested_all(struct kvm_vcpu *vcpu,
+ unsigned long lpid)
+{
+ struct kvm *kvm = vcpu->kvm;
+ struct kvm_nested_guest *gp;
+
+ gp = kvmhv_get_nested(kvm, lpid, false);
+ if (gp) {
+ kvmhv_emulate_tlbie_lpid(vcpu, gp, RIC_FLUSH_ALL);
+ kvmhv_put_nested(gp);
+ }
+ return H_SUCCESS;
+}
+
+long kvmhv_h_rpti_nested(struct kvm_vcpu *vcpu, unsigned long lpid,
+ unsigned long type, unsigned long pg_sizes,
+ unsigned long start, unsigned long end)
+{
+ struct kvm_nested_guest *gp;
+ long ret;
+ unsigned long psize, ap;
+
+ /*
+ * If L2 lpid isn't valid, we need to return H_PARAMETER.
+ * Nested KVM issues a L2 lpid flush call when creating
+ * partition table entries for L2. This happens even before
+ * the corresponding shadow lpid is created in HV. Until
+ * this is fixed, ignore such flush requests.
+ */
+ gp = kvmhv_find_nested(vcpu->kvm, lpid);
+ if (!gp)
+ return H_SUCCESS;
+
+ if ((type & H_RPTI_TYPE_NESTED_ALL) == H_RPTI_TYPE_NESTED_ALL)
+ return do_tlb_invalidate_nested_all(vcpu, lpid);
+
+ if ((type & H_RPTI_TYPE_TLB) == H_RPTI_TYPE_TLB) {
+ if (pg_sizes & H_RPTI_PAGE_64K) {
+ psize = rpti_pgsize_to_psize(pg_sizes & H_RPTI_PAGE_64K);
+ ap = mmu_get_ap(psize);
+
+ ret = do_tlb_invalidate_nested_tlb(vcpu, lpid,
+ (1UL << 16),
+ ap, start, end);
+ if (ret)
+ return H_P4;
+ }
+
+ if (pg_sizes & H_RPTI_PAGE_2M) {
+ psize = rpti_pgsize_to_psize(pg_sizes & H_RPTI_PAGE_2M);
+ ap = mmu_get_ap(psize);
+
+ ret = do_tlb_invalidate_nested_tlb(vcpu, lpid,
+ (1UL << 21),
+ ap, start, end);
+ if (ret)
+ return H_P4;
+ }
+
+ if (pg_sizes & H_RPTI_PAGE_1G) {
+ psize = rpti_pgsize_to_psize(pg_sizes & H_RPTI_PAGE_1G);
+ ap = mmu_get_ap(psize);
+
+ ret = do_tlb_invalidate_nested_tlb(vcpu, lpid,
+ (1UL << 30),
+ ap, start, end);
+ if (ret)
+ return H_P4;
+ }
+ }
+ return H_SUCCESS;
+}
+
/* Used to convert a nested guest real address to a L1 guest real address */
static int kvmhv_translate_addr_nested(struct kvm_vcpu *vcpu,
struct kvm_nested_guest *gp,
diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
index dd7d141e33e8..fddd742dafeb 100644
--- a/arch/powerpc/kvm/powerpc.c
+++ b/arch/powerpc/kvm/powerpc.c
@@ -675,6 +675,9 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
r = hv_enabled && kvmppc_hv_ops->enable_svm &&
!kvmppc_hv_ops->enable_svm(NULL);
break;
+ case KVM_CAP_RPT_INVALIDATE:
+ r = 1;
+ break;
#endif
default:
r = 0;
diff --git a/arch/powerpc/mm/book3s64/radix_tlb.c b/arch/powerpc/mm/book3s64/radix_tlb.c
index 0d233763441f..ffbb43fd1c6f 100644
--- a/arch/powerpc/mm/book3s64/radix_tlb.c
+++ b/arch/powerpc/mm/book3s64/radix_tlb.c
@@ -18,10 +18,6 @@
#include <asm/cputhreads.h>
#include <asm/plpar_wrappers.h>
-#define RIC_FLUSH_TLB 0
-#define RIC_FLUSH_PWC 1
-#define RIC_FLUSH_ALL 2
-
/*
* tlbiel instruction for radix, set invalidation
* i.e., r=1 and is=01 or is=10 or is=11
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index 4fdf30316582..67170f9267ff 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -1031,6 +1031,7 @@ struct kvm_ppc_resize_hpt {
#define KVM_CAP_PPC_SECURE_GUEST 181
#define KVM_CAP_HALT_POLL 182
#define KVM_CAP_ASYNC_PF_INT 183
+#define KVM_CAP_RPT_INVALIDATE 184
#ifdef KVM_CAP_IRQ_ROUTING
--
2.21.3
^ permalink raw reply related
* [RFC PATCH v0 0/2] Use H_RPT_INVALIDATE for nested guest
From: Bharata B Rao @ 2020-07-03 10:44 UTC (permalink / raw)
To: kvm-ppc, linuxppc-dev; +Cc: aneesh.kumar, Bharata B Rao, npiggin
This patchset adds support for the new hcall H_RPT_INVALIDATE
(currently handles nested case only) and replaces the nested tlb flush
calls with this new hcall if the support for the same exists.
This applies on top of "[PATCH v3 0/3] Off-load TLB invalidations to host
for !GTSE" patchset that was posted at:
https://lore.kernel.org/linuxppc-dev/20200703053608.12884-1-bharata@linux.ibm.com/T/#t
H_RPT_INVALIDATE
================
Syntax:
int64 /* H_Success: Return code on successful completion */
/* H_Busy - repeat the call with the same */
/* H_Parameter, H_P2, H_P3, H_P4, H_P5 : Invalid parameters */
hcall(const uint64 H_RPT_INVALIDATE, /* Invalidate RPT translation lookaside information */
uint64 pid, /* PID/LPID to invalidate */
uint64 target, /* Invalidation target */
uint64 type, /* Type of lookaside information */
uint64 pageSizes, /* Page sizes */
uint64 start, /* Start of Effective Address (EA) range (inclusive) */
uint64 end) /* End of EA range (exclusive) */
Invalidation targets (target)
-----------------------------
Core MMU 0x01 /* All virtual processors in the partition */
Core local MMU 0x02 /* Current virtual processor */
Nest MMU 0x04 /* All nest/accelerator agents in use by the partition */
A combination of the above can be specified, except core and core local.
Type of translation to invalidate (type)
---------------------------------------
NESTED 0x0001 /* Invalidate nested guest partition-scope */
TLB 0x0002 /* Invalidate TLB */
PWC 0x0004 /* Invalidate Page Walk Cache */
PRT 0x0008 /* Invalidate Process Table Entries if NESTED is clear */
PAT 0x0008 /* Invalidate Partition Table Entries if NESTED is set */
A combination of the above can be specified.
Page size mask (pageSizes)
--------------------------
4K 0x01
64K 0x02
2M 0x04
1G 0x08
All sizes (-1UL)
A combination of the above can be specified.
All page sizes can be selected with -1.
Semantics: Invalidate radix tree lookaside information
matching the parameters given.
* Return H_P2, H_P3 or H_P4 if target, type, or pageSizes parameters are
different from the defined values.
* Return H_PARAMETER if NESTED is set and pid is not a valid nested
LPID allocated to this partition
* Return H_P5 if (start, end) doesn't form a valid range. Start and end
should be a valid Quadrant address and end > start.
* Return H_NotSupported if the partition is not in running in radix
translation mode.
* May invalidate more translation information than requested.
* If start = 0 and end = -1, set the range to cover all valid addresses.
Else start and end should be aligned to 4kB (lower 11 bits clear).
* If NESTED is clear, then invalidate process scoped lookaside information.
Else pid specifies a nested LPID, and the invalidation is performed
on nested guest partition table and nested guest partition scope real
addresses.
* If pid = 0 and NESTED is clear, then valid addresses are quadrant 3 and
quadrant 0 spaces, Else valid addresses are quadrant 0.
* Pages which are fully covered by the range are to be invalidated.
Those which are partially covered are considered outside invalidation
range, which allows a caller to optimally invalidate ranges that may
contain mixed page sizes.
* Return H_SUCCESS on success.
Bharata B Rao (2):
KVM: PPC: Book3S HV: Add support for H_RPT_INVALIDATE (nested case
only)
KVM: PPC: Book3S HV: Use H_RPT_INVALIDATE in nested KVM
Documentation/virt/kvm/api.rst | 17 +++
.../include/asm/book3s/64/tlbflush-radix.h | 18 +++
arch/powerpc/include/asm/firmware.h | 4 +-
arch/powerpc/include/asm/kvm_book3s.h | 3 +
arch/powerpc/kvm/book3s_64_mmu_radix.c | 26 ++++-
arch/powerpc/kvm/book3s_hv.c | 32 ++++++
arch/powerpc/kvm/book3s_hv_nested.c | 107 +++++++++++++++++-
arch/powerpc/kvm/powerpc.c | 3 +
arch/powerpc/mm/book3s64/radix_tlb.c | 4 -
arch/powerpc/platforms/pseries/firmware.c | 1 +
include/uapi/linux/kvm.h | 1 +
11 files changed, 204 insertions(+), 12 deletions(-)
--
2.21.3
^ permalink raw reply
* Re: [PATCH V3 (RESEND) 2/3] mm/sparsemem: Enable vmem_altmap support in vmemmap_alloc_block_buf()
From: Michael Ellerman @ 2020-07-03 10:43 UTC (permalink / raw)
To: Catalin Marinas, Anshuman Khandual
Cc: x86, Peter Zijlstra, Dave Hansen, linuxppc-dev, linux-kernel,
linux-mm, Ingo Molnar, Paul Mackerras, Andy Lutomirski,
H. Peter Anvin, Borislav Petkov, Thomas Gleixner, Will Deacon,
Andrew Morton, linux-arm-kernel
In-Reply-To: <20200702140752.GF22241@gaia>
Catalin Marinas <catalin.marinas@arm.com> writes:
> On Thu, Jun 18, 2020 at 06:45:29AM +0530, Anshuman Khandual wrote:
>> There are many instances where vmemap allocation is often switched between
>> regular memory and device memory just based on whether altmap is available
>> or not. vmemmap_alloc_block_buf() is used in various platforms to allocate
>> vmemmap mappings. Lets also enable it to handle altmap based device memory
>> allocation along with existing regular memory allocations. This will help
>> in avoiding the altmap based allocation switch in many places.
>>
>> While here also implement a regular memory allocation fallback mechanism
>> when the first preferred device memory allocation fails. This will ensure
>> preserving the existing semantics on powerpc platform. To summarize there
>> are three different methods to call vmemmap_alloc_block_buf().
>>
>> (., NULL, false) /* Allocate from system RAM */
>> (., altmap, false) /* Allocate from altmap without any fallback */
>> (., altmap, true) /* Allocate from altmap with fallback (system RAM) */
> [...]
>> diff --git a/arch/powerpc/mm/init_64.c b/arch/powerpc/mm/init_64.c
>> index bc73abf0bc25..01e25b56eccb 100644
>> --- a/arch/powerpc/mm/init_64.c
>> +++ b/arch/powerpc/mm/init_64.c
>> @@ -225,12 +225,12 @@ int __meminit vmemmap_populate(unsigned long start, unsigned long end, int node,
>> * fall back to system memory if the altmap allocation fail.
>> */
>> if (altmap && !altmap_cross_boundary(altmap, start, page_size)) {
>> - p = altmap_alloc_block_buf(page_size, altmap);
>> - if (!p)
>> - pr_debug("altmap block allocation failed, falling back to system memory");
>> + p = vmemmap_alloc_block_buf(page_size, node,
>> + altmap, true);
>> + } else {
>> + p = vmemmap_alloc_block_buf(page_size, node,
>> + NULL, false);
>> }
>> - if (!p)
>> - p = vmemmap_alloc_block_buf(page_size, node);
>> if (!p)
>> return -ENOMEM;
>
> Is the fallback argument actually necessary. It may be cleaner to just
> leave the code as is with the choice between altmap and NULL. If an arch
> needs a fallback (only powerpc), they have the fallback in place
> already. I don't see the powerpc code any better after this change.
Yeah I agree.
cheers
^ permalink raw reply
* Re: [PATCH 0/4] ASoC: fsl_asrc: allow selecting arbitrary clocks
From: Arnaud Ferraris @ 2020-07-03 9:38 UTC (permalink / raw)
To: Nicolin Chen
Cc: devicetree, alsa-devel, Timur Tabi, Xiubo Li, Liam Girdwood,
Mark Brown, linux-kernel, Takashi Iwai, Rob Herring,
Fabio Estevam, kernel, linuxppc-dev
In-Reply-To: <20200702184226.GA23935@Asurada-Nvidia>
Hi Nic,
Le 02/07/2020 à 20:42, Nicolin Chen a écrit :
> Hi Arnaud,
>
> On Thu, Jul 02, 2020 at 04:22:31PM +0200, Arnaud Ferraris wrote:
>> The current ASRC driver hardcodes the input and output clocks used for
>> sample rate conversions. In order to allow greater flexibility and to
>> cover more use cases, it would be preferable to select the clocks using
>> device-tree properties.
>
> We recent just merged a new change that auto-selecting internal
> clocks based on sample rates as the first option -- ideal ratio
> mode is the fallback mode now. Please refer to:
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?h=next-20200702&id=d0250cf4f2abfbea64ed247230f08f5ae23979f0
That looks interesting, thanks for pointing this out!
I'll rebase and see how it works for my use-case, will keep you informed.
Regards,
Arnaud
^ permalink raw reply
* Re: [PATCH v4 24/41] powerpc/book3s64/pkeys: Store/restore userspace AMR correctly on entry and exit from kernel
From: Aneesh Kumar K.V @ 2020-07-03 9:30 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev, mpe; +Cc: linuxram, bauerman
In-Reply-To: <1593766495.dqos6wxr3o.astroid@bobo.none>
On 7/3/20 2:48 PM, Nicholas Piggin wrote:
> Excerpts from Aneesh Kumar K.V's message of June 15, 2020 4:14 pm:
>> This prepare kernel to operate with a different value than userspace AMR.
>> For this, AMR needs to be saved and restored on entry and return from the
>> kernel.
>>
>> With KUAP we modify kernel AMR when accessing user address from the kernel
>> via copy_to/from_user interfaces.
>>
>> If MMU_FTR_KEY is enabled we always use the key mechanism to implement KUAP
>> feature. If MMU_FTR_KEY is not supported and if we support MMU_FTR_KUAP
>> (radix translation on POWER9), we can skip restoring AMR on return
>> to userspace. Userspace won't be using AMR in that specific config.
>>
>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
>> ---
>> arch/powerpc/include/asm/book3s/64/kup.h | 141 ++++++++++++++++++-----
>> arch/powerpc/kernel/entry_64.S | 6 +-
>> arch/powerpc/kernel/exceptions-64s.S | 4 +-
>> arch/powerpc/kernel/syscall_64.c | 26 ++++-
>> 4 files changed, 144 insertions(+), 33 deletions(-)
>>
>> diff --git a/arch/powerpc/include/asm/book3s/64/kup.h b/arch/powerpc/include/asm/book3s/64/kup.h
>> index e6ee1c34842f..6979cd1a0003 100644
>> --- a/arch/powerpc/include/asm/book3s/64/kup.h
>> +++ b/arch/powerpc/include/asm/book3s/64/kup.h
>> @@ -13,18 +13,47 @@
>>
>> #ifdef __ASSEMBLY__
>>
>> -.macro kuap_restore_amr gpr1, gpr2
>> -#ifdef CONFIG_PPC_KUAP
>> +.macro kuap_restore_user_amr gpr1
>> +#if defined(CONFIG_PPC_MEM_KEYS)
>> BEGIN_MMU_FTR_SECTION_NESTED(67)
>> - mfspr \gpr1, SPRN_AMR
>> + /*
>> + * AMR is going to be different when
>> + * returning to userspace.
>> + */
>> + ld \gpr1, STACK_REGS_KUAP(r1)
>> + isync
>> + mtspr SPRN_AMR, \gpr1
>> +
>> + /* No isync required, see kuap_restore_user_amr() */
>> + END_MMU_FTR_SECTION_NESTED_IFSET(MMU_FTR_PKEY , 67)
>> +#endif
>> +.endm
>> +
>> +.macro kuap_restore_kernel_amr gpr1, gpr2
>> +#if defined(CONFIG_PPC_MEM_KEYS)
>> + BEGIN_MMU_FTR_SECTION_NESTED(67)
>> + b 99f // handle_pkey_restore_amr
>> + END_MMU_FTR_SECTION_NESTED_IFSET(MMU_FTR_PKEY , 67)
>> +
>> + BEGIN_MMU_FTR_SECTION_NESTED(68)
>> + b 99f // handle_kuap_restore_amr
>> + MMU_FTR_SECTION_ELSE_NESTED(68)
>> + b 100f // skip_restore_amr
>> + ALT_MMU_FTR_SECTION_END_NESTED_IFSET(MMU_FTR_KUAP, 68)
>> +
>> +99:
>> + /*
>> + * AMR is going to be mostly the same since we are
>> + * returning to the kernel. Compare and do a mtspr.
>> + */
>> ld \gpr2, STACK_REGS_KUAP(r1)
>> + mfspr \gpr1, SPRN_AMR
>> cmpd \gpr1, \gpr2
>> - beq 998f
>> + beq 100f
>> isync
>> mtspr SPRN_AMR, \gpr2
>> /* No isync required, see kuap_restore_amr() */
>> -998:
>> - END_MMU_FTR_SECTION_NESTED_IFSET(MMU_FTR_KUAP, 67)
>> +100: // skip_restore_amr
>
> Can't you code it like this? (_IFCLR requires none of the bits to be
> set)
>
> BEGIN_MMU_FTR_SECTION_NESTED(67)
> b 99f // nothing using AMR, no need to restore
> END_MMU_FTR_SECTION_NESTED_IFCLR(MMU_FTR_PKEY | MMU_FTR_KUAP, 67)
>
> That saves you a branch in the common case of using AMR. Similar
> for others.
Yes i could switch to that. The code is taking extra 200 cycles even
with KUAP/KUEP disabled and no keys being used on hash. I am yet to
analyze this closely. So will rework things based on that analysis.
>
>> @@ -69,22 +133,40 @@
>>
>> extern u64 default_uamor;
>>
>> -static inline void kuap_restore_amr(struct pt_regs *regs, unsigned long amr)
>> +static inline void kuap_restore_user_amr(struct pt_regs *regs)
>> {
>> - if (mmu_has_feature(MMU_FTR_KUAP) && unlikely(regs->kuap != amr)) {
>> - isync();
>> - mtspr(SPRN_AMR, regs->kuap);
>> - /*
>> - * No isync required here because we are about to RFI back to
>> - * previous context before any user accesses would be made,
>> - * which is a CSI.
>> - */
>> + if (!mmu_has_feature(MMU_FTR_PKEY))
>> + return;
>
> If you have PKEY but not KUAP, do you still have to restore?
>
Yes, because user space pkey is now set on the exit path. This is needed
to handle things like exec/fork().
>> +
>> + isync();
>> + mtspr(SPRN_AMR, regs->kuap);
>> + /*
>> + * No isync required here because we are about to rfi
>> + * back to previous context before any user accesses
>> + * would be made, which is a CSI.
>> + */
>> +}
>> +
>> +static inline void kuap_restore_kernel_amr(struct pt_regs *regs,
>> + unsigned long amr)
>> +{
>> + if (mmu_has_feature(MMU_FTR_KUAP) || mmu_has_feature(MMU_FTR_PKEY)) {
>> +
>> + if (unlikely(regs->kuap != amr)) {
>> + isync();
>> + mtspr(SPRN_AMR, regs->kuap);
>> + /*
>> + * No isync required here because we are about to rfi
>> + * back to previous context before any user accesses
>> + * would be made, which is a CSI.
>> + */
>> + }
>> }
>> }
>>
>> static inline unsigned long kuap_get_and_check_amr(void)
>> {
>> - if (mmu_has_feature(MMU_FTR_KUAP)) {
>> + if (mmu_has_feature(MMU_FTR_KUAP) || mmu_has_feature(MMU_FTR_PKEY)) {
>> unsigned long amr = mfspr(SPRN_AMR);
>> if (IS_ENABLED(CONFIG_PPC_KUAP_DEBUG)) /* kuap_check_amr() */
>> WARN_ON_ONCE(amr != AMR_KUAP_BLOCKED);
>
> We could do a static key that's based on this condition, but that can
> wait for another day.
>
>>
>> static inline void kuap_check_amr(void)
>> {
>> - if (IS_ENABLED(CONFIG_PPC_KUAP_DEBUG) && mmu_has_feature(MMU_FTR_KUAP))
>> + if (IS_ENABLED(CONFIG_PPC_KUAP_DEBUG) &&
>> + (mmu_has_feature(MMU_FTR_KUAP) || mmu_has_feature(MMU_FTR_PKEY)))
>> WARN_ON_ONCE(mfspr(SPRN_AMR) != AMR_KUAP_BLOCKED);
>> }
>>
>> #else /* CONFIG_PPC_MEM_KEYS */
>>
>> -static inline void kuap_restore_amr(struct pt_regs *regs, unsigned long amr)
>> +static inline void kuap_restore_user_amr(struct pt_regs *regs)
>> +{
>> +}
>> +
>> +static inline void kuap_restore_kernel_amr(struct pt_regs *regs, unsigned long amr)
>> {
>> }
>>
>> @@ -113,6 +200,7 @@ static inline unsigned long kuap_get_and_check_amr(void)
>> {
>> return 0;
>> }
>> +
>> #endif /* CONFIG_PPC_MEM_KEYS */
>>
>>
>> @@ -187,7 +275,6 @@ bad_kuap_fault(struct pt_regs *regs, unsigned long address, bool is_write)
>> "Bug: %s fault blocked by AMR!", is_write ? "Write" : "Read");
>> }
>> #endif /* CONFIG_PPC_KUAP */
>> -
>> #endif /* __ASSEMBLY__ */
>>
>> #endif /* _ASM_POWERPC_BOOK3S_64_KUP_H */
>
> Unnecessary hunks.
will remove
>
>> diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
>> index 9d49338e0c85..a087cbe0b17d 100644
>> --- a/arch/powerpc/kernel/entry_64.S
>> +++ b/arch/powerpc/kernel/entry_64.S
>> @@ -481,8 +481,8 @@ _ASM_NOKPROBE_SYMBOL(fast_interrupt_return)
>> kuap_check_amr r3, r4
>> ld r5,_MSR(r1)
>> andi. r0,r5,MSR_PR
>> - bne .Lfast_user_interrupt_return
>> - kuap_restore_amr r3, r4
>> + bne .Lfast_user_interrupt_return_amr
>> + kuap_restore_kernel_amr r3, r4
>> andi. r0,r5,MSR_RI
>> li r3,0 /* 0 return value, no EMULATE_STACK_STORE */
>> bne+ .Lfast_kernel_interrupt_return
>> @@ -502,6 +502,8 @@ _ASM_NOKPROBE_SYMBOL(interrupt_return)
>> cmpdi r3,0
>> bne- .Lrestore_nvgprs
>>
>> +.Lfast_user_interrupt_return_amr:
>> + kuap_restore_user_amr r3
>> .Lfast_user_interrupt_return:
>> ld r11,_NIP(r1)
>> ld r12,_MSR(r1)
>> diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
>> index e70ebb5c318c..8226af444d77 100644
>> --- a/arch/powerpc/kernel/exceptions-64s.S
>> +++ b/arch/powerpc/kernel/exceptions-64s.S
>> @@ -971,7 +971,7 @@ EXC_COMMON_BEGIN(system_reset_common)
>> ld r10,SOFTE(r1)
>> stb r10,PACAIRQSOFTMASK(r13)
>>
>> - kuap_restore_amr r9, r10
>> + kuap_restore_kernel_amr r9, r10
>> EXCEPTION_RESTORE_REGS
>> RFI_TO_USER_OR_KERNEL
>>
>> @@ -2784,7 +2784,7 @@ EXC_COMMON_BEGIN(soft_nmi_common)
>> ld r10,SOFTE(r1)
>> stb r10,PACAIRQSOFTMASK(r13)
>>
>> - kuap_restore_amr r9, r10
>> + kuap_restore_kernel_amr r9, r10
>> EXCEPTION_RESTORE_REGS hsrr=0
>> RFI_TO_KERNEL
>>
>> diff --git a/arch/powerpc/kernel/syscall_64.c b/arch/powerpc/kernel/syscall_64.c
>> index 7e560a01afa4..fded67982fbe 100644
>> --- a/arch/powerpc/kernel/syscall_64.c
>> +++ b/arch/powerpc/kernel/syscall_64.c
>> @@ -35,7 +35,21 @@ notrace long system_call_exception(long r3, long r4, long r5,
>> BUG_ON(!FULL_REGS(regs));
>> BUG_ON(regs->softe != IRQS_ENABLED);
>>
>> - kuap_check_amr();
>> +#ifdef CONFIG_PPC_MEM_KEYS
>> + if (mmu_has_feature(MMU_FTR_PKEY)) {
>> + unsigned long amr;
>> + /*
>> + * When entering from userspace we mostly have the AMR
>> + * different from kernel default values. Hence don't compare.
>> + */
>> + amr = mfspr(SPRN_AMR);
>> + regs->kuap = amr;
>> + if (mmu_has_feature(MMU_FTR_KUAP))
>> + mtspr(SPRN_AMR, AMR_KUAP_BLOCKED);
>> + isync();
>
> isync should be inside the if(). Again do pkeys need to save this if
> KUAP is not being used? I haven't really looked at how all that works,
> but what's changing for the PKEY && !KUAP case?
>
There is no SPR switch in context switch now and all the AMR/IAMR
handling is now in the exit to userspace.
> This would be nice if it could all go into a wrapper function rather
> than ifdef.
>
will do
>> + } else
>> +#endif
>> + kuap_check_amr();
>>
>> account_cpu_user_entry();
>>
>> @@ -222,6 +236,10 @@ notrace unsigned long syscall_exit_prepare(unsigned long r3,
>>
>> account_cpu_user_exit();
>>
>> + /*
>> + * We do this at the end so that we do context switch with KERNEL AMR
>> + */
>> + kuap_restore_user_amr(regs);
>> return ret;
>
> Comment doesn't make sense, newline required before return.
Ok the detail there was we need to make sure we restore AMR towrads the
end and make sure all the kernel code continue to run with KERNEL AMR
value. There is a schedule() call in there with _TIF_NEED_RESCHED. But
those details are not really relevant. That was me tracking down some
issues and writing comment around that part of the code. The only real
detail is switch to userspace AMR in the end.
>
>> }
>>
>> @@ -306,6 +324,10 @@ notrace unsigned long interrupt_exit_user_prepare(struct pt_regs *regs, unsigned
>>
>> account_cpu_user_exit();
>>
>> + /*
>> + * We do this at the end so that we do context switch with KERNEL AMR
>> + */
>> + kuap_restore_user_amr(regs);
>
> Duplicated comments I prefer to just have like this instead of trying to
> keep them in sync. Can complete the circular reference by having a
>
> * similarly in interrupt_exit_user_prepare
>
> in the main comment, but if they come close to one another in the same
> file it's not so important to keep them together.
>
> + kuap_restore_user_amr(regs); /* see syscall_exit_prepare */
>
>> return ret;
>> }
>>
>> @@ -376,7 +398,7 @@ notrace unsigned long interrupt_exit_kernel_prepare(struct pt_regs *regs, unsign
>> * which would cause Read-After-Write stalls. Hence, we take the AMR
>> * value from the check above.
>> */
>> - kuap_restore_amr(regs, amr);
>> + kuap_restore_kernel_amr(regs, amr);
>>
>> return ret;
>> }
>> --
>> 2.26.2
>>
>>
-aneesh
^ permalink raw reply
* Re: [PATCH v5 3/3] mm/page_alloc: Keep memoryless cpuless node 0 offline
From: Michal Hocko @ 2020-07-03 9:24 UTC (permalink / raw)
To: Michal Suchánek
Cc: Gautham R Shenoy, Andi Kleen, Srikar Dronamraju,
David Hildenbrand, linuxppc-dev, linux-kernel, linux-mm,
Satheesh Rajendran, Mel Gorman, Kirill A. Shutemov, Andrew Morton,
Linus Torvalds, Christopher Lameter, Vlastimil Babka
In-Reply-To: <20200703091001.GJ21462@kitsune.suse.cz>
[Cc Andi]
On Fri 03-07-20 11:10:01, Michal Suchanek wrote:
> On Wed, Jul 01, 2020 at 02:21:10PM +0200, Michal Hocko wrote:
> > On Wed 01-07-20 13:30:57, David Hildenbrand wrote:
[...]
> > > Yep, looks like it.
> > >
> > > [ 0.009726] SRAT: PXM 1 -> APIC 0x00 -> Node 0
> > > [ 0.009727] SRAT: PXM 1 -> APIC 0x01 -> Node 0
> > > [ 0.009727] SRAT: PXM 1 -> APIC 0x02 -> Node 0
> > > [ 0.009728] SRAT: PXM 1 -> APIC 0x03 -> Node 0
> > > [ 0.009731] ACPI: SRAT: Node 0 PXM 1 [mem 0x00000000-0x0009ffff]
> > > [ 0.009732] ACPI: SRAT: Node 0 PXM 1 [mem 0x00100000-0xbfffffff]
> > > [ 0.009733] ACPI: SRAT: Node 0 PXM 1 [mem 0x100000000-0x13fffffff]
> >
> > This begs a question whether ppc can do the same thing?
> Or x86 stop doing it so that you can see on what node you are running?
>
> What's the point of this indirection other than another way of avoiding
> empty node 0?
Honestly, I do not have any idea. I've traced it down to
Author: Andi Kleen <ak@suse.de>
Date: Tue Jan 11 15:35:48 2005 -0800
[PATCH] x86_64: Fix ACPI SRAT NUMA parsing
Fix fallout from the recent nodemask_t changes. The node ids assigned
in the SRAT parser were off by one.
I added a new first_unset_node() function to nodemask.h to allocate
IDs sanely.
Signed-off-by: Andi Kleen <ak@suse.de>
Signed-off-by: Linus Torvalds <torvalds@osdl.org>
which doesn't really tell all that much. The historical baggage and a
long term behavior which is not really trivial to fix I suspect.
--
Michal Hocko
SUSE Labs
^ permalink raw reply
* Re: [PATCH 1/2] dt-bindings: sound: fsl-asoc-card: add new compatible for I2S slave
From: Arnaud Ferraris @ 2020-07-03 9:23 UTC (permalink / raw)
To: Mark Brown
Cc: devicetree, alsa-devel, Timur Tabi, Xiubo Li, Liam Girdwood,
linux-kernel, Takashi Iwai, Nicolin Chen, Rob Herring,
Fabio Estevam, kernel, linuxppc-dev
In-Reply-To: <20200702154207.GK4483@sirena.org.uk>
Le 02/07/2020 à 17:42, Mark Brown a écrit :
> On Thu, Jul 02, 2020 at 05:28:03PM +0200, Arnaud Ferraris wrote:
>> Le 02/07/2020 à 16:31, Mark Brown a écrit :
>
>>> Why require that the CODEC be clock master here - why not make this
>>> configurable, reusing the properties from the generic and audio graph
>>> cards?
>
>> This is partly because I'm not sure how to do it (yet), but mostly
>> because I don't have the hardware to test this (the 2 CODECs present on
>> my only i.MX6 board are both clock master)
>
> Take a look at what the generic cards are doing, it's a library function
> asoc_simple_parse_daifmt(). It's not the end of the world if you can't
> test it properly - if it turns out it's buggy somehow someone can always
> fix the code later but an ABI is an ABI so we can't change it.
>
Thanks for the hints, I'll look into it.
Regards,
Arnaud
^ permalink raw reply
* Re: [PATCH v4 24/41] powerpc/book3s64/pkeys: Store/restore userspace AMR correctly on entry and exit from kernel
From: Nicholas Piggin @ 2020-07-03 9:18 UTC (permalink / raw)
To: Aneesh Kumar K.V, linuxppc-dev, mpe; +Cc: linuxram, bauerman
In-Reply-To: <20200615061430.770174-25-aneesh.kumar@linux.ibm.com>
Excerpts from Aneesh Kumar K.V's message of June 15, 2020 4:14 pm:
> This prepare kernel to operate with a different value than userspace AMR.
> For this, AMR needs to be saved and restored on entry and return from the
> kernel.
>
> With KUAP we modify kernel AMR when accessing user address from the kernel
> via copy_to/from_user interfaces.
>
> If MMU_FTR_KEY is enabled we always use the key mechanism to implement KUAP
> feature. If MMU_FTR_KEY is not supported and if we support MMU_FTR_KUAP
> (radix translation on POWER9), we can skip restoring AMR on return
> to userspace. Userspace won't be using AMR in that specific config.
>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> ---
> arch/powerpc/include/asm/book3s/64/kup.h | 141 ++++++++++++++++++-----
> arch/powerpc/kernel/entry_64.S | 6 +-
> arch/powerpc/kernel/exceptions-64s.S | 4 +-
> arch/powerpc/kernel/syscall_64.c | 26 ++++-
> 4 files changed, 144 insertions(+), 33 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/book3s/64/kup.h b/arch/powerpc/include/asm/book3s/64/kup.h
> index e6ee1c34842f..6979cd1a0003 100644
> --- a/arch/powerpc/include/asm/book3s/64/kup.h
> +++ b/arch/powerpc/include/asm/book3s/64/kup.h
> @@ -13,18 +13,47 @@
>
> #ifdef __ASSEMBLY__
>
> -.macro kuap_restore_amr gpr1, gpr2
> -#ifdef CONFIG_PPC_KUAP
> +.macro kuap_restore_user_amr gpr1
> +#if defined(CONFIG_PPC_MEM_KEYS)
> BEGIN_MMU_FTR_SECTION_NESTED(67)
> - mfspr \gpr1, SPRN_AMR
> + /*
> + * AMR is going to be different when
> + * returning to userspace.
> + */
> + ld \gpr1, STACK_REGS_KUAP(r1)
> + isync
> + mtspr SPRN_AMR, \gpr1
> +
> + /* No isync required, see kuap_restore_user_amr() */
> + END_MMU_FTR_SECTION_NESTED_IFSET(MMU_FTR_PKEY , 67)
> +#endif
> +.endm
> +
> +.macro kuap_restore_kernel_amr gpr1, gpr2
> +#if defined(CONFIG_PPC_MEM_KEYS)
> + BEGIN_MMU_FTR_SECTION_NESTED(67)
> + b 99f // handle_pkey_restore_amr
> + END_MMU_FTR_SECTION_NESTED_IFSET(MMU_FTR_PKEY , 67)
> +
> + BEGIN_MMU_FTR_SECTION_NESTED(68)
> + b 99f // handle_kuap_restore_amr
> + MMU_FTR_SECTION_ELSE_NESTED(68)
> + b 100f // skip_restore_amr
> + ALT_MMU_FTR_SECTION_END_NESTED_IFSET(MMU_FTR_KUAP, 68)
> +
> +99:
> + /*
> + * AMR is going to be mostly the same since we are
> + * returning to the kernel. Compare and do a mtspr.
> + */
> ld \gpr2, STACK_REGS_KUAP(r1)
> + mfspr \gpr1, SPRN_AMR
> cmpd \gpr1, \gpr2
> - beq 998f
> + beq 100f
> isync
> mtspr SPRN_AMR, \gpr2
> /* No isync required, see kuap_restore_amr() */
> -998:
> - END_MMU_FTR_SECTION_NESTED_IFSET(MMU_FTR_KUAP, 67)
> +100: // skip_restore_amr
Can't you code it like this? (_IFCLR requires none of the bits to be
set)
BEGIN_MMU_FTR_SECTION_NESTED(67)
b 99f // nothing using AMR, no need to restore
END_MMU_FTR_SECTION_NESTED_IFCLR(MMU_FTR_PKEY | MMU_FTR_KUAP, 67)
That saves you a branch in the common case of using AMR. Similar
for others.
> @@ -69,22 +133,40 @@
>
> extern u64 default_uamor;
>
> -static inline void kuap_restore_amr(struct pt_regs *regs, unsigned long amr)
> +static inline void kuap_restore_user_amr(struct pt_regs *regs)
> {
> - if (mmu_has_feature(MMU_FTR_KUAP) && unlikely(regs->kuap != amr)) {
> - isync();
> - mtspr(SPRN_AMR, regs->kuap);
> - /*
> - * No isync required here because we are about to RFI back to
> - * previous context before any user accesses would be made,
> - * which is a CSI.
> - */
> + if (!mmu_has_feature(MMU_FTR_PKEY))
> + return;
If you have PKEY but not KUAP, do you still have to restore?
> +
> + isync();
> + mtspr(SPRN_AMR, regs->kuap);
> + /*
> + * No isync required here because we are about to rfi
> + * back to previous context before any user accesses
> + * would be made, which is a CSI.
> + */
> +}
> +
> +static inline void kuap_restore_kernel_amr(struct pt_regs *regs,
> + unsigned long amr)
> +{
> + if (mmu_has_feature(MMU_FTR_KUAP) || mmu_has_feature(MMU_FTR_PKEY)) {
> +
> + if (unlikely(regs->kuap != amr)) {
> + isync();
> + mtspr(SPRN_AMR, regs->kuap);
> + /*
> + * No isync required here because we are about to rfi
> + * back to previous context before any user accesses
> + * would be made, which is a CSI.
> + */
> + }
> }
> }
>
> static inline unsigned long kuap_get_and_check_amr(void)
> {
> - if (mmu_has_feature(MMU_FTR_KUAP)) {
> + if (mmu_has_feature(MMU_FTR_KUAP) || mmu_has_feature(MMU_FTR_PKEY)) {
> unsigned long amr = mfspr(SPRN_AMR);
> if (IS_ENABLED(CONFIG_PPC_KUAP_DEBUG)) /* kuap_check_amr() */
> WARN_ON_ONCE(amr != AMR_KUAP_BLOCKED);
We could do a static key that's based on this condition, but that can
wait for another day.
>
> static inline void kuap_check_amr(void)
> {
> - if (IS_ENABLED(CONFIG_PPC_KUAP_DEBUG) && mmu_has_feature(MMU_FTR_KUAP))
> + if (IS_ENABLED(CONFIG_PPC_KUAP_DEBUG) &&
> + (mmu_has_feature(MMU_FTR_KUAP) || mmu_has_feature(MMU_FTR_PKEY)))
> WARN_ON_ONCE(mfspr(SPRN_AMR) != AMR_KUAP_BLOCKED);
> }
>
> #else /* CONFIG_PPC_MEM_KEYS */
>
> -static inline void kuap_restore_amr(struct pt_regs *regs, unsigned long amr)
> +static inline void kuap_restore_user_amr(struct pt_regs *regs)
> +{
> +}
> +
> +static inline void kuap_restore_kernel_amr(struct pt_regs *regs, unsigned long amr)
> {
> }
>
> @@ -113,6 +200,7 @@ static inline unsigned long kuap_get_and_check_amr(void)
> {
> return 0;
> }
> +
> #endif /* CONFIG_PPC_MEM_KEYS */
>
>
> @@ -187,7 +275,6 @@ bad_kuap_fault(struct pt_regs *regs, unsigned long address, bool is_write)
> "Bug: %s fault blocked by AMR!", is_write ? "Write" : "Read");
> }
> #endif /* CONFIG_PPC_KUAP */
> -
> #endif /* __ASSEMBLY__ */
>
> #endif /* _ASM_POWERPC_BOOK3S_64_KUP_H */
Unnecessary hunks.
> diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
> index 9d49338e0c85..a087cbe0b17d 100644
> --- a/arch/powerpc/kernel/entry_64.S
> +++ b/arch/powerpc/kernel/entry_64.S
> @@ -481,8 +481,8 @@ _ASM_NOKPROBE_SYMBOL(fast_interrupt_return)
> kuap_check_amr r3, r4
> ld r5,_MSR(r1)
> andi. r0,r5,MSR_PR
> - bne .Lfast_user_interrupt_return
> - kuap_restore_amr r3, r4
> + bne .Lfast_user_interrupt_return_amr
> + kuap_restore_kernel_amr r3, r4
> andi. r0,r5,MSR_RI
> li r3,0 /* 0 return value, no EMULATE_STACK_STORE */
> bne+ .Lfast_kernel_interrupt_return
> @@ -502,6 +502,8 @@ _ASM_NOKPROBE_SYMBOL(interrupt_return)
> cmpdi r3,0
> bne- .Lrestore_nvgprs
>
> +.Lfast_user_interrupt_return_amr:
> + kuap_restore_user_amr r3
> .Lfast_user_interrupt_return:
> ld r11,_NIP(r1)
> ld r12,_MSR(r1)
> diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
> index e70ebb5c318c..8226af444d77 100644
> --- a/arch/powerpc/kernel/exceptions-64s.S
> +++ b/arch/powerpc/kernel/exceptions-64s.S
> @@ -971,7 +971,7 @@ EXC_COMMON_BEGIN(system_reset_common)
> ld r10,SOFTE(r1)
> stb r10,PACAIRQSOFTMASK(r13)
>
> - kuap_restore_amr r9, r10
> + kuap_restore_kernel_amr r9, r10
> EXCEPTION_RESTORE_REGS
> RFI_TO_USER_OR_KERNEL
>
> @@ -2784,7 +2784,7 @@ EXC_COMMON_BEGIN(soft_nmi_common)
> ld r10,SOFTE(r1)
> stb r10,PACAIRQSOFTMASK(r13)
>
> - kuap_restore_amr r9, r10
> + kuap_restore_kernel_amr r9, r10
> EXCEPTION_RESTORE_REGS hsrr=0
> RFI_TO_KERNEL
>
> diff --git a/arch/powerpc/kernel/syscall_64.c b/arch/powerpc/kernel/syscall_64.c
> index 7e560a01afa4..fded67982fbe 100644
> --- a/arch/powerpc/kernel/syscall_64.c
> +++ b/arch/powerpc/kernel/syscall_64.c
> @@ -35,7 +35,21 @@ notrace long system_call_exception(long r3, long r4, long r5,
> BUG_ON(!FULL_REGS(regs));
> BUG_ON(regs->softe != IRQS_ENABLED);
>
> - kuap_check_amr();
> +#ifdef CONFIG_PPC_MEM_KEYS
> + if (mmu_has_feature(MMU_FTR_PKEY)) {
> + unsigned long amr;
> + /*
> + * When entering from userspace we mostly have the AMR
> + * different from kernel default values. Hence don't compare.
> + */
> + amr = mfspr(SPRN_AMR);
> + regs->kuap = amr;
> + if (mmu_has_feature(MMU_FTR_KUAP))
> + mtspr(SPRN_AMR, AMR_KUAP_BLOCKED);
> + isync();
isync should be inside the if(). Again do pkeys need to save this if
KUAP is not being used? I haven't really looked at how all that works,
but what's changing for the PKEY && !KUAP case?
This would be nice if it could all go into a wrapper function rather
than ifdef.
> + } else
> +#endif
> + kuap_check_amr();
>
> account_cpu_user_entry();
>
> @@ -222,6 +236,10 @@ notrace unsigned long syscall_exit_prepare(unsigned long r3,
>
> account_cpu_user_exit();
>
> + /*
> + * We do this at the end so that we do context switch with KERNEL AMR
> + */
> + kuap_restore_user_amr(regs);
> return ret;
Comment doesn't make sense, newline required before return.
> }
>
> @@ -306,6 +324,10 @@ notrace unsigned long interrupt_exit_user_prepare(struct pt_regs *regs, unsigned
>
> account_cpu_user_exit();
>
> + /*
> + * We do this at the end so that we do context switch with KERNEL AMR
> + */
> + kuap_restore_user_amr(regs);
Duplicated comments I prefer to just have like this instead of trying to
keep them in sync. Can complete the circular reference by having a
* similarly in interrupt_exit_user_prepare
in the main comment, but if they come close to one another in the same
file it's not so important to keep them together.
+ kuap_restore_user_amr(regs); /* see syscall_exit_prepare */
> return ret;
> }
>
> @@ -376,7 +398,7 @@ notrace unsigned long interrupt_exit_kernel_prepare(struct pt_regs *regs, unsign
> * which would cause Read-After-Write stalls. Hence, we take the AMR
> * value from the check above.
> */
> - kuap_restore_amr(regs, amr);
> + kuap_restore_kernel_amr(regs, amr);
>
> return ret;
> }
> --
> 2.26.2
>
>
^ permalink raw reply
* Re: [PATCH v5 3/3] mm/page_alloc: Keep memoryless cpuless node 0 offline
From: Michal Suchánek @ 2020-07-03 9:10 UTC (permalink / raw)
To: Michal Hocko
Cc: Gautham R Shenoy, Srikar Dronamraju, David Hildenbrand,
linuxppc-dev, linux-kernel, linux-mm, Satheesh Rajendran,
Mel Gorman, Kirill A. Shutemov, Andrew Morton, Linus Torvalds,
Christopher Lameter, Vlastimil Babka
In-Reply-To: <20200701122110.GT2369@dhcp22.suse.cz>
On Wed, Jul 01, 2020 at 02:21:10PM +0200, Michal Hocko wrote:
> On Wed 01-07-20 13:30:57, David Hildenbrand wrote:
> > On 01.07.20 13:06, David Hildenbrand wrote:
> > > On 01.07.20 13:01, Srikar Dronamraju wrote:
> > >> * David Hildenbrand <david@redhat.com> [2020-07-01 12:15:54]:
> > >>
> > >>> On 01.07.20 12:04, Srikar Dronamraju wrote:
> > >>>> * Michal Hocko <mhocko@kernel.org> [2020-07-01 10:42:00]:
> > >>>>
> > >>>>>
> > >>>>>>
> > >>>>>> 2. Also existence of dummy node also leads to inconsistent information. The
> > >>>>>> number of online nodes is inconsistent with the information in the
> > >>>>>> device-tree and resource-dump
> > >>>>>>
> > >>>>>> 3. When the dummy node is present, single node non-Numa systems end up showing
> > >>>>>> up as NUMA systems and numa_balancing gets enabled. This will mean we take
> > >>>>>> the hit from the unnecessary numa hinting faults.
> > >>>>>
> > >>>>> I have to say that I dislike the node online/offline state and directly
> > >>>>> exporting that to the userspace. Users should only care whether the node
> > >>>>> has memory/cpus. Numa nodes can be online without any memory. Just
> > >>>>> offline all the present memory blocks but do not physically hot remove
> > >>>>> them and you are in the same situation. If users are confused by an
> > >>>>> output of tools like numactl -H then those could be updated and hide
> > >>>>> nodes without any memory&cpus.
> > >>>>>
> > >>>>> The autonuma problem sounds interesting but again this patch doesn't
> > >>>>> really solve the underlying problem because I strongly suspect that the
> > >>>>> problem is still there when a numa node gets all its memory offline as
> > >>>>> mentioned above.
>
> I would really appreciate a feedback to these two as well.
>
> > >>>>> While I completely agree that making node 0 special is wrong, I have
> > >>>>> still hard time to review this very simply looking patch because all the
> > >>>>> numa initialization is so spread around that this might just blow up
> > >>>>> at unexpected places. IIRC we have discussed testing in the previous
> > >>>>> version and David has provided a way to emulate these configurations
> > >>>>> on x86. Did you manage to use those instruction for additional testing
> > >>>>> on other than ppc architectures?
> > >>>>>
> > >>>>
> > >>>> I have tried all the steps that David mentioned and reported back at
> > >>>> https://lore.kernel.org/lkml/20200511174731.GD1961@linux.vnet.ibm.com/t/#u
> > >>>>
> > >>>> As a summary, David's steps are still not creating a memoryless/cpuless on
> > >>>> x86 VM.
> > >>>
> > >>> Now, that is wrong. You get a memoryless/cpuless node, which is *not
> > >>> online*. Once you hotplug some memory, it will switch online. Once you
> > >>> remove memory, it will switch back offline.
> > >>>
> > >>
> > >> Let me clarify, we are looking for a node 0 which is cpuless/memoryless at
> > >> boot. The code in question tries to handle a cpuless/memoryless node 0 at
> > >> boot.
> > >
> > > I was just correcting your statement, because it was wrong.
> > >
> > > Could be that x86 code maps PXM 1 to node 0 because PXM 1 does neither
> > > have CPUs nor memory. That would imply that we can, in fact, never have
> > > node 0 offline during boot.
> > >
> >
> > Yep, looks like it.
> >
> > [ 0.009726] SRAT: PXM 1 -> APIC 0x00 -> Node 0
> > [ 0.009727] SRAT: PXM 1 -> APIC 0x01 -> Node 0
> > [ 0.009727] SRAT: PXM 1 -> APIC 0x02 -> Node 0
> > [ 0.009728] SRAT: PXM 1 -> APIC 0x03 -> Node 0
> > [ 0.009731] ACPI: SRAT: Node 0 PXM 1 [mem 0x00000000-0x0009ffff]
> > [ 0.009732] ACPI: SRAT: Node 0 PXM 1 [mem 0x00100000-0xbfffffff]
> > [ 0.009733] ACPI: SRAT: Node 0 PXM 1 [mem 0x100000000-0x13fffffff]
>
> This begs a question whether ppc can do the same thing?
Or x86 stop doing it so that you can see on what node you are running?
What's the point of this indirection other than another way of avoiding
empty node 0?
Thanks
Michal
^ permalink raw reply
* [PATCH v2 6/6] powerpc/qspinlock: optimised atomic_try_cmpxchg_lock that adds the lock hint
From: Nicholas Piggin @ 2020-07-03 7:35 UTC (permalink / raw)
Cc: linux-arch, Peter Zijlstra, linuxppc-dev, Boqun Feng,
linux-kernel, Nicholas Piggin, virtualization, Ingo Molnar,
kvm-ppc, Waiman Long, Will Deacon
In-Reply-To: <20200703073516.1354108-1-npiggin@gmail.com>
This brings the behaviour of the uncontended fast path back to
roughly equivalent to simple spinlocks -- a single atomic op with
lock hint.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/include/asm/atomic.h | 28 ++++++++++++++++++++++++++++
arch/powerpc/include/asm/qspinlock.h | 2 +-
2 files changed, 29 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/include/asm/atomic.h b/arch/powerpc/include/asm/atomic.h
index 498785ffc25f..f6a3d145ffb7 100644
--- a/arch/powerpc/include/asm/atomic.h
+++ b/arch/powerpc/include/asm/atomic.h
@@ -193,6 +193,34 @@ static __inline__ int atomic_dec_return_relaxed(atomic_t *v)
#define atomic_xchg(v, new) (xchg(&((v)->counter), new))
#define atomic_xchg_relaxed(v, new) xchg_relaxed(&((v)->counter), (new))
+/*
+ * Don't want to override the generic atomic_try_cmpxchg_acquire, because
+ * we add a lock hint to the lwarx, which may not be wanted for the
+ * _acquire case (and is not used by the other _acquire variants so it
+ * would be a surprise).
+ */
+static __always_inline bool
+atomic_try_cmpxchg_lock(atomic_t *v, int *old, int new)
+{
+ int r, o = *old;
+
+ __asm__ __volatile__ (
+"1:\t" PPC_LWARX(%0,0,%2,1) " # atomic_try_cmpxchg_acquire \n"
+" cmpw 0,%0,%3 \n"
+" bne- 2f \n"
+" stwcx. %4,0,%2 \n"
+" bne- 1b \n"
+"\t" PPC_ACQUIRE_BARRIER " \n"
+"2: \n"
+ : "=&r" (r), "+m" (v->counter)
+ : "r" (&v->counter), "r" (o), "r" (new)
+ : "cr0", "memory");
+
+ if (unlikely(r != o))
+ *old = r;
+ return likely(r == o);
+}
+
/**
* atomic_fetch_add_unless - add unless the number is a given value
* @v: pointer of type atomic_t
diff --git a/arch/powerpc/include/asm/qspinlock.h b/arch/powerpc/include/asm/qspinlock.h
index 0960a0de2467..beb6aa4628e7 100644
--- a/arch/powerpc/include/asm/qspinlock.h
+++ b/arch/powerpc/include/asm/qspinlock.h
@@ -26,7 +26,7 @@ static __always_inline void queued_spin_lock(struct qspinlock *lock)
{
u32 val = 0;
- if (likely(atomic_try_cmpxchg_acquire(&lock->val, &val, _Q_LOCKED_VAL)))
+ if (likely(atomic_try_cmpxchg_lock(&lock->val, &val, _Q_LOCKED_VAL)))
return;
queued_spin_lock_slowpath(lock, val);
--
2.23.0
^ permalink raw reply related
* [PATCH v2 5/6] powerpc/pseries: implement paravirt qspinlocks for SPLPAR
From: Nicholas Piggin @ 2020-07-03 7:35 UTC (permalink / raw)
Cc: linux-arch, Peter Zijlstra, linuxppc-dev, Boqun Feng,
linux-kernel, Nicholas Piggin, virtualization, Ingo Molnar,
kvm-ppc, Waiman Long, Will Deacon
In-Reply-To: <20200703073516.1354108-1-npiggin@gmail.com>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/include/asm/paravirt.h | 28 ++++++++++
arch/powerpc/include/asm/qspinlock.h | 55 +++++++++++++++++++
arch/powerpc/include/asm/qspinlock_paravirt.h | 5 ++
arch/powerpc/platforms/pseries/Kconfig | 5 ++
arch/powerpc/platforms/pseries/setup.c | 6 +-
include/asm-generic/qspinlock.h | 2 +
6 files changed, 100 insertions(+), 1 deletion(-)
create mode 100644 arch/powerpc/include/asm/qspinlock_paravirt.h
diff --git a/arch/powerpc/include/asm/paravirt.h b/arch/powerpc/include/asm/paravirt.h
index 7a8546660a63..f2d51f929cf5 100644
--- a/arch/powerpc/include/asm/paravirt.h
+++ b/arch/powerpc/include/asm/paravirt.h
@@ -29,6 +29,16 @@ static inline void yield_to_preempted(int cpu, u32 yield_count)
{
plpar_hcall_norets(H_CONFER, get_hard_smp_processor_id(cpu), yield_count);
}
+
+static inline void prod_cpu(int cpu)
+{
+ plpar_hcall_norets(H_PROD, get_hard_smp_processor_id(cpu));
+}
+
+static inline void yield_to_any(void)
+{
+ plpar_hcall_norets(H_CONFER, -1, 0);
+}
#else
static inline bool is_shared_processor(void)
{
@@ -45,6 +55,19 @@ static inline void yield_to_preempted(int cpu, u32 yield_count)
{
___bad_yield_to_preempted(); /* This would be a bug */
}
+
+extern void ___bad_yield_to_any(void);
+static inline void yield_to_any(void)
+{
+ ___bad_yield_to_any(); /* This would be a bug */
+}
+
+extern void ___bad_prod_cpu(void);
+static inline void prod_cpu(int cpu)
+{
+ ___bad_prod_cpu(); /* This would be a bug */
+}
+
#endif
#define vcpu_is_preempted vcpu_is_preempted
@@ -57,5 +80,10 @@ static inline bool vcpu_is_preempted(int cpu)
return false;
}
+static inline bool pv_is_native_spin_unlock(void)
+{
+ return !is_shared_processor();
+}
+
#endif /* __KERNEL__ */
#endif /* __ASM_PARAVIRT_H */
diff --git a/arch/powerpc/include/asm/qspinlock.h b/arch/powerpc/include/asm/qspinlock.h
index c49e33e24edd..0960a0de2467 100644
--- a/arch/powerpc/include/asm/qspinlock.h
+++ b/arch/powerpc/include/asm/qspinlock.h
@@ -3,9 +3,36 @@
#define _ASM_POWERPC_QSPINLOCK_H
#include <asm-generic/qspinlock_types.h>
+#include <asm/paravirt.h>
#define _Q_PENDING_LOOPS (1 << 9) /* not tuned */
+#ifdef CONFIG_PARAVIRT_SPINLOCKS
+extern void native_queued_spin_lock_slowpath(struct qspinlock *lock, u32 val);
+extern void __pv_queued_spin_lock_slowpath(struct qspinlock *lock, u32 val);
+
+static __always_inline void queued_spin_lock_slowpath(struct qspinlock *lock, u32 val)
+{
+ if (!is_shared_processor())
+ native_queued_spin_lock_slowpath(lock, val);
+ else
+ __pv_queued_spin_lock_slowpath(lock, val);
+}
+#else
+extern void queued_spin_lock_slowpath(struct qspinlock *lock, u32 val);
+#endif
+
+static __always_inline void queued_spin_lock(struct qspinlock *lock)
+{
+ u32 val = 0;
+
+ if (likely(atomic_try_cmpxchg_acquire(&lock->val, &val, _Q_LOCKED_VAL)))
+ return;
+
+ queued_spin_lock_slowpath(lock, val);
+}
+#define queued_spin_lock queued_spin_lock
+
#define smp_mb__after_spinlock() smp_mb()
static __always_inline int queued_spin_is_locked(struct qspinlock *lock)
@@ -20,6 +47,34 @@ static __always_inline int queued_spin_is_locked(struct qspinlock *lock)
}
#define queued_spin_is_locked queued_spin_is_locked
+#ifdef CONFIG_PARAVIRT_SPINLOCKS
+#define SPIN_THRESHOLD (1<<15) /* not tuned */
+
+static __always_inline void pv_wait(u8 *ptr, u8 val)
+{
+ if (*ptr != val)
+ return;
+ yield_to_any();
+ /*
+ * We could pass in a CPU here if waiting in the queue and yield to
+ * the previous CPU in the queue.
+ */
+}
+
+static __always_inline void pv_kick(int cpu)
+{
+ prod_cpu(cpu);
+}
+
+extern void __pv_init_lock_hash(void);
+
+static inline void pv_spinlocks_init(void)
+{
+ __pv_init_lock_hash();
+}
+
+#endif
+
#include <asm-generic/qspinlock.h>
#endif /* _ASM_POWERPC_QSPINLOCK_H */
diff --git a/arch/powerpc/include/asm/qspinlock_paravirt.h b/arch/powerpc/include/asm/qspinlock_paravirt.h
new file mode 100644
index 000000000000..6dbdb8a4f84f
--- /dev/null
+++ b/arch/powerpc/include/asm/qspinlock_paravirt.h
@@ -0,0 +1,5 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+#ifndef __ASM_QSPINLOCK_PARAVIRT_H
+#define __ASM_QSPINLOCK_PARAVIRT_H
+
+#endif /* __ASM_QSPINLOCK_PARAVIRT_H */
diff --git a/arch/powerpc/platforms/pseries/Kconfig b/arch/powerpc/platforms/pseries/Kconfig
index 24c18362e5ea..756e727b383f 100644
--- a/arch/powerpc/platforms/pseries/Kconfig
+++ b/arch/powerpc/platforms/pseries/Kconfig
@@ -25,9 +25,14 @@ config PPC_PSERIES
select SWIOTLB
default y
+config PARAVIRT_SPINLOCKS
+ bool
+ default n
+
config PPC_SPLPAR
depends on PPC_PSERIES
bool "Support for shared-processor logical partitions"
+ select PARAVIRT_SPINLOCKS if PPC_QUEUED_SPINLOCKS
help
Enabling this option will make the kernel run more efficiently
on logically-partitioned pSeries systems which use shared
diff --git a/arch/powerpc/platforms/pseries/setup.c b/arch/powerpc/platforms/pseries/setup.c
index 2db8469e475f..747a203d9453 100644
--- a/arch/powerpc/platforms/pseries/setup.c
+++ b/arch/powerpc/platforms/pseries/setup.c
@@ -771,8 +771,12 @@ static void __init pSeries_setup_arch(void)
if (firmware_has_feature(FW_FEATURE_LPAR)) {
vpa_init(boot_cpuid);
- if (lppaca_shared_proc(get_lppaca()))
+ if (lppaca_shared_proc(get_lppaca())) {
static_branch_enable(&shared_processor);
+#ifdef CONFIG_PARAVIRT_SPINLOCKS
+ pv_spinlocks_init();
+#endif
+ }
ppc_md.power_save = pseries_lpar_idle;
ppc_md.enable_pmcs = pseries_lpar_enable_pmcs;
diff --git a/include/asm-generic/qspinlock.h b/include/asm-generic/qspinlock.h
index fb0a814d4395..38ca14e79a86 100644
--- a/include/asm-generic/qspinlock.h
+++ b/include/asm-generic/qspinlock.h
@@ -69,6 +69,7 @@ static __always_inline int queued_spin_trylock(struct qspinlock *lock)
extern void queued_spin_lock_slowpath(struct qspinlock *lock, u32 val);
+#ifndef queued_spin_lock
/**
* queued_spin_lock - acquire a queued spinlock
* @lock: Pointer to queued spinlock structure
@@ -82,6 +83,7 @@ static __always_inline void queued_spin_lock(struct qspinlock *lock)
queued_spin_lock_slowpath(lock, val);
}
+#endif
#ifndef queued_spin_unlock
/**
--
2.23.0
^ permalink raw reply related
* [PATCH v2 4/6] powerpc/64s: implement queued spinlocks and rwlocks
From: Nicholas Piggin @ 2020-07-03 7:35 UTC (permalink / raw)
Cc: linux-arch, Peter Zijlstra, linuxppc-dev, Boqun Feng,
linux-kernel, Nicholas Piggin, virtualization, Ingo Molnar,
kvm-ppc, Waiman Long, Will Deacon
In-Reply-To: <20200703073516.1354108-1-npiggin@gmail.com>
These have shown significantly improved performance and fairness when
spinlock contention is moderate to high on very large systems.
[ Numbers hopefully forthcoming after more testing, but initial
results look good ]
Thanks to the fast path, single threaded performance is not noticably
hurt.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/Kconfig | 13 ++++++++++++
arch/powerpc/include/asm/Kbuild | 2 ++
arch/powerpc/include/asm/qspinlock.h | 25 +++++++++++++++++++++++
arch/powerpc/include/asm/spinlock.h | 5 +++++
arch/powerpc/include/asm/spinlock_types.h | 5 +++++
arch/powerpc/lib/Makefile | 3 +++
include/asm-generic/qspinlock.h | 2 ++
7 files changed, 55 insertions(+)
create mode 100644 arch/powerpc/include/asm/qspinlock.h
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 9fa23eb320ff..b17575109876 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -145,6 +145,8 @@ config PPC
select ARCH_SUPPORTS_ATOMIC_RMW
select ARCH_USE_BUILTIN_BSWAP
select ARCH_USE_CMPXCHG_LOCKREF if PPC64
+ select ARCH_USE_QUEUED_RWLOCKS if PPC_QUEUED_SPINLOCKS
+ select ARCH_USE_QUEUED_SPINLOCKS if PPC_QUEUED_SPINLOCKS
select ARCH_WANT_IPC_PARSE_VERSION
select ARCH_WEAK_RELEASE_ACQUIRE
select BINFMT_ELF
@@ -490,6 +492,17 @@ config HOTPLUG_CPU
Say N if you are unsure.
+config PPC_QUEUED_SPINLOCKS
+ bool "Queued spinlocks"
+ depends on SMP
+ default "y" if PPC_BOOK3S_64
+ help
+ Say Y here to use to use queued spinlocks which are more complex
+ but give better salability and fairness on large SMP and NUMA
+ systems.
+
+ If unsure, say "Y" if you have lots of cores, otherwise "N".
+
config ARCH_CPU_PROBE_RELEASE
def_bool y
depends on HOTPLUG_CPU
diff --git a/arch/powerpc/include/asm/Kbuild b/arch/powerpc/include/asm/Kbuild
index dadbcf3a0b1e..1dd8b6adff5e 100644
--- a/arch/powerpc/include/asm/Kbuild
+++ b/arch/powerpc/include/asm/Kbuild
@@ -6,5 +6,7 @@ generated-y += syscall_table_spu.h
generic-y += export.h
generic-y += local64.h
generic-y += mcs_spinlock.h
+generic-y += qrwlock.h
+generic-y += qspinlock.h
generic-y += vtime.h
generic-y += early_ioremap.h
diff --git a/arch/powerpc/include/asm/qspinlock.h b/arch/powerpc/include/asm/qspinlock.h
new file mode 100644
index 000000000000..c49e33e24edd
--- /dev/null
+++ b/arch/powerpc/include/asm/qspinlock.h
@@ -0,0 +1,25 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _ASM_POWERPC_QSPINLOCK_H
+#define _ASM_POWERPC_QSPINLOCK_H
+
+#include <asm-generic/qspinlock_types.h>
+
+#define _Q_PENDING_LOOPS (1 << 9) /* not tuned */
+
+#define smp_mb__after_spinlock() smp_mb()
+
+static __always_inline int queued_spin_is_locked(struct qspinlock *lock)
+{
+ /*
+ * This barrier was added to simple spinlocks by commit 51d7d5205d338,
+ * but it should now be possible to remove it, asm arm64 has done with
+ * commit c6f5d02b6a0f.
+ */
+ smp_mb();
+ return atomic_read(&lock->val);
+}
+#define queued_spin_is_locked queued_spin_is_locked
+
+#include <asm-generic/qspinlock.h>
+
+#endif /* _ASM_POWERPC_QSPINLOCK_H */
diff --git a/arch/powerpc/include/asm/spinlock.h b/arch/powerpc/include/asm/spinlock.h
index 21357fe05fe0..434615f1d761 100644
--- a/arch/powerpc/include/asm/spinlock.h
+++ b/arch/powerpc/include/asm/spinlock.h
@@ -3,7 +3,12 @@
#define __ASM_SPINLOCK_H
#ifdef __KERNEL__
+#ifdef CONFIG_PPC_QUEUED_SPINLOCKS
+#include <asm/qspinlock.h>
+#include <asm/qrwlock.h>
+#else
#include <asm/simple_spinlock.h>
+#endif
#endif /* __KERNEL__ */
#endif /* __ASM_SPINLOCK_H */
diff --git a/arch/powerpc/include/asm/spinlock_types.h b/arch/powerpc/include/asm/spinlock_types.h
index 3906f52dae65..c5d742f18021 100644
--- a/arch/powerpc/include/asm/spinlock_types.h
+++ b/arch/powerpc/include/asm/spinlock_types.h
@@ -6,6 +6,11 @@
# error "please don't include this file directly"
#endif
+#ifdef CONFIG_PPC_QUEUED_SPINLOCKS
+#include <asm-generic/qspinlock_types.h>
+#include <asm-generic/qrwlock_types.h>
+#else
#include <asm/simple_spinlock_types.h>
+#endif
#endif
diff --git a/arch/powerpc/lib/Makefile b/arch/powerpc/lib/Makefile
index 5e994cda8e40..d66a645503eb 100644
--- a/arch/powerpc/lib/Makefile
+++ b/arch/powerpc/lib/Makefile
@@ -41,7 +41,10 @@ obj-$(CONFIG_PPC_BOOK3S_64) += copyuser_power7.o copypage_power7.o \
obj64-y += copypage_64.o copyuser_64.o mem_64.o hweight_64.o \
memcpy_64.o memcpy_mcsafe_64.o
+ifndef CONFIG_PPC_QUEUED_SPINLOCKS
obj64-$(CONFIG_SMP) += locks.o
+endif
+
obj64-$(CONFIG_ALTIVEC) += vmx-helper.o
obj64-$(CONFIG_KPROBES_SANITY_TEST) += test_emulate_step.o \
test_emulate_step_exec_instr.o
diff --git a/include/asm-generic/qspinlock.h b/include/asm-generic/qspinlock.h
index fde943d180e0..fb0a814d4395 100644
--- a/include/asm-generic/qspinlock.h
+++ b/include/asm-generic/qspinlock.h
@@ -12,6 +12,7 @@
#include <asm-generic/qspinlock_types.h>
+#ifndef queued_spin_is_locked
/**
* queued_spin_is_locked - is the spinlock locked?
* @lock: Pointer to queued spinlock structure
@@ -25,6 +26,7 @@ static __always_inline int queued_spin_is_locked(struct qspinlock *lock)
*/
return atomic_read(&lock->val);
}
+#endif
/**
* queued_spin_value_unlocked - is the spinlock structure unlocked?
--
2.23.0
^ permalink raw reply related
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