* [PATCH v10 16/24] selftests/vm: fix an assertion in test_pkey_alloc_exhaust()
From: Ram Pai @ 2018-01-22 18:52 UTC (permalink / raw)
To: shuahkh, linux-kselftest
Cc: mpe, linuxppc-dev, linux-mm, x86, linux-arch, linux-doc,
linux-kernel, mingo, akpm, dave.hansen, benh, paulus, khandual,
aneesh.kumar, bsingharora, hbabu, mhocko, bauerman, ebiederm,
linuxram, arnd
In-Reply-To: <1516647137-11174-1-git-send-email-linuxram@us.ibm.com>
The maximum number of keys that can be allocated has to
take into consideration, that some keys are reserved by
the architecture for specific purpose. Hence cannot
be allocated.
Fix the assertion in test_pkey_alloc_exhaust()
Signed-off-by: Ram Pai <linuxram@us.ibm.com>
---
tools/testing/selftests/vm/pkey-helpers.h | 14 ++++++++++++++
tools/testing/selftests/vm/protection_keys.c | 9 ++++-----
2 files changed, 18 insertions(+), 5 deletions(-)
diff --git a/tools/testing/selftests/vm/pkey-helpers.h b/tools/testing/selftests/vm/pkey-helpers.h
index 3559527..9d06b4a 100644
--- a/tools/testing/selftests/vm/pkey-helpers.h
+++ b/tools/testing/selftests/vm/pkey-helpers.h
@@ -401,4 +401,18 @@ static inline int get_start_key(void)
#endif /* arch */
}
+static inline int arch_reserved_keys(void)
+{
+#if defined(__i386__) || defined(__x86_64__) /* arch */
+ return NR_RESERVED_PKEYS;
+#elif __powerpc64__ /* arch */
+ if (sysconf(_SC_PAGESIZE) == 4096)
+ return NR_RESERVED_PKEYS_4K;
+ else
+ return NR_RESERVED_PKEYS_64K;
+#else /* arch */
+ NOT SUPPORTED
+#endif /* arch */
+}
+
#endif /* _PKEYS_HELPER_H */
diff --git a/tools/testing/selftests/vm/protection_keys.c b/tools/testing/selftests/vm/protection_keys.c
index 65e6dd6..33d5839 100644
--- a/tools/testing/selftests/vm/protection_keys.c
+++ b/tools/testing/selftests/vm/protection_keys.c
@@ -1167,12 +1167,11 @@ void test_pkey_alloc_exhaust(int *ptr, u16 pkey)
pkey_assert(i < NR_PKEYS*2);
/*
- * There are 16 pkeys supported in hardware. One is taken
- * up for the default (0) and another can be taken up by
- * an execute-only mapping. Ensure that we can allocate
- * at least 14 (16-2).
+ * There are NR_PKEYS pkeys supported in hardware. arch_reserved_keys()
+ * are reserved. One can be taken up by an execute-only mapping.
+ * Ensure that we can allocate at least the remaining.
*/
- pkey_assert(i >= NR_PKEYS-2);
+ pkey_assert(i >= (NR_PKEYS-arch_reserved_keys()-1));
for (i = 0; i < nr_allocated_pkeys; i++) {
err = sys_pkey_free(allocated_pkeys[i]);
--
1.7.1
^ permalink raw reply related
* [PATCH v10 17/24] selftests/vm: associate key on a mapped page and detect access violation
From: Ram Pai @ 2018-01-22 18:52 UTC (permalink / raw)
To: shuahkh, linux-kselftest
Cc: mpe, linuxppc-dev, linux-mm, x86, linux-arch, linux-doc,
linux-kernel, mingo, akpm, dave.hansen, benh, paulus, khandual,
aneesh.kumar, bsingharora, hbabu, mhocko, bauerman, ebiederm,
linuxram, arnd
In-Reply-To: <1516647137-11174-1-git-send-email-linuxram@us.ibm.com>
detect access-violation on a page to which access-disabled
key is associated much after the page is mapped.
Signed-off-by: Ram Pai <linuxram@us.ibm.com>
---
tools/testing/selftests/vm/protection_keys.c | 19 +++++++++++++++++++
1 files changed, 19 insertions(+), 0 deletions(-)
diff --git a/tools/testing/selftests/vm/protection_keys.c b/tools/testing/selftests/vm/protection_keys.c
index 33d5839..1d3f596 100644
--- a/tools/testing/selftests/vm/protection_keys.c
+++ b/tools/testing/selftests/vm/protection_keys.c
@@ -1016,6 +1016,24 @@ void test_read_of_access_disabled_region(int *ptr, u16 pkey)
dprintf1("*ptr: %d\n", ptr_contents);
expected_pkey_fault(pkey);
}
+
+void test_read_of_access_disabled_region_with_page_already_mapped(int *ptr,
+ u16 pkey)
+{
+ int ptr_contents;
+
+ dprintf1("disabling access to PKEY[%02d], doing read @ %p\n",
+ pkey, ptr);
+ ptr_contents = read_ptr(ptr);
+ dprintf1("reading ptr before disabling the read : %d\n",
+ ptr_contents);
+ rdpkey_reg();
+ pkey_access_deny(pkey);
+ ptr_contents = read_ptr(ptr);
+ dprintf1("*ptr: %d\n", ptr_contents);
+ expected_pkey_fault(pkey);
+}
+
void test_write_of_write_disabled_region(int *ptr, u16 pkey)
{
dprintf1("disabling write access to PKEY[%02d], doing write\n", pkey);
@@ -1310,6 +1328,7 @@ void test_mprotect_pkey_on_unsupported_cpu(int *ptr, u16 pkey)
void (*pkey_tests[])(int *ptr, u16 pkey) = {
test_read_of_write_disabled_region,
test_read_of_access_disabled_region,
+ test_read_of_access_disabled_region_with_page_already_mapped,
test_write_of_write_disabled_region,
test_write_of_access_disabled_region,
test_kernel_write_of_access_disabled_region,
--
1.7.1
^ permalink raw reply related
* [PATCH v10 18/24] selftests/vm: associate key on a mapped page and detect write violation
From: Ram Pai @ 2018-01-22 18:52 UTC (permalink / raw)
To: shuahkh, linux-kselftest
Cc: mpe, linuxppc-dev, linux-mm, x86, linux-arch, linux-doc,
linux-kernel, mingo, akpm, dave.hansen, benh, paulus, khandual,
aneesh.kumar, bsingharora, hbabu, mhocko, bauerman, ebiederm,
linuxram, arnd
In-Reply-To: <1516647137-11174-1-git-send-email-linuxram@us.ibm.com>
detect write-violation on a page to which write-disabled
key is associated much after the page is mapped.
Signed-off-by: Ram Pai <linuxram@us.ibm.com>
---
tools/testing/selftests/vm/protection_keys.c | 12 ++++++++++++
1 files changed, 12 insertions(+), 0 deletions(-)
diff --git a/tools/testing/selftests/vm/protection_keys.c b/tools/testing/selftests/vm/protection_keys.c
index 1d3f596..f90a8b6 100644
--- a/tools/testing/selftests/vm/protection_keys.c
+++ b/tools/testing/selftests/vm/protection_keys.c
@@ -1034,6 +1034,17 @@ void test_read_of_access_disabled_region_with_page_already_mapped(int *ptr,
expected_pkey_fault(pkey);
}
+void test_write_of_write_disabled_region_with_page_already_mapped(int *ptr,
+ u16 pkey)
+{
+ *ptr = __LINE__;
+ dprintf1("disabling write access; after accessing the page, "
+ "to PKEY[%02d], doing write\n", pkey);
+ pkey_write_deny(pkey);
+ *ptr = __LINE__;
+ expected_pkey_fault(pkey);
+}
+
void test_write_of_write_disabled_region(int *ptr, u16 pkey)
{
dprintf1("disabling write access to PKEY[%02d], doing write\n", pkey);
@@ -1330,6 +1341,7 @@ void test_mprotect_pkey_on_unsupported_cpu(int *ptr, u16 pkey)
test_read_of_access_disabled_region,
test_read_of_access_disabled_region_with_page_already_mapped,
test_write_of_write_disabled_region,
+ test_write_of_write_disabled_region_with_page_already_mapped,
test_write_of_access_disabled_region,
test_kernel_write_of_access_disabled_region,
test_kernel_write_of_write_disabled_region,
--
1.7.1
^ permalink raw reply related
* [PATCH v10 19/24] selftests/vm: detect write violation on a mapped access-denied-key page
From: Ram Pai @ 2018-01-22 18:52 UTC (permalink / raw)
To: shuahkh, linux-kselftest
Cc: mpe, linuxppc-dev, linux-mm, x86, linux-arch, linux-doc,
linux-kernel, mingo, akpm, dave.hansen, benh, paulus, khandual,
aneesh.kumar, bsingharora, hbabu, mhocko, bauerman, ebiederm,
linuxram, arnd
In-Reply-To: <1516647137-11174-1-git-send-email-linuxram@us.ibm.com>
detect write-violation on a page to which access-disabled
key is associated much after the page is mapped.
Signed-off-by: Ram Pai <linuxram@us.ibm.com>
---
tools/testing/selftests/vm/protection_keys.c | 13 +++++++++++++
1 files changed, 13 insertions(+), 0 deletions(-)
diff --git a/tools/testing/selftests/vm/protection_keys.c b/tools/testing/selftests/vm/protection_keys.c
index f90a8b6..3e9e36d 100644
--- a/tools/testing/selftests/vm/protection_keys.c
+++ b/tools/testing/selftests/vm/protection_keys.c
@@ -1059,6 +1059,18 @@ void test_write_of_access_disabled_region(int *ptr, u16 pkey)
*ptr = __LINE__;
expected_pkey_fault(pkey);
}
+
+void test_write_of_access_disabled_region_with_page_already_mapped(int *ptr,
+ u16 pkey)
+{
+ *ptr = __LINE__;
+ dprintf1("disabling access; after accessing the page, "
+ " to PKEY[%02d], doing write\n", pkey);
+ pkey_access_deny(pkey);
+ *ptr = __LINE__;
+ expected_pkey_fault(pkey);
+}
+
void test_kernel_write_of_access_disabled_region(int *ptr, u16 pkey)
{
int ret;
@@ -1343,6 +1355,7 @@ void test_mprotect_pkey_on_unsupported_cpu(int *ptr, u16 pkey)
test_write_of_write_disabled_region,
test_write_of_write_disabled_region_with_page_already_mapped,
test_write_of_access_disabled_region,
+ test_write_of_access_disabled_region_with_page_already_mapped,
test_kernel_write_of_access_disabled_region,
test_kernel_write_of_write_disabled_region,
test_kernel_gup_of_access_disabled_region,
--
1.7.1
^ permalink raw reply related
* [PATCH v10 20/24] selftests/vm: testcases must restore pkey-permissions
From: Ram Pai @ 2018-01-22 18:52 UTC (permalink / raw)
To: shuahkh, linux-kselftest
Cc: mpe, linuxppc-dev, linux-mm, x86, linux-arch, linux-doc,
linux-kernel, mingo, akpm, dave.hansen, benh, paulus, khandual,
aneesh.kumar, bsingharora, hbabu, mhocko, bauerman, ebiederm,
linuxram, arnd
In-Reply-To: <1516647137-11174-1-git-send-email-linuxram@us.ibm.com>
Generally the signal handler restores the state of the pkey register
before returning. However there are times when the read/write operation
can legitamely fail without invoking the signal handler. Eg: A
sys_read() operaton to a write-protected page should be disallowed. In
such a case the state of the pkey register is not restored to its
original state. The test case is responsible for restoring the key
register state to its original value.
Signed-off-by: Ram Pai <linuxram@us.ibm.com>
---
tools/testing/selftests/vm/protection_keys.c | 5 +++++
1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/tools/testing/selftests/vm/protection_keys.c b/tools/testing/selftests/vm/protection_keys.c
index 3e9e36d..5783587 100644
--- a/tools/testing/selftests/vm/protection_keys.c
+++ b/tools/testing/selftests/vm/protection_keys.c
@@ -1003,6 +1003,7 @@ void test_read_of_write_disabled_region(int *ptr, u16 pkey)
ptr_contents = read_ptr(ptr);
dprintf1("*ptr: %d\n", ptr_contents);
dprintf1("\n");
+ pkey_write_allow(pkey);
}
void test_read_of_access_disabled_region(int *ptr, u16 pkey)
{
@@ -1082,6 +1083,7 @@ void test_kernel_write_of_access_disabled_region(int *ptr, u16 pkey)
ret = read(test_fd, ptr, 1);
dprintf1("read ret: %d\n", ret);
pkey_assert(ret);
+ pkey_access_allow(pkey);
}
void test_kernel_write_of_write_disabled_region(int *ptr, u16 pkey)
{
@@ -1094,6 +1096,7 @@ void test_kernel_write_of_write_disabled_region(int *ptr, u16 pkey)
if (ret < 0 && (DEBUG_LEVEL > 0))
perror("verbose read result (OK for this to be bad)");
pkey_assert(ret);
+ pkey_write_allow(pkey);
}
void test_kernel_gup_of_access_disabled_region(int *ptr, u16 pkey)
@@ -1113,6 +1116,7 @@ void test_kernel_gup_of_access_disabled_region(int *ptr, u16 pkey)
vmsplice_ret = vmsplice(pipe_fds[1], &iov, 1, SPLICE_F_GIFT);
dprintf1("vmsplice() ret: %d\n", vmsplice_ret);
pkey_assert(vmsplice_ret == -1);
+ pkey_access_allow(pkey);
close(pipe_fds[0]);
close(pipe_fds[1]);
@@ -1133,6 +1137,7 @@ void test_kernel_gup_write_to_write_disabled_region(int *ptr, u16 pkey)
if (DEBUG_LEVEL > 0)
perror("futex");
dprintf1("futex() ret: %d\n", futex_ret);
+ pkey_write_allow(pkey);
}
/* Assumes that all pkeys other than 'pkey' are unallocated */
--
1.7.1
^ permalink raw reply related
* [PATCH v10 21/24] selftests/vm: sub-page allocator
From: Ram Pai @ 2018-01-22 18:52 UTC (permalink / raw)
To: shuahkh, linux-kselftest
Cc: mpe, linuxppc-dev, linux-mm, x86, linux-arch, linux-doc,
linux-kernel, mingo, akpm, dave.hansen, benh, paulus, khandual,
aneesh.kumar, bsingharora, hbabu, mhocko, bauerman, ebiederm,
linuxram, arnd
In-Reply-To: <1516647137-11174-1-git-send-email-linuxram@us.ibm.com>
introduce a new allocator that allocates 4k hardware-pages to back
64k linux-page. This allocator is only applicable on powerpc.
Signed-off-by: Ram Pai <linuxram@us.ibm.com>
---
tools/testing/selftests/vm/protection_keys.c | 30 ++++++++++++++++++++++++++
1 files changed, 30 insertions(+), 0 deletions(-)
diff --git a/tools/testing/selftests/vm/protection_keys.c b/tools/testing/selftests/vm/protection_keys.c
index 5783587..ae71dad 100644
--- a/tools/testing/selftests/vm/protection_keys.c
+++ b/tools/testing/selftests/vm/protection_keys.c
@@ -766,6 +766,35 @@ void free_pkey_malloc(void *ptr)
return ptr;
}
+void *malloc_pkey_with_mprotect_subpage(long size, int prot, u16 pkey)
+{
+#ifdef __powerpc64__
+ void *ptr;
+ int ret;
+
+ dprintf1("doing %s(size=%ld, prot=0x%x, pkey=%d)\n", __func__,
+ size, prot, pkey);
+ pkey_assert(pkey < NR_PKEYS);
+ ptr = mmap(NULL, size, prot, MAP_ANONYMOUS|MAP_PRIVATE, -1, 0);
+ pkey_assert(ptr != (void *)-1);
+
+ ret = syscall(__NR_subpage_prot, ptr, size, NULL);
+ if (ret) {
+ perror("subpage_perm");
+ return PTR_ERR_ENOTSUP;
+ }
+
+ ret = mprotect_pkey((void *)ptr, PAGE_SIZE, prot, pkey);
+ pkey_assert(!ret);
+ record_pkey_malloc(ptr, size);
+
+ dprintf1("%s() for pkey %d @ %p\n", __func__, pkey, ptr);
+ return ptr;
+#else /* __powerpc64__ */
+ return PTR_ERR_ENOTSUP;
+#endif /* __powerpc64__ */
+}
+
void *malloc_pkey_anon_huge(long size, int prot, u16 pkey)
{
int ret;
@@ -888,6 +917,7 @@ void setup_hugetlbfs(void)
void *(*pkey_malloc[])(long size, int prot, u16 pkey) = {
malloc_pkey_with_mprotect,
+ malloc_pkey_with_mprotect_subpage,
malloc_pkey_anon_huge,
malloc_pkey_hugetlb
/* can not do direct with the pkey_mprotect() API:
--
1.7.1
^ permalink raw reply related
* [PATCH v10 22/24] selftests/vm: Fix deadlock in protection_keys.c
From: Ram Pai @ 2018-01-22 18:52 UTC (permalink / raw)
To: shuahkh, linux-kselftest
Cc: mpe, linuxppc-dev, linux-mm, x86, linux-arch, linux-doc,
linux-kernel, mingo, akpm, dave.hansen, benh, paulus, khandual,
aneesh.kumar, bsingharora, hbabu, mhocko, bauerman, ebiederm,
linuxram, arnd
In-Reply-To: <1516647137-11174-1-git-send-email-linuxram@us.ibm.com>
From: Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com>
The sig_chld() handler calls dprintf2() taking care of setting
dprint_in_signal so that sigsafe_printf() won't call printf().
Unfortunately, this precaution is is negated by dprintf_level(), which
has a call to fflush().
This function acquires a lock, which means that if the signal interrupts an
ongoing fflush() the process will deadlock. At least on powerpc this is
easy to trigger, resulting in the following backtrace when attaching to the
frozen process:
(gdb) bt
#0 0x00007fff9f96c7d8 in __lll_lock_wait_private () from /lib64/power8/libc.so.6
#1 0x00007fff9f8cba4c in _IO_flush_all_lockp () from /lib64/power8/libc.so.6
#2 0x00007fff9f8cbd1c in __GI__IO_flush_all () from /lib64/power8/libc.so.6
#3 0x00007fff9f8b7424 in fflush () from /lib64/power8/libc.so.6
#4 0x00000000100504f8 in sig_chld (x=17) at protection_keys.c:283
#5 <signal handler called>
#6 0x00007fff9f8cb8ac in _IO_flush_all_lockp () from /lib64/power8/libc.so.6
#7 0x00007fff9f8cbd1c in __GI__IO_flush_all () from /lib64/power8/libc.so.6
#8 0x00007fff9f8b7424 in fflush () from /lib64/power8/libc.so.6
#9 0x0000000010050b50 in pkey_get (pkey=7, flags=0) at protection_keys.c:379
#10 0x0000000010050dc0 in pkey_disable_set (pkey=7, flags=2) at protection_keys.c:423
#11 0x0000000010051414 in pkey_write_deny (pkey=7) at protection_keys.c:486
#12 0x00000000100556bc in test_ptrace_of_child (ptr=0x7fff9f7f0000, pkey=7) at protection_keys.c:1288
#13 0x0000000010055f60 in run_tests_once () at protection_keys.c:1414
#14 0x00000000100561a4 in main () at protection_keys.c:1459
The fix is to refrain from calling fflush() when inside a signal handler.
The output may not be as pretty but at least the testcase will be able to
move on.
Signed-off-by: Ram Pai <linuxram@us.ibm.com>
Signed-off-by: Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com>
tools/testing/selftests/vm/pkey-helpers.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
---
tools/testing/selftests/vm/pkey-helpers.h | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/tools/testing/selftests/vm/pkey-helpers.h b/tools/testing/selftests/vm/pkey-helpers.h
index 9d06b4a..965cfcd 100644
--- a/tools/testing/selftests/vm/pkey-helpers.h
+++ b/tools/testing/selftests/vm/pkey-helpers.h
@@ -131,7 +131,8 @@ static inline void sigsafe_printf(const char *format, ...)
#define dprintf_level(level, args...) do { \
if (level <= DEBUG_LEVEL) \
sigsafe_printf(args); \
- fflush(NULL); \
+ if (!dprint_in_signal) \
+ fflush(NULL); \
} while (0)
#define dprintf0(args...) dprintf_level(0, args)
#define dprintf1(args...) dprintf_level(1, args)
--
1.7.1
^ permalink raw reply related
* [PATCH v10 23/24] selftests/powerpc: Add ptrace tests for Protection Key register
From: Ram Pai @ 2018-01-22 18:52 UTC (permalink / raw)
To: shuahkh, linux-kselftest
Cc: mpe, linuxppc-dev, linux-mm, x86, linux-arch, linux-doc,
linux-kernel, mingo, akpm, dave.hansen, benh, paulus, khandual,
aneesh.kumar, bsingharora, hbabu, mhocko, bauerman, ebiederm,
linuxram, arnd
In-Reply-To: <1516647137-11174-1-git-send-email-linuxram@us.ibm.com>
From: Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com>
This test exercises read and write access to the AMR.
Signed-off-by: Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com>
---
tools/testing/selftests/powerpc/include/reg.h | 1 +
tools/testing/selftests/powerpc/ptrace/Makefile | 5 +-
.../testing/selftests/powerpc/ptrace/ptrace-pkey.c | 443 ++++++++++++++++++++
3 files changed, 448 insertions(+), 1 deletions(-)
create mode 100644 tools/testing/selftests/powerpc/ptrace/ptrace-pkey.c
diff --git a/tools/testing/selftests/powerpc/include/reg.h b/tools/testing/selftests/powerpc/include/reg.h
index 4afdebc..7f348c0 100644
--- a/tools/testing/selftests/powerpc/include/reg.h
+++ b/tools/testing/selftests/powerpc/include/reg.h
@@ -54,6 +54,7 @@
#define SPRN_DSCR_PRIV 0x11 /* Privilege State DSCR */
#define SPRN_DSCR 0x03 /* Data Stream Control Register */
#define SPRN_PPR 896 /* Program Priority Register */
+#define SPRN_AMR 13 /* Authority Mask Register - problem state */
/* TEXASR register bits */
#define TEXASR_FC 0xFE00000000000000
diff --git a/tools/testing/selftests/powerpc/ptrace/Makefile b/tools/testing/selftests/powerpc/ptrace/Makefile
index 4803052..fd896b2 100644
--- a/tools/testing/selftests/powerpc/ptrace/Makefile
+++ b/tools/testing/selftests/powerpc/ptrace/Makefile
@@ -1,7 +1,7 @@
# SPDX-License-Identifier: GPL-2.0
TEST_PROGS := ptrace-gpr ptrace-tm-gpr ptrace-tm-spd-gpr \
ptrace-tar ptrace-tm-tar ptrace-tm-spd-tar ptrace-vsx ptrace-tm-vsx \
- ptrace-tm-spd-vsx ptrace-tm-spr
+ ptrace-tm-spd-vsx ptrace-tm-spr ptrace-pkey
include ../../lib.mk
@@ -9,6 +9,9 @@ all: $(TEST_PROGS)
CFLAGS += -m64 -I../../../../../usr/include -I../tm -mhtm -fno-pie
+ptrace-pkey: ../harness.c ../utils.c ../lib/reg.S ptrace.h ptrace-pkey.c
+ $(LINK.c) $^ $(LDLIBS) -pthread -o $@
+
$(TEST_PROGS): ../harness.c ../utils.c ../lib/reg.S ptrace.h
clean:
diff --git a/tools/testing/selftests/powerpc/ptrace/ptrace-pkey.c b/tools/testing/selftests/powerpc/ptrace/ptrace-pkey.c
new file mode 100644
index 0000000..2e5b676
--- /dev/null
+++ b/tools/testing/selftests/powerpc/ptrace/ptrace-pkey.c
@@ -0,0 +1,443 @@
+/*
+ * Ptrace test for Memory Protection Key registers
+ *
+ * Copyright (C) 2015 Anshuman Khandual, IBM Corporation.
+ * Copyright (C) 2017 IBM Corporation.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ */
+#include <semaphore.h>
+#include "ptrace.h"
+
+#ifndef __NR_pkey_alloc
+#define __NR_pkey_alloc 384
+#endif
+
+#ifndef __NR_pkey_free
+#define __NR_pkey_free 385
+#endif
+
+#ifndef NT_PPC_PKEY
+#define NT_PPC_PKEY 0x110
+#endif
+
+#ifndef PKEY_DISABLE_EXECUTE
+#define PKEY_DISABLE_EXECUTE 0x4
+#endif
+
+#define AMR_BITS_PER_PKEY 2
+#define PKEY_REG_BITS (sizeof(u64) * 8)
+#define pkeyshift(pkey) (PKEY_REG_BITS - ((pkey + 1) * AMR_BITS_PER_PKEY))
+
+static const char user_read[] = "[User Read (Running)]";
+static const char user_write[] = "[User Write (Running)]";
+static const char ptrace_read_running[] = "[Ptrace Read (Running)]";
+static const char ptrace_write_running[] = "[Ptrace Write (Running)]";
+
+/* Information shared between the parent and the child. */
+struct shared_info {
+ /* AMR value the parent expects to read from the child. */
+ unsigned long amr1;
+
+ /* AMR value the parent is expected to write to the child. */
+ unsigned long amr2;
+
+ /* AMR value that ptrace should refuse to write to the child. */
+ unsigned long amr3;
+
+ /* IAMR value the parent expects to read from the child. */
+ unsigned long expected_iamr;
+
+ /* UAMOR value the parent expects to read from the child. */
+ unsigned long expected_uamor;
+
+ /*
+ * IAMR and UAMOR values that ptrace should refuse to write to the child
+ * (even though they're valid ones) because userspace doesn't have
+ * access to those registers.
+ */
+ unsigned long new_iamr;
+ unsigned long new_uamor;
+
+ /* The parent waits on this semaphore. */
+ sem_t sem_parent;
+
+ /* If true, the child should give up as well. */
+ bool parent_gave_up;
+
+ /* The child waits on this semaphore. */
+ sem_t sem_child;
+
+ /* If true, the parent should give up as well. */
+ bool child_gave_up;
+};
+
+#define CHILD_FAIL_IF(x, info) \
+ do { \
+ if ((x)) { \
+ fprintf(stderr, \
+ "[FAIL] Test FAILED on line %d\n", __LINE__); \
+ (info)->child_gave_up = true; \
+ prod_parent(info); \
+ return 1; \
+ } \
+ } while (0)
+
+#define PARENT_FAIL_IF(x, info) \
+ do { \
+ if ((x)) { \
+ fprintf(stderr, \
+ "[FAIL] Test FAILED on line %d\n", __LINE__); \
+ (info)->parent_gave_up = true; \
+ prod_child(info); \
+ return 1; \
+ } \
+ } while (0)
+
+static int wait_child(struct shared_info *info)
+{
+ int ret;
+
+ /* Wait until the child prods us. */
+ ret = sem_wait(&info->sem_parent);
+ if (ret) {
+ perror("Error waiting for child");
+ return TEST_FAIL;
+ }
+
+ return info->child_gave_up ? TEST_FAIL : TEST_PASS;
+}
+
+static int prod_child(struct shared_info *info)
+{
+ int ret;
+
+ /* Unblock the child now. */
+ ret = sem_post(&info->sem_child);
+ if (ret) {
+ perror("Error prodding child");
+ return TEST_FAIL;
+ }
+
+ return TEST_PASS;
+}
+
+static int wait_parent(struct shared_info *info)
+{
+ int ret;
+
+ /* Wait until the parent prods us. */
+ ret = sem_wait(&info->sem_child);
+ if (ret) {
+ perror("Error waiting for parent");
+ return TEST_FAIL;
+ }
+
+ return info->parent_gave_up ? TEST_FAIL : TEST_PASS;
+}
+
+static int prod_parent(struct shared_info *info)
+{
+ int ret;
+
+ /* Unblock the parent now. */
+ ret = sem_post(&info->sem_parent);
+ if (ret) {
+ perror("Error prodding parent");
+ return TEST_FAIL;
+ }
+
+ return TEST_PASS;
+}
+
+static int sys_pkey_alloc(unsigned long flags, unsigned long init_access_rights)
+{
+ return syscall(__NR_pkey_alloc, flags, init_access_rights);
+}
+
+static int sys_pkey_free(int pkey)
+{
+ return syscall(__NR_pkey_free, pkey);
+}
+
+static int ptrace_read_regs(pid_t child, unsigned long regs[], int n)
+{
+ struct iovec iov;
+ long ret;
+
+ FAIL_IF(start_trace(child));
+
+ iov.iov_base = regs;
+ iov.iov_len = n * sizeof(unsigned long);
+
+ ret = ptrace(PTRACE_GETREGSET, child, NT_PPC_PKEY, &iov);
+ FAIL_IF(ret != 0);
+
+ FAIL_IF(stop_trace(child));
+
+ return TEST_PASS;
+}
+
+static long ptrace_write_regs(pid_t child, unsigned long regs[], int n)
+{
+ struct iovec iov;
+ long ret;
+
+ FAIL_IF(start_trace(child));
+
+ iov.iov_base = regs;
+ iov.iov_len = n * sizeof(unsigned long);
+
+ ret = ptrace(PTRACE_SETREGSET, child, NT_PPC_PKEY, &iov);
+
+ FAIL_IF(stop_trace(child));
+
+ return ret;
+}
+
+static int child(struct shared_info *info)
+{
+ unsigned long reg;
+ bool disable_execute = true;
+ int pkey1, pkey2, pkey3;
+ int ret;
+
+ /* Get some pkeys so that we can change their bits in the AMR. */
+ pkey1 = sys_pkey_alloc(0, PKEY_DISABLE_EXECUTE);
+ if (pkey1 < 0) {
+ pkey1 = sys_pkey_alloc(0, 0);
+ CHILD_FAIL_IF(pkey1 < 0, info);
+
+ disable_execute = false;
+ }
+
+ pkey2 = sys_pkey_alloc(0, 0);
+ CHILD_FAIL_IF(pkey2 < 0, info);
+
+ pkey3 = sys_pkey_alloc(0, 0);
+ CHILD_FAIL_IF(pkey3 < 0, info);
+
+ info->amr1 = 3ul << pkeyshift(pkey1);
+ info->amr2 = 3ul << pkeyshift(pkey2);
+ info->amr3 = info->amr2 | 3ul << pkeyshift(pkey3);
+
+ if (disable_execute)
+ info->expected_iamr = 1ul << pkeyshift(pkey1);
+ else
+ info->expected_iamr = 0;
+
+ info->expected_uamor = 3ul << pkeyshift(pkey1) |
+ 3ul << pkeyshift(pkey2);
+ info->new_iamr = 1ul << pkeyshift(pkey1) | 1ul << pkeyshift(pkey2);
+ info->new_uamor = 3ul << pkeyshift(pkey1);
+
+ /*
+ * We won't use pkey3. We just want a plausible but invalid key to test
+ * whether ptrace will let us write to AMR bits we are not supposed to.
+ *
+ * This also tests whether the kernel restores the UAMOR permissions
+ * after a key is freed.
+ */
+ sys_pkey_free(pkey3);
+
+ printf("%-30s AMR: %016lx pkey1: %d pkey2: %d pkey3: %d\n",
+ user_write, info->amr1, pkey1, pkey2, pkey3);
+
+ mtspr(SPRN_AMR, info->amr1);
+
+ /* Wait for parent to read our AMR value and write a new one. */
+ ret = prod_parent(info);
+ CHILD_FAIL_IF(ret, info);
+
+ ret = wait_parent(info);
+ if (ret)
+ return ret;
+
+ reg = mfspr(SPRN_AMR);
+
+ printf("%-30s AMR: %016lx\n", user_read, reg);
+
+ CHILD_FAIL_IF(reg != info->amr2, info);
+
+ /*
+ * Wait for parent to try to write an invalid AMR value.
+ */
+ ret = prod_parent(info);
+ CHILD_FAIL_IF(ret, info);
+
+ ret = wait_parent(info);
+ if (ret)
+ return ret;
+
+ reg = mfspr(SPRN_AMR);
+
+ printf("%-30s AMR: %016lx\n", user_read, reg);
+
+ CHILD_FAIL_IF(reg != info->amr2, info);
+
+ /*
+ * Wait for parent to try to write an IAMR and a UAMOR value. We can't
+ * verify them, but we can verify that the AMR didn't change.
+ */
+ ret = prod_parent(info);
+ CHILD_FAIL_IF(ret, info);
+
+ ret = wait_parent(info);
+ if (ret)
+ return ret;
+
+ reg = mfspr(SPRN_AMR);
+
+ printf("%-30s AMR: %016lx\n", user_read, reg);
+
+ CHILD_FAIL_IF(reg != info->amr2, info);
+
+ /* Now let parent now that we are finished. */
+
+ ret = prod_parent(info);
+ CHILD_FAIL_IF(ret, info);
+
+ return TEST_PASS;
+}
+
+static int parent(struct shared_info *info, pid_t pid)
+{
+ unsigned long regs[4];
+ int ret, status;
+
+ ret = wait_child(info);
+ if (ret)
+ return ret;
+
+ /* Verify that we can read the pkey registers from the child. */
+ ret = ptrace_read_regs(pid, regs, 3);
+ PARENT_FAIL_IF(ret, info);
+
+ printf("%-30s AMR: %016lx IAMR: %016lx UAMOR: %016lx\n",
+ ptrace_read_running, regs[0], regs[1], regs[2]);
+
+ PARENT_FAIL_IF(regs[0] != info->amr1, info);
+ PARENT_FAIL_IF(regs[1] != info->expected_iamr, info);
+ PARENT_FAIL_IF(regs[2] != info->expected_uamor, info);
+
+ /* Write valid AMR value in child. */
+ ret = ptrace_write_regs(pid, &info->amr2, 1);
+ PARENT_FAIL_IF(ret, info);
+
+ printf("%-30s AMR: %016lx\n", ptrace_write_running, info->amr2);
+
+ /* Wake up child so that it can verify it changed. */
+ ret = prod_child(info);
+ PARENT_FAIL_IF(ret, info);
+
+ ret = wait_child(info);
+ if (ret)
+ return ret;
+
+ /* Write invalid AMR value in child. */
+ ret = ptrace_write_regs(pid, &info->amr3, 1);
+ PARENT_FAIL_IF(ret, info);
+
+ printf("%-30s AMR: %016lx\n", ptrace_write_running, info->amr3);
+
+ /* Wake up child so that it can verify it didn't change. */
+ ret = prod_child(info);
+ PARENT_FAIL_IF(ret, info);
+
+ ret = wait_child(info);
+ if (ret)
+ return ret;
+
+ /* Try to write to IAMR. */
+ regs[0] = info->amr1;
+ regs[1] = info->new_iamr;
+ ret = ptrace_write_regs(pid, regs, 2);
+ PARENT_FAIL_IF(!ret, info);
+
+ printf("%-30s AMR: %016lx IAMR: %016lx\n",
+ ptrace_write_running, regs[0], regs[1]);
+
+ /* Try to write to IAMR and UAMOR. */
+ regs[2] = info->new_uamor;
+ ret = ptrace_write_regs(pid, regs, 3);
+ PARENT_FAIL_IF(!ret, info);
+
+ printf("%-30s AMR: %016lx IAMR: %016lx UAMOR: %016lx\n",
+ ptrace_write_running, regs[0], regs[1], regs[2]);
+
+ /* Verify that all registers still have their expected values. */
+ ret = ptrace_read_regs(pid, regs, 3);
+ PARENT_FAIL_IF(ret, info);
+
+ printf("%-30s AMR: %016lx IAMR: %016lx UAMOR: %016lx\n",
+ ptrace_read_running, regs[0], regs[1], regs[2]);
+
+ PARENT_FAIL_IF(regs[0] != info->amr2, info);
+ PARENT_FAIL_IF(regs[1] != info->expected_iamr, info);
+ PARENT_FAIL_IF(regs[2] != info->expected_uamor, info);
+
+ /* Wake up child so that it can verify AMR didn't change and wrap up. */
+ ret = prod_child(info);
+ PARENT_FAIL_IF(ret, info);
+
+ ret = wait(&status);
+ if (ret != pid) {
+ printf("Child's exit status not captured\n");
+ ret = TEST_PASS;
+ } else if (!WIFEXITED(status)) {
+ printf("Child exited abnormally\n");
+ ret = TEST_FAIL;
+ } else
+ ret = WEXITSTATUS(status) ? TEST_FAIL : TEST_PASS;
+
+ return ret;
+}
+
+static int ptrace_pkey(void)
+{
+ struct shared_info *info;
+ int shm_id;
+ int ret;
+ pid_t pid;
+
+ shm_id = shmget(IPC_PRIVATE, sizeof(*info), 0777 | IPC_CREAT);
+ info = shmat(shm_id, NULL, 0);
+
+ ret = sem_init(&info->sem_parent, 1, 0);
+ if (ret) {
+ perror("Semaphore initialization failed");
+ return TEST_FAIL;
+ }
+ ret = sem_init(&info->sem_child, 1, 0);
+ if (ret) {
+ perror("Semaphore initialization failed");
+ return TEST_FAIL;
+ }
+
+ pid = fork();
+ if (pid < 0) {
+ perror("fork() failed");
+ ret = TEST_FAIL;
+ } else if (pid == 0)
+ ret = child(info);
+ else
+ ret = parent(info, pid);
+
+ shmdt(info);
+
+ if (pid) {
+ sem_destroy(&info->sem_parent);
+ sem_destroy(&info->sem_child);
+ shmctl(shm_id, IPC_RMID, NULL);
+ }
+
+ return ret;
+}
+
+int main(int argc, char *argv[])
+{
+ return test_harness(ptrace_pkey, "ptrace_pkey");
+}
--
1.7.1
^ permalink raw reply related
* [PATCH v10 24/24] selftests/powerpc: Add core file test for Protection Key register
From: Ram Pai @ 2018-01-22 18:52 UTC (permalink / raw)
To: shuahkh, linux-kselftest
Cc: mpe, linuxppc-dev, linux-mm, x86, linux-arch, linux-doc,
linux-kernel, mingo, akpm, dave.hansen, benh, paulus, khandual,
aneesh.kumar, bsingharora, hbabu, mhocko, bauerman, ebiederm,
linuxram, arnd
In-Reply-To: <1516647137-11174-1-git-send-email-linuxram@us.ibm.com>
From: Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com>
This test verifies that the AMR is being written to a
process' core file.
Signed-off-by: Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com>
---
tools/testing/selftests/powerpc/ptrace/Makefile | 2 +-
tools/testing/selftests/powerpc/ptrace/core-pkey.c | 438 ++++++++++++++++++++
2 files changed, 439 insertions(+), 1 deletions(-)
create mode 100644 tools/testing/selftests/powerpc/ptrace/core-pkey.c
diff --git a/tools/testing/selftests/powerpc/ptrace/Makefile b/tools/testing/selftests/powerpc/ptrace/Makefile
index fd896b2..ca25fda 100644
--- a/tools/testing/selftests/powerpc/ptrace/Makefile
+++ b/tools/testing/selftests/powerpc/ptrace/Makefile
@@ -1,7 +1,7 @@
# SPDX-License-Identifier: GPL-2.0
TEST_PROGS := ptrace-gpr ptrace-tm-gpr ptrace-tm-spd-gpr \
ptrace-tar ptrace-tm-tar ptrace-tm-spd-tar ptrace-vsx ptrace-tm-vsx \
- ptrace-tm-spd-vsx ptrace-tm-spr ptrace-pkey
+ ptrace-tm-spd-vsx ptrace-tm-spr ptrace-pkey core-pkey
include ../../lib.mk
diff --git a/tools/testing/selftests/powerpc/ptrace/core-pkey.c b/tools/testing/selftests/powerpc/ptrace/core-pkey.c
new file mode 100644
index 0000000..2328f8c
--- /dev/null
+++ b/tools/testing/selftests/powerpc/ptrace/core-pkey.c
@@ -0,0 +1,438 @@
+/*
+ * Ptrace test for Memory Protection Key registers
+ *
+ * Copyright (C) 2015 Anshuman Khandual, IBM Corporation.
+ * Copyright (C) 2017 IBM Corporation.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ */
+#include <limits.h>
+#include <semaphore.h>
+#include <linux/kernel.h>
+#include <sys/mman.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <sys/time.h>
+#include <sys/resource.h>
+#include <fcntl.h>
+#include <unistd.h>
+#include "ptrace.h"
+
+#ifndef __NR_pkey_alloc
+#define __NR_pkey_alloc 384
+#endif
+
+#ifndef __NR_pkey_free
+#define __NR_pkey_free 385
+#endif
+
+#ifndef NT_PPC_PKEY
+#define NT_PPC_PKEY 0x110
+#endif
+
+#ifndef PKEY_DISABLE_EXECUTE
+#define PKEY_DISABLE_EXECUTE 0x4
+#endif
+
+#define AMR_BITS_PER_PKEY 2
+#define PKEY_REG_BITS (sizeof(u64) * 8)
+#define pkeyshift(pkey) (PKEY_REG_BITS - ((pkey + 1) * AMR_BITS_PER_PKEY))
+
+#define CORE_FILE_LIMIT (5 * 1024 * 1024) /* 5 MB should be enough */
+
+static const char core_pattern_file[] = "/proc/sys/kernel/core_pattern";
+
+static const char user_write[] = "[User Write (Running)]";
+static const char core_read_running[] = "[Core Read (Running)]";
+
+/* Information shared between the parent and the child. */
+struct shared_info {
+ /* AMR value the parent expects to read in the core file. */
+ unsigned long amr;
+
+ /* IAMR value the parent expects to read from the child. */
+ unsigned long iamr;
+
+ /* UAMOR value the parent expects to read from the child. */
+ unsigned long uamor;
+
+ /* When the child crashed. */
+ time_t core_time;
+};
+
+static int sys_pkey_alloc(unsigned long flags, unsigned long init_access_rights)
+{
+ return syscall(__NR_pkey_alloc, flags, init_access_rights);
+}
+
+static int sys_pkey_free(int pkey)
+{
+ return syscall(__NR_pkey_free, pkey);
+}
+
+static int increase_core_file_limit(void)
+{
+ struct rlimit rlim;
+ int ret;
+
+ ret = getrlimit(RLIMIT_CORE, &rlim);
+ FAIL_IF(ret);
+
+ if (rlim.rlim_cur != RLIM_INFINITY && rlim.rlim_cur < CORE_FILE_LIMIT) {
+ rlim.rlim_cur = CORE_FILE_LIMIT;
+
+ if (rlim.rlim_max != RLIM_INFINITY &&
+ rlim.rlim_max < CORE_FILE_LIMIT)
+ rlim.rlim_max = CORE_FILE_LIMIT;
+
+ ret = setrlimit(RLIMIT_CORE, &rlim);
+ FAIL_IF(ret);
+ }
+
+ ret = getrlimit(RLIMIT_FSIZE, &rlim);
+ FAIL_IF(ret);
+
+ if (rlim.rlim_cur != RLIM_INFINITY && rlim.rlim_cur < CORE_FILE_LIMIT) {
+ rlim.rlim_cur = CORE_FILE_LIMIT;
+
+ if (rlim.rlim_max != RLIM_INFINITY &&
+ rlim.rlim_max < CORE_FILE_LIMIT)
+ rlim.rlim_max = CORE_FILE_LIMIT;
+
+ ret = setrlimit(RLIMIT_FSIZE, &rlim);
+ FAIL_IF(ret);
+ }
+
+ return TEST_PASS;
+}
+
+static int child(struct shared_info *info)
+{
+ bool disable_execute = true;
+ int pkey1, pkey2, pkey3;
+ int *ptr, ret;
+
+ ret = increase_core_file_limit();
+ FAIL_IF(ret);
+
+ /* Get some pkeys so that we can change their bits in the AMR. */
+ pkey1 = sys_pkey_alloc(0, PKEY_DISABLE_EXECUTE);
+ if (pkey1 < 0) {
+ pkey1 = sys_pkey_alloc(0, 0);
+ FAIL_IF(pkey1 < 0);
+
+ disable_execute = false;
+ }
+
+ pkey2 = sys_pkey_alloc(0, 0);
+ FAIL_IF(pkey2 < 0);
+
+ pkey3 = sys_pkey_alloc(0, 0);
+ FAIL_IF(pkey3 < 0);
+
+ info->amr = 3ul << pkeyshift(pkey1) | 2ul << pkeyshift(pkey2);
+
+ if (disable_execute)
+ info->iamr = 1ul << pkeyshift(pkey1);
+ else
+ info->iamr = 0;
+
+ info->uamor = 3ul << pkeyshift(pkey1) | 3ul << pkeyshift(pkey2);
+
+ printf("%-30s AMR: %016lx pkey1: %d pkey2: %d pkey3: %d\n",
+ user_write, info->amr, pkey1, pkey2, pkey3);
+
+ mtspr(SPRN_AMR, info->amr);
+
+ /*
+ * We won't use pkey3. This tests whether the kernel restores the UAMOR
+ * permissions after a key is freed.
+ */
+ sys_pkey_free(pkey3);
+
+ info->core_time = time(NULL);
+
+ /* Crash. */
+ ptr = 0;
+ *ptr = 1;
+
+ /* Shouldn't get here. */
+ FAIL_IF(true);
+
+ return TEST_FAIL;
+}
+
+/* Return file size if filename exists and pass sanity check, or zero if not. */
+static off_t try_core_file(const char *filename, struct shared_info *info,
+ pid_t pid)
+{
+ struct stat buf;
+ int ret;
+
+ ret = stat(filename, &buf);
+ if (ret == -1)
+ return TEST_FAIL;
+
+ /* Make sure we're not using a stale core file. */
+ return buf.st_mtime >= info->core_time ? buf.st_size : TEST_FAIL;
+}
+
+static Elf64_Nhdr *next_note(Elf64_Nhdr *nhdr)
+{
+ return (void *) nhdr + sizeof(*nhdr) +
+ __ALIGN_KERNEL(nhdr->n_namesz, 4) +
+ __ALIGN_KERNEL(nhdr->n_descsz, 4);
+}
+
+static int check_core_file(struct shared_info *info, Elf64_Ehdr *ehdr,
+ off_t core_size)
+{
+ unsigned long *regs;
+ Elf64_Phdr *phdr;
+ Elf64_Nhdr *nhdr;
+ size_t phdr_size;
+ void *p = ehdr, *note;
+ int ret;
+
+ ret = memcmp(ehdr->e_ident, ELFMAG, SELFMAG);
+ FAIL_IF(ret);
+
+ FAIL_IF(ehdr->e_type != ET_CORE);
+ FAIL_IF(ehdr->e_machine != EM_PPC64);
+ FAIL_IF(ehdr->e_phoff == 0 || ehdr->e_phnum == 0);
+
+ /*
+ * e_phnum is at most 65535 so calculating the size of the
+ * program header cannot overflow.
+ */
+ phdr_size = sizeof(*phdr) * ehdr->e_phnum;
+
+ /* Sanity check the program header table location. */
+ FAIL_IF(ehdr->e_phoff + phdr_size < ehdr->e_phoff);
+ FAIL_IF(ehdr->e_phoff + phdr_size > core_size);
+
+ /* Find the PT_NOTE segment. */
+ for (phdr = p + ehdr->e_phoff;
+ (void *) phdr < p + ehdr->e_phoff + phdr_size;
+ phdr += ehdr->e_phentsize)
+ if (phdr->p_type == PT_NOTE)
+ break;
+
+ FAIL_IF((void *) phdr >= p + ehdr->e_phoff + phdr_size);
+
+ /* Find the NT_PPC_PKEY note. */
+ for (nhdr = p + phdr->p_offset;
+ (void *) nhdr < p + phdr->p_offset + phdr->p_filesz;
+ nhdr = next_note(nhdr))
+ if (nhdr->n_type == NT_PPC_PKEY)
+ break;
+
+ FAIL_IF((void *) nhdr >= p + phdr->p_offset + phdr->p_filesz);
+ FAIL_IF(nhdr->n_descsz == 0);
+
+ p = nhdr;
+ note = p + sizeof(*nhdr) + __ALIGN_KERNEL(nhdr->n_namesz, 4);
+
+ regs = (unsigned long *) note;
+
+ printf("%-30s AMR: %016lx IAMR: %016lx UAMOR: %016lx\n",
+ core_read_running, regs[0], regs[1], regs[2]);
+
+ FAIL_IF(regs[0] != info->amr);
+ FAIL_IF(regs[1] != info->iamr);
+ FAIL_IF(regs[2] != info->uamor);
+
+ return TEST_PASS;
+}
+
+static int parent(struct shared_info *info, pid_t pid)
+{
+ char *filenames, *filename[3];
+ int fd, i, ret, status;
+ off_t core_size;
+ void *core;
+
+ ret = wait(&status);
+ if (ret != pid) {
+ printf("Child's exit status not captured\n");
+ return TEST_FAIL;
+ } else if (!WIFSIGNALED(status) || !WCOREDUMP(status)) {
+ printf("Child didn't dump core\n");
+ return TEST_FAIL;
+ }
+
+ /* Construct array of core file names to try. */
+
+ filename[0] = filenames = malloc(PATH_MAX);
+ if (!filenames) {
+ perror("Error allocating memory");
+ return TEST_FAIL;
+ }
+
+ ret = snprintf(filename[0], PATH_MAX, "core-pkey.%d", pid);
+ if (ret < 0 || ret >= PATH_MAX) {
+ ret = TEST_FAIL;
+ goto out;
+ }
+
+ filename[1] = filename[0] + ret + 1;
+ ret = snprintf(filename[1], PATH_MAX - ret - 1, "core.%d", pid);
+ if (ret < 0 || ret >= PATH_MAX - ret - 1) {
+ ret = TEST_FAIL;
+ goto out;
+ }
+ filename[2] = "core";
+
+ for (i = 0; i < 3; i++) {
+ core_size = try_core_file(filename[i], info, pid);
+ if (core_size != TEST_FAIL)
+ break;
+ }
+
+ if (i == 3) {
+ printf("Couldn't find core file\n");
+ ret = TEST_FAIL;
+ goto out;
+ }
+
+ fd = open(filename[i], O_RDONLY);
+ if (fd == -1) {
+ perror("Error opening core file");
+ ret = TEST_FAIL;
+ goto out;
+ }
+
+ core = mmap(NULL, core_size, PROT_READ, MAP_PRIVATE, fd, 0);
+ if (core == (void *) -1) {
+ perror("Error mmaping core file");
+ ret = TEST_FAIL;
+ goto out;
+ }
+
+ ret = check_core_file(info, core, core_size);
+
+ munmap(core, core_size);
+ close(fd);
+ unlink(filename[i]);
+
+ out:
+ free(filenames);
+
+ return ret;
+}
+
+static int write_core_pattern(const char *core_pattern)
+{
+ size_t len = strlen(core_pattern), ret;
+ FILE *f;
+
+ f = fopen(core_pattern_file, "w");
+ if (!f) {
+ perror("Error writing to core_pattern file");
+ return TEST_FAIL;
+ }
+
+ ret = fwrite(core_pattern, 1, len, f);
+ fclose(f);
+ if (ret != len) {
+ perror("Error writing to core_pattern file");
+ return TEST_FAIL;
+ }
+
+ return TEST_PASS;
+}
+
+static int setup_core_pattern(char **core_pattern_, bool *changed_)
+{
+ FILE *f;
+ char *core_pattern;
+ int ret;
+
+ core_pattern = malloc(PATH_MAX);
+ if (!core_pattern) {
+ perror("Error allocating memory");
+ return TEST_FAIL;
+ }
+
+ f = fopen(core_pattern_file, "r");
+ if (!f) {
+ perror("Error opening core_pattern file");
+ ret = TEST_FAIL;
+ goto out;
+ }
+
+ ret = fread(core_pattern, 1, PATH_MAX, f);
+ fclose(f);
+ if (!ret) {
+ perror("Error reading core_pattern file");
+ ret = TEST_FAIL;
+ goto out;
+ }
+
+ /* Check whether we can predict the name of the core file. */
+ if (!strcmp(core_pattern, "core") || !strcmp(core_pattern, "core.%p"))
+ *changed_ = false;
+ else {
+ ret = write_core_pattern("core-pkey.%p");
+ if (ret)
+ goto out;
+
+ *changed_ = true;
+ }
+
+ *core_pattern_ = core_pattern;
+ ret = TEST_PASS;
+
+ out:
+ if (ret)
+ free(core_pattern);
+
+ return ret;
+}
+
+static int core_pkey(void)
+{
+ char *core_pattern;
+ bool changed_core_pattern;
+ struct shared_info *info;
+ int shm_id;
+ int ret;
+ pid_t pid;
+
+ ret = setup_core_pattern(&core_pattern, &changed_core_pattern);
+ if (ret)
+ return ret;
+
+ shm_id = shmget(IPC_PRIVATE, sizeof(*info), 0777 | IPC_CREAT);
+ info = shmat(shm_id, NULL, 0);
+
+ pid = fork();
+ if (pid < 0) {
+ perror("fork() failed");
+ ret = TEST_FAIL;
+ } else if (pid == 0)
+ ret = child(info);
+ else
+ ret = parent(info, pid);
+
+ shmdt(info);
+
+ if (pid) {
+ shmctl(shm_id, IPC_RMID, NULL);
+
+ if (changed_core_pattern)
+ write_core_pattern(core_pattern);
+ }
+
+ free(core_pattern);
+
+ return ret;
+}
+
+int main(int argc, char *argv[])
+{
+ return test_harness(core_pkey, "core_pkey");
+}
--
1.7.1
^ permalink raw reply related
* Re: Pull request: scottwood/linux.git next
From: Michael Ellerman @ 2018-01-23 5:24 UTC (permalink / raw)
To: Scott Wood; +Cc: linuxppc-dev, galak
In-Reply-To: <20180121075456.3knaw4imnsl77jaa@home.buserror.net>
Scott Wood <oss@buserror.net> writes:
> Contains fixes for CPM GPIO and an FSL PCI erratum workaround,
> plus a minor cleanup patch.
>
> The following changes since commit f2ac428e0edabbca41b9dfe9473a90147962e4e9:
>
> powerpc/pseries/cpuidle: add polling idle for shared processor guests (2018-01-18 15:43:44 +1100)
>
> are available in the git repository at:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/scottwood/linux.git next
>
> for you to fetch changes up to c095ff93f901c1620b28dce4d813dd548bc5236b:
>
> powerpc/sysdev: change CPM GPIO to platform_device (2018-01-20 23:29:02 -0600)
Pulled thanks.
cheers
^ permalink raw reply
* Re: [PATCH v10 01/27] mm, powerpc, x86: define VM_PKEY_BITx bits if CONFIG_ARCH_HAS_PKEYS is enabled
From: Ram Pai @ 2018-01-23 6:37 UTC (permalink / raw)
To: akpm
Cc: linuxppc-dev, linux-mm, x86, linux-arch, linux-doc,
linux-kselftest, linux-kernel, dave.hansen, benh, paulus,
khandual, aneesh.kumar, bsingharora, hbabu, mhocko, bauerman,
ebiederm, mpe, mingo, corbet, arnd
In-Reply-To: <1516326648-22775-2-git-send-email-linuxram@us.ibm.com>
Andrew,
Please apply the following two patches to your tree.
[PATCH v10 01/27] mm, powerpc, x86: define VM_PKEY_BITx bits if CONFIG_ARCH_HAS_PKEYS is enabled
[PATCH v10 02/27] mm, powerpc, x86: introduce an additional vma bit for powerpc pkey
I have not heard any complaints on these changes.
Dave Hansen had comments/suggestions in the initial revisions, which have been incorporated.
Michael Ellermen has accepted the rest of the powerpc related patches in this series.
Thanks,
RP
On Thu, Jan 18, 2018 at 05:50:22PM -0800, Ram Pai wrote:
> VM_PKEY_BITx are defined only if CONFIG_X86_INTEL_MEMORY_PROTECTION_KEYS
> is enabled. Powerpc also needs these bits. Hence lets define the
> VM_PKEY_BITx bits for any architecture that enables
> CONFIG_ARCH_HAS_PKEYS.
>
> Signed-off-by: Ram Pai <linuxram@us.ibm.com>
> ---
> fs/proc/task_mmu.c | 4 ++--
> include/linux/mm.h | 9 +++++----
> 2 files changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
> index 339e4c1..b139617 100644
> --- a/fs/proc/task_mmu.c
> +++ b/fs/proc/task_mmu.c
> @@ -674,13 +674,13 @@ static void show_smap_vma_flags(struct seq_file *m, struct vm_area_struct *vma)
> [ilog2(VM_MERGEABLE)] = "mg",
> [ilog2(VM_UFFD_MISSING)]= "um",
> [ilog2(VM_UFFD_WP)] = "uw",
> -#ifdef CONFIG_X86_INTEL_MEMORY_PROTECTION_KEYS
> +#ifdef CONFIG_ARCH_HAS_PKEYS
> /* These come out via ProtectionKey: */
> [ilog2(VM_PKEY_BIT0)] = "",
> [ilog2(VM_PKEY_BIT1)] = "",
> [ilog2(VM_PKEY_BIT2)] = "",
> [ilog2(VM_PKEY_BIT3)] = "",
> -#endif
> +#endif /* CONFIG_ARCH_HAS_PKEYS */
> };
> size_t i;
>
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index ea818ff..01381d3 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -228,15 +228,16 @@ extern int overcommit_kbytes_handler(struct ctl_table *, int, void __user *,
> #define VM_HIGH_ARCH_4 BIT(VM_HIGH_ARCH_BIT_4)
> #endif /* CONFIG_ARCH_USES_HIGH_VMA_FLAGS */
>
> -#if defined(CONFIG_X86)
> -# define VM_PAT VM_ARCH_1 /* PAT reserves whole VMA at once (x86) */
> -#if defined (CONFIG_X86_INTEL_MEMORY_PROTECTION_KEYS)
> +#ifdef CONFIG_ARCH_HAS_PKEYS
> # define VM_PKEY_SHIFT VM_HIGH_ARCH_BIT_0
> # define VM_PKEY_BIT0 VM_HIGH_ARCH_0 /* A protection key is a 4-bit value */
> # define VM_PKEY_BIT1 VM_HIGH_ARCH_1
> # define VM_PKEY_BIT2 VM_HIGH_ARCH_2
> # define VM_PKEY_BIT3 VM_HIGH_ARCH_3
> -#endif
> +#endif /* CONFIG_ARCH_HAS_PKEYS */
> +
> +#if defined(CONFIG_X86)
> +# define VM_PAT VM_ARCH_1 /* PAT reserves whole VMA at once (x86) */
> #elif defined(CONFIG_PPC)
> # define VM_SAO VM_ARCH_1 /* Strong Access Ordering (powerpc) */
> #elif defined(CONFIG_PARISC)
> --
> 1.7.1
>
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majordomo@kvack.org. For more info on Linux MM,
> see: https://urldefense.proofpoint.com/v2/url?u=http-3A__www.linux-2Dmm.org_&d=DwIBAg&c=jf_iaSHvJObTbx-siA1ZOg&r=m-UrKChQVkZtnPpjbF6YY99NbT8FBByQ-E-ygV8luxw&m=PsCrC-HVeq8M98fNireZs4GUBJvMwNZme7wZ1YdjMqs&s=V90akzFmL1g-sNEcgmcUn_XJgJ8EaYmmsAS3AcVYScw&e= .
> Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
--
Ram Pai
^ permalink raw reply
* Re: [PATCH] powerpc: pseries: use irq_of_parse_and_map helper
From: Michael Ellerman @ 2018-01-23 6:53 UTC (permalink / raw)
To: Rob Herring
Cc: linux-kernel, Benjamin Herrenschmidt, Paul Mackerras,
linuxppc-dev
In-Reply-To: <20180104224542.15333-4-robh@kernel.org>
Rob Herring <robh@kernel.org> writes:
> Instead of calling both of_irq_parse_one and irq_create_of_mapping, call
> of_irq_parse_and_map instead which does the same thing. This gets us closer
> to making the former 2 functions static.
>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: linuxppc-dev@lists.ozlabs.org
> Signed-off-by: Rob Herring <robh@kernel.org>
> ---
> arch/powerpc/platforms/pseries/event_sources.c | 11 ++++-------
> 1 file changed, 4 insertions(+), 7 deletions(-)
Sorry NAK, this doesn't boot.
> diff --git a/arch/powerpc/platforms/pseries/event_sources.c b/arch/powerpc/platforms/pseries/event_sources.c
> index 6eeb0d4bab61..b0d8c146fe7b 100644
> --- a/arch/powerpc/platforms/pseries/event_sources.c
> +++ b/arch/powerpc/platforms/pseries/event_sources.c
> @@ -16,7 +16,8 @@
> * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
> */
>
> -#include <asm/prom.h>
> +#include <linux/interrupt.h>
> +#include <linux/of_irq.h>
>
> #include "pseries.h"
>
> @@ -25,15 +26,11 @@ void request_event_sources_irqs(struct device_node *np,
> const char *name)
> {
> int i, index, count = 0;
> - struct of_phandle_args oirq;
> unsigned int virqs[16];
>
> /* First try to do a proper OF tree parsing */
> - for (index = 0; of_irq_parse_one(np, index, &oirq) == 0;
> - index++) {
> - if (count > 15)
> - break;
> - virqs[count] = irq_create_of_mapping(&oirq);
> + for (index = 0; count < 16; index++) {
> + virqs[count] = irq_of_parse_and_map(np, index);
> if (!virqs[count]) {
> pr_err("event-sources: Unable to allocate "
> "interrupt number for %pOF\n",
np);
WARN_ON(1);
} else {
count++;
}
}
Which is an infinite loop if we have less than 16 irqs, and spews the
warning continuously.
Are you trying to remove the low-level routines or is this just a
cleanup?
The patch below works, it loses the error handling if the interrupts
property is corrupt/empty, but that's probably overly paranoid anyway.
cheers
diff --git a/arch/powerpc/platforms/pseries/event_sources.c b/arch/powerpc/platforms/pseries/event_sources.c
index 6eeb0d4bab61..25c38077c894 100644
--- a/arch/powerpc/platforms/pseries/event_sources.c
+++ b/arch/powerpc/platforms/pseries/event_sources.c
@@ -16,7 +16,8 @@
* Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
*/
-#include <asm/prom.h>
+#include <linux/interrupt.h>
+#include <linux/of_irq.h>
#include "pseries.h"
@@ -24,34 +25,16 @@ void request_event_sources_irqs(struct device_node *np,
irq_handler_t handler,
const char *name)
{
- int i, index, count = 0;
- struct of_phandle_args oirq;
- unsigned int virqs[16];
+ unsigned int virq;
+ int i, rc;
- /* First try to do a proper OF tree parsing */
- for (index = 0; of_irq_parse_one(np, index, &oirq) == 0;
- index++) {
- if (count > 15)
+ for (i = 0; i < 16; i++) {
+ virq = irq_of_parse_and_map(np, i);
+ if (!virq)
break;
- virqs[count] = irq_create_of_mapping(&oirq);
- if (!virqs[count]) {
- pr_err("event-sources: Unable to allocate "
- "interrupt number for %pOF\n",
- np);
- WARN_ON(1);
- } else {
- count++;
- }
- }
- /* Now request them */
- for (i = 0; i < count; i++) {
- if (request_irq(virqs[i], handler, 0, name, NULL)) {
- pr_err("event-sources: Unable to request interrupt "
- "%d for %pOF\n", virqs[i], np);
- WARN_ON(1);
- return;
- }
+ rc = request_irq(virq, handler, 0, name, NULL);
+ WARN(rc, "event-sources: Unable to request interrupt %d for %pOF\n",
+ virq, np);
}
}
-
^ permalink raw reply related
* Re: Anyone working on the KPTI support for powerpc?
From: Michael Ellerman @ 2018-01-23 6:56 UTC (permalink / raw)
To: Yang Li, linuxppc-dev
In-Reply-To: <CADRPPNRVb_CGBesSD6zQcPmrATxqSHhSmywAEfTT7jqpioNaGg@mail.gmail.com>
Yang Li <pku.leo@gmail.com> writes:
> I assume some of the powerpc platforms would be impacted by the newly
> announced meltdown/spetre vulnerabilities. It will be good if we have
> an aligned implementation of the KPTI.
Hi Leo,
We aren't working on KPTI at the moment, but we did recently merge some
related changes:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=6bb821193b6e8b9cc40714f700da27b4688cbc22
cheers
^ permalink raw reply
* Re: [PATCH 13/26] KVM: PPC: Book3S PR: adds new kvmppc_copyto_vcpu_tm/kvmppc_copyfrom_vcpu_tm API for PR KVM.
From: Paul Mackerras @ 2018-01-23 5:52 UTC (permalink / raw)
To: wei.guo.simon; +Cc: linuxppc-dev, kvm, kvm-ppc
In-Reply-To: <1515665499-31710-14-git-send-email-wei.guo.simon@gmail.com>
On Thu, Jan 11, 2018 at 06:11:26PM +0800, wei.guo.simon@gmail.com wrote:
> From: Simon Guo <wei.guo.simon@gmail.com>
>
> This patch adds 2 new APIs: kvmppc_copyto_vcpu_tm() and
> kvmppc_copyfrom_vcpu_tm(). These 2 APIs will be used to copy from/to TM
> data between VCPU_TM/VCPU area.
>
> PR KVM will use these APIs for treclaim. or trchkpt. emulation.
>
> Signed-off-by: Simon Guo <wei.guo.simon@gmail.com>
> Reviewed-by: Paul Mackerras <paulus@ozlabs.org>
Actually, I take that back. You have missed XER. :)
Paul.
^ permalink raw reply
* Re: [PATCH 10/26] KVM: PPC: Book3S PR: set MSR HV bit accordingly for PPC970 and others.
From: Paul Mackerras @ 2018-01-23 5:51 UTC (permalink / raw)
To: wei.guo.simon; +Cc: linuxppc-dev, kvm, kvm-ppc
In-Reply-To: <1515665499-31710-11-git-send-email-wei.guo.simon@gmail.com>
On Thu, Jan 11, 2018 at 06:11:23PM +0800, wei.guo.simon@gmail.com wrote:
> From: Simon Guo <wei.guo.simon@gmail.com>
>
> Apple G5 machines(PPC970/FX/GX/MP) have supervisor mode disabled and
> MSR HV bit is forced into 1. We should follow this in PR KVM guest.
>
> This patch set MSR HV=1 for G5 machines and HV=0 for others on PR
> KVM guest.
>
> Signed-off-by: Simon Guo <wei.guo.simon@gmail.com>
> Suggested-by: Paul Mackerras <paulus@ozlabs.org>
Reviewed-by: Paul Mackerras <paulus@ozlabs.org>
^ permalink raw reply
* Re: [PATCH 02/26] KVM: PPC: Book3S PR: add new parameter (guest MSR) for kvmppc_save_tm()/kvmppc_restore_tm()
From: Paul Mackerras @ 2018-01-23 5:42 UTC (permalink / raw)
To: wei.guo.simon; +Cc: linuxppc-dev, kvm, kvm-ppc
In-Reply-To: <1515665499-31710-3-git-send-email-wei.guo.simon@gmail.com>
On Thu, Jan 11, 2018 at 06:11:15PM +0800, wei.guo.simon@gmail.com wrote:
> From: Simon Guo <wei.guo.simon@gmail.com>
>
> HV KVM and PR KVM need different MSR source to indicate whether
> treclaim. or trecheckpoint. is necessary.
>
> This patch add new parameter (guest MSR) for these kvmppc_save_tm/
> kvmppc_restore_tm() APIs:
> - For HV KVM, it is VCPU_MSR
> - For PR KVM, it is current host MSR or VCPU_SHADOW_SRR1
>
> This enhancement enables these 2 APIs to be reused by PR KVM later.
> And the patch keeps HV KVM logic unchanged.
>
> This patch also reworks kvmppc_save_tm()/kvmppc_restore_tm() to
> have a clean ABI: r3 for vcpu and r4 for guest_msr.
>
> Signed-off-by: Simon Guo <wei.guo.simon@gmail.com>
Question: why do you switch from using HSTATE_HOST_R1 to HSTATE_SCRATCH2
> @@ -42,11 +45,11 @@ _GLOBAL(kvmppc_save_tm)
> rldimi r8, r0, MSR_TM_LG, 63-MSR_TM_LG
> mtmsrd r8
>
> - ld r5, VCPU_MSR(r9)
> - rldicl. r5, r5, 64 - MSR_TS_S_LG, 62
> + rldicl. r4, r4, 64 - MSR_TS_S_LG, 62
> beq 1f /* TM not active in guest. */
>
> - std r1, HSTATE_HOST_R1(r13)
> + std r1, HSTATE_SCRATCH2(r13)
... here?
> @@ -166,17 +173,17 @@ _GLOBAL(kvmppc_restore_tm)
> * The user may change these outside of a transaction, so they must
> * always be context switched.
> */
> - ld r5, VCPU_TFHAR(r4)
> - ld r6, VCPU_TFIAR(r4)
> - ld r7, VCPU_TEXASR(r4)
> + ld r5, VCPU_TFHAR(r3)
> + ld r6, VCPU_TFIAR(r3)
> + ld r7, VCPU_TEXASR(r3)
> mtspr SPRN_TFHAR, r5
> mtspr SPRN_TFIAR, r6
> mtspr SPRN_TEXASR, r7
>
> - ld r5, VCPU_MSR(r4)
> + mr r5, r4
> rldicl. r5, r5, 64 - MSR_TS_S_LG, 62
> beqlr /* TM not active in guest */
> - std r1, HSTATE_HOST_R1(r13)
> + std r1, HSTATE_SCRATCH2(r13)
and here?
Please add a paragraph to the patch description explaining why you are
making that change.
Paul.
^ permalink raw reply
* Re: [PATCH 16/26] KVM: PPC: Book3S PR: add transaction memory save/restore skeleton for PR KVM
From: Paul Mackerras @ 2018-01-23 6:04 UTC (permalink / raw)
To: wei.guo.simon; +Cc: linuxppc-dev, kvm, kvm-ppc
In-Reply-To: <1515665499-31710-17-git-send-email-wei.guo.simon@gmail.com>
On Thu, Jan 11, 2018 at 06:11:29PM +0800, wei.guo.simon@gmail.com wrote:
> From: Simon Guo <wei.guo.simon@gmail.com>
>
> The transaction memory checkpoint area save/restore behavior is
> triggered when VCPU qemu process is switching out/into CPU. ie.
> at kvmppc_core_vcpu_put_pr() and kvmppc_core_vcpu_load_pr().
>
> MSR TM active state is determined by TS bits:
> active: 10(transactional) or 01 (suspended)
> inactive: 00 (non-transactional)
> We don't "fake" TM functionality for guest. We "sync" guest virtual
> MSR TM active state(10 or 01) with shadow MSR. That is to say,
> we don't emulate a transactional guest with a TM inactive MSR.
>
> TM SPR support(TFIAR/TFAR/TEXASR) has already been supported by
> commit 9916d57e64a4 ("KVM: PPC: Book3S PR: Expose TM registers").
> Math register support (FPR/VMX/VSX) will be done at subsequent
> patch.
>
> - TM save:
> When kvmppc_save_tm_pr() is invoked, whether TM context need to
> be saved can be determined by current host MSR state:
> * TM active - save TM context
> * TM inactive - no need to do so and only save TM SPRs.
>
> - TM restore:
> However when kvmppc_restore_tm_pr() is invoked, there is an
> issue to determine whether TM restore should be performed.
> The TM active host MSR val saved in kernel stack is not loaded yet.
I don't follow this exactly. What is the value saved on the kernel
stack?
I get that we may not have done the sync from the shadow MSR back to
the guest MSR, since that is done in kvmppc_handle_exit_pr() with
interrupts enabled and we might be unloading because we got
preempted. In that case we would have svcpu->in_use = 1, and we
should in fact do the sync of the TS bits from shadow_msr to the vcpu
MSR value in kvmppc_copy_from_svcpu(). If you did that then both the
load and put functions could just rely on the vcpu's MSR value.
> We don't know whether there is a transaction to be restored from
> current host MSR TM status at kvmppc_restore_tm_pr(). To solve this
> issue, we save current MSR into vcpu->arch.save_msr_tm at
> kvmppc_save_tm_pr(), and kvmppc_restore_tm_pr() check TS bits of
> vcpu->arch.save_msr_tm to decide whether to do TM restore.
>
> Signed-off-by: Simon Guo <wei.guo.simon@gmail.com>
> Suggested-by: Paul Mackerras <paulus@ozlabs.org>
> ---
> arch/powerpc/include/asm/kvm_book3s.h | 6 +++++
> arch/powerpc/include/asm/kvm_host.h | 1 +
> arch/powerpc/kvm/book3s_pr.c | 41 +++++++++++++++++++++++++++++++++++
> 3 files changed, 48 insertions(+)
>
> diff --git a/arch/powerpc/include/asm/kvm_book3s.h b/arch/powerpc/include/asm/kvm_book3s.h
> index 9a66700..d8dbfa5 100644
> --- a/arch/powerpc/include/asm/kvm_book3s.h
> +++ b/arch/powerpc/include/asm/kvm_book3s.h
> @@ -253,6 +253,12 @@ extern void kvmppc_copy_to_svcpu(struct kvmppc_book3s_shadow_vcpu *svcpu,
> struct kvm_vcpu *vcpu);
> extern void kvmppc_copy_from_svcpu(struct kvm_vcpu *vcpu,
> struct kvmppc_book3s_shadow_vcpu *svcpu);
> +
> +#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
> +void kvmppc_save_tm_pr(struct kvm_vcpu *vcpu);
> +void kvmppc_restore_tm_pr(struct kvm_vcpu *vcpu);
> +#endif
It would be cleaner at the point where you use these if you added a
#else clause to define a null version for the case when transactional
memory support is not configured, like this:
+#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
+void kvmppc_save_tm_pr(struct kvm_vcpu *vcpu);
+void kvmppc_restore_tm_pr(struct kvm_vcpu *vcpu);
+#else
+static inline void kvmppc_save_tm_pr(struct kvm_vcpu *vcpu) {}
+static inline void kvmppc_restore_tm_pr(struct kvm_vcpu *vcpu) {}
+#endif
That way you don't need the #ifdef at the call site.
> @@ -131,6 +135,10 @@ static void kvmppc_core_vcpu_put_pr(struct kvm_vcpu *vcpu)
> if (kvmppc_is_split_real(vcpu))
> kvmppc_unfixup_split_real(vcpu);
>
> +#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
> + kvmppc_save_tm_pr(vcpu);
> +#endif
> +
> kvmppc_giveup_ext(vcpu, MSR_FP | MSR_VEC | MSR_VSX);
> kvmppc_giveup_fac(vcpu, FSCR_TAR_LG);
I think you should do these giveup_ext/giveup_fac calls before calling
kvmppc_save_tm_pr, because the treclaim in kvmppc_save_tm_pr will
modify all the FP/VEC/VSX registers and the TAR.
Paul.
^ permalink raw reply
* Re: [PATCH 04/26] KVM: PPC: Book3S PR: add C function wrapper for _kvmppc_save/restore_tm()
From: Paul Mackerras @ 2018-01-23 5:49 UTC (permalink / raw)
To: wei.guo.simon; +Cc: linuxppc-dev, kvm, kvm-ppc
In-Reply-To: <1515665499-31710-5-git-send-email-wei.guo.simon@gmail.com>
On Thu, Jan 11, 2018 at 06:11:17PM +0800, wei.guo.simon@gmail.com wrote:
> From: Simon Guo <wei.guo.simon@gmail.com>
>
> Currently _kvmppc_save/restore_tm() APIs can only be invoked from
> assembly function. This patch adds C function wrappers for them so
> that they can be safely called from C function.
>
> Signed-off-by: Simon Guo <wei.guo.simon@gmail.com>
[snip]
> --- a/arch/powerpc/include/asm/asm-prototypes.h
> +++ b/arch/powerpc/include/asm/asm-prototypes.h
> @@ -126,4 +126,11 @@ unsigned long __init prom_init(unsigned long r3, unsigned long r4,
> void _mcount(void);
> unsigned long prepare_ftrace_return(unsigned long parent, unsigned long ip);
>
> +#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
> +/* Transaction memory related */
> +struct kvm_vcpu;
> +void _kvmppc_restore_tm_pr(struct kvm_vcpu *vcpu, u64 guest_msr);
> +void _kvmppc_save_tm_pr(struct kvm_vcpu *vcpu, u64 guest_msr);
> +#endif
It's not generally necessary to have ifdefs around function
declarations. If the function is never defined because the feature
is not configured in, that is fine.
> @@ -149,6 +149,58 @@ _GLOBAL(kvmppc_save_tm)
> blr
>
> /*
> + * _kvmppc_save_tm() is a wrapper around __kvmppc_save_tm(), so that it can
> + * be invoked from C function by PR KVM only.
> + */
> +_GLOBAL(_kvmppc_save_tm_pr)
> + mflr r5
> + std r5, PPC_LR_STKOFF(r1)
> + stdu r1, -SWITCH_FRAME_SIZE(r1)
> + SAVE_NVGPRS(r1)
> +
> + /* save MSR since TM/math bits might be impacted
> + * by __kvmppc_save_tm().
> + */
> + mfmsr r5
> + SAVE_GPR(5, r1)
> +
> + /* also save DSCR/CR so that it can be recovered later */
> + mfspr r6, SPRN_DSCR
> + SAVE_GPR(6, r1)
> +
> + mfcr r7
> + stw r7, _CCR(r1)
> +
> + /* allocate stack frame for __kvmppc_save_tm since
> + * it will save LR into its stackframe and we don't
> + * want to corrupt _kvmppc_save_tm_pr's.
> + */
> + stdu r1, -PPC_MIN_STKFRM(r1)
You don't need to do this. In the PowerPC ELF ABI, functions always
save their LR (i.e. their return address) in their *caller's* stack
frame, not their own. You have established a stack frame for
_kvmppc_save_tm_pr above, and that is sufficient. Same comment
applies for _kvmppc_restore_tm_pr.
Paul.
^ permalink raw reply
* Re: [PATCH 07/26] KVM: PPC: Book3S PR: add TEXASR related macros
From: Paul Mackerras @ 2018-01-23 5:50 UTC (permalink / raw)
To: wei.guo.simon; +Cc: linuxppc-dev, kvm, kvm-ppc
In-Reply-To: <1515665499-31710-8-git-send-email-wei.guo.simon@gmail.com>
On Thu, Jan 11, 2018 at 06:11:20PM +0800, wei.guo.simon@gmail.com wrote:
> From: Simon Guo <wei.guo.simon@gmail.com>
>
> This patches add some macros for CR0/TEXASR bits so that PR KVM TM
> logic(tbegin./treclaim./tabort.) can make use of them later.
>
> Signed-off-by: Simon Guo <wei.guo.simon@gmail.com>
Reviewed-by: Paul Mackerras <paulus@ozlabs.org>
This and some of your other patches will need to go via Michael
Ellerman's tree.
Paul.
^ permalink raw reply
* Re: [PATCH 00/26] KVM: PPC: Book3S PR: Transaction memory support on PR KVM
From: Paul Mackerras @ 2018-01-23 5:38 UTC (permalink / raw)
To: wei.guo.simon; +Cc: linuxppc-dev, kvm, kvm-ppc
In-Reply-To: <1515665499-31710-1-git-send-email-wei.guo.simon@gmail.com>
On Thu, Jan 11, 2018 at 06:11:13PM +0800, wei.guo.simon@gmail.com wrote:
> From: Simon Guo <wei.guo.simon@gmail.com>
>
> In current days, many OS distributions have utilized transaction
> memory functionality. In PowerPC, HV KVM supports TM. But PR KVM
> does not.
>
> The drive for the transaction memory support of PR KVM is the
> openstack Continuous Integration testing - They runs a HV(hypervisor)
> KVM(as level 1) and then run PR KVM(as level 2) on top of that.
>
> This patch set add transaction memory support on PR KVM.
Thanks for the patch set. It mostly looks good, though I have some
comments on the individual patches.
I don't see where you are implementing support for userspace accessing
the TM checkpointed register values using the GET_ONE_REG/SET_ONE_REG
API. This would mean that you couldn't migrate a guest that was in
the middle of a transaction. We will need to have the one_reg API
access to the TM checkpoint implemented, though there will be a
difficulty in that kvmppc_get_one_reg() and kvmppc_set_one_reg() are
called with the vcpu context loaded. With your scheme of having the
TM checkpoint stored in the CPU while the vcpu context is loaded, the
values you want to access in kvmppc_get/set_one_reg are inaccessible
since they're stored in the CPU. You would have to arrange for
kvmppc_get/set_one_reg to be called without the vcpu context loaded
(recent patches in the kvm next branch probably make that easier) or
else explicitly unload and reload the vcpu context in those functions.
(This is easier in HV KVM since the checkpoint is not in the CPU at
the point of doing kvmppc_get/set_one_reg.)
There is also complexity added because it's possible for the guest to
have TM, FP, VEC and VSX all enabled from its point of view but to
have FP/VEC/VSX not actually enabled in the hardware when the guest is
running. As you note in your patch descriptions, this means that the
guest can do tbegin and create a checkpoint with bogus values for the
FP/VEC/VSX registers. Rather than trying to detect and fix up this
situation after the fact, I would suggest that if the guest has TM
enabled then we make sure that the real FP/VEC/VSX bits in the MSR
match what the guest thinks it has. That way we would avoid the bogus
checkpoint problem. (There is still the possibility of getting bogus
checkpointed FP/VEC/VSX registers if the guest does tbegin with the
FP/VEC/VSX bits clear in the MSR, but that is the guest's problem to
deal with.)
Paul.
^ permalink raw reply
* Re: [PATCH 00/26] KVM: PPC: Book3S PR: Transaction memory support on PR KVM
From: Paul Mackerras @ 2018-01-23 7:16 UTC (permalink / raw)
To: wei.guo.simon; +Cc: linuxppc-dev, kvm, kvm-ppc
In-Reply-To: <20180123053832.GA3924@fergus.ozlabs.ibm.com>
On Tue, Jan 23, 2018 at 04:38:32PM +1100, Paul Mackerras wrote:
> On Thu, Jan 11, 2018 at 06:11:13PM +0800, wei.guo.simon@gmail.com wrote:
> > From: Simon Guo <wei.guo.simon@gmail.com>
> >
> > In current days, many OS distributions have utilized transaction
> > memory functionality. In PowerPC, HV KVM supports TM. But PR KVM
> > does not.
> >
> > The drive for the transaction memory support of PR KVM is the
> > openstack Continuous Integration testing - They runs a HV(hypervisor)
> > KVM(as level 1) and then run PR KVM(as level 2) on top of that.
> >
> > This patch set add transaction memory support on PR KVM.
>
> Thanks for the patch set. It mostly looks good, though I have some
> comments on the individual patches.
>
> I don't see where you are implementing support for userspace accessing
> the TM checkpointed register values using the GET_ONE_REG/SET_ONE_REG
> API. This would mean that you couldn't migrate a guest that was in
> the middle of a transaction. We will need to have the one_reg API
> access to the TM checkpoint implemented, though there will be a
> difficulty in that kvmppc_get_one_reg() and kvmppc_set_one_reg() are
> called with the vcpu context loaded. With your scheme of having the
> TM checkpoint stored in the CPU while the vcpu context is loaded, the
> values you want to access in kvmppc_get/set_one_reg are inaccessible
> since they're stored in the CPU. You would have to arrange for
> kvmppc_get/set_one_reg to be called without the vcpu context loaded
> (recent patches in the kvm next branch probably make that easier) or
> else explicitly unload and reload the vcpu context in those functions.
> (This is easier in HV KVM since the checkpoint is not in the CPU at
> the point of doing kvmppc_get/set_one_reg.)
Another complexity that hasn't been dealt with as far as I can see is
that if userspace does a KVM_SET_REGS that changes the TS field in the
guest MSR, we don't do anything to make the state of the CPU match.
As with GET/SET_ONE_REG, KVM_SET_REGS is called with the vcpu loaded,
so it needs to make the physical CPU state match what it would have
been had the new state been present at load time, perhaps by unloading
the CPU before changing the state, then reloading it. But if you do
that and you are using the vcpu->arch.save_msr_tm field that you add,
then you need to modify that when you do kvmppc_set_msr(). (I would
rather that your save/restore TM functions work off kvmppc_get_msr()
rather than having the save_msr_tm field.)
Paul.
^ permalink raw reply
* Re: [PATCH 17/26] KVM: PPC: Book3S PR: add math support for PR KVM HTM
From: Paul Mackerras @ 2018-01-23 7:29 UTC (permalink / raw)
To: wei.guo.simon; +Cc: linuxppc-dev, kvm, kvm-ppc
In-Reply-To: <1515665499-31710-18-git-send-email-wei.guo.simon@gmail.com>
On Thu, Jan 11, 2018 at 06:11:30PM +0800, wei.guo.simon@gmail.com wrote:
> ines: 219
>
> From: Simon Guo <wei.guo.simon@gmail.com>
>
> The math registers will be saved into vcpu->arch.fp/vr and corresponding
> vcpu->arch.fp_tm/vr_tm area.
>
> We flush or giveup the math regs into vcpu->arch.fp/vr before saving
> transaction. After transaction is restored, the math regs will be loaded
> back into regs.
It looks to me that you are loading up the math regs on every vcpu
load, not just those with an active transaction. That seems like
overkill.
> If there is a FP/VEC/VSX unavailable exception during transaction active
> state, the math checkpoint content might be incorrect and we need to do
> treclaim./load the correct checkpoint val/trechkpt. sequence to retry the
> transaction.
I would prefer a simpler approach where just before entering the
guest, we check if the guest MSR TM bit is set, and if so we make sure
that whichever math regs are enabled in the guest MSR are actually
loaded on the CPU, that is, that guest_owned_ext has the same bits set
as the guest MSR. Then we never have to handle a FP/VEC/VSX
unavailable interrupt with a transaction active (other than by simply
passing it on to the guest).
Paul.
^ permalink raw reply
* Re: [PATCH 18/26] KVM: PPC: Book3S PR: make mtspr/mfspr emulation behavior based on active TM SPRs
From: Paul Mackerras @ 2018-01-23 8:17 UTC (permalink / raw)
To: wei.guo.simon; +Cc: linuxppc-dev, kvm, kvm-ppc
In-Reply-To: <1515665499-31710-19-git-send-email-wei.guo.simon@gmail.com>
On Thu, Jan 11, 2018 at 06:11:31PM +0800, wei.guo.simon@gmail.com wrote:
> From: Simon Guo <wei.guo.simon@gmail.com>
>
> The mfspr/mtspr on TM SPRs(TEXASR/TFIAR/TFHAR) are non-privileged
> instructions and can be executed at PR KVM guest without trapping
> into host in problem state. We only emulate mtspr/mfspr
> texasr/tfiar/tfhar at guest PR=0 state.
>
> When we are emulating mtspr tm sprs at guest PR=0 state, the emulation
> result need to be visible to guest PR=1 state. That is, the actual TM
> SPR val should be loaded into actual registers.
>
> We already flush TM SPRs into vcpu when switching out of CPU, and load
> TM SPRs when switching back.
>
> This patch corrects mfspr()/mtspr() emulation for TM SPRs to make the
> actual source/dest based on actual TM SPRs.
>
> Signed-off-by: Simon Guo <wei.guo.simon@gmail.com>
> ---
> arch/powerpc/kvm/book3s_emulate.c | 35 +++++++++++++++++++++++++++--------
> 1 file changed, 27 insertions(+), 8 deletions(-)
>
> diff --git a/arch/powerpc/kvm/book3s_emulate.c b/arch/powerpc/kvm/book3s_emulate.c
> index e096d01..c2836330 100644
> --- a/arch/powerpc/kvm/book3s_emulate.c
> +++ b/arch/powerpc/kvm/book3s_emulate.c
> @@ -521,13 +521,26 @@ int kvmppc_core_emulate_mtspr_pr(struct kvm_vcpu *vcpu, int sprn, ulong spr_val)
> break;
> #ifdef CONFIG_PPC_TRANSACTIONAL_MEM
> case SPRN_TFHAR:
> - vcpu->arch.tfhar = spr_val;
> - break;
> case SPRN_TEXASR:
> - vcpu->arch.texasr = spr_val;
> - break;
> case SPRN_TFIAR:
> - vcpu->arch.tfiar = spr_val;
> + if (MSR_TM_ACTIVE(kvmppc_get_msr(vcpu))) {
> + /* it is illegal to mtspr() TM regs in
> + * other than non-transactional state.
> + */
> + kvmppc_core_queue_program(vcpu, SRR1_PROGTM);
> + emulated = EMULATE_AGAIN;
> + break;
> + }
We also need to check that the guest has TM enabled in the guest MSR,
and give them a facility unavailable interrupt if not.
> +
> + tm_enable();
> + if (sprn == SPRN_TFHAR)
> + mtspr(SPRN_TFHAR, spr_val);
> + else if (sprn == SPRN_TEXASR)
> + mtspr(SPRN_TEXASR, spr_val);
> + else
> + mtspr(SPRN_TFIAR, spr_val);
> + tm_disable();
I haven't seen any checks that we are on a CPU that has TM. What
happens if a guest does a mtmsrd with TM=1 and then a mtspr to TEXASR
when running on a POWER7 (assuming the host kernel was compiled with
CONFIG_PPC_TRANSACTIONAL_MEM=y)?
Ideally, if the host CPU does not have TM functionality, these mtsprs
would be treated as no-ops and attempts to set the TM or TS fields in
the guest MSR would be ignored.
> +
> break;
> #endif
> #endif
> @@ -674,13 +687,19 @@ int kvmppc_core_emulate_mfspr_pr(struct kvm_vcpu *vcpu, int sprn, ulong *spr_val
> break;
> #ifdef CONFIG_PPC_TRANSACTIONAL_MEM
> case SPRN_TFHAR:
> - *spr_val = vcpu->arch.tfhar;
> + tm_enable();
> + *spr_val = mfspr(SPRN_TFHAR);
> + tm_disable();
> break;
> case SPRN_TEXASR:
> - *spr_val = vcpu->arch.texasr;
> + tm_enable();
> + *spr_val = mfspr(SPRN_TEXASR);
> + tm_disable();
> break;
> case SPRN_TFIAR:
> - *spr_val = vcpu->arch.tfiar;
> + tm_enable();
> + *spr_val = mfspr(SPRN_TFIAR);
> + tm_disable();
> break;
These need to check MSR_TM in the guest MSR, and become no-ops on
machines without TM capability.
Paul.
^ permalink raw reply
* Re: [PATCH 19/26] KVM: PPC: Book3S PR: always fail transaction in guest privilege state
From: Paul Mackerras @ 2018-01-23 8:30 UTC (permalink / raw)
To: wei.guo.simon; +Cc: linuxppc-dev, kvm, kvm-ppc
In-Reply-To: <1515665499-31710-20-git-send-email-wei.guo.simon@gmail.com>
On Thu, Jan 11, 2018 at 06:11:32PM +0800, wei.guo.simon@gmail.com wrote:
> From: Simon Guo <wei.guo.simon@gmail.com>
>
> Currently kernel doesn't use transaction memory.
> And there is an issue for privilege guest that:
> tbegin/tsuspend/tresume/tabort TM instructions can impact MSR TM bits
> without trap into PR host. So following code will lead to a false mfmsr
> result:
> tbegin <- MSR bits update to Transaction active.
> beq <- failover handler branch
> mfmsr <- still read MSR bits from magic page with
> transaction inactive.
>
> It is not an issue for non-privilege guest since its mfmsr is not patched
> with magic page and will always trap into PR host.
>
> This patch will always fail tbegin attempt for privilege guest, so that
> the above issue is prevented. It is benign since currently (guest) kernel
> doesn't initiate a transaction.
>
> Test case:
> https://github.com/justdoitqd/publicFiles/blob/master/test_tbegin_pr.c
>
> Signed-off-by: Simon Guo <wei.guo.simon@gmail.com>
You need to handle the case where MSR_TM is not set in the guest MSR,
and give the guest a facility unavailable interrupt.
[snip]
> --- a/arch/powerpc/kvm/book3s_pr.c
> +++ b/arch/powerpc/kvm/book3s_pr.c
> @@ -255,7 +255,7 @@ static inline void kvmppc_save_tm_sprs(struct kvm_vcpu *vcpu)
> tm_disable();
> }
>
> -static inline void kvmppc_restore_tm_sprs(struct kvm_vcpu *vcpu)
> +inline void kvmppc_restore_tm_sprs(struct kvm_vcpu *vcpu)
You should probably remove the 'inline' here too.
Paul.
^ permalink raw reply
* Re: [PATCH 20/26] KVM: PPC: Book3S PR: enable NV reg restore for reading TM SPR at guest privilege state
From: Paul Mackerras @ 2018-01-23 9:08 UTC (permalink / raw)
To: wei.guo.simon; +Cc: linuxppc-dev, kvm, kvm-ppc
In-Reply-To: <1515665499-31710-21-git-send-email-wei.guo.simon@gmail.com>
On Thu, Jan 11, 2018 at 06:11:33PM +0800, wei.guo.simon@gmail.com wrote:
> From: Simon Guo <wei.guo.simon@gmail.com>
>
> Currently kvmppc_handle_fac() will not update NV GPRs and thus it can
> return with GUEST_RESUME.
>
> However PR KVM guest always disables MSR_TM bit at privilege state. If PR
> privilege guest are trying to read TM SPRs, it will trigger TM facility
> unavailable exception and fall into kvmppc_handle_fac(). Then the emulation
> will be done by kvmppc_core_emulate_mfspr_pr(). The mfspr instruction can
> include a RT with NV reg. So it is necessary to restore NV GPRs at this
> case, to reflect the update to NV RT.
>
> This patch make kvmppc_handle_fac() return GUEST_RESUME_NV at TM fac
> exception and with guest privilege state.
>
> Signed-off-by: Simon Guo <wei.guo.simon@gmail.com>
Reviewed-by: Paul Mackerras <paulus@ozlabs.org>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox