* [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
* [PATCH v2 1/6] asm-generic/tlb: Track freeing of page-table directories in struct mmu_gather
From: Santosh Sivaraj @ 2020-03-04 4:30 UTC (permalink / raw)
To: stable, linuxppc-dev; +Cc: Sasha Levin, Peter Zijlstra, Will Deacon, Greg KH
In-Reply-To: <20200304043028.280136-1-santosh@fossix.org>
From: Peter Zijlstra <peterz@infradead.org>
commit 22a61c3c4f1379ef8b0ce0d5cb78baf3178950e2 upstream
Some architectures require different TLB invalidation instructions
depending on whether it is only the last-level of page table being
changed, or whether there are also changes to the intermediate
(directory) entries higher up the tree.
Add a new bit to the flags bitfield in struct mmu_gather so that the
architecture code can operate accordingly if it's the intermediate
levels being invalidated.
Signed-off-by: Peter Zijlstra <peterz@infradead.org>
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 tlbflush backports]
---
include/asm-generic/tlb.h | 31 +++++++++++++++++++++++--------
1 file changed, 23 insertions(+), 8 deletions(-)
diff --git a/include/asm-generic/tlb.h b/include/asm-generic/tlb.h
index b3353e21f3b3..97306b32d8d2 100644
--- a/include/asm-generic/tlb.h
+++ b/include/asm-generic/tlb.h
@@ -97,12 +97,22 @@ struct mmu_gather {
#endif
unsigned long start;
unsigned long end;
- /* we are in the middle of an operation to clear
- * a full mm and can make some optimizations */
- unsigned int fullmm : 1,
- /* we have performed an operation which
- * requires a complete flush of the tlb */
- need_flush_all : 1;
+ /*
+ * we are in the middle of an operation to clear
+ * a full mm and can make some optimizations
+ */
+ unsigned int fullmm : 1;
+
+ /*
+ * we have performed an operation which
+ * requires a complete flush of the tlb
+ */
+ unsigned int need_flush_all : 1;
+
+ /*
+ * we have removed page directories
+ */
+ unsigned int freed_tables : 1;
struct mmu_gather_batch *active;
struct mmu_gather_batch local;
@@ -137,6 +147,7 @@ static inline void __tlb_reset_range(struct mmu_gather *tlb)
tlb->start = TASK_SIZE;
tlb->end = 0;
}
+ tlb->freed_tables = 0;
}
static inline void tlb_flush_mmu_tlbonly(struct mmu_gather *tlb)
@@ -278,6 +289,7 @@ 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; \
__pte_free_tlb(tlb, ptep, address); \
} while (0)
#endif
@@ -285,7 +297,8 @@ static inline void tlb_remove_check_page_size_change(struct mmu_gather *tlb,
#ifndef pmd_free_tlb
#define pmd_free_tlb(tlb, pmdp, address) \
do { \
- __tlb_adjust_range(tlb, address, PAGE_SIZE); \
+ __tlb_adjust_range(tlb, address, PAGE_SIZE); \
+ tlb->freed_tables = 1; \
__pmd_free_tlb(tlb, pmdp, address); \
} while (0)
#endif
@@ -295,6 +308,7 @@ 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; \
__pud_free_tlb(tlb, pudp, address); \
} while (0)
#endif
@@ -304,7 +318,8 @@ static inline void tlb_remove_check_page_size_change(struct mmu_gather *tlb,
#ifndef p4d_free_tlb
#define p4d_free_tlb(tlb, pudp, address) \
do { \
- __tlb_adjust_range(tlb, address, PAGE_SIZE); \
+ __tlb_adjust_range(tlb, address, PAGE_SIZE); \
+ tlb->freed_tables = 1; \
__p4d_free_tlb(tlb, pudp, address); \
} while (0)
#endif
--
2.24.1
^ permalink raw reply related
* [PATCH v2 0/6] Memory corruption may occur due to incorrent tlb flush
From: Santosh Sivaraj @ 2020-03-04 4:30 UTC (permalink / raw)
To: stable, linuxppc-dev; +Cc: Sasha Levin, Greg KH
The TLB flush optimisation (a46cc7a90f: powerpc/mm/radix: Improve TLB/PWC
flushes) may result in random memory corruption. Any concurrent page-table walk
could end up with a Use-after-Free. 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.
The series is a backport of the fix sent by Peter [1].
The first three patches are dependencies for the last patch (avoid potential
double flush). If the performance impact due to double flush is considered
trivial then the first three patches and last patch may be dropped.
This is only for v4.19 stable.
Changelog:
* Send the patches with the correct format (commit sha1 upstream) for stable
--
Aneesh Kumar K.V (1):
powerpc/mmu_gather: enable RCU_TABLE_FREE even for !SMP case
Peter Zijlstra (4):
asm-generic/tlb: Track freeing of page-table directories in struct
mmu_gather
asm-generic/tlb, arch: Invert CONFIG_HAVE_RCU_TABLE_INVALIDATE
mm/mmu_gather: invalidate TLB correctly on batch allocation failure
and flush
asm-generic/tlb: avoid potential double flush
Will Deacon (1):
asm-generic/tlb: Track which levels of the page tables have been
cleared
arch/Kconfig | 3 -
arch/powerpc/Kconfig | 2 +-
arch/powerpc/include/asm/book3s/32/pgalloc.h | 8 --
arch/powerpc/include/asm/book3s/64/pgalloc.h | 2 -
arch/powerpc/include/asm/tlb.h | 11 ++
arch/powerpc/mm/pgtable-book3s64.c | 7 --
arch/sparc/include/asm/tlb_64.h | 9 ++
arch/x86/Kconfig | 1 -
include/asm-generic/tlb.h | 103 ++++++++++++++++---
mm/memory.c | 20 ++--
10 files changed, 122 insertions(+), 44 deletions(-)
--
2.24.1
^ permalink raw reply
* Re: [PATCH v3 13/27] powerpc/powernv/pmem: Read the capability registers & wait for device ready
From: Alastair D'Silva @ 2020-03-04 4:15 UTC (permalink / raw)
To: Frederic Barrat
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: <3d2de7c1-ee95-ed6c-0346-4a1d20a0b75e@linux.ibm.com>
On Mon, 2020-03-02 at 18:51 +0100, Frederic Barrat wrote:
>
> Le 21/02/2020 à 04:27, Alastair D'Silva a écrit :
> > From: Alastair D'Silva <alastair@d-silva.org>
> >
> > This patch reads timeouts & firmware version from the controller,
> > and
> > uses those timeouts to wait for the controller to report that it is
> > ready
> > before handing the memory over to libnvdimm.
> >
> > Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
> > ---
> > arch/powerpc/platforms/powernv/pmem/Makefile | 2 +-
> > arch/powerpc/platforms/powernv/pmem/ocxl.c | 92
> > +++++++++++++++++++
> > .../platforms/powernv/pmem/ocxl_internal.c | 19 ++++
> > .../platforms/powernv/pmem/ocxl_internal.h | 24 +++++
> > 4 files changed, 136 insertions(+), 1 deletion(-)
> > create mode 100644
> > arch/powerpc/platforms/powernv/pmem/ocxl_internal.c
> >
> > diff --git a/arch/powerpc/platforms/powernv/pmem/Makefile
> > b/arch/powerpc/platforms/powernv/pmem/Makefile
> > index 1c55c4193175..4ceda25907d4 100644
> > --- a/arch/powerpc/platforms/powernv/pmem/Makefile
> > +++ b/arch/powerpc/platforms/powernv/pmem/Makefile
> > @@ -4,4 +4,4 @@ ccflags-$(CONFIG_PPC_WERROR) += -Werror
> >
> > obj-$(CONFIG_OCXL_PMEM) += ocxlpmem.o
> >
> > -ocxlpmem-y := ocxl.o
> > +ocxlpmem-y := ocxl.o ocxl_internal.o
> > diff --git a/arch/powerpc/platforms/powernv/pmem/ocxl.c
> > b/arch/powerpc/platforms/powernv/pmem/ocxl.c
> > index 3c4eeb5dcc0f..431212c9f0cc 100644
> > --- a/arch/powerpc/platforms/powernv/pmem/ocxl.c
> > +++ b/arch/powerpc/platforms/powernv/pmem/ocxl.c
> > @@ -8,6 +8,7 @@
> >
> > #include <linux/module.h>
> > #include <misc/ocxl.h>
> > +#include <linux/delay.h>
> > #include <linux/ndctl.h>
> > #include <linux/mm_types.h>
> > #include <linux/memory_hotplug.h>
> > @@ -215,6 +216,36 @@ static int register_lpc_mem(struct ocxlpmem
> > *ocxlpmem)
> > return 0;
> > }
> >
> > +/**
> > + * is_usable() - Is a controller usable?
> > + * @ocxlpmem: the device metadata
> > + * @verbose: True to log errors
> > + * Return: true if the controller is usable
> > + */
> > +static bool is_usable(const struct ocxlpmem *ocxlpmem, bool
> > verbose)
> > +{
> > + u64 chi = 0;
> > + int rc = ocxlpmem_chi(ocxlpmem, &chi);
> > +
> > + if (rc < 0)
> > + return false;
> > +
> > + if (!(chi & GLOBAL_MMIO_CHI_CRDY)) {
> > + if (verbose)
> > + dev_err(&ocxlpmem->dev, "controller is not
> > ready.\n");
> > + return false;
> > + }
> > +
> > + if (!(chi & GLOBAL_MMIO_CHI_MA)) {
> > + if (verbose)
> > + dev_err(&ocxlpmem->dev,
> > + "controller does not have memory
> > available.\n");
> > + return false;
> > + }
> > +
> > + return true;
> > +}
> > +
> > /**
> > * allocate_minor() - Allocate a minor number to use for an
> > OpenCAPI pmem device
> > * @ocxlpmem: the device metadata
> > @@ -328,6 +359,48 @@ static void ocxlpmem_remove(struct pci_dev
> > *pdev)
> > }
> > }
> >
> > +/**
> > + * read_device_metadata() - Retrieve config information from the
> > AFU and save it for future use
> > + * @ocxlpmem: the device metadata
> > + * Return: 0 on success, negative on failure
> > + */
> > +static int read_device_metadata(struct ocxlpmem *ocxlpmem)
> > +{
> > + u64 val;
> > + int rc;
> > +
> > + rc = ocxl_global_mmio_read64(ocxlpmem->ocxl_afu,
> > GLOBAL_MMIO_CCAP0,
> > + OCXL_LITTLE_ENDIAN, &val);
> > + if (rc)
> > + return rc;
> > +
> > + ocxlpmem->scm_revision = val & 0xFFFF;
> > + ocxlpmem->read_latency = (val >> 32) & 0xFF;
> > + ocxlpmem->readiness_timeout = (val >> 48) & 0x0F;
> > + ocxlpmem->memory_available_timeout = val >> 52;
> > +
> > + rc = ocxl_global_mmio_read64(ocxlpmem->ocxl_afu,
> > GLOBAL_MMIO_CCAP1,
> > + OCXL_LITTLE_ENDIAN, &val);
> > + if (rc)
> > + return rc;
> > +
> > + ocxlpmem->max_controller_dump_size = val & 0xFFFFFFFF;
> > +
> > + // Extract firmware version text
> > + rc = ocxl_global_mmio_read64(ocxlpmem->ocxl_afu,
> > GLOBAL_MMIO_FWVER,
> > + OCXL_HOST_ENDIAN, (u64 *)ocxlpmem-
> > >fw_version);
> > + if (rc)
> > + return rc;
> > +
> > + ocxlpmem->fw_version[8] = '\0';
> > +
> > + dev_info(&ocxlpmem->dev,
> > + "Firmware version '%s' SCM revision %d:%d\n",
> > ocxlpmem->fw_version,
> > + ocxlpmem->scm_revision >> 4, ocxlpmem->scm_revision &
> > 0x0F);
> > +
> > + return 0;
> > +}
> > +
> > /**
> > * 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,
> > @@ -368,6 +441,7 @@ static int probe(struct pci_dev *pdev, const
> > struct pci_device_id *ent)
> > {
> > struct ocxlpmem *ocxlpmem;
> > int rc;
> > + u16 elapsed, timeout;
> >
> > if (PCI_FUNC(pdev->devfn) == 0)
> > return probe_function0(pdev);
> > @@ -422,6 +496,24 @@ static int probe(struct pci_dev *pdev, const
> > struct pci_device_id *ent)
> > goto err;
> > }
> >
> > + if (read_device_metadata(ocxlpmem)) {
> > + dev_err(&pdev->dev, "Could not read metadata\n");
>
>
> Need to set rc
>
>
Whoops :)
>
> > + goto err;
> > + }
> > +
> > + elapsed = 0;
> > + timeout = ocxlpmem->readiness_timeout + ocxlpmem-
> > >memory_available_timeout;
> > + while (!is_usable(ocxlpmem, false)) {
> > + if (elapsed++ > timeout) {
> > + dev_warn(&ocxlpmem->dev, "OpenCAPI Persistent
> > Memory ready timeout.\n");
> > + (void)is_usable(ocxlpmem, true);
>
> I guess that extra call to is_usable() is just to log the cause of
> the
> error. However, with some bad luck, the call could now succeed.
>
Yeah, that's pretty ugly, I'll re-engineer it.
>
> Fred
>
>
> > + rc = -ENXIO;
> > + goto err;
> > + }
> > +
> > + msleep(1000);
> > + }
> > +
> > rc = register_lpc_mem(ocxlpmem);
> > if (rc) {
> > dev_err(&pdev->dev, "Could not register OpenCAPI
> > persistent memory with libnvdimm\n");
> > diff --git a/arch/powerpc/platforms/powernv/pmem/ocxl_internal.c
> > b/arch/powerpc/platforms/powernv/pmem/ocxl_internal.c
> > new file mode 100644
> > index 000000000000..617ca943b1b8
> > --- /dev/null
> > +++ b/arch/powerpc/platforms/powernv/pmem/ocxl_internal.c
> > @@ -0,0 +1,19 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +// Copyright 2019 IBM Corp.
> > +
> > +#include <misc/ocxl.h>
> > +#include <linux/delay.h>
> > +#include "ocxl_internal.h"
> > +
> > +int ocxlpmem_chi(const struct ocxlpmem *ocxlpmem, u64 *chi)
> > +{
> > + u64 val;
> > + int rc = ocxl_global_mmio_read64(ocxlpmem->ocxl_afu,
> > GLOBAL_MMIO_CHI,
> > + OCXL_LITTLE_ENDIAN, &val);
> > + if (rc)
> > + return rc;
> > +
> > + *chi = val;
> > +
> > + return 0;
> > +}
> > diff --git a/arch/powerpc/platforms/powernv/pmem/ocxl_internal.h
> > b/arch/powerpc/platforms/powernv/pmem/ocxl_internal.h
> > index 9cf3e42750e7..ba0301533d00 100644
> > --- a/arch/powerpc/platforms/powernv/pmem/ocxl_internal.h
> > +++ b/arch/powerpc/platforms/powernv/pmem/ocxl_internal.h
> > @@ -97,4 +97,28 @@ struct ocxlpmem {
> > void *metadata_addr;
> > struct resource pmem_res;
> > struct nd_region *nd_region;
> > + char fw_version[8+1];
> > +
> > + u32 max_controller_dump_size;
> > + u16 scm_revision; // major/minor
> > + u8 readiness_timeout; /* The worst case time (in seconds) that
> > the host shall
> > + * wait for the controller to become
> > operational following a reset (CHI.CRDY).
> > + */
> > + u8 memory_available_timeout; /* The worst case time (in
> > seconds) that the host shall
> > + * wait for memory to become
> > available following a reset (CHI.MA).
> > + */
> > +
> > + u16 read_latency; /* The nominal measure of latency (in
> > nanoseconds)
> > + * associated with an unassisted read of a
> > memory block.
> > + * This represents the capability of the raw
> > media technology without assistance
> > + */
> > };
> > +
> > +/**
> > + * ocxlpmem_chi() - Get the value of the CHI register
> > + * @ocxlpmem: the device metadata
> > + * @chi: returns the CHI value
> > + *
> > + * Returns 0 on success, negative on error
> > + */
> > +int ocxlpmem_chi(const struct ocxlpmem *ocxlpmem, u64 *chi);
> >
--
Alastair D'Silva
Open Source Developer
Linux Technology Centre, IBM Australia
mob: 0423 762 819
^ permalink raw reply
* Re: [PATCH v3 26/27] powerpc/powernv/pmem: Expose the firmware version in sysfs
From: Alastair D'Silva @ 2020-03-04 4:11 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: <2253010d-c7ad-347b-4668-d5f1a95d3f1e@linux.ibm.com>
On Mon, 2020-03-02 at 18:35 +1100, Andrew Donnellan wrote:
> On 21/2/20 2:27 pm, Alastair D'Silva wrote:
> > From: Alastair D'Silva <alastair@d-silva.org>
> >
> > This information will be used by ndctl in userspace to help users
> > identify
> > the device.
>
> You should include the information from the subject line in the body
> of
> the commit message too.
>
> I think this patch could probably be squashed in with the last one.
>
Ok
--
Alastair D'Silva
Open Source Developer
Linux Technology Centre, IBM Australia
mob: 0423 762 819
^ permalink raw reply
* [PATCH v2] ima: add a new CONFIG for loading arch-specific policies
From: Nayna Jain @ 2020-03-04 2:33 UTC (permalink / raw)
To: linux-integrity, linuxppc-dev, linux-efi, linux-s390
Cc: Nayna Jain, linux-kernel, zohar, Philipp Rudo, Ard Biesheuvel
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>
---
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
+ 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 related
* [PATCH v4 5/5] libnvdimm/region: Introduce an 'align' attribute
From: Dan Williams @ 2020-03-04 2:08 UTC (permalink / raw)
To: linux-nvdimm
Cc: Aneesh Kumar K.V, Jeff Moyer, linuxppc-dev, linux-kernel,
vishal.l.verma
In-Reply-To: <158328768294.2223916.16551505954326988623.stgit@dwillia2-desk3.amr.corp.intel.com>
The align attribute applies an alignment constraint for namespace
creation in a region. Whereas the 'align' attribute of a namespace
applied alignment padding via an info block, the 'align' attribute
applies alignment constraints to the free space allocation.
The default for 'align' is the maximum known memremap_compat_align()
across all archs (16MiB from PowerPC at time of writing) multiplied by
the number of interleave ways if there is blk-aliasing. The minimum is
PAGE_SIZE and allows for the creation of cross-arch incompatible
namespaces, just as previous kernels allowed, but the expectation is
cross-arch and mode-independent compatibility by default.
The regression risk with this change is limited to cases that were
dependent on the ability to create unaligned namespaces, *and* for some
reason are unable to opt-out of aligned namespaces by writing to
'regionX/align'. If such a scenario arises the default can be flipped
from opt-out to opt-in of compat-aligned namespace creation, but that is
a last resort. The kernel will otherwise continue to support existing
defined misaligned namespaces.
Unfortunately this change needs to touch several parts of the
implementation at once:
- region/available_size: expand busy extents to current align
- region/max_available_extent: expand busy extents to current align
- namespace/size: trim free space to current align
...to keep the free space accounting conforming to the dynamic align
setting.
Reported-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
Reported-by: Jeff Moyer <jmoyer@redhat.com>
Reviewed-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
Reviewed-by: Jeff Moyer <jmoyer@redhat.com>
Link: https://lore.kernel.org/r/158041478371.3889308.14542630147672668068.stgit@dwillia2-desk3.amr.corp.intel.com
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
drivers/nvdimm/dimm_devs.c | 86 +++++++++++++++++++++++----
drivers/nvdimm/namespace_devs.c | 9 ++-
drivers/nvdimm/nd.h | 1
drivers/nvdimm/region_devs.c | 122 ++++++++++++++++++++++++++++++++++++---
4 files changed, 192 insertions(+), 26 deletions(-)
diff --git a/drivers/nvdimm/dimm_devs.c b/drivers/nvdimm/dimm_devs.c
index 39a61a514746..b7b77e8d9027 100644
--- a/drivers/nvdimm/dimm_devs.c
+++ b/drivers/nvdimm/dimm_devs.c
@@ -563,6 +563,21 @@ int nvdimm_security_freeze(struct nvdimm *nvdimm)
return rc;
}
+static unsigned long dpa_align(struct nd_region *nd_region)
+{
+ struct device *dev = &nd_region->dev;
+
+ if (dev_WARN_ONCE(dev, !is_nvdimm_bus_locked(dev),
+ "bus lock required for capacity provision\n"))
+ return 0;
+ if (dev_WARN_ONCE(dev, !nd_region->ndr_mappings || nd_region->align
+ % nd_region->ndr_mappings,
+ "invalid region align %#lx mappings: %d\n",
+ nd_region->align, nd_region->ndr_mappings))
+ return 0;
+ return nd_region->align / nd_region->ndr_mappings;
+}
+
int alias_dpa_busy(struct device *dev, void *data)
{
resource_size_t map_end, blk_start, new;
@@ -571,6 +586,7 @@ int alias_dpa_busy(struct device *dev, void *data)
struct nd_region *nd_region;
struct nvdimm_drvdata *ndd;
struct resource *res;
+ unsigned long align;
int i;
if (!is_memory(dev))
@@ -608,13 +624,21 @@ int alias_dpa_busy(struct device *dev, void *data)
* Find the free dpa from the end of the last pmem allocation to
* the end of the interleave-set mapping.
*/
+ align = dpa_align(nd_region);
+ if (!align)
+ return 0;
+
for_each_dpa_resource(ndd, res) {
+ resource_size_t start, end;
+
if (strncmp(res->name, "pmem", 4) != 0)
continue;
- if ((res->start >= blk_start && res->start < map_end)
- || (res->end >= blk_start
- && res->end <= map_end)) {
- new = max(blk_start, min(map_end + 1, res->end + 1));
+
+ start = ALIGN_DOWN(res->start, align);
+ end = ALIGN(res->end + 1, align) - 1;
+ if ((start >= blk_start && start < map_end)
+ || (end >= blk_start && end <= map_end)) {
+ new = max(blk_start, min(map_end, end) + 1);
if (new != blk_start) {
blk_start = new;
goto retry;
@@ -654,6 +678,7 @@ resource_size_t nd_blk_available_dpa(struct nd_region *nd_region)
.res = NULL,
};
struct resource *res;
+ unsigned long align;
if (!ndd)
return 0;
@@ -661,10 +686,20 @@ resource_size_t nd_blk_available_dpa(struct nd_region *nd_region)
device_for_each_child(&nvdimm_bus->dev, &info, alias_dpa_busy);
/* now account for busy blk allocations in unaliased dpa */
+ align = dpa_align(nd_region);
+ if (!align)
+ return 0;
for_each_dpa_resource(ndd, res) {
+ resource_size_t start, end, size;
+
if (strncmp(res->name, "blk", 3) != 0)
continue;
- info.available -= resource_size(res);
+ start = ALIGN_DOWN(res->start, align);
+ end = ALIGN(res->end + 1, align) - 1;
+ size = end - start + 1;
+ if (size >= info.available)
+ return 0;
+ info.available -= size;
}
return info.available;
@@ -683,19 +718,31 @@ resource_size_t nd_pmem_max_contiguous_dpa(struct nd_region *nd_region,
struct nvdimm_bus *nvdimm_bus;
resource_size_t max = 0;
struct resource *res;
+ unsigned long align;
/* if a dimm is disabled the available capacity is zero */
if (!ndd)
return 0;
+ align = dpa_align(nd_region);
+ if (!align)
+ return 0;
+
nvdimm_bus = walk_to_nvdimm_bus(ndd->dev);
if (__reserve_free_pmem(&nd_region->dev, nd_mapping->nvdimm))
return 0;
for_each_dpa_resource(ndd, res) {
+ resource_size_t start, end;
+
if (strcmp(res->name, "pmem-reserve") != 0)
continue;
- if (resource_size(res) > max)
- max = resource_size(res);
+ /* trim free space relative to current alignment setting */
+ start = ALIGN(res->start, align);
+ end = ALIGN_DOWN(res->end + 1, align) - 1;
+ if (end < start)
+ continue;
+ if (end - start + 1 > max)
+ max = end - start + 1;
}
release_free_pmem(nvdimm_bus, nd_mapping);
return max;
@@ -723,24 +770,33 @@ resource_size_t nd_pmem_available_dpa(struct nd_region *nd_region,
struct nvdimm_drvdata *ndd = to_ndd(nd_mapping);
struct resource *res;
const char *reason;
+ unsigned long align;
if (!ndd)
return 0;
+ align = dpa_align(nd_region);
+ if (!align)
+ return 0;
+
map_start = nd_mapping->start;
map_end = map_start + nd_mapping->size - 1;
blk_start = max(map_start, map_end + 1 - *overlap);
for_each_dpa_resource(ndd, res) {
- if (res->start >= map_start && res->start < map_end) {
+ resource_size_t start, end;
+
+ start = ALIGN_DOWN(res->start, align);
+ end = ALIGN(res->end + 1, align) - 1;
+ if (start >= map_start && start < map_end) {
if (strncmp(res->name, "blk", 3) == 0)
blk_start = min(blk_start,
- max(map_start, res->start));
- else if (res->end > map_end) {
+ max(map_start, start));
+ else if (end > map_end) {
reason = "misaligned to iset";
goto err;
} else
- busy += resource_size(res);
- } else if (res->end >= map_start && res->end <= map_end) {
+ busy += end - start + 1;
+ } else if (end >= map_start && end <= map_end) {
if (strncmp(res->name, "blk", 3) == 0) {
/*
* If a BLK allocation overlaps the start of
@@ -749,8 +805,8 @@ resource_size_t nd_pmem_available_dpa(struct nd_region *nd_region,
*/
blk_start = map_start;
} else
- busy += resource_size(res);
- } else if (map_start > res->start && map_start < res->end) {
+ busy += end - start + 1;
+ } else if (map_start > start && map_start < end) {
/* total eclipse of the mapping */
busy += nd_mapping->size;
blk_start = map_start;
@@ -760,7 +816,7 @@ resource_size_t nd_pmem_available_dpa(struct nd_region *nd_region,
*overlap = map_end + 1 - blk_start;
available = blk_start - map_start;
if (busy < available)
- return available - busy;
+ return ALIGN_DOWN(available - busy, align);
return 0;
err:
diff --git a/drivers/nvdimm/namespace_devs.c b/drivers/nvdimm/namespace_devs.c
index 01f6c22f0d1a..ae155e860fdc 100644
--- a/drivers/nvdimm/namespace_devs.c
+++ b/drivers/nvdimm/namespace_devs.c
@@ -542,6 +542,11 @@ static void space_valid(struct nd_region *nd_region, struct nvdimm_drvdata *ndd,
{
bool is_reserve = strcmp(label_id->id, "pmem-reserve") == 0;
bool is_pmem = strncmp(label_id->id, "pmem", 4) == 0;
+ unsigned long align;
+
+ align = nd_region->align / nd_region->ndr_mappings;
+ valid->start = ALIGN(valid->start, align);
+ valid->end = ALIGN_DOWN(valid->end + 1, align) - 1;
if (valid->start >= valid->end)
goto invalid;
@@ -981,10 +986,10 @@ static ssize_t __size_store(struct device *dev, unsigned long long val)
return -ENXIO;
}
- div_u64_rem(val, PAGE_SIZE * nd_region->ndr_mappings, &remainder);
+ div_u64_rem(val, nd_region->align, &remainder);
if (remainder) {
dev_dbg(dev, "%llu is not %ldK aligned\n", val,
- (PAGE_SIZE * nd_region->ndr_mappings) / SZ_1K);
+ nd_region->align / SZ_1K);
return -EINVAL;
}
diff --git a/drivers/nvdimm/nd.h b/drivers/nvdimm/nd.h
index ca39abe29c7c..c4d69c1cce55 100644
--- a/drivers/nvdimm/nd.h
+++ b/drivers/nvdimm/nd.h
@@ -146,6 +146,7 @@ struct nd_region {
struct device *btt_seed;
struct device *pfn_seed;
struct device *dax_seed;
+ unsigned long align;
u16 ndr_mappings;
u64 ndr_size;
u64 ndr_start;
diff --git a/drivers/nvdimm/region_devs.c b/drivers/nvdimm/region_devs.c
index a5fc6e4c56ff..bf239e783940 100644
--- a/drivers/nvdimm/region_devs.c
+++ b/drivers/nvdimm/region_devs.c
@@ -216,21 +216,25 @@ int nd_region_to_nstype(struct nd_region *nd_region)
}
EXPORT_SYMBOL(nd_region_to_nstype);
-static ssize_t size_show(struct device *dev,
- struct device_attribute *attr, char *buf)
+static unsigned long long region_size(struct nd_region *nd_region)
{
- struct nd_region *nd_region = to_nd_region(dev);
- unsigned long long size = 0;
-
- if (is_memory(dev)) {
- size = nd_region->ndr_size;
+ if (is_memory(&nd_region->dev)) {
+ return nd_region->ndr_size;
} else if (nd_region->ndr_mappings == 1) {
struct nd_mapping *nd_mapping = &nd_region->mapping[0];
- size = nd_mapping->size;
+ return nd_mapping->size;
}
- return sprintf(buf, "%llu\n", size);
+ return 0;
+}
+
+static ssize_t size_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct nd_region *nd_region = to_nd_region(dev);
+
+ return sprintf(buf, "%llu\n", region_size(nd_region));
}
static DEVICE_ATTR_RO(size);
@@ -529,6 +533,55 @@ static ssize_t read_only_store(struct device *dev,
}
static DEVICE_ATTR_RW(read_only);
+static ssize_t align_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct nd_region *nd_region = to_nd_region(dev);
+
+ return sprintf(buf, "%#lx\n", nd_region->align);
+}
+
+static ssize_t align_store(struct device *dev,
+ struct device_attribute *attr, const char *buf, size_t len)
+{
+ struct nd_region *nd_region = to_nd_region(dev);
+ unsigned long val, dpa;
+ u32 remainder;
+ int rc;
+
+ rc = kstrtoul(buf, 0, &val);
+ if (rc)
+ return rc;
+
+ if (!nd_region->ndr_mappings)
+ return -ENXIO;
+
+ /*
+ * Ensure space-align is evenly divisible by the region
+ * interleave-width because the kernel typically has no facility
+ * to determine which DIMM(s), dimm-physical-addresses, would
+ * contribute to the tail capacity in system-physical-address
+ * space for the namespace.
+ */
+ dpa = val;
+ remainder = do_div(dpa, nd_region->ndr_mappings);
+ if (!is_power_of_2(dpa) || dpa < PAGE_SIZE
+ || val > region_size(nd_region) || remainder)
+ return -EINVAL;
+
+ /*
+ * Given that space allocation consults this value multiple
+ * times ensure it does not change for the duration of the
+ * allocation.
+ */
+ nvdimm_bus_lock(dev);
+ nd_region->align = val;
+ nvdimm_bus_unlock(dev);
+
+ return len;
+}
+static DEVICE_ATTR_RW(align);
+
static ssize_t region_badblocks_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
@@ -571,6 +624,7 @@ static DEVICE_ATTR_RO(persistence_domain);
static struct attribute *nd_region_attributes[] = {
&dev_attr_size.attr,
+ &dev_attr_align.attr,
&dev_attr_nstype.attr,
&dev_attr_mappings.attr,
&dev_attr_btt_seed.attr,
@@ -626,6 +680,19 @@ static umode_t region_visible(struct kobject *kobj, struct attribute *a, int n)
return a->mode;
}
+ if (a == &dev_attr_align.attr) {
+ int i;
+
+ for (i = 0; i < nd_region->ndr_mappings; i++) {
+ struct nd_mapping *nd_mapping = &nd_region->mapping[i];
+ struct nvdimm *nvdimm = nd_mapping->nvdimm;
+
+ if (test_bit(NDD_LABELING, &nvdimm->flags))
+ return a->mode;
+ }
+ return 0;
+ }
+
if (a != &dev_attr_set_cookie.attr
&& a != &dev_attr_available_size.attr)
return a->mode;
@@ -935,6 +1002,42 @@ void nd_region_release_lane(struct nd_region *nd_region, unsigned int lane)
}
EXPORT_SYMBOL(nd_region_release_lane);
+/*
+ * PowerPC requires this alignment for memremap_pages(). All other archs
+ * should be ok with SUBSECTION_SIZE (see memremap_compat_align()).
+ */
+#define MEMREMAP_COMPAT_ALIGN_MAX SZ_16M
+
+static unsigned long default_align(struct nd_region *nd_region)
+{
+ unsigned long align, per_mapping;
+ int i, mappings;
+ u32 remainder;
+
+ if (is_nd_blk(&nd_region->dev))
+ align = PAGE_SIZE;
+ else
+ align = MEMREMAP_COMPAT_ALIGN_MAX;
+
+ for (i = 0; i < nd_region->ndr_mappings; i++) {
+ struct nd_mapping *nd_mapping = &nd_region->mapping[i];
+ struct nvdimm *nvdimm = nd_mapping->nvdimm;
+
+ if (test_bit(NDD_ALIASING, &nvdimm->flags)) {
+ align = MEMREMAP_COMPAT_ALIGN_MAX;
+ break;
+ }
+ }
+
+ mappings = max_t(u16, 1, nd_region->ndr_mappings);
+ per_mapping = align;
+ remainder = do_div(per_mapping, mappings);
+ if (remainder)
+ align *= mappings;
+
+ return align;
+}
+
static struct nd_region *nd_region_create(struct nvdimm_bus *nvdimm_bus,
struct nd_region_desc *ndr_desc,
const struct device_type *dev_type, const char *caller)
@@ -1039,6 +1142,7 @@ static struct nd_region *nd_region_create(struct nvdimm_bus *nvdimm_bus,
dev->of_node = ndr_desc->of_node;
nd_region->ndr_size = resource_size(ndr_desc->res);
nd_region->ndr_start = ndr_desc->res->start;
+ nd_region->align = default_align(nd_region);
if (ndr_desc->flush)
nd_region->flush = ndr_desc->flush;
else
^ permalink raw reply related
* [PATCH v4 4/5] libnvdimm/region: Introduce NDD_LABELING
From: Dan Williams @ 2020-03-04 2:08 UTC (permalink / raw)
To: linux-nvdimm
Cc: Vishal Verma, linux-kernel, Jeff Moyer, Oliver O'Halloran,
Aneesh Kumar K.V, linuxppc-dev
In-Reply-To: <158328768294.2223916.16551505954326988623.stgit@dwillia2-desk3.amr.corp.intel.com>
The NDD_ALIASING flag is used to indicate where pmem capacity might
alias with blk capacity and require labeling. It is also used to
indicate whether the DIMM supports labeling. Separate this latter
capability into its own flag so that the NDD_ALIASING flag is scoped to
true aliased configurations.
To my knowledge aliased configurations only exist in the ACPI spec,
there are no known platforms that ship this support in production.
This clarity allows namespace-capacity alignment constraints around
interleave-ways to be relaxed.
Cc: Vishal Verma <vishal.l.verma@intel.com>
Cc: Oliver O'Halloran <oohall@gmail.com>
Reviewed-by: Jeff Moyer <jmoyer@redhat.com>
Reviewed-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
Link: https://lore.kernel.org/r/158041477856.3889308.4212605617834097674.stgit@dwillia2-desk3.amr.corp.intel.com
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
arch/powerpc/platforms/pseries/papr_scm.c | 2 +-
drivers/acpi/nfit/core.c | 4 +++-
drivers/nvdimm/dimm.c | 2 +-
drivers/nvdimm/dimm_devs.c | 9 +++++----
drivers/nvdimm/namespace_devs.c | 2 +-
drivers/nvdimm/nd.h | 2 +-
drivers/nvdimm/region_devs.c | 10 +++++-----
include/linux/libnvdimm.h | 2 ++
8 files changed, 19 insertions(+), 14 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/papr_scm.c b/arch/powerpc/platforms/pseries/papr_scm.c
index 0b4467e378e5..589858cb3203 100644
--- a/arch/powerpc/platforms/pseries/papr_scm.c
+++ b/arch/powerpc/platforms/pseries/papr_scm.c
@@ -328,7 +328,7 @@ static int papr_scm_nvdimm_init(struct papr_scm_priv *p)
}
dimm_flags = 0;
- set_bit(NDD_ALIASING, &dimm_flags);
+ set_bit(NDD_LABELING, &dimm_flags);
p->nvdimm = nvdimm_create(p->bus, p, NULL, dimm_flags,
PAPR_SCM_DIMM_CMD_MASK, 0, NULL);
diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
index a3320f93616d..71d7f2aa1b12 100644
--- a/drivers/acpi/nfit/core.c
+++ b/drivers/acpi/nfit/core.c
@@ -2026,8 +2026,10 @@ static int acpi_nfit_register_dimms(struct acpi_nfit_desc *acpi_desc)
continue;
}
- if (nfit_mem->bdw && nfit_mem->memdev_pmem)
+ if (nfit_mem->bdw && nfit_mem->memdev_pmem) {
set_bit(NDD_ALIASING, &flags);
+ set_bit(NDD_LABELING, &flags);
+ }
/* collate flags across all memdevs for this dimm */
list_for_each_entry(nfit_memdev, &acpi_desc->memdevs, list) {
diff --git a/drivers/nvdimm/dimm.c b/drivers/nvdimm/dimm.c
index 64776ed15bb3..7d4ddc4d9322 100644
--- a/drivers/nvdimm/dimm.c
+++ b/drivers/nvdimm/dimm.c
@@ -99,7 +99,7 @@ static int nvdimm_probe(struct device *dev)
if (ndd->ns_current >= 0) {
rc = nd_label_reserve_dpa(ndd);
if (rc == 0)
- nvdimm_set_aliasing(dev);
+ nvdimm_set_labeling(dev);
}
nvdimm_bus_unlock(dev);
diff --git a/drivers/nvdimm/dimm_devs.c b/drivers/nvdimm/dimm_devs.c
index 94ea6dba6b4f..39a61a514746 100644
--- a/drivers/nvdimm/dimm_devs.c
+++ b/drivers/nvdimm/dimm_devs.c
@@ -32,7 +32,7 @@ int nvdimm_check_config_data(struct device *dev)
if (!nvdimm->cmd_mask ||
!test_bit(ND_CMD_GET_CONFIG_DATA, &nvdimm->cmd_mask)) {
- if (test_bit(NDD_ALIASING, &nvdimm->flags))
+ if (test_bit(NDD_LABELING, &nvdimm->flags))
return -ENXIO;
else
return -ENOTTY;
@@ -173,11 +173,11 @@ int nvdimm_set_config_data(struct nvdimm_drvdata *ndd, size_t offset,
return rc;
}
-void nvdimm_set_aliasing(struct device *dev)
+void nvdimm_set_labeling(struct device *dev)
{
struct nvdimm *nvdimm = to_nvdimm(dev);
- set_bit(NDD_ALIASING, &nvdimm->flags);
+ set_bit(NDD_LABELING, &nvdimm->flags);
}
void nvdimm_set_locked(struct device *dev)
@@ -312,8 +312,9 @@ static ssize_t flags_show(struct device *dev,
{
struct nvdimm *nvdimm = to_nvdimm(dev);
- return sprintf(buf, "%s%s\n",
+ return sprintf(buf, "%s%s%s\n",
test_bit(NDD_ALIASING, &nvdimm->flags) ? "alias " : "",
+ test_bit(NDD_LABELING, &nvdimm->flags) ? "label " : "",
test_bit(NDD_LOCKED, &nvdimm->flags) ? "lock " : "");
}
static DEVICE_ATTR_RO(flags);
diff --git a/drivers/nvdimm/namespace_devs.c b/drivers/nvdimm/namespace_devs.c
index 77e211c7d94d..01f6c22f0d1a 100644
--- a/drivers/nvdimm/namespace_devs.c
+++ b/drivers/nvdimm/namespace_devs.c
@@ -2538,7 +2538,7 @@ static int init_active_labels(struct nd_region *nd_region)
if (!ndd) {
if (test_bit(NDD_LOCKED, &nvdimm->flags))
/* fail, label data may be unreadable */;
- else if (test_bit(NDD_ALIASING, &nvdimm->flags))
+ else if (test_bit(NDD_LABELING, &nvdimm->flags))
/* fail, labels needed to disambiguate dpa */;
else
return 0;
diff --git a/drivers/nvdimm/nd.h b/drivers/nvdimm/nd.h
index c9f6a5b5253a..ca39abe29c7c 100644
--- a/drivers/nvdimm/nd.h
+++ b/drivers/nvdimm/nd.h
@@ -252,7 +252,7 @@ int nvdimm_set_config_data(struct nvdimm_drvdata *ndd, size_t offset,
void *buf, size_t len);
long nvdimm_clear_poison(struct device *dev, phys_addr_t phys,
unsigned int len);
-void nvdimm_set_aliasing(struct device *dev);
+void nvdimm_set_labeling(struct device *dev);
void nvdimm_set_locked(struct device *dev);
void nvdimm_clear_locked(struct device *dev);
int nvdimm_security_setup_events(struct device *dev);
diff --git a/drivers/nvdimm/region_devs.c b/drivers/nvdimm/region_devs.c
index a19e535830d9..a5fc6e4c56ff 100644
--- a/drivers/nvdimm/region_devs.c
+++ b/drivers/nvdimm/region_devs.c
@@ -195,16 +195,16 @@ EXPORT_SYMBOL_GPL(nd_blk_region_set_provider_data);
int nd_region_to_nstype(struct nd_region *nd_region)
{
if (is_memory(&nd_region->dev)) {
- u16 i, alias;
+ u16 i, label;
- for (i = 0, alias = 0; i < nd_region->ndr_mappings; i++) {
+ for (i = 0, label = 0; i < nd_region->ndr_mappings; i++) {
struct nd_mapping *nd_mapping = &nd_region->mapping[i];
struct nvdimm *nvdimm = nd_mapping->nvdimm;
- if (test_bit(NDD_ALIASING, &nvdimm->flags))
- alias++;
+ if (test_bit(NDD_LABELING, &nvdimm->flags))
+ label++;
}
- if (alias)
+ if (label)
return ND_DEVICE_NAMESPACE_PMEM;
else
return ND_DEVICE_NAMESPACE_IO;
diff --git a/include/linux/libnvdimm.h b/include/linux/libnvdimm.h
index 9df091bd30ba..18da4059be09 100644
--- a/include/linux/libnvdimm.h
+++ b/include/linux/libnvdimm.h
@@ -37,6 +37,8 @@ enum {
NDD_WORK_PENDING = 4,
/* ignore / filter NSLABEL_FLAG_LOCAL for this DIMM, i.e. no aliasing */
NDD_NOBLK = 5,
+ /* dimm supports namespace labels */
+ NDD_LABELING = 6,
/* need to set a limit somewhere, but yes, this is likely overkill */
ND_IOCTL_MAX_BUFLEN = SZ_4M,
^ permalink raw reply related
* [PATCH v4 3/5] libnvdimm/namespace: Enforce memremap_compat_align()
From: Dan Williams @ 2020-03-04 2:08 UTC (permalink / raw)
To: linux-nvdimm
Cc: Aneesh Kumar K.V, Jeff Moyer, linuxppc-dev, linux-kernel,
vishal.l.verma
In-Reply-To: <158328768294.2223916.16551505954326988623.stgit@dwillia2-desk3.amr.corp.intel.com>
The pmem driver on PowerPC crashes with the following signature when
instantiating misaligned namespaces that map their capacity via
memremap_pages().
BUG: Unable to handle kernel data access at 0xc001000406000000
Faulting instruction address: 0xc000000000090790
NIP [c000000000090790] arch_add_memory+0xc0/0x130
LR [c000000000090744] arch_add_memory+0x74/0x130
Call Trace:
arch_add_memory+0x74/0x130 (unreliable)
memremap_pages+0x74c/0xa30
devm_memremap_pages+0x3c/0xa0
pmem_attach_disk+0x188/0x770
nvdimm_bus_probe+0xd8/0x470
With the assumption that only memremap_pages() has alignment
constraints, enforce memremap_compat_align() for
pmem_should_map_pages(), nd_pfn, and nd_dax cases. This includes
preventing the creation of namespaces where the base address is
misaligned and cases there infoblock padding parameters are invalid.
Reported-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
Cc: Jeff Moyer <jmoyer@redhat.com>
Fixes: a3619190d62e ("libnvdimm/pfn: stop padding pmem namespaces to section alignment")
Reviewed-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
drivers/nvdimm/namespace_devs.c | 17 +++++++++++++++++
drivers/nvdimm/pfn.h | 12 ++++++++++++
drivers/nvdimm/pfn_devs.c | 32 +++++++++++++++++++++++++++++---
3 files changed, 58 insertions(+), 3 deletions(-)
diff --git a/drivers/nvdimm/namespace_devs.c b/drivers/nvdimm/namespace_devs.c
index 032dc61725ff..77e211c7d94d 100644
--- a/drivers/nvdimm/namespace_devs.c
+++ b/drivers/nvdimm/namespace_devs.c
@@ -10,6 +10,7 @@
#include <linux/nd.h>
#include "nd-core.h"
#include "pmem.h"
+#include "pfn.h"
#include "nd.h"
static void namespace_io_release(struct device *dev)
@@ -1739,6 +1740,22 @@ struct nd_namespace_common *nvdimm_namespace_common_probe(struct device *dev)
return ERR_PTR(-ENODEV);
}
+ /*
+ * Note, alignment validation for fsdax and devdax mode
+ * namespaces happens in nd_pfn_validate() where infoblock
+ * padding parameters can be applied.
+ */
+ if (pmem_should_map_pages(dev)) {
+ struct nd_namespace_io *nsio = to_nd_namespace_io(&ndns->dev);
+ struct resource *res = &nsio->res;
+
+ if (!IS_ALIGNED(res->start | (res->end + 1),
+ memremap_compat_align())) {
+ dev_err(&ndns->dev, "%pr misaligned, unable to map\n", res);
+ return ERR_PTR(-EOPNOTSUPP);
+ }
+ }
+
if (is_namespace_pmem(&ndns->dev)) {
struct nd_namespace_pmem *nspm;
diff --git a/drivers/nvdimm/pfn.h b/drivers/nvdimm/pfn.h
index acb19517f678..37cb1b8a2a39 100644
--- a/drivers/nvdimm/pfn.h
+++ b/drivers/nvdimm/pfn.h
@@ -24,6 +24,18 @@ struct nd_pfn_sb {
__le64 npfns;
__le32 mode;
/* minor-version-1 additions for section alignment */
+ /**
+ * @start_pad: Deprecated attribute to pad start-misaligned namespaces
+ *
+ * start_pad is deprecated because the original definition did
+ * not comprehend that dataoff is relative to the base address
+ * of the namespace not the start_pad adjusted base. The result
+ * is that the dax path is broken, but the block-I/O path is
+ * not. The kernel will no longer create namespaces using start
+ * padding, but it still supports block-I/O for legacy
+ * configurations mainly to allow a backup, reconfigure the
+ * namespace, and restore flow to repair dax operation.
+ */
__le32 start_pad;
__le32 end_trunc;
/* minor-version-2 record the base alignment of the mapping */
diff --git a/drivers/nvdimm/pfn_devs.c b/drivers/nvdimm/pfn_devs.c
index 79fe02d6f657..34db557dbad1 100644
--- a/drivers/nvdimm/pfn_devs.c
+++ b/drivers/nvdimm/pfn_devs.c
@@ -446,6 +446,7 @@ static bool nd_supported_alignment(unsigned long align)
int nd_pfn_validate(struct nd_pfn *nd_pfn, const char *sig)
{
u64 checksum, offset;
+ struct resource *res;
enum nd_pfn_mode mode;
struct nd_namespace_io *nsio;
unsigned long align, start_pad;
@@ -578,13 +579,14 @@ int nd_pfn_validate(struct nd_pfn *nd_pfn, const char *sig)
* established.
*/
nsio = to_nd_namespace_io(&ndns->dev);
- if (offset >= resource_size(&nsio->res)) {
+ res = &nsio->res;
+ if (offset >= resource_size(res)) {
dev_err(&nd_pfn->dev, "pfn array size exceeds capacity of %s\n",
dev_name(&ndns->dev));
return -EOPNOTSUPP;
}
- if ((align && !IS_ALIGNED(nsio->res.start + offset + start_pad, align))
+ if ((align && !IS_ALIGNED(res->start + offset + start_pad, align))
|| !IS_ALIGNED(offset, PAGE_SIZE)) {
dev_err(&nd_pfn->dev,
"bad offset: %#llx dax disabled align: %#lx\n",
@@ -592,6 +594,18 @@ int nd_pfn_validate(struct nd_pfn *nd_pfn, const char *sig)
return -EOPNOTSUPP;
}
+ if (!IS_ALIGNED(res->start + le32_to_cpu(pfn_sb->start_pad),
+ memremap_compat_align())) {
+ dev_err(&nd_pfn->dev, "resource start misaligned\n");
+ return -EOPNOTSUPP;
+ }
+
+ if (!IS_ALIGNED(res->end + 1 - le32_to_cpu(pfn_sb->end_trunc),
+ memremap_compat_align())) {
+ dev_err(&nd_pfn->dev, "resource end misaligned\n");
+ return -EOPNOTSUPP;
+ }
+
return 0;
}
EXPORT_SYMBOL(nd_pfn_validate);
@@ -750,7 +764,19 @@ static int nd_pfn_init(struct nd_pfn *nd_pfn)
start = nsio->res.start;
size = resource_size(&nsio->res);
npfns = PHYS_PFN(size - SZ_8K);
- align = max(nd_pfn->align, SUBSECTION_SIZE);
+ align = max(nd_pfn->align, memremap_compat_align());
+
+ /*
+ * When @start is misaligned fail namespace creation. See
+ * the 'struct nd_pfn_sb' commentary on why ->start_pad is not
+ * an option.
+ */
+ if (!IS_ALIGNED(start, memremap_compat_align())) {
+ dev_err(&nd_pfn->dev, "%s: start %pa misaligned to %#lx\n",
+ dev_name(&ndns->dev), &start,
+ memremap_compat_align());
+ return -EINVAL;
+ }
end_trunc = start + size - ALIGN_DOWN(start + size, align);
if (nd_pfn->mode == PFN_MODE_PMEM) {
/*
^ permalink raw reply related
* [PATCH v4 2/5] libnvdimm/pfn: Prevent raw mode fallback if pfn-infoblock valid
From: Dan Williams @ 2020-03-04 2:08 UTC (permalink / raw)
To: linux-nvdimm
Cc: Aneesh Kumar K.V, Jeff Moyer, linuxppc-dev, linux-kernel,
vishal.l.verma
In-Reply-To: <158328768294.2223916.16551505954326988623.stgit@dwillia2-desk3.amr.corp.intel.com>
The EOPNOTSUPP return code from the pmem driver indicates that the
namespace has a configuration that may be valid, but the current kernel
does not support it. Expand this to all of the nd_pfn_validate() error
conditions after the infoblock has been verified as self consistent.
This prevents exposing the namespace to I/O when the infoblock needs to
be corrected, or the system needs to be put into a different
configuration (like changing the page size on PowerPC).
Cc: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
Cc: Jeff Moyer <jmoyer@redhat.com>
Reviewed-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
drivers/nvdimm/pfn_devs.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/nvdimm/pfn_devs.c b/drivers/nvdimm/pfn_devs.c
index a5c25cb87116..79fe02d6f657 100644
--- a/drivers/nvdimm/pfn_devs.c
+++ b/drivers/nvdimm/pfn_devs.c
@@ -561,14 +561,14 @@ int nd_pfn_validate(struct nd_pfn *nd_pfn, const char *sig)
dev_dbg(&nd_pfn->dev, "align: %lx:%lx mode: %d:%d\n",
nd_pfn->align, align, nd_pfn->mode,
mode);
- return -EINVAL;
+ return -EOPNOTSUPP;
}
}
if (align > nvdimm_namespace_capacity(ndns)) {
dev_err(&nd_pfn->dev, "alignment: %lx exceeds capacity %llx\n",
align, nvdimm_namespace_capacity(ndns));
- return -EINVAL;
+ return -EOPNOTSUPP;
}
/*
@@ -581,7 +581,7 @@ int nd_pfn_validate(struct nd_pfn *nd_pfn, const char *sig)
if (offset >= resource_size(&nsio->res)) {
dev_err(&nd_pfn->dev, "pfn array size exceeds capacity of %s\n",
dev_name(&ndns->dev));
- return -EBUSY;
+ return -EOPNOTSUPP;
}
if ((align && !IS_ALIGNED(nsio->res.start + offset + start_pad, align))
@@ -589,7 +589,7 @@ int nd_pfn_validate(struct nd_pfn *nd_pfn, const char *sig)
dev_err(&nd_pfn->dev,
"bad offset: %#llx dax disabled align: %#lx\n",
offset, align);
- return -ENXIO;
+ return -EOPNOTSUPP;
}
return 0;
^ permalink raw reply related
* [PATCH v4 1/5] mm/memremap_pages: Introduce memremap_compat_align()
From: Dan Williams @ 2020-03-04 2:08 UTC (permalink / raw)
To: linux-nvdimm
Cc: Aneesh Kumar K.V, linux-kernel, Jeff Moyer, Paul Mackerras,
vishal.l.verma, linuxppc-dev
In-Reply-To: <158328768294.2223916.16551505954326988623.stgit@dwillia2-desk3.amr.corp.intel.com>
The "sub-section memory hotplug" facility allows memremap_pages() users
like libnvdimm to compensate for hardware platforms like x86 that have a
section size larger than their hardware memory mapping granularity. The
compensation that sub-section support affords is being tolerant of
physical memory resources shifting by units smaller (64MiB on x86) than
the memory-hotplug section size (128 MiB). Where the platform
physical-memory mapping granularity is limited by the number and
capability of address-decode-registers in the memory controller.
While the sub-section support allows memremap_pages() to operate on
sub-section (2MiB) granularity, the Power architecture may still
require 16MiB alignment on "!radix_enabled()" platforms.
In order for libnvdimm to be able to detect and manage this per-arch
limitation, introduce memremap_compat_align() as a common minimum
alignment across all driver-facing memory-mapping interfaces, and let
Power override it to 16MiB in the "!radix_enabled()" case.
The assumption / requirement for 16MiB to be a viable
memremap_compat_align() value is that Power does not have platforms
where its equivalent of address-decode-registers never hardware remaps a
persistent memory resource on smaller than 16MiB boundaries. Note that I
tried my best to not add a new Kconfig symbol, but header include
entanglements defeated the #ifndef memremap_compat_align design pattern
and the need to export it defeats the __weak design pattern for arch
overrides.
Based on an initial patch by Aneesh.
Link: http://lore.kernel.org/r/CAPcyv4gBGNP95APYaBcsocEa50tQj9b5h__83vgngjq3ouGX_Q@mail.gmail.com
Reported-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
Reported-by: Jeff Moyer <jmoyer@redhat.com>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Reviewed-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
arch/powerpc/Kconfig | 1 +
arch/powerpc/mm/ioremap.c | 21 +++++++++++++++++++++
drivers/nvdimm/pfn_devs.c | 2 +-
include/linux/memremap.h | 8 ++++++++
include/linux/mmzone.h | 1 +
lib/Kconfig | 3 +++
mm/memremap.c | 23 +++++++++++++++++++++++
7 files changed, 58 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 497b7d0b2d7e..e6ffe905e2b9 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -122,6 +122,7 @@ config PPC
select ARCH_HAS_GCOV_PROFILE_ALL
select ARCH_HAS_KCOV
select ARCH_HAS_HUGEPD if HUGETLB_PAGE
+ select ARCH_HAS_MEMREMAP_COMPAT_ALIGN
select ARCH_HAS_MMIOWB if PPC64
select ARCH_HAS_PHYS_TO_DMA
select ARCH_HAS_PMEM_API
diff --git a/arch/powerpc/mm/ioremap.c b/arch/powerpc/mm/ioremap.c
index fc669643ce6a..b1a0aebe8c48 100644
--- a/arch/powerpc/mm/ioremap.c
+++ b/arch/powerpc/mm/ioremap.c
@@ -2,6 +2,7 @@
#include <linux/io.h>
#include <linux/slab.h>
+#include <linux/mmzone.h>
#include <linux/vmalloc.h>
#include <asm/io-workarounds.h>
@@ -97,3 +98,23 @@ void __iomem *do_ioremap(phys_addr_t pa, phys_addr_t offset, unsigned long size,
return NULL;
}
+
+#ifdef CONFIG_ZONE_DEVICE
+/*
+ * Override the generic version in mm/memremap.c.
+ *
+ * With hash translation, the direct-map range is mapped with just one
+ * page size selected by htab_init_page_sizes(). Consult
+ * mmu_psize_defs[] to determine the minimum page size alignment.
+*/
+unsigned long memremap_compat_align(void)
+{
+ unsigned int shift = mmu_psize_defs[mmu_linear_psize].shift;
+
+ if (radix_enabled())
+ return SUBSECTION_SIZE;
+ return max(SUBSECTION_SIZE, 1UL << shift);
+
+}
+EXPORT_SYMBOL_GPL(memremap_compat_align);
+#endif
diff --git a/drivers/nvdimm/pfn_devs.c b/drivers/nvdimm/pfn_devs.c
index b94f7a7e94b8..a5c25cb87116 100644
--- a/drivers/nvdimm/pfn_devs.c
+++ b/drivers/nvdimm/pfn_devs.c
@@ -750,7 +750,7 @@ static int nd_pfn_init(struct nd_pfn *nd_pfn)
start = nsio->res.start;
size = resource_size(&nsio->res);
npfns = PHYS_PFN(size - SZ_8K);
- align = max(nd_pfn->align, (1UL << SUBSECTION_SHIFT));
+ align = max(nd_pfn->align, SUBSECTION_SIZE);
end_trunc = start + size - ALIGN_DOWN(start + size, align);
if (nd_pfn->mode == PFN_MODE_PMEM) {
/*
diff --git a/include/linux/memremap.h b/include/linux/memremap.h
index 6fefb09af7c3..8af1cbd8f293 100644
--- a/include/linux/memremap.h
+++ b/include/linux/memremap.h
@@ -132,6 +132,7 @@ struct dev_pagemap *get_dev_pagemap(unsigned long pfn,
unsigned long vmem_altmap_offset(struct vmem_altmap *altmap);
void vmem_altmap_free(struct vmem_altmap *altmap, unsigned long nr_pfns);
+unsigned long memremap_compat_align(void);
#else
static inline void *devm_memremap_pages(struct device *dev,
struct dev_pagemap *pgmap)
@@ -165,6 +166,12 @@ static inline void vmem_altmap_free(struct vmem_altmap *altmap,
unsigned long nr_pfns)
{
}
+
+/* when memremap_pages() is disabled all archs can remap a single page */
+static inline unsigned long memremap_compat_align(void)
+{
+ return PAGE_SIZE;
+}
#endif /* CONFIG_ZONE_DEVICE */
static inline void put_dev_pagemap(struct dev_pagemap *pgmap)
@@ -172,4 +179,5 @@ static inline void put_dev_pagemap(struct dev_pagemap *pgmap)
if (pgmap)
percpu_ref_put(pgmap->ref);
}
+
#endif /* _LINUX_MEMREMAP_H_ */
diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
index 462f6873905a..6b77f7239af5 100644
--- a/include/linux/mmzone.h
+++ b/include/linux/mmzone.h
@@ -1170,6 +1170,7 @@ static inline unsigned long section_nr_to_pfn(unsigned long sec)
#define SECTION_ALIGN_DOWN(pfn) ((pfn) & PAGE_SECTION_MASK)
#define SUBSECTION_SHIFT 21
+#define SUBSECTION_SIZE (1UL << SUBSECTION_SHIFT)
#define PFN_SUBSECTION_SHIFT (SUBSECTION_SHIFT - PAGE_SHIFT)
#define PAGES_PER_SUBSECTION (1UL << PFN_SUBSECTION_SHIFT)
diff --git a/lib/Kconfig b/lib/Kconfig
index bc7e56370129..5d53f9609c25 100644
--- a/lib/Kconfig
+++ b/lib/Kconfig
@@ -615,6 +615,9 @@ config ARCH_HAS_PMEM_API
config MEMREGION
bool
+config ARCH_HAS_MEMREMAP_COMPAT_ALIGN
+ bool
+
# use memcpy to implement user copies for nommu architectures
config UACCESS_MEMCPY
bool
diff --git a/mm/memremap.c b/mm/memremap.c
index 09b5b7adc773..3e7afaf05639 100644
--- a/mm/memremap.c
+++ b/mm/memremap.c
@@ -7,6 +7,7 @@
#include <linux/mm.h>
#include <linux/pfn_t.h>
#include <linux/swap.h>
+#include <linux/mmzone.h>
#include <linux/swapops.h>
#include <linux/types.h>
#include <linux/wait_bit.h>
@@ -14,6 +15,28 @@
static DEFINE_XARRAY(pgmap_array);
+/*
+ * The memremap() and memremap_pages() interfaces are alternately used
+ * to map persistent memory namespaces. These interfaces place different
+ * constraints on the alignment and size of the mapping (namespace).
+ * memremap() can map individual PAGE_SIZE pages. memremap_pages() can
+ * only map subsections (2MB), and at least one architecture (PowerPC)
+ * the minimum mapping granularity of memremap_pages() is 16MB.
+ *
+ * The role of memremap_compat_align() is to communicate the minimum
+ * arch supported alignment of a namespace such that it can freely
+ * switch modes without violating the arch constraint. Namely, do not
+ * allow a namespace to be PAGE_SIZE aligned since that namespace may be
+ * reconfigured into a mode that requires SUBSECTION_SIZE alignment.
+ */
+#ifndef CONFIG_ARCH_HAS_MEMREMAP_COMPAT_ALIGN
+unsigned long memremap_compat_align(void)
+{
+ return SUBSECTION_SIZE;
+}
+EXPORT_SYMBOL_GPL(memremap_compat_align);
+#endif
+
#ifdef CONFIG_DEV_PAGEMAP_OPS
DEFINE_STATIC_KEY_FALSE(devmap_managed_key);
EXPORT_SYMBOL(devmap_managed_key);
^ permalink raw reply related
* [PATCH v4 0/5] libnvdimm: Cross-arch compatible namespace alignment
From: Dan Williams @ 2020-03-04 2:08 UTC (permalink / raw)
To: linux-nvdimm
Cc: Vishal Verma, linux-kernel, Jeff Moyer, Oliver O'Halloran,
Aneesh Kumar K.V, Paul Mackerras, linuxppc-dev
Changes since v3 [1]:
- Collect Aneesh's Reviewed-by for Patch1-Patch3
- Add commentary to "libnvdimm/namespace: Enforce
memremap_compat_align()" to clarify alignment validation and why
start_pad is deprecated (Aneesh)
[1]: http://lore.kernel.org/r/158291746615.1609624.7591692546429050845.stgit@dwillia2-desk3.amr.corp.intel.com
---
Patch "mm/memremap_pages: Introduce memremap_compat_align()" still
needs a PowerPC maintainer ack for the touches to
arch/powerpc/mm/ioremap.c.
---
Aneesh reports that PowerPC requires 16MiB alignment for the address
range passed to devm_memremap_pages(), and Jeff reports that it is
possible to create a misaligned namespace which blocks future namespace
creation in that region. Both of these issues require namespace
alignment to be managed at the region level rather than padding at the
namespace level which has been a broken approach to date.
Introduce memremap_compat_align() to indicate the hard requirements of
an arch's memremap_pages() implementation. Use the maximum known
memremap_compat_align() to set the default namespace alignment for
libnvdimm. Consult that alignment when allocating free space. Finally,
allow the default region alignment to be overridden to maintain the same
namespace creation capability as previous kernels (modulo dax operation
not being supported with a non-zero start_pad).
The ndctl unit tests, which have some misaligned namespace assumptions,
are updated to use the alignment override where necessary.
Thanks to Aneesh for early feedback and testing on this change to
alignment handling.
---
Dan Williams (5):
mm/memremap_pages: Introduce memremap_compat_align()
libnvdimm/pfn: Prevent raw mode fallback if pfn-infoblock valid
libnvdimm/namespace: Enforce memremap_compat_align()
libnvdimm/region: Introduce NDD_LABELING
libnvdimm/region: Introduce an 'align' attribute
arch/powerpc/Kconfig | 1
arch/powerpc/mm/ioremap.c | 21 +++++
arch/powerpc/platforms/pseries/papr_scm.c | 2
drivers/acpi/nfit/core.c | 4 +
drivers/nvdimm/dimm.c | 2
drivers/nvdimm/dimm_devs.c | 95 +++++++++++++++++----
drivers/nvdimm/namespace_devs.c | 28 +++++-
drivers/nvdimm/nd.h | 3 -
drivers/nvdimm/pfn.h | 12 +++
drivers/nvdimm/pfn_devs.c | 40 +++++++--
drivers/nvdimm/region_devs.c | 132 ++++++++++++++++++++++++++---
include/linux/libnvdimm.h | 2
include/linux/memremap.h | 8 ++
include/linux/mmzone.h | 1
lib/Kconfig | 3 +
mm/memremap.c | 23 +++++
16 files changed, 330 insertions(+), 47 deletions(-)
base-commit: 1d0827b75ee7df497f611a2ac412a88135fb0ef5
^ permalink raw reply
* Re: [PATCH net-next 00/23] Clean driver, module and FW versions
From: David Miller @ 2020-03-04 1:55 UTC (permalink / raw)
To: leon
Cc: ajit.khaparde, madalin.bucur, kda, prashant, _govind,
somnath.kotur, vishal, GR-everest-linux-l2, leedom, opendmb,
bcm-kernel-feedback-list, kuba, linus.walleij, sgoutham, pkaustub,
aelior, ulli.kroll, sburla, fmanlunas, leonro, claudiu.manoil,
f.fainelli, sathya.perla, michael.chan, linux-arm-kernel,
rvatsavayi, GR-Linux-NIC-Dev, fugang.duan, sriharsha.basavapatna,
linux-parisc, siva.kallam, rmody, netdev, linux-kernel,
leoyang.li, hsweeten, rrichter, dchickles, linuxppc-dev, skalluru,
benve
In-Reply-To: <20200301144457.119795-1-leon@kernel.org>
From: Leon Romanovsky <leon@kernel.org>
Date: Sun, 1 Mar 2020 16:44:33 +0200
> From: Leon Romanovsky <leonro@mellanox.com>
>
> Hi,
>
> This is second batch of the series which removes various static versions
> in favour of globaly defined Linux kernel version.
>
> The first part with better cover letter can be found here
> https://lore.kernel.org/lkml/20200224085311.460338-1-leon@kernel.org
>
> The code is based on
> 68e2c37690b0 ("Merge branch 'hsr-several-code-cleanup-for-hsr-module'")
>
> and WIP branch is
> https://git.kernel.org/pub/scm/linux/kernel/git/leon/linux-rdma.git/log/?h=ethtool
Series applied, thanks.
^ permalink raw reply
* Re: [PATCH V14] mm/debug: Add tests validating architecture page table helpers
From: Qian Cai @ 2020-03-04 1:39 UTC (permalink / raw)
To: 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: <e8516497-f1b9-b222-e219-73b68880ac75@arm.com>
> 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()?
[ 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]
[ 60.418913][ C8] Modules linked in:
[ 60.418927][ C8] irq event stamp: 2896762
[ 60.418945][ C8] hardirqs last enabled at (2896761): [<c00000000000dec8>]
fast_exc_return_irq+0x28/0x34
[ 60.418960][ C8] hardirqs last disabled at (2896762): [<c00000000000924c>]
decrementer_common+0x10c/0x130
[ 60.418985][ C8] softirqs last enabled at (2896760): [<c0000000009a1bf0>]
__do_softirq+0x640/0x8c8
[ 60.419009][ C8] softirqs last disabled at (2896753): [<c000000000113cbc>]
irq_exit+0x16c/0x1d0
[ 60.419024][ C8] CPU: 8 PID: 1 Comm: swapper/0 Not tainted 5.6.0-rc4-next-
20200303+ #7
[ 60.419055][ C8] NIP: c00000000103dc14 LR: c00000000103db0c CTR:
0000000000000000
[ 60.419076][ C8] REGS: c00000003dd4fa30 TRAP: 0901 Not tainted (5.6.0-
rc4-next-20200303+)
[ 60.419107][ C8] MSR: 9000000000009033 <SF,HV,EE,ME,IR,DR,RI,LE> CR:
42000222 XER: 00000000
[ 60.419134][ C8] CFAR: c00000000103dc1c IRQMASK: 0
[ 60.419134][ C8] GPR00: c00000000103db0c c00000003dd4fcc0 c000000001657d00
05210001000000c0
[ 60.419134][ C8] GPR04: 8000000000000105 000000000000000a 00000000f4d9864c
0000000000000001
[ 60.419134][ C8] GPR08: 0000000000000000 ffffffffffffffff 0000000000000001
00000000000a0000
[ 60.419134][ C8] GPR12: 0000000000000000 c000001fffff9880
[ 60.419220][ C8] NIP [c00000000103dc14] debug_vm_pgtable+0x7a8/0xbb4
hash__pte_update at arch/powerpc/include/asm/book3s/64/hash.h:159
(inlined by) pte_update at arch/powerpc/include/asm/book3s/64/pgtable.h:359
(inlined by) pte_clear at arch/powerpc/include/asm/book3s/64/pgtable.h:477
(inlined by) pte_clear_tests at mm/debug_vm_pgtable.c:259
(inlined by) debug_vm_pgtable at mm/debug_vm_pgtable.c:368
[ 60.419241][ C8] LR [c00000000103db0c] debug_vm_pgtable+0x6a0/0xbb4
pmd_basic_tests at mm/debug_vm_pgtable.c:74
(inlined by) debug_vm_pgtable at mm/debug_vm_pgtable.c:363
[ 60.419260][ C8] Call Trace:
[ 60.419278][ C8] [c00000003dd4fcc0] [c00000000103d994]
debug_vm_pgtable+0x528/0xbb4 (unreliable)
[ 60.419302][ C8] [c00000003dd4fdb0] [c000000000010eac]
kernel_init+0x30/0x194
[ 60.419325][ C8] [c00000003dd4fe20] [c00000000000b748]
ret_from_kernel_thread+0x5c/0x74
[ 60.419363][ C8] Instruction dump:
[ 60.419382][ C8] 7d075078 7ce74b78 7ce0f9ad 40c2fff0 7e449378 7fc3f378
4b03531d 60000000
[ 60.419416][ C8] 48000080 3920ffff 39400001 39000000 <7e00f8a8> 7e075039
40c2fff8 7e074878
[ 98.908889][ C8] rcu: INFO: rcu_sched self-detected stall on CPU
[ 98.908933][ C8] rcu: 8-....: (6500 ticks this GP)
idle=522/1/0x4000000000000002 softirq=132/132 fqs=3250
[ 98.908963][ C8] (t=6501 jiffies g=-719 q=510)
[ 98.908984][ C8] NMI backtrace for cpu 8
[ 98.909012][ C8] CPU: 8 PID: 1 Comm: swapper/0 Tainted:
G L 5.6.0-rc4-next-20200303+ #7
[ 98.909025][ C8] Call Trace:
[ 98.909046][ C8] [c00000003dd4f360] [c000000000970fe0]
dump_stack+0xf4/0x164 (unreliable)
[ 98.909070][ C8] [c00000003dd4f3b0] [c00000000097dcf4]
nmi_cpu_backtrace+0x1b4/0x1e0
[ 98.909084][ C8] [c00000003dd4f450] [c00000000097df48]
nmi_trigger_cpumask_backtrace+0x228/0x2c0
[ 98.909118][ C8] [c00000003dd4f500] [c000000000057bf8]
arch_trigger_cpumask_backtrace+0x28/0x40
[ 98.909152][ C8] [c00000003dd4f520] [c000000000202dd4]
rcu_dump_cpu_stacks+0x1c4/0x234
[ 98.909184][ C8] [c00000003dd4f5a0] [c000000000201634]
rcu_sched_clock_irq+0xd54/0x1130
[ 98.909207][ C8] [c00000003dd4f6c0] [c000000000217068]
update_process_times+0x48/0xb0
[ 98.909239][ C8] [c00000003dd4f6f0] [c0000000002358b4]
tick_sched_handle+0x34/0xb0
[ 98.909262][ C8] [c00000003dd4f720] [c0000000002361d8]
tick_sched_timer+0x68/0xe0
[ 98.909284][ C8] [c00000003dd4f760] [c000000000219768]
__hrtimer_run_queues+0x528/0xa60
[ 98.909306][ C8] [c00000003dd4f880] [c00000000021ab58]
hrtimer_interrupt+0x128/0x330
[ 98.909329][ C8] [c00000003dd4f930] [c00000000002e1b4]
timer_interrupt+0x264/0x680
[ 98.909352][ C8] [c00000003dd4f9c0] [c000000000009264]
decrementer_common+0x124/0x130
[ 98.909366][ C8] --- interrupt: 901 at debug_vm_pgtable+0x7a8/0xbb4
[ 98.909366][ C8] LR = debug_vm_pgtable+0x6a0/0xbb4
[ 98.909402][ C8] [c00000003dd4fcc0] [c00000000103d994]
debug_vm_pgtable+0x528/0xbb4 (unreliable)
[ 98.909435][ C8] [c00000003dd4fdb0] [c000000000010eac]
kernel_init+0x30/0x194
[ 98.909467][ C8] [c00000003dd4fe20] [c00000000000b748]
ret_from_kernel_thread+0x5c/0x74
[ 124.418885][ C8] watchdog: BUG: soft lockup - CPU#8 stuck for 22s!
[swapper/0:1]
[ 124.418914][ C8] Modules linked in:
[ 124.418926][ C8] irq event stamp: 2937938
[ 124.418940][ C8] hardirqs last enabled at (2937937): [<c00000000000dec8>]
fast_exc_return_irq+0x28/0x34
[ 124.418964][ C8] hardirqs last disabled at (2937938): [<c00000000000924c>]
decrementer_common+0x10c/0x130
[ 124.418980][ C8] softirqs last enabled at (2937936): [<c0000000009a1bf0>]
__do_softirq+0x640/0x8c8
[ 124.419013][ C8] softirqs last disabled at (2937929): [<c000000000113cbc>]
irq_exit+0x16c/0x1d0
[ 124.419036][ C8] CPU: 8 PID: 1 Comm: swapper/0 Tainted:
G L 5.6.0-rc4-next-20200303+ #7
[ 124.419059][ C8] NIP: c00000000103dc14 LR: c00000000103db0c CTR:
0000000000000000
[ 124.419080][ C8] REGS: c00000003dd4fa30 TRAP: 0901 Tainted:
G L (5.6.0-rc4-next-20200303+)
[ 124.419103][ C8] MSR: 9000000000009033 <SF,HV,EE,ME,IR,DR,RI,LE> CR:
42000222 XER: 00000000
[ 124.419121][ C8] CFAR: c00000000103dc1c IRQMASK: 0
[ 124.419121][ C8] GPR00: c00000000103db0c c00000003dd4fcc0 c000000001657d00
05210001000000c0
[ 124.419121][ C8] GPR04: 8000000000000105 000000000000000a 00000000f4d9864c
0000000000000001
[ 124.419121][ C8] GPR08: 0000000000000000 ffffffffffffffff 0000000000000001
00000000000a0000
[ 124.419121][ C8] GPR12: 0000000000000000 c000001fffff9880
[ 124.419234][ C8] NIP [c00000000103dc14] debug_vm_pgtable+0x7a8/0xbb4
[ 124.419254][ C8] LR [c00000000103db0c] debug_vm_pgtable+0x6a0/0xbb4
[ 124.419274][ C8] Call Trace:
[ 124.419291][ C8] [c00000003dd4fcc0] [c00000000103d994]
debug_vm_pgtable+0x528/0xbb4 (unreliable)
[ 124.419324][ C8] [c00000003dd4fdb0] [c000000000010eac]
kernel_init+0x30/0x194
[ 124.419347][ C8] [c00000003dd4fe20] [c00000000000b748]
ret_from_kernel_thread+0x5c/0x74
[ 124.419359][ C8] Instruction dump:
[ 124.419378][ C8] 7d075078 7ce74b78 7ce0f9ad 40c2fff0 7e449378 7fc3f378
4b03531d 60000000
[ 124.419412][ C8] 48000080 3920ffff 39400001 39000000 <7e00f8a8> 7e075039
40c2fff8 7e074878
>
> mm/debug_vm_pgtable.c | 12 +++++++-----
> 1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/mm/debug_vm_pgtable.c b/mm/debug_vm_pgtable.c
> index 96dd7d574cef..c310f52c2b80 100644
> --- a/mm/debug_vm_pgtable.c
> +++ b/mm/debug_vm_pgtable.c
> @@ -250,13 +250,14 @@ static void __init pgd_populate_tests(struct mm_struct *mm, pgd_t *pgdp,
> }
> #endif
>
> -static void __init pte_clear_tests(struct mm_struct *mm, pte_t *ptep)
> +static void __init pte_clear_tests(struct mm_struct *mm, pte_t *ptep,
> + unsigned long vaddr)
> {
> pte_t pte = READ_ONCE(*ptep);
>
> pte = __pte(pte_val(pte) | RANDOM_ORVALUE);
> WRITE_ONCE(*ptep, pte);
> - pte_clear(mm, 0, ptep);
> + pte_clear(mm, vaddr, ptep);
> pte = READ_ONCE(*ptep);
> WARN_ON(!pte_none(pte));
> }
> @@ -302,6 +303,7 @@ static unsigned long __init get_random_vaddr(void)
> void __init debug_vm_pgtable(void)
> {
> struct mm_struct *mm;
> + spinlock_t *uninitialized_var(ptl);
> pgd_t *pgdp;
> p4d_t *p4dp, *saved_p4dp;
> pud_t *pudp, *saved_pudp;
> @@ -344,7 +346,7 @@ void __init debug_vm_pgtable(void)
> p4dp = p4d_alloc(mm, pgdp, vaddr);
> pudp = pud_alloc(mm, p4dp, vaddr);
> pmdp = pmd_alloc(mm, pudp, vaddr);
> - ptep = pte_alloc_map(mm, pmdp, vaddr);
> + ptep = pte_alloc_map_lock(mm, pmdp, vaddr, &ptl);
>
> /*
> * Save all the page table page addresses as the page table
> @@ -364,13 +366,13 @@ void __init debug_vm_pgtable(void)
> p4d_basic_tests(p4d_aligned, prot);
> pgd_basic_tests(pgd_aligned, prot);
>
> - pte_clear_tests(mm, ptep);
> + pte_clear_tests(mm, ptep, vaddr);
> pmd_clear_tests(mm, pmdp);
> pud_clear_tests(mm, pudp);
> p4d_clear_tests(mm, p4dp);
> pgd_clear_tests(mm, pgdp);
>
> - pte_unmap(ptep);
> + pte_unmap_unlock(ptep, ptl);
>
> pmd_populate_tests(mm, pmdp, saved_ptep);
> pud_populate_tests(mm, pudp, saved_pmdp);
> --
> 2.20.1
^ permalink raw reply
* [PATCH v3] powerpc: Replace setup_irq() by request_irq()
From: afzal mohammed @ 2020-03-04 0:47 UTC (permalink / raw)
To: linuxppc-dev, linux-kernel; +Cc: Scott Wood, Paul Mackerras, afzal mohammed
request_irq() is preferred over setup_irq(). Invocations of setup_irq()
occur after memory allocators are ready.
Per tglx[1], setup_irq() existed in olden days when allocators were not
ready by the time early interrupts were initialized.
Hence replace setup_irq() by request_irq().
[1] https://lkml.kernel.org/r/alpine.DEB.2.20.1710191609480.1971@nanos
Signed-off-by: afzal mohammed <afzal.mohd.ma@gmail.com>
---
Hi powerpc maintainers,
if okay w/ this change, please consider taking it thr' your tree, else please
let me know.
Regards
afzal
Link to v2 & v1,
[v2] https://lkml.kernel.org/r/cover.1582471508.git.afzal.mohd.ma@gmail.com
[v1] https://lkml.kernel.org/r/cover.1581478323.git.afzal.mohd.ma@gmail.com
v3:
* Split out from tree wide series, as Thomas suggested to get it thr'
respective maintainers
* Modify pr_err displayed in case of error
* Re-arrange code & choose pr_err args as required to improve readability
* Remove irrelevant parts from commit message & improve
v2:
* Replace pr_err("request_irq() on %s failed" by
pr_err("%s: request_irq() failed"
* Commit message massage
arch/powerpc/platforms/85xx/mpc85xx_cds.c | 10 +++-----
arch/powerpc/platforms/8xx/cpm1.c | 9 ++-----
arch/powerpc/platforms/8xx/m8xx_setup.c | 9 ++-----
arch/powerpc/platforms/chrp/setup.c | 14 ++++-------
arch/powerpc/platforms/powermac/pic.c | 29 +++++++++--------------
arch/powerpc/platforms/powermac/smp.c | 12 ++++------
6 files changed, 28 insertions(+), 55 deletions(-)
diff --git a/arch/powerpc/platforms/85xx/mpc85xx_cds.c b/arch/powerpc/platforms/85xx/mpc85xx_cds.c
index 6b1436abe9b1..1c5598877d70 100644
--- a/arch/powerpc/platforms/85xx/mpc85xx_cds.c
+++ b/arch/powerpc/platforms/85xx/mpc85xx_cds.c
@@ -218,12 +218,6 @@ static irqreturn_t mpc85xx_8259_cascade_action(int irq, void *dev_id)
{
return IRQ_HANDLED;
}
-
-static struct irqaction mpc85xxcds_8259_irqaction = {
- .handler = mpc85xx_8259_cascade_action,
- .flags = IRQF_SHARED | IRQF_NO_THREAD,
- .name = "8259 cascade",
-};
#endif /* PPC_I8259 */
#endif /* CONFIG_PCI */
@@ -271,7 +265,9 @@ static int mpc85xx_cds_8259_attach(void)
* disabled when the last user of the shared IRQ line frees their
* interrupt.
*/
- if ((ret = setup_irq(cascade_irq, &mpc85xxcds_8259_irqaction))) {
+ ret = request_irq(cascade_irq, mpc85xx_8259_cascade_action,
+ IRQF_SHARED | IRQF_NO_THREAD, "8259 cascade", NULL);
+ if (ret) {
printk(KERN_ERR "Failed to setup cascade interrupt\n");
return ret;
}
diff --git a/arch/powerpc/platforms/8xx/cpm1.c b/arch/powerpc/platforms/8xx/cpm1.c
index a43ee7d1ff85..4db4ca2e1222 100644
--- a/arch/powerpc/platforms/8xx/cpm1.c
+++ b/arch/powerpc/platforms/8xx/cpm1.c
@@ -120,12 +120,6 @@ static irqreturn_t cpm_error_interrupt(int irq, void *dev)
return IRQ_HANDLED;
}
-static struct irqaction cpm_error_irqaction = {
- .handler = cpm_error_interrupt,
- .flags = IRQF_NO_THREAD,
- .name = "error",
-};
-
static const struct irq_domain_ops cpm_pic_host_ops = {
.map = cpm_pic_host_map,
};
@@ -187,7 +181,8 @@ unsigned int __init cpm_pic_init(void)
if (!eirq)
goto end;
- if (setup_irq(eirq, &cpm_error_irqaction))
+ if (request_irq(eirq, cpm_error_interrupt, IRQF_NO_THREAD, "error",
+ NULL))
printk(KERN_ERR "Could not allocate CPM error IRQ!");
setbits32(&cpic_reg->cpic_cicr, CICR_IEN);
diff --git a/arch/powerpc/platforms/8xx/m8xx_setup.c b/arch/powerpc/platforms/8xx/m8xx_setup.c
index f1c805c8adbc..df4d57d07f9a 100644
--- a/arch/powerpc/platforms/8xx/m8xx_setup.c
+++ b/arch/powerpc/platforms/8xx/m8xx_setup.c
@@ -39,12 +39,6 @@ static irqreturn_t timebase_interrupt(int irq, void *dev)
return IRQ_HANDLED;
}
-static struct irqaction tbint_irqaction = {
- .handler = timebase_interrupt,
- .flags = IRQF_NO_THREAD,
- .name = "tbint",
-};
-
/* per-board overridable init_internal_rtc() function. */
void __init __attribute__ ((weak))
init_internal_rtc(void)
@@ -157,7 +151,8 @@ void __init mpc8xx_calibrate_decr(void)
(TBSCR_TBF | TBSCR_TBE));
immr_unmap(sys_tmr2);
- if (setup_irq(virq, &tbint_irqaction))
+ if (request_irq(virq, timebase_interrupt, IRQF_NO_THREAD, "tbint",
+ NULL))
panic("Could not allocate timer IRQ!");
}
diff --git a/arch/powerpc/platforms/chrp/setup.c b/arch/powerpc/platforms/chrp/setup.c
index fcf6f2342ef4..8328cd5817b0 100644
--- a/arch/powerpc/platforms/chrp/setup.c
+++ b/arch/powerpc/platforms/chrp/setup.c
@@ -451,13 +451,6 @@ static void __init chrp_find_openpic(void)
of_node_put(np);
}
-#if defined(CONFIG_VT) && defined(CONFIG_INPUT_ADBHID) && defined(CONFIG_XMON)
-static struct irqaction xmon_irqaction = {
- .handler = xmon_irq,
- .name = "XMON break",
-};
-#endif
-
static void __init chrp_find_8259(void)
{
struct device_node *np, *pic = NULL;
@@ -541,8 +534,11 @@ static void __init chrp_init_IRQ(void)
if (of_node_is_type(kbd->parent, "adb"))
break;
of_node_put(kbd);
- if (kbd)
- setup_irq(HYDRA_INT_ADB_NMI, &xmon_irqaction);
+ if (kbd) {
+ if (request_irq(HYDRA_INT_ADB_NMI, xmon_irq, 0, "XMON break",
+ NULL))
+ pr_err("Failed to register XMON break interrupt\n");
+ }
#endif
}
diff --git a/arch/powerpc/platforms/powermac/pic.c b/arch/powerpc/platforms/powermac/pic.c
index 2e969073473d..4921bccf0376 100644
--- a/arch/powerpc/platforms/powermac/pic.c
+++ b/arch/powerpc/platforms/powermac/pic.c
@@ -250,20 +250,6 @@ static unsigned int pmac_pic_get_irq(void)
return irq_linear_revmap(pmac_pic_host, irq);
}
-#ifdef CONFIG_XMON
-static struct irqaction xmon_action = {
- .handler = xmon_irq,
- .flags = IRQF_NO_THREAD,
- .name = "NMI - XMON"
-};
-#endif
-
-static struct irqaction gatwick_cascade_action = {
- .handler = gatwick_action,
- .flags = IRQF_NO_THREAD,
- .name = "cascade",
-};
-
static int pmac_pic_host_match(struct irq_domain *h, struct device_node *node,
enum irq_domain_bus_token bus_token)
{
@@ -384,12 +370,17 @@ static void __init pmac_pic_probe_oldstyle(void)
out_le32(&pmac_irq_hw[i]->enable, 0);
/* Hookup cascade irq */
- if (slave && pmac_irq_cascade)
- setup_irq(pmac_irq_cascade, &gatwick_cascade_action);
+ if (slave && pmac_irq_cascade) {
+ if (request_irq(pmac_irq_cascade, gatwick_action,
+ IRQF_NO_THREAD, "cascade", NULL))
+ pr_err("Failed to register cascade interrupt\n");
+ }
printk(KERN_INFO "irq: System has %d possible interrupts\n", max_irqs);
#ifdef CONFIG_XMON
- setup_irq(irq_create_mapping(NULL, 20), &xmon_action);
+ i = irq_create_mapping(NULL, 20);
+ if (request_irq(i, xmon_irq, IRQF_NO_THREAD, "NMI - XMON", NULL))
+ pr_err("Failed to register NMI-XMON interrupt\n");
#endif
}
@@ -441,7 +432,9 @@ static void __init pmac_pic_setup_mpic_nmi(struct mpic *mpic)
nmi_irq = irq_of_parse_and_map(pswitch, 0);
if (nmi_irq) {
mpic_irq_set_priority(nmi_irq, 9);
- setup_irq(nmi_irq, &xmon_action);
+ if (request_irq(nmi_irq, xmon_irq, IRQF_NO_THREAD,
+ "NMI - XMON", NULL))
+ pr_err("Failed to register NMI-XMON interrupt\n");
}
of_node_put(pswitch);
}
diff --git a/arch/powerpc/platforms/powermac/smp.c b/arch/powerpc/platforms/powermac/smp.c
index f95fbdee6efe..c55bf474ed4e 100644
--- a/arch/powerpc/platforms/powermac/smp.c
+++ b/arch/powerpc/platforms/powermac/smp.c
@@ -399,21 +399,19 @@ static int __init smp_psurge_kick_cpu(int nr)
return 0;
}
-static struct irqaction psurge_irqaction = {
- .handler = psurge_ipi_intr,
- .flags = IRQF_PERCPU | IRQF_NO_THREAD,
- .name = "primary IPI",
-};
-
static void __init smp_psurge_setup_cpu(int cpu_nr)
{
+ unsigned long flags = IRQF_PERCPU | IRQF_NO_THREAD;
+ int irq;
+
if (cpu_nr != 0 || !psurge_start)
return;
/* reset the entry point so if we get another intr we won't
* try to startup again */
out_be32(psurge_start, 0x100);
- if (setup_irq(irq_create_mapping(NULL, 30), &psurge_irqaction))
+ irq = irq_create_mapping(NULL, 30);
+ if (request_irq(irq, psurge_ipi_intr, flags, "primary IPI", NULL))
printk(KERN_ERR "Couldn't get primary IPI interrupt");
}
--
2.25.1
^ permalink raw reply related
* Re: [PATCH v3 14/18] docs: powerpc: convert vcpudispatch_stats.txt to ReST
From: Michael Ellerman @ 2020-03-04 0:17 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: linux-doc, Mauro Carvalho Chehab, Jonathan Corbet, Paul Mackerras,
linuxppc-dev
In-Reply-To: <b85d594912818721b84b3c9a0aafa472d6d4af44.1583243826.git.mchehab+huawei@kernel.org>
Mauro Carvalho Chehab <mchehab+huawei@kernel.org> writes:
> - Add a SPDX header;
> - Use standard markup for document title;
> - Adjust identation on lists and add blank lines where
> needed;
> - Add it to the powerpc index.rst file.
>
> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
> ---
> Documentation/powerpc/index.rst | 1 +
> ...ispatch_stats.txt => vcpudispatch_stats.rst} | 17 ++++++++++++-----
> 2 files changed, 13 insertions(+), 5 deletions(-)
> rename Documentation/powerpc/{vcpudispatch_stats.txt => vcpudispatch_stats.rst} (94%)
LGTM.
Acked-by: Michael Ellerman <mpe@ellerman.id.au> (powerpc)
I'm going to assume this will go via the docs tree, unless you tell me otherwise.
cheers
^ permalink raw reply
* [PATCH v2] powerpc/mm: Fix missing KUAP disable in flush_coherent_icache()
From: Michael Ellerman @ 2020-03-03 23:57 UTC (permalink / raw)
To: linuxppc-dev
We received a report of strange kernel faults which turned out to be
due to a missing KUAP disable in flush_coherent_icache() called
from flush_icache_range().
The fault looks like:
Kernel attempted to access user page (7fffc30d9c00) - exploit attempt? (uid: 1009)
BUG: Unable to handle kernel data access on read at 0x7fffc30d9c00
Faulting instruction address: 0xc00000000007232c
Oops: Kernel access of bad area, sig: 11 [#1]
LE PAGE_SIZE=64K MMU=Radix SMP NR_CPUS=2048 NUMA PowerNV
CPU: 35 PID: 5886 Comm: sigtramp Not tainted 5.6.0-rc2-gcc-8.2.0-00003-gfc37a1632d40 #79
NIP: c00000000007232c LR: c00000000003b7fc CTR: 0000000000000000
REGS: c000001e11093940 TRAP: 0300 Not tainted (5.6.0-rc2-gcc-8.2.0-00003-gfc37a1632d40)
MSR: 900000000280b033 <SF,HV,VEC,VSX,EE,FP,ME,IR,DR,RI,LE> CR: 28000884 XER: 00000000
CFAR: c0000000000722fc DAR: 00007fffc30d9c00 DSISR: 08000000 IRQMASK: 0
GPR00: c00000000003b7fc c000001e11093bd0 c0000000023ac200 00007fffc30d9c00
GPR04: 00007fffc30d9c18 0000000000000000 c000001e11093bd4 0000000000000000
GPR08: 0000000000000000 0000000000000001 0000000000000000 c000001e1104ed80
GPR12: 0000000000000000 c000001fff6ab380 c0000000016be2d0 4000000000000000
GPR16: c000000000000000 bfffffffffffffff 0000000000000000 0000000000000000
GPR20: 00007fffc30d9c00 00007fffc30d8f58 00007fffc30d9c18 00007fffc30d9c20
GPR24: 00007fffc30d9c18 0000000000000000 c000001e11093d90 c000001e1104ed80
GPR28: c000001e11093e90 0000000000000000 c0000000023d9d18 00007fffc30d9c00
NIP flush_icache_range+0x5c/0x80
LR handle_rt_signal64+0x95c/0xc2c
Call Trace:
0xc000001e11093d90 (unreliable)
handle_rt_signal64+0x93c/0xc2c
do_notify_resume+0x310/0x430
ret_from_except_lite+0x70/0x74
Instruction dump:
409e002c 7c0802a6 3c62ff31 3863f6a0 f8010080 48195fed 60000000 48fe4c8d
60000000 e8010080 7c0803a6 7c0004ac <7c00ffac> 7c0004ac 4c00012c 38210070
This path through handle_rt_signal64() to setup_trampoline() and
flush_icache_range() is only triggered by 64-bit processes that have
unmapped their VDSO, which is rare.
flush_icache_range() takes a range of addresses to flush. In
flush_coherent_icache() we implement an optimisation for CPUs where we
know we don't actually have to flush the whole range, we just need to
do a single icbi.
However we still execute the icbi on the user address of the start of
the range we're flushing. On CPUs that also implement KUAP (Power9)
that leads to the spurious fault above.
We should be able to pass any address, including a kernel address, to
the icbi on these CPUs, which would avoid any interaction with KUAP.
But I don't want to make that change in a bug fix, just in case it
surfaces some strange behaviour on some CPU.
So for now just disable KUAP around the icbi. Note the icbi is treated
as a load, so we allow read access, not write as you'd expect.
Fixes: 890274c2dc4c ("powerpc/64s: Implement KUAP for Radix MMU")
Cc: stable@vger.kernel.org # v5.2+
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
arch/powerpc/mm/mem.c | 2 ++
1 file changed, 2 insertions(+)
v2: Use L1_CACHE_BYTES as suggested by Christophe.
diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
index ef7b1119b2e2..36a8c7b105ce 100644
--- a/arch/powerpc/mm/mem.c
+++ b/arch/powerpc/mm/mem.c
@@ -373,7 +373,9 @@ static inline bool flush_coherent_icache(unsigned long addr)
*/
if (cpu_has_feature(CPU_FTR_COHERENT_ICACHE)) {
mb(); /* sync */
+ allow_read_from_user((void *)addr, L1_CACHE_BYTES);
icbi((void *)addr);
+ prevent_read_from_user((void *)addr, L1_CACHE_BYTES);
mb(); /* sync */
isync();
return true;
--
2.21.1
^ permalink raw reply related
* Re: [PATCH] powerpc/mm: Fix missing KUAP disable in flush_coherent_icache()
From: Michael Ellerman @ 2020-03-03 23:50 UTC (permalink / raw)
To: Christophe Leroy, linuxppc-dev
In-Reply-To: <2ad423ee-590b-9198-7fc3-ea6f8900ad23@c-s.fr>
Christophe Leroy <christophe.leroy@c-s.fr> writes:
> Le 03/03/2020 à 13:59, Michael Ellerman a écrit :
>> We received a report of strange kernel faults which turned out to be
>> due to a missing KUAP disable in flush_coherent_icache() called
>> from flush_icache_range().
>>
>> The fault looks like:
>>
>> Kernel attempted to access user page (7fffc30d9c00) - exploit attempt? (uid: 1009)
>> BUG: Unable to handle kernel data access on read at 0x7fffc30d9c00
>> Faulting instruction address: 0xc00000000007232c
>> Oops: Kernel access of bad area, sig: 11 [#1]
>> LE PAGE_SIZE=64K MMU=Radix SMP NR_CPUS=2048 NUMA PowerNV
>> CPU: 35 PID: 5886 Comm: sigtramp Not tainted 5.6.0-rc2-gcc-8.2.0-00003-gfc37a1632d40 #79
>> NIP: c00000000007232c LR: c00000000003b7fc CTR: 0000000000000000
>> REGS: c000001e11093940 TRAP: 0300 Not tainted (5.6.0-rc2-gcc-8.2.0-00003-gfc37a1632d40)
>> MSR: 900000000280b033 <SF,HV,VEC,VSX,EE,FP,ME,IR,DR,RI,LE> CR: 28000884 XER: 00000000
>> CFAR: c0000000000722fc DAR: 00007fffc30d9c00 DSISR: 08000000 IRQMASK: 0
>> GPR00: c00000000003b7fc c000001e11093bd0 c0000000023ac200 00007fffc30d9c00
>> GPR04: 00007fffc30d9c18 0000000000000000 c000001e11093bd4 0000000000000000
>> GPR08: 0000000000000000 0000000000000001 0000000000000000 c000001e1104ed80
>> GPR12: 0000000000000000 c000001fff6ab380 c0000000016be2d0 4000000000000000
>> GPR16: c000000000000000 bfffffffffffffff 0000000000000000 0000000000000000
>> GPR20: 00007fffc30d9c00 00007fffc30d8f58 00007fffc30d9c18 00007fffc30d9c20
>> GPR24: 00007fffc30d9c18 0000000000000000 c000001e11093d90 c000001e1104ed80
>> GPR28: c000001e11093e90 0000000000000000 c0000000023d9d18 00007fffc30d9c00
>> NIP flush_icache_range+0x5c/0x80
>> LR handle_rt_signal64+0x95c/0xc2c
>> Call Trace:
>> 0xc000001e11093d90 (unreliable)
>> handle_rt_signal64+0x93c/0xc2c
>> do_notify_resume+0x310/0x430
>> ret_from_except_lite+0x70/0x74
>> Instruction dump:
>> 409e002c 7c0802a6 3c62ff31 3863f6a0 f8010080 48195fed 60000000 48fe4c8d
>> 60000000 e8010080 7c0803a6 7c0004ac <7c00ffac> 7c0004ac 4c00012c 38210070
>>
>> This path through handle_rt_signal64() to setup_trampoline() and
>> flush_icache_range() is only triggered by 64-bit processes that have
>> unmapped their VDSO, which is rare.
>>
>> flush_icache_range() takes a range of addresses to flush. In
>> flush_coherent_icache() we implement an optimisation for CPUs where we
>> know we don't actually have to flush the whole range, we just need to
>> do a single icbi.
>>
>> However we still execute the icbi on the user address of the start of
>> the range we're flushing. On CPUs that also implement KUAP (Power9)
>> that leads to the spurious fault above.
>>
>> We should be able to pass any address, including a kernel address, to
>> the icbi on these CPUs, which would avoid any interaction with KUAP.
>> But I don't want to make that change in a bug fix, just in case it
>> surfaces some strange behaviour on some CPU.
>>
>> So for now just disable KUAP around the icbi. Note the icbi is treated
>> as a load, so we allow read access, not write as you'd expect.
>>
>> Fixes: 890274c2dc4c ("powerpc/64s: Implement KUAP for Radix MMU")
>> Cc: stable@vger.kernel.org # v5.2+
>> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
>> ---
>> arch/powerpc/mm/mem.c | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
>> index ef7b1119b2e2..184850d9d000 100644
>> --- a/arch/powerpc/mm/mem.c
>> +++ b/arch/powerpc/mm/mem.c
>> @@ -373,7 +373,9 @@ static inline bool flush_coherent_icache(unsigned long addr)
>> */
>> if (cpu_has_feature(CPU_FTR_COHERENT_ICACHE)) {
>> mb(); /* sync */
>> + allow_read_from_user((void *)addr, 4);
>
> I know that's ignored on Radix, but shouldn't we use L1_CACHE_BYTES as a
> length ?
Yes you're right, I actually meant to do that but forgot.
Will send a v2.
cheers
^ permalink raw reply
* Re: [RFC PATCH v1] powerpc/prom_init: disable XIVE in Secure VM.
From: Greg Kurz @ 2020-03-03 17:45 UTC (permalink / raw)
To: Ram Pai
Cc: aik, andmike, kvm-ppc, Cédric Le Goater, sukadev,
linuxppc-dev, bauerman, David Gibson
In-Reply-To: <20200303170205.GA5416@oc0525413822.ibm.com>
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 ?
> 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.
> > and the remaining problem with XIVE is the KVM page fault handler
> > populating the TIMA and ESB pages. Ultravisor doesn't seem to support
> > this feature and this breaks interrupt management in the guest.
>
> Yes. This is the bigger issue that needs to be fixed. When the secure guest
> accesses the page associated with the xive memslot, a page fault is
> generated, which the ultravisor reflects to the hypervisor. Hypervisor
> seems to be mapping Hardware-page to that GPA. Unforatunately it is not
> informing the ultravisor of that map. I am trying to understand the
> root cause. But since I am not sure what more issues I might run into
> after chasing down that issue, I figured its better to disable xive
> support in SVM in the interim.
>
> **** 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".
>
> >
> > But, kernel_irqchip=off should work out of the box. It seems it doesn't.
> > Something to investigate.
>
> Dont know why.
>
> Does this option, disable the chip from interrupting the
> guest directly; instead mediates the interrupt through the hypervisor?
>
> >
> > >
> > > 5) All the above said, having to use XICS is pretty crappy. You
> > > should really get working on XIVE support for secure VMs.
> >
> > Yes.
>
> and yes too.
>
>
> Summary: I am dropping this patch for now.
>
> >
> > Thanks,
> >
> > C.
>
^ permalink raw reply
* Re: [PATCH v3 02/14] powerpc: Define new SRR1 bits for a future ISA version
From: Alistair Popple @ 2020-03-03 23:31 UTC (permalink / raw)
To: Jordan Niethe; +Cc: dja, linuxppc-dev, bala24
In-Reply-To: <20200226040716.32395-3-jniethe5@gmail.com>
On Wednesday, 26 February 2020 3:07:04 PM AEDT Jordan Niethe wrote:
> Add the BOUNDARY SRR1 bit definition for when the cause of an alignment
> exception is a prefixed instruction that crosses a 64-byte boundary.
> Add the PREFIXED SRR1 bit definition for exceptions caused by prefixed
> instructions.
>
> Bit 35 of SRR1 is called SRR1_ISI_N_OR_G. This name comes from it being
> used to indicate that an ISI was due to the access being no-exec or
> guarded. A future ISA version adds another purpose. It is also set if
> there is an access in a cache-inhibited location for prefixed
> instruction. Rename from SRR1_ISI_N_OR_G to SRR1_ISI_N_G_OR_CIP.
>
> Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
Confirmed the definitions here match the specifications so:
Reviewed-by: Alistair Popple <alistair@popple.id.au>
> ---
> v2: Combined all the commits concerning SRR1 bits.
> ---
> arch/powerpc/include/asm/reg.h | 4 +++-
> arch/powerpc/kvm/book3s_hv_nested.c | 2 +-
> arch/powerpc/kvm/book3s_hv_rm_mmu.c | 2 +-
> 3 files changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
> index c7758c2ccc5f..173f33df4fab 100644
> --- a/arch/powerpc/include/asm/reg.h
> +++ b/arch/powerpc/include/asm/reg.h
> @@ -762,7 +762,7 @@
> #endif
>
> #define SRR1_ISI_NOPT 0x40000000 /* ISI: Not found in hash */
> -#define SRR1_ISI_N_OR_G 0x10000000 /* ISI: Access is no-exec or G */
> +#define SRR1_ISI_N_G_OR_CIP 0x10000000 /* ISI: Access is no-exec or G or
> CI for a prefixed instruction */ #define SRR1_ISI_PROT 0x08000000 /*
> ISI: Other protection fault */ #define SRR1_WAKEMASK 0x00380000 /*
> reason for wakeup */
> #define SRR1_WAKEMASK_P8 0x003c0000 /* reason for wakeup on POWER8 and 9
> */ @@ -789,6 +789,8 @@
> #define SRR1_PROGADDR 0x00010000 /* SRR0 contains subsequent addr */
>
> #define SRR1_MCE_MCP 0x00080000 /* Machine check signal caused
interrupt
> */ +#define SRR1_BOUNDARY 0x10000000 /* Prefixed instruction crosses
> 64-byte boundary */ +#define SRR1_PREFIXED 0x20000000 /* Exception
> caused by prefixed instruction */
>
> #define SPRN_HSRR0 0x13A /* Save/Restore Register 0 */
> #define SPRN_HSRR1 0x13B /* Save/Restore Register 1 */
> diff --git a/arch/powerpc/kvm/book3s_hv_nested.c
> b/arch/powerpc/kvm/book3s_hv_nested.c index dc97e5be76f6..6ab685227574
> 100644
> --- a/arch/powerpc/kvm/book3s_hv_nested.c
> +++ b/arch/powerpc/kvm/book3s_hv_nested.c
> @@ -1169,7 +1169,7 @@ static int kvmhv_translate_addr_nested(struct kvm_vcpu
> *vcpu, } else if (vcpu->arch.trap == BOOK3S_INTERRUPT_H_INST_STORAGE) { /*
> Can we execute? */
> if (!gpte_p->may_execute) {
> - flags |= SRR1_ISI_N_OR_G;
> + flags |= SRR1_ISI_N_G_OR_CIP;
> goto forward_to_l1;
> }
> } else {
> diff --git a/arch/powerpc/kvm/book3s_hv_rm_mmu.c
> b/arch/powerpc/kvm/book3s_hv_rm_mmu.c index 220305454c23..b53a9f1c1a46
> 100644
> --- a/arch/powerpc/kvm/book3s_hv_rm_mmu.c
> +++ b/arch/powerpc/kvm/book3s_hv_rm_mmu.c
> @@ -1260,7 +1260,7 @@ long kvmppc_hpte_hv_fault(struct kvm_vcpu *vcpu,
> unsigned long addr, status &= ~DSISR_NOHPTE; /* DSISR_NOHPTE ==
> SRR1_ISI_NOPT */
> if (!data) {
> if (gr & (HPTE_R_N | HPTE_R_G))
> - return status | SRR1_ISI_N_OR_G;
> + return status | SRR1_ISI_N_G_OR_CIP;
> if (!hpte_read_permission(pp, slb_v & key))
> return status | SRR1_ISI_PROT;
> } else if (status & DSISR_ISSTORE) {
^ permalink raw reply
* Re: [PATCH v3 01/14] powerpc: Enable Prefixed Instructions
From: Alistair Popple @ 2020-03-03 23:20 UTC (permalink / raw)
To: Nicholas Piggin; +Cc: Jordan Niethe, Daniel Axtens, linuxppc-dev, Balamuruhan S
In-Reply-To: <1582864874.q013n2u3v1.astroid@bobo.none>
> But hmm, the dt_cpu_ftrs parsing should be picking those up and setting
> them by default I would think (or maybe not because you don't expect
> FSCR bit if OS support is not required).
Right - the generic dt_cpu_ftrs didn't do the FSCR enablement which I assume
is because if OS support is required anyway we can just add it in a similar
way to the below patch. My thinking was that we could use it to disable prefix
with a firmware if required, however outside of a lab environment there isn't
much practical use for that so I'm ok with just doing something similar to the
below.
> All the other FSCR bits are done similarly to this AFAIKS:
>
> https://patchwork.ozlabs.org/patch/1244476/
>
> I would do that for now rather than digging into it too far, but we
> should look at cleaning that up and doing something more like what
> you have here.
>
> >> > struct dt_cpu_feature_match {
> >> >
> >> > const char *name;
> >> > int (*enable)(struct dt_cpu_feature *f);
> >> >
> >> > @@ -626,6 +648,7 @@ static struct dt_cpu_feature_match __initdata
> >> >
> >> > {"vector-binary128", feat_enable, 0},
> >> > {"vector-binary16", feat_enable, 0},
> >> > {"wait-v3", feat_enable, 0},
> >> >
> >> > + {"prefix-instructions", feat_enable_prefix, 0},
> >>
> >> That's reasonable to make that a feature, will it specify a minimum
> >> base set of prefix instructions or just that prefix instructions
> >> with the prefix/suffix arrangement exist?
> >
> > This was just going to be that they exist.
> >
> >> You may not need "-instructions" on the end, none of the other
> >> instructions do.
> >
> > Good point.
> >
> >> I would maybe just hold off upstreaming the dt_cpu_ftrs changes for
> >> a bit. We have to do a pass over new CPU feature device tree, and
> >> some compatibility questions have come up recently.
> >>
> >> If you wouldn't mind just adding the new [H]FSCR bits and faults
> >> upstream for now, that would be good.
> >
> > No problem.
>
> Thanks,
> Nick
^ permalink raw reply
* Re: [PATCH] powerpc/64: BE option to use ELFv2 ABI for big endian kernels
From: Segher Boessenkool @ 2020-03-03 23:09 UTC (permalink / raw)
To: Nicholas Piggin; +Cc: linuxppc-dev
In-Reply-To: <20200303014527.39377-1-npiggin@gmail.com>
Hi!
On Tue, Mar 03, 2020 at 11:45:27AM +1000, Nicholas Piggin wrote:
> Provide an option to use ELFv2 ABI for big endian builds. This works on
> GCC and clang (since 2014). it is is not officially supported by the GNU
> toolchain, but it can give some useful advantages of the ELFv2 ABI for
> BE (e.g., less stack usage). Some distros build BE ELFv2 userspace.
It is not officially supported in the sense that a) as a host config,
it does not exist *at all* (this isn't relevant for the kernel, it does
not use a libc or other libraries, of course); and b) as a target config,
it is not supported in the sense that no one tests it, so we cannot say
anything about what quality code it generates, if it works at all, etc.
But we *do* allow "-mbig -mabi=elfv2", it's just a chicken-and-egg
problem to have this properly tested. If someone would regularly test
it (incl. sending the test results to gcc-testresults@), I don't see why
it would not become a supported platform.
> +override flavour := linux-ppc64v2
That isn't a good name, heh. This isn't "v2" of anything... Spell out
the name "ELFv2"? Or as "elfv2"? It is just a name after all, it is
version 1 in all three version fields in the ELF headers!
Anyway, looks like it will work, let's see where this goes :-)
Reviewed-by: Segher Boessenkool <segher@kernel.crashing.org>
Segher
^ permalink raw reply
* RE: [RFC PATCH v1] powerpc/prom_init: disable XIVE in Secure VM.
From: Ram Pai @ 2020-03-03 20:29 UTC (permalink / raw)
To: Cédric Le Goater
Cc: aik, andmike, groug, kvm-ppc, sukadev, linuxppc-dev, bauerman,
David Gibson
In-Reply-To: <6f7ea308-3505-6070-dde1-20fee8fdddc3@kaod.org>
On Tue, Mar 03, 2020 at 08:08:51PM +0100, Cédric Le Goater wrote:
> >>> 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. However the patch by itself does not solve the xive problem
> > for secure VM.
>
> yes because you also need to share the XIVE TIMA and ESB pages mapped
> in xive_native_esb_fault() and xive_native_tima_fault().
These pages belong to the xive memory slot right? If that is the case,
they are implicitly shared. The Ultravisor will set them up to be
shared. The guest kernel should not be doing anything.
We still need some fixes in KVM and Ultravisor to correctly map the
hardware pages to GPA ranges of the xive memory slot. Work is in progress...
>
> >> and the remaining problem with XIVE is the KVM page fault handler
> >> populating the TIMA and ESB pages. Ultravisor doesn't seem to support
> >> this feature and this breaks interrupt management in the guest.
> >
> > Yes. This is the bigger issue that needs to be fixed. When the secure guest
> > accesses the page associated with the xive memslot, a page fault is
> > generated, which the ultravisor reflects to the hypervisor. Hypervisor
> > seems to be mapping Hardware-page to that GPA. Unforatunately it is not
> > informing the ultravisor of that map. I am trying to understand the
> > root cause. But since I am not sure what more issues I might run into
> > after chasing down that issue, I figured its better to disable xive
> > support in SVM in the interim.
>
> Is it possible to call uv_share_page() from the hypervisor ?
No. Not allowed. If allowed hypervisor can easily attack the SVM.
>
> > **** 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. *****
>
> Yes.
>
> >> But, kernel_irqchip=off should work out of the box. It seems it doesn't.
> >> Something to investigate.
> >
> > Dont know why.
>
> We need to understand why.
>
> You still need the patch to share the event queue page allocated by the
> guest OS because QEMU will enqueue events. But you should not need anything
> else.
ok. that is assuring.
>
> > Does this option, disable the chip from interrupting the
> > guest directly; instead mediates the interrupt through the hypervisor?
>
> Yes. The KVM backend is unused, the XIVE interrupt controller is deactivated
> for the guest and QEMU notifies the vCPUs directly.
>
> The TIMA and ESB pages belong the QEMU process and the guest OS will do
> some load and store operations onto them for interrupt management. Is that
> OK from a UV perspective ?
Yes. These GPA ranges are needed; by design, to be read/writable from qemu/KVM and
the SVM. Just that the implementation in its current form, needs some
fixing.
RP
^ permalink raw reply
* Re: [PATCH] macintosh: windfarm: fix MODINFO regression
From: Wolfram Sang @ 2020-03-03 19:54 UTC (permalink / raw)
To: Andreas Schwab
Cc: Erhard Furtner, Mathieu Malaterre, debian-powerpc, linux-i2c,
John Paul Adrian Glaubitz, linuxppc-dev
In-Reply-To: <87d09tw9is.fsf@igel.home>
[-- Attachment #1: Type: text/plain, Size: 163 bytes --]
> > sound/aoa/codecs/onyx.c
> > sound/aoa/codecs/tas.c
>
> These are loaded explicitly via request_module (as snd-aoa-codec-%s).
Good to know, thanks!
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [EXTERNAL] Re: [RFC PATCH v1] powerpc/prom_init: disable XIVE in Secure VM.
From: Cédric Le Goater @ 2020-03-03 19:18 UTC (permalink / raw)
To: Greg Kurz, Ram Pai
Cc: aik, andmike, kvm-ppc, sukadev, linuxppc-dev, bauerman,
David Gibson
In-Reply-To: <20200303184520.632be270@bahia.home>
>> **** 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".
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.
C.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox