* Re: [PATCH 24/29] mm: remove __vmalloc_node_flags_caller
From: Geert Uytterhoeven @ 2020-04-20 9:27 UTC (permalink / raw)
To: Christoph Hellwig
Cc: linux-hyperv, David Airlie, DRI Development, Linux MM,
K. Y. Srinivasan, Sumit Semwal, Linux-Arch, linux-s390, Wei Liu,
Stephen Hemminger, the arch/x86 maintainers, Peter Zijlstra,
Laura Abbott, Nitin Gupta, Daniel Vetter, Haiyang Zhang,
linaro-mm-sig, bpf, Linux ARM, Robin Murphy,
Linux Kernel Mailing List, Minchan Kim, Linux IOMMU, Sakari Ailus,
Andrew Morton, linuxppc-dev
In-Reply-To: <20200414131348.444715-25-hch@lst.de>
Hi Christoph,
On Tue, Apr 14, 2020 at 3:21 PM Christoph Hellwig <hch@lst.de> wrote:
> Just use __vmalloc_node instead which gets and extra argument. To be
> able to to use __vmalloc_node in all caller make it available outside
> of vmalloc and implement it in nommu.c.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
One more nommu failure below...
> --- a/mm/nommu.c
> +++ b/mm/nommu.c
> @@ -150,8 +150,8 @@ void *__vmalloc(unsigned long size, gfp_t gfp_mask)
> }
> EXPORT_SYMBOL(__vmalloc);
>
> -void *__vmalloc_node_flags_caller(unsigned long size, int node, gfp_t flags,
> - void *caller)
> +void *__vmalloc_node(unsigned long size, unsigned long align, gfp_t gfp_mask,
> + int node, const void *caller)
> {
> return __vmalloc(size, flags);
On Mon, Apr 20, 2020 at 10:39 AM <noreply@ellerman.id.au> wrote:
> FAILED linux-next/m5272c3_defconfig/m68k-gcc8 Mon Apr 20, 18:38
>
> http://kisskb.ellerman.id.au/kisskb/buildresult/14213623/
>
> mm/nommu.c:164:25: error: 'flags' undeclared (first use in this function); did you mean 'class'?
"return __vmalloc(size, gfp_mask);"
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH 26/29] mm: remove vmalloc_user_node_flags
From: Geert Uytterhoeven @ 2020-04-20 9:26 UTC (permalink / raw)
To: Christoph Hellwig
Cc: linux-hyperv, David Airlie, DRI Development, Linux MM,
K. Y. Srinivasan, Sumit Semwal, Linux-Arch, linux-s390, Wei Liu,
Stephen Hemminger, the arch/x86 maintainers, Peter Zijlstra,
Laura Abbott, Nitin Gupta, Daniel Vetter, Haiyang Zhang,
linaro-mm-sig, bpf, Linux ARM, Robin Murphy,
Linux Kernel Mailing List, Minchan Kim, Linux IOMMU, Sakari Ailus,
Johannes Weiner, Andrew Morton, linuxppc-dev
In-Reply-To: <20200414131348.444715-27-hch@lst.de>
Hi Christoph,
On Tue, Apr 14, 2020 at 3:22 PM Christoph Hellwig <hch@lst.de> wrote:
> Open code it in __bpf_map_area_alloc, which is the only caller. Also
> clean up __bpf_map_area_alloc to have a single vmalloc call with
> slightly different flags instead of the current two different calls.
>
> For this to compile for the nommu case add a __vmalloc_node_range stub
> to nommu.c.
Apparently your nommu-cross-compilers are in quarantaine? ;-)
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> Acked-by: Johannes Weiner <hannes@cmpxchg.org>
> --- a/mm/nommu.c
> +++ b/mm/nommu.c
> @@ -150,6 +150,14 @@ void *__vmalloc(unsigned long size, gfp_t gfp_mask)
> }
> EXPORT_SYMBOL(__vmalloc);
>
> +void *__vmalloc_node_range(unsigned long size, unsigned long align,
> + unsigned long start, unsigned long end, gfp_t gfp_mask,
> + pgprot_t prot, unsigned long vm_flags, int node,
> + const void *caller)
> +{
> + return __vmalloc(size, flags);
On Mon, Apr 20, 2020 at 10:39 AM <noreply@ellerman.id.au> wrote:
> FAILED linux-next/m5272c3_defconfig/m68k-gcc8 Mon Apr 20, 18:38
>
> http://kisskb.ellerman.id.au/kisskb/buildresult/14213623/
>
> mm/nommu.c:158:25: error: 'flags' undeclared (first use in this function); did you mean 'class'?
"return __vmalloc(size, gfp_mask);", I assume?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* [PATCH v3 2/2] powerpc/fadump: consider reserved ranges while reserving memory
From: Hari Bathini @ 2020-04-20 8:56 UTC (permalink / raw)
To: Michael Ellerman, linuxppc-dev
Cc: stable, Vasant Hegde, Sourabh Jain, Mahesh J Salgaonkar
In-Reply-To: <158737294432.26700.4830263187856221314.stgit@hbathini.in.ibm.com>
Commit 0962e8004e97 ("powerpc/prom: Scan reserved-ranges node for
memory reservations") enabled support to parse reserved-ranges DT
node and reserve kernel memory falling in these ranges for F/W
purposes. Memory reserved for FADump should not overlap with these
ranges as it could corrupt memory meant for F/W or crash'ed kernel
memory to be exported as vmcore.
But since commit 579ca1a27675 ("powerpc/fadump: make use of memblock's
bottom up allocation mode"), memblock_find_in_range() is being used to
find the appropriate area to reserve memory for FADump, which can't
account for reserved-ranges as these ranges are reserved only after
FADump memory reservation.
With reserved-ranges now being populated during early boot, look out
for these memory ranges while reserving memory for FADump. Without
this change, MPIPL on PowerNV systems aborts with hostboot failure,
when memory reserved for FADump is less than 4096MB.
Fixes: 579ca1a27675 ("powerpc/fadump: make use of memblock's bottom up allocation mode")
Cc: stable@vger.kernel.org
Signed-off-by: Hari Bathini <hbathini@linux.ibm.com>
Reviewed-by: Mahesh Salgaonkar <mahesh@linux.ibm.com>
---
Changes in v3:
* Updated fadump_locate_reserve_mem() to use return '0' instead of an out parameter
as suggested by Mahesh and added his 'Reviewed-by' tag with that change.
arch/powerpc/kernel/fadump.c | 76 +++++++++++++++++++++++++++++++++++++-----
1 file changed, 67 insertions(+), 9 deletions(-)
diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c
index 679277b..63aac8b 100644
--- a/arch/powerpc/kernel/fadump.c
+++ b/arch/powerpc/kernel/fadump.c
@@ -445,10 +445,72 @@ static int __init fadump_get_boot_mem_regions(void)
return ret;
}
+/*
+ * Returns true, if the given range overlaps with reserved memory ranges
+ * starting at idx. Also, updates idx to index of overlapping memory range
+ * with the given memory range.
+ * False, otherwise.
+ */
+static bool overlaps_reserved_ranges(u64 base, u64 end, int *idx)
+{
+ bool ret = false;
+ int i;
+
+ for (i = *idx; i < reserved_mrange_info.mem_range_cnt; i++) {
+ u64 rbase = reserved_mrange_info.mem_ranges[i].base;
+ u64 rend = rbase + reserved_mrange_info.mem_ranges[i].size;
+
+ if (end <= rbase)
+ break;
+
+ if ((end > rbase) && (base < rend)) {
+ *idx = i;
+ ret = true;
+ break;
+ }
+ }
+
+ return ret;
+}
+
+/*
+ * Locate a suitable memory area to reserve memory for FADump. While at it,
+ * lookup reserved-ranges & avoid overlap with them, as they are used by F/W.
+ */
+static u64 __init fadump_locate_reserve_mem(u64 base, u64 size)
+{
+ struct fadump_memory_range *mrngs;
+ phys_addr_t mstart, mend;
+ int idx = 0;
+ u64 i, ret = 0;
+
+ mrngs = reserved_mrange_info.mem_ranges;
+ for_each_free_mem_range(i, NUMA_NO_NODE, MEMBLOCK_NONE,
+ &mstart, &mend, NULL) {
+ pr_debug("%llu) mstart: %llx, mend: %llx, base: %llx\n",
+ i, mstart, mend, base);
+
+ if (mstart > base)
+ base = PAGE_ALIGN(mstart);
+
+ while ((mend > base) && ((mend - base) >= size)) {
+ if (!overlaps_reserved_ranges(base, base+size, &idx)) {
+ ret = base;
+ goto out;
+ }
+
+ base = mrngs[idx].base + mrngs[idx].size;
+ base = PAGE_ALIGN(base);
+ }
+ }
+
+out:
+ return ret;
+}
+
int __init fadump_reserve_mem(void)
{
- u64 base, size, mem_boundary, bootmem_min, align = PAGE_SIZE;
- bool is_memblock_bottom_up = memblock_bottom_up();
+ u64 base, size, mem_boundary, bootmem_min;
int ret = 1;
if (!fw_dump.fadump_enabled)
@@ -469,9 +531,9 @@ int __init fadump_reserve_mem(void)
PAGE_ALIGN(fadump_calculate_reserve_size());
#ifdef CONFIG_CMA
if (!fw_dump.nocma) {
- align = FADUMP_CMA_ALIGNMENT;
fw_dump.boot_memory_size =
- ALIGN(fw_dump.boot_memory_size, align);
+ ALIGN(fw_dump.boot_memory_size,
+ FADUMP_CMA_ALIGNMENT);
}
#endif
@@ -539,11 +601,7 @@ int __init fadump_reserve_mem(void)
* Reserve memory at an offset closer to bottom of the RAM to
* minimize the impact of memory hot-remove operation.
*/
- memblock_set_bottom_up(true);
- base = memblock_find_in_range(base, mem_boundary, size, align);
-
- /* Restore the previous allocation mode */
- memblock_set_bottom_up(is_memblock_bottom_up);
+ base = fadump_locate_reserve_mem(base, size);
if (!base) {
pr_err("Failed to find memory chunk for reservation!\n");
^ permalink raw reply related
* [PATCH v3 1/2] powerpc/fadump: use static allocation for reserved memory ranges
From: Hari Bathini @ 2020-04-20 8:56 UTC (permalink / raw)
To: Michael Ellerman, linuxppc-dev
Cc: stable, Vasant Hegde, Sourabh Jain, Mahesh J Salgaonkar
At times, memory ranges have to be looked up during early boot, when
kernel couldn't be initialized for dynamic memory allocation. In fact,
reserved-ranges look up is needed during FADump memory reservation.
Without accounting for reserved-ranges in reserving memory for FADump,
MPIPL boot fails with memory corruption issues. So, extend memory
ranges handling to support static allocation and populate reserved
memory ranges during early boot.
Fixes: dda9dbfeeb7a ("powerpc/fadump: consider reserved ranges while releasing memory")
Cc: stable@vger.kernel.org
Signed-off-by: Hari Bathini <hbathini@linux.ibm.com>
Reviewed-by: Mahesh Salgaonkar <mahesh@linux.ibm.com>
---
Changes in v3:
* No code change. Added Mahesh's 'Reviewed-by' tag.
arch/powerpc/include/asm/fadump-internal.h | 4 +
arch/powerpc/kernel/fadump.c | 77 ++++++++++++++++------------
2 files changed, 48 insertions(+), 33 deletions(-)
diff --git a/arch/powerpc/include/asm/fadump-internal.h b/arch/powerpc/include/asm/fadump-internal.h
index c814a2b..8d61c8f 100644
--- a/arch/powerpc/include/asm/fadump-internal.h
+++ b/arch/powerpc/include/asm/fadump-internal.h
@@ -64,12 +64,14 @@ struct fadump_memory_range {
};
/* fadump memory ranges info */
+#define RNG_NAME_SZ 16
struct fadump_mrange_info {
- char name[16];
+ char name[RNG_NAME_SZ];
struct fadump_memory_range *mem_ranges;
u32 mem_ranges_sz;
u32 mem_range_cnt;
u32 max_mem_ranges;
+ bool is_static;
};
/* Platform specific callback functions */
diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c
index 59e60a9..679277b 100644
--- a/arch/powerpc/kernel/fadump.c
+++ b/arch/powerpc/kernel/fadump.c
@@ -40,8 +40,17 @@ struct kobject *fadump_kobj;
#ifndef CONFIG_PRESERVE_FA_DUMP
static DEFINE_MUTEX(fadump_mutex);
-struct fadump_mrange_info crash_mrange_info = { "crash", NULL, 0, 0, 0 };
-struct fadump_mrange_info reserved_mrange_info = { "reserved", NULL, 0, 0, 0 };
+struct fadump_mrange_info crash_mrange_info = { "crash", NULL, 0, 0, 0, false };
+
+#define RESERVED_RNGS_SZ 16384 /* 16K - 128 entries */
+#define RESERVED_RNGS_CNT (RESERVED_RNGS_SZ / \
+ sizeof(struct fadump_memory_range))
+static struct fadump_memory_range rngs[RESERVED_RNGS_CNT];
+struct fadump_mrange_info reserved_mrange_info = { "reserved", rngs,
+ RESERVED_RNGS_SZ, 0,
+ RESERVED_RNGS_CNT, true };
+
+static void __init early_init_dt_scan_reserved_ranges(unsigned long node);
#ifdef CONFIG_CMA
static struct cma *fadump_cma;
@@ -110,6 +119,11 @@ static int __init fadump_cma_init(void) { return 1; }
int __init early_init_dt_scan_fw_dump(unsigned long node, const char *uname,
int depth, void *data)
{
+ if (depth == 0) {
+ early_init_dt_scan_reserved_ranges(node);
+ return 0;
+ }
+
if (depth != 1)
return 0;
@@ -728,10 +742,14 @@ void fadump_free_cpu_notes_buf(void)
static void fadump_free_mem_ranges(struct fadump_mrange_info *mrange_info)
{
+ if (mrange_info->is_static) {
+ mrange_info->mem_range_cnt = 0;
+ return;
+ }
+
kfree(mrange_info->mem_ranges);
- mrange_info->mem_ranges = NULL;
- mrange_info->mem_ranges_sz = 0;
- mrange_info->max_mem_ranges = 0;
+ memset((void *)((u64)mrange_info + RNG_NAME_SZ), 0,
+ (sizeof(struct fadump_mrange_info) - RNG_NAME_SZ));
}
/*
@@ -788,6 +806,12 @@ static inline int fadump_add_mem_range(struct fadump_mrange_info *mrange_info,
if (mrange_info->mem_range_cnt == mrange_info->max_mem_ranges) {
int ret;
+ if (mrange_info->is_static) {
+ pr_err("Reached array size limit for %s memory ranges\n",
+ mrange_info->name);
+ return -ENOSPC;
+ }
+
ret = fadump_alloc_mem_ranges(mrange_info);
if (ret)
return ret;
@@ -1204,20 +1228,19 @@ static void sort_and_merge_mem_ranges(struct fadump_mrange_info *mrange_info)
* Scan reserved-ranges to consider them while reserving/releasing
* memory for FADump.
*/
-static inline int fadump_scan_reserved_mem_ranges(void)
+static void __init early_init_dt_scan_reserved_ranges(unsigned long node)
{
- struct device_node *root;
const __be32 *prop;
int len, ret = -1;
unsigned long i;
- root = of_find_node_by_path("/");
- if (!root)
- return ret;
+ /* reserved-ranges already scanned */
+ if (reserved_mrange_info.mem_range_cnt != 0)
+ return;
- prop = of_get_property(root, "reserved-ranges", &len);
+ prop = of_get_flat_dt_prop(node, "reserved-ranges", &len);
if (!prop)
- return ret;
+ return;
/*
* Each reserved range is an (address,size) pair, 2 cells each,
@@ -1239,7 +1262,8 @@ static inline int fadump_scan_reserved_mem_ranges(void)
}
}
- return ret;
+ /* Compact reserved ranges */
+ sort_and_merge_mem_ranges(&reserved_mrange_info);
}
/*
@@ -1253,32 +1277,21 @@ static void fadump_release_memory(u64 begin, u64 end)
u64 ra_start, ra_end, tstart;
int i, ret;
- fadump_scan_reserved_mem_ranges();
-
ra_start = fw_dump.reserve_dump_area_start;
ra_end = ra_start + fw_dump.reserve_dump_area_size;
/*
- * Add reserved dump area to reserved ranges list
- * and exclude all these ranges while releasing memory.
+ * If reserved ranges array limit is hit, overwrite the last reserved
+ * memory range with reserved dump area to ensure it is excluded from
+ * the memory being released (reused for next FADump registration).
*/
- ret = fadump_add_mem_range(&reserved_mrange_info, ra_start, ra_end);
- if (ret != 0) {
- /*
- * Not enough memory to setup reserved ranges but the system is
- * running shortage of memory. So, release all the memory except
- * Reserved dump area (reused for next fadump registration).
- */
- if (begin < ra_end && end > ra_start) {
- if (begin < ra_start)
- fadump_release_reserved_area(begin, ra_start);
- if (end > ra_end)
- fadump_release_reserved_area(ra_end, end);
- } else
- fadump_release_reserved_area(begin, end);
+ if (reserved_mrange_info.mem_range_cnt ==
+ reserved_mrange_info.max_mem_ranges)
+ reserved_mrange_info.mem_range_cnt--;
+ ret = fadump_add_mem_range(&reserved_mrange_info, ra_start, ra_end);
+ if (ret != 0)
return;
- }
/* Get the reserved ranges list in order first. */
sort_and_merge_mem_ranges(&reserved_mrange_info);
^ permalink raw reply related
* [PATCH v2 2/2] powerpc/fadump: consider reserved ranges while reserving memory
From: Hari Bathini @ 2020-04-20 8:04 UTC (permalink / raw)
To: Michael Ellerman, linuxppc-dev
Cc: stable, Vasant Hegde, Sourabh Jain, Mahesh J Salgaonkar
In-Reply-To: <158736979865.22985.14884734774613813693.stgit@hbathini.in.ibm.com>
Commit 0962e8004e97 ("powerpc/prom: Scan reserved-ranges node for
memory reservations") enabled support to parse reserved-ranges DT
node and reserve kernel memory falling in these ranges for F/W
purposes. Memory reserved for FADump should not overlap with these
ranges as it could corrupt memory meant for F/W or crash'ed kernel
memory to be exported as vmcore.
But since commit 579ca1a27675 ("powerpc/fadump: make use of memblock's
bottom up allocation mode"), memblock_find_in_range() is being used to
find the appropriate area to reserve memory for FADump, which can't
account for reserved-ranges as these ranges are reserved only after
FADump memory reservation.
With reserved-ranges now being populated during early boot, look out
for these memory ranges while reserving memory for FADump. Without
this change, MPIPL on PowerNV systems aborts with hostboot failure,
when memory reserved for FADump is less than 4096MB.
Fixes: 579ca1a27675 ("powerpc/fadump: make use of memblock's bottom up allocation mode")
Cc: stable@vger.kernel.org
Signed-off-by: Hari Bathini <hbathini@linux.ibm.com>
---
Changes in v2:
* Add an out parameter 'found' for fadump_locate_reserve_mem() and set it to "true"
when a suitable memory area is located.
arch/powerpc/kernel/fadump.c | 81 +++++++++++++++++++++++++++++++++++++-----
1 file changed, 71 insertions(+), 10 deletions(-)
diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c
index 679277b..0ffe69c 100644
--- a/arch/powerpc/kernel/fadump.c
+++ b/arch/powerpc/kernel/fadump.c
@@ -445,10 +445,73 @@ static int __init fadump_get_boot_mem_regions(void)
return ret;
}
+/*
+ * Returns true, if the given range overlaps with reserved memory ranges
+ * starting at idx. Also, updates idx to index of overlapping memory range
+ * with the given memory range.
+ * False, otherwise.
+ */
+static bool overlaps_reserved_ranges(u64 base, u64 end, int *idx)
+{
+ bool ret = false;
+ int i;
+
+ for (i = *idx; i < reserved_mrange_info.mem_range_cnt; i++) {
+ u64 rbase = reserved_mrange_info.mem_ranges[i].base;
+ u64 rend = rbase + reserved_mrange_info.mem_ranges[i].size;
+
+ if (end <= rbase)
+ break;
+
+ if ((end > rbase) && (base < rend)) {
+ *idx = i;
+ ret = true;
+ break;
+ }
+ }
+
+ return ret;
+}
+
+/*
+ * Locate a suitable memory area to reserve memory for FADump. While at it,
+ * lookup reserved-ranges & avoid overlap with them, as they are used by F/W.
+ */
+static u64 __init fadump_locate_reserve_mem(u64 base, u64 size, bool *found)
+{
+ struct fadump_memory_range *mrngs;
+ phys_addr_t mstart, mend;
+ int idx = 0;
+ u64 i;
+
+ *found = false;
+ mrngs = reserved_mrange_info.mem_ranges;
+ for_each_free_mem_range(i, NUMA_NO_NODE, MEMBLOCK_NONE,
+ &mstart, &mend, NULL) {
+ pr_debug("%llu) mstart: %llx, mend: %llx, base: %llx\n",
+ i, mstart, mend, base);
+
+ if (mstart > base)
+ base = PAGE_ALIGN(mstart);
+
+ while ((mend > base) && ((mend - base) >= size)) {
+ if (!overlaps_reserved_ranges(base, base+size, &idx)) {
+ *found = true;
+ goto out;
+ }
+
+ base = mrngs[idx].base + mrngs[idx].size;
+ base = PAGE_ALIGN(base);
+ }
+ }
+
+out:
+ return base;
+}
+
int __init fadump_reserve_mem(void)
{
- u64 base, size, mem_boundary, bootmem_min, align = PAGE_SIZE;
- bool is_memblock_bottom_up = memblock_bottom_up();
+ u64 base, size, mem_boundary, bootmem_min;
int ret = 1;
if (!fw_dump.fadump_enabled)
@@ -469,9 +532,9 @@ int __init fadump_reserve_mem(void)
PAGE_ALIGN(fadump_calculate_reserve_size());
#ifdef CONFIG_CMA
if (!fw_dump.nocma) {
- align = FADUMP_CMA_ALIGNMENT;
fw_dump.boot_memory_size =
- ALIGN(fw_dump.boot_memory_size, align);
+ ALIGN(fw_dump.boot_memory_size,
+ FADUMP_CMA_ALIGNMENT);
}
#endif
@@ -535,17 +598,15 @@ int __init fadump_reserve_mem(void)
pr_debug("Reserve dump area start address: 0x%lx\n",
fw_dump.reserve_dump_area_start);
} else {
+ bool found = false;
+
/*
* Reserve memory at an offset closer to bottom of the RAM to
* minimize the impact of memory hot-remove operation.
*/
- memblock_set_bottom_up(true);
- base = memblock_find_in_range(base, mem_boundary, size, align);
-
- /* Restore the previous allocation mode */
- memblock_set_bottom_up(is_memblock_bottom_up);
+ base = fadump_locate_reserve_mem(base, size, &found);
- if (!base) {
+ if (!found || (base > (mem_boundary - size))) {
pr_err("Failed to find memory chunk for reservation!\n");
goto error_out;
}
^ permalink raw reply related
* [PATCH v2 1/2] powerpc/fadump: use static allocation for reserved memory ranges
From: Hari Bathini @ 2020-04-20 8:03 UTC (permalink / raw)
To: Michael Ellerman, linuxppc-dev
Cc: stable, Vasant Hegde, Sourabh Jain, Mahesh J Salgaonkar
At times, memory ranges have to be looked up during early boot, when
kernel couldn't be initialized for dynamic memory allocation. In fact,
reserved-ranges look up is needed during FADump memory reservation.
Without accounting for reserved-ranges in reserving memory for FADump,
MPIPL boot fails with memory corruption issues. So, extend memory
ranges handling to support static allocation and populate reserved
memory ranges during early boot.
Fixes: dda9dbfeeb7a ("powerpc/fadump: consider reserved ranges while releasing memory")
Cc: stable@vger.kernel.org
Signed-off-by: Hari Bathini <hbathini@linux.ibm.com>
Reviewed-by: Mahesh Salgaonkar <mahesh@linux.ibm.com>
---
arch/powerpc/include/asm/fadump-internal.h | 4 +
arch/powerpc/kernel/fadump.c | 77 ++++++++++++++++------------
2 files changed, 48 insertions(+), 33 deletions(-)
diff --git a/arch/powerpc/include/asm/fadump-internal.h b/arch/powerpc/include/asm/fadump-internal.h
index c814a2b..8d61c8f 100644
--- a/arch/powerpc/include/asm/fadump-internal.h
+++ b/arch/powerpc/include/asm/fadump-internal.h
@@ -64,12 +64,14 @@ struct fadump_memory_range {
};
/* fadump memory ranges info */
+#define RNG_NAME_SZ 16
struct fadump_mrange_info {
- char name[16];
+ char name[RNG_NAME_SZ];
struct fadump_memory_range *mem_ranges;
u32 mem_ranges_sz;
u32 mem_range_cnt;
u32 max_mem_ranges;
+ bool is_static;
};
/* Platform specific callback functions */
diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c
index 59e60a9..679277b 100644
--- a/arch/powerpc/kernel/fadump.c
+++ b/arch/powerpc/kernel/fadump.c
@@ -40,8 +40,17 @@ struct kobject *fadump_kobj;
#ifndef CONFIG_PRESERVE_FA_DUMP
static DEFINE_MUTEX(fadump_mutex);
-struct fadump_mrange_info crash_mrange_info = { "crash", NULL, 0, 0, 0 };
-struct fadump_mrange_info reserved_mrange_info = { "reserved", NULL, 0, 0, 0 };
+struct fadump_mrange_info crash_mrange_info = { "crash", NULL, 0, 0, 0, false };
+
+#define RESERVED_RNGS_SZ 16384 /* 16K - 128 entries */
+#define RESERVED_RNGS_CNT (RESERVED_RNGS_SZ / \
+ sizeof(struct fadump_memory_range))
+static struct fadump_memory_range rngs[RESERVED_RNGS_CNT];
+struct fadump_mrange_info reserved_mrange_info = { "reserved", rngs,
+ RESERVED_RNGS_SZ, 0,
+ RESERVED_RNGS_CNT, true };
+
+static void __init early_init_dt_scan_reserved_ranges(unsigned long node);
#ifdef CONFIG_CMA
static struct cma *fadump_cma;
@@ -110,6 +119,11 @@ static int __init fadump_cma_init(void) { return 1; }
int __init early_init_dt_scan_fw_dump(unsigned long node, const char *uname,
int depth, void *data)
{
+ if (depth == 0) {
+ early_init_dt_scan_reserved_ranges(node);
+ return 0;
+ }
+
if (depth != 1)
return 0;
@@ -728,10 +742,14 @@ void fadump_free_cpu_notes_buf(void)
static void fadump_free_mem_ranges(struct fadump_mrange_info *mrange_info)
{
+ if (mrange_info->is_static) {
+ mrange_info->mem_range_cnt = 0;
+ return;
+ }
+
kfree(mrange_info->mem_ranges);
- mrange_info->mem_ranges = NULL;
- mrange_info->mem_ranges_sz = 0;
- mrange_info->max_mem_ranges = 0;
+ memset((void *)((u64)mrange_info + RNG_NAME_SZ), 0,
+ (sizeof(struct fadump_mrange_info) - RNG_NAME_SZ));
}
/*
@@ -788,6 +806,12 @@ static inline int fadump_add_mem_range(struct fadump_mrange_info *mrange_info,
if (mrange_info->mem_range_cnt == mrange_info->max_mem_ranges) {
int ret;
+ if (mrange_info->is_static) {
+ pr_err("Reached array size limit for %s memory ranges\n",
+ mrange_info->name);
+ return -ENOSPC;
+ }
+
ret = fadump_alloc_mem_ranges(mrange_info);
if (ret)
return ret;
@@ -1204,20 +1228,19 @@ static void sort_and_merge_mem_ranges(struct fadump_mrange_info *mrange_info)
* Scan reserved-ranges to consider them while reserving/releasing
* memory for FADump.
*/
-static inline int fadump_scan_reserved_mem_ranges(void)
+static void __init early_init_dt_scan_reserved_ranges(unsigned long node)
{
- struct device_node *root;
const __be32 *prop;
int len, ret = -1;
unsigned long i;
- root = of_find_node_by_path("/");
- if (!root)
- return ret;
+ /* reserved-ranges already scanned */
+ if (reserved_mrange_info.mem_range_cnt != 0)
+ return;
- prop = of_get_property(root, "reserved-ranges", &len);
+ prop = of_get_flat_dt_prop(node, "reserved-ranges", &len);
if (!prop)
- return ret;
+ return;
/*
* Each reserved range is an (address,size) pair, 2 cells each,
@@ -1239,7 +1262,8 @@ static inline int fadump_scan_reserved_mem_ranges(void)
}
}
- return ret;
+ /* Compact reserved ranges */
+ sort_and_merge_mem_ranges(&reserved_mrange_info);
}
/*
@@ -1253,32 +1277,21 @@ static void fadump_release_memory(u64 begin, u64 end)
u64 ra_start, ra_end, tstart;
int i, ret;
- fadump_scan_reserved_mem_ranges();
-
ra_start = fw_dump.reserve_dump_area_start;
ra_end = ra_start + fw_dump.reserve_dump_area_size;
/*
- * Add reserved dump area to reserved ranges list
- * and exclude all these ranges while releasing memory.
+ * If reserved ranges array limit is hit, overwrite the last reserved
+ * memory range with reserved dump area to ensure it is excluded from
+ * the memory being released (reused for next FADump registration).
*/
- ret = fadump_add_mem_range(&reserved_mrange_info, ra_start, ra_end);
- if (ret != 0) {
- /*
- * Not enough memory to setup reserved ranges but the system is
- * running shortage of memory. So, release all the memory except
- * Reserved dump area (reused for next fadump registration).
- */
- if (begin < ra_end && end > ra_start) {
- if (begin < ra_start)
- fadump_release_reserved_area(begin, ra_start);
- if (end > ra_end)
- fadump_release_reserved_area(ra_end, end);
- } else
- fadump_release_reserved_area(begin, end);
+ if (reserved_mrange_info.mem_range_cnt ==
+ reserved_mrange_info.max_mem_ranges)
+ reserved_mrange_info.mem_range_cnt--;
+ ret = fadump_add_mem_range(&reserved_mrange_info, ra_start, ra_end);
+ if (ret != 0)
return;
- }
/* Get the reserved ranges list in order first. */
sort_and_merge_mem_ranges(&reserved_mrange_info);
^ permalink raw reply related
* [PATCH] powerpc/32s: Fix build failure with CONFIG_PPC_KUAP_DEBUG
From: Christophe Leroy @ 2020-04-20 7:47 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: linuxppc-dev, linux-kernel
gpr2 is not a parametre of kuap_check(), it doesn't exist.
Use gpr instead.
Fixes: a68c31fc01ef ("powerpc/32s: Implement Kernel Userspace Access Protection")
Cc: stable@vger.kernel.org
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
arch/powerpc/include/asm/book3s/32/kup.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/include/asm/book3s/32/kup.h b/arch/powerpc/include/asm/book3s/32/kup.h
index 3c0ba22dc360..db0a1c281587 100644
--- a/arch/powerpc/include/asm/book3s/32/kup.h
+++ b/arch/powerpc/include/asm/book3s/32/kup.h
@@ -75,7 +75,7 @@
.macro kuap_check current, gpr
#ifdef CONFIG_PPC_KUAP_DEBUG
- lwz \gpr2, KUAP(thread)
+ lwz \gpr, KUAP(thread)
999: twnei \gpr, 0
EMIT_BUG_ENTRY 999b, __FILE__, __LINE__, (BUGFLAG_WARNING | BUGFLAG_ONCE)
#endif
--
2.25.0
^ permalink raw reply related
* Re: [PATCH 2/2] powerpc/fadump: consider reserved ranges while reserving memory
From: Hari Bathini @ 2020-04-20 7:30 UTC (permalink / raw)
To: mahesh; +Cc: Vasant Hegde, Sourabh Jain, linuxppc-dev
In-Reply-To: <20200420052008.hmktqzerdeema5ae@in.ibm.com>
On 20/04/20 10:50 AM, Mahesh J Salgaonkar wrote:
> On 2020-03-11 01:57:10 Wed, Hari Bathini wrote:
>> Commit 0962e8004e97 ("powerpc/prom: Scan reserved-ranges node for
>> memory reservations") enabled support to parse reserved-ranges DT
>> node and reserve kernel memory falling in these ranges for F/W
>> purposes. Memory reserved for FADump should not overlap with these
>> ranges as it could corrupt memory meant for F/W or crash'ed kernel
>> memory to be exported as vmcore.
>>
>> But since commit 579ca1a27675 ("powerpc/fadump: make use of memblock's
>> bottom up allocation mode"), memblock_find_in_range() is being used to
>> find the appropriate area to reserve memory for FADump, which can't
>> account for reserved-ranges as these ranges are reserved only after
>> FADump memory reservation.
>>
>> With reserved-ranges now being populated during early boot, look out
>> for these memory ranges while reserving memory for FADump. Without
>> this change, MPIPL on PowerNV systems aborts with hostboot failure,
>> when memory reserved for FADump is less than 4096MB.
>>
>> Fixes: 579ca1a27675 ("powerpc/fadump: make use of memblock's bottom up allocation mode")
>> Cc: stable@vger.kernel.org # v5.4+
>> Signed-off-by: Hari Bathini <hbathini@linux.ibm.com>
>> ---
>> arch/powerpc/kernel/fadump.c | 76 ++++++++++++++++++++++++++++++++++++------
>> 1 file changed, 66 insertions(+), 10 deletions(-)
>>
>> diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c
>> index 7fcf4a8f..ab83be9 100644
>> --- a/arch/powerpc/kernel/fadump.c
>> +++ b/arch/powerpc/kernel/fadump.c
>> @@ -443,10 +443,70 @@ static int __init fadump_get_boot_mem_regions(void)
>> return ret;
>> }
>>
>> +/*
>> + * Returns true, if the given range overlaps with reserved memory ranges
>> + * starting at idx. Also, updates idx to index of overlapping memory range
>> + * with the given memory range.
>> + * False, otherwise.
>> + */
>> +static bool overlaps_reserved_ranges(u64 base, u64 end, int *idx)
>> +{
>> + bool ret = false;
>> + int i;
>> +
>> + for (i = *idx; i < reserved_mrange_info.mem_range_cnt; i++) {
>> + u64 rbase = reserved_mrange_info.mem_ranges[i].base;
>> + u64 rend = rbase + reserved_mrange_info.mem_ranges[i].size;
>> +
>> + if (end <= rbase)
>> + break;
>> +
>> + if ((end > rbase) && (base < rend)) {
>> + *idx = i;
>> + ret = true;
>> + break;
>> + }
>> + }
>> +
>> + return ret;
>> +}
>> +
>> +/*
>> + * Locate a suitable memory area to reserve memory for FADump. While at it,
>> + * lookup reserved-ranges & avoid overlap with them, as they are used by F/W.
>> + */
>> +static u64 __init fadump_locate_reserve_mem(u64 base, u64 size)
>> +{
>> + struct fadump_memory_range *mrngs;
>> + phys_addr_t mstart, mend;
>> + int idx = 0;
>> + u64 i;
>> +
>> + mrngs = reserved_mrange_info.mem_ranges;
>> + for_each_free_mem_range(i, NUMA_NO_NODE, MEMBLOCK_NONE,
>> + &mstart, &mend, NULL) {
>> + pr_debug("%llu) mstart: %llx, mend: %llx, base: %llx\n",
>> + i, mstart, mend, base);
>> +
>> + if (mstart > base)
>> + base = PAGE_ALIGN(mstart);
>> +
>> + while ((mend > base) && ((mend - base) >= size)) {
>> + if (!overlaps_reserved_ranges(base, base + size, &idx))
>> + goto out;
>> +
>> + base = mrngs[idx].base + mrngs[idx].size;
>> + base = PAGE_ALIGN(base);
>
> What happens when all the memory ranges found to be overlaped with
> reserved ranges ? Shoudn't this function return NULL ? Looks like in
> that case this function returns the last set base address which is
> either still overlaped or not big enough in size.
Thanks for the review, Mahesh. I overlooked that corner case.
Just posted v2 fixing it.
- Hari
^ permalink raw reply
* [PATCH v2 2/2] powerpc/fadump: consider reserved ranges while reserving memory
From: Hari Bathini @ 2020-04-20 7:26 UTC (permalink / raw)
To: Michael Ellerman, linuxppc-dev
Cc: stable, Vasant Hegde, Sourabh Jain, Mahesh J Salgaonkar
In-Reply-To: <158736754835.20831.15003408332721022400.stgit@hbathini.in.ibm.com>
Commit 0962e8004e97 ("powerpc/prom: Scan reserved-ranges node for
memory reservations") enabled support to parse reserved-ranges DT
node and reserve kernel memory falling in these ranges for F/W
purposes. Memory reserved for FADump should not overlap with these
ranges as it could corrupt memory meant for F/W or crash'ed kernel
memory to be exported as vmcore.
But since commit 579ca1a27675 ("powerpc/fadump: make use of memblock's
bottom up allocation mode"), memblock_find_in_range() is being used to
find the appropriate area to reserve memory for FADump, which can't
account for reserved-ranges as these ranges are reserved only after
FADump memory reservation.
With reserved-ranges now being populated during early boot, look out
for these memory ranges while reserving memory for FADump. Without
this change, MPIPL on PowerNV systems aborts with hostboot failure,
when memory reserved for FADump is less than 4096MB.
Fixes: 579ca1a27675 ("powerpc/fadump: make use of memblock's bottom up allocation mode")
Cc: stable@vger.kernel.org
Signed-off-by: Hari Bathini <hbathini@linux.ibm.com>
---
Changes in v2:
* Add an out parameter 'found' for fadump_locate_reserve_mem() and set it to "true"
when a suitable memory area is located.
arch/powerpc/kernel/fadump.c | 81 +++++++++++++++++++++++++++++++++++++-----
1 file changed, 71 insertions(+), 10 deletions(-)
diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c
index 679277b..0ffe69c 100644
--- a/arch/powerpc/kernel/fadump.c
+++ b/arch/powerpc/kernel/fadump.c
@@ -445,10 +445,73 @@ static int __init fadump_get_boot_mem_regions(void)
return ret;
}
+/*
+ * Returns true, if the given range overlaps with reserved memory ranges
+ * starting at idx. Also, updates idx to index of overlapping memory range
+ * with the given memory range.
+ * False, otherwise.
+ */
+static bool overlaps_reserved_ranges(u64 base, u64 end, int *idx)
+{
+ bool ret = false;
+ int i;
+
+ for (i = *idx; i < reserved_mrange_info.mem_range_cnt; i++) {
+ u64 rbase = reserved_mrange_info.mem_ranges[i].base;
+ u64 rend = rbase + reserved_mrange_info.mem_ranges[i].size;
+
+ if (end <= rbase)
+ break;
+
+ if ((end > rbase) && (base < rend)) {
+ *idx = i;
+ ret = true;
+ break;
+ }
+ }
+
+ return ret;
+}
+
+/*
+ * Locate a suitable memory area to reserve memory for FADump. While at it,
+ * lookup reserved-ranges & avoid overlap with them, as they are used by F/W.
+ */
+static u64 __init fadump_locate_reserve_mem(u64 base, u64 size, bool *found)
+{
+ struct fadump_memory_range *mrngs;
+ phys_addr_t mstart, mend;
+ int idx = 0;
+ u64 i;
+
+ *found = false;
+ mrngs = reserved_mrange_info.mem_ranges;
+ for_each_free_mem_range(i, NUMA_NO_NODE, MEMBLOCK_NONE,
+ &mstart, &mend, NULL) {
+ pr_debug("%llu) mstart: %llx, mend: %llx, base: %llx\n",
+ i, mstart, mend, base);
+
+ if (mstart > base)
+ base = PAGE_ALIGN(mstart);
+
+ while ((mend > base) && ((mend - base) >= size)) {
+ if (!overlaps_reserved_ranges(base, base+size, &idx)) {
+ *found = true;
+ goto out;
+ }
+
+ base = mrngs[idx].base + mrngs[idx].size;
+ base = PAGE_ALIGN(base);
+ }
+ }
+
+out:
+ return base;
+}
+
int __init fadump_reserve_mem(void)
{
- u64 base, size, mem_boundary, bootmem_min, align = PAGE_SIZE;
- bool is_memblock_bottom_up = memblock_bottom_up();
+ u64 base, size, mem_boundary, bootmem_min;
int ret = 1;
if (!fw_dump.fadump_enabled)
@@ -469,9 +532,9 @@ int __init fadump_reserve_mem(void)
PAGE_ALIGN(fadump_calculate_reserve_size());
#ifdef CONFIG_CMA
if (!fw_dump.nocma) {
- align = FADUMP_CMA_ALIGNMENT;
fw_dump.boot_memory_size =
- ALIGN(fw_dump.boot_memory_size, align);
+ ALIGN(fw_dump.boot_memory_size,
+ FADUMP_CMA_ALIGNMENT);
}
#endif
@@ -535,17 +598,15 @@ int __init fadump_reserve_mem(void)
pr_debug("Reserve dump area start address: 0x%lx\n",
fw_dump.reserve_dump_area_start);
} else {
+ bool found = false;
+
/*
* Reserve memory at an offset closer to bottom of the RAM to
* minimize the impact of memory hot-remove operation.
*/
- memblock_set_bottom_up(true);
- base = memblock_find_in_range(base, mem_boundary, size, align);
-
- /* Restore the previous allocation mode */
- memblock_set_bottom_up(is_memblock_bottom_up);
+ base = fadump_locate_reserve_mem(base, size, &found);
- if (!base) {
+ if (!found || (base > (mem_boundary - size))) {
pr_err("Failed to find memory chunk for reservation!\n");
goto error_out;
}
^ permalink raw reply related
* [PATCH v2 1/2] powerpc/fadump: use static allocation for reserved memory ranges
From: Hari Bathini @ 2020-04-20 7:25 UTC (permalink / raw)
To: Michael Ellerman, linuxppc-dev
Cc: stable, Vasant Hegde, Sourabh Jain, Mahesh J Salgaonkar
At times, memory ranges have to be looked up during early boot, when
kernel couldn't be initialized for dynamic memory allocation. In fact,
reserved-ranges look up is needed during FADump memory reservation.
Without accounting for reserved-ranges in reserving memory for FADump,
MPIPL boot fails with memory corruption issues. So, extend memory
ranges handling to support static allocation and populate reserved
memory ranges during early boot.
Fixes: dda9dbfeeb7a ("powerpc/fadump: consider reserved ranges while releasing memory")
Cc: stable@vger.kernel.org
Signed-off-by: Hari Bathini <hbathini@linux.ibm.com>
---
arch/powerpc/include/asm/fadump-internal.h | 4 +
arch/powerpc/kernel/fadump.c | 77 ++++++++++++++++------------
2 files changed, 48 insertions(+), 33 deletions(-)
diff --git a/arch/powerpc/include/asm/fadump-internal.h b/arch/powerpc/include/asm/fadump-internal.h
index c814a2b..8d61c8f 100644
--- a/arch/powerpc/include/asm/fadump-internal.h
+++ b/arch/powerpc/include/asm/fadump-internal.h
@@ -64,12 +64,14 @@ struct fadump_memory_range {
};
/* fadump memory ranges info */
+#define RNG_NAME_SZ 16
struct fadump_mrange_info {
- char name[16];
+ char name[RNG_NAME_SZ];
struct fadump_memory_range *mem_ranges;
u32 mem_ranges_sz;
u32 mem_range_cnt;
u32 max_mem_ranges;
+ bool is_static;
};
/* Platform specific callback functions */
diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c
index 59e60a9..679277b 100644
--- a/arch/powerpc/kernel/fadump.c
+++ b/arch/powerpc/kernel/fadump.c
@@ -40,8 +40,17 @@ struct kobject *fadump_kobj;
#ifndef CONFIG_PRESERVE_FA_DUMP
static DEFINE_MUTEX(fadump_mutex);
-struct fadump_mrange_info crash_mrange_info = { "crash", NULL, 0, 0, 0 };
-struct fadump_mrange_info reserved_mrange_info = { "reserved", NULL, 0, 0, 0 };
+struct fadump_mrange_info crash_mrange_info = { "crash", NULL, 0, 0, 0, false };
+
+#define RESERVED_RNGS_SZ 16384 /* 16K - 128 entries */
+#define RESERVED_RNGS_CNT (RESERVED_RNGS_SZ / \
+ sizeof(struct fadump_memory_range))
+static struct fadump_memory_range rngs[RESERVED_RNGS_CNT];
+struct fadump_mrange_info reserved_mrange_info = { "reserved", rngs,
+ RESERVED_RNGS_SZ, 0,
+ RESERVED_RNGS_CNT, true };
+
+static void __init early_init_dt_scan_reserved_ranges(unsigned long node);
#ifdef CONFIG_CMA
static struct cma *fadump_cma;
@@ -110,6 +119,11 @@ static int __init fadump_cma_init(void) { return 1; }
int __init early_init_dt_scan_fw_dump(unsigned long node, const char *uname,
int depth, void *data)
{
+ if (depth == 0) {
+ early_init_dt_scan_reserved_ranges(node);
+ return 0;
+ }
+
if (depth != 1)
return 0;
@@ -728,10 +742,14 @@ void fadump_free_cpu_notes_buf(void)
static void fadump_free_mem_ranges(struct fadump_mrange_info *mrange_info)
{
+ if (mrange_info->is_static) {
+ mrange_info->mem_range_cnt = 0;
+ return;
+ }
+
kfree(mrange_info->mem_ranges);
- mrange_info->mem_ranges = NULL;
- mrange_info->mem_ranges_sz = 0;
- mrange_info->max_mem_ranges = 0;
+ memset((void *)((u64)mrange_info + RNG_NAME_SZ), 0,
+ (sizeof(struct fadump_mrange_info) - RNG_NAME_SZ));
}
/*
@@ -788,6 +806,12 @@ static inline int fadump_add_mem_range(struct fadump_mrange_info *mrange_info,
if (mrange_info->mem_range_cnt == mrange_info->max_mem_ranges) {
int ret;
+ if (mrange_info->is_static) {
+ pr_err("Reached array size limit for %s memory ranges\n",
+ mrange_info->name);
+ return -ENOSPC;
+ }
+
ret = fadump_alloc_mem_ranges(mrange_info);
if (ret)
return ret;
@@ -1204,20 +1228,19 @@ static void sort_and_merge_mem_ranges(struct fadump_mrange_info *mrange_info)
* Scan reserved-ranges to consider them while reserving/releasing
* memory for FADump.
*/
-static inline int fadump_scan_reserved_mem_ranges(void)
+static void __init early_init_dt_scan_reserved_ranges(unsigned long node)
{
- struct device_node *root;
const __be32 *prop;
int len, ret = -1;
unsigned long i;
- root = of_find_node_by_path("/");
- if (!root)
- return ret;
+ /* reserved-ranges already scanned */
+ if (reserved_mrange_info.mem_range_cnt != 0)
+ return;
- prop = of_get_property(root, "reserved-ranges", &len);
+ prop = of_get_flat_dt_prop(node, "reserved-ranges", &len);
if (!prop)
- return ret;
+ return;
/*
* Each reserved range is an (address,size) pair, 2 cells each,
@@ -1239,7 +1262,8 @@ static inline int fadump_scan_reserved_mem_ranges(void)
}
}
- return ret;
+ /* Compact reserved ranges */
+ sort_and_merge_mem_ranges(&reserved_mrange_info);
}
/*
@@ -1253,32 +1277,21 @@ static void fadump_release_memory(u64 begin, u64 end)
u64 ra_start, ra_end, tstart;
int i, ret;
- fadump_scan_reserved_mem_ranges();
-
ra_start = fw_dump.reserve_dump_area_start;
ra_end = ra_start + fw_dump.reserve_dump_area_size;
/*
- * Add reserved dump area to reserved ranges list
- * and exclude all these ranges while releasing memory.
+ * If reserved ranges array limit is hit, overwrite the last reserved
+ * memory range with reserved dump area to ensure it is excluded from
+ * the memory being released (reused for next FADump registration).
*/
- ret = fadump_add_mem_range(&reserved_mrange_info, ra_start, ra_end);
- if (ret != 0) {
- /*
- * Not enough memory to setup reserved ranges but the system is
- * running shortage of memory. So, release all the memory except
- * Reserved dump area (reused for next fadump registration).
- */
- if (begin < ra_end && end > ra_start) {
- if (begin < ra_start)
- fadump_release_reserved_area(begin, ra_start);
- if (end > ra_end)
- fadump_release_reserved_area(ra_end, end);
- } else
- fadump_release_reserved_area(begin, end);
+ if (reserved_mrange_info.mem_range_cnt ==
+ reserved_mrange_info.max_mem_ranges)
+ reserved_mrange_info.mem_range_cnt--;
+ ret = fadump_add_mem_range(&reserved_mrange_info, ra_start, ra_end);
+ if (ret != 0)
return;
- }
/* Get the reserved ranges list in order first. */
sort_and_merge_mem_ranges(&reserved_mrange_info);
^ permalink raw reply related
* Re: [PATCH 1/7] KVM: s390: clean up redundant 'kvm_run' parameters
From: Tianjia Zhang @ 2020-04-20 7:17 UTC (permalink / raw)
To: Christian Borntraeger, pbonzini, tsbogend, paulus, mpe, benh,
frankja, david, cohuck, heiko.carstens, gor,
sean.j.christopherson, vkuznets, wanpengli, jmattson, joro, tglx,
mingo, bp, x86, hpa, maz, james.morse, julien.thierry.kdev,
suzuki.poulose, christoffer.dall, peterx, thuth
Cc: linux-s390, kvm, linux-mips, kvm-ppc, linux-kernel, linuxppc-dev,
kvmarm, linux-arm-kernel
In-Reply-To: <7a783487-2f9b-08a6-0ff6-f57bb90495a1@de.ibm.com>
On 2020/4/20 15:07, Christian Borntraeger wrote:
>
>
> On 19.04.20 09:51, Tianjia Zhang wrote:
>> In the current kvm version, 'kvm_run' has been included in the 'kvm_vcpu'
>> structure. Earlier than historical reasons, many kvm-related function
>> parameters retain the 'kvm_run' and 'kvm_vcpu' parameters at the same time.
>> This patch does a unified cleanup of these remaining redundant parameters.
>>
>> Signed-off-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com>
>> ---
>> arch/s390/kvm/kvm-s390.c | 127 +++++++++++++++++++++------------------
>> 1 file changed, 67 insertions(+), 60 deletions(-)
>>
>> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
>> index 443af3ead739..cf420d013ba3 100644
>> --- a/arch/s390/kvm/kvm-s390.c
>> +++ b/arch/s390/kvm/kvm-s390.c
>> @@ -4173,24 +4173,25 @@ static int __vcpu_run(struct kvm_vcpu *vcpu)
>> return rc;
>> }
>>
>> -static void sync_regs_fmt2(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
>> +static void sync_regs_fmt2(struct kvm_vcpu *vcpu)
>> {
>> + struct kvm_run *run = vcpu->run;
>
> Please use kvm_run as variable name. This makes all of the changes below go away.
>
It's OK, I will fix it in v2 patch.
Thanks,
Tianjia
>
>> struct runtime_instr_cb *riccb;
>> struct gs_cb *gscb;
>>
>> - riccb = (struct runtime_instr_cb *) &kvm_run->s.regs.riccb;
>> - gscb = (struct gs_cb *) &kvm_run->s.regs.gscb;
>> - vcpu->arch.sie_block->gpsw.mask = kvm_run->psw_mask;
>> - vcpu->arch.sie_block->gpsw.addr = kvm_run->psw_addr;
>> - if (kvm_run->kvm_dirty_regs & KVM_SYNC_ARCH0) {
>> - vcpu->arch.sie_block->todpr = kvm_run->s.regs.todpr;
>> - vcpu->arch.sie_block->pp = kvm_run->s.regs.pp;
>> - vcpu->arch.sie_block->gbea = kvm_run->s.regs.gbea;
>> - }
>> - if (kvm_run->kvm_dirty_regs & KVM_SYNC_PFAULT) {
>> - vcpu->arch.pfault_token = kvm_run->s.regs.pft;
>> - vcpu->arch.pfault_select = kvm_run->s.regs.pfs;
>> - vcpu->arch.pfault_compare = kvm_run->s.regs.pfc;
>> + riccb = (struct runtime_instr_cb *) &run->s.regs.riccb;
>> + gscb = (struct gs_cb *) &run->s.regs.gscb;
>> + vcpu->arch.sie_block->gpsw.mask = run->psw_mask;
>> + vcpu->arch.sie_block->gpsw.addr = run->psw_addr;
>> + if (run->kvm_dirty_regs & KVM_SYNC_ARCH0) {
>> + vcpu->arch.sie_block->todpr = run->s.regs.todpr;
>> + vcpu->arch.sie_block->pp = run->s.regs.pp;
>> + vcpu->arch.sie_block->gbea = run->s.regs.gbea;
>> + }
>> + if (run->kvm_dirty_regs & KVM_SYNC_PFAULT) {
>> + vcpu->arch.pfault_token = run->s.regs.pft;
>> + vcpu->arch.pfault_select = run->s.regs.pfs;
>> + vcpu->arch.pfault_compare = run->s.regs.pfc;
>> if (vcpu->arch.pfault_token == KVM_S390_PFAULT_TOKEN_INVALID)
>> kvm_clear_async_pf_completion_queue(vcpu);
>> }
>> @@ -4198,7 +4199,7 @@ static void sync_regs_fmt2(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
>> * If userspace sets the riccb (e.g. after migration) to a valid state,
>> * we should enable RI here instead of doing the lazy enablement.
>> */
>> - if ((kvm_run->kvm_dirty_regs & KVM_SYNC_RICCB) &&
>> + if ((run->kvm_dirty_regs & KVM_SYNC_RICCB) &&
>> test_kvm_facility(vcpu->kvm, 64) &&
>> riccb->v &&
>> !(vcpu->arch.sie_block->ecb3 & ECB3_RI)) {
>> @@ -4209,7 +4210,7 @@ static void sync_regs_fmt2(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
>> * If userspace sets the gscb (e.g. after migration) to non-zero,
>> * we should enable GS here instead of doing the lazy enablement.
>> */
>> - if ((kvm_run->kvm_dirty_regs & KVM_SYNC_GSCB) &&
>> + if ((run->kvm_dirty_regs & KVM_SYNC_GSCB) &&
>> test_kvm_facility(vcpu->kvm, 133) &&
>> gscb->gssm &&
>> !vcpu->arch.gs_enabled) {
>> @@ -4218,10 +4219,10 @@ static void sync_regs_fmt2(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
>> vcpu->arch.sie_block->ecd |= ECD_HOSTREGMGMT;
>> vcpu->arch.gs_enabled = 1;
>> }
>> - if ((kvm_run->kvm_dirty_regs & KVM_SYNC_BPBC) &&
>> + if ((run->kvm_dirty_regs & KVM_SYNC_BPBC) &&
>> test_kvm_facility(vcpu->kvm, 82)) {
>> vcpu->arch.sie_block->fpf &= ~FPF_BPBC;
>> - vcpu->arch.sie_block->fpf |= kvm_run->s.regs.bpbc ? FPF_BPBC : 0;
>> + vcpu->arch.sie_block->fpf |= run->s.regs.bpbc ? FPF_BPBC : 0;
>> }
>> if (MACHINE_HAS_GS) {
>> preempt_disable();
>> @@ -4232,45 +4233,47 @@ static void sync_regs_fmt2(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
>> }
>> if (vcpu->arch.gs_enabled) {
>> current->thread.gs_cb = (struct gs_cb *)
>> - &vcpu->run->s.regs.gscb;
>> + &run->s.regs.gscb;
>> restore_gs_cb(current->thread.gs_cb);
>> }
>> preempt_enable();
>> }
>> - /* SIE will load etoken directly from SDNX and therefore kvm_run */
>> + /* SIE will load etoken directly from SDNX and therefore run */
>> }
>>
>> -static void sync_regs(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
>> +static void sync_regs(struct kvm_vcpu *vcpu)
>> {
>> - if (kvm_run->kvm_dirty_regs & KVM_SYNC_PREFIX)
>> - kvm_s390_set_prefix(vcpu, kvm_run->s.regs.prefix);
>> - if (kvm_run->kvm_dirty_regs & KVM_SYNC_CRS) {
>> - memcpy(&vcpu->arch.sie_block->gcr, &kvm_run->s.regs.crs, 128);
>> + struct kvm_run *run = vcpu->run;
>> +
>> + if (run->kvm_dirty_regs & KVM_SYNC_PREFIX)
>> + kvm_s390_set_prefix(vcpu, run->s.regs.prefix);
>> + if (run->kvm_dirty_regs & KVM_SYNC_CRS) {
>> + memcpy(&vcpu->arch.sie_block->gcr, &run->s.regs.crs, 128);
>> /* some control register changes require a tlb flush */
>> kvm_make_request(KVM_REQ_TLB_FLUSH, vcpu);
>> }
>> - if (kvm_run->kvm_dirty_regs & KVM_SYNC_ARCH0) {
>> - kvm_s390_set_cpu_timer(vcpu, kvm_run->s.regs.cputm);
>> - vcpu->arch.sie_block->ckc = kvm_run->s.regs.ckc;
>> + if (run->kvm_dirty_regs & KVM_SYNC_ARCH0) {
>> + kvm_s390_set_cpu_timer(vcpu, run->s.regs.cputm);
>> + vcpu->arch.sie_block->ckc = run->s.regs.ckc;
>> }
>> save_access_regs(vcpu->arch.host_acrs);
>> - restore_access_regs(vcpu->run->s.regs.acrs);
>> + restore_access_regs(run->s.regs.acrs);
>> /* save host (userspace) fprs/vrs */
>> save_fpu_regs();
>> vcpu->arch.host_fpregs.fpc = current->thread.fpu.fpc;
>> vcpu->arch.host_fpregs.regs = current->thread.fpu.regs;
>> if (MACHINE_HAS_VX)
>> - current->thread.fpu.regs = vcpu->run->s.regs.vrs;
>> + current->thread.fpu.regs = run->s.regs.vrs;
>> else
>> - current->thread.fpu.regs = vcpu->run->s.regs.fprs;
>> - current->thread.fpu.fpc = vcpu->run->s.regs.fpc;
>> + current->thread.fpu.regs = run->s.regs.fprs;
>> + current->thread.fpu.fpc = run->s.regs.fpc;
>> if (test_fp_ctl(current->thread.fpu.fpc))
>> /* User space provided an invalid FPC, let's clear it */
>> current->thread.fpu.fpc = 0;
>>
>> /* Sync fmt2 only data */
>> if (likely(!kvm_s390_pv_cpu_is_protected(vcpu))) {
>> - sync_regs_fmt2(vcpu, kvm_run);
>> + sync_regs_fmt2(vcpu);
>> } else {
>> /*
>> * In several places we have to modify our internal view to
>> @@ -4282,19 +4285,21 @@ static void sync_regs(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
>> * do only accept the condition code from userspace.
>> */
>> vcpu->arch.sie_block->gpsw.mask &= ~PSW_MASK_CC;
>> - vcpu->arch.sie_block->gpsw.mask |= kvm_run->psw_mask &
>> + vcpu->arch.sie_block->gpsw.mask |= run->psw_mask &
>> PSW_MASK_CC;
>> }
>>
>> - kvm_run->kvm_dirty_regs = 0;
>> + run->kvm_dirty_regs = 0;
>> }
>>
>> -static void store_regs_fmt2(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
>> +static void store_regs_fmt2(struct kvm_vcpu *vcpu)
>> {
>> - kvm_run->s.regs.todpr = vcpu->arch.sie_block->todpr;
>> - kvm_run->s.regs.pp = vcpu->arch.sie_block->pp;
>> - kvm_run->s.regs.gbea = vcpu->arch.sie_block->gbea;
>> - kvm_run->s.regs.bpbc = (vcpu->arch.sie_block->fpf & FPF_BPBC) == FPF_BPBC;
>> + struct kvm_run *run = vcpu->run;
>> +
>> + run->s.regs.todpr = vcpu->arch.sie_block->todpr;
>> + run->s.regs.pp = vcpu->arch.sie_block->pp;
>> + run->s.regs.gbea = vcpu->arch.sie_block->gbea;
>> + run->s.regs.bpbc = (vcpu->arch.sie_block->fpf & FPF_BPBC) == FPF_BPBC;
>> if (MACHINE_HAS_GS) {
>> __ctl_set_bit(2, 4);
>> if (vcpu->arch.gs_enabled)
>> @@ -4310,39 +4315,41 @@ static void store_regs_fmt2(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
>> /* SIE will save etoken directly into SDNX and therefore kvm_run */
>> }
>>
>> -static void store_regs(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
>> +static void store_regs(struct kvm_vcpu *vcpu)
>> {
>> - kvm_run->psw_mask = vcpu->arch.sie_block->gpsw.mask;
>> - kvm_run->psw_addr = vcpu->arch.sie_block->gpsw.addr;
>> - kvm_run->s.regs.prefix = kvm_s390_get_prefix(vcpu);
>> - memcpy(&kvm_run->s.regs.crs, &vcpu->arch.sie_block->gcr, 128);
>> - kvm_run->s.regs.cputm = kvm_s390_get_cpu_timer(vcpu);
>> - kvm_run->s.regs.ckc = vcpu->arch.sie_block->ckc;
>> - kvm_run->s.regs.pft = vcpu->arch.pfault_token;
>> - kvm_run->s.regs.pfs = vcpu->arch.pfault_select;
>> - kvm_run->s.regs.pfc = vcpu->arch.pfault_compare;
>> - save_access_regs(vcpu->run->s.regs.acrs);
>> + struct kvm_run *run = vcpu->run;
>> +
>> + run->psw_mask = vcpu->arch.sie_block->gpsw.mask;
>> + run->psw_addr = vcpu->arch.sie_block->gpsw.addr;
>> + run->s.regs.prefix = kvm_s390_get_prefix(vcpu);
>> + memcpy(&run->s.regs.crs, &vcpu->arch.sie_block->gcr, 128);
>> + run->s.regs.cputm = kvm_s390_get_cpu_timer(vcpu);
>> + run->s.regs.ckc = vcpu->arch.sie_block->ckc;
>> + run->s.regs.pft = vcpu->arch.pfault_token;
>> + run->s.regs.pfs = vcpu->arch.pfault_select;
>> + run->s.regs.pfc = vcpu->arch.pfault_compare;
>> + save_access_regs(run->s.regs.acrs);
>> restore_access_regs(vcpu->arch.host_acrs);
>> /* Save guest register state */
>> save_fpu_regs();
>> - vcpu->run->s.regs.fpc = current->thread.fpu.fpc;
>> + run->s.regs.fpc = current->thread.fpu.fpc;
>> /* Restore will be done lazily at return */
>> current->thread.fpu.fpc = vcpu->arch.host_fpregs.fpc;
>> current->thread.fpu.regs = vcpu->arch.host_fpregs.regs;
>> if (likely(!kvm_s390_pv_cpu_is_protected(vcpu)))
>> - store_regs_fmt2(vcpu, kvm_run);
>> + store_regs_fmt2(vcpu);
>> }
>>
>> int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu)
>> {
>> - struct kvm_run *kvm_run = vcpu->run;
>> + struct kvm_run *run = vcpu->run;
>> int rc;
>>
>> - if (kvm_run->immediate_exit)
>> + if (run->immediate_exit)
>> return -EINTR;
>>
>> - if (kvm_run->kvm_valid_regs & ~KVM_SYNC_S390_VALID_FIELDS ||
>> - kvm_run->kvm_dirty_regs & ~KVM_SYNC_S390_VALID_FIELDS)
>> + if (run->kvm_valid_regs & ~KVM_SYNC_S390_VALID_FIELDS ||
>> + run->kvm_dirty_regs & ~KVM_SYNC_S390_VALID_FIELDS)
>> return -EINVAL;
>>
>> vcpu_load(vcpu);
>> @@ -4368,14 +4375,14 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu)
>> goto out;
>> }
>>
>> - sync_regs(vcpu, kvm_run);
>> + sync_regs(vcpu);
>> enable_cpu_timer_accounting(vcpu);
>>
>> might_fault();
>> rc = __vcpu_run(vcpu);
>>
>> if (signal_pending(current) && !rc) {
>> - kvm_run->exit_reason = KVM_EXIT_INTR;
>> + run->exit_reason = KVM_EXIT_INTR;
>> rc = -EINTR;
>> }
>>
>> @@ -4390,7 +4397,7 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu)
>> }
>>
>> disable_cpu_timer_accounting(vcpu);
>> - store_regs(vcpu, kvm_run);
>> + store_regs(vcpu);
>>
>> kvm_sigset_deactivate(vcpu);
>>
>>
^ permalink raw reply
* Re: [PATCH v5 4/4] powerpc/papr_scm: Implement support for DSM_PAPR_SCM_HEALTH
From: Vaibhav Jain @ 2020-04-20 7:14 UTC (permalink / raw)
To: Dan Williams
Cc: Alastair D'Silva, linux-nvdimm, Aneesh Kumar K . V,
Vishal Verma, Jeff Moyer, Oliver O'Halloran, linuxppc-dev
In-Reply-To: <CAPcyv4gnrp1NCAvbsa=DN+VrWFPcGm6WNO8-CahaJDPHpEafSg@mail.gmail.com>
Hi Dan / Mpe,
I have sent out a v6 of this patch set that addresses your review
comments so far. Also I have added a new doc patch in the patchset that
adds documentation for PAPR_SCM_HEALTH hcall specification.
Requesting you to please review the new patchset at
https://lore.kernel.org/linux-nvdimm/20200420070711.223545-1-vaibhav@linux.ibm.com
Thanks,
~ Vaibhav Jain
Dan Williams <dan.j.williams@intel.com> writes:
> On Tue, Mar 31, 2020 at 7:33 AM Vaibhav Jain <vaibhav@linux.ibm.com> wrote:
>>
>> This patch implements support for papr_scm command
>> 'DSM_PAPR_SCM_HEALTH' that returns a newly introduced 'struct
>> nd_papr_scm_dimm_health_stat' instance containing dimm health
>> information back to user space in response to ND_CMD_CALL. This
>> functionality is implemented in newly introduced papr_scm_get_health()
>> that queries the scm-dimm health information and then copies these bitmaps
>> to the package payload whose layout is defined by 'struct
>> papr_scm_ndctl_health'.
>>
>> The patch also introduces a new member a new member 'struct
>> papr_scm_priv.health' thats an instance of 'struct
>> nd_papr_scm_dimm_health_stat' to cache the health information of a
>> scm-dimm. As a result functions drc_pmem_query_health() and
>> papr_flags_show() are updated to populate and use this new struct
>> instead of two be64 integers that we earlier used.
>
> Link to HCALL specification?
>
>>
>> Signed-off-by: Vaibhav Jain <vaibhav@linux.ibm.com>
>> ---
>> Changelog:
>>
>> v4..v5: None
>>
>> v3..v4: Call the DSM_PAPR_SCM_HEALTH service function from
>> papr_scm_service_dsm() instead of papr_scm_ndctl(). [Aneesh]
>>
>> v2..v3: Updated struct nd_papr_scm_dimm_health_stat_v1 to use '__xx'
>> types as its exported to the userspace [Aneesh]
>> Changed the constants DSM_PAPR_SCM_DIMM_XX indicating dimm
>> health from enum to #defines [Aneesh]
>>
>> v1..v2: New patch in the series
>> ---
>> arch/powerpc/include/uapi/asm/papr_scm_dsm.h | 40 +++++++
>> arch/powerpc/platforms/pseries/papr_scm.c | 109 ++++++++++++++++---
>> 2 files changed, 132 insertions(+), 17 deletions(-)
>>
>> diff --git a/arch/powerpc/include/uapi/asm/papr_scm_dsm.h b/arch/powerpc/include/uapi/asm/papr_scm_dsm.h
>> index c039a49b41b4..8265125304ca 100644
>> --- a/arch/powerpc/include/uapi/asm/papr_scm_dsm.h
>> +++ b/arch/powerpc/include/uapi/asm/papr_scm_dsm.h
>> @@ -132,6 +132,7 @@ struct nd_papr_scm_cmd_pkg {
>> */
>> enum dsm_papr_scm {
>> DSM_PAPR_SCM_MIN = 0x10000,
>> + DSM_PAPR_SCM_HEALTH,
>> DSM_PAPR_SCM_MAX,
>> };
>>
>> @@ -158,4 +159,43 @@ static void *papr_scm_pcmd_to_payload(struct nd_papr_scm_cmd_pkg *pcmd)
>> else
>> return (void *)((__u8 *) pcmd + pcmd->payload_offset);
>> }
>> +
>> +/* Various scm-dimm health indicators */
>> +#define DSM_PAPR_SCM_DIMM_HEALTHY 0
>> +#define DSM_PAPR_SCM_DIMM_UNHEALTHY 1
>> +#define DSM_PAPR_SCM_DIMM_CRITICAL 2
>> +#define DSM_PAPR_SCM_DIMM_FATAL 3
>> +
>> +/*
>> + * Struct exchanged between kernel & ndctl in for PAPR_DSM_PAPR_SMART_HEALTH
>> + * Various bitflags indicate the health status of the dimm.
>> + *
>> + * dimm_unarmed : Dimm not armed. So contents wont persist.
>> + * dimm_bad_shutdown : Previous shutdown did not persist contents.
>> + * dimm_bad_restore : Contents from previous shutdown werent restored.
>> + * dimm_scrubbed : Contents of the dimm have been scrubbed.
>> + * dimm_locked : Contents of the dimm cant be modified until CEC reboot
>> + * dimm_encrypted : Contents of dimm are encrypted.
>> + * dimm_health : Dimm health indicator.
>> + */
>> +struct nd_papr_scm_dimm_health_stat_v1 {
>> + __u8 dimm_unarmed;
>> + __u8 dimm_bad_shutdown;
>> + __u8 dimm_bad_restore;
>> + __u8 dimm_scrubbed;
>> + __u8 dimm_locked;
>> + __u8 dimm_encrypted;
>> + __u16 dimm_health;
>> +};
>
> Does the structure pack the same across different compilers and configurations?
>
>> +
>> +/*
>> + * Typedef the current struct for dimm_health so that any application
>> + * or kernel recompiled after introducing a new version automatically
>> + * supports the new version.
>> + */
>> +#define nd_papr_scm_dimm_health_stat nd_papr_scm_dimm_health_stat_v1
>> +
>> +/* Current version number for the dimm health struct */
>> +#define ND_PAPR_SCM_DIMM_HEALTH_VERSION 1
>> +
>> #endif /* _UAPI_ASM_POWERPC_PAPR_SCM_DSM_H_ */
>> diff --git a/arch/powerpc/platforms/pseries/papr_scm.c b/arch/powerpc/platforms/pseries/papr_scm.c
>> index e8ce96d2249e..ce94762954e0 100644
>> --- a/arch/powerpc/platforms/pseries/papr_scm.c
>> +++ b/arch/powerpc/platforms/pseries/papr_scm.c
>> @@ -47,8 +47,7 @@ struct papr_scm_priv {
>> struct mutex dimm_mutex;
>>
>> /* Health information for the dimm */
>> - __be64 health_bitmap;
>> - __be64 health_bitmap_valid;
>> + struct nd_papr_scm_dimm_health_stat health;
>> };
>>
>> static int drc_pmem_bind(struct papr_scm_priv *p)
>> @@ -158,6 +157,7 @@ static int drc_pmem_query_health(struct papr_scm_priv *p)
>> {
>> unsigned long ret[PLPAR_HCALL_BUFSIZE];
>> int64_t rc;
>> + __be64 health;
>>
>> rc = plpar_hcall(H_SCM_HEALTH, ret, p->drc_index);
>> if (rc != H_SUCCESS) {
>> @@ -172,13 +172,41 @@ static int drc_pmem_query_health(struct papr_scm_priv *p)
>> return rc;
>>
>> /* Store the retrieved health information in dimm platform data */
>> - p->health_bitmap = ret[0];
>> - p->health_bitmap_valid = ret[1];
>> + health = ret[0] & ret[1];
>>
>> dev_dbg(&p->pdev->dev,
>> "Queried dimm health info. Bitmap:0x%016llx Mask:0x%016llx\n",
>> - be64_to_cpu(p->health_bitmap),
>> - be64_to_cpu(p->health_bitmap_valid));
>> + be64_to_cpu(ret[0]),
>> + be64_to_cpu(ret[1]));
>> +
>> + memset(&p->health, 0, sizeof(p->health));
>> +
>> + /* Check for various masks in bitmap and set the buffer */
>> + if (health & PAPR_SCM_DIMM_UNARMED_MASK)
>> + p->health.dimm_unarmed = true;
>> +
>> + if (health & PAPR_SCM_DIMM_BAD_SHUTDOWN_MASK)
>> + p->health.dimm_bad_shutdown = true;
>> +
>> + if (health & PAPR_SCM_DIMM_BAD_RESTORE_MASK)
>> + p->health.dimm_bad_restore = true;
>> +
>> + if (health & PAPR_SCM_DIMM_ENCRYPTED)
>> + p->health.dimm_encrypted = true;
>> +
>> + if (health & PAPR_SCM_DIMM_SCRUBBED_AND_LOCKED) {
>> + p->health.dimm_locked = true;
>> + p->health.dimm_scrubbed = true;
>> + }
>
> I don't think bool is suitable for ioctl ABI. For example the true
> value may be positive, or negative depending on the arch. This should
> be using explicit integer values.
>
> I assume the reason you are translating this rather than transmitting
> that raw 64-bit health value is for future compatibility if the HCALL
> format changes / is extended?
>
>> +
>> + if (health & PAPR_SCM_DIMM_HEALTH_UNHEALTHY)
>> + p->health.dimm_health = DSM_PAPR_SCM_DIMM_UNHEALTHY;
>> +
>> + if (health & PAPR_SCM_DIMM_HEALTH_CRITICAL)
>> + p->health.dimm_health = DSM_PAPR_SCM_DIMM_CRITICAL;
>> +
>> + if (health & PAPR_SCM_DIMM_HEALTH_FATAL)
>> + p->health.dimm_health = DSM_PAPR_SCM_DIMM_FATAL;
>
>>
>> mutex_unlock(&p->dimm_mutex);
>> return 0;
>> @@ -331,6 +359,51 @@ static int is_cmd_valid(struct nvdimm *nvdimm, unsigned int cmd, void *buf,
>> return 0;
>> }
>>
>> +/* Fetch the DIMM health info and populate it in provided package. */
>> +static int papr_scm_get_health(struct papr_scm_priv *p,
>> + struct nd_papr_scm_cmd_pkg *pkg)
>> +{
>> + int rc;
>> + size_t copysize = sizeof(p->health);
>> +
>> + rc = drc_pmem_query_health(p);
>> + if (rc)
>> + goto out;
>> + /*
>> + * If the requested payload version is greater than one we know
>> + * about, return the payload version we know about and let
>> + * caller/userspace handle.
>> + */
>> + if (pkg->payload_version > ND_PAPR_SCM_DIMM_HEALTH_VERSION)
>> + pkg->payload_version = ND_PAPR_SCM_DIMM_HEALTH_VERSION;
>> +
>> + if (pkg->hdr.nd_size_out < copysize) {
>> + dev_dbg(&p->pdev->dev, "%s Payload not large enough\n",
>> + __func__);
>> + dev_dbg(&p->pdev->dev, "%s Expected %lu, available %u\n",
>> + __func__, copysize, pkg->hdr.nd_size_out);
>> + rc = -ENOSPC;
>> + goto out;
>> + }
>> +
>> + dev_dbg(&p->pdev->dev, "%s Copying payload size=%lu version=0x%x\n",
>> + __func__, copysize, pkg->payload_version);
>> +
>> + /* Copy a subset of health struct based on copysize */
>> + memcpy(papr_scm_pcmd_to_payload(pkg), &p->health, copysize);
>> + pkg->hdr.nd_fw_size = copysize;
>> +
>> +out:
>> + /*
>> + * Put the error in out package and return success from function
>> + * so that errors if any are propogated back to userspace.
>> + */
>> + pkg->cmd_status = rc;
>> + dev_dbg(&p->pdev->dev, "%s completion code = %d\n", __func__, rc);
>> +
>> + return 0;
>> +}
>> +
>> static int papr_scm_service_dsm(struct papr_scm_priv *p,
>> struct nd_papr_scm_cmd_pkg *call_pkg)
>> {
>> @@ -345,6 +418,9 @@ static int papr_scm_service_dsm(struct papr_scm_priv *p,
>>
>> /* Depending on the DSM command call appropriate service routine */
>> switch (call_pkg->hdr.nd_command) {
>> + case DSM_PAPR_SCM_HEALTH:
>> + return papr_scm_get_health(p, call_pkg);
>> +
>> default:
>> pr_debug("Unsupported DSM command 0x%llx\n",
>> call_pkg->hdr.nd_command);
>> @@ -431,7 +507,6 @@ static ssize_t papr_flags_show(struct device *dev,
>> {
>> struct nvdimm *dimm = to_nvdimm(dev);
>> struct papr_scm_priv *p = nvdimm_provider_data(dimm);
>> - __be64 health;
>> int rc;
>>
>> rc = drc_pmem_query_health(p);
>> @@ -443,26 +518,26 @@ static ssize_t papr_flags_show(struct device *dev,
>> if (rc)
>> return rc;
>>
>> - health = p->health_bitmap & p->health_bitmap_valid;
>> -
>> - /* Check for various masks in bitmap and set the buffer */
>> - if (health & PAPR_SCM_DIMM_UNARMED_MASK)
>> + if (p->health.dimm_unarmed)
>> rc += sprintf(buf, "not_armed ");
>>
>> - if (health & PAPR_SCM_DIMM_BAD_SHUTDOWN_MASK)
>> + if (p->health.dimm_bad_shutdown)
>> rc += sprintf(buf + rc, "save_fail ");
>
> Per my patch1 comment is this "save_fail" or "flush_fail"?
>
>>
>> - if (health & PAPR_SCM_DIMM_BAD_RESTORE_MASK)
>> + if (p->health.dimm_bad_restore)
>> rc += sprintf(buf + rc, "restore_fail ");
>>
>> - if (health & PAPR_SCM_DIMM_ENCRYPTED)
>> + if (p->health.dimm_encrypted)
>> rc += sprintf(buf + rc, "encrypted ");
>>
>> - if (health & PAPR_SCM_DIMM_SMART_EVENT_MASK)
>> + if (p->health.dimm_health)
>> rc += sprintf(buf + rc, "smart_notify ");
>>
>> - if (health & PAPR_SCM_DIMM_SCRUBBED_AND_LOCKED)
>> - rc += sprintf(buf + rc, "scrubbed locked ");
>> + if (p->health.dimm_scrubbed)
>> + rc += sprintf(buf + rc, "scrubbed ");
>> +
>> + if (p->health.dimm_locked)
>> + rc += sprintf(buf + rc, "locked ");
>>
>> if (rc > 0)
>> rc += sprintf(buf + rc, "\n");
>> --
>> 2.25.1
>>
--
^ permalink raw reply
* Re: [PATCH] KVM: X86: Fix compile error in svm/sev.c
From: Tianjia Zhang @ 2020-04-20 7:14 UTC (permalink / raw)
To: Xiaoyao Li, pbonzini, tsbogend, paulus, mpe, benh, borntraeger,
frankja, david, cohuck, heiko.carstens, gor,
sean.j.christopherson, vkuznets, wanpengli, jmattson, joro, tglx,
mingo, bp, x86, hpa, maz, james.morse, julien.thierry.kdev,
suzuki.poulose, christoffer.dall, peterx, thuth
Cc: linux-s390, kvm, linux-mips, kvm-ppc, linux-kernel, linuxppc-dev,
kvmarm, linux-arm-kernel
In-Reply-To: <82ce1798-1dab-d271-d084-e9a89bb44e71@intel.com>
On 2020/4/19 16:24, Xiaoyao Li wrote:
> On 4/19/2020 3:30 PM, Tianjia Zhang wrote:
>> The compiler reported the following compilation errors:
>>
>> arch/x86/kvm/svm/sev.c: In function ‘sev_pin_memory’:
>> arch/x86/kvm/svm/sev.c:361:3: error: implicit declaration of function
>> ‘release_pages’ [-Werror=implicit-function-declaration]
>> release_pages(pages, npinned);
>> ^~~~~~~~~~~~~
>>
>> The reason is that the 'pagemap.h' header file is not included.
>>
>
> FYI.
>
> Boris has sent the Patch:
> https://lkml.kernel.org/r/20200411160927.27954-1-bp@alien8.de
>
> and it's already in kvm master/queue branch
>
>
This is the same fix, please ignore this patch.
Thanks and best,
Tianjia
^ permalink raw reply
* Re: [RFC 00/11] perf: Enhancing perf to export processor hazard information
From: Madhavan Srinivasan @ 2020-04-20 7:09 UTC (permalink / raw)
To: Kim Phillips, Ravi Bangoria
Cc: Mark Rutland, Andi Kleen, Peter Zijlstra, Jiri Olsa, LKML,
Stephane Eranian, Adrian Hunter, Alexander Shishkin, yao.jin,
Ingo Molnar, Paul Mackerras, Arnaldo Carvalho de Melo,
Robert Richter, Namhyung Kim, linuxppc-dev, Alexey Budankov,
Liang, Kan
In-Reply-To: <ee86b3e9-9e6e-8601-b381-9556cb7b67de@amd.com>
On 3/27/20 1:18 AM, Kim Phillips wrote:
>
> On 3/26/20 5:19 AM, maddy wrote:
>>
>> On 3/18/20 11:05 PM, Kim Phillips wrote:
>>> Hi Maddy,
>>>
>>> On 3/17/20 1:50 AM, maddy wrote:
>>>> On 3/13/20 4:08 AM, Kim Phillips wrote:
>>>>> On 3/11/20 11:00 AM, Ravi Bangoria wrote:
>>>>>> On 3/6/20 3:36 AM, Kim Phillips wrote:
>>>>>>>> On 3/3/20 3:55 AM, Kim Phillips wrote:
>>>>>>>>> On 3/2/20 2:21 PM, Stephane Eranian wrote:
>>>>>>>>>> On Mon, Mar 2, 2020 at 2:13 AM Peter Zijlstra <peterz@infradead.org> wrote:
>>>>>>>>>>> On Mon, Mar 02, 2020 at 10:53:44AM +0530, Ravi Bangoria wrote:
>>>>>>>>>>>> Modern processors export such hazard data in Performance
>>>>>>>>>>>> Monitoring Unit (PMU) registers. Ex, 'Sampled Instruction Event
>>>>>>>>>>>> Register' on IBM PowerPC[1][2] and 'Instruction-Based Sampling' on
>>>>>>>>>>>> AMD[3] provides similar information.
>>>>>>>>>>>>
>>>>>>>>>>>> Implementation detail:
>>>>>>>>>>>>
>>>>>>>>>>>> A new sample_type called PERF_SAMPLE_PIPELINE_HAZ is introduced.
>>>>>>>>>>>> If it's set, kernel converts arch specific hazard information
>>>>>>>>>>>> into generic format:
>>>>>>>>>>>>
>>>>>>>>>>>> struct perf_pipeline_haz_data {
>>>>>>>>>>>> /* Instruction/Opcode type: Load, Store, Branch .... */
>>>>>>>>>>>> __u8 itype;
>>>>>>>>>>>> /* Instruction Cache source */
>>>>>>>>>>>> __u8 icache;
>>>>>>>>>>>> /* Instruction suffered hazard in pipeline stage */
>>>>>>>>>>>> __u8 hazard_stage;
>>>>>>>>>>>> /* Hazard reason */
>>>>>>>>>>>> __u8 hazard_reason;
>>>>>>>>>>>> /* Instruction suffered stall in pipeline stage */
>>>>>>>>>>>> __u8 stall_stage;
>>>>>>>>>>>> /* Stall reason */
>>>>>>>>>>>> __u8 stall_reason;
>>>>>>>>>>>> __u16 pad;
>>>>>>>>>>>> };
>>>>>>>>>>> Kim, does this format indeed work for AMD IBS?
>>>>>>>>> It's not really 1:1, we don't have these separations of stages
>>>>>>>>> and reasons, for example: we have missed in L2 cache, for example.
>>>>>>>>> So IBS output is flatter, with more cycle latency figures than
>>>>>>>>> IBM's AFAICT.
>>>>>>>> AMD IBS captures pipeline latency data incase Fetch sampling like the
>>>>>>>> Fetch latency, tag to retire latency, completion to retire latency and
>>>>>>>> so on. Yes, Ops sampling do provide more data on load/store centric
>>>>>>>> information. But it also captures more detailed data for Branch instructions.
>>>>>>>> And we also looked at ARM SPE, which also captures more details pipeline
>>>>>>>> data and latency information.
>>>>>>>>
>>>>>>>>>> Personally, I don't like the term hazard. This is too IBM Power
>>>>>>>>>> specific. We need to find a better term, maybe stall or penalty.
>>>>>>>>> Right, IBS doesn't have a filter to only count stalled or otherwise
>>>>>>>>> bad events. IBS' PPR descriptions has one occurrence of the
>>>>>>>>> word stall, and no penalty. The way I read IBS is it's just
>>>>>>>>> reporting more sample data than just the precise IP: things like
>>>>>>>>> hits, misses, cycle latencies, addresses, types, etc., so words
>>>>>>>>> like 'extended', or the 'auxiliary' already used today even
>>>>>>>>> are more appropriate for IBS, although I'm the last person to
>>>>>>>>> bikeshed.
>>>>>>>> We are thinking of using "pipeline" word instead of Hazard.
>>>>>>> Hm, the word 'pipeline' occurs 0 times in IBS documentation.
>>>>>> NP. We thought pipeline is generic hw term so we proposed "pipeline"
>>>>>> word. We are open to term which can be generic enough.
>>>>>>
>>>>>>> I realize there are a couple of core pipeline-specific pieces
>>>>>>> of information coming out of it, but the vast majority
>>>>>>> are addresses, latencies of various components in the memory
>>>>>>> hierarchy, and various component hit/miss bits.
>>>>>> Yes. we should capture core pipeline specific details. For example,
>>>>>> IBS generates Branch unit information(IbsOpData1) and Icahce related
>>>>>> data(IbsFetchCtl) which is something that shouldn't be extended as
>>>>>> part of perf-mem, IMO.
>>>>> Sure, IBS Op-side output is more 'perf mem' friendly, and so it
>>>>> should populate perf_mem_data_src fields, just like POWER9 can:
>>>>>
>>>>> union perf_mem_data_src {
>>>>> ...
>>>>> __u64 mem_rsvd:24,
>>>>> mem_snoopx:2, /* snoop mode, ext */
>>>>> mem_remote:1, /* remote */
>>>>> mem_lvl_num:4, /* memory hierarchy level number */
>>>>> mem_dtlb:7, /* tlb access */
>>>>> mem_lock:2, /* lock instr */
>>>>> mem_snoop:5, /* snoop mode */
>>>>> mem_lvl:14, /* memory hierarchy level */
>>>>> mem_op:5; /* type of opcode */
>>>>>
>>>>>
>>>>> E.g., SIER[LDST] SIER[A_XLATE_SRC] can be used to populate
>>>>> mem_lvl[_num], SIER_TYPE can be used to populate 'mem_op',
>>>>> 'mem_lock', and the Reload Bus Source Encoding bits can
>>>>> be used to populate mem_snoop, right?
>>>> Hi Kim,
>>>>
>>>> Yes. We do expose these data as part of perf-mem for POWER.
>>> OK, I see relevant PERF_MEM_S bits in arch/powerpc/perf/isa207-common.c:
>>> isa207_find_source now, thanks.
>>>
>>>>> For IBS, I see PERF_SAMPLE_ADDR and PERF_SAMPLE_PHYS_ADDR can be
>>>>> used for the ld/st target addresses, too.
>>>>>
>>>>>>> What's needed here is a vendor-specific extended
>>>>>>> sample information that all these technologies gather,
>>>>>>> of which things like e.g., 'L1 TLB cycle latency' we
>>>>>>> all should have in common.
>>>>>> Yes. We will include fields to capture the latency cycles (like Issue
>>>>>> latency, Instruction completion latency etc..) along with other pipeline
>>>>>> details in the proposed structure.
>>>>> Latency figures are just an example, and from what I
>>>>> can tell, struct perf_sample_data already has a 'weight' member,
>>>>> used with PERF_SAMPLE_WEIGHT, that is used by intel-pt to
>>>>> transfer memory access latency figures. Granted, that's
>>>>> a bad name given all other vendors don't call latency
>>>>> 'weight'.
>>>>>
>>>>> I didn't see any latency figures coming out of POWER9,
>>>>> and do not expect this patchseries to implement those
>>>>> of other vendors, e.g., AMD's IBS; leave each vendor
>>>>> to amend perf to suit their own h/w output please.
>>>> Reference structure proposed in this patchset did not have members
>>>> to capture latency info for that exact reason. But idea here is to
>>>> abstract as vendor specific as possible. So if we include u16 array,
>>>> then this format can also capture data from IBS since it provides
>>>> few latency details.
>>> OK, that sounds a bit different from the 6 x u8's + 1 u16 padded
>>> struct presented in this patchset.
>>>
>>> IBS Ops can report e.g.:
>>>
>>> 15 tag-to-retire cycles bits,
>>> 15 completion to retire count bits,
>>> 15 L1 DTLB refill latency bits,
>>> 15 DC miss latency bits,
>>> 5 outstanding memory requests on mem refill bits, and so on.
>>>
>>> IBS Fetch reports 15 bits of fetch latency, and another 16
>>> for iTLB latency, among others.
>>>
>>> Some of these may/may not be valid simultaneously, and
>>> there are IBS specific rules to establish validity.
>>>
>>>>> My main point there, however, was that each vendor should
>>>>> use streamlined record-level code to just copy the data
>>>>> in the proprietary format that their hardware produces,
>>>>> and then then perf tooling can synthesize the events
>>>>> from the raw data at report/script/etc. time.
>>>>>
>>>>>>> I'm not sure why a new PERF_SAMPLE_PIPELINE_HAZ is needed
>>>>>>> either. Can we use PERF_SAMPLE_AUX instead?
>>>>>> We took a look at PERF_SAMPLE_AUX. IIUC, PERF_SAMPLE_AUX is intended when
>>>>>> large volume of data needs to be captured as part of perf.data without
>>>>>> frequent PMIs. But proposed type is to address the capture of pipeline
>>>>> SAMPLE_AUX shouldn't care whether the volume is large, or how frequent
>>>>> PMIs are, even though it may be used in those environments.
>>>>>
>>>>>> information on each sample using PMI at periodic intervals. Hence proposing
>>>>>> PERF_SAMPLE_PIPELINE_HAZ.
>>>>> And that's fine for any extra bits that POWER9 has to convey
>>>>> to its users beyond things already represented by other sample
>>>>> types like PERF_SAMPLE_DATA_SRC, but the capturing of both POWER9
>>>>> and other vendor e.g., AMD IBS data can be made vendor-independent
>>>>> at record time by using SAMPLE_AUX, or SAMPLE_RAW even, which is
>>>>> what IBS currently uses.
>>>> My bad. Not sure what you mean by this. We are trying to abstract
>>>> as much vendor specific data as possible with this (like perf-mem).
>>> Perhaps if I say it this way: instead of doing all the
>>> isa207_get_phazard_data() work past the mfspr(SPRN_SIER)
>>> in patch 4/11, rather/instead just put the raw sier value in a
>>> PERF_SAMPLE_RAW or _AUX event, and call perf_event_update_userpage.
>>> Specific SIER capabilities can be written as part of the perf.data
>>> header. Then synthesize the true pipe events from the raw SIER
>>> values later, and in userspace.
>> Hi Kim,
>>
>> Would like to stay away from SAMPLE_RAW type for these comments in perf_events.h
>>
>> * #
>> * # The RAW record below is opaque data wrt the ABI
>> * #
>> * # That is, the ABI doesn't make any promises wrt to
>> * # the stability of its content, it may vary depending
>> * # on event, hardware, kernel version and phase of
>> * # the moon.
>> * #
>> * # In other words, PERF_SAMPLE_RAW contents are not an ABI.
>> * #
> The "it may vary depending on ... hardware" clause makes it sound
> appropriate for the use-case where the raw hardware register contents
> are copied directly into the user buffer.
Hi Kim,
Sorry for the delayed response.
But perf tool side needs infrastructure to handle the raw sample
data from cpu-pmu (used by tracepoints). I am not sure whether
his is the approach we should look here.
peterz any comments?
>
>> Secondly, sorry I didn't understand your suggestion about using PERF_SAMPLE_AUX.
>> IIUC, SAMPLE_AUX will go to AUX ring buffer, which is more memory and more
>> challenging when correlating and presenting the pipeline details for each IP.
>> IMO, having a new sample type can be useful to capture the pipeline data
>> both in perf_sample_data and if _AUX is enabled, can be made to push to
>> AUX buffer.
> OK, I didn't think SAMPLE_AUX and the aux ring buffer were
> interdependent, sorry.
>
> Thanks,
>
> Kim
^ permalink raw reply
* Re: [PATCH 1/7] KVM: s390: clean up redundant 'kvm_run' parameters
From: Christian Borntraeger @ 2020-04-20 7:07 UTC (permalink / raw)
To: Tianjia Zhang, pbonzini, tsbogend, paulus, mpe, benh, frankja,
david, cohuck, heiko.carstens, gor, sean.j.christopherson,
vkuznets, wanpengli, jmattson, joro, tglx, mingo, bp, x86, hpa,
maz, james.morse, julien.thierry.kdev, suzuki.poulose,
christoffer.dall, peterx, thuth
Cc: linux-s390, kvm, linux-mips, kvm-ppc, linux-kernel, linuxppc-dev,
kvmarm, linux-arm-kernel
In-Reply-To: <20200419075106.16248-2-tianjia.zhang@linux.alibaba.com>
On 19.04.20 09:51, Tianjia Zhang wrote:
> In the current kvm version, 'kvm_run' has been included in the 'kvm_vcpu'
> structure. Earlier than historical reasons, many kvm-related function
> parameters retain the 'kvm_run' and 'kvm_vcpu' parameters at the same time.
> This patch does a unified cleanup of these remaining redundant parameters.
>
> Signed-off-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com>
> ---
> arch/s390/kvm/kvm-s390.c | 127 +++++++++++++++++++++------------------
> 1 file changed, 67 insertions(+), 60 deletions(-)
>
> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> index 443af3ead739..cf420d013ba3 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
> @@ -4173,24 +4173,25 @@ static int __vcpu_run(struct kvm_vcpu *vcpu)
> return rc;
> }
>
> -static void sync_regs_fmt2(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
> +static void sync_regs_fmt2(struct kvm_vcpu *vcpu)
> {
> + struct kvm_run *run = vcpu->run;
Please use kvm_run as variable name. This makes all of the changes below go away.
> struct runtime_instr_cb *riccb;
> struct gs_cb *gscb;
>
> - riccb = (struct runtime_instr_cb *) &kvm_run->s.regs.riccb;
> - gscb = (struct gs_cb *) &kvm_run->s.regs.gscb;
> - vcpu->arch.sie_block->gpsw.mask = kvm_run->psw_mask;
> - vcpu->arch.sie_block->gpsw.addr = kvm_run->psw_addr;
> - if (kvm_run->kvm_dirty_regs & KVM_SYNC_ARCH0) {
> - vcpu->arch.sie_block->todpr = kvm_run->s.regs.todpr;
> - vcpu->arch.sie_block->pp = kvm_run->s.regs.pp;
> - vcpu->arch.sie_block->gbea = kvm_run->s.regs.gbea;
> - }
> - if (kvm_run->kvm_dirty_regs & KVM_SYNC_PFAULT) {
> - vcpu->arch.pfault_token = kvm_run->s.regs.pft;
> - vcpu->arch.pfault_select = kvm_run->s.regs.pfs;
> - vcpu->arch.pfault_compare = kvm_run->s.regs.pfc;
> + riccb = (struct runtime_instr_cb *) &run->s.regs.riccb;
> + gscb = (struct gs_cb *) &run->s.regs.gscb;
> + vcpu->arch.sie_block->gpsw.mask = run->psw_mask;
> + vcpu->arch.sie_block->gpsw.addr = run->psw_addr;
> + if (run->kvm_dirty_regs & KVM_SYNC_ARCH0) {
> + vcpu->arch.sie_block->todpr = run->s.regs.todpr;
> + vcpu->arch.sie_block->pp = run->s.regs.pp;
> + vcpu->arch.sie_block->gbea = run->s.regs.gbea;
> + }
> + if (run->kvm_dirty_regs & KVM_SYNC_PFAULT) {
> + vcpu->arch.pfault_token = run->s.regs.pft;
> + vcpu->arch.pfault_select = run->s.regs.pfs;
> + vcpu->arch.pfault_compare = run->s.regs.pfc;
> if (vcpu->arch.pfault_token == KVM_S390_PFAULT_TOKEN_INVALID)
> kvm_clear_async_pf_completion_queue(vcpu);
> }
> @@ -4198,7 +4199,7 @@ static void sync_regs_fmt2(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
> * If userspace sets the riccb (e.g. after migration) to a valid state,
> * we should enable RI here instead of doing the lazy enablement.
> */
> - if ((kvm_run->kvm_dirty_regs & KVM_SYNC_RICCB) &&
> + if ((run->kvm_dirty_regs & KVM_SYNC_RICCB) &&
> test_kvm_facility(vcpu->kvm, 64) &&
> riccb->v &&
> !(vcpu->arch.sie_block->ecb3 & ECB3_RI)) {
> @@ -4209,7 +4210,7 @@ static void sync_regs_fmt2(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
> * If userspace sets the gscb (e.g. after migration) to non-zero,
> * we should enable GS here instead of doing the lazy enablement.
> */
> - if ((kvm_run->kvm_dirty_regs & KVM_SYNC_GSCB) &&
> + if ((run->kvm_dirty_regs & KVM_SYNC_GSCB) &&
> test_kvm_facility(vcpu->kvm, 133) &&
> gscb->gssm &&
> !vcpu->arch.gs_enabled) {
> @@ -4218,10 +4219,10 @@ static void sync_regs_fmt2(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
> vcpu->arch.sie_block->ecd |= ECD_HOSTREGMGMT;
> vcpu->arch.gs_enabled = 1;
> }
> - if ((kvm_run->kvm_dirty_regs & KVM_SYNC_BPBC) &&
> + if ((run->kvm_dirty_regs & KVM_SYNC_BPBC) &&
> test_kvm_facility(vcpu->kvm, 82)) {
> vcpu->arch.sie_block->fpf &= ~FPF_BPBC;
> - vcpu->arch.sie_block->fpf |= kvm_run->s.regs.bpbc ? FPF_BPBC : 0;
> + vcpu->arch.sie_block->fpf |= run->s.regs.bpbc ? FPF_BPBC : 0;
> }
> if (MACHINE_HAS_GS) {
> preempt_disable();
> @@ -4232,45 +4233,47 @@ static void sync_regs_fmt2(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
> }
> if (vcpu->arch.gs_enabled) {
> current->thread.gs_cb = (struct gs_cb *)
> - &vcpu->run->s.regs.gscb;
> + &run->s.regs.gscb;
> restore_gs_cb(current->thread.gs_cb);
> }
> preempt_enable();
> }
> - /* SIE will load etoken directly from SDNX and therefore kvm_run */
> + /* SIE will load etoken directly from SDNX and therefore run */
> }
>
> -static void sync_regs(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
> +static void sync_regs(struct kvm_vcpu *vcpu)
> {
> - if (kvm_run->kvm_dirty_regs & KVM_SYNC_PREFIX)
> - kvm_s390_set_prefix(vcpu, kvm_run->s.regs.prefix);
> - if (kvm_run->kvm_dirty_regs & KVM_SYNC_CRS) {
> - memcpy(&vcpu->arch.sie_block->gcr, &kvm_run->s.regs.crs, 128);
> + struct kvm_run *run = vcpu->run;
> +
> + if (run->kvm_dirty_regs & KVM_SYNC_PREFIX)
> + kvm_s390_set_prefix(vcpu, run->s.regs.prefix);
> + if (run->kvm_dirty_regs & KVM_SYNC_CRS) {
> + memcpy(&vcpu->arch.sie_block->gcr, &run->s.regs.crs, 128);
> /* some control register changes require a tlb flush */
> kvm_make_request(KVM_REQ_TLB_FLUSH, vcpu);
> }
> - if (kvm_run->kvm_dirty_regs & KVM_SYNC_ARCH0) {
> - kvm_s390_set_cpu_timer(vcpu, kvm_run->s.regs.cputm);
> - vcpu->arch.sie_block->ckc = kvm_run->s.regs.ckc;
> + if (run->kvm_dirty_regs & KVM_SYNC_ARCH0) {
> + kvm_s390_set_cpu_timer(vcpu, run->s.regs.cputm);
> + vcpu->arch.sie_block->ckc = run->s.regs.ckc;
> }
> save_access_regs(vcpu->arch.host_acrs);
> - restore_access_regs(vcpu->run->s.regs.acrs);
> + restore_access_regs(run->s.regs.acrs);
> /* save host (userspace) fprs/vrs */
> save_fpu_regs();
> vcpu->arch.host_fpregs.fpc = current->thread.fpu.fpc;
> vcpu->arch.host_fpregs.regs = current->thread.fpu.regs;
> if (MACHINE_HAS_VX)
> - current->thread.fpu.regs = vcpu->run->s.regs.vrs;
> + current->thread.fpu.regs = run->s.regs.vrs;
> else
> - current->thread.fpu.regs = vcpu->run->s.regs.fprs;
> - current->thread.fpu.fpc = vcpu->run->s.regs.fpc;
> + current->thread.fpu.regs = run->s.regs.fprs;
> + current->thread.fpu.fpc = run->s.regs.fpc;
> if (test_fp_ctl(current->thread.fpu.fpc))
> /* User space provided an invalid FPC, let's clear it */
> current->thread.fpu.fpc = 0;
>
> /* Sync fmt2 only data */
> if (likely(!kvm_s390_pv_cpu_is_protected(vcpu))) {
> - sync_regs_fmt2(vcpu, kvm_run);
> + sync_regs_fmt2(vcpu);
> } else {
> /*
> * In several places we have to modify our internal view to
> @@ -4282,19 +4285,21 @@ static void sync_regs(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
> * do only accept the condition code from userspace.
> */
> vcpu->arch.sie_block->gpsw.mask &= ~PSW_MASK_CC;
> - vcpu->arch.sie_block->gpsw.mask |= kvm_run->psw_mask &
> + vcpu->arch.sie_block->gpsw.mask |= run->psw_mask &
> PSW_MASK_CC;
> }
>
> - kvm_run->kvm_dirty_regs = 0;
> + run->kvm_dirty_regs = 0;
> }
>
> -static void store_regs_fmt2(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
> +static void store_regs_fmt2(struct kvm_vcpu *vcpu)
> {
> - kvm_run->s.regs.todpr = vcpu->arch.sie_block->todpr;
> - kvm_run->s.regs.pp = vcpu->arch.sie_block->pp;
> - kvm_run->s.regs.gbea = vcpu->arch.sie_block->gbea;
> - kvm_run->s.regs.bpbc = (vcpu->arch.sie_block->fpf & FPF_BPBC) == FPF_BPBC;
> + struct kvm_run *run = vcpu->run;
> +
> + run->s.regs.todpr = vcpu->arch.sie_block->todpr;
> + run->s.regs.pp = vcpu->arch.sie_block->pp;
> + run->s.regs.gbea = vcpu->arch.sie_block->gbea;
> + run->s.regs.bpbc = (vcpu->arch.sie_block->fpf & FPF_BPBC) == FPF_BPBC;
> if (MACHINE_HAS_GS) {
> __ctl_set_bit(2, 4);
> if (vcpu->arch.gs_enabled)
> @@ -4310,39 +4315,41 @@ static void store_regs_fmt2(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
> /* SIE will save etoken directly into SDNX and therefore kvm_run */
> }
>
> -static void store_regs(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
> +static void store_regs(struct kvm_vcpu *vcpu)
> {
> - kvm_run->psw_mask = vcpu->arch.sie_block->gpsw.mask;
> - kvm_run->psw_addr = vcpu->arch.sie_block->gpsw.addr;
> - kvm_run->s.regs.prefix = kvm_s390_get_prefix(vcpu);
> - memcpy(&kvm_run->s.regs.crs, &vcpu->arch.sie_block->gcr, 128);
> - kvm_run->s.regs.cputm = kvm_s390_get_cpu_timer(vcpu);
> - kvm_run->s.regs.ckc = vcpu->arch.sie_block->ckc;
> - kvm_run->s.regs.pft = vcpu->arch.pfault_token;
> - kvm_run->s.regs.pfs = vcpu->arch.pfault_select;
> - kvm_run->s.regs.pfc = vcpu->arch.pfault_compare;
> - save_access_regs(vcpu->run->s.regs.acrs);
> + struct kvm_run *run = vcpu->run;
> +
> + run->psw_mask = vcpu->arch.sie_block->gpsw.mask;
> + run->psw_addr = vcpu->arch.sie_block->gpsw.addr;
> + run->s.regs.prefix = kvm_s390_get_prefix(vcpu);
> + memcpy(&run->s.regs.crs, &vcpu->arch.sie_block->gcr, 128);
> + run->s.regs.cputm = kvm_s390_get_cpu_timer(vcpu);
> + run->s.regs.ckc = vcpu->arch.sie_block->ckc;
> + run->s.regs.pft = vcpu->arch.pfault_token;
> + run->s.regs.pfs = vcpu->arch.pfault_select;
> + run->s.regs.pfc = vcpu->arch.pfault_compare;
> + save_access_regs(run->s.regs.acrs);
> restore_access_regs(vcpu->arch.host_acrs);
> /* Save guest register state */
> save_fpu_regs();
> - vcpu->run->s.regs.fpc = current->thread.fpu.fpc;
> + run->s.regs.fpc = current->thread.fpu.fpc;
> /* Restore will be done lazily at return */
> current->thread.fpu.fpc = vcpu->arch.host_fpregs.fpc;
> current->thread.fpu.regs = vcpu->arch.host_fpregs.regs;
> if (likely(!kvm_s390_pv_cpu_is_protected(vcpu)))
> - store_regs_fmt2(vcpu, kvm_run);
> + store_regs_fmt2(vcpu);
> }
>
> int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu)
> {
> - struct kvm_run *kvm_run = vcpu->run;
> + struct kvm_run *run = vcpu->run;
> int rc;
>
> - if (kvm_run->immediate_exit)
> + if (run->immediate_exit)
> return -EINTR;
>
> - if (kvm_run->kvm_valid_regs & ~KVM_SYNC_S390_VALID_FIELDS ||
> - kvm_run->kvm_dirty_regs & ~KVM_SYNC_S390_VALID_FIELDS)
> + if (run->kvm_valid_regs & ~KVM_SYNC_S390_VALID_FIELDS ||
> + run->kvm_dirty_regs & ~KVM_SYNC_S390_VALID_FIELDS)
> return -EINVAL;
>
> vcpu_load(vcpu);
> @@ -4368,14 +4375,14 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu)
> goto out;
> }
>
> - sync_regs(vcpu, kvm_run);
> + sync_regs(vcpu);
> enable_cpu_timer_accounting(vcpu);
>
> might_fault();
> rc = __vcpu_run(vcpu);
>
> if (signal_pending(current) && !rc) {
> - kvm_run->exit_reason = KVM_EXIT_INTR;
> + run->exit_reason = KVM_EXIT_INTR;
> rc = -EINTR;
> }
>
> @@ -4390,7 +4397,7 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu)
> }
>
> disable_cpu_timer_accounting(vcpu);
> - store_regs(vcpu, kvm_run);
> + store_regs(vcpu);
>
> kvm_sigset_deactivate(vcpu);
>
>
^ permalink raw reply
* [PATCH v6 4/4] powerpc/papr_scm: Implement support for PAPR_SCM_PDSM_HEALTH
From: Vaibhav Jain @ 2020-04-20 7:07 UTC (permalink / raw)
To: linuxppc-dev, linux-nvdimm
Cc: Santosh Sivaraj, Oliver O'Halloran, Aneesh Kumar K . V,
Vaibhav Jain, Dan Williams
In-Reply-To: <20200420070711.223545-1-vaibhav@linux.ibm.com>
This patch implements support for PDSM request 'PAPR_SCM_PDSM_HEALTH'
that returns a newly introduced 'struct nd_papr_pdsm_health' instance
containing dimm health information back to user space in response to
ND_CMD_CALL. This functionality is implemented in newly introduced
papr_scm_get_health() that queries the scm-dimm health information and
then copies this information to the package payload whose layout is
defined by 'struct nd_papr_pdsm_health'.
The patch also introduces a new member 'struct papr_scm_priv.health'
thats an instance of 'struct nd_papr_pdsm_health' to cache the health
information of a nvdimm. As a result functions drc_pmem_query_health()
and flags_show() are updated to populate and use this new struct
instead of a u64 integer that was earlier used.
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: "Aneesh Kumar K . V" <aneesh.kumar@linux.ibm.com>
Signed-off-by: Vaibhav Jain <vaibhav@linux.ibm.com>
---
Changelog:
v5..v6:
* Added attribute '__packed' to 'struct nd_papr_pdsm_health_v1' to
gaurd against possibility of different compilers adding different
paddings to the struct [ Dan Williams ]
* Updated 'struct nd_papr_pdsm_health_v1' to use __u8 instead of
'bool' and also updated drc_pmem_query_health() to take this into
account. [ Dan Williams ]
v4..v5:
* None
v3..v4:
* Call the DSM_PAPR_SCM_HEALTH service function from
papr_scm_service_dsm() instead of papr_scm_ndctl(). [Aneesh]
v2..v3:
* Updated struct nd_papr_scm_dimm_health_stat_v1 to use '__xx' types
as its exported to the userspace [Aneesh]
* Changed the constants DSM_PAPR_SCM_DIMM_XX indicating dimm health
from enum to #defines [Aneesh]
v1..v2:
* New patch in the series
---
arch/powerpc/include/uapi/asm/papr_scm_pdsm.h | 39 ++++++
arch/powerpc/platforms/pseries/papr_scm.c | 122 +++++++++++++++---
2 files changed, 145 insertions(+), 16 deletions(-)
diff --git a/arch/powerpc/include/uapi/asm/papr_scm_pdsm.h b/arch/powerpc/include/uapi/asm/papr_scm_pdsm.h
index ec48b5c7fc18..f387ed3958bb 100644
--- a/arch/powerpc/include/uapi/asm/papr_scm_pdsm.h
+++ b/arch/powerpc/include/uapi/asm/papr_scm_pdsm.h
@@ -123,6 +123,7 @@ struct nd_pdsm_cmd_pkg {
*/
enum papr_scm_pdsm {
PAPR_SCM_PDSM_MIN = 0x0,
+ PAPR_SCM_PDSM_HEALTH,
PAPR_SCM_PDSM_MAX,
};
@@ -150,4 +151,42 @@ static void *pdsm_cmd_to_payload(struct nd_pdsm_cmd_pkg *pcmd)
return (void *)((__u8 *) pcmd + pcmd->payload_offset);
}
+/* Various scm-dimm health indicators */
+#define PAPR_PDSM_DIMM_HEALTHY 0
+#define PAPR_PDSM_DIMM_UNHEALTHY 1
+#define PAPR_PDSM_DIMM_CRITICAL 2
+#define PAPR_PDSM_DIMM_FATAL 3
+
+/*
+ * Struct exchanged between kernel & ndctl in for PAPR_SCM_PDSM_HEALTH
+ * Various flags indicate the health status of the dimm.
+ *
+ * dimm_unarmed : Dimm not armed. So contents wont persist.
+ * dimm_bad_shutdown : Previous shutdown did not persist contents.
+ * dimm_bad_restore : Contents from previous shutdown werent restored.
+ * dimm_scrubbed : Contents of the dimm have been scrubbed.
+ * dimm_locked : Contents of the dimm cant be modified until CEC reboot
+ * dimm_encrypted : Contents of dimm are encrypted.
+ * dimm_health : Dimm health indicator. One of PAPR_PDSM_DIMM_XXXX
+ */
+struct nd_papr_pdsm_health_v1 {
+ __u8 dimm_unarmed;
+ __u8 dimm_bad_shutdown;
+ __u8 dimm_bad_restore;
+ __u8 dimm_scrubbed;
+ __u8 dimm_locked;
+ __u8 dimm_encrypted;
+ __u16 dimm_health;
+} __packed;
+
+/*
+ * Typedef the current struct for dimm_health so that any application
+ * or kernel recompiled after introducing a new version automatically
+ * supports the new version.
+ */
+#define nd_papr_pdsm_health nd_papr_pdsm_health_v1
+
+/* Current version number for the dimm health struct */
+#define ND_PAPR_PDSM_HEALTH_VERSION 1
+
#endif /* _UAPI_ASM_POWERPC_PAPR_SCM_PDSM_H_ */
diff --git a/arch/powerpc/platforms/pseries/papr_scm.c b/arch/powerpc/platforms/pseries/papr_scm.c
index 20da1b837017..dfb02c9e3784 100644
--- a/arch/powerpc/platforms/pseries/papr_scm.c
+++ b/arch/powerpc/platforms/pseries/papr_scm.c
@@ -50,7 +50,7 @@ struct papr_scm_priv {
unsigned long lasthealth_jiffies;
/* Health information for the dimm */
- u64 health_bitmap;
+ struct nd_papr_pdsm_health health;
};
static int drc_pmem_bind(struct papr_scm_priv *p)
@@ -170,6 +170,7 @@ static int drc_pmem_query_health(struct papr_scm_priv *p, bool force)
unsigned long ret[PLPAR_HCALL_BUFSIZE];
s64 rc;
unsigned long cache_timeout;
+ u64 health;
/* Protect concurrent modifications to papr_scm_priv */
rc = mutex_lock_interruptible(&p->dimm_mutex);
@@ -196,12 +197,41 @@ static int drc_pmem_query_health(struct papr_scm_priv *p, bool force)
}
p->lasthealth_jiffies = jiffies;
- p->health_bitmap = ret[0] & ret[1];
+ health = ret[0] & ret[1];
dev_dbg(&p->pdev->dev,
"Queried dimm health info. Bitmap:0x%016lx Mask:0x%016lx\n",
ret[0], ret[1]);
+ memset(&p->health, 0, sizeof(p->health));
+
+ /* Check for various masks in bitmap and set the buffer */
+ if (health & PAPR_SCM_DIMM_UNARMED_MASK)
+ p->health.dimm_unarmed = 1;
+
+ if (health & PAPR_SCM_DIMM_BAD_SHUTDOWN_MASK)
+ p->health.dimm_bad_shutdown = 1;
+
+ if (health & PAPR_SCM_DIMM_BAD_RESTORE_MASK)
+ p->health.dimm_bad_restore = 1;
+
+ if (health & PAPR_SCM_DIMM_ENCRYPTED)
+ p->health.dimm_encrypted = 1;
+
+ if (health & PAPR_SCM_DIMM_SCRUBBED_AND_LOCKED) {
+ p->health.dimm_locked = 1;
+ p->health.dimm_scrubbed = 1;
+ }
+
+ if (health & PAPR_SCM_DIMM_HEALTH_UNHEALTHY)
+ p->health.dimm_health = PAPR_PDSM_DIMM_UNHEALTHY;
+
+ if (health & PAPR_SCM_DIMM_HEALTH_CRITICAL)
+ p->health.dimm_health = PAPR_PDSM_DIMM_CRITICAL;
+
+ if (health & PAPR_SCM_DIMM_HEALTH_FATAL)
+ p->health.dimm_health = PAPR_PDSM_DIMM_FATAL;
+
out:
mutex_unlock(&p->dimm_mutex);
return rc;
@@ -359,6 +389,61 @@ static int is_cmd_valid(struct nvdimm *nvdimm, unsigned int cmd, void *buf,
return 0;
}
+/* Fetch the DIMM health info and populate it in provided package. */
+static int papr_scm_get_health(struct papr_scm_priv *p,
+ struct nd_pdsm_cmd_pkg *pkg)
+{
+ int rc;
+ size_t copysize = sizeof(p->health);
+
+ /* Always fetch upto date dimm health data ignoring cached values */
+ rc = drc_pmem_query_health(p, true);
+ if (rc)
+ goto out;
+ /*
+ * If the requested payload version is greater than one we know
+ * about, return the payload version we know about and let
+ * caller/userspace handle.
+ */
+ if (pkg->payload_version > ND_PAPR_PDSM_HEALTH_VERSION)
+ pkg->payload_version = ND_PAPR_PDSM_HEALTH_VERSION;
+
+ if (pkg->hdr.nd_size_out < copysize) {
+ dev_dbg(&p->pdev->dev, "Truncated payload (%u). Expected (%lu)",
+ pkg->hdr.nd_size_out, copysize);
+ rc = -ENOSPC;
+ goto out;
+ }
+
+ dev_dbg(&p->pdev->dev, "Copying payload size=%lu version=0x%x\n",
+ copysize, pkg->payload_version);
+
+ /*
+ * Copy a subset of health struct based on copysize ensuring dimm mutex
+ * is locked to prevent a simultaneous read/write of health data
+ */
+ rc = mutex_lock_interruptible(&p->dimm_mutex);
+ if (rc)
+ goto out;
+
+ /* Copy the health struct to the payload */
+ memcpy(pdsm_cmd_to_payload(pkg), &p->health, copysize);
+
+ mutex_unlock(&p->dimm_mutex);
+
+ pkg->hdr.nd_fw_size = copysize;
+
+out:
+ /*
+ * Put the error in out package and return success from function
+ * so that errors if any are propogated back to userspace.
+ */
+ pkg->cmd_status = rc;
+ dev_dbg(&p->pdev->dev, "completion code = %d\n", rc);
+
+ return 0;
+}
+
static int papr_scm_service_pdsm(struct papr_scm_priv *p,
struct nd_pdsm_cmd_pkg *call_pkg)
{
@@ -373,6 +458,9 @@ static int papr_scm_service_pdsm(struct papr_scm_priv *p,
/* Depending on the DSM command call appropriate service routine */
switch (call_pkg->hdr.nd_command) {
+ case PAPR_SCM_PDSM_HEALTH:
+ return papr_scm_get_health(p, call_pkg);
+
default:
dev_dbg(&p->pdev->dev, "Unsupported PDSM request 0x%llx\n",
call_pkg->hdr.nd_command);
@@ -459,39 +547,41 @@ static ssize_t flags_show(struct device *dev,
struct nvdimm *dimm = to_nvdimm(dev);
struct papr_scm_priv *p = nvdimm_provider_data(dimm);
int rc;
- u64 health;
rc = drc_pmem_query_health(p, false);
if (rc)
return rc;
- /*
- * Copy the LE byte-ordered health_bitmap locally, check for various
- * masks and update the sysfs out buffer.
- */
- health = p->health_bitmap;
+ /* Protect against concurrent modifications to papr_scm_priv */
+ rc = mutex_lock_interruptible(&p->dimm_mutex);
+ if (rc)
+ return rc;
- if (health & PAPR_SCM_DIMM_UNARMED_MASK)
+ if (p->health.dimm_unarmed)
rc += sprintf(buf, "not_armed ");
- if (health & PAPR_SCM_DIMM_BAD_SHUTDOWN_MASK)
- rc += sprintf(buf + rc, "save_fail ");
+ if (p->health.dimm_bad_shutdown)
+ rc += sprintf(buf + rc, "flush_fail ");
- if (health & PAPR_SCM_DIMM_BAD_RESTORE_MASK)
+ if (p->health.dimm_bad_restore)
rc += sprintf(buf + rc, "restore_fail ");
- if (health & PAPR_SCM_DIMM_ENCRYPTED)
+ if (p->health.dimm_encrypted)
rc += sprintf(buf + rc, "encrypted ");
- if (health & PAPR_SCM_DIMM_SMART_EVENT_MASK)
+ if (p->health.dimm_health)
rc += sprintf(buf + rc, "smart_notify ");
- if (health & PAPR_SCM_DIMM_SCRUBBED_AND_LOCKED)
- rc += sprintf(buf + rc, "scrubbed locked ");
+ if (p->health.dimm_scrubbed)
+ rc += sprintf(buf + rc, "scrubbed ");
+
+ if (p->health.dimm_locked)
+ rc += sprintf(buf + rc, "locked ");
if (rc > 0)
rc += sprintf(buf + rc, "\n");
+ mutex_unlock(&p->dimm_mutex);
return rc;
}
DEVICE_ATTR_RO(flags);
--
2.25.3
^ permalink raw reply related
* [PATCH v6 3/4] ndctl/papr_scm, uapi: Add support for PAPR nvdimm specific methods
From: Vaibhav Jain @ 2020-04-20 7:07 UTC (permalink / raw)
To: linuxppc-dev, linux-nvdimm
Cc: Santosh Sivaraj, Oliver O'Halloran, Aneesh Kumar K . V,
Vaibhav Jain, Dan Williams
In-Reply-To: <20200420070711.223545-1-vaibhav@linux.ibm.com>
Introduce support for Papr nvDimm Specific Methods (PDSM) in papr_scm
modules and add the command family to the white list of NVDIMM command
sets. Also advertise support for ND_CMD_CALL for the dimm
command mask and implement necessary scaffolding in the module to
handle ND_CMD_CALL ioctl and PDSM requests that we receive.
The layout of the PDSM request as we expect from libnvdimm/libndctl is
described in newly introduced uapi header 'papr_scm_pdsm.h' which
defines a new 'struct nd_pdsm_cmd_pkg' header. This header is used
to communicate the PDSM request via member
'nd_pkg_papr_scm->nd_command' and size of payload that need to be
sent/received for servicing the PDSM.
A new function is_cmd_valid() is implemented that reads the args to
papr_scm_ndctl() and performs sanity tests on them. A new function
papr_scm_service_pdsm() is introduced and is called from
papr_scm_ndctl() in case of a PDSM request is received via ND_CMD_CALL
command from libnvdimm.
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: "Aneesh Kumar K . V" <aneesh.kumar@linux.ibm.com>
Signed-off-by: Vaibhav Jain <vaibhav@linux.ibm.com>
---
Changelog
v5..v6 :
* Changed the usage of the term DSM to PDSM to distinguish it from the
ACPI term [ Dan Williams ]
* Renamed papr_scm_dsm.h to papr_scm_pdsm.h and updated various struct
to reflect the new terminology.
* Updated the patch description and title to reflect the new terminology.
* Squashed patch to introduce new command family in 'ndctl.h' with
this patch [ Dan Williams ]
* Updated the papr_scm_pdsm method starting index from 0x10000 to 0x0
[ Dan Williams ]
* Removed redundant license text from the papr_scm_psdm.h file.
[ Dan Williams ]
* s/envelop/envelope/ at various places [ Dan Williams ]
* Added '__packed' attribute to command package header to gaurd
against different compiler adding paddings between the fields.
[ Dan Williams]
* Converted various pr_debug to dev_debug [ Dan Williams ]
v4..v5 :
* None
v3..v4 :
* None
v2..v3 :
* Updated the patch prefix to 'ndctl/uapi' [Aneesh]
v1..v2 :
* None
---
arch/powerpc/include/uapi/asm/papr_scm_pdsm.h | 153 ++++++++++++++++++
arch/powerpc/platforms/pseries/papr_scm.c | 101 +++++++++++-
include/uapi/linux/ndctl.h | 1 +
3 files changed, 249 insertions(+), 6 deletions(-)
create mode 100644 arch/powerpc/include/uapi/asm/papr_scm_pdsm.h
diff --git a/arch/powerpc/include/uapi/asm/papr_scm_pdsm.h b/arch/powerpc/include/uapi/asm/papr_scm_pdsm.h
new file mode 100644
index 000000000000..ec48b5c7fc18
--- /dev/null
+++ b/arch/powerpc/include/uapi/asm/papr_scm_pdsm.h
@@ -0,0 +1,153 @@
+/* SPDX-License-Identifier: GPL-2.0+ WITH Linux-syscall-note */
+/*
+ * PAPR-SCM Dimm specific methods (PDSM) and structs for libndctl
+ *
+ * (C) Copyright IBM 2020
+ *
+ * Author: Vaibhav Jain <vaibhav at linux.ibm.com>
+ */
+
+#ifndef _UAPI_ASM_POWERPC_PAPR_SCM_PDSM_H_
+#define _UAPI_ASM_POWERPC_PAPR_SCM_PDSM_H_
+
+#include <linux/types.h>
+
+#ifdef __KERNEL__
+#include <linux/ndctl.h>
+#else
+#include <ndctl.h>
+#endif
+
+/*
+ * PDSM Envelope:
+ *
+ * The ioctl ND_CMD_CALL transfers data between user-space and kernel via
+ * 'envelopes' which consists of a header and user-defined payload sections.
+ * The header is described by 'struct nd_pdsm_cmd_pkg' which expects a
+ * payload following it and offset of which relative to the struct is provided
+ * by 'nd_pdsm_cmd_pkg.payload_offset'. *
+ *
+ * +-------------+---------------------+---------------------------+
+ * | 64-Bytes | 8-Bytes | Max 184-Bytes |
+ * +-------------+---------------------+---------------------------+
+ * | nd_pdsm_cmd_pkg | |
+ * |-------------+ | |
+ * | nd_cmd_pkg | | |
+ * +-------------+---------------------+---------------------------+
+ * | nd_family | | |
+ * | nd_size_out | cmd_status | |
+ * | nd_size_in | payload_version | PAYLOAD |
+ * | nd_command | payload_offset -----> |
+ * | nd_fw_size | | |
+ * +-------------+---------------------+---------------------------+
+ *
+ * PDSM Header:
+ *
+ * The header is defined as 'struct nd_pdsm_cmd_pkg' which embeds a
+ * 'struct nd_cmd_pkg' instance. The PDSM command is assigned to member
+ * 'nd_cmd_pkg.nd_command'. Apart from size information of the envelope which is
+ * contained in 'struct nd_cmd_pkg', the header also has members following
+ * members:
+ *
+ * 'cmd_status' : (Out) Errors if any encountered while servicing PDSM.
+ * 'payload_version' : (In/Out) Version number associated with the payload.
+ * 'payload_offset' : (In)Relative offset of payload from start of envelope.
+ *
+ * PDSM Payload:
+ *
+ * The layout of the PDSM Payload is defined by various structs shared between
+ * papr_scm and libndctl so that contents of payload can be interpreted. During
+ * servicing of a PDSM the papr_scm module will read input args from the payload
+ * field by casting its contents to an appropriate struct pointer based on the
+ * PDSM command. Similarly the output of servicing the PDSM command will be
+ * copied to the payload field using the same struct.
+ *
+ * 'libnvdimm' enforces a hard limit of 256 bytes on the envelope size, which
+ * leaves around 184 bytes for the envelope payload (ignoring any padding that
+ * the compiler may silently introduce).
+ *
+ * Payload Version:
+ *
+ * A 'payload_version' field is present in PDSM header that indicates a specific
+ * version of the structure present in PDSM Payload for a given PDSM command.
+ * This provides backward compatibility in case the PDSM Payload structure
+ * evolves and different structures are supported by 'papr_scm' and 'libndctl'.
+ *
+ * When sending a PDSM Payload to 'papr_scm', 'libndctl' should send the version
+ * of the payload struct it supports via 'payload_version' field. The 'papr_scm'
+ * module when servicing the PDSM envelope checks the 'payload_version' and then
+ * uses 'payload struct version' == MIN('payload_version field',
+ * 'max payload-struct-version supported by papr_scm') to service the PDSM.
+ * After servicing the PDSM, 'papr_scm' put the negotiated version of payload
+ * struct in returned 'payload_version' field.
+ *
+ * Libndctl on receiving the envelope back from papr_scm again checks the
+ * 'payload_version' field and based on it use the appropriate version dsm
+ * struct to parse the results.
+ *
+ * Backward Compatibility:
+ *
+ * Above scheme of exchanging different versioned PDSM struct between libndctl
+ * and papr_scm should provide backward compatibility until following two
+ * assumptions/conditions when defining new PDSM structs hold:
+ *
+ * Let T(X) = { set of attributes in PDSM struct 'T' versioned X }
+ *
+ * 1. T(X) is a proper subset of T(Y) if X > Y.
+ * i.e Each new version of PDSM struct should retain existing struct
+ * attributes from previous version
+ *
+ * 2. If an entity (libndctl or papr_scm) supports a PDSM struct T(X) then
+ * it should also support T(1), T(2)...T(X - 1).
+ * i.e When adding support for new version of a PDSM struct, libndctl
+ * and papr_scm should retain support of the existing PDSM struct
+ * version they support.
+ */
+
+#ifndef __packed
+#define __packed __attribute__((packed))
+#endif
+
+/* Papr-scm-header + payload expected with ND_CMD_CALL ioctl from libnvdimm */
+struct nd_pdsm_cmd_pkg {
+ struct nd_cmd_pkg hdr; /* Package header containing sub-cmd */
+ __s32 cmd_status; /* Out: Sub-cmd status returned back */
+ __u16 payload_offset; /* In: offset from start of struct */
+ __u16 payload_version; /* In/Out: version of the payload */
+ __u8 payload[]; /* In/Out: Sub-cmd data buffer */
+} __packed;
+
+/*
+ * Methods to be embedded in ND_CMD_CALL request. These are sent to the kernel
+ * via 'nd_pdsm_cmd_pkg.hdr.nd_command' member of the ioctl struct
+ */
+enum papr_scm_pdsm {
+ PAPR_SCM_PDSM_MIN = 0x0,
+ PAPR_SCM_PDSM_MAX,
+};
+
+/* Helpers to evaluate the size of PDSM envelope */
+/* Calculate the papr_scm-header size */
+#define ND_PDSM_ENVELOPE_CONTENT_HDR_SIZE \
+ (sizeof(struct nd_pdsm_cmd_pkg) - sizeof(struct nd_cmd_pkg))
+
+/* Given a type calculate envelope-content size (papr_scm-header + payload) */
+#define ND_PDSM_ENVELOPE_CONTENT_SIZE(_type_) \
+ (sizeof(_type_) + ND_PDSM_ENVELOPE_CONTENT_HDR_SIZE)
+
+/* Convert a libnvdimm nd_cmd_pkg to pdsm specific pkg */
+static struct nd_pdsm_cmd_pkg *nd_to_pdsm_cmd_pkg(struct nd_cmd_pkg *cmd)
+{
+ return (struct nd_pdsm_cmd_pkg *) cmd;
+}
+
+/* Return the payload pointer for a given pcmd */
+static void *pdsm_cmd_to_payload(struct nd_pdsm_cmd_pkg *pcmd)
+{
+ if (pcmd->hdr.nd_size_in == 0 && pcmd->hdr.nd_size_out == 0)
+ return NULL;
+ else
+ return (void *)((__u8 *) pcmd + pcmd->payload_offset);
+}
+
+#endif /* _UAPI_ASM_POWERPC_PAPR_SCM_PDSM_H_ */
diff --git a/arch/powerpc/platforms/pseries/papr_scm.c b/arch/powerpc/platforms/pseries/papr_scm.c
index f8fe579e6f2e..20da1b837017 100644
--- a/arch/powerpc/platforms/pseries/papr_scm.c
+++ b/arch/powerpc/platforms/pseries/papr_scm.c
@@ -15,13 +15,15 @@
#include <asm/plpar_wrappers.h>
#include <asm/papr_scm.h>
+#include <asm/papr_scm_pdsm.h>
#define BIND_ANY_ADDR (~0ul)
#define PAPR_SCM_DIMM_CMD_MASK \
((1ul << ND_CMD_GET_CONFIG_SIZE) | \
(1ul << ND_CMD_GET_CONFIG_DATA) | \
- (1ul << ND_CMD_SET_CONFIG_DATA))
+ (1ul << ND_CMD_SET_CONFIG_DATA) | \
+ (1ul << ND_CMD_CALL))
struct papr_scm_priv {
struct platform_device *pdev;
@@ -306,16 +308,97 @@ static int papr_scm_meta_set(struct papr_scm_priv *p,
return 0;
}
-int papr_scm_ndctl(struct nvdimm_bus_descriptor *nd_desc, struct nvdimm *nvdimm,
- unsigned int cmd, void *buf, unsigned int buf_len, int *cmd_rc)
+/*
+ * Validate the inputs args to dimm-control function and return '0' if valid.
+ * This also does initial sanity validation to ND_CMD_CALL sub-command packages.
+ */
+static int is_cmd_valid(struct nvdimm *nvdimm, unsigned int cmd, void *buf,
+ unsigned int buf_len)
{
- struct nd_cmd_get_config_size *get_size_hdr;
+ unsigned long cmd_mask = PAPR_SCM_DIMM_CMD_MASK;
+ struct nd_pdsm_cmd_pkg *pkg = nd_to_pdsm_cmd_pkg(buf);
struct papr_scm_priv *p;
/* Only dimm-specific calls are supported atm */
if (!nvdimm)
return -EINVAL;
+ /* get the provider date from struct nvdimm */
+ p = nvdimm_provider_data(nvdimm);
+
+ if (!test_bit(cmd, &cmd_mask)) {
+ dev_dbg(&p->pdev->dev, "Unsupported cmd=%u\n", cmd);
+ return -EINVAL;
+ } else if (cmd == ND_CMD_CALL) {
+
+ /* Verify the envelope package */
+ if (!buf || buf_len < sizeof(struct nd_pdsm_cmd_pkg)) {
+ dev_dbg(&p->pdev->dev, "Invalid pkg size=%u\n",
+ buf_len);
+ return -EINVAL;
+ }
+
+ /* Verify that the PDSM family is valid */
+ if (pkg->hdr.nd_family != NVDIMM_FAMILY_PAPR_SCM) {
+ dev_dbg(&p->pdev->dev, "Invalid pkg family=0x%llx\n",
+ pkg->hdr.nd_family);
+ return -EINVAL;
+
+ }
+
+ /* We except a payload with all PDSM commands */
+ if (pdsm_cmd_to_payload(pkg) == NULL) {
+ dev_dbg(&p->pdev->dev,
+ "Empty payload for sub-command=0x%llx\n",
+ pkg->hdr.nd_command);
+ return -EINVAL;
+ }
+ }
+
+ /* Command looks valid */
+ return 0;
+}
+
+static int papr_scm_service_pdsm(struct papr_scm_priv *p,
+ struct nd_pdsm_cmd_pkg *call_pkg)
+{
+ /* unknown subcommands return error in packages */
+ if (call_pkg->hdr.nd_command <= PAPR_SCM_PDSM_MIN ||
+ call_pkg->hdr.nd_command >= PAPR_SCM_PDSM_MAX) {
+ dev_dbg(&p->pdev->dev, "Invalid PDSM request 0x%llx\n",
+ call_pkg->hdr.nd_command);
+ call_pkg->cmd_status = -EINVAL;
+ return 0;
+ }
+
+ /* Depending on the DSM command call appropriate service routine */
+ switch (call_pkg->hdr.nd_command) {
+ default:
+ dev_dbg(&p->pdev->dev, "Unsupported PDSM request 0x%llx\n",
+ call_pkg->hdr.nd_command);
+ call_pkg->cmd_status = -ENOENT;
+ return 0;
+ }
+}
+
+int papr_scm_ndctl(struct nvdimm_bus_descriptor *nd_desc, struct nvdimm *nvdimm,
+ unsigned int cmd, void *buf, unsigned int buf_len, int *cmd_rc)
+{
+ struct nd_cmd_get_config_size *get_size_hdr;
+ struct papr_scm_priv *p;
+ struct nd_pdsm_cmd_pkg *call_pkg = NULL;
+ int rc;
+
+ /* Use a local variable in case cmd_rc pointer is NULL */
+ if (cmd_rc == NULL)
+ cmd_rc = &rc;
+
+ *cmd_rc = is_cmd_valid(nvdimm, cmd, buf, buf_len);
+ if (*cmd_rc) {
+ pr_debug("Invalid cmd=0x%x. Err=%d\n", cmd, *cmd_rc);
+ return *cmd_rc;
+ }
+
p = nvdimm_provider_data(nvdimm);
switch (cmd) {
@@ -336,13 +419,19 @@ int papr_scm_ndctl(struct nvdimm_bus_descriptor *nd_desc, struct nvdimm *nvdimm,
*cmd_rc = papr_scm_meta_set(p, buf);
break;
+ case ND_CMD_CALL:
+ call_pkg = nd_to_pdsm_cmd_pkg(buf);
+ *cmd_rc = papr_scm_service_pdsm(p, call_pkg);
+ break;
+
default:
- return -EINVAL;
+ dev_dbg(&p->pdev->dev, "Unknown command = %d\n", cmd);
+ *cmd_rc = -EINVAL;
}
dev_dbg(&p->pdev->dev, "returned with cmd_rc = %d\n", *cmd_rc);
- return 0;
+ return *cmd_rc;
}
static inline int papr_scm_node(int node)
diff --git a/include/uapi/linux/ndctl.h b/include/uapi/linux/ndctl.h
index de5d90212409..99fb60600ef8 100644
--- a/include/uapi/linux/ndctl.h
+++ b/include/uapi/linux/ndctl.h
@@ -244,6 +244,7 @@ struct nd_cmd_pkg {
#define NVDIMM_FAMILY_HPE2 2
#define NVDIMM_FAMILY_MSFT 3
#define NVDIMM_FAMILY_HYPERV 4
+#define NVDIMM_FAMILY_PAPR_SCM 5
#define ND_IOCTL_CALL _IOWR(ND_IOCTL, ND_CMD_CALL,\
struct nd_cmd_pkg)
--
2.25.3
^ permalink raw reply related
* [PATCH v6 1/4] powerpc: Document details on H_SCM_HEALTH hcall
From: Vaibhav Jain @ 2020-04-20 7:07 UTC (permalink / raw)
To: linuxppc-dev, linux-nvdimm
Cc: Santosh Sivaraj, Oliver O'Halloran, Aneesh Kumar K . V,
Vaibhav Jain, Dan Williams
In-Reply-To: <20200420070711.223545-1-vaibhav@linux.ibm.com>
Add documentation to 'papr_hcalls.rst' describing the bitmap flags
that are returned from H_SCM_HEALTH hcall as per the PAPR-SCM
specification.
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: "Aneesh Kumar K . V" <aneesh.kumar@linux.ibm.com>
Signed-off-by: Vaibhav Jain <vaibhav@linux.ibm.com>
---
Changelog:
v5..v6
* New patch in the series
---
Documentation/powerpc/papr_hcalls.rst | 43 ++++++++++++++++++++++++---
1 file changed, 39 insertions(+), 4 deletions(-)
diff --git a/Documentation/powerpc/papr_hcalls.rst b/Documentation/powerpc/papr_hcalls.rst
index 3493631a60f8..9a5ba5eaf323 100644
--- a/Documentation/powerpc/papr_hcalls.rst
+++ b/Documentation/powerpc/papr_hcalls.rst
@@ -220,13 +220,48 @@ from the LPAR memory.
**H_SCM_HEALTH**
| Input: drcIndex
-| Out: *health-bitmap, health-bit-valid-bitmap*
+| Out: *health-bitmap (r4), health-bit-valid-bitmap (r5)*
| Return Value: *H_Success, H_Parameter, H_Hardware*
Given a DRC Index return the info on predictive failure and overall health of
-the NVDIMM. The asserted bits in the health-bitmap indicate a single predictive
-failure and health-bit-valid-bitmap indicate which bits in health-bitmap are
-valid.
+the NVDIMM. The asserted bits in the health-bitmap indicate one or more states
+(described in table below) of the NVDIMM and health-bit-valid-bitmap indicate
+which bits in health-bitmap are valid.
+
+Health Bitmap Flags:
+
++------+-----------------------------------------------------------------------+
+| Bit | Definition |
++======+=======================================================================+
+| 00 | SCM device is unable to persist memory contents. |
+| | If the system is powered down, nothing will be saved. |
++------+-----------------------------------------------------------------------+
+| 01 | SCM device failed to persist memory contents. Either contents were not|
+| | saved successfully on power down or were not restored properly on |
+| | power up. |
++------+-----------------------------------------------------------------------+
+| 02 | SCM device contents are persisted from previous IPL. The data from |
+| | the last boot were successfully restored. |
++------+-----------------------------------------------------------------------+
+| 03 | SCM device contents are not persisted from previous IPL. There was no |
+| | data to restore from the last boot. |
++------+-----------------------------------------------------------------------+
+| 04 | SCM device memory life remaining is critically low |
++------+-----------------------------------------------------------------------+
+| 05 | SCM device will be garded off next IPL due to failure |
++------+-----------------------------------------------------------------------+
+| 06 | SCM contents cannot persist due to current platform health status. A |
+| | hardware failure may prevent data from being saved or restored. |
++------+-----------------------------------------------------------------------+
+| 07 | SCM device is unable to persist memory contents in certain conditions |
++------+-----------------------------------------------------------------------+
+| 08 | SCM device is encrypted |
++------+-----------------------------------------------------------------------+
+| 09 | SCM device has successfully completed a requested erase or secure |
+| | erase procedure. |
++------+-----------------------------------------------------------------------+
+|10:63 | Reserved / Unused |
++------+-----------------------------------------------------------------------+
**H_SCM_PERFORMANCE_STATS**
--
2.25.3
^ permalink raw reply related
* [PATCH v6 2/4] powerpc/papr_scm: Fetch nvdimm health information from PHYP
From: Vaibhav Jain @ 2020-04-20 7:07 UTC (permalink / raw)
To: linuxppc-dev, linux-nvdimm
Cc: Santosh Sivaraj, Oliver O'Halloran, Aneesh Kumar K . V,
Vaibhav Jain, Dan Williams
In-Reply-To: <20200420070711.223545-1-vaibhav@linux.ibm.com>
Implement support for fetching nvdimm health information via
H_SCM_HEALTH hcall as documented in Ref[1]. The hcall returns a pair
of 64-bit big-endian integers, bitwise-and of which is then stored in
'struct papr_scm_priv' and subsequently partially exposed to
user-space via newly introduced dimm specific attribute
'papr/flags'. Since the hcall is costly, the health information is
cached and only re-queried, 60s after the previous successful hcall.
The patch also adds a new asm header named 'papr_scm.h' describing the
interface between PHYP and guest kernel. A documentation text
describing flags reported by the the new sysfs attribute 'papr/flags'
is also introduced at Documentation/ABI/testing/sysfs-bus-papr-scm.
[1] commit 58b278f568f0 ("powerpc: Provide initial documentation for
PAPR hcalls")
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: "Aneesh Kumar K . V" <aneesh.kumar@linux.ibm.com>
Signed-off-by: Vaibhav Jain <vaibhav@linux.ibm.com>
---
Changelog
v5..v6 :
* Change the flags sysfs attribute from 'papr_flags' to 'papr/flags'
[Dan Williams]
* Include documentation for 'papr/flags' attr [Dan Williams]
* Change flag 'save_fail' to 'flush_fail' [Dan Williams]
* Caching of health bitmap to reduce expensive hcalls [Dan Williams]
* Removed usage of PPC_BIT from 'papr-scm.h' header [Mpe]
* Replaced two __be64 integers from papr_scm_priv to a single u64
integer [Mpe]
* Updated patch description to reflect the changes made in this
version.
* Removed avoidable usage of 'papr_scm_priv.dimm_mutex' from
flags_show() [Dan Williams]
v4..v5 :
* None
v3..v4 :
* None
v2..v3 :
* Removed PAPR_SCM_DIMM_HEALTH_NON_CRITICAL as a condition for
NVDIMM unarmed [Aneesh]
v1..v2 :
* New patch in the series.
---
Documentation/ABI/testing/sysfs-bus-papr-scm | 27 ++++
arch/powerpc/include/asm/papr_scm.h | 49 ++++++++
arch/powerpc/platforms/pseries/papr_scm.c | 126 ++++++++++++++++++-
3 files changed, 200 insertions(+), 2 deletions(-)
create mode 100644 Documentation/ABI/testing/sysfs-bus-papr-scm
create mode 100644 arch/powerpc/include/asm/papr_scm.h
diff --git a/Documentation/ABI/testing/sysfs-bus-papr-scm b/Documentation/ABI/testing/sysfs-bus-papr-scm
new file mode 100644
index 000000000000..001e4d34ab5c
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-bus-papr-scm
@@ -0,0 +1,27 @@
+What: /sys/bus/nd/devices/nmemX/papr/flags
+Date: Apr, 2020
+KernelVersion: v5.8
+Contact: linuxppc-dev <linuxppc-dev@lists.ozlabs.org>, linux-nvdimm@lists.01.org,
+Description:
+ (RO) Report flags indicating various states of a
+ papr-scm NVDIMM device. Each flag maps to a one or
+ more bits set in the dimm-health-bitmap retrieved in
+ response to H_SCM_HEALTH hcall. The details of the bit
+ flags returned in response to this hcall is available
+ at 'Documentation/powerpc/papr_hcalls.rst' . Below are
+ the flags reported in this sysfs file:
+
+ * "not_armed" : Indicating that nvdimm contents will not
+ survive a power cycle.
+ * "flush_fail" : Indicating that nvdimm contents
+ couldn't be flushed during last
+ shutdown event.
+ * "restore_fail": Indicating that nvdimm contents
+ couldn't be restored during dimm
+ initialization.
+ * "encrypted" : Dimm contents are encrypted.
+ * "smart_notify": There is health event for the nvdimm.
+ * "scrubbed" : Indicating that contents of the
+ nvdimm have been scrubbed.
+ * "locked" : Indicating that nvdimm contents cant
+ be modified until next power cycle.
diff --git a/arch/powerpc/include/asm/papr_scm.h b/arch/powerpc/include/asm/papr_scm.h
new file mode 100644
index 000000000000..b51c048e906a
--- /dev/null
+++ b/arch/powerpc/include/asm/papr_scm.h
@@ -0,0 +1,49 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Structures and defines needed to manage nvdimms for spapr guests.
+ */
+#ifndef _ASM_POWERPC_PAPR_SCM_H_
+#define _ASM_POWERPC_PAPR_SCM_H_
+
+#include <linux/types.h>
+#include <asm/bitsperlong.h>
+
+/* DIMM health bitmap bitmap indicators */
+
+/* SCM device is unable to persist memory contents */
+#define PAPR_SCM_DIMM_UNARMED (1ULL << (63 - 0))
+/* SCM device failed to persist memory contents */
+#define PAPR_SCM_DIMM_SHUTDOWN_DIRTY (1ULL << (63 - 1))
+/* SCM device contents are persisted from previous IPL */
+#define PAPR_SCM_DIMM_SHUTDOWN_CLEAN (1ULL << (63 - 2))
+/* SCM device contents are not persisted from previous IPL */
+#define PAPR_SCM_DIMM_EMPTY (1ULL << (63 - 3))
+/* SCM device memory life remaining is critically low */
+#define PAPR_SCM_DIMM_HEALTH_CRITICAL (1ULL << (63 - 4))
+/* SCM device will be garded off next IPL due to failure */
+#define PAPR_SCM_DIMM_HEALTH_FATAL (1ULL << (63 - 5))
+/* SCM contents cannot persist due to current platform health status */
+#define PAPR_SCM_DIMM_HEALTH_UNHEALTHY (1ULL << (63 - 6))
+/* SCM device is unable to persist memory contents in certain conditions */
+#define PAPR_SCM_DIMM_HEALTH_NON_CRITICAL (1ULL << (63 - 7))
+/* SCM device is encrypted */
+#define PAPR_SCM_DIMM_ENCRYPTED (1ULL << (63 - 8))
+/* SCM device has been scrubbed and locked */
+#define PAPR_SCM_DIMM_SCRUBBED_AND_LOCKED (1ULL << (63 - 9))
+
+/* Bits status indicators for health bitmap indicating unarmed dimm */
+#define PAPR_SCM_DIMM_UNARMED_MASK (PAPR_SCM_DIMM_UNARMED | \
+ PAPR_SCM_DIMM_HEALTH_UNHEALTHY)
+
+/* Bits status indicators for health bitmap indicating unflushed dimm */
+#define PAPR_SCM_DIMM_BAD_SHUTDOWN_MASK (PAPR_SCM_DIMM_SHUTDOWN_DIRTY)
+
+/* Bits status indicators for health bitmap indicating unrestored dimm */
+#define PAPR_SCM_DIMM_BAD_RESTORE_MASK (PAPR_SCM_DIMM_EMPTY)
+
+/* Bit status indicators for smart event notification */
+#define PAPR_SCM_DIMM_SMART_EVENT_MASK (PAPR_SCM_DIMM_HEALTH_CRITICAL | \
+ PAPR_SCM_DIMM_HEALTH_FATAL | \
+ PAPR_SCM_DIMM_HEALTH_UNHEALTHY)
+
+#endif
diff --git a/arch/powerpc/platforms/pseries/papr_scm.c b/arch/powerpc/platforms/pseries/papr_scm.c
index 0b4467e378e5..f8fe579e6f2e 100644
--- a/arch/powerpc/platforms/pseries/papr_scm.c
+++ b/arch/powerpc/platforms/pseries/papr_scm.c
@@ -14,6 +14,7 @@
#include <linux/delay.h>
#include <asm/plpar_wrappers.h>
+#include <asm/papr_scm.h>
#define BIND_ANY_ADDR (~0ul)
@@ -39,6 +40,15 @@ struct papr_scm_priv {
struct resource res;
struct nd_region *region;
struct nd_interleave_set nd_set;
+
+ /* Protect dimm health data from concurrent read/writes */
+ struct mutex dimm_mutex;
+
+ /* Last time the health information of the dimm was updated */
+ unsigned long lasthealth_jiffies;
+
+ /* Health information for the dimm */
+ u64 health_bitmap;
};
static int drc_pmem_bind(struct papr_scm_priv *p)
@@ -144,6 +154,56 @@ static int drc_pmem_query_n_bind(struct papr_scm_priv *p)
return drc_pmem_bind(p);
}
+/* Min interval in seconds between successive H_SCM_HEALTH hcalls */
+#define MIN_HEALTH_QUERY_INTERVAL 60
+
+/*
+ * Issue hcall if needed to retrieve dimm health info. Information is cached
+ * and subsequent calls may return success without issueing the hcall.
+ * Use 'force == true' to force issue of the hcall ignoring the cache
+ * timeout.
+ */
+static int drc_pmem_query_health(struct papr_scm_priv *p, bool force)
+{
+ unsigned long ret[PLPAR_HCALL_BUFSIZE];
+ s64 rc;
+ unsigned long cache_timeout;
+
+ /* Protect concurrent modifications to papr_scm_priv */
+ rc = mutex_lock_interruptible(&p->dimm_mutex);
+ if (rc)
+ return rc;
+
+ /* Jiffies offset for which the health data is assumed to be same */
+ cache_timeout = p->lasthealth_jiffies +
+ msecs_to_jiffies(MIN_HEALTH_QUERY_INTERVAL * 1000);
+
+ /* Dont issue the hcall if health information is relatively new */
+ if (!force && time_after(cache_timeout, jiffies)) {
+ rc = 0;
+ goto out;
+ }
+
+ /* issue the hcall */
+ rc = plpar_hcall(H_SCM_HEALTH, ret, p->drc_index);
+ if (rc != H_SUCCESS) {
+ dev_err(&p->pdev->dev,
+ "Failed to query health information, Err:%lld\n", rc);
+ rc = -ENXIO;
+ goto out;
+ }
+
+ p->lasthealth_jiffies = jiffies;
+ p->health_bitmap = ret[0] & ret[1];
+
+ dev_dbg(&p->pdev->dev,
+ "Queried dimm health info. Bitmap:0x%016lx Mask:0x%016lx\n",
+ ret[0], ret[1]);
+
+out:
+ mutex_unlock(&p->dimm_mutex);
+ return rc;
+}
static int papr_scm_meta_get(struct papr_scm_priv *p,
struct nd_cmd_get_config_data_hdr *hdr)
@@ -304,6 +364,65 @@ static inline int papr_scm_node(int node)
return min_node;
}
+static ssize_t flags_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct nvdimm *dimm = to_nvdimm(dev);
+ struct papr_scm_priv *p = nvdimm_provider_data(dimm);
+ int rc;
+ u64 health;
+
+ rc = drc_pmem_query_health(p, false);
+ if (rc)
+ return rc;
+
+ /*
+ * Copy the LE byte-ordered health_bitmap locally, check for various
+ * masks and update the sysfs out buffer.
+ */
+ health = p->health_bitmap;
+
+ if (health & PAPR_SCM_DIMM_UNARMED_MASK)
+ rc += sprintf(buf, "not_armed ");
+
+ if (health & PAPR_SCM_DIMM_BAD_SHUTDOWN_MASK)
+ rc += sprintf(buf + rc, "save_fail ");
+
+ if (health & PAPR_SCM_DIMM_BAD_RESTORE_MASK)
+ rc += sprintf(buf + rc, "restore_fail ");
+
+ if (health & PAPR_SCM_DIMM_ENCRYPTED)
+ rc += sprintf(buf + rc, "encrypted ");
+
+ if (health & PAPR_SCM_DIMM_SMART_EVENT_MASK)
+ rc += sprintf(buf + rc, "smart_notify ");
+
+ if (health & PAPR_SCM_DIMM_SCRUBBED_AND_LOCKED)
+ rc += sprintf(buf + rc, "scrubbed locked ");
+
+ if (rc > 0)
+ rc += sprintf(buf + rc, "\n");
+
+ return rc;
+}
+DEVICE_ATTR_RO(flags);
+
+/* papr_scm specific dimm attributes */
+static struct attribute *papr_scm_nd_attributes[] = {
+ &dev_attr_flags.attr,
+ NULL,
+};
+
+static struct attribute_group papr_scm_nd_attribute_group = {
+ .name = "papr",
+ .attrs = papr_scm_nd_attributes,
+};
+
+static const struct attribute_group *papr_scm_dimm_attr_groups[] = {
+ &papr_scm_nd_attribute_group,
+ NULL,
+};
+
static int papr_scm_nvdimm_init(struct papr_scm_priv *p)
{
struct device *dev = &p->pdev->dev;
@@ -330,8 +449,8 @@ static int papr_scm_nvdimm_init(struct papr_scm_priv *p)
dimm_flags = 0;
set_bit(NDD_ALIASING, &dimm_flags);
- p->nvdimm = nvdimm_create(p->bus, p, NULL, dimm_flags,
- PAPR_SCM_DIMM_CMD_MASK, 0, NULL);
+ p->nvdimm = nvdimm_create(p->bus, p, papr_scm_dimm_attr_groups,
+ dimm_flags, PAPR_SCM_DIMM_CMD_MASK, 0, NULL);
if (!p->nvdimm) {
dev_err(dev, "Error creating DIMM object for %pOF\n", p->dn);
goto err;
@@ -415,6 +534,9 @@ static int papr_scm_probe(struct platform_device *pdev)
if (!p)
return -ENOMEM;
+ /* Initialize the dimm mutex */
+ mutex_init(&p->dimm_mutex);
+
/* optional DT properties */
of_property_read_u32(dn, "ibm,metadata-size", &metadata_size);
--
2.25.3
^ permalink raw reply related
* [PATCH v6 0/4] powerpc/papr_scm: Add support for reporting nvdimm health
From: Vaibhav Jain @ 2020-04-20 7:07 UTC (permalink / raw)
To: linuxppc-dev, linux-nvdimm
Cc: Vaibhav Jain, Santosh Sivaraj, Oliver O'Halloran,
Aneesh Kumar K . V
The PAPR standard[1][3] provides mechanisms to query the health and
performance stats of an NVDIMM via various hcalls as described in
Ref[2]. Until now these stats were never available nor exposed to the
user-space tools like 'ndctl'. This is partly due to PAPR platform not
having support for ACPI and NFIT. Hence 'ndctl' is unable to query and
report the dimm health status and a user had no way to determine the
current health status of a NDVIMM.
To overcome this limitation, this patch-set updates papr_scm kernel
module to query and fetch nvdimm health stats using hcalls described
in Ref[2]. This health and performance stats are then exposed to
userspace via syfs and PAPR-nvDimm-Specific-Methods(PDSM) issued by
libndctl.
These changes coupled with proposed ndtcl changes located at Ref[4]
should provide a way for the user to retrieve NVDIMM health status
using ndtcl.
Below is a sample output using proposed kernel + ndctl for PAPR NVDIMM
in a emulation environment:
# ndctl list -DH
[
{
"dev":"nmem0",
"health":{
"health_state":"fatal",
"shutdown_state":"dirty"
}
}
]
Dimm health report output on a pseries guest lpar with vPMEM or HMS
based nvdimms that are in perfectly healthy conditions:
# ndctl list -d nmem0 -H
[
{
"dev":"nmem0",
"health":{
"health_state":"ok",
"shutdown_state":"clean"
}
}
]
PAPR nvDimm-Specific-Methods(PDSM)
==================================
PDSM requests are issued by vendor specific code in libndctl to
execute certain operations or fetch information from NVDIMMS. PDSMs
requests can be sent to papr_scm module via libndctl(userspace) and
libnvdimm (kernel) using the ND_CMD_CALL ioctl command which can be
handled in the dimm control function papr_scm_ndctl(). Current
patchset proposes a single PDSM to retrieve NVDIMM health, defined in
the newly introduced uapi header named 'papr_scm_pdsm.h'. Support for
more PDSMs will be added in future.
Structure of the patch-set
==========================
The patchset starts with implementing support for fetching nvdimm health
information from PHYP and partially exposing it to user-space via a nvdimm
sysfs flag.
Second & Third patches deal with implementing support for servicing PDSM
commands in papr_scm module.
Finally the Fourth patch implements support for servicing PDSM
'PAPR_SCM_PDSM_HEALTH' that returns the nvdimm health information to
libndctl.
Changelog:
==========
v5..v6:
* Incorporate review comments from Mpe and Dan Williams.
* Changed the usage of term DSM to PDSM as former conflicted with
usage in ACPI context.
* UAPI updates to remove usage of bool and marking the structs
defined as 'packed'.
* Simplified the health-bitmap handling in papr_scm to use u64
instead of __be64 integers.
* Caching of the health information so reading the dimm-flag file
doesn't result in costly hcalls everytime.
* Changed dimm-flag 'save_fail' to 'flush_fail'
* Moved the dimm flag file from 'papr_flags' to 'papr/flags'.
* Added a patch to document H_SCM_HEALTH hcall return values.
* Added sysfs ABI documentation for newly introduce dimm-flag
sysfs file 'papr/flags'
v4..v5:
* Fixed a bug in new implementation of papr_scm_ndctl() that was triggering
a false error condition.
v3..v4:
* Restructured papr_scm_ndctl() to dispatch ND_CMD_CALL commands to a new
function named papr_scm_service_dsm() to serivice DSM requests. [Aneesh]
v2..v3:
* Updated the papr_scm_dsm.h header to be more confimant general kernel
guidelines for UAPI headers. [Aneesh]
* Changed the definition of macro PAPR_SCM_DIMM_UNARMED_MASK to not
include case when the nvdimm is unarmed because its a vPMEM
nvdimm. [Aneesh]
v1..v2:
* Restructured the patch-set based on review comments on V1 patch-set to
simplify the patch review. Multiple small patches have been combined into
single patches to reduce cross referencing that was needed in earlier
patch-set. Hence most of the patches in this patch-set as now new. [Aneesh]
* Removed the initial work done for fetch nvdimm performance statistics.
These changes will be re-proposed in a separate patch-set. [Aneesh]
* Simplified handling of versioning of 'struct
nd_papr_scm_dimm_health_stat_v1' as only one version of the structure is
currently in existence.
References:
[1] "Power Architecture Platform Reference"
https://en.wikipedia.org/wiki/Power_Architecture_Platform_Reference
[2] commit 58b278f568f0
("powerpc: Provide initial documentation for PAPR hcalls")
[3] "Linux on Power Architecture Platform Reference"
https://members.openpowerfoundation.org/document/dl/469
[4] https://patchwork.kernel.org/project/linux-nvdimm/list/?series=244625
Vaibhav Jain (4):
powerpc: Document details on H_SCM_HEALTH hcall
powerpc/papr_scm: Fetch nvdimm health information from PHYP
ndctl/papr_scm,uapi: Add support for PAPR nvdimm specific methods
powerpc/papr_scm: Implement support for PAPR_SCM_PDSM_HEALTH
Documentation/ABI/testing/sysfs-bus-papr-scm | 27 ++
Documentation/powerpc/papr_hcalls.rst | 43 ++-
arch/powerpc/include/asm/papr_scm.h | 49 +++
arch/powerpc/include/uapi/asm/papr_scm_pdsm.h | 192 +++++++++++
arch/powerpc/platforms/pseries/papr_scm.c | 317 +++++++++++++++++-
include/uapi/linux/ndctl.h | 1 +
6 files changed, 617 insertions(+), 12 deletions(-)
create mode 100644 Documentation/ABI/testing/sysfs-bus-papr-scm
create mode 100644 arch/powerpc/include/asm/papr_scm.h
create mode 100644 arch/powerpc/include/uapi/asm/papr_scm_pdsm.h
--
2.25.3
^ permalink raw reply
* Re: [PATCH] powerpc/book3s64/kuap: SPRN_AMR modification need CSI instructions before and after
From: Michael Ellerman @ 2020-04-20 7:04 UTC (permalink / raw)
To: Nicholas Piggin, Aneesh Kumar K.V, linuxppc-dev
In-Reply-To: <1587345091.ewst0wvt61.astroid@bobo.none>
Nicholas Piggin <npiggin@gmail.com> writes:
> Excerpts from Nicholas Piggin's message of April 20, 2020 10:17 am:
>> Excerpts from Aneesh Kumar K.V's message of April 19, 2020 11:53 pm:
>>> As per the ISA, context synchronizing instructions is needed before and after
>>> SPRN_AMR update. Use isync before and the CSI after is implied by the rfid
>>> that we will use to switch to a new context.
>>
>> Not entirely sure if we need this. This will restore AMR to more
>> permissive, so if it executes ahead of a stray load from this
>> context, it won't make it fault.
I thought we'd convinced ourselves it didn't matter in practice due to
the proximity of the entry/exit.
>> That said, leaving this end open makes it harder to reason about
>> user access protection I guess, so let's add it.
>
> We probably should test whether it needs updating, like the entry
> code does.
That will be the common case (no update), so yes I agree.
cheers
^ permalink raw reply
* Re: crash in cpuidle_enter_state with 5.7-rc1
From: Michal Suchánek @ 2020-04-20 6:50 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev
In-Reply-To: <871roiisxg.fsf@mpe.ellerman.id.au>
Hello,
On Mon, Apr 20, 2020 at 04:15:39PM +1000, Michael Ellerman wrote:
> Michal Suchánek <msuchanek@suse.de> writes:
> > Hello,
> >
> > I observe crash in cpuidle_enter_state in early boot on POWER9 pSeries
> > machine with 5.7-rc1 kernel. The crash is not 100% reliable. Sometimes
> > the machine boots.
> >
> > Attaching config, dmesg, and sample crash message. The stack below
> > cpuidle_enter_state appears random - different in each crash.
> >
> > Any idea what could cause this?
>
> Nothing immediately springs to mind.
>
> > Preparing to boot Linux version 5.7.0-rc1-1.g8f6a41f-default (geeko@buildhost) (gcc version 9.3.1 20200406 [revision 6db837a5288ee3ca5ec504fbd5a765817e556ac2] (SUSE Linux), GNU ld (GNU Binutils; openSUSE Tumbleweed) 2.34.0.20200325-1) #1 SMP Fri Apr 17 10:39:25 UTC 2020 (8f6a41f)
> > Detected machine type: 0000000000000101
> > command line: BOOT_IMAGE=/boot/vmlinux-5.7.0-rc1-1.g8f6a41f-default root=UUID=04f3f652-7c85-470b-9d5f-490601f371f8 mitigations=auto quiet crashkernel=242M
> > Max number of cores passed to firmware: 256 (NR_CPUS = 2048)
> > Calling ibm,client-architecture-support... done
> > memory layout at init:
> > memory_limit : 0000000000000000 (16 MB aligned)
> > alloc_bottom : 000000000e680000
> > alloc_top : 0000000020000000
> > alloc_top_hi : 0000000020000000
> > rmo_top : 0000000020000000
> > ram_top : 0000000020000000
> > instantiating rtas at 0x000000001ecb0000... done
> > prom_hold_cpus: skipped
> > copying OF device tree...
> > Building dt strings...
> > Building dt structure...
> > Device tree strings 0x000000000e690000 -> 0x000000000e691886
> > Device tree struct 0x000000000e6a0000 -> 0x000000000e6b0000
> > Quiescing Open Firmware ...
> > Booting Linux via __start() @ 0x000000000a6e0000 ...
> > [ 1.234639] BUG: Unable to handle kernel data access on read at 0xc0000000026970e0
> > [ 1.234654] Faulting instruction address: 0xc0000000000088dc
> > [ 1.234665] Oops: Kernel access of bad area, sig: 11 [#1]
> > [ 1.234675] LE PAGE_SIZE=64K MMU=Hash SMP NR_CPUS=2048 NUMA pSeries
> > [ 1.234686] Modules linked in:
> > [ 1.234698] CPU: 4 PID: 0 Comm: swapper/4 Not tainted 5.7.0-rc1-1.g8f6a41f-default #1 openSUSE Tumbleweed (unreleased)
> > [ 1.234714] NIP: c0000000000088dc LR: c000000000aad890 CTR: c0000000000087a0
> > [ 1.234727] REGS: c000000007596e90 TRAP: 0300 Not tainted (5.7.0-rc1-1.g8f6a41f-default)
> > [ 1.234742] MSR: 8000000000001033 <SF,ME,IR,DR,RI,LE> CR: 28000022 XER: 00000000
>
> MMU was on when we faulted (IR & DR), so it's not real mode weirdness.
>
> > [ 1.234760] CFAR: c0000000000087fc DAR: c0000000026970e0 DSISR: 40000000 IRQMASK: 0
> > [ 1.234760] GPR00: c000000000aa9384 c000000007597120 c00000000269f000 0000000000000000
> > [ 1.234760] GPR04: c0000000025d2778 0000000000000000 000000000000c800 c0000000026d69c0
> > [ 1.234760] GPR08: 000e98056e8436d9 0000000000000300 0000000000000000 0000000000000000
> > [ 1.234760] GPR12: 8000000000001033 c00000001ec79c00 0000000000000000 000000001ef3d880
> > [ 1.234760] GPR16: 0000000000000000 0000000000000000 c000000000058990 0000000000000000
> > [ 1.234760] GPR20: c0000000025d2778 c0000003ffa967c8 0000000000000001 0000000000080000
> > [ 2.234760] GPR24: c000000007538100 0000000000000000 0000000000000000 000000004995ca42
> > [ 1.234760] GPR28: c0000000025d2778 c0000003ffa967c8 0000000004945388 0000000000000000
> > [ 1.234869] NIP [c0000000000088dc] data_access_common_virt+0x13c/0x170
> > [ 1.234882] LR [c000000000aad890] snooze_loop+0x70/0x220
> > [ 1.234888] Call Trace:
> > [ 1.234899] [c000000007597420] [0000000000000000] 0x0
> > [ 1.234908] [c000000007597720] [0000000000000a6d] 0xa6d
> > [ 1.234919] [c000000007597a20] [0000000000000000] 0x0
> > [ 1.234931] [c000000007597d20] [0000000000000004] 0x4
> > [ 1.234943] [c000000007597d50] [c000000000aa9384] cpuidle_enter_state+0xa4/0x590
> > [ 1.234954] Freeing unused kernel memory: 5312K
> > [ 1.234958] [c000000007597dd0] [c000000000aa990c] cpuidle_enter+0x4c/0x70
> > [ 1.234965] [c000000007597e10] [c00000000019635c] call_cpuidle+0x4c/0x90
> > [ 1.234969] [c000000007597e30] [c000000000196978] do_idle+0x308/0x420
> > [ 1.234973] [c000000007597ed0] [c000000000196cd8] cpu_startup_entry+0x38/0x40
> > [ 1.234977] [c000000007597f00] [c00000000005b178] start_secondary+0x628/0x650
> > [ 1.234980] [c000000007597f90] [c00000000000c354] start_secondary_prolog+0x10/0x14
> > [ 1.234983] Instruction dump:
> > [ 1.234985] f8e100a8 f90100b0 7d2802a6 e84d0010 f9210190 894d0988 7d6102a6 f94101a8
> > [ 1.234989] f9610198 39200300 f92101b0 39400000 <e96280e0> f94101c8 f9610060 894d0989
>
> That's:
>
> std r7,168(r1)
> std r8,176(r1)
> mflr r9
> ld r2,16(r13)
> std r9,400(r1)
> lbz r10,2440(r13)
> mfxer r11
> std r10,424(r1)
> std r11,408(r1)
> li r9,768
> std r9,432(r1)
> li r10,0
> ld r11,-32544(r2)
>
> Which is from:
>
> .macro __GEN_COMMON_BODY name
> ...
> mflr r9 /* Get LR, later save to stack */
> ld r2,PACATOC(r13) /* get kernel TOC into r2 */
> std r9,_LINK(r1)
> lbz r10,PACAIRQSOFTMASK(r13)
> mfspr r11,SPRN_XER /* save XER in stackframe */
> std r10,SOFTE(r1)
> std r11,_XER(r1)
> li r9,IVEC
> std r9,_TRAP(r1) /* set trap number */
> li r10,0
> ld r11,exception_marker@toc(r2)
>
> The address that faulted c0000000026970e0 is ~38MB which looks plausible
> for the exception marker, it's definitely not completely off in the weeds.
>
> So it looks like we're just taking a spurious fault on the linear
> mapping, which is .. bad.
>
>
> And I've just hit it with your config on a machine here, but the crash
> is different:
That does not look like it.
You don't have this part in the stack trace:
> [ 1.234899] [c000000007597420] [0000000000000000] 0x0
> [ 1.234908] [c000000007597720] [0000000000000a6d] 0xa6d
> [ 1.234919] [c000000007597a20] [0000000000000000] 0x0
> [ 1.234931] [c000000007597d20] [0000000000000004] 0x4
which is somewhat random but at least on such line is always present in
the traces I get. Also I always get crash in cpuidle_enter_state
>
> [ 27.598246][ T1] Freeing unused kernel memory: 5504K
> [ 27.616971][ T588] BUG: Unable to handle kernel data access on read at 0xc00000000120a5b8
> [ 27.616972][ T1] Run /sbin/init as init process
> [ 27.616981][ T588] Faulting instruction address: 0xc0000000008561c0
> cpu 0x11: Vector: 300 (Data Access) at [c00000000678b2d0]
> pc: c0000000008561c0: cmp_ex_search+0x0/0x40
> lr: c0000000007e6844: bsearch+0x84/0x110
> sp: c00000000678b560
> msr: 8000000000001033
> dar: c00000000120a5b8
> dsisr: 40000000
> current = 0xc000000006752e00
> paca = 0xc00000000ffe6e00 irqmask: 0x03 irq_happened: 0x01
> pid = 588, comm = kworker/17:1
> Linux version 5.7.0-rc2-gcc-8.2.0-1.g8f6a41f-default+ (michael@Raptor-2.ozlabs.ibm.com) (gcc version 8.2.0 (crosstool-NG 1.24.0-rc1.16-9627a04), GNU ld (crosstool-NG 1.24.0-rc1.16-9627a04) 2.32) #287 SMP Mon Apr 20 15:10:22 AEST 2020
> enter ? for help
> [link register ] c0000000007e6844 bsearch+0x84/0x110
> [c00000000678b560] c00000000678b590 (unreliable)
> [c00000000678b5c0] c0000000008563a4 search_extable+0x34/0x50
> [c00000000678b5f0] c00000000017acc8 search_exception_tables+0x48/0xb0
> [c00000000678b620] c000000000089b5c bad_page_fault+0x3c/0x1e0
> [c00000000678b6a0] c00000000000bf80 handle_page_fault+0x28/0x2c
> --- Exception: 400 (Instruction Access) at c000000000004400 exc_virt_0x4400_instruction_access+0x0/0x80
> [c00000000678b9a0] c00000000018c8e4 update_rq_clock+0x44/0x1c0 (unreliable)
> [c00000000678ba10] c0000000001ab820 _nohz_idle_balance+0x250/0x330
> [c00000000678bae0] c0000000001ac2a0 newidle_balance+0x470/0x580
> [c00000000678bba0] c0000000001ac48c pick_next_task_fair+0x7c/0x4b0
> [c00000000678bc00] c000000000dfddbc __schedule+0x19c/0x970
> [c00000000678bce0] c000000000dfe608 schedule+0x78/0x130
> [c00000000678bd10] c000000000174ec0 worker_thread+0x130/0x650
> [c00000000678bdb0] c00000000017e59c kthread+0x19c/0x1b0
> [c00000000678be20] c00000000000cba8 ret_from_kernel_thread+0x5c/0x74
> 11:mon>
>
>
> Which is very wrong.
>
> I'm going to guess it's STRICT_KERNEL_RWX that's at fault.
I can try without that as well.
Thanks
Michal
^ permalink raw reply
* Re: crash in cpuidle_enter_state with 5.7-rc1
From: Michael Ellerman @ 2020-04-20 6:15 UTC (permalink / raw)
To: Michal Suchánek, linuxppc-dev
In-Reply-To: <20200417160348.GE25468@kitsune.suse.cz>
Michal Suchánek <msuchanek@suse.de> writes:
> Hello,
>
> I observe crash in cpuidle_enter_state in early boot on POWER9 pSeries
> machine with 5.7-rc1 kernel. The crash is not 100% reliable. Sometimes
> the machine boots.
>
> Attaching config, dmesg, and sample crash message. The stack below
> cpuidle_enter_state appears random - different in each crash.
>
> Any idea what could cause this?
Nothing immediately springs to mind.
> Preparing to boot Linux version 5.7.0-rc1-1.g8f6a41f-default (geeko@buildhost) (gcc version 9.3.1 20200406 [revision 6db837a5288ee3ca5ec504fbd5a765817e556ac2] (SUSE Linux), GNU ld (GNU Binutils; openSUSE Tumbleweed) 2.34.0.20200325-1) #1 SMP Fri Apr 17 10:39:25 UTC 2020 (8f6a41f)
> Detected machine type: 0000000000000101
> command line: BOOT_IMAGE=/boot/vmlinux-5.7.0-rc1-1.g8f6a41f-default root=UUID=04f3f652-7c85-470b-9d5f-490601f371f8 mitigations=auto quiet crashkernel=242M
> Max number of cores passed to firmware: 256 (NR_CPUS = 2048)
> Calling ibm,client-architecture-support... done
> memory layout at init:
> memory_limit : 0000000000000000 (16 MB aligned)
> alloc_bottom : 000000000e680000
> alloc_top : 0000000020000000
> alloc_top_hi : 0000000020000000
> rmo_top : 0000000020000000
> ram_top : 0000000020000000
> instantiating rtas at 0x000000001ecb0000... done
> prom_hold_cpus: skipped
> copying OF device tree...
> Building dt strings...
> Building dt structure...
> Device tree strings 0x000000000e690000 -> 0x000000000e691886
> Device tree struct 0x000000000e6a0000 -> 0x000000000e6b0000
> Quiescing Open Firmware ...
> Booting Linux via __start() @ 0x000000000a6e0000 ...
> [ 1.234639] BUG: Unable to handle kernel data access on read at 0xc0000000026970e0
> [ 1.234654] Faulting instruction address: 0xc0000000000088dc
> [ 1.234665] Oops: Kernel access of bad area, sig: 11 [#1]
> [ 1.234675] LE PAGE_SIZE=64K MMU=Hash SMP NR_CPUS=2048 NUMA pSeries
> [ 1.234686] Modules linked in:
> [ 1.234698] CPU: 4 PID: 0 Comm: swapper/4 Not tainted 5.7.0-rc1-1.g8f6a41f-default #1 openSUSE Tumbleweed (unreleased)
> [ 1.234714] NIP: c0000000000088dc LR: c000000000aad890 CTR: c0000000000087a0
> [ 1.234727] REGS: c000000007596e90 TRAP: 0300 Not tainted (5.7.0-rc1-1.g8f6a41f-default)
> [ 1.234742] MSR: 8000000000001033 <SF,ME,IR,DR,RI,LE> CR: 28000022 XER: 00000000
MMU was on when we faulted (IR & DR), so it's not real mode weirdness.
> [ 1.234760] CFAR: c0000000000087fc DAR: c0000000026970e0 DSISR: 40000000 IRQMASK: 0
> [ 1.234760] GPR00: c000000000aa9384 c000000007597120 c00000000269f000 0000000000000000
> [ 1.234760] GPR04: c0000000025d2778 0000000000000000 000000000000c800 c0000000026d69c0
> [ 1.234760] GPR08: 000e98056e8436d9 0000000000000300 0000000000000000 0000000000000000
> [ 1.234760] GPR12: 8000000000001033 c00000001ec79c00 0000000000000000 000000001ef3d880
> [ 1.234760] GPR16: 0000000000000000 0000000000000000 c000000000058990 0000000000000000
> [ 1.234760] GPR20: c0000000025d2778 c0000003ffa967c8 0000000000000001 0000000000080000
> [ 2.234760] GPR24: c000000007538100 0000000000000000 0000000000000000 000000004995ca42
> [ 1.234760] GPR28: c0000000025d2778 c0000003ffa967c8 0000000004945388 0000000000000000
> [ 1.234869] NIP [c0000000000088dc] data_access_common_virt+0x13c/0x170
> [ 1.234882] LR [c000000000aad890] snooze_loop+0x70/0x220
> [ 1.234888] Call Trace:
> [ 1.234899] [c000000007597420] [0000000000000000] 0x0
> [ 1.234908] [c000000007597720] [0000000000000a6d] 0xa6d
> [ 1.234919] [c000000007597a20] [0000000000000000] 0x0
> [ 1.234931] [c000000007597d20] [0000000000000004] 0x4
> [ 1.234943] [c000000007597d50] [c000000000aa9384] cpuidle_enter_state+0xa4/0x590
> [ 1.234954] Freeing unused kernel memory: 5312K
> [ 1.234958] [c000000007597dd0] [c000000000aa990c] cpuidle_enter+0x4c/0x70
> [ 1.234965] [c000000007597e10] [c00000000019635c] call_cpuidle+0x4c/0x90
> [ 1.234969] [c000000007597e30] [c000000000196978] do_idle+0x308/0x420
> [ 1.234973] [c000000007597ed0] [c000000000196cd8] cpu_startup_entry+0x38/0x40
> [ 1.234977] [c000000007597f00] [c00000000005b178] start_secondary+0x628/0x650
> [ 1.234980] [c000000007597f90] [c00000000000c354] start_secondary_prolog+0x10/0x14
> [ 1.234983] Instruction dump:
> [ 1.234985] f8e100a8 f90100b0 7d2802a6 e84d0010 f9210190 894d0988 7d6102a6 f94101a8
> [ 1.234989] f9610198 39200300 f92101b0 39400000 <e96280e0> f94101c8 f9610060 894d0989
That's:
std r7,168(r1)
std r8,176(r1)
mflr r9
ld r2,16(r13)
std r9,400(r1)
lbz r10,2440(r13)
mfxer r11
std r10,424(r1)
std r11,408(r1)
li r9,768
std r9,432(r1)
li r10,0
ld r11,-32544(r2)
Which is from:
.macro __GEN_COMMON_BODY name
...
mflr r9 /* Get LR, later save to stack */
ld r2,PACATOC(r13) /* get kernel TOC into r2 */
std r9,_LINK(r1)
lbz r10,PACAIRQSOFTMASK(r13)
mfspr r11,SPRN_XER /* save XER in stackframe */
std r10,SOFTE(r1)
std r11,_XER(r1)
li r9,IVEC
std r9,_TRAP(r1) /* set trap number */
li r10,0
ld r11,exception_marker@toc(r2)
The address that faulted c0000000026970e0 is ~38MB which looks plausible
for the exception marker, it's definitely not completely off in the weeds.
So it looks like we're just taking a spurious fault on the linear
mapping, which is .. bad.
And I've just hit it with your config on a machine here, but the crash
is different:
[ 27.598246][ T1] Freeing unused kernel memory: 5504K
[ 27.616971][ T588] BUG: Unable to handle kernel data access on read at 0xc00000000120a5b8
[ 27.616972][ T1] Run /sbin/init as init process
[ 27.616981][ T588] Faulting instruction address: 0xc0000000008561c0
cpu 0x11: Vector: 300 (Data Access) at [c00000000678b2d0]
pc: c0000000008561c0: cmp_ex_search+0x0/0x40
lr: c0000000007e6844: bsearch+0x84/0x110
sp: c00000000678b560
msr: 8000000000001033
dar: c00000000120a5b8
dsisr: 40000000
current = 0xc000000006752e00
paca = 0xc00000000ffe6e00 irqmask: 0x03 irq_happened: 0x01
pid = 588, comm = kworker/17:1
Linux version 5.7.0-rc2-gcc-8.2.0-1.g8f6a41f-default+ (michael@Raptor-2.ozlabs.ibm.com) (gcc version 8.2.0 (crosstool-NG 1.24.0-rc1.16-9627a04), GNU ld (crosstool-NG 1.24.0-rc1.16-9627a04) 2.32) #287 SMP Mon Apr 20 15:10:22 AEST 2020
enter ? for help
[link register ] c0000000007e6844 bsearch+0x84/0x110
[c00000000678b560] c00000000678b590 (unreliable)
[c00000000678b5c0] c0000000008563a4 search_extable+0x34/0x50
[c00000000678b5f0] c00000000017acc8 search_exception_tables+0x48/0xb0
[c00000000678b620] c000000000089b5c bad_page_fault+0x3c/0x1e0
[c00000000678b6a0] c00000000000bf80 handle_page_fault+0x28/0x2c
--- Exception: 400 (Instruction Access) at c000000000004400 exc_virt_0x4400_instruction_access+0x0/0x80
[c00000000678b9a0] c00000000018c8e4 update_rq_clock+0x44/0x1c0 (unreliable)
[c00000000678ba10] c0000000001ab820 _nohz_idle_balance+0x250/0x330
[c00000000678bae0] c0000000001ac2a0 newidle_balance+0x470/0x580
[c00000000678bba0] c0000000001ac48c pick_next_task_fair+0x7c/0x4b0
[c00000000678bc00] c000000000dfddbc __schedule+0x19c/0x970
[c00000000678bce0] c000000000dfe608 schedule+0x78/0x130
[c00000000678bd10] c000000000174ec0 worker_thread+0x130/0x650
[c00000000678bdb0] c00000000017e59c kthread+0x19c/0x1b0
[c00000000678be20] c00000000000cba8 ret_from_kernel_thread+0x5c/0x74
11:mon>
Which is very wrong.
I'm going to guess it's STRICT_KERNEL_RWX that's at fault.
And indeed with that off I can't reproduce.
Not sure why though.
cheers
^ permalink raw reply
* [PATCH v2 2/2] powerpc/eeh: Release EEH device state synchronously
From: Sam Bobroff @ 2020-04-20 5:47 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Oliver O'Halloran
In-Reply-To: <cover.1587361657.git.sbobroff@linux.ibm.com>
EEH device state is currently removed (by eeh_remove_device()) during
the device release handler, which is invoked as the device's reference
count drops to zero. This may take some time, or forever, as other
threads may hold references.
However, the PCI device state is released synchronously by
pci_stop_and_remove_bus_device(). This mismatch causes problems, for
example the device may be re-discovered as a new device before the
release handler has been called, leaving the PCI and EEH state
mismatched.
So instead, call eeh_remove_device() from the bus device removal
handlers, which are called synchronously in the removal path.
Signed-off-by: Sam Bobroff <sbobroff@linux.ibm.com>
---
v2 - Added comment explaining why the add case can't be handled similarly to the remove case.
arch/powerpc/kernel/eeh.c | 31 +++++++++++++++++++++++++++++++
arch/powerpc/kernel/pci-hotplug.c | 2 --
2 files changed, 31 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
index 17cb3e9b5697..64361311bc8e 100644
--- a/arch/powerpc/kernel/eeh.c
+++ b/arch/powerpc/kernel/eeh.c
@@ -1106,6 +1106,37 @@ static int eeh_init(void)
core_initcall_sync(eeh_init);
+static int eeh_device_notifier(struct notifier_block *nb,
+ unsigned long action, void *data)
+{
+ struct device *dev = data;
+
+ switch (action) {
+ /*
+ * Note: It's not possible to perform EEH device addition (i.e.
+ * {pseries,pnv}_pcibios_bus_add_device()) here because it depends on
+ * the device's resources, which have not yet been set up.
+ */
+ case BUS_NOTIFY_DEL_DEVICE:
+ eeh_remove_device(to_pci_dev(dev));
+ break;
+ default:
+ break;
+ }
+ return NOTIFY_DONE;
+}
+
+static struct notifier_block eeh_device_nb = {
+ .notifier_call = eeh_device_notifier,
+};
+
+static __init int eeh_set_bus_notifier(void)
+{
+ bus_register_notifier(&pci_bus_type, &eeh_device_nb);
+ return 0;
+}
+arch_initcall(eeh_set_bus_notifier);
+
/**
* eeh_add_device_early - Enable EEH for the indicated device node
* @pdn: PCI device node for which to set up EEH
diff --git a/arch/powerpc/kernel/pci-hotplug.c b/arch/powerpc/kernel/pci-hotplug.c
index d6a67f814983..28e9aa274f64 100644
--- a/arch/powerpc/kernel/pci-hotplug.c
+++ b/arch/powerpc/kernel/pci-hotplug.c
@@ -57,8 +57,6 @@ void pcibios_release_device(struct pci_dev *dev)
struct pci_controller *phb = pci_bus_to_host(dev->bus);
struct pci_dn *pdn = pci_get_pdn(dev);
- eeh_remove_device(dev);
-
if (phb->controller_ops.release_device)
phb->controller_ops.release_device(dev);
--
2.22.0.216.g00a2a96fc9
^ permalink raw reply related
* [PATCH v2 1/2] powerpc/eeh: fix pseries_eeh_configure_bridge()
From: Sam Bobroff @ 2020-04-20 5:47 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Oliver O'Halloran
In-Reply-To: <cover.1587361657.git.sbobroff@linux.ibm.com>
If a device is hot unplgged during EEH recovery, it's possible for the
RTAS call to ibm,configure-pe in pseries_eeh_configure() to return
parameter error (-3), however negative return values are not checked
for and this leads to an infinite loop.
Fix this by correctly bailing out on negative values.
Signed-off-by: Sam Bobroff <sbobroff@linux.ibm.com>
---
arch/powerpc/platforms/pseries/eeh_pseries.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/platforms/pseries/eeh_pseries.c b/arch/powerpc/platforms/pseries/eeh_pseries.c
index 893ba3f562c4..c4ef03bec0de 100644
--- a/arch/powerpc/platforms/pseries/eeh_pseries.c
+++ b/arch/powerpc/platforms/pseries/eeh_pseries.c
@@ -605,7 +605,7 @@ static int pseries_eeh_configure_bridge(struct eeh_pe *pe)
config_addr, BUID_HI(pe->phb->buid),
BUID_LO(pe->phb->buid));
- if (!ret)
+ if (ret <= 0)
return ret;
/*
--
2.22.0.216.g00a2a96fc9
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox