* [PATCH v2 1/3] powerpc/powernv/npu: Reduce eieio usage when issuing ATSD invalidates
From: Mark Hairgrove @ 2018-10-03 18:51 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Alistair Popple, Mark Hairgrove, Reza Arbab
In-Reply-To: <1538592694-18739-1-git-send-email-mhairgrove@nvidia.com>
There are two types of ATSDs issued to the NPU: invalidates targeting a
specific virtual address and invalidates targeting the whole address
space. In both cases prior to this change, the sequence was:
for each NPU
- Write the target address to the XTS_ATSD_AVA register
- EIEIO
- Write the launch value to issue the ATSD
First, a target address is not required when invalidating the whole
address space, so that write and the EIEIO have been removed. The AP
(size) field in the launch is not needed either.
Second, for per-address invalidates the above sequence is inefficient in
the common case of multiple NPUs because an EIEIO is issued per NPU. This
unnecessarily forces the launches of later ATSDs to be ordered with the
launches of earlier ones. The new sequence only issues a single EIEIO:
for each NPU
- Write the target address to the XTS_ATSD_AVA register
EIEIO
for each NPU
- Write the launch value to issue the ATSD
Performance results were gathered using a microbenchmark which creates a
1G allocation then uses mprotect with PROT_NONE to trigger invalidates in
strides across the allocation.
With only a single NPU active (one GPU) the difference is in the noise for
both types of invalidates (+/-1%).
With two NPUs active (on a 6-GPU system) the effect is more noticeable:
mprotect rate (GB/s)
Stride Before After Speedup
64K 5.9 6.5 10%
1M 31.2 33.4 7%
2M 36.3 38.7 7%
4M 322.6 356.7 11%
Signed-off-by: Mark Hairgrove <mhairgrove@nvidia.com>
---
arch/powerpc/platforms/powernv/npu-dma.c | 99 ++++++++++++++---------------
1 files changed, 48 insertions(+), 51 deletions(-)
diff --git a/arch/powerpc/platforms/powernv/npu-dma.c b/arch/powerpc/platforms/powernv/npu-dma.c
index 8006c54..c8f438a 100644
--- a/arch/powerpc/platforms/powernv/npu-dma.c
+++ b/arch/powerpc/platforms/powernv/npu-dma.c
@@ -454,79 +454,76 @@ static void put_mmio_atsd_reg(struct npu *npu, int reg)
}
/* MMIO ATSD register offsets */
-#define XTS_ATSD_AVA 1
-#define XTS_ATSD_STAT 2
+#define XTS_ATSD_LAUNCH 0
+#define XTS_ATSD_AVA 1
+#define XTS_ATSD_STAT 2
-static void mmio_launch_invalidate(struct mmio_atsd_reg *mmio_atsd_reg,
- unsigned long launch, unsigned long va)
+static unsigned long get_atsd_launch_val(unsigned long pid, unsigned long psize,
+ bool flush)
{
- struct npu *npu = mmio_atsd_reg->npu;
- int reg = mmio_atsd_reg->reg;
+ unsigned long launch = 0;
- __raw_writeq_be(va, npu->mmio_atsd_regs[reg] + XTS_ATSD_AVA);
- eieio();
- __raw_writeq_be(launch, npu->mmio_atsd_regs[reg]);
+ if (psize == MMU_PAGE_COUNT) {
+ /* IS set to invalidate entire matching PID */
+ launch |= PPC_BIT(12);
+ } else {
+ /* AP set to invalidate region of psize */
+ launch |= (u64)mmu_get_ap(psize) << PPC_BITLSHIFT(17);
+ }
+
+ /* PRS set to process-scoped */
+ launch |= PPC_BIT(13);
+
+ /* PID */
+ launch |= pid << PPC_BITLSHIFT(38);
+
+ /* No flush */
+ launch |= !flush << PPC_BITLSHIFT(39);
+
+ return launch;
}
-static void mmio_invalidate_pid(struct mmio_atsd_reg mmio_atsd_reg[NV_MAX_NPUS],
- unsigned long pid, bool flush)
+static void mmio_atsd_regs_write(struct mmio_atsd_reg
+ mmio_atsd_reg[NV_MAX_NPUS], unsigned long offset,
+ unsigned long val)
{
- int i;
- unsigned long launch;
+ struct npu *npu;
+ int i, reg;
for (i = 0; i <= max_npu2_index; i++) {
- if (mmio_atsd_reg[i].reg < 0)
+ reg = mmio_atsd_reg[i].reg;
+ if (reg < 0)
continue;
- /* IS set to invalidate matching PID */
- launch = PPC_BIT(12);
-
- /* PRS set to process-scoped */
- launch |= PPC_BIT(13);
-
- /* AP */
- launch |= (u64)
- mmu_get_ap(mmu_virtual_psize) << PPC_BITLSHIFT(17);
-
- /* PID */
- launch |= pid << PPC_BITLSHIFT(38);
+ npu = mmio_atsd_reg[i].npu;
+ __raw_writeq_be(val, npu->mmio_atsd_regs[reg] + offset);
+ }
+}
- /* No flush */
- launch |= !flush << PPC_BITLSHIFT(39);
+static void mmio_invalidate_pid(struct mmio_atsd_reg mmio_atsd_reg[NV_MAX_NPUS],
+ unsigned long pid, bool flush)
+{
+ unsigned long launch = get_atsd_launch_val(pid, MMU_PAGE_COUNT, flush);
- /* Invalidating the entire process doesn't use a va */
- mmio_launch_invalidate(&mmio_atsd_reg[i], launch, 0);
- }
+ /* Invalidating the entire process doesn't use a va */
+ mmio_atsd_regs_write(mmio_atsd_reg, XTS_ATSD_LAUNCH, launch);
}
static void mmio_invalidate_va(struct mmio_atsd_reg mmio_atsd_reg[NV_MAX_NPUS],
unsigned long va, unsigned long pid, bool flush)
{
- int i;
unsigned long launch;
- for (i = 0; i <= max_npu2_index; i++) {
- if (mmio_atsd_reg[i].reg < 0)
- continue;
-
- /* IS set to invalidate target VA */
- launch = 0;
+ launch = get_atsd_launch_val(pid, mmu_virtual_psize, flush);
- /* PRS set to process scoped */
- launch |= PPC_BIT(13);
+ /* Write all VAs first */
+ mmio_atsd_regs_write(mmio_atsd_reg, XTS_ATSD_AVA, va);
- /* AP */
- launch |= (u64)
- mmu_get_ap(mmu_virtual_psize) << PPC_BITLSHIFT(17);
-
- /* PID */
- launch |= pid << PPC_BITLSHIFT(38);
-
- /* No flush */
- launch |= !flush << PPC_BITLSHIFT(39);
+ /* Issue one barrier for all address writes */
+ eieio();
- mmio_launch_invalidate(&mmio_atsd_reg[i], launch, va);
- }
+ /* Launch */
+ mmio_atsd_regs_write(mmio_atsd_reg, XTS_ATSD_LAUNCH, launch);
}
#define mn_to_npu_context(x) container_of(x, struct npu_context, mn)
--
1.7.2.5
^ permalink raw reply related
* [PATCH v2 3/3] powerpc/powernv/npu: Remove atsd_threshold debugfs setting
From: Mark Hairgrove @ 2018-10-03 18:51 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Alistair Popple, Mark Hairgrove, Reza Arbab
In-Reply-To: <1538592694-18739-1-git-send-email-mhairgrove@nvidia.com>
This threshold is no longer used now that all invalidates issue a single
ATSD to each active NPU.
Signed-off-by: Mark Hairgrove <mhairgrove@nvidia.com>
---
arch/powerpc/platforms/powernv/npu-dma.c | 14 --------------
1 files changed, 0 insertions(+), 14 deletions(-)
diff --git a/arch/powerpc/platforms/powernv/npu-dma.c b/arch/powerpc/platforms/powernv/npu-dma.c
index e4c0fab..6f60e09 100644
--- a/arch/powerpc/platforms/powernv/npu-dma.c
+++ b/arch/powerpc/platforms/powernv/npu-dma.c
@@ -17,7 +17,6 @@
#include <linux/pci.h>
#include <linux/memblock.h>
#include <linux/iommu.h>
-#include <linux/debugfs.h>
#include <linux/sizes.h>
#include <asm/debugfs.h>
@@ -43,14 +42,6 @@
static DEFINE_SPINLOCK(npu_context_lock);
/*
- * When an address shootdown range exceeds this threshold we invalidate the
- * entire TLB on the GPU for the given PID rather than each specific address in
- * the range.
- */
-static uint64_t atsd_threshold = 2 * 1024 * 1024;
-static struct dentry *atsd_threshold_dentry;
-
-/*
* Other types of TCE cache invalidation are not functional in the
* hardware.
*/
@@ -966,11 +957,6 @@ int pnv_npu2_init(struct pnv_phb *phb)
static int npu_index;
uint64_t rc = 0;
- if (!atsd_threshold_dentry) {
- atsd_threshold_dentry = debugfs_create_x64("atsd_threshold",
- 0600, powerpc_debugfs_root, &atsd_threshold);
- }
-
phb->npu.nmmu_flush =
of_property_read_bool(phb->hose->dn, "ibm,nmmu-flush");
for_each_child_of_node(phb->hose->dn, dn) {
--
1.7.2.5
^ permalink raw reply related
* [PATCH] selftests/powerpc: Add check for TM SPRS on coredump
From: Breno Leitao @ 2018-10-03 21:31 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Breno Leitao, mikey, gromero
Add a selftest to check if the TM SPRs are being properly saved into a
coredump. The segfault is caused by an illegal instruction and ideally it
happens after load_tm overflowed and TM became lazily disabled.
This test is implemented basically setting three TM_SPR and sleeping until
load_tm is expected to be zero. Then it causes a segfault and open the
coredump to check if the SPRs saved in the coredump notes section contain
the same values that were set by the test.
This test needs root privileges in order to change the coredump file
pattern (/proc/sys/kernel/core_pattern).
Signed-off-by: Breno Leitao <leitao@debian.org>
Signed-off-by: Gustavo Romero <gromero@linux.vnet.ibm.com>
---
tools/testing/selftests/powerpc/tm/Makefile | 3 +-
tools/testing/selftests/powerpc/tm/tm-core.c | 480 +++++++++++++++++++
tools/testing/selftests/powerpc/tm/tm.h | 4 +
3 files changed, 486 insertions(+), 1 deletion(-)
create mode 100644 tools/testing/selftests/powerpc/tm/tm-core.c
diff --git a/tools/testing/selftests/powerpc/tm/Makefile b/tools/testing/selftests/powerpc/tm/Makefile
index c0e45d2dde25..b06a58fc6f79 100644
--- a/tools/testing/selftests/powerpc/tm/Makefile
+++ b/tools/testing/selftests/powerpc/tm/Makefile
@@ -4,7 +4,7 @@ SIGNAL_CONTEXT_CHK_TESTS := tm-signal-context-chk-gpr tm-signal-context-chk-fpu
TEST_GEN_PROGS := tm-resched-dscr tm-syscall tm-signal-msr-resv tm-signal-stack \
tm-vmxcopy tm-fork tm-tar tm-tmspr tm-vmx-unavail tm-unavailable tm-trap \
- $(SIGNAL_CONTEXT_CHK_TESTS) tm-sigreturn
+ $(SIGNAL_CONTEXT_CHK_TESTS) tm-sigreturn tm-core
include ../../lib.mk
@@ -19,6 +19,7 @@ $(OUTPUT)/tm-vmx-unavail: CFLAGS += -pthread -m64
$(OUTPUT)/tm-resched-dscr: ../pmu/lib.c
$(OUTPUT)/tm-unavailable: CFLAGS += -O0 -pthread -m64 -Wno-error=uninitialized -mvsx
$(OUTPUT)/tm-trap: CFLAGS += -O0 -pthread -m64
+$(OUTPUT)/tm-core: CFLAGS += -pthread
SIGNAL_CONTEXT_CHK_TESTS := $(patsubst %,$(OUTPUT)/%,$(SIGNAL_CONTEXT_CHK_TESTS))
$(SIGNAL_CONTEXT_CHK_TESTS): tm-signal.S
diff --git a/tools/testing/selftests/powerpc/tm/tm-core.c b/tools/testing/selftests/powerpc/tm/tm-core.c
new file mode 100644
index 000000000000..b01a3a1ae1bf
--- /dev/null
+++ b/tools/testing/selftests/powerpc/tm/tm-core.c
@@ -0,0 +1,480 @@
+// SPDX-License-Identifier: GPL-2.0
+
+/*
+ * Copyright 2018, Breno Leitao, Gustavo Romero, IBM Corp.
+ * Licensed under GPLv2.
+ *
+ * Test case that sets TM SPR and sleeps, waiting kernel load_tm to be
+ * zero. Then causes a segfault to generate a core dump file that will be
+ * analyzed. The coredump needs to have the same HTM SPR values set
+ * initially for a successful test.
+ */
+
+#define _GNU_SOURCE
+#include <assert.h>
+#include <error.h>
+#include <elf.h>
+#include <fcntl.h>
+#include <inttypes.h>
+#include <pthread.h>
+#include <sched.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#include <sys/mman.h>
+#include <sys/stat.h>
+#include <sys/resource.h>
+#include <sys/time.h>
+#include <sys/types.h>
+#include <sys/wait.h>
+#include <linux/kernel.h>
+
+#include "utils.h"
+#include "tm.h"
+
+/* Default time that causes load_tm = 0 on P8/pseries */
+#define DEFAULT_SLEEP_TIME 0x00d0000000
+
+/* MAX string length for string concatenation */
+#define LEN_MAX 1024
+
+/* Maximum coredump file size */
+#define CORE_FILE_LIMIT (5 * 1024 * 1024)
+
+/* Name of the coredump file to be generated */
+#define COREDUMPFILE "core-tm-spr"
+
+/* Function that returns concatenated strings */
+#define COREDUMP(suffix) (COREDUMPFILE#suffix)
+
+/* File to change the coredump file name pattern */
+#define CORE_PATTERN_FILE "/proc/sys/kernel/core_pattern"
+
+/* Logging macros */
+#define err_at_line(status, errnum, format, ...) \
+ error_at_line(status, errnum, __FILE__, __LINE__, format ##__VA_ARGS__)
+#define pr_err(code, format, ...) err_at_line(1, code, format, ##__VA_ARGS__)
+
+/* Child PID */
+static pid_t child;
+
+/* pthread attribute for both ping and pong threads */
+static pthread_attr_t attr;
+
+/* SPR values to be written to TM SPRs and verified later */
+const unsigned long texasr = 0x31;
+const unsigned long tfiar = 0xdeadbeef0;
+const unsigned long tfhar = 0xbaadf00d0;
+
+struct tm_sprs {
+ unsigned long texasr;
+ unsigned long tfhar;
+ unsigned long tfiar;
+};
+
+struct coremem {
+ void *p;
+ off_t len;
+};
+
+/* Set the process limits to be able to create a coredump file */
+static int increase_core_file_limit(void)
+{
+ struct rlimit rlim;
+ int ret;
+
+ ret = getrlimit(RLIMIT_CORE, &rlim);
+ if (ret != 0) {
+ pr_err(ret, "getrlimit CORE failed\n");
+ return -1;
+ }
+
+ 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);
+ if (ret != 0) {
+ pr_err(ret, "setrlimit CORE failed\n");
+ return -1;
+ }
+ }
+
+ ret = getrlimit(RLIMIT_FSIZE, &rlim);
+ if (ret != 0) {
+ pr_err(ret, "getrlimit FSIZE failed\n");
+ return -1;
+ }
+
+ 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);
+ if (ret != 0) {
+ pr_err(ret, "setrlimit FSIZE failed\n");
+ return -1;
+ }
+ }
+
+ return 0;
+}
+
+/*
+ * Set pattern for coredump file name. It returns current pattern being
+ * used as 'old' if old != NULL
+ */
+static int write_core_pattern(const char *core_pattern, char *old)
+{
+ FILE *f;
+ size_t len = strlen(core_pattern), ret;
+
+ f = fopen(CORE_PATTERN_FILE, "r+");
+ if (!f) {
+ perror("Error writing to core_pattern file");
+ return -1;
+ }
+
+ /* Skip saving old value */
+ if (old != NULL) {
+ ret = fread(old, 1, LEN_MAX, f);
+ if (!ret) {
+ perror("Error reading core_pattern file");
+ fclose(f);
+ return -1;
+ }
+ rewind(f);
+ }
+
+ ret = fwrite(core_pattern, 1, len, f);
+
+ fclose(f);
+
+ if (ret != len) {
+ perror("Error writing to core_pattern file");
+ return -1;
+ }
+
+ return 0;
+}
+
+
+/* Thread to force context switch. Will die when the test is done */
+static void __attribute__((noreturn)) *tm_core_pong(void *not_used)
+{
+ while (1)
+ sched_yield();
+}
+
+/* Sleep in user space waiting for load_tm to reach zero */
+static void wait_lazy(unsigned long counter)
+{
+ asm volatile (
+ "mtctr %[counter] ;"
+ "1: bdnz 1b ;"
+ :
+ : [counter] "r" (counter)
+ :
+ );
+}
+
+/*
+ * This function will fork, and the child will set the SPRs and sleep
+ * expecting load_tm to be zero. After a while, it will segfault and
+ * generate a core dump. The parent process just wait the child to die
+ * before continuing.
+ */
+static void *sleep_and_dump(void *time)
+{
+ int status;
+ unsigned long t = *(unsigned long *)time;
+
+ /* Fork, and the child process will sleep and die */
+ child = fork();
+ if (child < 0) {
+ pr_err(child, "fork failure");
+ } else if (child == 0) {
+ /* Set TM SPRS to be checked later */
+ mtspr(SPRN_TFIAR, tfiar);
+ mtspr(SPRN_TFHAR, tfhar);
+ mtspr(SPRN_TEXASR, texasr);
+
+ /*
+ * Sleep in userspace. Not sleeping with
+ * sleep()/nanosleep() because we want to continue to do
+ * context switch, incrementing load_tm until it overflows
+ */
+ wait_lazy(t);
+
+ /*
+ * Cause a segfault and coredump. Can not call any syscalls,
+ * which will reload load_tm due to 'tabort.' being executed
+ * before each syscall by some glibc versions.
+ */
+ asm(".long 0x0");
+ }
+
+ /* Only parent will continue here */
+ waitpid(child, &status, 0);
+ if (!WCOREDUMP(status)) {
+ pr_err(status, "Core dump not generated.");
+ return (void *) -1;
+ }
+
+ return NULL;
+}
+
+
+/* Speed up load_tm overflow with this thread */
+static int start_pong_thread(void)
+{
+ int ret;
+ pthread_t t1;
+ cpu_set_t cpuset;
+
+ CPU_ZERO(&cpuset);
+ CPU_SET(0, &cpuset);
+
+ /* Init pthread attribute. */
+ ret = pthread_attr_init(&attr);
+ if (ret) {
+ pr_err(ret, "pthread_attr_init()");
+ return ret;
+ }
+
+ ret = pthread_attr_setaffinity_np(&attr, sizeof(cpu_set_t), &cpuset);
+ if (ret) {
+ pr_err(ret, "pthread_attr_setaffinity_np()");
+ return ret;
+ }
+
+ ret = pthread_create(&t1, &attr /* CPU 0 */, tm_core_pong, NULL);
+ if (ret) {
+ pr_err(ret, "pthread_create()");
+ return ret;
+ }
+
+ return 0;
+}
+
+
+/* Main test thread */
+static int start_main_thread(unsigned long t)
+{
+ pthread_t t0;
+ void *ret_value;
+ int ret, rc = 0;
+ char old_core_pattern[LEN_MAX];
+
+ ret = increase_core_file_limit();
+ if (ret)
+ return ret;
+
+ /* Change the name of the core dump file */
+ ret = write_core_pattern(COREDUMP(.%p), old_core_pattern);
+ if (ret) {
+ pr_err(ret, "Not able to change core pattern. Are you root!?");
+ return -1;
+ }
+
+ ret = pthread_create(&t0, &attr, sleep_and_dump, &t);
+ if (ret) {
+ pr_err(ret, "pthread_create()");
+ /* Not returning because core_pattern should be restored */
+ rc = ret;
+ }
+
+ ret = pthread_join(t0, &ret_value);
+ if (ret || ret_value != NULL) {
+ pr_err(ret, "sleep_and_dump didn't finished successfully");
+ rc += ret;
+ }
+
+ /* Restore old core pattern to the original value */
+ ret = write_core_pattern(old_core_pattern, NULL);
+ if (ret != 0) {
+ pr_err(ret, "/proc/sys/kernel/core_pattern not restored properly");
+ rc += ret;
+ }
+
+ return rc;
+}
+
+/* Open coredump file and return it mapped into memory */
+static void open_coredump(struct coremem *c)
+{
+ struct stat buf;
+ int fd; int ret;
+ void *core;
+ off_t core_size;
+
+ char coredump[LEN_MAX];
+
+ /* default return value */
+ c->p = NULL;
+
+ sprintf(coredump, COREDUMP(.%d), child);
+
+ fd = open(coredump, O_RDONLY);
+ if (fd == -1)
+ perror("Error opening core file");
+
+ ret = stat(coredump, &buf);
+ if (ret == -1) {
+ printf("Coredump does not exist!\n");
+ return;
+ }
+ core_size = buf.st_size;
+
+ core = mmap(NULL, core_size, PROT_READ, MAP_PRIVATE, fd, 0);
+ if (core == MAP_FAILED) {
+ perror("Error mmaping core file");
+ return;
+ }
+ c->p = core;
+ c->len = core_size;
+}
+
+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);
+}
+
+/* Parse elf in memory and return TM SPRS values */
+static void parse_elf(Elf64_Ehdr *ehdr, struct tm_sprs *ret)
+{
+ void *p = ehdr;
+ Elf64_Phdr *phdr;
+ Elf64_Nhdr *nhdr;
+ size_t phdr_size;
+ unsigned long *regs, *note;
+
+ assert(memcmp(ehdr->e_ident, ELFMAG, SELFMAG) == 0);
+
+ assert(ehdr->e_type == ET_CORE);
+ assert(ehdr->e_machine == EM_PPC64);
+ assert(ehdr->e_phoff != 0 || ehdr->e_phnum != 0);
+
+ phdr_size = sizeof(*phdr) * ehdr->e_phnum;
+
+ for (phdr = p + ehdr->e_phoff;
+ (void *) phdr < p + ehdr->e_phoff + phdr_size;
+ phdr += ehdr->e_phentsize)
+ /* Stop at NOTES section type */
+ if (phdr->p_type == PT_NOTE)
+ break;
+
+ for (nhdr = p + phdr->p_offset;
+ (void *) nhdr < p + phdr->p_offset + phdr->p_filesz;
+ nhdr = next_note(nhdr))
+ /* Stop at TM SPR segment */
+ if (nhdr->n_type == NT_PPC_TM_SPR)
+ break;
+
+ assert(nhdr->n_descsz != 0);
+
+ p = nhdr;
+ note = p + sizeof(*nhdr) + __ALIGN_KERNEL(nhdr->n_namesz, 4);
+ regs = (unsigned long *) note;
+
+ ret->texasr = regs[1];
+ ret->tfhar = regs[0];
+ ret->tfiar = regs[2];
+}
+
+static int check_return_value(struct tm_sprs *s)
+{
+ if ((s->texasr == texasr) &&
+ (s->tfiar == tfiar) &&
+ (s->tfhar == tfhar)) {
+ return 0;
+ }
+
+ /* Corrupted values detected */
+ printf("Detected one or more TM SPR corrupted:\n");
+ printf("TFIAR : %016lx vs %016lx\n", s->texasr, texasr);
+ printf("TEXASR: %016lx vs %016lx\n", s->tfiar, tfiar);
+ printf("TFHAR : %016lx vs %016lx\n", s->tfhar, tfhar);
+
+ return 1;
+}
+
+/* Remove coredump file generated by this test */
+static int clear_coredump(void)
+{
+ char file[LEN_MAX];
+ int ret;
+
+ sprintf(file, COREDUMP(.%d), child);
+ ret = remove(file);
+ if (ret != 0)
+ perror("Not able to remove coredump file");
+
+ return ret;
+}
+
+/* Main function */
+static int tm_core_test(void)
+{
+ unsigned long time = DEFAULT_SLEEP_TIME;
+ struct tm_sprs sprs;
+ struct coremem mem;
+ int ret;
+
+ /* Skip if TM is not available */
+ SKIP_IF(!have_htm());
+
+ /*
+ * Skip if not root. Could not change the default coredump name
+ * pattern
+ */
+ SKIP_IF(geteuid() != 0);
+
+ printf("Sleeping for %lu cycles\n", time);
+ ret = start_pong_thread();
+ if (ret != 0)
+ return ret;
+
+ ret = start_main_thread(time);
+ if (ret != 0)
+ return ret;
+
+ open_coredump(&mem);
+ if (mem.p == NULL) {
+ /* if open_coredump failed, mem.p returns NULL */
+ pr_err(1, "Open coredump file failed");
+ return -1;
+ }
+
+ parse_elf(mem.p, &sprs);
+
+ /* unmap memory allocated in open_coredump() */
+ munmap(mem.p, mem.len);
+
+ ret = clear_coredump();
+ if (ret != 0)
+ return ret;
+
+ ret = check_return_value(&sprs);
+ if (ret == 0)
+ printf("Success!\n");
+ else
+ printf("Failure!\n");
+
+ return ret;
+}
+
+int main(int argc, char **argv)
+{
+ return test_harness(tm_core_test, "tm_core_test");
+}
diff --git a/tools/testing/selftests/powerpc/tm/tm.h b/tools/testing/selftests/powerpc/tm/tm.h
index df4204247d45..cfe2ead9d366 100644
--- a/tools/testing/selftests/powerpc/tm/tm.h
+++ b/tools/testing/selftests/powerpc/tm/tm.h
@@ -12,6 +12,10 @@
#include "utils.h"
+#ifndef NT_PPC_TM_SPR
+#define NT_PPC_TM_SPR 0x10c
+#endif
+
static inline bool have_htm(void)
{
#ifdef PPC_FEATURE2_HTM
--
2.19.0
^ permalink raw reply related
* Re: [PATCH] migration/mm: Add WARN_ON to try_offline_node
From: Tyrel Datwyler @ 2018-10-03 23:05 UTC (permalink / raw)
To: Michael Bringmann, Tyrel Datwyler, Michal Hocko
Cc: Thomas Falcon, Kees Cook, Mathieu Malaterre, linux-kernel,
Nicholas Piggin, Pavel Tatashin, linux-mm,
Mauricio Faria de Oliveira, Juliet Kim, Thiago Jung Bauermann,
Nathan Fontenot, Andrew Morton, YASUAKI ISHIMATSU, linuxppc-dev,
Dan Williams, Oscar Salvador
In-Reply-To: <17781f9e-abfb-8c1e-eb18-39571d1b5cd6@linux.vnet.ibm.com>
On 10/03/2018 06:27 AM, Michael Bringmann wrote:
> On 10/02/2018 02:45 PM, Tyrel Datwyler wrote:
>> On 10/02/2018 11:13 AM, Michael Bringmann wrote:
>>>
>>>
>>> On 10/02/2018 11:04 AM, Michal Hocko wrote:
>>>> On Tue 02-10-18 10:14:49, Michael Bringmann wrote:
>>>>> On 10/02/2018 09:59 AM, Michal Hocko wrote:
>>>>>> On Tue 02-10-18 09:51:40, Michael Bringmann wrote:
>>>>>> [...]
>>>>>>> When the device-tree affinity attributes have changed for memory,
>>>>>>> the 'nid' affinity calculated points to a different node for the
>>>>>>> memory block than the one used to install it, previously on the
>>>>>>> source system. The newly calculated 'nid' affinity may not yet
>>>>>>> be initialized on the target system. The current memory tracking
>>>>>>> mechanisms do not record the node to which a memory block was
>>>>>>> associated when it was added. Nathan is looking at adding this
>>>>>>> feature to the new implementation of LMBs, but it is not there
>>>>>>> yet, and won't be present in earlier kernels without backporting a
>>>>>>> significant number of changes.
>>>>>>
>>>>>> Then the patch you have proposed here just papers over a real issue, no?
>>>>>> IIUC then you simply do not remove the memory if you lose the race.
>>>>>
>>>>> The problem occurs when removing memory after an affinity change
>>>>> references a node that was previously unreferenced. Other code
>>>>> in 'kernel/mm/memory_hotplug.c' deals with initializing an empty
>>>>> node when adding memory to a system. The 'removing memory' case is
>>>>> specific to systems that perform LPM and allow device-tree changes.
>>>>> The powerpc kernel does not have the option of accepting some PRRN
>>>>> requests and accepting others. It must perform them all.
>>>>
>>>> I am sorry, but you are still too cryptic for me. Either there is a
>>>> correctness issue and the the patch doesn't really fix anything or the
>>>> final race doesn't make any difference and then the ppc code should be
>>>> explicit about that. Checking the node inside the hotplug core code just
>>>> looks as a wrong layer to mitigate an arch specific problem. I am not
>>>> saying the patch is a no-go but if anything we want a big fat comment
>>>> explaining how this is possible because right now it just points to an
>>>> incorrect API usage.
>>>>
>>>> That being said, this sounds pretty much ppc specific problem and I
>>>> would _prefer_ it to be handled there (along with a big fat comment of
>>>> course).
>>>
>>> Let me try again. Regardless of the path to which we get to this condition,
>>> we currently crash the kernel. This patch changes that to a WARN_ON notice
>>> and continues executing the kernel without shutting down the system. I saw
>>> the problem during powerpc testing, because that is the focus of my work.
>>> There are other paths to this function besides powerpc. I feel that the
>>> kernel should keep running instead of halting.
>>
>> This is still basically a hack to get around a known race. In itself this patch is still worth while in that we shouldn't crash the kernel on a null pointer dereference. However, I think the actual problem still needs to be addressed. We shouldn't run any PRRN events for the source system on the target after a migration. The device tree update should have taken care of telling us about new affinities and what not. Can we just throw out any queued PRRN events when we wake up on the target?
>
> We are not talking about queued events provided on the source system, but about
> new PRRN events sent by phyp to the kernel on the target system to update the
> kernel state after migration. No way to predict the content.
Okay, but either way shouldn't your other proposed patches to update memory affinity by re-adding memory and changing the time topology updates are stopped to include the post-mobility updates put things in the right nodes? Or, am I missing something? I would assume a PRRN on the target would assume the target was up-to-date with respect to where things are supposed to be located.
-Tyrel
>
>>
>> -Tyrel
>>>
>>> Regards,
>>>
>
> Michael
>
^ permalink raw reply
* Re: [PATCH 5/5] dma-direct: always allow dma mask <= physiscal memory size
From: Alexander Duyck @ 2018-10-03 23:10 UTC (permalink / raw)
To: Christoph Hellwig, Guenter Roeck
Cc: Robin Murphy, LKML, open list:INTEL IOMMU (VT-d), Greg KH,
open list:LINUX FOR POWERPC (32-BIT AND 64-BIT)
In-Reply-To: <20180927223539.28449-6-hch@lst.de>
On Thu, Sep 27, 2018 at 3:38 PM Christoph Hellwig <hch@lst.de> wrote:
>
> This way an architecture with less than 4G of RAM can support dma_mask
> smaller than 32-bit without a ZONE_DMA. Apparently that is a common
> case on powerpc.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Reviewed-by: Robin Murphy <robin.murphy@arm.com>
> ---
> kernel/dma/direct.c | 28 ++++++++++++++++------------
> 1 file changed, 16 insertions(+), 12 deletions(-)
>
> diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
> index 60c433b880e0..170bd322a94a 100644
> --- a/kernel/dma/direct.c
> +++ b/kernel/dma/direct.c
> @@ -284,21 +284,25 @@ int dma_direct_map_sg(struct device *dev, struct scatterlist *sgl, int nents,
> return nents;
> }
>
> +/*
> + * Because 32-bit DMA masks are so common we expect every architecture to be
> + * able to satisfy them - either by not supporting more physical memory, or by
> + * providing a ZONE_DMA32. If neither is the case, the architecture needs to
> + * use an IOMMU instead of the direct mapping.
> + */
> int dma_direct_supported(struct device *dev, u64 mask)
> {
> -#ifdef CONFIG_ZONE_DMA
> - if (mask < phys_to_dma(dev, DMA_BIT_MASK(ARCH_ZONE_DMA_BITS)))
> - return 0;
> -#else
> - /*
> - * Because 32-bit DMA masks are so common we expect every architecture
> - * to be able to satisfy them - either by not supporting more physical
> - * memory, or by providing a ZONE_DMA32. If neither is the case, the
> - * architecture needs to use an IOMMU instead of the direct mapping.
> - */
> - if (mask < phys_to_dma(dev, DMA_BIT_MASK(32)))
> + u64 min_mask;
> +
> + if (IS_ENABLED(CONFIG_ZONE_DMA))
> + min_mask = DMA_BIT_MASK(ARCH_ZONE_DMA_BITS);
> + else
> + min_mask = DMA_BIT_MASK(32);
> +
> + min_mask = min_t(u64, min_mask, (max_pfn - 1) << PAGE_SHIFT);
> +
> + if (mask >= phys_to_dma(dev, min_mask))
> return 0;
> -#endif
> return 1;
> }
So I believe I have run into the same issue that Guenter reported. On
an x86_64 system w/ Intel IOMMU. I wasn't able to complete boot and
all probe attempts for various devices were failing with -EIO errors.
I believe the last mask check should be "if (mask < phys_to_dma(dev,
min_mask))" not a ">=" check.
- Alex
^ permalink raw reply
* [PATCH] dma-direct: Fix return value of dma_direct_supported
From: Alexander Duyck @ 2018-10-03 23:48 UTC (permalink / raw)
To: iommu; +Cc: robin.murphy, linux-kernel, gregkh, linuxppc-dev, linux
It appears that in commit 9d7a224b463e ("dma-direct: always allow dma mask
<= physiscal memory size") the logic of the test was changed from a "<" to
a ">=" however I don't see any reason for that change. I am assuming that
there was some additional change planned, specifically I suspect the logic
was intended to be reversed and possibly used for a return. Since that is
the case I have gone ahead and done that.
This addresses issues I had on my system that prevented me from booting
with the above mentioned commit applied on an x86_64 system w/ Intel IOMMU.
Fixes: 9d7a224b463e ("dma-direct: always allow dma mask <= physiscal memory size")
Signed-off-by: Alexander Duyck <alexander.h.duyck@linux.intel.com>
---
kernel/dma/direct.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
index 5a0806b5351b..65872f6c2e93 100644
--- a/kernel/dma/direct.c
+++ b/kernel/dma/direct.c
@@ -301,9 +301,7 @@ int dma_direct_supported(struct device *dev, u64 mask)
min_mask = min_t(u64, min_mask, (max_pfn - 1) << PAGE_SHIFT);
- if (mask >= phys_to_dma(dev, min_mask))
- return 0;
- return 1;
+ return mask >= phys_to_dma(dev, min_mask);
}
int dma_direct_mapping_error(struct device *dev, dma_addr_t dma_addr)
^ permalink raw reply related
* Re: [PATCH] migration/mm: Add WARN_ON to try_offline_node
From: Michael Bringmann @ 2018-10-04 1:02 UTC (permalink / raw)
To: Tyrel Datwyler, Tyrel Datwyler, Michal Hocko
Cc: Thomas Falcon, Kees Cook, Mathieu Malaterre, Pavel Tatashin,
Nicholas Piggin, linux-kernel, linux-mm,
Mauricio Faria de Oliveira, Juliet Kim, Thiago Jung Bauermann,
Nathan Fontenot, Andrew Morton, YASUAKI ISHIMATSU, linuxppc-dev,
Dan Williams, Oscar Salvador
In-Reply-To: <e7c0f7cc-02a4-47ff-9d7c-0b63f106932e@gmail.com>
On 10/03/2018 06:05 PM, Tyrel Datwyler wrote:
> On 10/03/2018 06:27 AM, Michael Bringmann wrote:
>> On 10/02/2018 02:45 PM, Tyrel Datwyler wrote:
>>> On 10/02/2018 11:13 AM, Michael Bringmann wrote:
>>>>
>>>>
>>>> On 10/02/2018 11:04 AM, Michal Hocko wrote:
>>>>> On Tue 02-10-18 10:14:49, Michael Bringmann wrote:
>>>>>> On 10/02/2018 09:59 AM, Michal Hocko wrote:
>>>>>>> On Tue 02-10-18 09:51:40, Michael Bringmann wrote:
>>>>>>> [...]
>>>>>>>> When the device-tree affinity attributes have changed for memory,
>>>>>>>> the 'nid' affinity calculated points to a different node for the
>>>>>>>> memory block than the one used to install it, previously on the
>>>>>>>> source system. The newly calculated 'nid' affinity may not yet
>>>>>>>> be initialized on the target system. The current memory tracking
>>>>>>>> mechanisms do not record the node to which a memory block was
>>>>>>>> associated when it was added. Nathan is looking at adding this
>>>>>>>> feature to the new implementation of LMBs, but it is not there
>>>>>>>> yet, and won't be present in earlier kernels without backporting a
>>>>>>>> significant number of changes.
>>>>>>>
>>>>>>> Then the patch you have proposed here just papers over a real issue, no?
>>>>>>> IIUC then you simply do not remove the memory if you lose the race.
>>>>>>
>>>>>> The problem occurs when removing memory after an affinity change
>>>>>> references a node that was previously unreferenced. Other code
>>>>>> in 'kernel/mm/memory_hotplug.c' deals with initializing an empty
>>>>>> node when adding memory to a system. The 'removing memory' case is
>>>>>> specific to systems that perform LPM and allow device-tree changes.
>>>>>> The powerpc kernel does not have the option of accepting some PRRN
>>>>>> requests and accepting others. It must perform them all.
>>>>>
>>>>> I am sorry, but you are still too cryptic for me. Either there is a
>>>>> correctness issue and the the patch doesn't really fix anything or the
>>>>> final race doesn't make any difference and then the ppc code should be
>>>>> explicit about that. Checking the node inside the hotplug core code just
>>>>> looks as a wrong layer to mitigate an arch specific problem. I am not
>>>>> saying the patch is a no-go but if anything we want a big fat comment
>>>>> explaining how this is possible because right now it just points to an
>>>>> incorrect API usage.
>>>>>
>>>>> That being said, this sounds pretty much ppc specific problem and I
>>>>> would _prefer_ it to be handled there (along with a big fat comment of
>>>>> course).
>>>>
>>>> Let me try again. Regardless of the path to which we get to this condition,
>>>> we currently crash the kernel. This patch changes that to a WARN_ON notice
>>>> and continues executing the kernel without shutting down the system. I saw
>>>> the problem during powerpc testing, because that is the focus of my work.
>>>> There are other paths to this function besides powerpc. I feel that the
>>>> kernel should keep running instead of halting.
>>>
>>> This is still basically a hack to get around a known race. In itself this patch is still worth while in that we shouldn't crash the kernel on a null pointer dereference. However, I think the actual problem still needs to be addressed. We shouldn't run any PRRN events for the source system on the target after a migration. The device tree update should have taken care of telling us about new affinities and what not. Can we just throw out any queued PRRN events when we wake up on the target?
>>
>> We are not talking about queued events provided on the source system, but about
>> new PRRN events sent by phyp to the kernel on the target system to update the
>> kernel state after migration. No way to predict the content.
>
> Okay, but either way shouldn't your other proposed patches to update memory affinity by re-adding memory and changing the time topology updates are stopped to include the post-mobility updates put things in the right nodes? Or, am I missing something? I would assume a PRRN on the target would assume the target was up-to-date with respect to where things are supposed to be located.
This bug only recently came to our attention in a defect on a SLES12 SP3 platform running PHYP Memory Mover concurrently with LPM. Not a normal test case.
>
> -Tyrel
Michael
--
Michael W. Bringmann
Linux Technology Center
IBM Corporation
Tie-Line 363-5196
External: (512) 286-5196
Cell: (512) 466-0650
mwb@linux.vnet.ibm.com
^ permalink raw reply
* Re: [PATCH v03 1/5] powerpc/drmem: Export 'dynamic-memory' loader
From: Nathan Fontenot @ 2018-10-04 2:24 UTC (permalink / raw)
To: Michael Ellerman, Michael Bringmann, linuxppc-dev
Cc: Juliet Kim, Thomas Falcon, Tyrel Datwyler
In-Reply-To: <87y3bfkfnx.fsf@concordia.ellerman.id.au>
On 10/02/2018 08:00 PM, Michael Ellerman wrote:
> Michael Bringmann <mwb@linux.vnet.ibm.com> writes:
>
>> powerpc/drmem: Export many of the functions of DRMEM to parse
>> "ibm,dynamic-memory" and "ibm,dynamic-memory-v2" during hotplug
>> operations and for Post Migration events.
>
> This isn't a criticism of your patch, but I think the drmem.c code
> should be moved into platforms/pseries.
>
> That would then make most of it private to platforms/pseries and we
> wouldn't need to export things in arch/powerpc/include/asm.
I don't have an issue with moving it to platform/pseries. I originally
put it in arch/powerpc/mm because the numa code also uses the drmem code.
The numa code was updated so that it could just be given a lmb struct
instead of having to parse the device tree directly for dynamic
reconfiguration memory. Having to support two versions of this dt
property this made more sense.
-Nathan
>
>
>> Also modify the DRMEM initialization code to allow it to,
>>
>> * Be called after system initialization
>> * Provide a separate user copy of the LMB array that is produces
>> * Free the user copy upon request
>
> Is there any reason those can't be done as separate patches?
>
>> In addition, a couple of changes were made to make the creation
>> of additional copies of the LMB array more useful including,
>>
>> * Add new iterator to work through a pair of drmem_info arrays.
>> * Modify DRMEM code to replace usages of dt_root_addr_cells, and
>> dt_mem_next_cell, as these are only available at first boot.
>
> Likewise?
>
> cheers
>
>> diff --git a/arch/powerpc/include/asm/drmem.h b/arch/powerpc/include/asm/drmem.h
>> index ce242b9..b0e70fd 100644
>> --- a/arch/powerpc/include/asm/drmem.h
>> +++ b/arch/powerpc/include/asm/drmem.h
>> @@ -35,6 +35,18 @@ struct drmem_lmb_info {
>> &drmem_info->lmbs[0], \
>> &drmem_info->lmbs[drmem_info->n_lmbs - 1])
>>
>> +#define for_each_dinfo_lmb(dinfo, lmb) \
>> + for_each_drmem_lmb_in_range((lmb), \
>> + &dinfo->lmbs[0], \
>> + &dinfo->lmbs[dinfo->n_lmbs - 1])
>> +
>> +#define for_each_pair_dinfo_lmb(dinfo1, lmb1, dinfo2, lmb2) \
>> + for ((lmb1) = (&dinfo1->lmbs[0]), \
>> + (lmb2) = (&dinfo2->lmbs[0]); \
>> + ((lmb1) <= (&dinfo1->lmbs[dinfo1->n_lmbs - 1])) && \
>> + ((lmb2) <= (&dinfo2->lmbs[dinfo2->n_lmbs - 1])); \
>> + (lmb1)++, (lmb2)++)
>> +
>> /*
>> * The of_drconf_cell_v1 struct defines the layout of the LMB data
>> * specified in the ibm,dynamic-memory device tree property.
>> @@ -94,6 +106,9 @@ void __init walk_drmem_lmbs(struct device_node *dn,
>> void (*func)(struct drmem_lmb *, const __be32 **));
>> int drmem_update_dt(void);
>>
>> +struct drmem_lmb_info *drmem_lmbs_init(struct property *prop);
>> +void drmem_lmbs_free(struct drmem_lmb_info *dinfo);
>> +
>> #ifdef CONFIG_PPC_PSERIES
>> void __init walk_drmem_lmbs_early(unsigned long node,
>> void (*func)(struct drmem_lmb *, const __be32 **));
>> diff --git a/arch/powerpc/mm/drmem.c b/arch/powerpc/mm/drmem.c
>> index 3f18036..13d2abb 100644
>> --- a/arch/powerpc/mm/drmem.c
>> +++ b/arch/powerpc/mm/drmem.c
>> @@ -20,6 +20,7 @@
>>
>> static struct drmem_lmb_info __drmem_info;
>> struct drmem_lmb_info *drmem_info = &__drmem_info;
>> +static int n_root_addr_cells;
>>
>> u64 drmem_lmb_memory_max(void)
>> {
>> @@ -193,12 +194,13 @@ int drmem_update_dt(void)
>> return rc;
>> }
>>
>> -static void __init read_drconf_v1_cell(struct drmem_lmb *lmb,
>> +static void read_drconf_v1_cell(struct drmem_lmb *lmb,
>> const __be32 **prop)
>> {
>> const __be32 *p = *prop;
>>
>> - lmb->base_addr = dt_mem_next_cell(dt_root_addr_cells, &p);
>> + lmb->base_addr = of_read_number(p, n_root_addr_cells);
>> + p += n_root_addr_cells;
>> lmb->drc_index = of_read_number(p++, 1);
>>
>> p++; /* skip reserved field */
>> @@ -209,7 +211,7 @@ static void __init read_drconf_v1_cell(struct drmem_lmb *lmb,
>> *prop = p;
>> }
>>
>> -static void __init __walk_drmem_v1_lmbs(const __be32 *prop, const __be32 *usm,
>> +static void __walk_drmem_v1_lmbs(const __be32 *prop, const __be32 *usm,
>> void (*func)(struct drmem_lmb *, const __be32 **))
>> {
>> struct drmem_lmb lmb;
>> @@ -225,13 +227,14 @@ static void __init __walk_drmem_v1_lmbs(const __be32 *prop, const __be32 *usm,
>> }
>> }
>>
>> -static void __init read_drconf_v2_cell(struct of_drconf_cell_v2 *dr_cell,
>> +static void read_drconf_v2_cell(struct of_drconf_cell_v2 *dr_cell,
>> const __be32 **prop)
>> {
>> const __be32 *p = *prop;
>>
>> dr_cell->seq_lmbs = of_read_number(p++, 1);
>> - dr_cell->base_addr = dt_mem_next_cell(dt_root_addr_cells, &p);
>> + dr_cell->base_addr = of_read_number(p, n_root_addr_cells);
>> + p += n_root_addr_cells;
>> dr_cell->drc_index = of_read_number(p++, 1);
>> dr_cell->aa_index = of_read_number(p++, 1);
>> dr_cell->flags = of_read_number(p++, 1);
>> @@ -239,7 +242,7 @@ static void __init read_drconf_v2_cell(struct of_drconf_cell_v2 *dr_cell,
>> *prop = p;
>> }
>>
>> -static void __init __walk_drmem_v2_lmbs(const __be32 *prop, const __be32 *usm,
>> +static void __walk_drmem_v2_lmbs(const __be32 *prop, const __be32 *usm,
>> void (*func)(struct drmem_lmb *, const __be32 **))
>> {
>> struct of_drconf_cell_v2 dr_cell;
>> @@ -275,6 +278,9 @@ void __init walk_drmem_lmbs_early(unsigned long node,
>> const __be32 *prop, *usm;
>> int len;
>>
>> + if (n_root_addr_cells == 0)
>> + n_root_addr_cells = dt_root_addr_cells;
>> +
>> prop = of_get_flat_dt_prop(node, "ibm,lmb-size", &len);
>> if (!prop || len < dt_root_size_cells * sizeof(__be32))
>> return;
>> @@ -353,24 +359,26 @@ void __init walk_drmem_lmbs(struct device_node *dn,
>> }
>> }
>>
>> -static void __init init_drmem_v1_lmbs(const __be32 *prop)
>> +static void init_drmem_v1_lmbs(const __be32 *prop,
>> + struct drmem_lmb_info *dinfo)
>> {
>> struct drmem_lmb *lmb;
>>
>> - drmem_info->n_lmbs = of_read_number(prop++, 1);
>> - if (drmem_info->n_lmbs == 0)
>> + dinfo->n_lmbs = of_read_number(prop++, 1);
>> + if (dinfo->n_lmbs == 0)
>> return;
>>
>> - drmem_info->lmbs = kcalloc(drmem_info->n_lmbs, sizeof(*lmb),
>> + dinfo->lmbs = kcalloc(dinfo->n_lmbs, sizeof(*lmb),
>> GFP_KERNEL);
>> - if (!drmem_info->lmbs)
>> + if (!dinfo->lmbs)
>> return;
>>
>> - for_each_drmem_lmb(lmb)
>> + for_each_dinfo_lmb(dinfo, lmb)
>> read_drconf_v1_cell(lmb, &prop);
>> }
>>
>> -static void __init init_drmem_v2_lmbs(const __be32 *prop)
>> +static void init_drmem_v2_lmbs(const __be32 *prop,
>> + struct drmem_lmb_info *dinfo)
>> {
>> struct drmem_lmb *lmb;
>> struct of_drconf_cell_v2 dr_cell;
>> @@ -386,12 +394,12 @@ static void __init init_drmem_v2_lmbs(const __be32 *prop)
>> p = prop;
>> for (i = 0; i < lmb_sets; i++) {
>> read_drconf_v2_cell(&dr_cell, &p);
>> - drmem_info->n_lmbs += dr_cell.seq_lmbs;
>> + dinfo->n_lmbs += dr_cell.seq_lmbs;
>> }
>>
>> - drmem_info->lmbs = kcalloc(drmem_info->n_lmbs, sizeof(*lmb),
>> + dinfo->lmbs = kcalloc(dinfo->n_lmbs, sizeof(*lmb),
>> GFP_KERNEL);
>> - if (!drmem_info->lmbs)
>> + if (!dinfo->lmbs)
>> return;
>>
>> /* second pass, read in the LMB information */
>> @@ -402,10 +410,10 @@ static void __init init_drmem_v2_lmbs(const __be32 *prop)
>> read_drconf_v2_cell(&dr_cell, &p);
>>
>> for (j = 0; j < dr_cell.seq_lmbs; j++) {
>> - lmb = &drmem_info->lmbs[lmb_index++];
>> + lmb = &dinfo->lmbs[lmb_index++];
>>
>> lmb->base_addr = dr_cell.base_addr;
>> - dr_cell.base_addr += drmem_info->lmb_size;
>> + dr_cell.base_addr += dinfo->lmb_size;
>>
>> lmb->drc_index = dr_cell.drc_index;
>> dr_cell.drc_index++;
>> @@ -416,11 +424,38 @@ static void __init init_drmem_v2_lmbs(const __be32 *prop)
>> }
>> }
>>
>> +void drmem_lmbs_free(struct drmem_lmb_info *dinfo)
>> +{
>> + if (dinfo) {
>> + kfree(dinfo->lmbs);
>> + kfree(dinfo);
>> + }
>> +}
>> +
>> +struct drmem_lmb_info *drmem_lmbs_init(struct property *prop)
>> +{
>> + struct drmem_lmb_info *dinfo;
>> +
>> + dinfo = kzalloc(sizeof(*dinfo), GFP_KERNEL);
>> + if (!dinfo)
>> + return NULL;
>> +
>> + if (!strcmp("ibm,dynamic-memory", prop->name))
>> + init_drmem_v1_lmbs(prop->value, dinfo);
>> + else if (!strcmp("ibm,dynamic-memory-v2", prop->name))
>> + init_drmem_v2_lmbs(prop->value, dinfo);
>> +
>> + return dinfo;
>> +}
>> +
>> static int __init drmem_init(void)
>> {
>> struct device_node *dn;
>> const __be32 *prop;
>>
>> + if (n_root_addr_cells == 0)
>> + n_root_addr_cells = dt_root_addr_cells;
>> +
>> dn = of_find_node_by_path("/ibm,dynamic-reconfiguration-memory");
>> if (!dn) {
>> pr_info("No dynamic reconfiguration memory found\n");
>> @@ -434,11 +469,11 @@ static int __init drmem_init(void)
>>
>> prop = of_get_property(dn, "ibm,dynamic-memory", NULL);
>> if (prop) {
>> - init_drmem_v1_lmbs(prop);
>> + init_drmem_v1_lmbs(prop, drmem_info);
>> } else {
>> prop = of_get_property(dn, "ibm,dynamic-memory-v2", NULL);
>> if (prop)
>> - init_drmem_v2_lmbs(prop);
>> + init_drmem_v2_lmbs(prop, drmem_info);
>> }
>>
>> of_node_put(dn);
>
^ permalink raw reply
* Re: [PATCH v3 23/33] KVM: PPC: Book3S HV: Introduce rmap to track nested guest mappings
From: Paul Mackerras @ 2018-10-04 3:05 UTC (permalink / raw)
To: David Gibson; +Cc: linuxppc-dev, kvm-ppc, kvm
In-Reply-To: <20181003055637.GS1886@umbus.fritz.box>
On Wed, Oct 03, 2018 at 03:56:37PM +1000, David Gibson wrote:
> On Tue, Oct 02, 2018 at 09:31:22PM +1000, Paul Mackerras wrote:
> > From: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
> >
> > When a host (L0) page which is mapped into a (L1) guest is in turn
> > mapped through to a nested (L2) guest we keep a reverse mapping (rmap)
> > so that these mappings can be retrieved later.
> >
> > Whenever we create an entry in a shadow_pgtable for a nested guest we
> > create a corresponding rmap entry and add it to the list for the
> > L1 guest memslot at the index of the L1 guest page it maps. This means
> > at the L1 guest memslot we end up with lists of rmaps.
> >
> > When we are notified of a host page being invalidated which has been
> > mapped through to a (L1) guest, we can then walk the rmap list for that
> > guest page, and find and invalidate all of the corresponding
> > shadow_pgtable entries.
> >
> > In order to reduce memory consumption, we compress the information for
> > each rmap entry down to 52 bits -- 12 bits for the LPID and 40 bits
> > for the guest real page frame number -- which will fit in a single
> > unsigned long. To avoid a scenario where a guest can trigger
> > unbounded memory allocations, we scan the list when adding an entry to
> > see if there is already an entry with the contents we need. This can
> > occur, because we don't ever remove entries from the middle of a list.
> >
> > A struct nested guest rmap is a list pointer and an rmap entry;
> > ----------------
> > | next pointer |
> > ----------------
> > | rmap entry |
> > ----------------
> >
> > Thus the rmap pointer for each guest frame number in the memslot can be
> > either NULL, a single entry, or a pointer to a list of nested rmap entries.
> >
> > gfn memslot rmap array
> > -------------------------
> > 0 | NULL | (no rmap entry)
> > -------------------------
> > 1 | single rmap entry | (rmap entry with low bit set)
> > -------------------------
> > 2 | list head pointer | (list of rmap entries)
> > -------------------------
> >
> > The final entry always has the lowest bit set and is stored in the next
> > pointer of the last list entry, or as a single rmap entry.
> > With a list of rmap entries looking like;
> >
> > ----------------- ----------------- -------------------------
> > | list head ptr | ----> | next pointer | ----> | single rmap entry |
> > ----------------- ----------------- -------------------------
> > | rmap entry | | rmap entry |
> > ----------------- -------------------------
> >
> > Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
> > Signed-off-by: Paul Mackerras <paulus@ozlabs.org>
> > ---
> > arch/powerpc/include/asm/kvm_book3s.h | 3 +
> > arch/powerpc/include/asm/kvm_book3s_64.h | 70 ++++++++++++++++-
> > arch/powerpc/kvm/book3s_64_mmu_radix.c | 44 +++++++----
> > arch/powerpc/kvm/book3s_hv.c | 1 +
> > arch/powerpc/kvm/book3s_hv_nested.c | 130 ++++++++++++++++++++++++++++++-
> > 5 files changed, 233 insertions(+), 15 deletions(-)
> >
> > diff --git a/arch/powerpc/include/asm/kvm_book3s.h b/arch/powerpc/include/asm/kvm_book3s.h
> > index d983778..1d2286d 100644
> > --- a/arch/powerpc/include/asm/kvm_book3s.h
> > +++ b/arch/powerpc/include/asm/kvm_book3s.h
> > @@ -196,6 +196,9 @@ extern int kvmppc_mmu_radix_translate_table(struct kvm_vcpu *vcpu, gva_t eaddr,
> > int table_index, u64 *pte_ret_p);
> > extern int kvmppc_mmu_radix_xlate(struct kvm_vcpu *vcpu, gva_t eaddr,
> > struct kvmppc_pte *gpte, bool data, bool iswrite);
> > +extern void kvmppc_unmap_pte(struct kvm *kvm, pte_t *pte, unsigned long gpa,
> > + unsigned int shift, struct kvm_memory_slot *memslot,
> > + unsigned int lpid);
> > extern bool kvmppc_hv_handle_set_rc(struct kvm *kvm, pgd_t *pgtable,
> > bool writing, unsigned long gpa,
> > unsigned int lpid);
> > diff --git a/arch/powerpc/include/asm/kvm_book3s_64.h b/arch/powerpc/include/asm/kvm_book3s_64.h
> > index 5496152..38614f0 100644
> > --- a/arch/powerpc/include/asm/kvm_book3s_64.h
> > +++ b/arch/powerpc/include/asm/kvm_book3s_64.h
> > @@ -53,6 +53,66 @@ struct kvm_nested_guest {
> > struct kvm_nested_guest *next;
> > };
> >
> > +/*
> > + * We define a nested rmap entry as a single 64-bit quantity
> > + * 0xFFF0000000000000 12-bit lpid field
> > + * 0x000FFFFFFFFFF000 40-bit guest physical address field
>
> I thought we could potentially support guests with >1TiB of RAM..?
We can, that's really a (4k) page frame number, not a physical
address. We can support 52-bit guest physical addresses.
Paul.
^ permalink raw reply
* Re: [PATCH v3 30/33] KVM: PPC: Book3S HV: Allow HV module to load without hypervisor mode
From: Paul Mackerras @ 2018-10-04 3:03 UTC (permalink / raw)
To: David Gibson; +Cc: linuxppc-dev, kvm-ppc, kvm
In-Reply-To: <20181003061514.GW1886@umbus.fritz.box>
On Wed, Oct 03, 2018 at 04:15:15PM +1000, David Gibson wrote:
> On Tue, Oct 02, 2018 at 09:31:29PM +1000, Paul Mackerras wrote:
> > With this, the KVM-HV module can be loaded in a guest running under
> > KVM-HV, and if the hypervisor supports nested virtualization, this
> > guest can now act as a nested hypervisor and run nested guests.
> >
> > This also adds some checks to inform userspace that HPT guests are not
> > supported by nested hypervisors, and to prevent userspace from
> > configuring a guest to use HPT mode.
> >
> > Signed-off-by: Paul Mackerras <paulus@ozlabs.org>
> > ---
> > arch/powerpc/kvm/book3s_hv.c | 20 ++++++++++++++++----
> > 1 file changed, 16 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> > index f630e91..196bff1 100644
> > --- a/arch/powerpc/kvm/book3s_hv.c
> > +++ b/arch/powerpc/kvm/book3s_hv.c
> > @@ -4237,6 +4237,10 @@ static int kvm_vm_ioctl_get_smmu_info_hv(struct kvm *kvm,
> > {
> > struct kvm_ppc_one_seg_page_size *sps;
> >
> > + /* If we're a nested hypervisor, we only support radix guests */
> > + if (kvmhv_on_pseries())
> > + return -EINVAL;
> > +
> > /*
> > * POWER7, POWER8 and POWER9 all support 32 storage keys for data.
> > * POWER7 doesn't support keys for instruction accesses,
> > @@ -4822,11 +4826,15 @@ static int kvmppc_core_emulate_mfspr_hv(struct kvm_vcpu *vcpu, int sprn,
> >
> > static int kvmppc_core_check_processor_compat_hv(void)
> > {
> > - if (!cpu_has_feature(CPU_FTR_HVMODE) ||
> > - !cpu_has_feature(CPU_FTR_ARCH_206))
> > - return -EIO;
> > + if (cpu_has_feature(CPU_FTR_HVMODE) &&
> > + cpu_has_feature(CPU_FTR_ARCH_206))
> > + return 0;
> >
> > - return 0;
> > + /* Can run as nested hypervisor on POWER9 in radix mode. */
> > + if (cpu_has_feature(CPU_FTR_ARCH_300) && radix_enabled())
>
> Shouldn't we probe the parent hypervisor for ability to support nested
> guests before we say "yes" here?
Well, we do check that the parent hypervisor can support nested
hypervisors, it's just done later on. And to match nitpick with
nitpick, this is a function evaluating _processor_ compatibility, and
a POWER9 processor in radix mode does have everything necessary to
support nested hypervisors -- if the parent hypervisor doesn't support
nested hypervisors, that's not a deficiency in the processor.
Paul.
^ permalink raw reply
* Re: [PATCH v2 1/3] powerpc/powernv/npu: Reduce eieio usage when issuing ATSD invalidates
From: Alistair Popple @ 2018-10-04 5:20 UTC (permalink / raw)
To: Mark Hairgrove; +Cc: linuxppc-dev, Reza Arbab
In-Reply-To: <1538592694-18739-2-git-send-email-mhairgrove@nvidia.com>
Reviewed-by: Alistair Popple <alistair@popple.id.au>
On Wednesday, 3 October 2018 11:51:32 AM AEST Mark Hairgrove wrote:
> There are two types of ATSDs issued to the NPU: invalidates targeting a
> specific virtual address and invalidates targeting the whole address
> space. In both cases prior to this change, the sequence was:
>
> for each NPU
> - Write the target address to the XTS_ATSD_AVA register
> - EIEIO
> - Write the launch value to issue the ATSD
>
> First, a target address is not required when invalidating the whole
> address space, so that write and the EIEIO have been removed. The AP
> (size) field in the launch is not needed either.
>
> Second, for per-address invalidates the above sequence is inefficient in
> the common case of multiple NPUs because an EIEIO is issued per NPU. This
> unnecessarily forces the launches of later ATSDs to be ordered with the
> launches of earlier ones. The new sequence only issues a single EIEIO:
>
> for each NPU
> - Write the target address to the XTS_ATSD_AVA register
> EIEIO
> for each NPU
> - Write the launch value to issue the ATSD
>
> Performance results were gathered using a microbenchmark which creates a
> 1G allocation then uses mprotect with PROT_NONE to trigger invalidates in
> strides across the allocation.
>
> With only a single NPU active (one GPU) the difference is in the noise for
> both types of invalidates (+/-1%).
>
> With two NPUs active (on a 6-GPU system) the effect is more noticeable:
>
> mprotect rate (GB/s)
> Stride Before After Speedup
> 64K 5.9 6.5 10%
> 1M 31.2 33.4 7%
> 2M 36.3 38.7 7%
> 4M 322.6 356.7 11%
>
> Signed-off-by: Mark Hairgrove <mhairgrove@nvidia.com>
> ---
> arch/powerpc/platforms/powernv/npu-dma.c | 99 ++++++++++++++---------------
> 1 files changed, 48 insertions(+), 51 deletions(-)
>
> diff --git a/arch/powerpc/platforms/powernv/npu-dma.c b/arch/powerpc/platforms/powernv/npu-dma.c
> index 8006c54..c8f438a 100644
> --- a/arch/powerpc/platforms/powernv/npu-dma.c
> +++ b/arch/powerpc/platforms/powernv/npu-dma.c
> @@ -454,79 +454,76 @@ static void put_mmio_atsd_reg(struct npu *npu, int reg)
> }
>
> /* MMIO ATSD register offsets */
> -#define XTS_ATSD_AVA 1
> -#define XTS_ATSD_STAT 2
> +#define XTS_ATSD_LAUNCH 0
> +#define XTS_ATSD_AVA 1
> +#define XTS_ATSD_STAT 2
>
> -static void mmio_launch_invalidate(struct mmio_atsd_reg *mmio_atsd_reg,
> - unsigned long launch, unsigned long va)
> +static unsigned long get_atsd_launch_val(unsigned long pid, unsigned long psize,
> + bool flush)
> {
> - struct npu *npu = mmio_atsd_reg->npu;
> - int reg = mmio_atsd_reg->reg;
> + unsigned long launch = 0;
>
> - __raw_writeq_be(va, npu->mmio_atsd_regs[reg] + XTS_ATSD_AVA);
> - eieio();
> - __raw_writeq_be(launch, npu->mmio_atsd_regs[reg]);
> + if (psize == MMU_PAGE_COUNT) {
> + /* IS set to invalidate entire matching PID */
> + launch |= PPC_BIT(12);
> + } else {
> + /* AP set to invalidate region of psize */
> + launch |= (u64)mmu_get_ap(psize) << PPC_BITLSHIFT(17);
> + }
> +
> + /* PRS set to process-scoped */
> + launch |= PPC_BIT(13);
> +
> + /* PID */
> + launch |= pid << PPC_BITLSHIFT(38);
> +
> + /* No flush */
> + launch |= !flush << PPC_BITLSHIFT(39);
> +
> + return launch;
> }
>
> -static void mmio_invalidate_pid(struct mmio_atsd_reg mmio_atsd_reg[NV_MAX_NPUS],
> - unsigned long pid, bool flush)
> +static void mmio_atsd_regs_write(struct mmio_atsd_reg
> + mmio_atsd_reg[NV_MAX_NPUS], unsigned long offset,
> + unsigned long val)
> {
> - int i;
> - unsigned long launch;
> + struct npu *npu;
> + int i, reg;
>
> for (i = 0; i <= max_npu2_index; i++) {
> - if (mmio_atsd_reg[i].reg < 0)
> + reg = mmio_atsd_reg[i].reg;
> + if (reg < 0)
> continue;
>
> - /* IS set to invalidate matching PID */
> - launch = PPC_BIT(12);
> -
> - /* PRS set to process-scoped */
> - launch |= PPC_BIT(13);
> -
> - /* AP */
> - launch |= (u64)
> - mmu_get_ap(mmu_virtual_psize) << PPC_BITLSHIFT(17);
> -
> - /* PID */
> - launch |= pid << PPC_BITLSHIFT(38);
> + npu = mmio_atsd_reg[i].npu;
> + __raw_writeq_be(val, npu->mmio_atsd_regs[reg] + offset);
> + }
> +}
>
> - /* No flush */
> - launch |= !flush << PPC_BITLSHIFT(39);
> +static void mmio_invalidate_pid(struct mmio_atsd_reg mmio_atsd_reg[NV_MAX_NPUS],
> + unsigned long pid, bool flush)
> +{
> + unsigned long launch = get_atsd_launch_val(pid, MMU_PAGE_COUNT, flush);
>
> - /* Invalidating the entire process doesn't use a va */
> - mmio_launch_invalidate(&mmio_atsd_reg[i], launch, 0);
> - }
> + /* Invalidating the entire process doesn't use a va */
> + mmio_atsd_regs_write(mmio_atsd_reg, XTS_ATSD_LAUNCH, launch);
> }
>
> static void mmio_invalidate_va(struct mmio_atsd_reg mmio_atsd_reg[NV_MAX_NPUS],
> unsigned long va, unsigned long pid, bool flush)
> {
> - int i;
> unsigned long launch;
>
> - for (i = 0; i <= max_npu2_index; i++) {
> - if (mmio_atsd_reg[i].reg < 0)
> - continue;
> -
> - /* IS set to invalidate target VA */
> - launch = 0;
> + launch = get_atsd_launch_val(pid, mmu_virtual_psize, flush);
>
> - /* PRS set to process scoped */
> - launch |= PPC_BIT(13);
> + /* Write all VAs first */
> + mmio_atsd_regs_write(mmio_atsd_reg, XTS_ATSD_AVA, va);
>
> - /* AP */
> - launch |= (u64)
> - mmu_get_ap(mmu_virtual_psize) << PPC_BITLSHIFT(17);
> -
> - /* PID */
> - launch |= pid << PPC_BITLSHIFT(38);
> -
> - /* No flush */
> - launch |= !flush << PPC_BITLSHIFT(39);
> + /* Issue one barrier for all address writes */
> + eieio();
>
> - mmio_launch_invalidate(&mmio_atsd_reg[i], launch, va);
> - }
> + /* Launch */
> + mmio_atsd_regs_write(mmio_atsd_reg, XTS_ATSD_LAUNCH, launch);
> }
>
> #define mn_to_npu_context(x) container_of(x, struct npu_context, mn)
>
^ permalink raw reply
* Re: [PATCH v2 2/3] powerpc/powernv/npu: Use size-based ATSD invalidates
From: Alistair Popple @ 2018-10-04 5:20 UTC (permalink / raw)
To: Mark Hairgrove; +Cc: linuxppc-dev, Reza Arbab
In-Reply-To: <1538592694-18739-3-git-send-email-mhairgrove@nvidia.com>
Reviewed-By: Alistair Popple <alistair@popple.id.au>
On Wednesday, 3 October 2018 11:51:33 AM AEST Mark Hairgrove wrote:
> Prior to this change only two types of ATSDs were issued to the NPU:
> invalidates targeting a single page and invalidates targeting the whole
> address space. The crossover point happened at the configurable
> atsd_threshold which defaulted to 2M. Invalidates that size or smaller
> would issue per-page invalidates for the whole range.
>
> The NPU supports more invalidation sizes however: 64K, 2M, 1G, and all.
> These invalidates target addresses aligned to their size. 2M is a common
> invalidation size for GPU-enabled applications because that is a GPU
> page size, so reducing the number of invalidates by 32x in that case is a
> clear improvement.
>
> ATSD latency is high in general so now we always issue a single invalidate
> rather than multiple. This will over-invalidate in some cases, but for any
> invalidation size over 2M it matches or improves the prior behavior.
> There's also an improvement for single-page invalidates since the prior
> version issued two invalidates for that case instead of one.
>
> With this change all issued ATSDs now perform a flush, so the flush
> parameter has been removed from all the helpers.
>
> To show the benefit here are some performance numbers from a
> microbenchmark which creates a 1G allocation then uses mprotect with
> PROT_NONE to trigger invalidates in strides across the allocation.
>
> One NPU (1 GPU):
>
> mprotect rate (GB/s)
> Stride Before After Speedup
> 64K 5.3 5.6 5%
> 1M 39.3 57.4 46%
> 2M 49.7 82.6 66%
> 4M 286.6 285.7 0%
>
> Two NPUs (6 GPUs):
>
> mprotect rate (GB/s)
> Stride Before After Speedup
> 64K 6.5 7.4 13%
> 1M 33.4 67.9 103%
> 2M 38.7 93.1 141%
> 4M 356.7 354.6 -1%
>
> Anything over 2M is roughly the same as before since both cases issue a
> single ATSD.
>
> Signed-off-by: Mark Hairgrove <mhairgrove@nvidia.com>
> ---
> arch/powerpc/platforms/powernv/npu-dma.c | 103 ++++++++++++++++--------------
> 1 files changed, 55 insertions(+), 48 deletions(-)
>
> diff --git a/arch/powerpc/platforms/powernv/npu-dma.c b/arch/powerpc/platforms/powernv/npu-dma.c
> index c8f438a..e4c0fab 100644
> --- a/arch/powerpc/platforms/powernv/npu-dma.c
> +++ b/arch/powerpc/platforms/powernv/npu-dma.c
> @@ -18,6 +18,7 @@
> #include <linux/memblock.h>
> #include <linux/iommu.h>
> #include <linux/debugfs.h>
> +#include <linux/sizes.h>
>
> #include <asm/debugfs.h>
> #include <asm/tlb.h>
> @@ -458,8 +459,7 @@ static void put_mmio_atsd_reg(struct npu *npu, int reg)
> #define XTS_ATSD_AVA 1
> #define XTS_ATSD_STAT 2
>
> -static unsigned long get_atsd_launch_val(unsigned long pid, unsigned long psize,
> - bool flush)
> +static unsigned long get_atsd_launch_val(unsigned long pid, unsigned long psize)
> {
> unsigned long launch = 0;
>
> @@ -477,8 +477,7 @@ static unsigned long get_atsd_launch_val(unsigned long pid, unsigned long psize,
> /* PID */
> launch |= pid << PPC_BITLSHIFT(38);
>
> - /* No flush */
> - launch |= !flush << PPC_BITLSHIFT(39);
> + /* Leave "No flush" (bit 39) 0 so every ATSD performs a flush */
>
> return launch;
> }
> @@ -501,23 +500,22 @@ static void mmio_atsd_regs_write(struct mmio_atsd_reg
> }
>
> static void mmio_invalidate_pid(struct mmio_atsd_reg mmio_atsd_reg[NV_MAX_NPUS],
> - unsigned long pid, bool flush)
> + unsigned long pid)
> {
> - unsigned long launch = get_atsd_launch_val(pid, MMU_PAGE_COUNT, flush);
> + unsigned long launch = get_atsd_launch_val(pid, MMU_PAGE_COUNT);
>
> /* Invalidating the entire process doesn't use a va */
> mmio_atsd_regs_write(mmio_atsd_reg, XTS_ATSD_LAUNCH, launch);
> }
>
> -static void mmio_invalidate_va(struct mmio_atsd_reg mmio_atsd_reg[NV_MAX_NPUS],
> - unsigned long va, unsigned long pid, bool flush)
> +static void mmio_invalidate_range(struct mmio_atsd_reg
> + mmio_atsd_reg[NV_MAX_NPUS], unsigned long pid,
> + unsigned long start, unsigned long psize)
> {
> - unsigned long launch;
> -
> - launch = get_atsd_launch_val(pid, mmu_virtual_psize, flush);
> + unsigned long launch = get_atsd_launch_val(pid, psize);
>
> /* Write all VAs first */
> - mmio_atsd_regs_write(mmio_atsd_reg, XTS_ATSD_AVA, va);
> + mmio_atsd_regs_write(mmio_atsd_reg, XTS_ATSD_AVA, start);
>
> /* Issue one barrier for all address writes */
> eieio();
> @@ -609,14 +607,36 @@ static void release_atsd_reg(struct mmio_atsd_reg mmio_atsd_reg[NV_MAX_NPUS])
> }
>
> /*
> - * Invalidate either a single address or an entire PID depending on
> - * the value of va.
> + * Invalidate a virtual address range
> */
> -static void mmio_invalidate(struct npu_context *npu_context, int va,
> - unsigned long address, bool flush)
> +static void mmio_invalidate(struct npu_context *npu_context,
> + unsigned long start, unsigned long size)
> {
> struct mmio_atsd_reg mmio_atsd_reg[NV_MAX_NPUS];
> unsigned long pid = npu_context->mm->context.id;
> + unsigned long atsd_start = 0;
> + unsigned long end = start + size - 1;
> + int atsd_psize = MMU_PAGE_COUNT;
> +
> + /*
> + * Convert the input range into one of the supported sizes. If the range
> + * doesn't fit, use the next larger supported size. Invalidation latency
> + * is high, so over-invalidation is preferred to issuing multiple
> + * invalidates.
> + *
> + * A 4K page size isn't supported by NPU/GPU ATS, so that case is
> + * ignored.
> + */
> + if (size == SZ_64K) {
> + atsd_start = start;
> + atsd_psize = MMU_PAGE_64K;
> + } else if (ALIGN_DOWN(start, SZ_2M) == ALIGN_DOWN(end, SZ_2M)) {
> + atsd_start = ALIGN_DOWN(start, SZ_2M);
> + atsd_psize = MMU_PAGE_2M;
> + } else if (ALIGN_DOWN(start, SZ_1G) == ALIGN_DOWN(end, SZ_1G)) {
> + atsd_start = ALIGN_DOWN(start, SZ_1G);
> + atsd_psize = MMU_PAGE_1G;
> + }
>
> if (npu_context->nmmu_flush)
> /*
> @@ -631,23 +651,25 @@ static void mmio_invalidate(struct npu_context *npu_context, int va,
> * an invalidate.
> */
> acquire_atsd_reg(npu_context, mmio_atsd_reg);
> - if (va)
> - mmio_invalidate_va(mmio_atsd_reg, address, pid, flush);
> +
> + if (atsd_psize == MMU_PAGE_COUNT)
> + mmio_invalidate_pid(mmio_atsd_reg, pid);
> else
> - mmio_invalidate_pid(mmio_atsd_reg, pid, flush);
> + mmio_invalidate_range(mmio_atsd_reg, pid, atsd_start,
> + atsd_psize);
>
> mmio_invalidate_wait(mmio_atsd_reg);
> - if (flush) {
> - /*
> - * The GPU requires two flush ATSDs to ensure all entries have
> - * been flushed. We use PID 0 as it will never be used for a
> - * process on the GPU.
> - */
> - mmio_invalidate_pid(mmio_atsd_reg, 0, true);
> - mmio_invalidate_wait(mmio_atsd_reg);
> - mmio_invalidate_pid(mmio_atsd_reg, 0, true);
> - mmio_invalidate_wait(mmio_atsd_reg);
> - }
> +
> + /*
> + * The GPU requires two flush ATSDs to ensure all entries have been
> + * flushed. We use PID 0 as it will never be used for a process on the
> + * GPU.
> + */
> + mmio_invalidate_pid(mmio_atsd_reg, 0);
> + mmio_invalidate_wait(mmio_atsd_reg);
> + mmio_invalidate_pid(mmio_atsd_reg, 0);
> + mmio_invalidate_wait(mmio_atsd_reg);
> +
> release_atsd_reg(mmio_atsd_reg);
> }
>
> @@ -664,7 +686,7 @@ static void pnv_npu2_mn_release(struct mmu_notifier *mn,
> * There should be no more translation requests for this PID, but we
> * need to ensure any entries for it are removed from the TLB.
> */
> - mmio_invalidate(npu_context, 0, 0, true);
> + mmio_invalidate(npu_context, 0, ~0UL);
> }
>
> static void pnv_npu2_mn_change_pte(struct mmu_notifier *mn,
> @@ -673,8 +695,7 @@ static void pnv_npu2_mn_change_pte(struct mmu_notifier *mn,
> pte_t pte)
> {
> struct npu_context *npu_context = mn_to_npu_context(mn);
> -
> - mmio_invalidate(npu_context, 1, address, true);
> + mmio_invalidate(npu_context, address, PAGE_SIZE);
> }
>
> static void pnv_npu2_mn_invalidate_range(struct mmu_notifier *mn,
> @@ -682,21 +703,7 @@ static void pnv_npu2_mn_invalidate_range(struct mmu_notifier *mn,
> unsigned long start, unsigned long end)
> {
> struct npu_context *npu_context = mn_to_npu_context(mn);
> - unsigned long address;
> -
> - if (end - start > atsd_threshold) {
> - /*
> - * Just invalidate the entire PID if the address range is too
> - * large.
> - */
> - mmio_invalidate(npu_context, 0, 0, true);
> - } else {
> - for (address = start; address < end; address += PAGE_SIZE)
> - mmio_invalidate(npu_context, 1, address, false);
> -
> - /* Do the flush only on the final addess == end */
> - mmio_invalidate(npu_context, 1, address, true);
> - }
> + mmio_invalidate(npu_context, start, end - start);
> }
>
> static const struct mmu_notifier_ops nv_nmmu_notifier_ops = {
>
^ permalink raw reply
* Re: [PATCH v2 3/3] powerpc/powernv/npu: Remove atsd_threshold debugfs setting
From: Alistair Popple @ 2018-10-04 5:21 UTC (permalink / raw)
To: Mark Hairgrove; +Cc: linuxppc-dev, Reza Arbab
In-Reply-To: <1538592694-18739-4-git-send-email-mhairgrove@nvidia.com>
Reviewed-by: Alistair Popple <alistair@popple.id.au>
On Wednesday, 3 October 2018 11:51:34 AM AEST Mark Hairgrove wrote:
> This threshold is no longer used now that all invalidates issue a single
> ATSD to each active NPU.
>
> Signed-off-by: Mark Hairgrove <mhairgrove@nvidia.com>
> ---
> arch/powerpc/platforms/powernv/npu-dma.c | 14 --------------
> 1 files changed, 0 insertions(+), 14 deletions(-)
>
> diff --git a/arch/powerpc/platforms/powernv/npu-dma.c b/arch/powerpc/platforms/powernv/npu-dma.c
> index e4c0fab..6f60e09 100644
> --- a/arch/powerpc/platforms/powernv/npu-dma.c
> +++ b/arch/powerpc/platforms/powernv/npu-dma.c
> @@ -17,7 +17,6 @@
> #include <linux/pci.h>
> #include <linux/memblock.h>
> #include <linux/iommu.h>
> -#include <linux/debugfs.h>
> #include <linux/sizes.h>
>
> #include <asm/debugfs.h>
> @@ -43,14 +42,6 @@
> static DEFINE_SPINLOCK(npu_context_lock);
>
> /*
> - * When an address shootdown range exceeds this threshold we invalidate the
> - * entire TLB on the GPU for the given PID rather than each specific address in
> - * the range.
> - */
> -static uint64_t atsd_threshold = 2 * 1024 * 1024;
> -static struct dentry *atsd_threshold_dentry;
> -
> -/*
> * Other types of TCE cache invalidation are not functional in the
> * hardware.
> */
> @@ -966,11 +957,6 @@ int pnv_npu2_init(struct pnv_phb *phb)
> static int npu_index;
> uint64_t rc = 0;
>
> - if (!atsd_threshold_dentry) {
> - atsd_threshold_dentry = debugfs_create_x64("atsd_threshold",
> - 0600, powerpc_debugfs_root, &atsd_threshold);
> - }
> -
> phb->npu.nmmu_flush =
> of_property_read_bool(phb->hose->dn, "ibm,nmmu-flush");
> for_each_child_of_node(phb->hose->dn, dn) {
>
^ permalink raw reply
* Re: [PATCH] powerpc: Add doorbell tracepoints
From: Russell Currey @ 2018-10-04 5:41 UTC (permalink / raw)
To: Anton Blanchard, benh, paulus, mpe, npiggin; +Cc: linuxppc-dev
In-Reply-To: <20181003002957.7711-1-anton@ozlabs.org>
On Wed, 2018-10-03 at 10:29 +1000, Anton Blanchard wrote:
> When analysing sources of OS jitter, I noticed that doorbells cannot be
> traced.
>
> Signed-off-by: Anton Blanchard <anton@ozlabs.org>
Hi Anton,
snowpatch builds failed for this patch on all 64-bit configurations (ppc64e, ppc64
and ppc64le) with the following:
arch/powerpc/kernel/dbell.c:85:9: error: undefined identifier
'trace_doorbell_entry'
arch/powerpc/kernel/dbell.c:96:9: error: undefined identifier
'trace_doorbell_exit'
./arch/powerpc/include/asm/spinlock.h:171:9: warning: context imbalance in
'key_user_put' - unexpected unlock
arch/powerpc/kernel/dbell.c: In function 'doorbell_exception':
arch/powerpc/kernel/dbell.c:85:2: error: implicit declaration of function
'trace_doorbell_entry'; did you mean 'trace_irq_entry'? [-Werror=implicit-
function-declaration]
trace_doorbell_entry(regs);
^~~~~~~~~~~~~~~~~~~~
trace_irq_entry
arch/powerpc/kernel/dbell.c:96:2: error: implicit declaration of function
'trace_doorbell_exit'; did you mean 'trace_irq_exit'? [-Werror=implicit-function-
declaration]
trace_doorbell_exit(regs);
^~~~~~~~~~~~~~~~~~~
trace_irq_exit
cc1: all warnings being treated as errors
scripts/Makefile.build:305: recipe for target 'arch/powerpc/kernel/dbell.o' failed
make[1]: *** [arch/powerpc/kernel/dbell.o] Error 1
Makefile:1060: recipe for target 'arch/powerpc/kernel' failed
So does something else need to check for CONFIG_PPC_DOORBELL maybe?
You can see the failures here: http://patchwork.ozlabs.org/patch/978088/ - output
in build_new.log (I know it's not pretty in there yet, you can search for "Error
1" to find the build failure)
- Russell
^ permalink raw reply
* Re: powerpc: cell: use irq_of_parse_and_map helper
From: Michael Ellerman @ 2018-10-04 6:14 UTC (permalink / raw)
To: Rob Herring; +Cc: Paul Mackerras, linuxppc-dev, linux-kernel, Arnd Bergmann
In-Reply-To: <20180104224542.15333-3-robh@kernel.org>
On Thu, 2018-01-04 at 22:45:41 UTC, Rob Herring wrote:
> 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: Arnd Bergmann <arnd@arndb.de>
> 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>
> Acked-by: Arnd Bergmann <arnd@arndb.de>
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/8c8933eba0c2853ecbd6a9ef7542b9
cheers
^ permalink raw reply
* Re: [v2] powerpc: pseries: use of_irq_get helper in request_event_sources_irqs
From: Michael Ellerman @ 2018-10-04 6:14 UTC (permalink / raw)
To: Rob Herring; +Cc: Paul Mackerras, linuxppc-dev, linux-kernel
In-Reply-To: <20180201175922.18930-1-robh@kernel.org>
On Thu, 2018-02-01 at 17:59:22 UTC, Rob Herring wrote:
> Instead of calling both of_irq_parse_one and irq_create_of_mapping, call
> of_irq_get instead which does essentially the same thing. of_irq_get
> also calls irq_find_host for deferred probe support, but this should be
> fine as irq_create_of_mapping also calls that internally. This gets us
> closer to making the former 2 functions static.
>
> In the process of simplifying request_event_sources_irqs, combine the
> the pr_err and WARN_ON calls to just a WARN().
>
> 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>
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/c417596d2409125b1814c05e994a21
cheers
^ permalink raw reply
* Re: [v2] powerpc/tm: Print 64-bits MSR
From: Michael Ellerman @ 2018-10-04 6:14 UTC (permalink / raw)
To: Breno Leitao, linuxppc-dev; +Cc: Breno Leitao, mikey
In-Reply-To: <1533648900-7933-1-git-send-email-leitao@debian.org>
On Tue, 2018-08-07 at 13:35:00 UTC, Breno Leitao wrote:
> On a kernel TM Bad thing program exception, the Machine State Register
> (MSR) is not being properly displayed. The exception code dumps a 32-bits
> value but MSR is a 64 bits register for all platforms that have HTM
> enabled.
>
> This patch dumps the MSR value as a 64-bits value instead of 32 bits. In
> order to do so, the 'reason' variable could not be used, since it trimmed
> MSR to 32-bits (int).
>
> Signed-off-by: Breno Leitao <leitao@debian.org>
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/51303113e32fd92d327b3c441c45e2
cheers
^ permalink raw reply
* Re: powerpc/mm: Don't report hugepage tables as memory leaks when using kmemleak
From: Michael Ellerman @ 2018-10-04 6:14 UTC (permalink / raw)
To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <fa22576baa70fd0cdf8b3f5af3767116a1c680e7.1534164087.git.christophe.leroy@c-s.fr>
On Mon, 2018-08-13 at 13:19:52 UTC, Christophe Leroy wrote:
> When a process allocates a hugepage, the following leak is
> reported by kmemleak. This is a false positive which is
> due to the pointer to the table being stored in the PGD
> as physical memory address and not virtual memory pointer.
>
> unreferenced object 0xc30f8200 (size 512):
> comm "mmap", pid 374, jiffies 4872494 (age 627.630s)
> hex dump (first 32 bytes):
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
> backtrace:
> [<e32b68da>] huge_pte_alloc+0xdc/0x1f8
> [<9e0df1e1>] hugetlb_fault+0x560/0x8f8
> [<7938ec6c>] follow_hugetlb_page+0x14c/0x44c
> [<afbdb405>] __get_user_pages+0x1c4/0x3dc
> [<b8fd7cd9>] __mm_populate+0xac/0x140
> [<3215421e>] vm_mmap_pgoff+0xb4/0xb8
> [<c148db69>] ksys_mmap_pgoff+0xcc/0x1fc
> [<4fcd760f>] ret_from_syscall+0x0/0x38
>
> See commit a984506c542e2 ("powerpc/mm: Don't report PUDs as
> memory leaks when using kmemleak") for detailed explanation.
>
> To fix that, this patch tells kmemleak to ignore the allocated
> hugepage table.
>
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/803d690e68f0c5230183f1a42c7d50
cheers
^ permalink raw reply
* Re: [v3] powerpc/tm: Remove msr_tm_active()
From: Michael Ellerman @ 2018-10-04 6:14 UTC (permalink / raw)
To: Breno Leitao, linuxppc-dev; +Cc: Breno Leitao, mikey
In-Reply-To: <1534440067-5146-1-git-send-email-leitao@debian.org>
On Thu, 2018-08-16 at 17:21:07 UTC, Breno Leitao wrote:
> Currently msr_tm_active() is a wrapper around MSR_TM_ACTIVE() if
> CONFIG_PPC_TRANSACTIONAL_MEM is set, or it is just a function that
> returns false if CONFIG_PPC_TRANSACTIONAL_MEM is not set.
>
> This function is not necessary, since MSR_TM_ACTIVE() just do the same and
> could be used, removing the dualism and simplifying the code.
>
> This patchset remove every instance of msr_tm_active() and replaced it
> by MSR_TM_ACTIVE().
>
> Signed-off-by: Breno Leitao <leitao@debian.org>
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/5c784c8414fba11b62e12439f11e10
cheers
^ permalink raw reply
* Re: powerpc: Convert to using %pOFn instead of device_node.name
From: Michael Ellerman @ 2018-10-04 6:14 UTC (permalink / raw)
To: Rob Herring, linux-kernel; +Cc: linuxppc-dev, Paul Mackerras, Arnd Bergmann
In-Reply-To: <20180828015252.28511-6-robh@kernel.org>
On Tue, 2018-08-28 at 01:52:07 UTC, Rob Herring wrote:
> In preparation to remove the node name pointer from struct device_node,
> convert printf users to use the %pOFn format specifier.
>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: linuxppc-dev@lists.ozlabs.org
> Signed-off-by: Rob Herring <robh@kernel.org>
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/b9ef7b4b867f56114bedbe6bf104cf
cheers
^ permalink raw reply
* Re: [v3] macintosh: Convert to using %pOFn instead of device_node.name
From: Michael Ellerman @ 2018-10-04 6:14 UTC (permalink / raw)
To: Rob Herring, linux-kernel; +Cc: linuxppc-dev
In-Reply-To: <20180904212744.16747-1-robh@kernel.org>
On Tue, 2018-09-04 at 21:27:44 UTC, Rob Herring wrote:
> In preparation to remove the node name pointer from struct device_node,
> convert printf users to use the %pOFn format specifier.
>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: linuxppc-dev@lists.ozlabs.org
> Signed-off-by: Rob Herring <robh@kernel.org>
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/0bdba867f01d69cffefee707504d31
cheers
^ permalink raw reply
* Re: [v3] powerpc/powernv: Make possible for user to force a full ipl cec reboot
From: Michael Ellerman @ 2018-10-04 6:14 UTC (permalink / raw)
To: Vaibhav Jain, Benjamin Herrenschmidt, Stewart Smith
Cc: Michael Neuling, Nicholas Piggin, Vasant Hegde,
Oliver O'Halloran, Andrew Donnellan, Vaibhav Jain,
linuxppc-dev
In-Reply-To: <20180907073448.6908-1-vaibhav@linux.ibm.com>
On Fri, 2018-09-07 at 07:34:48 UTC, Vaibhav Jain wrote:
> Ever since fast reboot is enabled by default in opal,
> opal_cec_reboot() will use fast-reset instead of full IPL to perform
> system reboot. This leaves the user with no direct way to force a full
> IPL reboot except changing an nvram setting that persistently disables
> fast-reset for all subsequent reboots.
>
> This patch provides a more direct way for the user to force a one-shot
> full IPL reboot by passing the command line argument 'full' to the
> reboot command. So the user will be able to tweak the reboot behavior
> via:
>
> $ sudo reboot full # Force a full ipl reboot skipping fast-reset
>
> or
> $ sudo reboot # default reboot path (usually fast-reset)
>
> The reboot command passes the un-parsed command argument to the kernel
> via the 'Reboot' syscall which is then passed on to the arch function
> pnv_restart(). The patch updates pnv_restart() to handle this cmd-arg
> and issues opal_cec_reboot2 with OPAL_REBOOT_FULL_IPL to force a full
> IPL reset.
>
> Signed-off-by: Vaibhav Jain <vaibhav@linux.ibm.com>
> Acked-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/8139046a5a34787849df81f4a5875c
cheers
^ permalink raw reply
* Re: powerpc/perf: Add missing break in power7_marked_instr_event()
From: Michael Ellerman @ 2018-10-04 6:14 UTC (permalink / raw)
To: Michael Ellerman, linuxppc-dev; +Cc: maddy, paulus
In-Reply-To: <20180920094111.4125-1-mpe@ellerman.id.au>
On Thu, 2018-09-20 at 09:41:11 UTC, Michael Ellerman wrote:
> In power7_marked_instr_event() there is a switch case that is missing
> a break or an explicit fallthrough, it's not immediately clear which
> it should be.
>
> The function determines based on the PMU event code, whether the event
> is a "marked" event (which then requires us to configure the PMU in a
> certain way). On Power7 there is no specific bit(s) in the event to
> tell us that, we just have to know.
>
> Rather than having a full list of every event and whether they are
> marked, we pull apart the event code and for events with certain
> values of certain fields we can say that those are all marked events.
>
> We take the psel (bits 0-7) of the event, and look at bits 4-7. For a
> value of 6 we say that if the entire psel == 0x64 then if the pmc == 3
> the event is marked, else not, and otherwise we continue.
>
> It is then that we fallthrough to the 8 case, where we return true if
> the unit == 0xd.
>
> The question is should the 6 case also fallthrough and check for
> unit == 0xd, or should it return.
>
> Looking at the full list of events we see that there are zero events
> where (psel >> 4) == 0x6 and unit == 0xd.
>
> So the answer is it doesn't really matter, there are no valid event
> codes that will return a different result whether we fallthrough or
> break.
>
> But equally, testing the 6 case events against unit == 0xd is slightly
> bogus, as there are no such events. So to make the code clearer, and
> avoid any future confusion, have the 6 case break rather than falling
> through.
>
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
> Reviewed-by: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
Applied to powerpc next.
https://git.kernel.org/powerpc/c/db6711b7a17f03921e734e11e3a1e9
cheers
^ permalink raw reply
* Re: [1/3] powerpc: Redefine TIF_32BITS thread flag
From: Michael Ellerman @ 2018-10-04 6:14 UTC (permalink / raw)
To: Breno Leitao, linuxppc-dev; +Cc: Breno Leitao
In-Reply-To: <1537461907-32003-1-git-send-email-leitao@debian.org>
On Thu, 2018-09-20 at 16:45:05 UTC, Breno Leitao wrote:
> Moving TIF_32BIT to use bit 20 instead of 4 in the task flag field.
>
> This change is making room for an upcoming new task macro
> (_TIF_SYSCALL_EMU) which is preferred to set a bit in the lower 16-bits
> part of the word.
>
> This upcoming flag macro will take part in a composed macro
> (_TIF_SYSCALL_DOTRACE) which will contain other flags as well, and it is
> preferred that the whole _TIF_SYSCALL_DOTRACE macro only sets the lower 16
> bits of a word, so, it could be handled using immediate operations (as load
> immediate, add immediate, ...) where the immediate operand (SI) is limited
> to 16-bits.
>
> Another possible solution would be using the LOAD_REG_IMMEDIATE() macro
> to load a full 64-bits word immediate, but it takes 5 operations instead of
> one.
>
> Having TIF_32BITS being redefined to use an upper bit is not a problem
> since there is only one place in the assembly code where TIF_32BIT is being
> used, and it could be replaced with an operation with right shift (addis),
> since it is used alone, i.e. not being part of a composed macro, which has
> different bits set, and would require LOAD_REG_IMMEDIATE().
>
> Tested on a 64 bits Big Endian machine running a 32 bits task.
>
> Signed-off-by: Breno Leitao <leitao@debian.org>
Series applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/16d7c69c898531210d13dbd1eb2053
cheers
^ permalink raw reply
* Re: [v2] powerpc/traps: merge unrecoverable_exception() and nonrecoverable_exception()
From: Michael Ellerman @ 2018-10-04 6:14 UTC (permalink / raw)
To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <86e85efc4b5a90523de6e964c5cef8858eb1177d.1537884465.git.christophe.leroy@c-s.fr>
On Tue, 2018-09-25 at 14:10:04 UTC, Christophe Leroy wrote:
> PPC32 uses nonrecoverable_exception() while PPC64 uses
> unrecoverable_exception().
>
> Both functions are doing almost the same thing.
>
> This patch removes nonrecoverable_exception()
>
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/51423a9c9b09352bea1c53b8324db7
cheers
^ 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