* Re: [RFC PATCH v1] powerpc/prom_init: disable XIVE in Secure VM.
From: Greg Kurz @ 2020-03-04 10:59 UTC (permalink / raw)
To: Ram Pai
Cc: aik, andmike, kvm-ppc, Cédric Le Goater, sukadev,
linuxppc-dev, bauerman, David Gibson
In-Reply-To: <20200303185645.GB5416@oc0525413822.ibm.com>
On Tue, 3 Mar 2020 10:56:45 -0800
Ram Pai <linuxram@us.ibm.com> wrote:
> On Tue, Mar 03, 2020 at 06:45:20PM +0100, Greg Kurz wrote:
> > On Tue, 3 Mar 2020 09:02:05 -0800
> > Ram Pai <linuxram@us.ibm.com> wrote:
> >
> > > On Tue, Mar 03, 2020 at 07:50:08AM +0100, Cédric Le Goater wrote:
> > > > On 3/3/20 12:32 AM, David Gibson wrote:
> > > > > On Fri, Feb 28, 2020 at 11:54:04PM -0800, Ram Pai wrote:
> > > > >> XIVE is not correctly enabled for Secure VM in the KVM Hypervisor yet.
> > > > >>
> > > > >> Hence Secure VM, must always default to XICS interrupt controller.
> > > > >>
> > > > >> If XIVE is requested through kernel command line option "xive=on",
> > > > >> override and turn it off.
> > > > >>
> > > > >> If XIVE is the only supported platform interrupt controller; specified
> > > > >> through qemu option "ic-mode=xive", simply abort. Otherwise default to
> > > > >> XICS.
> > > > >
> > > > > Uh... the discussion thread here seems to have gotten oddly off
> > > > > track.
> > > >
> > > > There seem to be multiple issues. It is difficult to have a clear status.
> > > >
> > > > > So, to try to clean up some misunderstandings on both sides:
> > > > >
> > > > > 1) The guest is the main thing that knows that it will be in secure
> > > > > mode, so it's reasonable for it to conditionally use XIVE based
> > > > > on that
> > > >
> > > > FW support is required AFAIUI.
> > > > > 2) The mechanism by which we do it here isn't quite right. Here the
> > > > > guest is checking itself that the host only allows XIVE, but we
> > > > > can't do XIVE and is panic()ing. Instead, in the SVM case we
> > > > > should force support->xive to false, and send that in the CAS
> > > > > request to the host. We expect the host to just terminate
> > > > > us because of the mismatch, but this will interact better with
> > > > > host side options setting policy for panic states and the like.
> > > > > Essentially an SVM kernel should behave like an old kernel with
> > > > > no XIVE support at all, at least w.r.t. the CAS irq mode flags.
> > > >
> > > > Yes. XIVE shouldn't be requested by the guest.
> > >
> > > Ok.
> > >
> > > > This is the last option
> > > > I proposed but I thought there was some negotiation with the hypervisor
> > > > which is not the case.
> > > >
> > > > > 3) Although there are means by which the hypervisor can kind of know
> > > > > a guest is in secure mode, there's not really an "svm=on" option
> > > > > on the host side. For the most part secure mode is based on
> > > > > discussion directly between the guest and the ultravisor with
> > > > > almost no hypervisor intervention.
> > > >
> > > > Is there a negotiation with the ultravisor ?
> > >
> > > The VM has no negotiation with the ultravisor w.r.t CAS.
> > >
> > > >
> > > > > 4) I'm guessing the problem with XIVE in SVM mode is that XIVE needs
> > > > > to write to event queues in guest memory, which would have to be
> > > > > explicitly shared for secure mode. That's true whether it's KVM
> > > > > or qemu accessing the guest memory, so kernel_irqchip=on/off is
> > > > > entirely irrelevant.
> > > >
> > > > This problem should be already fixed.
> > > > The XIVE event queues are shared
> > >
> > > Yes i have a patch for the guest kernel that shares the event
> > > queue page with the hypervisor. This is done using the
> > > UV_SHARE_PAGE ultracall. This patch is not sent out to any any mailing
> > > lists yet.
> >
> > Why ?
>
> At this point I am not sure if this is the only change, I need to the
> guest kernel.
Maybe but we're already sure that this change is needed. I don't really see
the point in holding this any longer.
> I also need changes to KVM and to the ultravisor. Its bit
> premature to send the patch without having figured out everything
> to get xive working on a Secure VM.
>
I'm a bit confused... why did you send this workaround patch in
the first place then ? I mean, this raises a concern and we're
just trying to move forward.
> >
> > > However the patch by itself does not solve the xive problem
> > > for secure VM.
> > >
> >
> > This patch would allow at least to answer Cedric's question about
> > kernel_irqchip=off, since this looks like the only thing needed
> > to make it work.
>
> hmm.. I am not sure. Are you saying
> (a) patch the guest kernel to share the event queue page
> (b) run the qemu with "kernel_irqchip=off"
> (c) and the guest kernel with "svm=on"
>
> and it should all work?
>
Yes.
> RP
>
^ permalink raw reply
* Re: [PATCH v3 21/27] powerpc/powernv/pmem: Add an IOCTL to request controller health & perf data
From: Frederic Barrat @ 2020-03-04 11:06 UTC (permalink / raw)
To: Andrew Donnellan, Alastair D'Silva, alastair
Cc: Madhavan Srinivasan, Alexey Kardashevskiy, Masahiro Yamada,
Oliver O'Halloran, Mauro Carvalho Chehab, Ira Weiny,
Thomas Gleixner, Rob Herring, Dave Jiang, linux-nvdimm,
Aneesh Kumar K . V, Krzysztof Kozlowski, Anju T Sudhakar,
Mahesh Salgaonkar, Arnd Bergmann, Greg Kurz, Nicholas Piggin,
Cédric Le Goater, Dan Williams, Hari Bathini, linux-mm,
Greg Kroah-Hartman, linux-kernel, Vishal Verma, Paul Mackerras,
Andrew Morton, linuxppc-dev, David S. Miller
In-Reply-To: <fdc5faec-d03d-3cba-4a9c-add7e522ad13@linux.ibm.com>
Le 28/02/2020 à 07:12, Andrew Donnellan a écrit :
> On 21/2/20 2:27 pm, Alastair D'Silva wrote:
>> From: Alastair D'Silva <alastair@d-silva.org>
>>
>> When health & performance data is requested from the controller,
>> it responds with an error log containing the requested information.
>>
>> This patch allows the request to me issued via an IOCTL.
>
> A better explanation would be good - this IOCTL triggers a request to
> the controller to collect controller health/perf data, and the
> controller will later respond with an error log that can be picked up
> via the error log IOCTL that you've defined earlier.
And even more precisely (to also check my understanding):
> this IOCTL triggers a request to
> the controller to collect controller health/perf data, and the
> controller will later respond
by raising an interrupt to let the user app know that
> an error log that can be picked up
> via the error log IOCTL that you've defined earlier.
The rest of the patch looks ok to me.
Fred
^ permalink raw reply
* [PATCH] selftests/powerpc: Add a test of sigreturn vs VDSO
From: Michael Ellerman @ 2020-03-04 11:04 UTC (permalink / raw)
To: linuxppc-dev
There's two different paths through the sigreturn code, depending on
whether the VDSO is mapped or not. We recently discovered a bug in the
unmapped case, because it's not commonly used these days.
So add a test that sends itself a signal, then moves the VDSO, takes
another signal and finally unmaps the VDSO before sending itself
another signal. That tests the standard signal path, the code that
handles the VDSO being moved, and also the signal path in the case
where the VDSO is unmapped.
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
.../selftests/powerpc/signal/.gitignore | 1 +
.../testing/selftests/powerpc/signal/Makefile | 2 +-
.../selftests/powerpc/signal/sigreturn_vdso.c | 127 ++++++++++++++++++
3 files changed, 129 insertions(+), 1 deletion(-)
create mode 100644 tools/testing/selftests/powerpc/signal/sigreturn_vdso.c
diff --git a/tools/testing/selftests/powerpc/signal/.gitignore b/tools/testing/selftests/powerpc/signal/.gitignore
index dca5852a1546..03dafa795255 100644
--- a/tools/testing/selftests/powerpc/signal/.gitignore
+++ b/tools/testing/selftests/powerpc/signal/.gitignore
@@ -1,3 +1,4 @@
signal
signal_tm
sigfuz
+sigreturn_vdso
diff --git a/tools/testing/selftests/powerpc/signal/Makefile b/tools/testing/selftests/powerpc/signal/Makefile
index 113838fbbe7f..63b57583e07d 100644
--- a/tools/testing/selftests/powerpc/signal/Makefile
+++ b/tools/testing/selftests/powerpc/signal/Makefile
@@ -1,5 +1,5 @@
# SPDX-License-Identifier: GPL-2.0
-TEST_GEN_PROGS := signal signal_tm sigfuz
+TEST_GEN_PROGS := signal signal_tm sigfuz sigreturn_vdso
CFLAGS += -maltivec
$(OUTPUT)/signal_tm: CFLAGS += -mhtm
diff --git a/tools/testing/selftests/powerpc/signal/sigreturn_vdso.c b/tools/testing/selftests/powerpc/signal/sigreturn_vdso.c
new file mode 100644
index 000000000000..e282fff0fe25
--- /dev/null
+++ b/tools/testing/selftests/powerpc/signal/sigreturn_vdso.c
@@ -0,0 +1,127 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Test that we can take signals with and without the VDSO mapped, which trigger
+ * different paths in the signal handling code.
+ *
+ * See handle_rt_signal64() and setup_trampoline() in signal_64.c
+ */
+
+#define _GNU_SOURCE
+
+#include <errno.h>
+#include <stdio.h>
+#include <signal.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/mman.h>
+#include <sys/types.h>
+#include <unistd.h>
+
+// Ensure assert() is not compiled out
+#undef NDEBUG
+#include <assert.h>
+
+#include "utils.h"
+
+static int search_proc_maps(char *needle, unsigned long *low, unsigned long *high)
+{
+ unsigned long start, end;
+ static char buf[4096];
+ char name[128];
+ FILE *f;
+ int rc = -1;
+
+ f = fopen("/proc/self/maps", "r");
+ if (!f) {
+ perror("fopen");
+ return -1;
+ }
+
+ while (fgets(buf, sizeof(buf), f)) {
+ rc = sscanf(buf, "%lx-%lx %*c%*c%*c%*c %*x %*d:%*d %*d %127s\n",
+ &start, &end, name);
+ if (rc == 2)
+ continue;
+
+ if (rc != 3) {
+ printf("sscanf errored\n");
+ rc = -1;
+ break;
+ }
+
+ if (strstr(name, needle)) {
+ *low = start;
+ *high = end - 1;
+ rc = 0;
+ break;
+ }
+ }
+
+ fclose(f);
+
+ return rc;
+}
+
+static volatile sig_atomic_t took_signal = 0;
+
+static void sigusr1_handler(int sig)
+{
+ took_signal++;
+}
+
+int test_sigreturn_vdso(void)
+{
+ unsigned long low, high, size;
+ struct sigaction act;
+ char *p;
+
+ act.sa_handler = sigusr1_handler;
+ act.sa_flags = 0;
+ sigemptyset(&act.sa_mask);
+
+ assert(sigaction(SIGUSR1, &act, NULL) == 0);
+
+ // Confirm the VDSO is mapped, and work out where it is
+ assert(search_proc_maps("[vdso]", &low, &high) == 0);
+ size = high - low + 1;
+ printf("VDSO is at 0x%lx-0x%lx (%lu bytes)\n", low, high, size);
+
+ kill(getpid(), SIGUSR1);
+ assert(took_signal == 1);
+ printf("Signal delivered OK with VDSO mapped\n");
+
+ // Remap the VDSO somewhere else
+ p = mmap(NULL, size, PROT_READ|PROT_WRITE, MAP_ANONYMOUS|MAP_PRIVATE, -1, 0);
+ assert(p != MAP_FAILED);
+ assert(mremap((void *)low, size, size, MREMAP_MAYMOVE|MREMAP_FIXED, p) != MAP_FAILED);
+ assert(search_proc_maps("[vdso]", &low, &high) == 0);
+ size = high - low + 1;
+ printf("VDSO moved to 0x%lx-0x%lx (%lu bytes)\n", low, high, size);
+
+ kill(getpid(), SIGUSR1);
+ assert(took_signal == 2);
+ printf("Signal delivered OK with VDSO moved\n");
+
+ assert(munmap((void *)low, size) == 0);
+ printf("Unmapped VDSO\n");
+
+ // Confirm the VDSO is not mapped anymore
+ assert(search_proc_maps("[vdso]", &low, &high) != 0);
+
+ // Make the stack executable
+ assert(search_proc_maps("[stack]", &low, &high) == 0);
+ size = high - low + 1;
+ mprotect((void *)low, size, PROT_READ|PROT_WRITE|PROT_EXEC);
+ printf("Remapped the stack executable\n");
+
+ kill(getpid(), SIGUSR1);
+ assert(took_signal == 3);
+ printf("Signal delivered OK with VDSO unmapped\n");
+
+ return 0;
+}
+
+int main(void)
+{
+ return test_harness(test_sigreturn_vdso, "sigreturn_vdso");
+}
--
2.21.1
^ permalink raw reply related
* Re: [PATCH v3 20/27] powerpc/powernv/pmem: Forward events to userspace
From: Frederic Barrat @ 2020-03-04 11:00 UTC (permalink / raw)
To: Alastair D'Silva, alastair
Cc: Madhavan Srinivasan, Alexey Kardashevskiy, Masahiro Yamada,
Oliver O'Halloran, Mauro Carvalho Chehab, Ira Weiny,
Thomas Gleixner, Rob Herring, Dave Jiang, linux-nvdimm,
Aneesh Kumar K . V, Krzysztof Kozlowski, Anju T Sudhakar,
Mahesh Salgaonkar, Andrew Donnellan, Arnd Bergmann, Greg Kurz,
Nicholas Piggin, Cédric Le Goater, Dan Williams,
Hari Bathini, linux-mm, Greg Kroah-Hartman, linux-kernel,
Vishal Verma, Paul Mackerras, Andrew Morton, linuxppc-dev,
David S. Miller
In-Reply-To: <20200221032720.33893-21-alastair@au1.ibm.com>
Le 21/02/2020 à 04:27, Alastair D'Silva a écrit :
> From: Alastair D'Silva <alastair@d-silva.org>
>
> Some of the interrupts that the card generates are better handled
> by the userspace daemon, in particular:
> Controller Hardware/Firmware Fatal
> Controller Dump Available
> Error Log available
>
> This patch allows a userspace application to register an eventfd with
> the driver via SCM_IOCTL_EVENTFD to receive notifications of these
> interrupts.
>
> Userspace can then identify what events have occurred by calling
> SCM_IOCTL_EVENT_CHECK and checking against the SCM_IOCTL_EVENT_FOO
> masks.
>
> Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
> ---
> arch/powerpc/platforms/powernv/pmem/ocxl.c | 216 ++++++++++++++++++
> .../platforms/powernv/pmem/ocxl_internal.h | 5 +
> include/uapi/nvdimm/ocxl-pmem.h | 16 ++
> 3 files changed, 237 insertions(+)
>
> diff --git a/arch/powerpc/platforms/powernv/pmem/ocxl.c b/arch/powerpc/platforms/powernv/pmem/ocxl.c
> index 009d4fd29e7d..e46696d3cc36 100644
> --- a/arch/powerpc/platforms/powernv/pmem/ocxl.c
> +++ b/arch/powerpc/platforms/powernv/pmem/ocxl.c
> @@ -10,6 +10,7 @@
> #include <misc/ocxl.h>
> #include <linux/delay.h>
> #include <linux/ndctl.h>
> +#include <linux/eventfd.h>
> #include <linux/fs.h>
> #include <linux/mm_types.h>
> #include <linux/memory_hotplug.h>
> @@ -335,11 +336,22 @@ static void free_ocxlpmem(struct ocxlpmem *ocxlpmem)
> {
> int rc;
>
> + // Disable doorbells
> + (void)ocxl_global_mmio_set64(ocxlpmem->ocxl_afu, GLOBAL_MMIO_CHIEC,
> + OCXL_LITTLE_ENDIAN,
> + GLOBAL_MMIO_CHI_ALL);
> +
> if (ocxlpmem->nvdimm_bus)
> nvdimm_bus_unregister(ocxlpmem->nvdimm_bus);
>
> free_minor(ocxlpmem);
>
> + if (ocxlpmem->irq_addr[1])
> + iounmap(ocxlpmem->irq_addr[1]);
> +
> + if (ocxlpmem->irq_addr[0])
> + iounmap(ocxlpmem->irq_addr[0]);
> +
> if (ocxlpmem->cdev.owner)
> cdev_del(&ocxlpmem->cdev);
>
> @@ -443,6 +455,11 @@ static int file_release(struct inode *inode, struct file *file)
> {
> struct ocxlpmem *ocxlpmem = file->private_data;
>
> + if (ocxlpmem->ev_ctx) {
> + eventfd_ctx_put(ocxlpmem->ev_ctx);
> + ocxlpmem->ev_ctx = NULL;
> + }
> +
> ocxlpmem_put(ocxlpmem);
> return 0;
> }
> @@ -938,6 +955,51 @@ static int ioctl_controller_stats(struct ocxlpmem *ocxlpmem,
> return rc;
> }
>
> +static int ioctl_eventfd(struct ocxlpmem *ocxlpmem,
> + struct ioctl_ocxl_pmem_eventfd __user *uarg)
> +{
> + struct ioctl_ocxl_pmem_eventfd args;
> +
> + if (copy_from_user(&args, uarg, sizeof(args)))
> + return -EFAULT;
> +
> + if (ocxlpmem->ev_ctx)
> + return -EINVAL;
EBUSY?
> +
> + ocxlpmem->ev_ctx = eventfd_ctx_fdget(args.eventfd);
> + if (!ocxlpmem->ev_ctx)
> + return -EFAULT;
Why not use what eventfd_ctx_fdget() returned? (through some IS_ERR()
and PTR_ERR() convolution)
> +
> + return 0;
> +}
> +
> +static int ioctl_event_check(struct ocxlpmem *ocxlpmem, u64 __user *uarg)
> +{
> + u64 val = 0;
> + int rc;
> + u64 chi = 0;
> +
> + rc = ocxlpmem_chi(ocxlpmem, &chi);
> + if (rc < 0)
> + return rc;
> +
> + if (chi & GLOBAL_MMIO_CHI_ELA)
> + val |= IOCTL_OCXL_PMEM_EVENT_ERROR_LOG_AVAILABLE;
> +
> + if (chi & GLOBAL_MMIO_CHI_CDA)
> + val |= IOCTL_OCXL_PMEM_EVENT_CONTROLLER_DUMP_AVAILABLE;
> +
> + if (chi & GLOBAL_MMIO_CHI_CFFS)
> + val |= IOCTL_OCXL_PMEM_EVENT_FIRMWARE_FATAL;
> +
> + if (chi & GLOBAL_MMIO_CHI_CHFS)
> + val |= IOCTL_OCXL_PMEM_EVENT_HARDWARE_FATAL;
> +
> + rc = copy_to_user((u64 __user *) uarg, &val, sizeof(val));
> +
copy_to_user doesn't return an errno. Should be:
if (copy_to_user((u64 __user *) uarg, &val, sizeof(val)))
return -EFAULT;
> + return rc;
> +}
> +
> static long file_ioctl(struct file *file, unsigned int cmd, unsigned long args)
> {
> struct ocxlpmem *ocxlpmem = file->private_data;
> @@ -966,6 +1028,15 @@ static long file_ioctl(struct file *file, unsigned int cmd, unsigned long args)
> rc = ioctl_controller_stats(ocxlpmem,
> (struct ioctl_ocxl_pmem_controller_stats __user *)args);
> break;
> +
> + case IOCTL_OCXL_PMEM_EVENTFD:
> + rc = ioctl_eventfd(ocxlpmem,
> + (struct ioctl_ocxl_pmem_eventfd __user *)args);
> + break;
> +
> + case IOCTL_OCXL_PMEM_EVENT_CHECK:
> + rc = ioctl_event_check(ocxlpmem, (u64 __user *)args);
> + break;
> }
>
> return rc;
> @@ -1107,6 +1178,146 @@ static void dump_error_log(struct ocxlpmem *ocxlpmem)
> kfree(buf);
> }
>
> +static irqreturn_t imn0_handler(void *private)
> +{
> + struct ocxlpmem *ocxlpmem = private;
> + u64 chi = 0;
> +
> + (void)ocxlpmem_chi(ocxlpmem, &chi);
> +
> + if (chi & GLOBAL_MMIO_CHI_ELA) {
> + dev_warn(&ocxlpmem->dev, "Error log is available\n");
> +
> + if (ocxlpmem->ev_ctx)
> + eventfd_signal(ocxlpmem->ev_ctx, 1);
> + }
> +
> + if (chi & GLOBAL_MMIO_CHI_CDA) {
> + dev_warn(&ocxlpmem->dev, "Controller dump is available\n");
> +
> + if (ocxlpmem->ev_ctx)
> + eventfd_signal(ocxlpmem->ev_ctx, 1);
> + }
> +
> +
(at least) one empty line too many.
> + return IRQ_HANDLED;
> +}
> +
> +static irqreturn_t imn1_handler(void *private)
> +{
> + struct ocxlpmem *ocxlpmem = private;
> + u64 chi = 0;
> +
> + (void)ocxlpmem_chi(ocxlpmem, &chi);
> +
> + if (chi & (GLOBAL_MMIO_CHI_CFFS | GLOBAL_MMIO_CHI_CHFS)) {
> + dev_err(&ocxlpmem->dev,
> + "Controller status is fatal, chi=0x%llx, going offline\n", chi);
> +
> + if (ocxlpmem->nvdimm_bus) {
> + nvdimm_bus_unregister(ocxlpmem->nvdimm_bus);
> + ocxlpmem->nvdimm_bus = NULL;
> + }
> +
> + if (ocxlpmem->ev_ctx)
> + eventfd_signal(ocxlpmem->ev_ctx, 1);
> + }
> +
> + return IRQ_HANDLED;
> +}
> +
> +
> +/**
> + * ocxlpmem_setup_irq() - Set up the IRQs for the OpenCAPI Persistent Memory device
> + * @ocxlpmem: the device metadata
> + * Return: 0 on success, negative on failure
> + */
> +static int ocxlpmem_setup_irq(struct ocxlpmem *ocxlpmem)
> +{
> + int rc;
> + u64 irq_addr;
> +
> + rc = ocxl_afu_irq_alloc(ocxlpmem->ocxl_context, &ocxlpmem->irq_id[0]);
> + if (rc)
> + return rc;
> +
> + rc = ocxl_irq_set_handler(ocxlpmem->ocxl_context, ocxlpmem->irq_id[0],
> + imn0_handler, NULL, ocxlpmem);
> +
> + irq_addr = ocxl_afu_irq_get_addr(ocxlpmem->ocxl_context, ocxlpmem->irq_id[0]);
> + if (!irq_addr)
> + return -EINVAL;
> +
> + ocxlpmem->irq_addr[0] = ioremap(irq_addr, PAGE_SIZE);
> + if (!ocxlpmem->irq_addr[0])
> + return -EINVAL;
> +
> + rc = ocxl_global_mmio_write64(ocxlpmem->ocxl_afu, GLOBAL_MMIO_IMA0_OHP,
> + OCXL_LITTLE_ENDIAN,
> + (u64)ocxlpmem->irq_addr[0]);
> + if (rc)
> + goto out_irq0;
> +
> + rc = ocxl_global_mmio_write64(ocxlpmem->ocxl_afu, GLOBAL_MMIO_IMA0_CFP,
> + OCXL_LITTLE_ENDIAN, 0);
> + if (rc)
> + goto out_irq0;
That's a few lines of duplicate code. On the other hand, there's enough
varying parameters between the 2 interrupts that factorizing in a
subfunction would be slightly less readable. So duplicating is probably ok.
> + rc = ocxl_afu_irq_alloc(ocxlpmem->ocxl_context, &ocxlpmem->irq_id[1]);
> + if (rc)
> + goto out_irq0;
> +
> +
> + rc = ocxl_irq_set_handler(ocxlpmem->ocxl_context, ocxlpmem->irq_id[1],
> + imn1_handler, NULL, ocxlpmem);
> + if (rc)
> + goto out_irq0;
> +
> + irq_addr = ocxl_afu_irq_get_addr(ocxlpmem->ocxl_context, ocxlpmem->irq_id[1]);
> + if (!irq_addr) {
> + rc = -EFAULT;
> + goto out_irq0;
> + }
> +
> + ocxlpmem->irq_addr[1] = ioremap(irq_addr, PAGE_SIZE);
> + if (!ocxlpmem->irq_addr[1]) {
> + rc = -EINVAL;
> + goto out_irq0;
> + }
> +
> + rc = ocxl_global_mmio_write64(ocxlpmem->ocxl_afu, GLOBAL_MMIO_IMA1_OHP,
> + OCXL_LITTLE_ENDIAN,
> + (u64)ocxlpmem->irq_addr[1]);
> + if (rc)
> + goto out_irq1;
> +
> + rc = ocxl_global_mmio_write64(ocxlpmem->ocxl_afu, GLOBAL_MMIO_IMA1_CFP,
> + OCXL_LITTLE_ENDIAN, 0);
> + if (rc)
> + goto out_irq1;
> +
> + // Enable doorbells
> + rc = ocxl_global_mmio_set64(ocxlpmem->ocxl_afu, GLOBAL_MMIO_CHIE,
> + OCXL_LITTLE_ENDIAN,
> + GLOBAL_MMIO_CHI_ELA | GLOBAL_MMIO_CHI_CDA |
> + GLOBAL_MMIO_CHI_CFFS | GLOBAL_MMIO_CHI_CHFS |
> + GLOBAL_MMIO_CHI_NSCRA);
GLOBAL_MMIO_CHI_NSCRA doesn't seem to be handled in the handlers.
> + if (rc)
> + goto out_irq1;
> +
> + return 0;
> +
> +out_irq1:
> + iounmap(ocxlpmem->irq_addr[1]);
> + ocxlpmem->irq_addr[1] = NULL;
> +
> +out_irq0:
> + iounmap(ocxlpmem->irq_addr[0]);
> + ocxlpmem->irq_addr[0] = NULL;
> +
> + return rc;
> +}
> +
> /**
> * probe_function0() - Set up function 0 for an OpenCAPI persistent memory device
> * This is important as it enables templates higher than 0 across all other functions,
> @@ -1216,6 +1427,11 @@ static int probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> goto err;
> }
>
> + if (ocxlpmem_setup_irq(ocxlpmem)) {
> + dev_err(&pdev->dev, "Could not set up OCXL IRQs\n");
Like with other patches, rc needs to be set.
> + goto err;
> + }
> +
> if (setup_command_metadata(ocxlpmem)) {
> dev_err(&pdev->dev, "Could not read OCXL command matada\n");
> goto err;
> diff --git a/arch/powerpc/platforms/powernv/pmem/ocxl_internal.h b/arch/powerpc/platforms/powernv/pmem/ocxl_internal.h
> index b953ee522ed4..927690f4888f 100644
> --- a/arch/powerpc/platforms/powernv/pmem/ocxl_internal.h
> +++ b/arch/powerpc/platforms/powernv/pmem/ocxl_internal.h
> @@ -103,6 +103,10 @@ struct ocxlpmem {
> struct pci_dev *pdev;
> struct cdev cdev;
> struct ocxl_fn *ocxl_fn;
> +#define SCM_IRQ_COUNT 2
> + int irq_id[SCM_IRQ_COUNT];
> + struct dev_pagemap irq_pgmap[SCM_IRQ_COUNT];
irq_pgmap is not used.
> + void *irq_addr[SCM_IRQ_COUNT];
> struct nd_interleave_set nd_set;
> struct nvdimm_bus_descriptor bus_desc;
> struct nvdimm_bus *nvdimm_bus;
> @@ -113,6 +117,7 @@ struct ocxlpmem {
> struct command_metadata ns_command;
> struct resource pmem_res;
> struct nd_region *nd_region;
> + struct eventfd_ctx *ev_ctx;
> char fw_version[8+1];
> u32 timeouts[ADMIN_COMMAND_MAX+1];
>
> diff --git a/include/uapi/nvdimm/ocxl-pmem.h b/include/uapi/nvdimm/ocxl-pmem.h
> index add223aa2fdb..988eb0bc413d 100644
> --- a/include/uapi/nvdimm/ocxl-pmem.h
> +++ b/include/uapi/nvdimm/ocxl-pmem.h
> @@ -66,6 +66,20 @@ struct ioctl_ocxl_pmem_controller_stats {
> __u64 cache_write_latency; /* nanoseconds */
> };
>
> +struct ioctl_ocxl_pmem_eventfd {
> + __s32 eventfd;
> + __u32 reserved;
> +};
> +
> +#ifndef BIT_ULL
> +#define BIT_ULL(nr) (1ULL << (nr))
> +#endif
> +
> +#define IOCTL_OCXL_PMEM_EVENT_CONTROLLER_DUMP_AVAILABLE BIT_ULL(0)
> +#define IOCTL_OCXL_PMEM_EVENT_ERROR_LOG_AVAILABLE BIT_ULL(1)
> +#define IOCTL_OCXL_PMEM_EVENT_HARDWARE_FATAL BIT_ULL(2)
> +#define IOCTL_OCXL_PMEM_EVENT_FIRMWARE_FATAL BIT_ULL(3)
> +
I'm not fond of adding a macro with such a generic name as BIT_ULL() in
a user header file. What's wrong with:
#define IOCTL_OCXL_PMEM_EVENT_CONTROLLER_DUMP_AVAILABLE 0x1
#define IOCTL_OCXL_PMEM_EVENT_ERROR_LOG_AVAILABLE 0x2
#define IOCTL_OCXL_PMEM_EVENT_HARDWARE_FATAL 0x4
#define IOCTL_OCXL_PMEM_EVENT_FIRMWARE_FATAL 0x8
Fred
> /* ioctl numbers */
> #define OCXL_PMEM_MAGIC 0x5C
> /* SCM devices */
> @@ -74,5 +88,7 @@ struct ioctl_ocxl_pmem_controller_stats {
> #define IOCTL_OCXL_PMEM_CONTROLLER_DUMP_DATA _IOWR(OCXL_PMEM_MAGIC, 0x03, struct ioctl_ocxl_pmem_controller_dump_data)
> #define IOCTL_OCXL_PMEM_CONTROLLER_DUMP_COMPLETE _IO(OCXL_PMEM_MAGIC, 0x04)
> #define IOCTL_OCXL_PMEM_CONTROLLER_STATS _IO(OCXL_PMEM_MAGIC, 0x05)
> +#define IOCTL_OCXL_PMEM_EVENTFD _IOW(OCXL_PMEM_MAGIC, 0x06, struct ioctl_ocxl_pmem_eventfd)
> +#define IOCTL_OCXL_PMEM_EVENT_CHECK _IOR(OCXL_PMEM_MAGIC, 0x07, __u64)
>
> #endif /* _UAPI_OCXL_SCM_H */
>
^ permalink raw reply
* Re: [PATCH v3 19/27] powerpc/powernv/pmem: Add an IOCTL to report controller statistics
From: Frederic Barrat @ 2020-03-04 9:25 UTC (permalink / raw)
To: Alastair D'Silva, alastair
Cc: Madhavan Srinivasan, Alexey Kardashevskiy, Masahiro Yamada,
Oliver O'Halloran, Mauro Carvalho Chehab, Ira Weiny,
Thomas Gleixner, Rob Herring, Dave Jiang, linux-nvdimm,
Aneesh Kumar K . V, Krzysztof Kozlowski, Anju T Sudhakar,
Mahesh Salgaonkar, Andrew Donnellan, Arnd Bergmann, Greg Kurz,
Nicholas Piggin, Cédric Le Goater, Dan Williams,
Hari Bathini, linux-mm, Greg Kroah-Hartman, linux-kernel,
Vishal Verma, Paul Mackerras, Andrew Morton, linuxppc-dev,
David S. Miller
In-Reply-To: <20200221032720.33893-20-alastair@au1.ibm.com>
Le 21/02/2020 à 04:27, Alastair D'Silva a écrit :
> From: Alastair D'Silva <alastair@d-silva.org>
>
> The controller can report a number of statistics that are useful
> in evaluating the performance and reliability of the card.
>
> This patch exposes this information via an IOCTL.
>
> Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
> ---
> arch/powerpc/platforms/powernv/pmem/ocxl.c | 185 +++++++++++++++++++++
> include/uapi/nvdimm/ocxl-pmem.h | 17 ++
> 2 files changed, 202 insertions(+)
>
> diff --git a/arch/powerpc/platforms/powernv/pmem/ocxl.c b/arch/powerpc/platforms/powernv/pmem/ocxl.c
> index 2cabafe1fc58..009d4fd29e7d 100644
> --- a/arch/powerpc/platforms/powernv/pmem/ocxl.c
> +++ b/arch/powerpc/platforms/powernv/pmem/ocxl.c
> @@ -758,6 +758,186 @@ static int ioctl_controller_dump_complete(struct ocxlpmem *ocxlpmem)
> GLOBAL_MMIO_HCI_CONTROLLER_DUMP_COLLECTED);
> }
>
> +/**
> + * controller_stats_header_parse() - Parse the first 64 bits of the controller stats admin command response
> + * @ocxlpmem: the device metadata
> + * @length: out, returns the number of bytes in the response (excluding the 64 bit header)
> + */
> +static int controller_stats_header_parse(struct ocxlpmem *ocxlpmem,
> + u32 *length)
> +{
> + int rc;
> + u64 val;
> +
unexpected empty line
> + u16 data_identifier;
> + u32 data_length;
> +
> + rc = ocxl_global_mmio_read64(ocxlpmem->ocxl_afu,
> + ocxlpmem->admin_command.data_offset,
> + OCXL_LITTLE_ENDIAN, &val);
> + if (rc)
> + return rc;
> +
> + data_identifier = val >> 48;
> + data_length = val & 0xFFFFFFFF;
> +
> + if (data_identifier != 0x4353) { // 'CS'
> + dev_err(&ocxlpmem->dev,
> + "Bad data identifier for controller stats, expected 'CS', got '%-.*s'\n",
> + 2, (char *)&data_identifier);
Wow, I'm clueless what that string format looks like :-)
2 arguments? Did you check the kernel string formatter does what you want?
You may consider unifying the format though, the error log patch uses a
simpler (better?) format for a similar message.
> + return -EINVAL;
> + }
> +
> + *length = data_length;
> + return 0;
> +}
> +
> +static int ioctl_controller_stats(struct ocxlpmem *ocxlpmem,
> + struct ioctl_ocxl_pmem_controller_stats __user *uarg)
> +{
> + struct ioctl_ocxl_pmem_controller_stats args;
> + u32 length;
> + int rc;
> + u64 val;
> +
> + memset(&args, '\0', sizeof(args));
> +
> + mutex_lock(&ocxlpmem->admin_command.lock);
> +
> + rc = admin_command_request(ocxlpmem, ADMIN_COMMAND_CONTROLLER_STATS);
> + if (rc)
> + goto out;
> +
> + rc = ocxl_global_mmio_write64(ocxlpmem->ocxl_afu,
> + ocxlpmem->admin_command.request_offset + 0x08,
> + OCXL_LITTLE_ENDIAN, 0);
> + if (rc)
> + goto out;
> +
> + rc = admin_command_execute(ocxlpmem);
> + if (rc)
> + goto out;
> +
> +
> + rc = admin_command_complete_timeout(ocxlpmem,
> + ADMIN_COMMAND_CONTROLLER_STATS);
> + if (rc < 0) {
> + dev_warn(&ocxlpmem->dev, "Controller stats timed out\n");
> + goto out;
> + }
> +
> + rc = admin_response(ocxlpmem);
> + if (rc < 0)
> + goto out;
> + if (rc != STATUS_SUCCESS) {
> + warn_status(ocxlpmem,
> + "Unexpected status from controller stats", rc);
> + goto out;
> + }
All those ioctls commands follow the same pattern:
1. admin_command_request()
2. optionnaly, set some mmio registers specific to the command
3. admin_command_execute()
4. admin_command_complete_timeout()
5. admin_response()
By swapping 1 and 2, we could then factorize steps 1, 3, 4 and 5 in a
function and simplify/shorten the code each time a command is called.
Regarding step 2 (and that's true for all similar patches), a comment
about what the mmio tuning does would help and avoid looking up the
spec. Looking up the spec during the review is expected, but it will
ease reading the code 6 months from now.
> +
> + rc = controller_stats_header_parse(ocxlpmem, &length);
> + if (rc)
> + goto out;
> +
> + if (length != 0x140)
> + warn_status(ocxlpmem,
> + "Unexpected length for controller stats data, expected 0x140, got 0x%x",
> + length);
> +
> + rc = ocxl_global_mmio_read64(ocxlpmem->ocxl_afu,
> + ocxlpmem->admin_command.data_offset + 0x08 + 0x08,
> + OCXL_LITTLE_ENDIAN, &val);
> + if (rc)
> + goto out;
> +
> + args.reset_count = val >> 32;
> + args.reset_uptime = val & 0xFFFFFFFF;
> +
> + rc = ocxl_global_mmio_read64(ocxlpmem->ocxl_afu,
> + ocxlpmem->admin_command.data_offset + 0x08 + 0x10,
> + OCXL_LITTLE_ENDIAN, &val);
> + if (rc)
> + goto out;
> +
> + args.power_on_uptime = val >> 32;
> +
> + rc = ocxl_global_mmio_read64(ocxlpmem->ocxl_afu,
> + ocxlpmem->admin_command.data_offset + 0x08 + 0x40 + 0x08,
> + OCXL_LITTLE_ENDIAN, &args.host_load_count);
Those offsets are hard to understand, even with the spec next to me. And
it seems that we could harden things a bit:
each block as a "statistics parameter ID" and the length of the data for
that block. We should check that and make sure we're reading what we expect.
For example, from the spec I'm looking (110d), I would expect the host
load count to be at offset 0x10. It's entirely possible I'm misreading
it though.
> + if (rc)
> + goto out;
> +
> + rc = ocxl_global_mmio_read64(ocxlpmem->ocxl_afu,
> + ocxlpmem->admin_command.data_offset + 0x08 + 0x40 + 0x10,
> + OCXL_LITTLE_ENDIAN, &args.host_store_count);
> + if (rc)
> + goto out;
> +
> + rc = ocxl_global_mmio_read64(ocxlpmem->ocxl_afu,
> + ocxlpmem->admin_command.data_offset + 0x08 + 0x40 + 0x18,
> + OCXL_LITTLE_ENDIAN, &args.media_read_count);
> + if (rc)
> + goto out;
> +
> + rc = ocxl_global_mmio_read64(ocxlpmem->ocxl_afu,
> + ocxlpmem->admin_command.data_offset + 0x08 + 0x40 + 0x20,
> + OCXL_LITTLE_ENDIAN, &args.media_write_count);
> + if (rc)
> + goto out;
> +
> + rc = ocxl_global_mmio_read64(ocxlpmem->ocxl_afu,
> + ocxlpmem->admin_command.data_offset + 0x08 + 0x40 + 0x28,
> + OCXL_LITTLE_ENDIAN, &args.cache_hit_count);
> + if (rc)
> + goto out;
> +
> + rc = ocxl_global_mmio_read64(ocxlpmem->ocxl_afu,
> + ocxlpmem->admin_command.data_offset + 0x08 + 0x40 + 0x30,
> + OCXL_LITTLE_ENDIAN, &args.cache_miss_count);
> + if (rc)
> + goto out;
> +
> + rc = ocxl_global_mmio_read64(ocxlpmem->ocxl_afu,
> + ocxlpmem->admin_command.data_offset + 0x08 + 0x40 + 0x38,
> + OCXL_LITTLE_ENDIAN, &args.media_read_latency);
> + if (rc)
> + goto out;
> +
> + rc = ocxl_global_mmio_read64(ocxlpmem->ocxl_afu,
> + ocxlpmem->admin_command.data_offset + 0x08 + 0x40 + 0x40,
> + OCXL_LITTLE_ENDIAN, &args.media_write_latency);
> + if (rc)
> + goto out;
> +
> + rc = ocxl_global_mmio_read64(ocxlpmem->ocxl_afu,
> + ocxlpmem->admin_command.data_offset + 0x08 + 0x40 + 0x48,
> + OCXL_LITTLE_ENDIAN, &args.cache_read_latency);
> + if (rc)
> + goto out;
> +
> + rc = ocxl_global_mmio_read64(ocxlpmem->ocxl_afu,
> + ocxlpmem->admin_command.data_offset + 0x08 + 0x40 + 0x50,
> + OCXL_LITTLE_ENDIAN, &args.cache_write_latency);
> + if (rc)
> + goto out;
> +
> + if (copy_to_user(uarg, &args, sizeof(args))) {
> + rc = -EFAULT;
> + goto out;
> + }
> +
> + rc = admin_response_handled(ocxlpmem);
> + if (rc)
> + goto out;
> +
> + rc = 0;
> + goto out;
That may be more of a personal habit, but that final goto disrupts the
"good case" flow. And I think it's pretty unusual within the kernel.
> +
> +out:
> + mutex_unlock(&ocxlpmem->admin_command.lock);
> + return rc;
> +}
> +
> static long file_ioctl(struct file *file, unsigned int cmd, unsigned long args)
> {
> struct ocxlpmem *ocxlpmem = file->private_data;
> @@ -781,6 +961,11 @@ static long file_ioctl(struct file *file, unsigned int cmd, unsigned long args)
> case IOCTL_OCXL_PMEM_CONTROLLER_DUMP_COMPLETE:
> rc = ioctl_controller_dump_complete(ocxlpmem);
> break;
> +
> + case IOCTL_OCXL_PMEM_CONTROLLER_STATS:
> + rc = ioctl_controller_stats(ocxlpmem,
> + (struct ioctl_ocxl_pmem_controller_stats __user *)args);
> + break;
> }
>
> return rc;
> diff --git a/include/uapi/nvdimm/ocxl-pmem.h b/include/uapi/nvdimm/ocxl-pmem.h
> index d4d8512d03f7..add223aa2fdb 100644
> --- a/include/uapi/nvdimm/ocxl-pmem.h
> +++ b/include/uapi/nvdimm/ocxl-pmem.h
> @@ -50,6 +50,22 @@ struct ioctl_ocxl_pmem_controller_dump_data {
> __u64 reserved[8];
> };
>
> +struct ioctl_ocxl_pmem_controller_stats {
> + __u32 reset_count;
> + __u32 reset_uptime; /* seconds */
> + __u32 power_on_uptime; /* seconds */
Same as before, we're going to have some padding here.
Fred
> + __u64 host_load_count;
> + __u64 host_store_count;
> + __u64 media_read_count;
> + __u64 media_write_count;
> + __u64 cache_hit_count;
> + __u64 cache_miss_count;
> + __u64 media_read_latency; /* nanoseconds */
> + __u64 media_write_latency; /* nanoseconds */
> + __u64 cache_read_latency; /* nanoseconds */
> + __u64 cache_write_latency; /* nanoseconds */
> +};
> +
> /* ioctl numbers */
> #define OCXL_PMEM_MAGIC 0x5C
> /* SCM devices */
> @@ -57,5 +73,6 @@ struct ioctl_ocxl_pmem_controller_dump_data {
> #define IOCTL_OCXL_PMEM_CONTROLLER_DUMP _IO(OCXL_PMEM_MAGIC, 0x02)
> #define IOCTL_OCXL_PMEM_CONTROLLER_DUMP_DATA _IOWR(OCXL_PMEM_MAGIC, 0x03, struct ioctl_ocxl_pmem_controller_dump_data)
> #define IOCTL_OCXL_PMEM_CONTROLLER_DUMP_COMPLETE _IO(OCXL_PMEM_MAGIC, 0x04)
> +#define IOCTL_OCXL_PMEM_CONTROLLER_STATS _IO(OCXL_PMEM_MAGIC, 0x05)
>
> #endif /* _UAPI_OCXL_SCM_H */
>
^ permalink raw reply
* Re: [EXTERNAL] Re: [RFC PATCH v1] powerpc/prom_init: disable XIVE in Secure VM.
From: Greg Kurz @ 2020-03-04 8:34 UTC (permalink / raw)
To: Cédric Le Goater
Cc: aik, andmike, Ram Pai, kvm-ppc, sukadev, linuxppc-dev, bauerman,
David Gibson
In-Reply-To: <a1570b0d-c443-3140-31f0-bddd9f31f54b@kaod.org>
On Tue, 3 Mar 2020 20:18:18 +0100
Cédric Le Goater <clg@kaod.org> wrote:
> >> **** BTW: I figured, I dont need this intermin patch to disable xive for
> >> secure VM. Just doing "svm=on xive=off" on the kernel command line is
> >> sufficient for now. *****
> >>
> >
> > No it is not. If the hypervisor doesn't propose XIVE (ie. ic-mode=xive
> > on the QEMU command line), the kernel simply ignores "xive=off".
>
Ah... sorry for the typo... "doesn't propose XICS" of course :)
> If I am correct, with the option ic-mode=xive, the hypervisor will
> propose only 'xive' in OV5 and not both 'xive' and 'xics'. But the
> result is the same because xive can not be turned off and "xive=off"
> is ignored.
>
> Anyway, it's not the most common case of usage of the QEMU command
> like. I think it's OK to use "xive=off" on the kernel command line
> for now.
>
Sure, I just wanted to make things clear. Like you said it's a chicken
switch introduced for distro testing. I think it should not be used
to do anything else. If "svm=1" needs to enforce supported.xive == false
as a temporary workaround, it should do it explicitly.
> C.
^ permalink raw reply
* [PATCHv3 2/2] pseries/scm: buffer pmem's bound addr in dt for kexec kernel
From: Pingfan Liu @ 2020-03-04 8:47 UTC (permalink / raw)
To: linuxppc-dev
Cc: Andrew Donnellan, kexec, Pingfan Liu, Rob Herring, Paul Mackerras,
Aneesh Kumar K . V, Oliver O'Halloran, Dan Williams,
Frank Rowand, Hari Bathini
In-Reply-To: <1583311651-29310-1-git-send-email-kernelfans@gmail.com>
At present, plpar_hcall(H_SCM_BIND_MEM, ...) takes a very long time, so
if dumping to fsdax, it will take a very long time.
Take a closer look, during the papr_scm initialization, the only
configuration is through drc_pmem_bind()-> plpar_hcall(H_SCM_BIND_MEM,
...), which helps to set up the bound address.
On pseries, for kexec -l/-p kernel, there is no reset of hardware, and this
step can be stepped around to save times. So the pmem bound address can be
passed to the 2nd kernel through a dynamic added property "bound-addr" in
dt node 'ibm,pmemory'.
Signed-off-by: Pingfan Liu <kernelfans@gmail.com>
To: linuxppc-dev@lists.ozlabs.org
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Hari Bathini <hbathini@linux.ibm.com>
Cc: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
Cc: Oliver O'Halloran <oohall@gmail.com>
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: Andrew Donnellan <ajd@linux.ibm.com>
Cc: Christophe Leroy <christophe.leroy@c-s.fr>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Frank Rowand <frowand.list@gmail.com>
Cc: kexec@lists.infradead.org
---
note: This patch has not been tested since I can not get such a pseries with pmem.
Please kindly to give some suggestion, thanks.
---
arch/powerpc/platforms/pseries/of_helpers.c | 1 +
arch/powerpc/platforms/pseries/papr_scm.c | 33 ++++++++++++++++++++---------
drivers/of/base.c | 1 +
3 files changed, 25 insertions(+), 10 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/of_helpers.c b/arch/powerpc/platforms/pseries/of_helpers.c
index 1022e0f..2c7bab4 100644
--- a/arch/powerpc/platforms/pseries/of_helpers.c
+++ b/arch/powerpc/platforms/pseries/of_helpers.c
@@ -34,6 +34,7 @@ struct property *new_property(const char *name, const int length,
kfree(new);
return NULL;
}
+EXPORT_SYMBOL(new_property);
/**
* pseries_of_derive_parent - basically like dirname(1)
diff --git a/arch/powerpc/platforms/pseries/papr_scm.c b/arch/powerpc/platforms/pseries/papr_scm.c
index 0b4467e..54ae903 100644
--- a/arch/powerpc/platforms/pseries/papr_scm.c
+++ b/arch/powerpc/platforms/pseries/papr_scm.c
@@ -14,6 +14,7 @@
#include <linux/delay.h>
#include <asm/plpar_wrappers.h>
+#include "of_helpers.h"
#define BIND_ANY_ADDR (~0ul)
@@ -383,7 +384,7 @@ static int papr_scm_probe(struct platform_device *pdev)
{
struct device_node *dn = pdev->dev.of_node;
u32 drc_index, metadata_size;
- u64 blocks, block_size;
+ u64 blocks, block_size, bound_addr = 0;
struct papr_scm_priv *p;
const char *uuid_str;
u64 uuid[2];
@@ -440,17 +441,29 @@ static int papr_scm_probe(struct platform_device *pdev)
p->metadata_size = metadata_size;
p->pdev = pdev;
- /* request the hypervisor to bind this region to somewhere in memory */
- rc = drc_pmem_bind(p);
+ of_property_read_u64(dn, "bound-addr", &bound_addr);
+ if (bound_addr) {
+ p->bound_addr = bound_addr;
+ } else {
+ struct property *property;
+ u64 big;
- /* If phyp says drc memory still bound then force unbound and retry */
- if (rc == H_OVERLAP)
- rc = drc_pmem_query_n_bind(p);
+ /* request the hypervisor to bind this region to somewhere in memory */
+ rc = drc_pmem_bind(p);
- if (rc != H_SUCCESS) {
- dev_err(&p->pdev->dev, "bind err: %d\n", rc);
- rc = -ENXIO;
- goto err;
+ /* If phyp says drc memory still bound then force unbound and retry */
+ if (rc == H_OVERLAP)
+ rc = drc_pmem_query_n_bind(p);
+
+ if (rc != H_SUCCESS) {
+ dev_err(&p->pdev->dev, "bind err: %d\n", rc);
+ rc = -ENXIO;
+ goto err;
+ }
+ big = cpu_to_be64(p->bound_addr);
+ property = new_property("bound-addr", sizeof(u64), (const unsigned char *)&big,
+ NULL);
+ of_add_property(dn, property);
}
/* setup the resource for the newly bound range */
diff --git a/drivers/of/base.c b/drivers/of/base.c
index ae03b12..602d2a9 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -1817,6 +1817,7 @@ int of_add_property(struct device_node *np, struct property *prop)
return rc;
}
+EXPORT_SYMBOL_GPL(of_add_property);
int __of_remove_property(struct device_node *np, struct property *prop)
{
--
2.7.5
^ permalink raw reply related
* [PATCHv3 1/2] powerpc/of: split out new_property() for reusing
From: Pingfan Liu @ 2020-03-04 8:47 UTC (permalink / raw)
To: linuxppc-dev
Cc: Andrew Donnellan, kexec, Pingfan Liu, Rob Herring, Paul Mackerras,
Aneesh Kumar K . V, Oliver O'Halloran, Dan Williams,
Frank Rowand, Hari Bathini
In-Reply-To: <1583311651-29310-1-git-send-email-kernelfans@gmail.com>
Splitting out new_property() for coming reusing and moving it to
of_helpers.c.
Also do some coding style cleanup.
Signed-off-by: Pingfan Liu <kernelfans@gmail.com>
To: linuxppc-dev@lists.ozlabs.org
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Hari Bathini <hbathini@linux.ibm.com>
Cc: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
Cc: Oliver O'Halloran <oohall@gmail.com>
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: Andrew Donnellan <ajd@linux.ibm.com>
Cc: Christophe Leroy <christophe.leroy@c-s.fr>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Frank Rowand <frowand.list@gmail.com>
Cc: kexec@lists.infradead.org
---
arch/powerpc/platforms/pseries/of_helpers.c | 28 ++++++++++++++++++++++++++++
arch/powerpc/platforms/pseries/of_helpers.h | 3 +++
arch/powerpc/platforms/pseries/reconfig.c | 26 --------------------------
3 files changed, 31 insertions(+), 26 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/of_helpers.c b/arch/powerpc/platforms/pseries/of_helpers.c
index 66dfd82..1022e0f 100644
--- a/arch/powerpc/platforms/pseries/of_helpers.c
+++ b/arch/powerpc/platforms/pseries/of_helpers.c
@@ -7,6 +7,34 @@
#include "of_helpers.h"
+struct property *new_property(const char *name, const int length,
+ const unsigned char *value, struct property *last)
+{
+ struct property *new = kzalloc(sizeof(*new), GFP_KERNEL);
+
+ if (!new)
+ return NULL;
+
+ new->name = kstrdup(name, GFP_KERNEL);
+ if (!new->name)
+ goto cleanup;
+ new->value = kmalloc(length + 1, GFP_KERNEL);
+ if (!new->value)
+ goto cleanup;
+
+ memcpy(new->value, value, length);
+ *(((char *)new->value) + length) = 0;
+ new->length = length;
+ new->next = last;
+ return new;
+
+cleanup:
+ kfree(new->name);
+ kfree(new->value);
+ kfree(new);
+ return NULL;
+}
+
/**
* pseries_of_derive_parent - basically like dirname(1)
* @path: the full_name of a node to be added to the tree
diff --git a/arch/powerpc/platforms/pseries/of_helpers.h b/arch/powerpc/platforms/pseries/of_helpers.h
index decad65..34add82 100644
--- a/arch/powerpc/platforms/pseries/of_helpers.h
+++ b/arch/powerpc/platforms/pseries/of_helpers.h
@@ -4,6 +4,9 @@
#include <linux/of.h>
+struct property *new_property(const char *name, const int length,
+ const unsigned char *value, struct property *last);
+
struct device_node *pseries_of_derive_parent(const char *path);
#endif /* _PSERIES_OF_HELPERS_H */
diff --git a/arch/powerpc/platforms/pseries/reconfig.c b/arch/powerpc/platforms/pseries/reconfig.c
index 7f7369f..8e5a2ba 100644
--- a/arch/powerpc/platforms/pseries/reconfig.c
+++ b/arch/powerpc/platforms/pseries/reconfig.c
@@ -165,32 +165,6 @@ static char * parse_next_property(char *buf, char *end, char **name, int *length
return tmp;
}
-static struct property *new_property(const char *name, const int length,
- const unsigned char *value, struct property *last)
-{
- struct property *new = kzalloc(sizeof(*new), GFP_KERNEL);
-
- if (!new)
- return NULL;
-
- if (!(new->name = kstrdup(name, GFP_KERNEL)))
- goto cleanup;
- if (!(new->value = kmalloc(length + 1, GFP_KERNEL)))
- goto cleanup;
-
- memcpy(new->value, value, length);
- *(((char *)new->value) + length) = 0;
- new->length = length;
- new->next = last;
- return new;
-
-cleanup:
- kfree(new->name);
- kfree(new->value);
- kfree(new);
- return NULL;
-}
-
static int do_add_node(char *buf, size_t bufsize)
{
char *path, *end, *name;
--
2.7.5
^ permalink raw reply related
* [PATCHv3 0/2] pseries/scm: buffer pmem's bound addr in dt for kexec kernel
From: Pingfan Liu @ 2020-03-04 8:47 UTC (permalink / raw)
To: linuxppc-dev
Cc: Andrew Donnellan, kexec, Pingfan Liu, Rob Herring, Paul Mackerras,
Aneesh Kumar K . V, Oliver O'Halloran, Dan Williams,
Frank Rowand, Hari Bathini
In-Reply-To: <1582882895-3142-1-git-send-email-kernelfans@gmail.com>
V2 -> V3:
in [2/2], EXPORT_SYMBOL(new_property) and EXPORT_SYMBOL_GPL(of_add_property)
To: linuxppc-dev@lists.ozlabs.org
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Hari Bathini <hbathini@linux.ibm.com>
Cc: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
Cc: Oliver O'Halloran <oohall@gmail.com>
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: Andrew Donnellan <ajd@linux.ibm.com>
Cc: Christophe Leroy <christophe.leroy@c-s.fr>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Frank Rowand <frowand.list@gmail.com>
Cc: kexec@lists.infradead.org
Pingfan Liu (2):
powerpc/of: split out new_property() for reusing
pseries/scm: buffer pmem's bound addr in dt for kexec kernel
arch/powerpc/platforms/pseries/of_helpers.c | 29 +++++++++++++++++++++++++
arch/powerpc/platforms/pseries/of_helpers.h | 3 +++
arch/powerpc/platforms/pseries/papr_scm.c | 33 ++++++++++++++++++++---------
arch/powerpc/platforms/pseries/reconfig.c | 26 -----------------------
drivers/of/base.c | 1 +
5 files changed, 56 insertions(+), 36 deletions(-)
--
2.7.5
^ permalink raw reply
* Re: [PATCH 1/1] powerpc/kernel: Enables memory hot-remove after reboot on pseries guests
From: Leonardo Bras @ 2020-03-04 7:18 UTC (permalink / raw)
To: Bharata B Rao
Cc: ndfont, linuxppc-dev, Greg Kroah-Hartman, Claudio Carvalho,
Michael Roth, linux-kernel@vger.kernel.org, Paul Mackerras,
Thomas Gleixner, arbab, Hari Bathini
In-Reply-To: <CAGZKiBr=8i11YPDn+1y5j6YfGj+tVbbTKakoGje9QQ8TEw9g5g@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1095 bytes --]
Hello Bharata, thanks for this feedback!
On Wed, 2020-03-04 at 10:13 +0530, Bharata B Rao wrote:
> Hi,
>
> I tried this a few years back
> (https://patchwork.ozlabs.org/patch/800142/) and didn't pursue it
> further because at that time, it was felt that the approach might not
> work for PowerVM guests, because all the present memory except RMA
> gets marked as hot-pluggable by PowerVM. This discussion is not
> present in the above thread, but during my private discussions with
> Reza and Nathan, it was noted that making all that memory as MOVABLE
> is not preferable for PowerVM guests as we might run out of memory for
> kernel allocations.
Humm, this makes sense.
But with mu change, these pieces of memory only get into ZONE_MOVABLE
if the boot parameter 'movable_node' gets passed to guest kernel.
So, even if we are unable to sort out some flag combination that work
fine for both use-cases, if PowerVM don't pass 'movable_node' as boot
parameter to kernel, it will behave just as today.
What are your thoughts on that?
Best regards,
Leonardo Bras
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v2] ima: add a new CONFIG for loading arch-specific policies
From: Ard Biesheuvel @ 2020-03-04 7:14 UTC (permalink / raw)
To: Nayna Jain
Cc: linux-s390, linux-efi, Linux Kernel Mailing List, Mimi Zohar,
Philipp Rudo, linux-integrity, linuxppc-dev
In-Reply-To: <1583289211-5420-1-git-send-email-nayna@linux.ibm.com>
On Wed, 4 Mar 2020 at 03:34, Nayna Jain <nayna@linux.ibm.com> wrote:
>
> Every time a new architecture defines the IMA architecture specific
> functions - arch_ima_get_secureboot() and arch_ima_get_policy(), the IMA
> include file needs to be updated. To avoid this "noise", this patch
> defines a new IMA Kconfig IMA_SECURE_AND_OR_TRUSTED_BOOT option, allowing
> the different architectures to select it.
>
> Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
> Signed-off-by: Nayna Jain <nayna@linux.ibm.com>
> Cc: Ard Biesheuvel <ardb@kernel.org>
> Cc: Philipp Rudo <prudo@linux.ibm.com>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
Acked-by: Ard Biesheuvel <ardb@kernel.org>
for the x86 bits, but I'm not an x86 maintainer. Also, you may need to
split this if you want to permit arch maintainers to pick up their
parts individually.
> ---
> v2:
> * Fixed the issue identified by Mimi. Thanks Mimi, Ard, Heiko and Michael for
> discussing the fix.
>
> arch/powerpc/Kconfig | 1 +
> arch/s390/Kconfig | 1 +
> arch/x86/Kconfig | 1 +
> include/linux/ima.h | 3 +--
> security/integrity/ima/Kconfig | 9 +++++++++
> 5 files changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index 497b7d0b2d7e..a5cfde432983 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -979,6 +979,7 @@ config PPC_SECURE_BOOT
> bool
> depends on PPC_POWERNV
> depends on IMA_ARCH_POLICY
> + select IMA_SECURE_AND_OR_TRUSTED_BOOT
> help
> Systems with firmware secure boot enabled need to define security
> policies to extend secure boot to the OS. This config allows a user
> diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
> index 8abe77536d9d..4a502fbcb800 100644
> --- a/arch/s390/Kconfig
> +++ b/arch/s390/Kconfig
> @@ -195,6 +195,7 @@ config S390
> select ARCH_HAS_FORCE_DMA_UNENCRYPTED
> select SWIOTLB
> select GENERIC_ALLOCATOR
> + select IMA_SECURE_AND_OR_TRUSTED_BOOT if IMA_ARCH_POLICY
>
>
> config SCHED_OMIT_FRAME_POINTER
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index beea77046f9b..7f5bfaf0cbd2 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -230,6 +230,7 @@ config X86
> select VIRT_TO_BUS
> select X86_FEATURE_NAMES if PROC_FS
> select PROC_PID_ARCH_STATUS if PROC_FS
> + select IMA_SECURE_AND_OR_TRUSTED_BOOT if EFI && IMA_ARCH_POLICY
>
> config INSTRUCTION_DECODER
> def_bool y
> diff --git a/include/linux/ima.h b/include/linux/ima.h
> index 1659217e9b60..aefe758f4466 100644
> --- a/include/linux/ima.h
> +++ b/include/linux/ima.h
> @@ -30,8 +30,7 @@ extern void ima_kexec_cmdline(const void *buf, int size);
> extern void ima_add_kexec_buffer(struct kimage *image);
> #endif
>
> -#if (defined(CONFIG_X86) && defined(CONFIG_EFI)) || defined(CONFIG_S390) \
> - || defined(CONFIG_PPC_SECURE_BOOT)
> +#ifdef CONFIG_IMA_SECURE_AND_OR_TRUSTED_BOOT
> extern bool arch_ima_get_secureboot(void);
> extern const char * const *arch_get_ima_policy(void);
> #else
> diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
> index 3f3ee4e2eb0d..d17972aa413a 100644
> --- a/security/integrity/ima/Kconfig
> +++ b/security/integrity/ima/Kconfig
> @@ -327,3 +327,12 @@ config IMA_QUEUE_EARLY_BOOT_KEYS
> depends on IMA_MEASURE_ASYMMETRIC_KEYS
> depends on SYSTEM_TRUSTED_KEYRING
> default y
> +
> +config IMA_SECURE_AND_OR_TRUSTED_BOOT
> + bool
> + depends on IMA
> + depends on IMA_ARCH_POLICY
Doesn't the latter already depend on the former?
> + default n
> + help
> + This option is selected by architectures to enable secure and/or
> + trusted boot based on IMA runtime policies.
> --
> 2.13.6
>
^ permalink raw reply
* Re: [PATCH v3 18/27] powerpc/powernv/pmem: Add controller dump IOCTLs
From: Andrew Donnellan @ 2020-03-04 6:53 UTC (permalink / raw)
To: Alastair D'Silva, alastair
Cc: Madhavan Srinivasan, Alexey Kardashevskiy, Masahiro Yamada,
Oliver O'Halloran, Mauro Carvalho Chehab, Ira Weiny,
Thomas Gleixner, Rob Herring, Dave Jiang, linux-nvdimm,
Aneesh Kumar K . V, Krzysztof Kozlowski, Anju T Sudhakar,
Mahesh Salgaonkar, Arnd Bergmann, Greg Kurz, Nicholas Piggin,
Cédric Le Goater, Dan Williams, Hari Bathini, linux-mm,
Greg Kroah-Hartman, linux-kernel, Vishal Verma, Frederic Barrat,
Paul Mackerras, Andrew Morton, linuxppc-dev, David S. Miller
In-Reply-To: <20200221032720.33893-19-alastair@au1.ibm.com>
On 21/2/20 2:27 pm, Alastair D'Silva wrote:
> +static int ioctl_controller_dump_data(struct ocxlpmem *ocxlpmem,
> + struct ioctl_ocxl_pmem_controller_dump_data __user *uarg)
> +{
> + struct ioctl_ocxl_pmem_controller_dump_data args;
> + u16 i;
> + u64 val;
> + int rc;
> +
> + if (copy_from_user(&args, uarg, sizeof(args)))
> + return -EFAULT;
> +
> + if (args.buf_size % 8)
> + return -EINVAL;
> +
> + if (args.buf_size > ocxlpmem->admin_command.data_size)
> + return -EINVAL;
> +
> + mutex_lock(&ocxlpmem->admin_command.lock);
> +
> + rc = admin_command_request(ocxlpmem, ADMIN_COMMAND_CONTROLLER_DUMP);
> + if (rc)
> + goto out;
> +
> + val = ((u64)args.offset) << 32;
> + val |= args.buf_size;
> + rc = ocxl_global_mmio_write64(ocxlpmem->ocxl_afu,
> + ocxlpmem->admin_command.request_offset + 0x08,
> + OCXL_LITTLE_ENDIAN, val);
> + if (rc)
> + goto out;
> +
> + rc = admin_command_execute(ocxlpmem);
> + if (rc)
> + goto out;
> +
> + rc = admin_command_complete_timeout(ocxlpmem,
> + ADMIN_COMMAND_CONTROLLER_DUMP);
> + if (rc < 0) {
> + dev_warn(&ocxlpmem->dev, "Controller dump timed out\n");
> + goto out;
> + }
> +
> + rc = admin_response(ocxlpmem);
> + if (rc < 0)
> + goto out;
> + if (rc != STATUS_SUCCESS) {
> + warn_status(ocxlpmem,
> + "Unexpected status from retrieve error log",
Controller dump
> + rc);
> + goto out;
> + }
> +
> + for (i = 0; i < args.buf_size; i += 8) {
> + u64 val;
> +
> + rc = ocxl_global_mmio_read64(ocxlpmem->ocxl_afu,
> + ocxlpmem->admin_command.data_offset + i,
> + OCXL_HOST_ENDIAN, &val);
Is a controller dump something where we want to do endian swapping?
Any reason we're not doing the usual check of the data identifier,
additional data length etc?
> + if (rc)
> + goto out;
> +
> + if (copy_to_user(&args.buf[i], &val, sizeof(u64))) {
> + rc = -EFAULT;
> + goto out;
> + }
> + }
> +
> + if (copy_to_user(uarg, &args, sizeof(args))) {
> + rc = -EFAULT;
> + goto out;
> + }
> +
> + rc = admin_response_handled(ocxlpmem);
> + if (rc)
> + goto out;
> +
> +out:
> + mutex_unlock(&ocxlpmem->admin_command.lock);
> + return rc;
> +}
> +
> +int request_controller_dump(struct ocxlpmem *ocxlpmem)
> +{
> + int rc;
> + u64 busy = 1;
> +
> + rc = ocxl_global_mmio_set64(ocxlpmem->ocxl_afu, GLOBAL_MMIO_CHIC,
> + OCXL_LITTLE_ENDIAN,
> + GLOBAL_MMIO_CHI_CDA);
This return code is ignored
> +
> +
> + rc = ocxl_global_mmio_set64(ocxlpmem->ocxl_afu, GLOBAL_MMIO_HCI,
> + OCXL_LITTLE_ENDIAN,
> + GLOBAL_MMIO_HCI_CONTROLLER_DUMP);
> + if (rc)
> + return rc;
> +
> + while (busy) {
> + rc = ocxl_global_mmio_read64(ocxlpmem->ocxl_afu,
> + GLOBAL_MMIO_HCI,
> + OCXL_LITTLE_ENDIAN, &busy);
> + if (rc)
> + return rc;
> +
> + busy &= GLOBAL_MMIO_HCI_CONTROLLER_DUMP;
> + cond_resched();
> + }
> +
> + return 0;
> +}
--
Andrew Donnellan OzLabs, ADL Canberra
ajd@linux.ibm.com IBM Australia Limited
^ permalink raw reply
* Re: [PATCH V14] mm/debug: Add tests validating architecture page table helpers
From: Christophe Leroy @ 2020-03-04 6:48 UTC (permalink / raw)
To: Qian Cai, Anshuman Khandual
Cc: Heiko Carstens, Linux Memory Management List, Paul Mackerras,
H. Peter Anvin, linux-riscv, Will Deacon, linux-arch, linux-s390,
the arch/x86 maintainers, Mike Rapoport, Christian Borntraeger,
Ingo Molnar, Catalin Marinas, linux-snps-arc, Vasily Gorbik,
Borislav Petkov, Paul Walmsley, Kirill A . Shutemov,
Thomas Gleixner, linux-arm-kernel, Vineet Gupta, LKML,
Palmer Dabbelt, Aneesh Kumar K.V, Andrew Morton, linuxppc-dev
In-Reply-To: <12260F9A-695D-40F8-932F-61D86D77D441@lca.pw>
Le 04/03/2020 à 02:39, Qian Cai a écrit :
>
>> Below is slightly modified version of your change above and should still
>> prevent the bug on powerpc. Will it be possible for you to re-test this
>> ? Once confirmed, will send a patch enabling this test on powerpc64
>> keeping your authorship. Thank you.
>
> This works fine on radix MMU but I decided to go a bit future to test hash
> MMU. The kernel will stuck here below. I did confirm that pte_alloc_map_lock()
> was successful, so I don’t understand hash MMU well enough to tell why
> it could still take an interrupt at pte_clear_tests() even before we calls
> pte_unmap_unlock()?
AFAIU, you are not taking an interrupt here. You are stuck in the
pte_update(), most likely due to nested locks. Try with LOCKDEP ?
Christophe
>
> [ 33.881515][ T1] ok 8 - property-entry
> [ 33.883653][ T1] debug_vm_pgtable: debug_vm_pgtable: Validating
> architecture page table helpers
> [ 60.418885][ C8] watchdog: BUG: soft lockup - CPU#8 stuck for 23s!
> [swapper/0:1]
^ permalink raw reply
* Re: [PATCH v3 17/27] powerpc/powernv/pmem: Implement the Read Error Log command
From: Andrew Donnellan @ 2020-03-04 5:58 UTC (permalink / raw)
To: Alastair D'Silva, alastair
Cc: Madhavan Srinivasan, Alexey Kardashevskiy, Masahiro Yamada,
Oliver O'Halloran, Mauro Carvalho Chehab, Ira Weiny,
Thomas Gleixner, Rob Herring, Dave Jiang, linux-nvdimm,
Aneesh Kumar K . V, Krzysztof Kozlowski, Anju T Sudhakar,
Mahesh Salgaonkar, Arnd Bergmann, Greg Kurz, Nicholas Piggin,
Cédric Le Goater, Dan Williams, Hari Bathini, linux-mm,
Greg Kroah-Hartman, linux-kernel, Vishal Verma, Frederic Barrat,
Paul Mackerras, Andrew Morton, linuxppc-dev, David S. Miller
In-Reply-To: <20200221032720.33893-18-alastair@au1.ibm.com>
On 21/2/20 2:27 pm, Alastair D'Silva wrote:
> From: Alastair D'Silva <alastair@d-silva.org>
>
> The read error log command extracts information from the controller's
> internal error log.
>
> This patch exposes this information in 2 ways:
> - During probe, if an error occurs & a log is available, print it to the
> console
> - After probe, make the error log available to userspace via an IOCTL.
> Userspace is notified of pending error logs in a later patch
> ("powerpc/powernv/pmem: Forward events to userspace")
>
> Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
A few minor style checks at
https://openpower.xyz/job/snowpatch/job/snowpatch-linux-checkpatch/11787//artifact/linux/checkpatch.log
We should also add some documentation for the user interfaces we're
adding (same applies for all the remaining patches in this series that
add more interfaces).
> ---
> arch/powerpc/platforms/powernv/pmem/ocxl.c | 269 ++++++++++++++++++
> .../platforms/powernv/pmem/ocxl_internal.h | 1 +
> include/uapi/nvdimm/ocxl-pmem.h | 46 +++
> 3 files changed, 316 insertions(+)
> create mode 100644 include/uapi/nvdimm/ocxl-pmem.h
>
> diff --git a/arch/powerpc/platforms/powernv/pmem/ocxl.c b/arch/powerpc/platforms/powernv/pmem/ocxl.c
> index 63109a870d2c..2b64504f9129 100644
> --- a/arch/powerpc/platforms/powernv/pmem/ocxl.c
> +++ b/arch/powerpc/platforms/powernv/pmem/ocxl.c
> @@ -447,10 +447,219 @@ static int file_release(struct inode *inode, struct file *file)
> return 0;
> }
>
> +/**
> + * error_log_header_parse() - Parse the first 64 bits of the error log command response
> + * @ocxlpmem: the device metadata
> + * @length: out, returns the number of bytes in the response (excluding the 64 bit header)
> + */
> +static int error_log_header_parse(struct ocxlpmem *ocxlpmem, u16 *length)
> +{
> + int rc;
> + u64 val;
> +
> + u16 data_identifier;
> + u32 data_length;
> +
> + rc = ocxl_global_mmio_read64(ocxlpmem->ocxl_afu,
> + ocxlpmem->admin_command.data_offset,
> + OCXL_LITTLE_ENDIAN, &val);
> + if (rc)
> + return rc;
> +
> + data_identifier = val >> 48;
> + data_length = val & 0xFFFF;
> +
> + if (data_identifier != 0x454C) { // 'EL'
> + dev_err(&ocxlpmem->dev,
> + "Bad data identifier for error log data, expected 'EL', got '%2s' (%#x), data_length=%u\n",
> + (char *)&data_identifier,
> + (unsigned int)data_identifier, data_length);
> + return -EINVAL;
This should be something other than EINVAL I think
> + }
> +
> + *length = data_length;
> + return 0;
> +}
> +
> +static int error_log_offset_0x08(struct ocxlpmem *ocxlpmem,
> + u32 *log_identifier, u32 *program_ref_code)
> +{
> + int rc;
> + u64 val;
> +
> + rc = ocxl_global_mmio_read64(ocxlpmem->ocxl_afu,
> + ocxlpmem->admin_command.data_offset + 0x08,
> + OCXL_LITTLE_ENDIAN, &val);
> + if (rc)
> + return rc;
> +
> + *log_identifier = val >> 32;
> + *program_ref_code = val & 0xFFFFFFFF;
> +
> + return 0;
> +}
> +
> +static int read_error_log(struct ocxlpmem *ocxlpmem,
> + struct ioctl_ocxl_pmem_error_log *log, bool buf_is_user)
> +{
> + u64 val;
> + u16 user_buf_length;
> + u16 buf_length;
> + u16 i;
> + int rc;
> +
> + if (log->buf_size % 8)
> + return -EINVAL;
> +
> + rc = ocxlpmem_chi(ocxlpmem, &val);
> + if (rc)
> + goto out;
> +
> + if (!(val & GLOBAL_MMIO_CHI_ELA))
> + return -EAGAIN;
> +
> + user_buf_length = log->buf_size;
> +
> + mutex_lock(&ocxlpmem->admin_command.lock);
> +
> + rc = admin_command_request(ocxlpmem, ADMIN_COMMAND_ERRLOG);
> + if (rc)
> + goto out;
> +
> + rc = admin_command_execute(ocxlpmem);
> + if (rc)
> + goto out;
> +
> + rc = admin_command_complete_timeout(ocxlpmem, ADMIN_COMMAND_ERRLOG);
> + if (rc < 0) {
> + dev_warn(&ocxlpmem->dev, "Read error log timed out\n");
> + goto out;
> + }
> +
> + rc = admin_response(ocxlpmem);
> + if (rc < 0)
> + goto out;
> + if (rc != STATUS_SUCCESS) {
> + warn_status(ocxlpmem, "Unexpected status from retrieve error log", rc);
> + goto out;
> + }
> +
> +
> + rc = error_log_header_parse(ocxlpmem, &log->buf_size);
> + if (rc)
> + goto out;
> + // log->buf_size now contains the returned buffer size, not the user size
In the event that the log is truncated to fit the user buffer, we return
the full log size, I assume this is intentional to signal it's truncated
as per the nd stuff?
> +
> + rc = error_log_offset_0x08(ocxlpmem, &log->log_identifier,
> + &log->program_reference_code);
> + if (rc)
> + goto out;
> +
> + rc = ocxl_global_mmio_read64(ocxlpmem->ocxl_afu,
> + ocxlpmem->admin_command.data_offset + 0x10,
> + OCXL_LITTLE_ENDIAN, &val);
> + if (rc)
> + goto out;
> +
> + log->error_log_type = val >> 56;
> + log->action_flags = (log->error_log_type == OCXL_PMEM_ERROR_LOG_TYPE_GENERAL) ?
> + (val >> 32) & 0xFFFFFF : 0;
> + log->power_on_seconds = val & 0xFFFFFFFF;
> +
> + rc = ocxl_global_mmio_read64(ocxlpmem->ocxl_afu,
> + ocxlpmem->admin_command.data_offset + 0x18,
> + OCXL_LITTLE_ENDIAN, &log->timestamp);
> + if (rc)
> + goto out;
> +
> + rc = ocxl_global_mmio_read64(ocxlpmem->ocxl_afu,
> + ocxlpmem->admin_command.data_offset + 0x20,
> + OCXL_HOST_ENDIAN, &log->wwid[0]);
> + if (rc)
> + goto out;
> +
> + rc = ocxl_global_mmio_read64(ocxlpmem->ocxl_afu,
> + ocxlpmem->admin_command.data_offset + 0x28,
> + OCXL_HOST_ENDIAN, &log->wwid[1]);
> + if (rc)
> + goto out;
> +
> + rc = ocxl_global_mmio_read64(ocxlpmem->ocxl_afu,
> + ocxlpmem->admin_command.data_offset + 0x30,
> + OCXL_HOST_ENDIAN, (u64 *)log->fw_revision);
Why the difference between HOST and LITTLE_ENDIAN between these fields?
> + if (rc)
> + goto out;
> + log->fw_revision[8] = '\0';
> +
> + buf_length = (user_buf_length < log->buf_size) ?
> + user_buf_length : log->buf_size;
> + for (i = 0; i < buf_length + 0x48; i += 8) {
+ 0x48 here doesn't look right...
> + u64 val;
> +
> + rc = ocxl_global_mmio_read64(ocxlpmem->ocxl_afu,
> + ocxlpmem->admin_command.data_offset + i,
...did you mean to add 0x48 here?
> + OCXL_HOST_ENDIAN, &val);
> + if (rc)
> + goto out;
> +
> + if (buf_is_user) {
> + if (copy_to_user(&log->buf[i], &val, sizeof(u64))) {
> + rc = -EFAULT;
> + goto out;
> + }
> + } else
> + log->buf[i] = val;
Please use braces consistently on both sides of if/else.
> + }
> +
> + rc = admin_response_handled(ocxlpmem);
> + if (rc)
> + goto out;
> +
> +out:
> + mutex_unlock(&ocxlpmem->admin_command.lock);
> + return rc;
> +
> +}
> +
> +static int ioctl_error_log(struct ocxlpmem *ocxlpmem,
> + struct ioctl_ocxl_pmem_error_log __user *uarg)
> +{
> + struct ioctl_ocxl_pmem_error_log args;
> + int rc;
> +
> + if (copy_from_user(&args, uarg, sizeof(args)))
> + return -EFAULT;
> +
> + rc = read_error_log(ocxlpmem, &args, true);
> + if (rc)
> + return rc;
> +
> + if (copy_to_user(uarg, &args, sizeof(args)))
> + return -EFAULT;
> +
> + return 0;
> +}
> +
> +static long file_ioctl(struct file *file, unsigned int cmd, unsigned long args)
> +{
> + struct ocxlpmem *ocxlpmem = file->private_data;
> + int rc = -EINVAL;
> +
> + switch (cmd) {
> + case IOCTL_OCXL_PMEM_ERROR_LOG:
> + rc = ioctl_error_log(ocxlpmem,
> + (struct ioctl_ocxl_pmem_error_log __user *)args);
> + break;
> + }
> + return rc;
> +}
> +
> static const struct file_operations fops = {
> .owner = THIS_MODULE,
> .open = file_open,
> .release = file_release,
> + .unlocked_ioctl = file_ioctl,
> + .compat_ioctl = file_ioctl,
> };
>
> /**
> @@ -527,6 +736,60 @@ static int read_device_metadata(struct ocxlpmem *ocxlpmem)
> return 0;
> }
>
> +static const char *decode_error_log_type(u8 error_log_type)
> +{
> + switch (error_log_type) {
> + case 0x00:
> + return "general";
> + case 0x01:
> + return "predictive failure";
> + case 0x02:
> + return "thermal warning";
> + case 0x03:
> + return "data loss";
> + case 0x04:
> + return "health & performance";
> + default:
> + return "unknown";
> + }
> +}
> +
> +static void dump_error_log(struct ocxlpmem *ocxlpmem)
> +{
> + struct ioctl_ocxl_pmem_error_log log;
> + u32 buf_size;
> + u8 *buf;
> + int rc;
> +
> + if (ocxlpmem->admin_command.data_size == 0)
> + return;
> +
> + buf_size = ocxlpmem->admin_command.data_size - 0x48;
> + buf = kzalloc(buf_size, GFP_KERNEL);
> + if (!buf)
> + return;
> +
> + log.buf = buf;
> + log.buf_size = buf_size;
> +
> + rc = read_error_log(ocxlpmem, &log, false);
> + if (rc < 0)
> + goto out;
> +
> + dev_warn(&ocxlpmem->dev,
> + "OCXL PMEM Error log: WWID=0x%016llx%016llx LID=0x%x PRC=%x type=0x%x %s, Uptime=%u seconds timestamp=0x%llx\n",
> + log.wwid[0], log.wwid[1],
> + log.log_identifier, log.program_reference_code,
> + log.error_log_type,
> + decode_error_log_type(log.error_log_type),
> + log.power_on_seconds, log.timestamp);
> + print_hex_dump(KERN_WARNING, "buf", DUMP_PREFIX_OFFSET, 16, 1, buf,
> + log.buf_size, false); > +
> +out:
> + kfree(buf);
> +}
> +
> /**
> * probe_function0() - Set up function 0 for an OpenCAPI persistent memory device
> * This is important as it enables templates higher than 0 across all other functions,
> @@ -568,6 +831,7 @@ static int probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> struct ocxlpmem *ocxlpmem;
> int rc;
> u16 elapsed, timeout;
> + u64 chi;
>
> if (PCI_FUNC(pdev->devfn) == 0)
> return probe_function0(pdev);
> @@ -667,6 +931,11 @@ static int probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> return 0;
>
> err:
> + if (ocxlpmem &&
> + (ocxlpmem_chi(ocxlpmem, &chi) == 0) &&
> + (chi & GLOBAL_MMIO_CHI_ELA))
> + dump_error_log(ocxlpmem);
> +
> /*
> * Further cleanup is done in the release handler via free_ocxlpmem()
> * This allows us to keep the character device live to handle IOCTLs to
> diff --git a/arch/powerpc/platforms/powernv/pmem/ocxl_internal.h b/arch/powerpc/platforms/powernv/pmem/ocxl_internal.h
> index d2d81fec7bb1..b953ee522ed4 100644
> --- a/arch/powerpc/platforms/powernv/pmem/ocxl_internal.h
> +++ b/arch/powerpc/platforms/powernv/pmem/ocxl_internal.h
> @@ -5,6 +5,7 @@
> #include <linux/cdev.h>
> #include <misc/ocxl.h>
> #include <linux/libnvdimm.h>
> +#include <uapi/nvdimm/ocxl-pmem.h>
> #include <linux/mm.h>
>
> #define LABEL_AREA_SIZE (1UL << PA_SECTION_SHIFT)
> diff --git a/include/uapi/nvdimm/ocxl-pmem.h b/include/uapi/nvdimm/ocxl-pmem.h
> new file mode 100644
> index 000000000000..b10f8ac0c20f
> --- /dev/null
> +++ b/include/uapi/nvdimm/ocxl-pmem.h
> @@ -0,0 +1,46 @@
> +/* SPDX-License-Identifier: GPL-2.0+ WITH Linux-syscall-note */
> +/* Copyright 2017 IBM Corp. */
> +#ifndef _UAPI_OCXL_SCM_H
> +#define _UAPI_OCXL_SCM_H
> +
> +#include <linux/types.h>
> +#include <linux/ioctl.h>
> +
> +#define OCXL_PMEM_ERROR_LOG_ACTION_RESET (1 << (32-32))
> +#define OCXL_PMEM_ERROR_LOG_ACTION_CHKFW (1 << (53-32))
> +#define OCXL_PMEM_ERROR_LOG_ACTION_REPLACE (1 << (54-32))
> +#define OCXL_PMEM_ERROR_LOG_ACTION_DUMP (1 << (55-32))
> +
> +#define OCXL_PMEM_ERROR_LOG_TYPE_GENERAL (0x00)
> +#define OCXL_PMEM_ERROR_LOG_TYPE_PREDICTIVE_FAILURE (0x01)
> +#define OCXL_PMEM_ERROR_LOG_TYPE_THERMAL_WARNING (0x02)
> +#define OCXL_PMEM_ERROR_LOG_TYPE_DATA_LOSS (0x03)
> +#define OCXL_PMEM_ERROR_LOG_TYPE_HEALTH_PERFORMANCE (0x04)
> +
> +struct ioctl_ocxl_pmem_error_log {
> + __u32 log_identifier; /* out */
> + __u32 program_reference_code; /* out */
> + __u32 action_flags; /* out, recommended course of action */
> + __u32 power_on_seconds; /* out, Number of seconds the controller has been on when the error occurred */
> + __u64 timestamp; /* out, relative time since the current IPL */
> + __u64 wwid[2]; /* out, the NAA formatted WWID associated with the controller */
> + char fw_revision[8+1]; /* out, firmware revision as null terminated text */
> + __u16 buf_size; /* in/out, buffer size provided/required.
> + * If required is greater than provided, the buffer
> + * will be truncated to the amount provided. If its
> + * less, then only the required bytes will be populated.
> + * If it is 0, then there are no more error log entries.
> + */
> + __u8 error_log_type;
> + __u8 reserved1;
> + __u32 reserved2;
> + __u64 reserved3[2];
> + __u8 *buf; /* pointer to output buffer */
> +};
> +
> +/* ioctl numbers */
> +#define OCXL_PMEM_MAGIC 0x5C
> +/* SCM devices */
> +#define IOCTL_OCXL_PMEM_ERROR_LOG _IOWR(OCXL_PMEM_MAGIC, 0x01, struct ioctl_ocxl_pmem_error_log)
> +
> +#endif /* _UAPI_OCXL_SCM_H */
>
--
Andrew Donnellan OzLabs, ADL Canberra
ajd@linux.ibm.com IBM Australia Limited
^ permalink raw reply
* Re: [RFC 2/3] mm/vma: Introduce VM_ACCESS_FLAGS
From: Anshuman Khandual @ 2020-03-04 5:52 UTC (permalink / raw)
To: Vlastimil Babka, linux-mm
Cc: linux-sh, Catalin Marinas, Dave Hansen, Heiko Carstens, devel,
linux-s390, linux-c6x-dev, Yoshinori Sato, Russell King,
Ley Foon Tan, Mark Salter, Rob Springer, Thomas Gleixner,
Guan Xuetao, linux-arm-kernel, Nick Hu, Greg Kroah-Hartman,
linux-kernel, nios2-dev, Andrew Morton, linuxppc-dev
In-Reply-To: <52b4565f-2dab-c3e5-ead8-d76258f43a10@suse.cz>
On 03/03/2020 11:18 PM, Vlastimil Babka wrote:
> On 3/2/20 7:47 AM, Anshuman Khandual wrote:
>> There are many places where all basic VMA access flags (read, write, exec)
>> are initialized or checked against as a group. One such example is during
>> page fault. Existing vma_is_accessible() wrapper already creates the notion
>> of VMA accessibility as a group access permissions. Hence lets just create
>> VM_ACCESS_FLAGS (VM_READ|VM_WRITE|VM_EXEC) which will not only reduce code
>> duplication but also extend the VMA accessibility concept in general.
>>
>> Cc: Russell King <linux@armlinux.org.uk>
>> CC: Catalin Marinas <catalin.marinas@arm.com>
>> CC: Mark Salter <msalter@redhat.com>
>> Cc: Nick Hu <nickhu@andestech.com>
>> CC: Ley Foon Tan <ley.foon.tan@intel.com>
>> Cc: Michael Ellerman <mpe@ellerman.id.au>
>> Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
>> Cc: Yoshinori Sato <ysato@users.sourceforge.jp>
>> Cc: Guan Xuetao <gxt@pku.edu.cn>
>> Cc: Dave Hansen <dave.hansen@linux.intel.com>
>> Cc: Thomas Gleixner <tglx@linutronix.de>
>> Cc: Rob Springer <rspringer@google.com>
>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>> Cc: Andrew Morton <akpm@linux-foundation.org>
>> Cc: linux-arm-kernel@lists.infradead.org
>> Cc: linux-c6x-dev@linux-c6x.org
>> Cc: nios2-dev@lists.rocketboards.org
>> Cc: linuxppc-dev@lists.ozlabs.org
>> Cc: linux-s390@vger.kernel.org
>> Cc: linux-sh@vger.kernel.org
>> Cc: devel@driverdev.osuosl.org
>> Cc: linux-mm@kvack.org
>> Cc: linux-kernel@vger.kernel.org
>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
>
> Dunno. Such mask seems ok for testing flags, but it's a bit awkward when
> initializing flags, where it covers just one of many combinations that seem
> used. But no strong opinions, patch looks correct.
Fair enough. The fact that it covers only one of the many init combinations
used at various places, is indeed a good point. The page fault handlers does
start with VMA flags mask as VM_ACCESS_FLAGS, hence will keep them and drop
other init cases here.
^ permalink raw reply
* Re: [PATCH v3 20/27] powerpc/powernv/pmem: Forward events to userspace
From: Alastair D'Silva @ 2020-03-04 5:48 UTC (permalink / raw)
To: Andrew Donnellan
Cc: Madhavan Srinivasan, Alexey Kardashevskiy, Masahiro Yamada,
Oliver O'Halloran, Mauro Carvalho Chehab, Ira Weiny,
Thomas Gleixner, Rob Herring, Dave Jiang, linux-nvdimm,
Aneesh Kumar K . V, Krzysztof Kozlowski, Anju T Sudhakar,
Mahesh Salgaonkar, Arnd Bergmann, Greg Kurz, Nicholas Piggin,
Cédric Le Goater, Dan Williams, Hari Bathini, linux-mm,
Greg Kroah-Hartman, linux-kernel, Vishal Verma, Frederic Barrat,
Paul Mackerras, Andrew Morton, linuxppc-dev, David S. Miller
In-Reply-To: <d50b19fc-b88a-b4de-4cc5-07790a4ddac0@linux.ibm.com>
On Tue, 2020-03-03 at 18:02 +1100, Andrew Donnellan wrote:
> On 21/2/20 2:27 pm, Alastair D'Silva wrote:> @@ -938,6 +955,51 @@
> static
> int ioctl_controller_stats(struct ocxlpmem *ocxlpmem,
> > return rc;
> > }
> >
> > +static int ioctl_eventfd(struct ocxlpmem *ocxlpmem,
> > + struct ioctl_ocxl_pmem_eventfd __user *uarg)
> > +{
> > + struct ioctl_ocxl_pmem_eventfd args;
> > +
> > + if (copy_from_user(&args, uarg, sizeof(args)))
> > + return -EFAULT;
> > +
> > + if (ocxlpmem->ev_ctx)
> > + return -EINVAL;
>
> I think EBUSY is more appropriate here.
>
Ok
> > +
> > + ocxlpmem->ev_ctx = eventfd_ctx_fdget(args.eventfd);
> > + if (!ocxlpmem->ev_ctx)
> > + return -EFAULT;
> > +
> > + return 0;
> > +}
> > +
> > +static int ioctl_event_check(struct ocxlpmem *ocxlpmem, u64 __user
> > *uarg)
> > +{
> > + u64 val = 0;
> > + int rc;
> > + u64 chi = 0;
> > +
> > + rc = ocxlpmem_chi(ocxlpmem, &chi);
> > + if (rc < 0)
> > + return rc;
> > +
> > + if (chi & GLOBAL_MMIO_CHI_ELA)
> > + val |= IOCTL_OCXL_PMEM_EVENT_ERROR_LOG_AVAILABLE;
> > +
> > + if (chi & GLOBAL_MMIO_CHI_CDA)
> > + val |= IOCTL_OCXL_PMEM_EVENT_CONTROLLER_DUMP_AVAILABLE;
> > +
> > + if (chi & GLOBAL_MMIO_CHI_CFFS)
> > + val |= IOCTL_OCXL_PMEM_EVENT_FIRMWARE_FATAL;
> > +
> > + if (chi & GLOBAL_MMIO_CHI_CHFS)
> > + val |= IOCTL_OCXL_PMEM_EVENT_HARDWARE_FATAL;
> > +
> > + rc = copy_to_user((u64 __user *) uarg, &val, sizeof(val));
> > +
> > + return rc;
> > +}
> > +
> > static long file_ioctl(struct file *file, unsigned int cmd,
> > unsigned long args)
> > {
> > struct ocxlpmem *ocxlpmem = file->private_data;
> > @@ -966,6 +1028,15 @@ static long file_ioctl(struct file *file,
> > unsigned int cmd, unsigned long args)
> > rc = ioctl_controller_stats(ocxlpmem,
> > (struct
> > ioctl_ocxl_pmem_controller_stats __user *)args);
> > break;
> > +
> > + case IOCTL_OCXL_PMEM_EVENTFD:
> > + rc = ioctl_eventfd(ocxlpmem,
> > + (struct ioctl_ocxl_pmem_eventfd
> > __user *)args);
> > + break;
> > +
> > + case IOCTL_OCXL_PMEM_EVENT_CHECK:
> > + rc = ioctl_event_check(ocxlpmem, (u64 __user *)args);
> > + break;
> > }
> >
> > return rc;
> > @@ -1107,6 +1178,146 @@ static void dump_error_log(struct ocxlpmem
> > *ocxlpmem)
> > kfree(buf);
> > }
> >
> > +static irqreturn_t imn0_handler(void *private)
> > +{
> > + struct ocxlpmem *ocxlpmem = private;
> > + u64 chi = 0;
> > +
> > + (void)ocxlpmem_chi(ocxlpmem, &chi);
> > +
> > + if (chi & GLOBAL_MMIO_CHI_ELA) {
> > + dev_warn(&ocxlpmem->dev, "Error log is available\n");
> > +
> > + if (ocxlpmem->ev_ctx)
> > + eventfd_signal(ocxlpmem->ev_ctx, 1);
> > + }
> > +
> > + if (chi & GLOBAL_MMIO_CHI_CDA) {
> > + dev_warn(&ocxlpmem->dev, "Controller dump is
> > available\n");
> > +
> > + if (ocxlpmem->ev_ctx)
> > + eventfd_signal(ocxlpmem->ev_ctx, 1);
> > + }
> > +
> > +
> > + return IRQ_HANDLED;
> > +}
> > +
> > +static irqreturn_t imn1_handler(void *private)
> > +{
> > + struct ocxlpmem *ocxlpmem = private;
> > + u64 chi = 0;
> > +
> > + (void)ocxlpmem_chi(ocxlpmem, &chi);
> > +
> > + if (chi & (GLOBAL_MMIO_CHI_CFFS | GLOBAL_MMIO_CHI_CHFS)) {
> > + dev_err(&ocxlpmem->dev,
> > + "Controller status is fatal, chi=0x%llx, going
> > offline\n", chi);
> > +
> > + if (ocxlpmem->nvdimm_bus) {
> > + nvdimm_bus_unregister(ocxlpmem->nvdimm_bus);
> > + ocxlpmem->nvdimm_bus = NULL;
> > + }
> > +
> > + if (ocxlpmem->ev_ctx)
> > + eventfd_signal(ocxlpmem->ev_ctx, 1);
> > + }
> > +
> > + return IRQ_HANDLED;
> > +}
> > +
> > +
> > +/**
> > + * ocxlpmem_setup_irq() - Set up the IRQs for the OpenCAPI
> > Persistent Memory device
> > + * @ocxlpmem: the device metadata
> > + * Return: 0 on success, negative on failure
> > + */
> > +static int ocxlpmem_setup_irq(struct ocxlpmem *ocxlpmem)
> > +{
> > + int rc;
> > + u64 irq_addr;
> > +
> > + rc = ocxl_afu_irq_alloc(ocxlpmem->ocxl_context, &ocxlpmem-
> > >irq_id[0]);
> > + if (rc)
> > + return rc;
> > +
> > + rc = ocxl_irq_set_handler(ocxlpmem->ocxl_context, ocxlpmem-
> > >irq_id[0],
> > + imn0_handler, NULL, ocxlpmem);
> > +
> > + irq_addr = ocxl_afu_irq_get_addr(ocxlpmem->ocxl_context,
> > ocxlpmem->irq_id[0]);
> > + if (!irq_addr)
> > + return -EINVAL;
> > +
> > + ocxlpmem->irq_addr[0] = ioremap(irq_addr, PAGE_SIZE);
> > + if (!ocxlpmem->irq_addr[0])
> > + return -EINVAL;
>
> Something other than EINVAL for these two
Ok
>
> > +
> > + rc = ocxl_global_mmio_write64(ocxlpmem->ocxl_afu,
> > GLOBAL_MMIO_IMA0_OHP,
> > + OCXL_LITTLE_ENDIAN,
> > + (u64)ocxlpmem->irq_addr[0]);
> > + if (rc)
> > + goto out_irq0;
> > +
> > + rc = ocxl_global_mmio_write64(ocxlpmem->ocxl_afu,
> > GLOBAL_MMIO_IMA0_CFP,
> > + OCXL_LITTLE_ENDIAN, 0);
> > + if (rc)
> > + goto out_irq0;
> > +
> > + rc = ocxl_afu_irq_alloc(ocxlpmem->ocxl_context, &ocxlpmem-
> > >irq_id[1]);
> > + if (rc)
> > + goto out_irq0;
> > +
> > +
> > + rc = ocxl_irq_set_handler(ocxlpmem->ocxl_context, ocxlpmem-
> > >irq_id[1],
> > + imn1_handler, NULL, ocxlpmem);
> > + if (rc)
> > + goto out_irq0;
> > +
> > + irq_addr = ocxl_afu_irq_get_addr(ocxlpmem->ocxl_context,
> > ocxlpmem->irq_id[1]);
> > + if (!irq_addr) {
> > + rc = -EFAULT;
> > + goto out_irq0;
> > + }
> > +
> > + ocxlpmem->irq_addr[1] = ioremap(irq_addr, PAGE_SIZE);
> > + if (!ocxlpmem->irq_addr[1]) {
> > + rc = -EINVAL;
> > + goto out_irq0;
> > + }
> > +
> > + rc = ocxl_global_mmio_write64(ocxlpmem->ocxl_afu,
> > GLOBAL_MMIO_IMA1_OHP,
> > + OCXL_LITTLE_ENDIAN,
> > + (u64)ocxlpmem->irq_addr[1]);
> > + if (rc)
> > + goto out_irq1;
> > +
> > + rc = ocxl_global_mmio_write64(ocxlpmem->ocxl_afu,
> > GLOBAL_MMIO_IMA1_CFP,
> > + OCXL_LITTLE_ENDIAN, 0);
> > + if (rc)
> > + goto out_irq1;
> > +
> > + // Enable doorbells
> > + rc = ocxl_global_mmio_set64(ocxlpmem->ocxl_afu,
> > GLOBAL_MMIO_CHIE,
> > + OCXL_LITTLE_ENDIAN,
> > + GLOBAL_MMIO_CHI_ELA |
> > GLOBAL_MMIO_CHI_CDA |
> > + GLOBAL_MMIO_CHI_CFFS |
> > GLOBAL_MMIO_CHI_CHFS |
> > + GLOBAL_MMIO_CHI_NSCRA);
>
> We don't actually do anything in the handlers with NSCRA...
Good catch, this belongs in the overwrite patch (which was dropped from
this series).
>
> > + if (rc)
> > + goto out_irq1;
> > +
> > + return 0;
> > +
> > +out_irq1:
> > + iounmap(ocxlpmem->irq_addr[1]);
> > + ocxlpmem->irq_addr[1] = NULL;
> > +
> > +out_irq0:
> > + iounmap(ocxlpmem->irq_addr[0]);
> > + ocxlpmem->irq_addr[0] = NULL;
> > +
> > + return rc;
> > +}
> > +
> > /**
> > * probe_function0() - Set up function 0 for an OpenCAPI
> > persistent memory device
> > * This is important as it enables templates higher than 0 across
> > all other functions,
> > @@ -1216,6 +1427,11 @@ static int probe(struct pci_dev *pdev, const
> > struct pci_device_id *ent)
> > goto err;
> > }
> >
> > + if (ocxlpmem_setup_irq(ocxlpmem)) {
> > + dev_err(&pdev->dev, "Could not set up OCXL IRQs\n");
> > + goto err;
> > + }
> > +
> > if (setup_command_metadata(ocxlpmem)) {
> > dev_err(&pdev->dev, "Could not read OCXL command
> > matada\n");
> > goto err;
> > diff --git a/arch/powerpc/platforms/powernv/pmem/ocxl_internal.h
> > b/arch/powerpc/platforms/powernv/pmem/ocxl_internal.h
> > index b953ee522ed4..927690f4888f 100644
> > --- a/arch/powerpc/platforms/powernv/pmem/ocxl_internal.h
> > +++ b/arch/powerpc/platforms/powernv/pmem/ocxl_internal.h
> > @@ -103,6 +103,10 @@ struct ocxlpmem {
> > struct pci_dev *pdev;
> > struct cdev cdev;
> > struct ocxl_fn *ocxl_fn;
> > +#define SCM_IRQ_COUNT 2
> > + int irq_id[SCM_IRQ_COUNT];
> > + struct dev_pagemap irq_pgmap[SCM_IRQ_COUNT];
> > + void *irq_addr[SCM_IRQ_COUNT];
>
> I think this should be tagged __iomem
>
Ok
--
Alastair D'Silva
Open Source Developer
Linux Technology Centre, IBM Australia
mob: 0423 762 819
^ permalink raw reply
* Re: [PATCH v3 03/27] powerpc: Map & release OpenCAPI LPC memory
From: Alastair D'Silva @ 2020-03-04 5:33 UTC (permalink / raw)
To: Andrew Donnellan
Cc: Madhavan Srinivasan, Alexey Kardashevskiy, Masahiro Yamada,
Oliver O'Halloran, Mauro Carvalho Chehab, Ira Weiny,
Thomas Gleixner, Rob Herring, Dave Jiang, linux-nvdimm,
Aneesh Kumar K . V, Krzysztof Kozlowski, Anju T Sudhakar,
Mahesh Salgaonkar, Arnd Bergmann, Greg Kurz, Nicholas Piggin,
Cédric Le Goater, Dan Williams, Hari Bathini, linux-mm,
Greg Kroah-Hartman, linux-kernel, Vishal Verma, Frederic Barrat,
Paul Mackerras, Andrew Morton, linuxppc-dev, David S. Miller
In-Reply-To: <33ff636c-6b85-ed0d-275b-3e8697b5316f@linux.ibm.com>
On Tue, 2020-03-03 at 17:10 +1100, Andrew Donnellan wrote:
> On 21/2/20 2:26 pm, Alastair D'Silva wrote:> +#ifdef
> CONFIG_MEMORY_HOTPLUG_SPARSE
> > +u64 pnv_ocxl_platform_lpc_setup(struct pci_dev *pdev, u64 size)
> > +{
> > + struct pci_controller *hose = pci_bus_to_host(pdev->bus);
> > + struct pnv_phb *phb = hose->private_data;
> > + u32 bdfn = pci_dev_id(pdev);
> > + __be64 base_addr_be64;
> > + u64 base_addr;
> > + int rc;
> > +
> > + rc = opal_npu_mem_alloc(phb->opal_id, bdfn, size,
> > &base_addr_be64);
>
> Sparse warning:
>
> https://openpower.xyz/job/snowpatch/job/snowpatch-linux-sparse/15776//artifact/linux/report.txt
>
> I think in patch 1 we need to change a uint64_t to a __be64.
>
Ok, thanks
--
Alastair D'Silva
Open Source Developer
Linux Technology Centre, IBM Australia
mob: 0423 762 819
^ permalink raw reply
* Re: [RFC 1/3] mm/vma: Define a default value for VM_DATA_DEFAULT_FLAGS
From: Anshuman Khandual @ 2020-03-04 5:19 UTC (permalink / raw)
To: Vlastimil Babka, linux-mm
Cc: Rich Felker, linux-ia64, linux-sh, Catalin Marinas,
Heiko Carstens, linux-mips, James E.J. Bottomley, Guo Ren,
sparclinux, linux-hexagon, linux-riscv, Jonas Bonn, linux-s390,
linux-snps-arc, linux-c6x-dev, Brian Cain, Russell King,
Ley Foon Tan, Geert Uytterhoeven, linux-parisc, Mark Salter,
Paul Burton, uclinux-h8-devel, linux-xtensa, Jeff Dike, linux-um,
linux-m68k, openrisc, Paul Walmsley, Thomas Gleixner, Guan Xuetao,
linux-arm-kernel, Richard Henderson, Chris Zankel, Michal Simek,
Tony Luck, Yoshinori Sato, Nick Hu, Vineet Gupta, linux-kernel,
Ralf Baechle, linux-alpha, nios2-dev, Andrew Morton, linuxppc-dev,
David S. Miller
In-Reply-To: <b243be54-7b5e-c6e9-fb68-46369d7d7aa4@suse.cz>
On 03/03/2020 10:55 PM, Vlastimil Babka wrote:
> On 3/2/20 7:47 AM, Anshuman Khandual wrote:
>> There are many platforms with exact same value for VM_DATA_DEFAULT_FLAGS
>> This creates a default value for VM_DATA_DEFAULT_FLAGS in line with the
>> existing VM_STACK_DEFAULT_FLAGS. While here, also define some more macros
>> with standard VMA access flag combinations that are used frequently across
>> many platforms. Apart from simplification, this reduces code duplication
>> as well.
>>
>> Cc: Richard Henderson <rth@twiddle.net>
>> Cc: Vineet Gupta <vgupta@synopsys.com>
>> Cc: Russell King <linux@armlinux.org.uk>
>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>> Cc: Mark Salter <msalter@redhat.com>
>> Cc: Guo Ren <guoren@kernel.org>
>> Cc: Yoshinori Sato <ysato@users.sourceforge.jp>
>> Cc: Brian Cain <bcain@codeaurora.org>
>> Cc: Tony Luck <tony.luck@intel.com>
>> Cc: Geert Uytterhoeven <geert@linux-m68k.org>
>> Cc: Michal Simek <monstr@monstr.eu>
>> Cc: Ralf Baechle <ralf@linux-mips.org>
>> Cc: Paul Burton <paulburton@kernel.org>
>> Cc: Nick Hu <nickhu@andestech.com>
>> Cc: Ley Foon Tan <ley.foon.tan@intel.com>
>> Cc: Jonas Bonn <jonas@southpole.se>
>> Cc: "James E.J. Bottomley" <James.Bottomley@HansenPartnership.com>
>> Cc: Michael Ellerman <mpe@ellerman.id.au>
>> Cc: Paul Walmsley <paul.walmsley@sifive.com>
>> Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
>> Cc: Rich Felker <dalias@libc.org>
>> Cc: "David S. Miller" <davem@davemloft.net>
>> Cc: Guan Xuetao <gxt@pku.edu.cn>
>> Cc: Thomas Gleixner <tglx@linutronix.de>
>> Cc: Jeff Dike <jdike@addtoit.com>
>> Cc: Chris Zankel <chris@zankel.net>
>> Cc: Andrew Morton <akpm@linux-foundation.org>
>> Cc: linux-alpha@vger.kernel.org
>> Cc: linux-kernel@vger.kernel.org
>> Cc: linux-snps-arc@lists.infradead.org
>> Cc: linux-arm-kernel@lists.infradead.org
>> Cc: linux-c6x-dev@linux-c6x.org
>> Cc: uclinux-h8-devel@lists.sourceforge.jp
>> Cc: linux-hexagon@vger.kernel.org
>> Cc: linux-ia64@vger.kernel.org
>> Cc: linux-m68k@lists.linux-m68k.org
>> Cc: linux-mips@vger.kernel.org
>> Cc: nios2-dev@lists.rocketboards.org
>> Cc: openrisc@lists.librecores.org
>> Cc: linux-parisc@vger.kernel.org
>> Cc: linuxppc-dev@lists.ozlabs.org
>> Cc: linux-riscv@lists.infradead.org
>> Cc: linux-s390@vger.kernel.org
>> Cc: linux-sh@vger.kernel.org
>> Cc: sparclinux@vger.kernel.org
>> Cc: linux-um@lists.infradead.org
>> Cc: linux-xtensa@linux-xtensa.org
>> Cc: linux-mm@kvack.org
>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com
> Reviewed-by: Vlastimil Babka <vbabka@suse.cz>
>
> Nit:
>
>> diff --git a/include/linux/mm.h b/include/linux/mm.h
>> index b0e53ef13ff1..7a764ae6ab68 100644
>> --- a/include/linux/mm.h
>> +++ b/include/linux/mm.h
>> @@ -342,6 +342,21 @@ extern unsigned int kobjsize(const void *objp);
>> /* Bits set in the VMA until the stack is in its final location */
>> #define VM_STACK_INCOMPLETE_SETUP (VM_RAND_READ | VM_SEQ_READ)
>>
>> +#define TASK_EXEC ((current->personality & READ_IMPLIES_EXEC) ? VM_EXEC : 0)
>> +
>> +/* Common data flag combinations */
>> +#define VM_DATA_FLAGS_TSK_EXEC (VM_READ | VM_WRITE | TASK_EXEC | \
>> + VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
>> +#define VM_DATA_FLAGS_NON_EXEC (VM_READ | VM_WRITE | VM_MAYREAD | \
>> + VM_MAYWRITE | VM_MAYEXEC)
>> +#define VM_DATA_FLAGS_EXEC (VM_READ | VM_WRITE | VM_EXEC | \
>> + VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
>> +
>> +#ifndef VM_DATA_DEFAULT_FLAGS /* arch can override this */
>> +#define VM_DATA_DEFAULT_FLAGS (VM_READ | VM_WRITE | VM_EXEC | \
>> + VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
>
> Should you use VM_DATA_FLAGS_EXEC here? Yeah one more macro to expand, but it's
> right above this.
Sure, can do that.
>
>> +#endif
>> +
>> #ifndef VM_STACK_DEFAULT_FLAGS /* arch can override this */
>> #define VM_STACK_DEFAULT_FLAGS VM_DATA_DEFAULT_FLAGS
>> #endif
>>
>
>
>
^ permalink raw reply
* Re: [PATCH 1/1] powerpc/kernel: Enables memory hot-remove after reboot on pseries guests
From: Bharata B Rao @ 2020-03-04 4:43 UTC (permalink / raw)
To: Leonardo Bras
Cc: ndfont, linux-kernel@vger.kernel.org, arbab, Claudio Carvalho,
Michael Roth, Paul Mackerras, Greg Kroah-Hartman, Thomas Gleixner,
linuxppc-dev, Hari Bathini
In-Reply-To: <20200228060439.52749-1-leonardo@linux.ibm.com>
On Fri, Feb 28, 2020 at 11:36 AM Leonardo Bras <leonardo@linux.ibm.com> wrote:
>
> While providing guests, it's desirable to resize it's memory on demand.
>
> By now, it's possible to do so by creating a guest with a small base
> memory, hot-plugging all the rest, and using 'movable_node' kernel
> command-line parameter, which puts all hot-plugged memory in
> ZONE_MOVABLE, allowing it to be removed whenever needed.
>
> But there is an issue regarding guest reboot:
> If memory is hot-plugged, and then the guest is rebooted, all hot-plugged
> memory goes to ZONE_NORMAL, which offers no guaranteed hot-removal.
> It usually prevents this memory to be hot-removed from the guest.
>
> It's possible to use device-tree information to fix that behavior, as
> it stores flags for LMB ranges on ibm,dynamic-memory-vN.
> It involves marking each memblock with the correct flags as hotpluggable
> memory, which mm/memblock.c puts in ZONE_MOVABLE during boot if
> 'movable_node' is passed.
>
> For base memory, qemu assigns these flags for it's LMBs:
> (DRCONF_MEM_AI_INVALID | DRCONF_MEM_RESERVED)
> For hot-plugged memory, it assigns (DRCONF_MEM_ASSIGNED).
>
> While guest kernel reads the device-tree, early_init_drmem_lmb() is
> called for every added LMBs, doing nothing for base memory, and adding
> memblocks for hot-plugged memory. Skipping base memory happens here:
>
> if ((lmb->flags & DRCONF_MEM_RESERVED) ||
> !(lmb->flags & DRCONF_MEM_ASSIGNED))
> return;
>
> Marking memblocks added by this function as hotplugable memory
> is enough to get the desirable behavior, and should cause no change
> if 'movable_node' parameter is not passed to kernel.
>
> Signed-off-by: Leonardo Bras <leonardo@linux.ibm.com>
> ---
> arch/powerpc/kernel/prom.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
> index 6620f37abe73..f4d14c67bf53 100644
> --- a/arch/powerpc/kernel/prom.c
> +++ b/arch/powerpc/kernel/prom.c
> @@ -518,6 +518,8 @@ static void __init early_init_drmem_lmb(struct drmem_lmb *lmb,
> DBG("Adding: %llx -> %llx\n", base, size);
> if (validate_mem_limit(base, &size))
> memblock_add(base, size);
> +
> + early_init_dt_mark_hotplug_memory_arch(base, size);
Hi,
I tried this a few years back
(https://patchwork.ozlabs.org/patch/800142/) and didn't pursue it
further because at that time, it was felt that the approach might not
work for PowerVM guests, because all the present memory except RMA
gets marked as hot-pluggable by PowerVM. This discussion is not
present in the above thread, but during my private discussions with
Reza and Nathan, it was noted that making all that memory as MOVABLE
is not preferable for PowerVM guests as we might run out of memory for
kernel allocations.
Regards,
Bharata.
--
http://raobharata.wordpress.com/
^ permalink raw reply
* RE: [PATCH v3 15/27] powerpc/powernv/pmem: Add support for near storage commands
From: Alastair D'Silva @ 2020-03-04 4:42 UTC (permalink / raw)
To: Dan Williams, Frederic Barrat
Cc: Madhavan Srinivasan, Alexey Kardashevskiy, Masahiro Yamada,
Oliver O'Halloran, Mauro Carvalho Chehab, Ira Weiny,
Rob Herring, Dave Jiang, linux-nvdimm, Aneesh Kumar K . V,
Krzysztof Kozlowski, Anju T Sudhakar, Mahesh Salgaonkar,
Andrew Donnellan, Arnd Bergmann, Greg Kurz, Nicholas Piggin,
Cédric Le Goater, Thomas Gleixner, Hari Bathini, Linux MM,
Greg Kroah-Hartman, Linux Kernel Mailing List, Vishal Verma,
Paul Mackerras, Andrew Morton, linuxppc-dev, David S. Miller
In-Reply-To: <CAPcyv4gCCjQFnLaSpRPEuKoDq3gOHSxjxLT_=X3N_nr=2ZOcSA@mail.gmail.com>
On Mon, 2020-03-02 at 10:42 -0800, Dan Williams wrote:
> On Mon, Mar 2, 2020 at 9:59 AM Frederic Barrat <fbarrat@linux.ibm.com
> > wrote:
> >
> >
> > Le 21/02/2020 à 04:27, Alastair D'Silva a écrit :
> > > From: Alastair D'Silva <alastair@d-silva.org>
> > >
> > > Similar to the previous patch, this adds support for near storage
> > > commands.
> > >
> > > Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
> > > ---
> >
> > Is any of these new functions ever called?
>
> This is my concern as well. The libnvdimm command support is limited
> to the commands that Linux will use. Other passthrough commands are
> supported through a passthrough interface. However, that passthrough
> interface is explicitly limited to publicly documented command sets
> so
> that the kernel has an opportunity to constrain and consolidate
> command implementations across vendors.
It will be in the patch that implements overwrite. I moved that patch
out of this series, as it needs more testing, so I guess I can submit
this alongside it.
--
Alastair D'Silva
Open Source Developer
Linux Technology Centre, IBM Australia
mob: 0423 762 819
^ permalink raw reply
* [PATCH v2 6/6] asm-generic/tlb: avoid potential double flush
From: Santosh Sivaraj @ 2020-03-04 4:30 UTC (permalink / raw)
To: stable, linuxppc-dev
Cc: Sasha Levin, Peter Zijlstra, Aneesh Kumar K . V, Greg KH
In-Reply-To: <20200304043028.280136-1-santosh@fossix.org>
From: Peter Zijlstra <peterz@infradead.org>
commit 0758cd8304942292e95a0f750c374533db378b32 upstream.
Aneesh reported that:
tlb_flush_mmu()
tlb_flush_mmu_tlbonly()
tlb_flush() <-- #1
tlb_flush_mmu_free()
tlb_table_flush()
tlb_table_invalidate()
tlb_flush_mmu_tlbonly()
tlb_flush() <-- #2
does two TLBIs when tlb->fullmm, because __tlb_reset_range() will not
clear tlb->end in that case.
Observe that any caller to __tlb_adjust_range() also sets at least one of
the tlb->freed_tables || tlb->cleared_p* bits, and those are
unconditionally cleared by __tlb_reset_range().
Change the condition for actually issuing TLBI to having one of those bits
set, as opposed to having tlb->end != 0.
Link: http://lkml.kernel.org/r/20200116064531.483522-4-aneesh.kumar@linux.ibm.com
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
Reported-by: "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com>
Cc: <stable@vger.kernel.org> # 4.19
Signed-off-by: Santosh Sivaraj <santosh@fossix.org>
[santosh: backported to 4.19 stable]
---
include/asm-generic/tlb.h | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/include/asm-generic/tlb.h b/include/asm-generic/tlb.h
index 19934cdd143e..427a70c56ddd 100644
--- a/include/asm-generic/tlb.h
+++ b/include/asm-generic/tlb.h
@@ -179,7 +179,12 @@ static inline void __tlb_reset_range(struct mmu_gather *tlb)
static inline void tlb_flush_mmu_tlbonly(struct mmu_gather *tlb)
{
- if (!tlb->end)
+ /*
+ * Anything calling __tlb_adjust_range() also sets at least one of
+ * these bits.
+ */
+ if (!(tlb->freed_tables || tlb->cleared_ptes || tlb->cleared_pmds ||
+ tlb->cleared_puds || tlb->cleared_p4ds))
return;
tlb_flush(tlb);
--
2.24.1
^ permalink raw reply related
* [PATCH v2 5/6] mm/mmu_gather: invalidate TLB correctly on batch allocation failure and flush
From: Santosh Sivaraj @ 2020-03-04 4:30 UTC (permalink / raw)
To: stable, linuxppc-dev
Cc: Sasha Levin, Peter Zijlstra, Aneesh Kumar K . V, Greg KH
In-Reply-To: <20200304043028.280136-1-santosh@fossix.org>
From: Peter Zijlstra <peterz@infradead.org>
commit 0ed1325967ab5f7a4549a2641c6ebe115f76e228 upstream.
Architectures for which we have hardware walkers of Linux page table
should flush TLB on mmu gather batch allocation failures and batch flush.
Some architectures like POWER supports multiple translation modes (hash
and radix) and in the case of POWER only radix translation mode needs the
above TLBI. This is because for hash translation mode kernel wants to
avoid this extra flush since there are no hardware walkers of linux page
table. With radix translation, the hardware also walks linux page table
and with that, kernel needs to make sure to TLB invalidate page walk cache
before page table pages are freed.
More details in commit d86564a2f085 ("mm/tlb, x86/mm: Support invalidating
TLB caches for RCU_TABLE_FREE")
The changes to sparc are to make sure we keep the old behavior since we
are now removing HAVE_RCU_TABLE_NO_INVALIDATE. The default value for
tlb_needs_table_invalidate is to always force an invalidate and sparc can
avoid the table invalidate. Hence we define tlb_needs_table_invalidate to
false for sparc architecture.
Link: http://lkml.kernel.org/r/20200116064531.483522-3-aneesh.kumar@linux.ibm.com
Fixes: a46cc7a90fd8 ("powerpc/mm/radix: Improve TLB/PWC flushes")
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
Cc: <stable@vger.kernel.org> # 4.19
Signed-off-by: Santosh Sivaraj <santosh@fossix.org>
[santosh: backported to 4.19 stable]
---
arch/Kconfig | 3 ---
arch/powerpc/Kconfig | 1 -
arch/powerpc/include/asm/tlb.h | 11 +++++++++++
arch/sparc/Kconfig | 1 -
arch/sparc/include/asm/tlb_64.h | 9 +++++++++
include/asm-generic/tlb.h | 15 +++++++++++++++
mm/memory.c | 16 ++++++++--------
7 files changed, 43 insertions(+), 13 deletions(-)
diff --git a/arch/Kconfig b/arch/Kconfig
index 061a12b8140e..3abbdb0cea44 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -363,9 +363,6 @@ config HAVE_ARCH_JUMP_LABEL
config HAVE_RCU_TABLE_FREE
bool
-config HAVE_RCU_TABLE_NO_INVALIDATE
- bool
-
config ARCH_HAVE_NMI_SAFE_CMPXCHG
bool
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 1a00ce4b0040..e5bc0cfea2b1 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -216,7 +216,6 @@ config PPC
select HAVE_PERF_REGS
select HAVE_PERF_USER_STACK_DUMP
select HAVE_RCU_TABLE_FREE
- select HAVE_RCU_TABLE_NO_INVALIDATE if HAVE_RCU_TABLE_FREE
select HAVE_REGS_AND_STACK_ACCESS_API
select HAVE_RELIABLE_STACKTRACE if PPC64 && CPU_LITTLE_ENDIAN
select HAVE_SYSCALL_TRACEPOINTS
diff --git a/arch/powerpc/include/asm/tlb.h b/arch/powerpc/include/asm/tlb.h
index f0e571b2dc7c..63418275f402 100644
--- a/arch/powerpc/include/asm/tlb.h
+++ b/arch/powerpc/include/asm/tlb.h
@@ -30,6 +30,17 @@
#define tlb_remove_check_page_size_change tlb_remove_check_page_size_change
extern void tlb_flush(struct mmu_gather *tlb);
+/*
+ * book3s:
+ * Hash does not use the linux page-tables, so we can avoid
+ * the TLB invalidate for page-table freeing, Radix otoh does use the
+ * page-tables and needs the TLBI.
+ *
+ * nohash:
+ * We still do TLB invalidate in the __pte_free_tlb routine before we
+ * add the page table pages to mmu gather table batch.
+ */
+#define tlb_needs_table_invalidate() radix_enabled()
/* Get the generic bits... */
#include <asm-generic/tlb.h>
diff --git a/arch/sparc/Kconfig b/arch/sparc/Kconfig
index d90d632868aa..e6f2a38d2e61 100644
--- a/arch/sparc/Kconfig
+++ b/arch/sparc/Kconfig
@@ -64,7 +64,6 @@ config SPARC64
select HAVE_KRETPROBES
select HAVE_KPROBES
select HAVE_RCU_TABLE_FREE if SMP
- select HAVE_RCU_TABLE_NO_INVALIDATE if HAVE_RCU_TABLE_FREE
select HAVE_MEMBLOCK_NODE_MAP
select HAVE_ARCH_TRANSPARENT_HUGEPAGE
select HAVE_DYNAMIC_FTRACE
diff --git a/arch/sparc/include/asm/tlb_64.h b/arch/sparc/include/asm/tlb_64.h
index a2f3fa61ee36..8cb8f3833239 100644
--- a/arch/sparc/include/asm/tlb_64.h
+++ b/arch/sparc/include/asm/tlb_64.h
@@ -28,6 +28,15 @@ void flush_tlb_pending(void);
#define __tlb_remove_tlb_entry(tlb, ptep, address) do { } while (0)
#define tlb_flush(tlb) flush_tlb_pending()
+/*
+ * SPARC64's hardware TLB fill does not use the Linux page-tables
+ * and therefore we don't need a TLBI when freeing page-table pages.
+ */
+
+#ifdef CONFIG_HAVE_RCU_TABLE_FREE
+#define tlb_needs_table_invalidate() (false)
+#endif
+
#include <asm-generic/tlb.h>
#endif /* _SPARC64_TLB_H */
diff --git a/include/asm-generic/tlb.h b/include/asm-generic/tlb.h
index f2b9dc9cbaf8..19934cdd143e 100644
--- a/include/asm-generic/tlb.h
+++ b/include/asm-generic/tlb.h
@@ -61,8 +61,23 @@ struct mmu_table_batch {
extern void tlb_table_flush(struct mmu_gather *tlb);
extern void tlb_remove_table(struct mmu_gather *tlb, void *table);
+/*
+ * This allows an architecture that does not use the linux page-tables for
+ * hardware to skip the TLBI when freeing page tables.
+ */
+#ifndef tlb_needs_table_invalidate
+#define tlb_needs_table_invalidate() (true)
#endif
+#else
+
+#ifdef tlb_needs_table_invalidate
+#error tlb_needs_table_invalidate() requires HAVE_RCU_TABLE_FREE
+#endif
+
+#endif /* CONFIG_HAVE_RCU_TABLE_FREE */
+
+
/*
* If we can't allocate a page to make a big batch of page pointers
* to work on, then just handle a few from the on-stack structure.
diff --git a/mm/memory.c b/mm/memory.c
index ba5689610c04..7daa7ae1b046 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -327,14 +327,14 @@ bool __tlb_remove_page_size(struct mmu_gather *tlb, struct page *page, int page_
*/
static inline void tlb_table_invalidate(struct mmu_gather *tlb)
{
-#ifndef CONFIG_HAVE_RCU_TABLE_NO_INVALIDATE
- /*
- * Invalidate page-table caches used by hardware walkers. Then we still
- * need to RCU-sched wait while freeing the pages because software
- * walkers can still be in-flight.
- */
- tlb_flush_mmu_tlbonly(tlb);
-#endif
+ if (tlb_needs_table_invalidate()) {
+ /*
+ * Invalidate page-table caches used by hardware walkers. Then
+ * we still need to RCU-sched wait while freeing the pages
+ * because software walkers can still be in-flight.
+ */
+ tlb_flush_mmu_tlbonly(tlb);
+ }
}
static void tlb_remove_table_smp_sync(void *arg)
--
2.24.1
^ permalink raw reply related
* [PATCH v2 4/6] powerpc/mmu_gather: enable RCU_TABLE_FREE even for !SMP case
From: Santosh Sivaraj @ 2020-03-04 4:30 UTC (permalink / raw)
To: stable, linuxppc-dev; +Cc: Sasha Levin, Aneesh Kumar K.V, Greg KH
In-Reply-To: <20200304043028.280136-1-santosh@fossix.org>
From: "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com>
commit 12e4d53f3f04e81f9e83d6fc10edc7314ab9f6b9 upstream.
Patch series "Fixup page directory freeing", v4.
This is a repost of patch series from Peter with the arch specific changes
except ppc64 dropped. ppc64 changes are added here because we are redoing
the patch series on top of ppc64 changes. This makes it easy to backport
these changes. Only the first 2 patches need to be backported to stable.
The thing is, on anything SMP, freeing page directories should observe the
exact same order as normal page freeing:
1) unhook page/directory
2) TLB invalidate
3) free page/directory
Without this, any concurrent page-table walk could end up with a
Use-after-Free. This is esp. trivial for anything that has software
page-table walkers (HAVE_FAST_GUP / software TLB fill) or the hardware
caches partial page-walks (ie. caches page directories).
Even on UP this might give issues since mmu_gather is preemptible these
days. An interrupt or preempted task accessing user pages might stumble
into the free page if the hardware caches page directories.
This patch series fixes ppc64 and add generic MMU_GATHER changes to
support the conversion of other architectures. I haven't added patches
w.r.t other architecture because they are yet to be acked.
This patch (of 9):
A followup patch is going to make sure we correctly invalidate page walk
cache before we free page table pages. In order to keep things simple
enable RCU_TABLE_FREE even for !SMP so that we don't have to fixup the
!SMP case differently in the followup patch
!SMP case is right now broken for radix translation w.r.t page walk
cache flush. We can get interrupted in between page table free and
that would imply we have page walk cache entries pointing to tables
which got freed already. Michael said "both our platforms that run on
Power9 force SMP on in Kconfig, so the !SMP case is unlikely to be a
problem for anyone in practice, unless they've hacked their kernel to
build it !SMP."
Link: http://lkml.kernel.org/r/20200116064531.483522-2-aneesh.kumar@linux.ibm.com
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
Cc: <stable@vger.kernel.org> # 4.19
Signed-off-by: Santosh Sivaraj <santosh@fossix.org>
[santosh: backported for 4.19 stable]
---
arch/powerpc/Kconfig | 2 +-
arch/powerpc/include/asm/book3s/32/pgalloc.h | 8 --------
arch/powerpc/include/asm/book3s/64/pgalloc.h | 2 --
arch/powerpc/mm/pgtable-book3s64.c | 7 -------
4 files changed, 1 insertion(+), 18 deletions(-)
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index e09cfb109b8c..1a00ce4b0040 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -215,7 +215,7 @@ config PPC
select HAVE_HARDLOCKUP_DETECTOR_PERF if PERF_EVENTS && HAVE_PERF_EVENTS_NMI && !HAVE_HARDLOCKUP_DETECTOR_ARCH
select HAVE_PERF_REGS
select HAVE_PERF_USER_STACK_DUMP
- select HAVE_RCU_TABLE_FREE if SMP
+ select HAVE_RCU_TABLE_FREE
select HAVE_RCU_TABLE_NO_INVALIDATE if HAVE_RCU_TABLE_FREE
select HAVE_REGS_AND_STACK_ACCESS_API
select HAVE_RELIABLE_STACKTRACE if PPC64 && CPU_LITTLE_ENDIAN
diff --git a/arch/powerpc/include/asm/book3s/32/pgalloc.h b/arch/powerpc/include/asm/book3s/32/pgalloc.h
index 82e44b1a00ae..79ba3fbb512e 100644
--- a/arch/powerpc/include/asm/book3s/32/pgalloc.h
+++ b/arch/powerpc/include/asm/book3s/32/pgalloc.h
@@ -110,7 +110,6 @@ static inline void pgtable_free(void *table, unsigned index_size)
#define check_pgt_cache() do { } while (0)
#define get_hugepd_cache_index(x) (x)
-#ifdef CONFIG_SMP
static inline void pgtable_free_tlb(struct mmu_gather *tlb,
void *table, int shift)
{
@@ -127,13 +126,6 @@ static inline void __tlb_remove_table(void *_table)
pgtable_free(table, shift);
}
-#else
-static inline void pgtable_free_tlb(struct mmu_gather *tlb,
- void *table, int shift)
-{
- pgtable_free(table, shift);
-}
-#endif
static inline void __pte_free_tlb(struct mmu_gather *tlb, pgtable_t table,
unsigned long address)
diff --git a/arch/powerpc/include/asm/book3s/64/pgalloc.h b/arch/powerpc/include/asm/book3s/64/pgalloc.h
index f9019b579903..1013c0214213 100644
--- a/arch/powerpc/include/asm/book3s/64/pgalloc.h
+++ b/arch/powerpc/include/asm/book3s/64/pgalloc.h
@@ -47,9 +47,7 @@ extern pmd_t *pmd_fragment_alloc(struct mm_struct *, unsigned long);
extern void pte_fragment_free(unsigned long *, int);
extern void pmd_fragment_free(unsigned long *);
extern void pgtable_free_tlb(struct mmu_gather *tlb, void *table, int shift);
-#ifdef CONFIG_SMP
extern void __tlb_remove_table(void *_table);
-#endif
static inline pgd_t *radix__pgd_alloc(struct mm_struct *mm)
{
diff --git a/arch/powerpc/mm/pgtable-book3s64.c b/arch/powerpc/mm/pgtable-book3s64.c
index 297db665d953..5b4e9fd8990c 100644
--- a/arch/powerpc/mm/pgtable-book3s64.c
+++ b/arch/powerpc/mm/pgtable-book3s64.c
@@ -432,7 +432,6 @@ static inline void pgtable_free(void *table, int index)
}
}
-#ifdef CONFIG_SMP
void pgtable_free_tlb(struct mmu_gather *tlb, void *table, int index)
{
unsigned long pgf = (unsigned long)table;
@@ -449,12 +448,6 @@ void __tlb_remove_table(void *_table)
return pgtable_free(table, index);
}
-#else
-void pgtable_free_tlb(struct mmu_gather *tlb, void *table, int index)
-{
- return pgtable_free(table, index);
-}
-#endif
#ifdef CONFIG_PROC_FS
atomic_long_t direct_pages_count[MMU_PAGE_COUNT];
--
2.24.1
^ permalink raw reply related
* [PATCH v2 3/6] asm-generic/tlb, arch: Invert CONFIG_HAVE_RCU_TABLE_INVALIDATE
From: Santosh Sivaraj @ 2020-03-04 4:30 UTC (permalink / raw)
To: stable, linuxppc-dev; +Cc: Sasha Levin, Peter Zijlstra, Greg KH
In-Reply-To: <20200304043028.280136-1-santosh@fossix.org>
From: Peter Zijlstra <peterz@infradead.org>
commit 96bc9567cbe112e9320250f01b9c060c882e8619 upstream.
Make issuing a TLB invalidate for page-table pages the normal case.
The reason is twofold:
- too many invalidates is safer than too few,
- most architectures use the linux page-tables natively
and would thus require this.
Make it an opt-out, instead of an opt-in.
No change in behavior intended.
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: <stable@vger.kernel.org> # 4.19
Signed-off-by: Santosh Sivaraj <santosh@fossix.org>
[santosh: prerequisite for upcoming tlbflush backports]
---
arch/Kconfig | 2 +-
arch/powerpc/Kconfig | 1 +
arch/sparc/Kconfig | 1 +
arch/x86/Kconfig | 1 -
mm/memory.c | 2 +-
5 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/arch/Kconfig b/arch/Kconfig
index a336548487e6..061a12b8140e 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -363,7 +363,7 @@ config HAVE_ARCH_JUMP_LABEL
config HAVE_RCU_TABLE_FREE
bool
-config HAVE_RCU_TABLE_INVALIDATE
+config HAVE_RCU_TABLE_NO_INVALIDATE
bool
config ARCH_HAVE_NMI_SAFE_CMPXCHG
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 6f475dc5829b..e09cfb109b8c 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -216,6 +216,7 @@ config PPC
select HAVE_PERF_REGS
select HAVE_PERF_USER_STACK_DUMP
select HAVE_RCU_TABLE_FREE if SMP
+ select HAVE_RCU_TABLE_NO_INVALIDATE if HAVE_RCU_TABLE_FREE
select HAVE_REGS_AND_STACK_ACCESS_API
select HAVE_RELIABLE_STACKTRACE if PPC64 && CPU_LITTLE_ENDIAN
select HAVE_SYSCALL_TRACEPOINTS
diff --git a/arch/sparc/Kconfig b/arch/sparc/Kconfig
index e6f2a38d2e61..d90d632868aa 100644
--- a/arch/sparc/Kconfig
+++ b/arch/sparc/Kconfig
@@ -64,6 +64,7 @@ config SPARC64
select HAVE_KRETPROBES
select HAVE_KPROBES
select HAVE_RCU_TABLE_FREE if SMP
+ select HAVE_RCU_TABLE_NO_INVALIDATE if HAVE_RCU_TABLE_FREE
select HAVE_MEMBLOCK_NODE_MAP
select HAVE_ARCH_TRANSPARENT_HUGEPAGE
select HAVE_DYNAMIC_FTRACE
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index af35f5caadbe..181d0d522977 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -181,7 +181,6 @@ config X86
select HAVE_PERF_REGS
select HAVE_PERF_USER_STACK_DUMP
select HAVE_RCU_TABLE_FREE if PARAVIRT
- select HAVE_RCU_TABLE_INVALIDATE if HAVE_RCU_TABLE_FREE
select HAVE_REGS_AND_STACK_ACCESS_API
select HAVE_RELIABLE_STACKTRACE if X86_64 && (UNWINDER_FRAME_POINTER || UNWINDER_ORC) && STACK_VALIDATION
select HAVE_STACKPROTECTOR if CC_HAS_SANE_STACKPROTECTOR
diff --git a/mm/memory.c b/mm/memory.c
index 1832c5ed6ac0..ba5689610c04 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -327,7 +327,7 @@ bool __tlb_remove_page_size(struct mmu_gather *tlb, struct page *page, int page_
*/
static inline void tlb_table_invalidate(struct mmu_gather *tlb)
{
-#ifdef CONFIG_HAVE_RCU_TABLE_INVALIDATE
+#ifndef CONFIG_HAVE_RCU_TABLE_NO_INVALIDATE
/*
* Invalidate page-table caches used by hardware walkers. Then we still
* need to RCU-sched wait while freeing the pages because software
--
2.24.1
^ permalink raw reply related
* [PATCH v2 2/6] asm-generic/tlb: Track which levels of the page tables have been cleared
From: Santosh Sivaraj @ 2020-03-04 4:30 UTC (permalink / raw)
To: stable, linuxppc-dev; +Cc: Sasha Levin, Will Deacon, Greg KH
In-Reply-To: <20200304043028.280136-1-santosh@fossix.org>
From: Will Deacon <will.deacon@arm.com>
commit a6d60245d6d9b1caf66b0d94419988c4836980af upstream
It is common for architectures with hugepage support to require only a
single TLB invalidation operation per hugepage during unmap(), rather than
iterating through the mapping at a PAGE_SIZE increment. Currently,
however, the level in the page table where the unmap() operation occurs
is not stored in the mmu_gather structure, therefore forcing
architectures to issue additional TLB invalidation operations or to give
up and over-invalidate by e.g. invalidating the entire TLB.
Ideally, we could add an interval rbtree to the mmu_gather structure,
which would allow us to associate the correct mapping granule with the
various sub-mappings within the range being invalidated. However, this
is costly in terms of book-keeping and memory management, so instead we
approximate by keeping track of the page table levels that are cleared
and provide a means to query the smallest granule required for invalidation.
Signed-off-by: Will Deacon <will.deacon@arm.com>
Cc: <stable@vger.kernel.org> # 4.19
Signed-off-by: Santosh Sivaraj <santosh@fossix.org>
[santosh: prerequisite for upcoming tlbflush backports]
---
include/asm-generic/tlb.h | 58 +++++++++++++++++++++++++++++++++------
mm/memory.c | 4 ++-
2 files changed, 53 insertions(+), 9 deletions(-)
diff --git a/include/asm-generic/tlb.h b/include/asm-generic/tlb.h
index 97306b32d8d2..f2b9dc9cbaf8 100644
--- a/include/asm-generic/tlb.h
+++ b/include/asm-generic/tlb.h
@@ -114,6 +114,14 @@ struct mmu_gather {
*/
unsigned int freed_tables : 1;
+ /*
+ * at which levels have we cleared entries?
+ */
+ unsigned int cleared_ptes : 1;
+ unsigned int cleared_pmds : 1;
+ unsigned int cleared_puds : 1;
+ unsigned int cleared_p4ds : 1;
+
struct mmu_gather_batch *active;
struct mmu_gather_batch local;
struct page *__pages[MMU_GATHER_BUNDLE];
@@ -148,6 +156,10 @@ static inline void __tlb_reset_range(struct mmu_gather *tlb)
tlb->end = 0;
}
tlb->freed_tables = 0;
+ tlb->cleared_ptes = 0;
+ tlb->cleared_pmds = 0;
+ tlb->cleared_puds = 0;
+ tlb->cleared_p4ds = 0;
}
static inline void tlb_flush_mmu_tlbonly(struct mmu_gather *tlb)
@@ -197,6 +209,25 @@ static inline void tlb_remove_check_page_size_change(struct mmu_gather *tlb,
}
#endif
+static inline unsigned long tlb_get_unmap_shift(struct mmu_gather *tlb)
+{
+ if (tlb->cleared_ptes)
+ return PAGE_SHIFT;
+ if (tlb->cleared_pmds)
+ return PMD_SHIFT;
+ if (tlb->cleared_puds)
+ return PUD_SHIFT;
+ if (tlb->cleared_p4ds)
+ return P4D_SHIFT;
+
+ return PAGE_SHIFT;
+}
+
+static inline unsigned long tlb_get_unmap_size(struct mmu_gather *tlb)
+{
+ return 1UL << tlb_get_unmap_shift(tlb);
+}
+
/*
* In the case of tlb vma handling, we can optimise these away in the
* case where we're doing a full MM flush. When we're doing a munmap,
@@ -230,13 +261,19 @@ static inline void tlb_remove_check_page_size_change(struct mmu_gather *tlb,
#define tlb_remove_tlb_entry(tlb, ptep, address) \
do { \
__tlb_adjust_range(tlb, address, PAGE_SIZE); \
+ tlb->cleared_ptes = 1; \
__tlb_remove_tlb_entry(tlb, ptep, address); \
} while (0)
-#define tlb_remove_huge_tlb_entry(h, tlb, ptep, address) \
- do { \
- __tlb_adjust_range(tlb, address, huge_page_size(h)); \
- __tlb_remove_tlb_entry(tlb, ptep, address); \
+#define tlb_remove_huge_tlb_entry(h, tlb, ptep, address) \
+ do { \
+ unsigned long _sz = huge_page_size(h); \
+ __tlb_adjust_range(tlb, address, _sz); \
+ if (_sz == PMD_SIZE) \
+ tlb->cleared_pmds = 1; \
+ else if (_sz == PUD_SIZE) \
+ tlb->cleared_puds = 1; \
+ __tlb_remove_tlb_entry(tlb, ptep, address); \
} while (0)
/**
@@ -250,6 +287,7 @@ static inline void tlb_remove_check_page_size_change(struct mmu_gather *tlb,
#define tlb_remove_pmd_tlb_entry(tlb, pmdp, address) \
do { \
__tlb_adjust_range(tlb, address, HPAGE_PMD_SIZE); \
+ tlb->cleared_pmds = 1; \
__tlb_remove_pmd_tlb_entry(tlb, pmdp, address); \
} while (0)
@@ -264,6 +302,7 @@ static inline void tlb_remove_check_page_size_change(struct mmu_gather *tlb,
#define tlb_remove_pud_tlb_entry(tlb, pudp, address) \
do { \
__tlb_adjust_range(tlb, address, HPAGE_PUD_SIZE); \
+ tlb->cleared_puds = 1; \
__tlb_remove_pud_tlb_entry(tlb, pudp, address); \
} while (0)
@@ -289,7 +328,8 @@ static inline void tlb_remove_check_page_size_change(struct mmu_gather *tlb,
#define pte_free_tlb(tlb, ptep, address) \
do { \
__tlb_adjust_range(tlb, address, PAGE_SIZE); \
- tlb->freed_tables = 1; \
+ tlb->freed_tables = 1; \
+ tlb->cleared_pmds = 1; \
__pte_free_tlb(tlb, ptep, address); \
} while (0)
#endif
@@ -298,7 +338,8 @@ static inline void tlb_remove_check_page_size_change(struct mmu_gather *tlb,
#define pmd_free_tlb(tlb, pmdp, address) \
do { \
__tlb_adjust_range(tlb, address, PAGE_SIZE); \
- tlb->freed_tables = 1; \
+ tlb->freed_tables = 1; \
+ tlb->cleared_puds = 1; \
__pmd_free_tlb(tlb, pmdp, address); \
} while (0)
#endif
@@ -308,7 +349,8 @@ static inline void tlb_remove_check_page_size_change(struct mmu_gather *tlb,
#define pud_free_tlb(tlb, pudp, address) \
do { \
__tlb_adjust_range(tlb, address, PAGE_SIZE); \
- tlb->freed_tables = 1; \
+ tlb->freed_tables = 1; \
+ tlb->cleared_p4ds = 1; \
__pud_free_tlb(tlb, pudp, address); \
} while (0)
#endif
@@ -319,7 +361,7 @@ static inline void tlb_remove_check_page_size_change(struct mmu_gather *tlb,
#define p4d_free_tlb(tlb, pudp, address) \
do { \
__tlb_adjust_range(tlb, address, PAGE_SIZE); \
- tlb->freed_tables = 1; \
+ tlb->freed_tables = 1; \
__p4d_free_tlb(tlb, pudp, address); \
} while (0)
#endif
diff --git a/mm/memory.c b/mm/memory.c
index bbf0cc4066c8..1832c5ed6ac0 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -267,8 +267,10 @@ void arch_tlb_finish_mmu(struct mmu_gather *tlb,
{
struct mmu_gather_batch *batch, *next;
- if (force)
+ if (force) {
+ __tlb_reset_range(tlb);
__tlb_adjust_range(tlb, start, end - start);
+ }
tlb_flush_mmu(tlb);
--
2.24.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox