* [PATCH RESEND 1/2] powerpc/mce: Add MCE notification chain
From: Santosh Sivaraj @ 2020-07-09 7:56 UTC (permalink / raw)
To: linuxppc-dev
Cc: Santosh Sivaraj, Aneesh Kumar K.V, Mahesh Salgaonkar,
Ganesh Goudar, Oliver, Vaibhav Jain
Introduce notification chain which lets know about uncorrected memory
errors(UE). This would help prospective users in pmem or nvdimm subsystem
to track bad blocks for better handling of persistent memory allocations.
Signed-off-by: Santosh S <santosh@fossix.org>
Signed-off-by: Ganesh Goudar <ganeshgr@linux.ibm.com>
---
arch/powerpc/include/asm/mce.h | 2 ++
arch/powerpc/kernel/mce.c | 15 +++++++++++++++
2 files changed, 17 insertions(+)
Send the two patches together, so the dependencies are clear. The earlier patch reviews are
here: https://lore.kernel.org/linuxppc-dev/20200330071219.12284-1-ganeshgr@linux.ibm.com/
Rebase the patches on top on 5.8-rc4
diff --git a/arch/powerpc/include/asm/mce.h b/arch/powerpc/include/asm/mce.h
index 376a395daf329..a57b0772702a9 100644
--- a/arch/powerpc/include/asm/mce.h
+++ b/arch/powerpc/include/asm/mce.h
@@ -220,6 +220,8 @@ extern void machine_check_print_event_info(struct machine_check_event *evt,
unsigned long addr_to_pfn(struct pt_regs *regs, unsigned long addr);
extern void mce_common_process_ue(struct pt_regs *regs,
struct mce_error_info *mce_err);
+extern int mce_register_notifier(struct notifier_block *nb);
+extern int mce_unregister_notifier(struct notifier_block *nb);
#ifdef CONFIG_PPC_BOOK3S_64
void flush_and_reload_slb(void);
#endif /* CONFIG_PPC_BOOK3S_64 */
diff --git a/arch/powerpc/kernel/mce.c b/arch/powerpc/kernel/mce.c
index fd90c0eda2290..b7b3ed4e61937 100644
--- a/arch/powerpc/kernel/mce.c
+++ b/arch/powerpc/kernel/mce.c
@@ -49,6 +49,20 @@ static struct irq_work mce_ue_event_irq_work = {
DECLARE_WORK(mce_ue_event_work, machine_process_ue_event);
+static BLOCKING_NOTIFIER_HEAD(mce_notifier_list);
+
+int mce_register_notifier(struct notifier_block *nb)
+{
+ return blocking_notifier_chain_register(&mce_notifier_list, nb);
+}
+EXPORT_SYMBOL_GPL(mce_register_notifier);
+
+int mce_unregister_notifier(struct notifier_block *nb)
+{
+ return blocking_notifier_chain_unregister(&mce_notifier_list, nb);
+}
+EXPORT_SYMBOL_GPL(mce_unregister_notifier);
+
static void mce_set_error_info(struct machine_check_event *mce,
struct mce_error_info *mce_err)
{
@@ -278,6 +292,7 @@ static void machine_process_ue_event(struct work_struct *work)
while (__this_cpu_read(mce_ue_count) > 0) {
index = __this_cpu_read(mce_ue_count) - 1;
evt = this_cpu_ptr(&mce_ue_event_queue[index]);
+ blocking_notifier_call_chain(&mce_notifier_list, 0, evt);
#ifdef CONFIG_MEMORY_FAILURE
/*
* This should probably queued elsewhere, but
--
2.26.2
^ permalink raw reply related
* [PATCH RESEND 2/2] papr/scm: Add bad memory ranges to nvdimm bad ranges
From: Santosh Sivaraj @ 2020-07-09 7:56 UTC (permalink / raw)
To: linuxppc-dev
Cc: Santosh Sivaraj, Aneesh Kumar K.V, Mahesh Salgaonkar,
Ganesh Goudar, Oliver, Vaibhav Jain
In-Reply-To: <20200709075635.643740-1-santosh@fossix.org>
Subscribe to the MCE notification and add the physical address which
generated a memory error to nvdimm bad range.
Reviewed-by: Mahesh Salgaonkar <mahesh@linux.ibm.com>
Signed-off-by: Santosh Sivaraj <santosh@fossix.org>
---
arch/powerpc/platforms/pseries/papr_scm.c | 98 ++++++++++++++++++++++-
1 file changed, 97 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/platforms/pseries/papr_scm.c b/arch/powerpc/platforms/pseries/papr_scm.c
index 9c569078a09fd..5ebb1c797795d 100644
--- a/arch/powerpc/platforms/pseries/papr_scm.c
+++ b/arch/powerpc/platforms/pseries/papr_scm.c
@@ -13,9 +13,11 @@
#include <linux/platform_device.h>
#include <linux/delay.h>
#include <linux/seq_buf.h>
+#include <linux/nd.h>
#include <asm/plpar_wrappers.h>
#include <asm/papr_pdsm.h>
+#include <asm/mce.h>
#define BIND_ANY_ADDR (~0ul)
@@ -80,6 +82,7 @@ struct papr_scm_priv {
struct resource res;
struct nd_region *region;
struct nd_interleave_set nd_set;
+ struct list_head region_list;
/* Protect dimm health data from concurrent read/writes */
struct mutex health_mutex;
@@ -91,6 +94,9 @@ struct papr_scm_priv {
u64 health_bitmap;
};
+LIST_HEAD(papr_nd_regions);
+DEFINE_MUTEX(papr_ndr_lock);
+
static int drc_pmem_bind(struct papr_scm_priv *p)
{
unsigned long ret[PLPAR_HCALL_BUFSIZE];
@@ -759,6 +765,10 @@ static int papr_scm_nvdimm_init(struct papr_scm_priv *p)
dev_info(dev, "Region registered with target node %d and online node %d",
target_nid, online_nid);
+ mutex_lock(&papr_ndr_lock);
+ list_add_tail(&p->region_list, &papr_nd_regions);
+ mutex_unlock(&papr_ndr_lock);
+
return 0;
err: nvdimm_bus_unregister(p->bus);
@@ -766,6 +776,70 @@ err: nvdimm_bus_unregister(p->bus);
return -ENXIO;
}
+static void papr_scm_add_badblock(struct nd_region *region,
+ struct nvdimm_bus *bus, u64 phys_addr)
+{
+ u64 aligned_addr = ALIGN_DOWN(phys_addr, L1_CACHE_BYTES);
+
+ if (nvdimm_bus_add_badrange(bus, aligned_addr, L1_CACHE_BYTES)) {
+ pr_err("Bad block registration for 0x%llx failed\n", phys_addr);
+ return;
+ }
+
+ pr_debug("Add memory range (0x%llx - 0x%llx) as bad range\n",
+ aligned_addr, aligned_addr + L1_CACHE_BYTES);
+
+ nvdimm_region_notify(region, NVDIMM_REVALIDATE_POISON);
+}
+
+static int handle_mce_ue(struct notifier_block *nb, unsigned long val,
+ void *data)
+{
+ struct machine_check_event *evt = data;
+ struct papr_scm_priv *p;
+ u64 phys_addr;
+ bool found = false;
+
+ if (evt->error_type != MCE_ERROR_TYPE_UE)
+ return NOTIFY_DONE;
+
+ if (list_empty(&papr_nd_regions))
+ return NOTIFY_DONE;
+
+ /*
+ * The physical address obtained here is PAGE_SIZE aligned, so get the
+ * exact address from the effective address
+ */
+ phys_addr = evt->u.ue_error.physical_address +
+ (evt->u.ue_error.effective_address & ~PAGE_MASK);
+
+ if (!evt->u.ue_error.physical_address_provided ||
+ !is_zone_device_page(pfn_to_page(phys_addr >> PAGE_SHIFT)))
+ return NOTIFY_DONE;
+
+ /* mce notifier is called from a process context, so mutex is safe */
+ mutex_lock(&papr_ndr_lock);
+ list_for_each_entry(p, &papr_nd_regions, region_list) {
+ struct resource res = p->res;
+
+ if (phys_addr >= res.start && phys_addr <= res.end) {
+ found = true;
+ break;
+ }
+ }
+
+ if (found)
+ papr_scm_add_badblock(p->region, p->bus, phys_addr);
+
+ mutex_unlock(&papr_ndr_lock);
+
+ return found ? NOTIFY_OK : NOTIFY_DONE;
+}
+
+static struct notifier_block mce_ue_nb = {
+ .notifier_call = handle_mce_ue
+};
+
static int papr_scm_probe(struct platform_device *pdev)
{
struct device_node *dn = pdev->dev.of_node;
@@ -866,6 +940,10 @@ static int papr_scm_remove(struct platform_device *pdev)
{
struct papr_scm_priv *p = platform_get_drvdata(pdev);
+ mutex_lock(&papr_ndr_lock);
+ list_del(&(p->region_list));
+ mutex_unlock(&papr_ndr_lock);
+
nvdimm_bus_unregister(p->bus);
drc_pmem_unbind(p);
kfree(p->bus_desc.provider_name);
@@ -888,7 +966,25 @@ static struct platform_driver papr_scm_driver = {
},
};
-module_platform_driver(papr_scm_driver);
+static int __init papr_scm_init(void)
+{
+ int ret;
+
+ ret = platform_driver_register(&papr_scm_driver);
+ if (!ret)
+ mce_register_notifier(&mce_ue_nb);
+
+return ret;
+}
+module_init(papr_scm_init);
+
+static void __exit papr_scm_exit(void)
+{
+ mce_unregister_notifier(&mce_ue_nb);
+ platform_driver_unregister(&papr_scm_driver);
+}
+module_exit(papr_scm_exit);
+
MODULE_DEVICE_TABLE(of, papr_scm_match);
MODULE_LICENSE("GPL");
MODULE_AUTHOR("IBM Corporation");
--
2.26.2
^ permalink raw reply related
* Re: [PATCH RESEND 1/2] powerpc/mce: Add MCE notification chain
From: Christophe Leroy @ 2020-07-09 8:07 UTC (permalink / raw)
To: Santosh Sivaraj, linuxppc-dev
Cc: Aneesh Kumar K.V, Oliver, Ganesh Goudar, Mahesh Salgaonkar,
Vaibhav Jain
In-Reply-To: <20200709075635.643740-1-santosh@fossix.org>
Le 09/07/2020 à 09:56, Santosh Sivaraj a écrit :
> Introduce notification chain which lets know about uncorrected memory
> errors(UE). This would help prospective users in pmem or nvdimm subsystem
> to track bad blocks for better handling of persistent memory allocations.
>
> Signed-off-by: Santosh S <santosh@fossix.org>
> Signed-off-by: Ganesh Goudar <ganeshgr@linux.ibm.com>
> ---
> arch/powerpc/include/asm/mce.h | 2 ++
> arch/powerpc/kernel/mce.c | 15 +++++++++++++++
> 2 files changed, 17 insertions(+)
>
> Send the two patches together, so the dependencies are clear. The earlier patch reviews are
> here: https://lore.kernel.org/linuxppc-dev/20200330071219.12284-1-ganeshgr@linux.ibm.com/
>
> Rebase the patches on top on 5.8-rc4
>
> diff --git a/arch/powerpc/include/asm/mce.h b/arch/powerpc/include/asm/mce.h
> index 376a395daf329..a57b0772702a9 100644
> --- a/arch/powerpc/include/asm/mce.h
> +++ b/arch/powerpc/include/asm/mce.h
> @@ -220,6 +220,8 @@ extern void machine_check_print_event_info(struct machine_check_event *evt,
> unsigned long addr_to_pfn(struct pt_regs *regs, unsigned long addr);
> extern void mce_common_process_ue(struct pt_regs *regs,
> struct mce_error_info *mce_err);
> +extern int mce_register_notifier(struct notifier_block *nb);
> +extern int mce_unregister_notifier(struct notifier_block *nb);
Using the 'extern' keyword on function declaration is pointless and
should be avoided in new patches. (checkpatch.pl --strict usually
complains about it).
> #ifdef CONFIG_PPC_BOOK3S_64
> void flush_and_reload_slb(void);
> #endif /* CONFIG_PPC_BOOK3S_64 */
> diff --git a/arch/powerpc/kernel/mce.c b/arch/powerpc/kernel/mce.c
> index fd90c0eda2290..b7b3ed4e61937 100644
> --- a/arch/powerpc/kernel/mce.c
> +++ b/arch/powerpc/kernel/mce.c
> @@ -49,6 +49,20 @@ static struct irq_work mce_ue_event_irq_work = {
>
> DECLARE_WORK(mce_ue_event_work, machine_process_ue_event);
>
> +static BLOCKING_NOTIFIER_HEAD(mce_notifier_list);
> +
> +int mce_register_notifier(struct notifier_block *nb)
> +{
> + return blocking_notifier_chain_register(&mce_notifier_list, nb);
> +}
> +EXPORT_SYMBOL_GPL(mce_register_notifier);
> +
> +int mce_unregister_notifier(struct notifier_block *nb)
> +{
> + return blocking_notifier_chain_unregister(&mce_notifier_list, nb);
> +}
> +EXPORT_SYMBOL_GPL(mce_unregister_notifier);
> +
> static void mce_set_error_info(struct machine_check_event *mce,
> struct mce_error_info *mce_err)
> {
> @@ -278,6 +292,7 @@ static void machine_process_ue_event(struct work_struct *work)
> while (__this_cpu_read(mce_ue_count) > 0) {
> index = __this_cpu_read(mce_ue_count) - 1;
> evt = this_cpu_ptr(&mce_ue_event_queue[index]);
> + blocking_notifier_call_chain(&mce_notifier_list, 0, evt);
> #ifdef CONFIG_MEMORY_FAILURE
> /*
> * This should probably queued elsewhere, but
>
Christophe
^ permalink raw reply
* Re: [PATCH v5 1/4] riscv: Move kernel mapping to vmalloc zone
From: Zong Li @ 2020-07-09 8:15 UTC (permalink / raw)
To: Palmer Dabbelt
Cc: Albert Ou, Alexandre Ghiti, Anup Patel,
linux-kernel@vger.kernel.org List, Atish Patra, Paul Mackerras,
Paul Walmsley, linux-riscv, linuxppc-dev
In-Reply-To: <mhng-831c4073-aefa-4aa0-a583-6a17f9aff9b7@palmerdabbelt-glaptop1>
On Thu, Jul 9, 2020 at 1:05 PM Palmer Dabbelt <palmer@dabbelt.com> wrote:
>
> On Sun, 07 Jun 2020 00:59:46 PDT (-0700), alex@ghiti.fr wrote:
> > This is a preparatory patch for relocatable kernel.
> >
> > The kernel used to be linked at PAGE_OFFSET address and used to be loaded
> > physically at the beginning of the main memory. Therefore, we could use
> > the linear mapping for the kernel mapping.
> >
> > But the relocated kernel base address will be different from PAGE_OFFSET
> > and since in the linear mapping, two different virtual addresses cannot
> > point to the same physical address, the kernel mapping needs to lie outside
> > the linear mapping.
>
> I know it's been a while, but I keep opening this up to review it and just
> can't get over how ugly it is to put the kernel's linear map in the vmalloc
> region.
>
> I guess I don't understand why this is necessary at all. Specifically: why
> can't we just relocate the kernel within the linear map? That would let the
> bootloader put the kernel wherever it wants, modulo the physical memory size we
> support. We'd need to handle the regions that are coupled to the kernel's
> execution address, but we could just put them in an explicit memory region
> which is what we should probably be doing anyway.
The original implementation of relocation doesn't move the kernel's linear map
to the vmalloc region, and I also give the KASLR RFC patch [1] based on that.
In original, we relocate the kernel in the linear map region, we would
calculate a
random value first as the offset, then we move the kernel image to the
new target
address which is obtained by adding this offset to it's VA and PA.
It's enough for
randomizing the kernel, but it seems to me if we want to decouple the kernel's
linear mapping, the physical mapping of RAM and virtual mapping of RAM,
it might be good to move the kernel's mapping out from the linear region.
Even so, it is still an intrusive change. As far as I know, only arm64
does something
like that.
[1] https://patchwork.kernel.org/project/linux-riscv/list/?series=260615
>
> > In addition, because modules and BPF must be close to the kernel (inside
> > +-2GB window), the kernel is placed at the end of the vmalloc zone minus
> > 2GB, which leaves room for modules and BPF. The kernel could not be
> > placed at the beginning of the vmalloc zone since other vmalloc
> > allocations from the kernel could get all the +-2GB window around the
> > kernel which would prevent new modules and BPF programs to be loaded.
>
> Well, that's not enough to make sure this doesn't happen -- it's just enough to
> make sure it doesn't happen very quickily. That's the same boat we're already
> in, though, so it's not like it's worse.
>
> > Signed-off-by: Alexandre Ghiti <alex@ghiti.fr>
> > Reviewed-by: Zong Li <zong.li@sifive.com>
> > ---
> > arch/riscv/boot/loader.lds.S | 3 +-
> > arch/riscv/include/asm/page.h | 10 +++++-
> > arch/riscv/include/asm/pgtable.h | 38 ++++++++++++++-------
> > arch/riscv/kernel/head.S | 3 +-
> > arch/riscv/kernel/module.c | 4 +--
> > arch/riscv/kernel/vmlinux.lds.S | 3 +-
> > arch/riscv/mm/init.c | 58 +++++++++++++++++++++++++-------
> > arch/riscv/mm/physaddr.c | 2 +-
> > 8 files changed, 88 insertions(+), 33 deletions(-)
> >
> > diff --git a/arch/riscv/boot/loader.lds.S b/arch/riscv/boot/loader.lds.S
> > index 47a5003c2e28..62d94696a19c 100644
> > --- a/arch/riscv/boot/loader.lds.S
> > +++ b/arch/riscv/boot/loader.lds.S
> > @@ -1,13 +1,14 @@
> > /* SPDX-License-Identifier: GPL-2.0 */
> >
> > #include <asm/page.h>
> > +#include <asm/pgtable.h>
> >
> > OUTPUT_ARCH(riscv)
> > ENTRY(_start)
> >
> > SECTIONS
> > {
> > - . = PAGE_OFFSET;
> > + . = KERNEL_LINK_ADDR;
> >
> > .payload : {
> > *(.payload)
> > diff --git a/arch/riscv/include/asm/page.h b/arch/riscv/include/asm/page.h
> > index 2d50f76efe48..48bb09b6a9b7 100644
> > --- a/arch/riscv/include/asm/page.h
> > +++ b/arch/riscv/include/asm/page.h
> > @@ -90,18 +90,26 @@ typedef struct page *pgtable_t;
> >
> > #ifdef CONFIG_MMU
> > extern unsigned long va_pa_offset;
> > +extern unsigned long va_kernel_pa_offset;
> > extern unsigned long pfn_base;
> > #define ARCH_PFN_OFFSET (pfn_base)
> > #else
> > #define va_pa_offset 0
> > +#define va_kernel_pa_offset 0
> > #define ARCH_PFN_OFFSET (PAGE_OFFSET >> PAGE_SHIFT)
> > #endif /* CONFIG_MMU */
> >
> > extern unsigned long max_low_pfn;
> > extern unsigned long min_low_pfn;
> > +extern unsigned long kernel_virt_addr;
> >
> > #define __pa_to_va_nodebug(x) ((void *)((unsigned long) (x) + va_pa_offset))
> > -#define __va_to_pa_nodebug(x) ((unsigned long)(x) - va_pa_offset)
> > +#define linear_mapping_va_to_pa(x) ((unsigned long)(x) - va_pa_offset)
> > +#define kernel_mapping_va_to_pa(x) \
> > + ((unsigned long)(x) - va_kernel_pa_offset)
> > +#define __va_to_pa_nodebug(x) \
> > + (((x) >= PAGE_OFFSET) ? \
> > + linear_mapping_va_to_pa(x) : kernel_mapping_va_to_pa(x))
> >
> > #ifdef CONFIG_DEBUG_VIRTUAL
> > extern phys_addr_t __virt_to_phys(unsigned long x);
> > diff --git a/arch/riscv/include/asm/pgtable.h b/arch/riscv/include/asm/pgtable.h
> > index 35b60035b6b0..94ef3b49dfb6 100644
> > --- a/arch/riscv/include/asm/pgtable.h
> > +++ b/arch/riscv/include/asm/pgtable.h
> > @@ -11,23 +11,29 @@
> >
> > #include <asm/pgtable-bits.h>
> >
> > -#ifndef __ASSEMBLY__
> > -
> > -/* Page Upper Directory not used in RISC-V */
> > -#include <asm-generic/pgtable-nopud.h>
> > -#include <asm/page.h>
> > -#include <asm/tlbflush.h>
> > -#include <linux/mm_types.h>
> > -
> > -#ifdef CONFIG_MMU
> > +#ifndef CONFIG_MMU
> > +#define KERNEL_VIRT_ADDR PAGE_OFFSET
> > +#define KERNEL_LINK_ADDR PAGE_OFFSET
> > +#else
> > +/*
> > + * Leave 2GB for modules and BPF that must lie within a 2GB range around
> > + * the kernel.
> > + */
> > +#define KERNEL_VIRT_ADDR (VMALLOC_END - SZ_2G + 1)
> > +#define KERNEL_LINK_ADDR KERNEL_VIRT_ADDR
>
> At a bare minimum this is going to make a mess of the 32-bit port, as
> non-relocatable kernels are now going to get linked at 1GiB which is where user
> code is supposed to live. That's an easy fix, though, as the 32-bit stuff
> doesn't need any module address restrictions.
>
> > #define VMALLOC_SIZE (KERN_VIRT_SIZE >> 1)
> > #define VMALLOC_END (PAGE_OFFSET - 1)
> > #define VMALLOC_START (PAGE_OFFSET - VMALLOC_SIZE)
> >
> > #define BPF_JIT_REGION_SIZE (SZ_128M)
> > -#define BPF_JIT_REGION_START (PAGE_OFFSET - BPF_JIT_REGION_SIZE)
> > -#define BPF_JIT_REGION_END (VMALLOC_END)
> > +#define BPF_JIT_REGION_START PFN_ALIGN((unsigned long)&_end)
> > +#define BPF_JIT_REGION_END (BPF_JIT_REGION_START + BPF_JIT_REGION_SIZE)
> > +
> > +#ifdef CONFIG_64BIT
> > +#define VMALLOC_MODULE_START BPF_JIT_REGION_END
> > +#define VMALLOC_MODULE_END (((unsigned long)&_start & PAGE_MASK) + SZ_2G)
> > +#endif
> >
> > /*
> > * Roughly size the vmemmap space to be large enough to fit enough
> > @@ -57,9 +63,16 @@
> > #define FIXADDR_SIZE PGDIR_SIZE
> > #endif
> > #define FIXADDR_START (FIXADDR_TOP - FIXADDR_SIZE)
> > -
> > #endif
> >
> > +#ifndef __ASSEMBLY__
> > +
> > +/* Page Upper Directory not used in RISC-V */
> > +#include <asm-generic/pgtable-nopud.h>
> > +#include <asm/page.h>
> > +#include <asm/tlbflush.h>
> > +#include <linux/mm_types.h>
> > +
> > #ifdef CONFIG_64BIT
> > #include <asm/pgtable-64.h>
> > #else
> > @@ -483,6 +496,7 @@ static inline void __kernel_map_pages(struct page *page, int numpages, int enabl
> >
> > #define kern_addr_valid(addr) (1) /* FIXME */
> >
> > +extern char _start[];
> > extern void *dtb_early_va;
> > void setup_bootmem(void);
> > void paging_init(void);
> > diff --git a/arch/riscv/kernel/head.S b/arch/riscv/kernel/head.S
> > index 98a406474e7d..8f5bb7731327 100644
> > --- a/arch/riscv/kernel/head.S
> > +++ b/arch/riscv/kernel/head.S
> > @@ -49,7 +49,8 @@ ENTRY(_start)
> > #ifdef CONFIG_MMU
> > relocate:
> > /* Relocate return address */
> > - li a1, PAGE_OFFSET
> > + la a1, kernel_virt_addr
> > + REG_L a1, 0(a1)
> > la a2, _start
> > sub a1, a1, a2
> > add ra, ra, a1
> > diff --git a/arch/riscv/kernel/module.c b/arch/riscv/kernel/module.c
> > index 8bbe5dbe1341..1a8fbe05accf 100644
> > --- a/arch/riscv/kernel/module.c
> > +++ b/arch/riscv/kernel/module.c
> > @@ -392,12 +392,10 @@ int apply_relocate_add(Elf_Shdr *sechdrs, const char *strtab,
> > }
> >
> > #if defined(CONFIG_MMU) && defined(CONFIG_64BIT)
> > -#define VMALLOC_MODULE_START \
> > - max(PFN_ALIGN((unsigned long)&_end - SZ_2G), VMALLOC_START)
> > void *module_alloc(unsigned long size)
> > {
> > return __vmalloc_node_range(size, 1, VMALLOC_MODULE_START,
> > - VMALLOC_END, GFP_KERNEL,
> > + VMALLOC_MODULE_END, GFP_KERNEL,
> > PAGE_KERNEL_EXEC, 0, NUMA_NO_NODE,
> > __builtin_return_address(0));
> > }
> > diff --git a/arch/riscv/kernel/vmlinux.lds.S b/arch/riscv/kernel/vmlinux.lds.S
> > index 0339b6bbe11a..a9abde62909f 100644
> > --- a/arch/riscv/kernel/vmlinux.lds.S
> > +++ b/arch/riscv/kernel/vmlinux.lds.S
> > @@ -4,7 +4,8 @@
> > * Copyright (C) 2017 SiFive
> > */
> >
> > -#define LOAD_OFFSET PAGE_OFFSET
> > +#include <asm/pgtable.h>
> > +#define LOAD_OFFSET KERNEL_LINK_ADDR
> > #include <asm/vmlinux.lds.h>
> > #include <asm/page.h>
> > #include <asm/cache.h>
> > diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> > index 736de6c8739f..71da78914645 100644
> > --- a/arch/riscv/mm/init.c
> > +++ b/arch/riscv/mm/init.c
> > @@ -22,6 +22,9 @@
> >
> > #include "../kernel/head.h"
> >
> > +unsigned long kernel_virt_addr = KERNEL_VIRT_ADDR;
> > +EXPORT_SYMBOL(kernel_virt_addr);
> > +
> > unsigned long empty_zero_page[PAGE_SIZE / sizeof(unsigned long)]
> > __page_aligned_bss;
> > EXPORT_SYMBOL(empty_zero_page);
> > @@ -178,8 +181,12 @@ void __init setup_bootmem(void)
> > }
> >
> > #ifdef CONFIG_MMU
> > +/* Offset between linear mapping virtual address and kernel load address */
> > unsigned long va_pa_offset;
> > EXPORT_SYMBOL(va_pa_offset);
> > +/* Offset between kernel mapping virtual address and kernel load address */
> > +unsigned long va_kernel_pa_offset;
> > +EXPORT_SYMBOL(va_kernel_pa_offset);
> > unsigned long pfn_base;
> > EXPORT_SYMBOL(pfn_base);
> >
> > @@ -271,7 +278,7 @@ static phys_addr_t __init alloc_pmd(uintptr_t va)
> > if (mmu_enabled)
> > return memblock_phys_alloc(PAGE_SIZE, PAGE_SIZE);
> >
> > - pmd_num = (va - PAGE_OFFSET) >> PGDIR_SHIFT;
> > + pmd_num = (va - kernel_virt_addr) >> PGDIR_SHIFT;
> > BUG_ON(pmd_num >= NUM_EARLY_PMDS);
> > return (uintptr_t)&early_pmd[pmd_num * PTRS_PER_PMD];
> > }
> > @@ -372,14 +379,30 @@ static uintptr_t __init best_map_size(phys_addr_t base, phys_addr_t size)
> > #error "setup_vm() is called from head.S before relocate so it should not use absolute addressing."
> > #endif
> >
> > +static uintptr_t load_pa, load_sz;
> > +
> > +static void __init create_kernel_page_table(pgd_t *pgdir, uintptr_t map_size)
> > +{
> > + uintptr_t va, end_va;
> > +
> > + end_va = kernel_virt_addr + load_sz;
> > + for (va = kernel_virt_addr; va < end_va; va += map_size)
> > + create_pgd_mapping(pgdir, va,
> > + load_pa + (va - kernel_virt_addr),
> > + map_size, PAGE_KERNEL_EXEC);
> > +}
> > +
> > asmlinkage void __init setup_vm(uintptr_t dtb_pa)
> > {
> > uintptr_t va, end_va;
> > - uintptr_t load_pa = (uintptr_t)(&_start);
> > - uintptr_t load_sz = (uintptr_t)(&_end) - load_pa;
> > uintptr_t map_size = best_map_size(load_pa, MAX_EARLY_MAPPING_SIZE);
> >
> > + load_pa = (uintptr_t)(&_start);
> > + load_sz = (uintptr_t)(&_end) - load_pa;
> > +
> > va_pa_offset = PAGE_OFFSET - load_pa;
> > + va_kernel_pa_offset = kernel_virt_addr - load_pa;
> > +
> > pfn_base = PFN_DOWN(load_pa);
> >
> > /*
> > @@ -402,26 +425,22 @@ asmlinkage void __init setup_vm(uintptr_t dtb_pa)
> > create_pmd_mapping(fixmap_pmd, FIXADDR_START,
> > (uintptr_t)fixmap_pte, PMD_SIZE, PAGE_TABLE);
> > /* Setup trampoline PGD and PMD */
> > - create_pgd_mapping(trampoline_pg_dir, PAGE_OFFSET,
> > + create_pgd_mapping(trampoline_pg_dir, kernel_virt_addr,
> > (uintptr_t)trampoline_pmd, PGDIR_SIZE, PAGE_TABLE);
> > - create_pmd_mapping(trampoline_pmd, PAGE_OFFSET,
> > + create_pmd_mapping(trampoline_pmd, kernel_virt_addr,
> > load_pa, PMD_SIZE, PAGE_KERNEL_EXEC);
> > #else
> > /* Setup trampoline PGD */
> > - create_pgd_mapping(trampoline_pg_dir, PAGE_OFFSET,
> > + create_pgd_mapping(trampoline_pg_dir, kernel_virt_addr,
> > load_pa, PGDIR_SIZE, PAGE_KERNEL_EXEC);
> > #endif
> >
> > /*
> > - * Setup early PGD covering entire kernel which will allows
> > + * Setup early PGD covering entire kernel which will allow
> > * us to reach paging_init(). We map all memory banks later
> > * in setup_vm_final() below.
> > */
> > - end_va = PAGE_OFFSET + load_sz;
> > - for (va = PAGE_OFFSET; va < end_va; va += map_size)
> > - create_pgd_mapping(early_pg_dir, va,
> > - load_pa + (va - PAGE_OFFSET),
> > - map_size, PAGE_KERNEL_EXEC);
> > + create_kernel_page_table(early_pg_dir, map_size);
> >
> > /* Create fixed mapping for early FDT parsing */
> > end_va = __fix_to_virt(FIX_FDT) + FIX_FDT_SIZE;
> > @@ -441,6 +460,7 @@ static void __init setup_vm_final(void)
> > uintptr_t va, map_size;
> > phys_addr_t pa, start, end;
> > struct memblock_region *reg;
> > + static struct vm_struct vm_kernel = { 0 };
> >
> > /* Set mmu_enabled flag */
> > mmu_enabled = true;
> > @@ -467,10 +487,22 @@ static void __init setup_vm_final(void)
> > for (pa = start; pa < end; pa += map_size) {
> > va = (uintptr_t)__va(pa);
> > create_pgd_mapping(swapper_pg_dir, va, pa,
> > - map_size, PAGE_KERNEL_EXEC);
> > + map_size, PAGE_KERNEL);
> > }
> > }
> >
> > + /* Map the kernel */
> > + create_kernel_page_table(swapper_pg_dir, PMD_SIZE);
> > +
> > + /* Reserve the vmalloc area occupied by the kernel */
> > + vm_kernel.addr = (void *)kernel_virt_addr;
> > + vm_kernel.phys_addr = load_pa;
> > + vm_kernel.size = (load_sz + PMD_SIZE - 1) & ~(PMD_SIZE - 1);
> > + vm_kernel.flags = VM_MAP | VM_NO_GUARD;
> > + vm_kernel.caller = __builtin_return_address(0);
> > +
> > + vm_area_add_early(&vm_kernel);
> > +
> > /* Clear fixmap PTE and PMD mappings */
> > clear_fixmap(FIX_PTE);
> > clear_fixmap(FIX_PMD);
> > diff --git a/arch/riscv/mm/physaddr.c b/arch/riscv/mm/physaddr.c
> > index e8e4dcd39fed..35703d5ef5fd 100644
> > --- a/arch/riscv/mm/physaddr.c
> > +++ b/arch/riscv/mm/physaddr.c
> > @@ -23,7 +23,7 @@ EXPORT_SYMBOL(__virt_to_phys);
> >
> > phys_addr_t __phys_addr_symbol(unsigned long x)
> > {
> > - unsigned long kernel_start = (unsigned long)PAGE_OFFSET;
> > + unsigned long kernel_start = (unsigned long)kernel_virt_addr;
> > unsigned long kernel_end = (unsigned long)_end;
> >
> > /*
^ permalink raw reply
* Re: [PATCH RESEND 2/2] papr/scm: Add bad memory ranges to nvdimm bad ranges
From: Christophe Leroy @ 2020-07-09 8:17 UTC (permalink / raw)
To: Santosh Sivaraj, linuxppc-dev
Cc: Aneesh Kumar K.V, Oliver, Ganesh Goudar, Mahesh Salgaonkar,
Vaibhav Jain
In-Reply-To: <20200709075635.643740-2-santosh@fossix.org>
Le 09/07/2020 à 09:56, Santosh Sivaraj a écrit :
> Subscribe to the MCE notification and add the physical address which
> generated a memory error to nvdimm bad range.
>
> Reviewed-by: Mahesh Salgaonkar <mahesh@linux.ibm.com>
> Signed-off-by: Santosh Sivaraj <santosh@fossix.org>
> ---
> arch/powerpc/platforms/pseries/papr_scm.c | 98 ++++++++++++++++++++++-
> 1 file changed, 97 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/platforms/pseries/papr_scm.c b/arch/powerpc/platforms/pseries/papr_scm.c
> index 9c569078a09fd..5ebb1c797795d 100644
> --- a/arch/powerpc/platforms/pseries/papr_scm.c
> +++ b/arch/powerpc/platforms/pseries/papr_scm.c
> @@ -13,9 +13,11 @@
> #include <linux/platform_device.h>
> #include <linux/delay.h>
> #include <linux/seq_buf.h>
> +#include <linux/nd.h>
>
> #include <asm/plpar_wrappers.h>
> #include <asm/papr_pdsm.h>
> +#include <asm/mce.h>
>
> #define BIND_ANY_ADDR (~0ul)
>
> @@ -80,6 +82,7 @@ struct papr_scm_priv {
> struct resource res;
> struct nd_region *region;
> struct nd_interleave_set nd_set;
> + struct list_head region_list;
>
> /* Protect dimm health data from concurrent read/writes */
> struct mutex health_mutex;
> @@ -91,6 +94,9 @@ struct papr_scm_priv {
> u64 health_bitmap;
> };
>
> +LIST_HEAD(papr_nd_regions);
> +DEFINE_MUTEX(papr_ndr_lock);
> +
> static int drc_pmem_bind(struct papr_scm_priv *p)
> {
> unsigned long ret[PLPAR_HCALL_BUFSIZE];
> @@ -759,6 +765,10 @@ static int papr_scm_nvdimm_init(struct papr_scm_priv *p)
> dev_info(dev, "Region registered with target node %d and online node %d",
> target_nid, online_nid);
>
> + mutex_lock(&papr_ndr_lock);
> + list_add_tail(&p->region_list, &papr_nd_regions);
> + mutex_unlock(&papr_ndr_lock);
> +
> return 0;
>
> err: nvdimm_bus_unregister(p->bus);
> @@ -766,6 +776,70 @@ err: nvdimm_bus_unregister(p->bus);
> return -ENXIO;
> }
>
> +static void papr_scm_add_badblock(struct nd_region *region,
> + struct nvdimm_bus *bus, u64 phys_addr)
> +{
> + u64 aligned_addr = ALIGN_DOWN(phys_addr, L1_CACHE_BYTES);
> +
> + if (nvdimm_bus_add_badrange(bus, aligned_addr, L1_CACHE_BYTES)) {
> + pr_err("Bad block registration for 0x%llx failed\n", phys_addr);
> + return;
> + }
> +
> + pr_debug("Add memory range (0x%llx - 0x%llx) as bad range\n",
> + aligned_addr, aligned_addr + L1_CACHE_BYTES);
> +
> + nvdimm_region_notify(region, NVDIMM_REVALIDATE_POISON);
> +}
> +
> +static int handle_mce_ue(struct notifier_block *nb, unsigned long val,
> + void *data)
> +{
> + struct machine_check_event *evt = data;
> + struct papr_scm_priv *p;
> + u64 phys_addr;
> + bool found = false;
> +
> + if (evt->error_type != MCE_ERROR_TYPE_UE)
> + return NOTIFY_DONE;
> +
> + if (list_empty(&papr_nd_regions))
> + return NOTIFY_DONE;
> +
> + /*
> + * The physical address obtained here is PAGE_SIZE aligned, so get the
> + * exact address from the effective address
> + */
> + phys_addr = evt->u.ue_error.physical_address +
> + (evt->u.ue_error.effective_address & ~PAGE_MASK);
Not properly aligned
> +
> + if (!evt->u.ue_error.physical_address_provided ||
> + !is_zone_device_page(pfn_to_page(phys_addr >> PAGE_SHIFT)))
> + return NOTIFY_DONE;
> +
> + /* mce notifier is called from a process context, so mutex is safe */
> + mutex_lock(&papr_ndr_lock);
> + list_for_each_entry(p, &papr_nd_regions, region_list) {
> + struct resource res = p->res;
Is this local struct really worth it ? Why not use p->res below directly ?
> +
> + if (phys_addr >= res.start && phys_addr <= res.end) {
> + found = true;
> + break;
> + }
> + }
> +
> + if (found)
> + papr_scm_add_badblock(p->region, p->bus, phys_addr);
> +
> + mutex_unlock(&papr_ndr_lock);
> +
> + return found ? NOTIFY_OK : NOTIFY_DONE;
> +}
> +
> +static struct notifier_block mce_ue_nb = {
> + .notifier_call = handle_mce_ue
> +};
> +
> static int papr_scm_probe(struct platform_device *pdev)
> {
> struct device_node *dn = pdev->dev.of_node;
> @@ -866,6 +940,10 @@ static int papr_scm_remove(struct platform_device *pdev)
> {
> struct papr_scm_priv *p = platform_get_drvdata(pdev);
>
> + mutex_lock(&papr_ndr_lock);
> + list_del(&(p->region_list));
> + mutex_unlock(&papr_ndr_lock);
> +
> nvdimm_bus_unregister(p->bus);
> drc_pmem_unbind(p);
> kfree(p->bus_desc.provider_name);
> @@ -888,7 +966,25 @@ static struct platform_driver papr_scm_driver = {
> },
> };
>
> -module_platform_driver(papr_scm_driver);
> +static int __init papr_scm_init(void)
> +{
> + int ret;
> +
> + ret = platform_driver_register(&papr_scm_driver);
> + if (!ret)
> + mce_register_notifier(&mce_ue_nb);
> +
> +return ret;
Not properly aligned.
> +}
> +module_init(papr_scm_init);
> +
> +static void __exit papr_scm_exit(void)
> +{
> + mce_unregister_notifier(&mce_ue_nb);
> + platform_driver_unregister(&papr_scm_driver);
> +}
> +module_exit(papr_scm_exit);
> +
> MODULE_DEVICE_TABLE(of, papr_scm_match);
> MODULE_LICENSE("GPL");
> MODULE_AUTHOR("IBM Corporation");
>
Christophe
^ permalink raw reply
* Re: [PATCH v3 0/6] powerpc: queued spinlocks and rwlocks
From: Peter Zijlstra @ 2020-07-09 8:31 UTC (permalink / raw)
To: Waiman Long
Cc: linux-arch, Will Deacon, Boqun Feng, linux-kernel, kvm-ppc,
virtualization, Ingo Molnar, Nicholas Piggin, linuxppc-dev
In-Reply-To: <a9834278-25bf-90e9-10f2-cd10e5407ff6@redhat.com>
On Wed, Jul 08, 2020 at 07:54:34PM -0400, Waiman Long wrote:
> On 7/8/20 4:41 AM, Peter Zijlstra wrote:
> > On Tue, Jul 07, 2020 at 03:57:06PM +1000, Nicholas Piggin wrote:
> > > Yes, powerpc could certainly get more performance out of the slow
> > > paths, and then there are a few parameters to tune.
> > Can you clarify? The slow path is already in use on ARM64 which is weak,
> > so I doubt there's superfluous serialization present. And Will spend a
> > fair amount of time on making that thing guarantee forward progressm, so
> > there just isn't too much room to play.
> >
> > > We don't have a good alternate patching for function calls yet, but
> > > that would be something to do for native vs pv.
> > Going by your jump_label implementation, support for static_call should
> > be fairly straight forward too, no?
> >
> > https://lkml.kernel.org/r/20200624153024.794671356@infradead.org
> >
> Speaking of static_call, I am also looking forward to it. Do you have an
> idea when that will be merged?
0day had one crash on the last round, I think Steve send a fix for that
last night and I'll go look at it.
That said, the last posting got 0 feedback, so either everybody is
really happy with it, or not interested. So let us know in the thread,
with some review feedback.
Once I get through enough of the inbox to actually find the fix and test
it, I'll also update the thread, and maybe threaten to merge it if
everybody stays silent :-)
^ permalink raw reply
* Re: [RFC PATCH v0 2/2] KVM: PPC: Book3S HV: Use H_RPT_INVALIDATE in nested KVM
From: Bharata B Rao @ 2020-07-09 9:08 UTC (permalink / raw)
To: Paul Mackerras; +Cc: aneesh.kumar, linuxppc-dev, npiggin, kvm-ppc
In-Reply-To: <20200709051803.GC2822576@thinks.paulus.ozlabs.org>
On Thu, Jul 09, 2020 at 03:18:03PM +1000, Paul Mackerras wrote:
> On Fri, Jul 03, 2020 at 04:14:20PM +0530, Bharata B Rao wrote:
> > In the nested KVM case, replace H_TLB_INVALIDATE by the new hcall
> > H_RPT_INVALIDATE if available. The availability of this hcall
> > is determined from "hcall-rpt-invalidate" string in ibm,hypertas-functions
> > DT property.
>
> What are we going to use when nested KVM supports HPT guests at L2?
> L1 will need to do partition-scoped tlbies with R=0 via a hypercall,
> but H_RPT_INVALIDATE says in its name that it only handles radix
> page tables (i.e. R=1).
For L2 HPT guests, the old hcall is expected to work after it adds
support for R=0 case?
The new hcall should be advertised via ibm,hypertas-functions only
for radix guests I suppose.
Regards,
Bharata.
^ permalink raw reply
* Re: [PATCH RESEND 2/2] papr/scm: Add bad memory ranges to nvdimm bad ranges
From: Santosh Sivaraj @ 2020-07-09 9:22 UTC (permalink / raw)
To: Christophe Leroy, linuxppc-dev
Cc: Aneesh Kumar K.V, Oliver, Ganesh Goudar, Mahesh Salgaonkar,
Vaibhav Jain
In-Reply-To: <d8fe2b1b-7779-ffee-e26b-858eb7cd3633@csgroup.eu>
Christophe Leroy <christophe.leroy@csgroup.eu> writes:
> Le 09/07/2020 à 09:56, Santosh Sivaraj a écrit :
>> Subscribe to the MCE notification and add the physical address which
>> generated a memory error to nvdimm bad range.
>>
>> Reviewed-by: Mahesh Salgaonkar <mahesh@linux.ibm.com>
>> Signed-off-by: Santosh Sivaraj <santosh@fossix.org>
>> ---
>> arch/powerpc/platforms/pseries/papr_scm.c | 98 ++++++++++++++++++++++-
>> 1 file changed, 97 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/powerpc/platforms/pseries/papr_scm.c b/arch/powerpc/platforms/pseries/papr_scm.c
>> index 9c569078a09fd..5ebb1c797795d 100644
>> --- a/arch/powerpc/platforms/pseries/papr_scm.c
>> +++ b/arch/powerpc/platforms/pseries/papr_scm.c
>> @@ -13,9 +13,11 @@
>> #include <linux/platform_device.h>
>> #include <linux/delay.h>
>> #include <linux/seq_buf.h>
>> +#include <linux/nd.h>
>>
>> #include <asm/plpar_wrappers.h>
>> #include <asm/papr_pdsm.h>
>> +#include <asm/mce.h>
>>
>> #define BIND_ANY_ADDR (~0ul)
>>
>> @@ -80,6 +82,7 @@ struct papr_scm_priv {
>> struct resource res;
>> struct nd_region *region;
>> struct nd_interleave_set nd_set;
>> + struct list_head region_list;
>>
>> /* Protect dimm health data from concurrent read/writes */
>> struct mutex health_mutex;
>> @@ -91,6 +94,9 @@ struct papr_scm_priv {
>> u64 health_bitmap;
>> };
>>
>> +LIST_HEAD(papr_nd_regions);
>> +DEFINE_MUTEX(papr_ndr_lock);
>> +
>> static int drc_pmem_bind(struct papr_scm_priv *p)
>> {
>> unsigned long ret[PLPAR_HCALL_BUFSIZE];
>> @@ -759,6 +765,10 @@ static int papr_scm_nvdimm_init(struct papr_scm_priv *p)
>> dev_info(dev, "Region registered with target node %d and online node %d",
>> target_nid, online_nid);
>>
>> + mutex_lock(&papr_ndr_lock);
>> + list_add_tail(&p->region_list, &papr_nd_regions);
>> + mutex_unlock(&papr_ndr_lock);
>> +
>> return 0;
>>
>> err: nvdimm_bus_unregister(p->bus);
>> @@ -766,6 +776,70 @@ err: nvdimm_bus_unregister(p->bus);
>> return -ENXIO;
>> }
>>
>> +static void papr_scm_add_badblock(struct nd_region *region,
>> + struct nvdimm_bus *bus, u64 phys_addr)
>> +{
>> + u64 aligned_addr = ALIGN_DOWN(phys_addr, L1_CACHE_BYTES);
>> +
>> + if (nvdimm_bus_add_badrange(bus, aligned_addr, L1_CACHE_BYTES)) {
>> + pr_err("Bad block registration for 0x%llx failed\n", phys_addr);
>> + return;
>> + }
>> +
>> + pr_debug("Add memory range (0x%llx - 0x%llx) as bad range\n",
>> + aligned_addr, aligned_addr + L1_CACHE_BYTES);
>> +
>> + nvdimm_region_notify(region, NVDIMM_REVALIDATE_POISON);
>> +}
>> +
>> +static int handle_mce_ue(struct notifier_block *nb, unsigned long val,
>> + void *data)
>> +{
>> + struct machine_check_event *evt = data;
>> + struct papr_scm_priv *p;
>> + u64 phys_addr;
>> + bool found = false;
>> +
>> + if (evt->error_type != MCE_ERROR_TYPE_UE)
>> + return NOTIFY_DONE;
>> +
>> + if (list_empty(&papr_nd_regions))
>> + return NOTIFY_DONE;
>> +
>> + /*
>> + * The physical address obtained here is PAGE_SIZE aligned, so get the
>> + * exact address from the effective address
>> + */
>> + phys_addr = evt->u.ue_error.physical_address +
>> + (evt->u.ue_error.effective_address & ~PAGE_MASK);
>
> Not properly aligned
Will fix it.
>
>> +
>> + if (!evt->u.ue_error.physical_address_provided ||
>> + !is_zone_device_page(pfn_to_page(phys_addr >> PAGE_SHIFT)))
>> + return NOTIFY_DONE;
>> +
>> + /* mce notifier is called from a process context, so mutex is safe */
>> + mutex_lock(&papr_ndr_lock);
>> + list_for_each_entry(p, &papr_nd_regions, region_list) {
>> + struct resource res = p->res;
>
> Is this local struct really worth it ? Why not use p->res below directly ?
>
Right, not really needed. I can fix that in v2.
>> +
>> + if (phys_addr >= res.start && phys_addr <= res.end) {
>> + found = true;
>> + break;
>> + }
>> + }
>> +
>> + if (found)
>> + papr_scm_add_badblock(p->region, p->bus, phys_addr);
>> +
>> + mutex_unlock(&papr_ndr_lock);
>> +
>> + return found ? NOTIFY_OK : NOTIFY_DONE;
>> +}
>> +
>> +static struct notifier_block mce_ue_nb = {
>> + .notifier_call = handle_mce_ue
>> +};
>> +
>> static int papr_scm_probe(struct platform_device *pdev)
>> {
>> struct device_node *dn = pdev->dev.of_node;
>> @@ -866,6 +940,10 @@ static int papr_scm_remove(struct platform_device *pdev)
>> {
>> struct papr_scm_priv *p = platform_get_drvdata(pdev);
>>
>> + mutex_lock(&papr_ndr_lock);
>> + list_del(&(p->region_list));
>> + mutex_unlock(&papr_ndr_lock);
>> +
>> nvdimm_bus_unregister(p->bus);
>> drc_pmem_unbind(p);
>> kfree(p->bus_desc.provider_name);
>> @@ -888,7 +966,25 @@ static struct platform_driver papr_scm_driver = {
>> },
>> };
>>
>> -module_platform_driver(papr_scm_driver);
>> +static int __init papr_scm_init(void)
>> +{
>> + int ret;
>> +
>> + ret = platform_driver_register(&papr_scm_driver);
>> + if (!ret)
>> + mce_register_notifier(&mce_ue_nb);
>> +
>> +return ret;
>
> Not properly aligned.
will fix it.
Thanks for the review!
Thanks,
Santosh
>
>> +}
>> +module_init(papr_scm_init);
>> +
>> +static void __exit papr_scm_exit(void)
>> +{
>> + mce_unregister_notifier(&mce_ue_nb);
>> + platform_driver_unregister(&papr_scm_driver);
>> +}
>> +module_exit(papr_scm_exit);
>> +
>> MODULE_DEVICE_TABLE(of, papr_scm_match);
>> MODULE_LICENSE("GPL");
>> MODULE_AUTHOR("IBM Corporation");
>>
>
> Christophe
^ permalink raw reply
* Re: [PATCH RESEND 1/2] powerpc/mce: Add MCE notification chain
From: Santosh Sivaraj @ 2020-07-09 9:24 UTC (permalink / raw)
To: Christophe Leroy, linuxppc-dev
Cc: Aneesh Kumar K.V, Oliver, Ganesh Goudar, Mahesh Salgaonkar,
Vaibhav Jain
In-Reply-To: <f722e532-070e-1961-3bae-6f385caa5ead@csgroup.eu>
Christophe Leroy <christophe.leroy@csgroup.eu> writes:
> Le 09/07/2020 à 09:56, Santosh Sivaraj a écrit :
>> Introduce notification chain which lets know about uncorrected memory
>> errors(UE). This would help prospective users in pmem or nvdimm subsystem
>> to track bad blocks for better handling of persistent memory allocations.
>>
>> Signed-off-by: Santosh S <santosh@fossix.org>
>> Signed-off-by: Ganesh Goudar <ganeshgr@linux.ibm.com>
>> ---
>> arch/powerpc/include/asm/mce.h | 2 ++
>> arch/powerpc/kernel/mce.c | 15 +++++++++++++++
>> 2 files changed, 17 insertions(+)
>>
>> Send the two patches together, so the dependencies are clear. The earlier patch reviews are
>> here: https://lore.kernel.org/linuxppc-dev/20200330071219.12284-1-ganeshgr@linux.ibm.com/
>>
>> Rebase the patches on top on 5.8-rc4
>>
>> diff --git a/arch/powerpc/include/asm/mce.h b/arch/powerpc/include/asm/mce.h
>> index 376a395daf329..a57b0772702a9 100644
>> --- a/arch/powerpc/include/asm/mce.h
>> +++ b/arch/powerpc/include/asm/mce.h
>> @@ -220,6 +220,8 @@ extern void machine_check_print_event_info(struct machine_check_event *evt,
>> unsigned long addr_to_pfn(struct pt_regs *regs, unsigned long addr);
>> extern void mce_common_process_ue(struct pt_regs *regs,
>> struct mce_error_info *mce_err);
>> +extern int mce_register_notifier(struct notifier_block *nb);
>> +extern int mce_unregister_notifier(struct notifier_block *nb);
>
> Using the 'extern' keyword on function declaration is pointless and
> should be avoided in new patches. (checkpatch.pl --strict usually
> complains about it).
I will remove that in the v2 which I will be sending for your comments for
the other patch.
Thanks,
Santosh
>
>> #ifdef CONFIG_PPC_BOOK3S_64
>> void flush_and_reload_slb(void);
>> #endif /* CONFIG_PPC_BOOK3S_64 */
>> diff --git a/arch/powerpc/kernel/mce.c b/arch/powerpc/kernel/mce.c
>> index fd90c0eda2290..b7b3ed4e61937 100644
>> --- a/arch/powerpc/kernel/mce.c
>> +++ b/arch/powerpc/kernel/mce.c
>> @@ -49,6 +49,20 @@ static struct irq_work mce_ue_event_irq_work = {
>>
>> DECLARE_WORK(mce_ue_event_work, machine_process_ue_event);
>>
>> +static BLOCKING_NOTIFIER_HEAD(mce_notifier_list);
>> +
>> +int mce_register_notifier(struct notifier_block *nb)
>> +{
>> + return blocking_notifier_chain_register(&mce_notifier_list, nb);
>> +}
>> +EXPORT_SYMBOL_GPL(mce_register_notifier);
>> +
>> +int mce_unregister_notifier(struct notifier_block *nb)
>> +{
>> + return blocking_notifier_chain_unregister(&mce_notifier_list, nb);
>> +}
>> +EXPORT_SYMBOL_GPL(mce_unregister_notifier);
>> +
>> static void mce_set_error_info(struct machine_check_event *mce,
>> struct mce_error_info *mce_err)
>> {
>> @@ -278,6 +292,7 @@ static void machine_process_ue_event(struct work_struct *work)
>> while (__this_cpu_read(mce_ue_count) > 0) {
>> index = __this_cpu_read(mce_ue_count) - 1;
>> evt = this_cpu_ptr(&mce_ue_event_queue[index]);
>> + blocking_notifier_call_chain(&mce_notifier_list, 0, evt);
>> #ifdef CONFIG_MEMORY_FAILURE
>> /*
>> * This should probably queued elsewhere, but
>>
>
> Christophe
^ permalink raw reply
* Re: [PATCH v3 1/6] powerpc/powernv: must include hvcall.h to get PAPR defines
From: Michael Ellerman @ 2020-07-09 10:05 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev
Cc: linux-arch, Peter Zijlstra, Boqun Feng, linux-kernel,
Nicholas Piggin, virtualization, Ingo Molnar, kvm-ppc,
Waiman Long, Will Deacon
In-Reply-To: <20200706043540.1563616-2-npiggin@gmail.com>
Nicholas Piggin <npiggin@gmail.com> writes:
> An include goes away in future patches which breaks compilation
> without this.
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> arch/powerpc/platforms/powernv/pci-ioda-tce.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/powerpc/platforms/powernv/pci-ioda-tce.c b/arch/powerpc/platforms/powernv/pci-ioda-tce.c
> index f923359d8afc..8eba6ece7808 100644
> --- a/arch/powerpc/platforms/powernv/pci-ioda-tce.c
> +++ b/arch/powerpc/platforms/powernv/pci-ioda-tce.c
> @@ -15,6 +15,7 @@
>
> #include <asm/iommu.h>
> #include <asm/tce.h>
> +#include <asm/hvcall.h> /* share error returns with PAPR */
> #include "pci.h"
>
> unsigned long pnv_ioda_parse_tce_sizes(struct pnv_phb *phb)
> --
> 2.23.0
This isn't needed anymore AFAICS, since:
5f202c1a1d42 ("powerpc/powernv/ioda: Return correct error if TCE level allocation failed")
cheers
^ permalink raw reply
* Re: [RFC PATCH v0 2/2] KVM: PPC: Book3S HV: Use H_RPT_INVALIDATE in nested KVM
From: Paul Mackerras @ 2020-07-09 10:07 UTC (permalink / raw)
To: Bharata B Rao; +Cc: aneesh.kumar, linuxppc-dev, npiggin, kvm-ppc
In-Reply-To: <20200709090851.GD7902@in.ibm.com>
On Thu, Jul 09, 2020 at 02:38:51PM +0530, Bharata B Rao wrote:
> On Thu, Jul 09, 2020 at 03:18:03PM +1000, Paul Mackerras wrote:
> > On Fri, Jul 03, 2020 at 04:14:20PM +0530, Bharata B Rao wrote:
> > > In the nested KVM case, replace H_TLB_INVALIDATE by the new hcall
> > > H_RPT_INVALIDATE if available. The availability of this hcall
> > > is determined from "hcall-rpt-invalidate" string in ibm,hypertas-functions
> > > DT property.
> >
> > What are we going to use when nested KVM supports HPT guests at L2?
> > L1 will need to do partition-scoped tlbies with R=0 via a hypercall,
> > but H_RPT_INVALIDATE says in its name that it only handles radix
> > page tables (i.e. R=1).
>
> For L2 HPT guests, the old hcall is expected to work after it adds
> support for R=0 case?
That was the plan.
> The new hcall should be advertised via ibm,hypertas-functions only
> for radix guests I suppose.
Well, the L1 hypervisor is a radix guest of L0, so it would have
H_RPT_INVALIDATE available to it?
I guess the question is whether H_RPT_INVALIDATE is supposed to do
everything, that is, radix process-scoped invalidations, radix
partition-scoped invalidations, and HPT partition-scoped
invalidations. If that is the plan then we should call it something
different.
This patchset seems to imply that H_RPT_INVALIDATE is at least going
to be used for radix partition-scoped invalidations as well as radix
process-scoped invalidations. If you are thinking that in future when
we need HPT partition-scoped invalidations for a radix L1 hypervisor
running a HPT L2 guest, we are going to define a new hypercall for
that, I suppose that is OK, though it doesn't really seem necessary.
Paul.
^ permalink raw reply
* Re: [PATCH v3 2/6] powerpc/pseries: move some PAPR paravirt functions to their own file
From: Michael Ellerman @ 2020-07-09 10:11 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev
Cc: linux-arch, Peter Zijlstra, Boqun Feng, linux-kernel,
Nicholas Piggin, virtualization, Ingo Molnar, kvm-ppc,
Waiman Long, Will Deacon
In-Reply-To: <20200706043540.1563616-3-npiggin@gmail.com>
Nicholas Piggin <npiggin@gmail.com> writes:
>
Little bit of changelog would be nice :D
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> arch/powerpc/include/asm/paravirt.h | 61 +++++++++++++++++++++++++++++
> arch/powerpc/include/asm/spinlock.h | 24 +-----------
> arch/powerpc/lib/locks.c | 12 +++---
> 3 files changed, 68 insertions(+), 29 deletions(-)
> create mode 100644 arch/powerpc/include/asm/paravirt.h
>
> diff --git a/arch/powerpc/include/asm/paravirt.h b/arch/powerpc/include/asm/paravirt.h
> new file mode 100644
> index 000000000000..7a8546660a63
> --- /dev/null
> +++ b/arch/powerpc/include/asm/paravirt.h
> @@ -0,0 +1,61 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +#ifndef __ASM_PARAVIRT_H
> +#define __ASM_PARAVIRT_H
Should be _ASM_POWERPC_PARAVIRT_H
> +#ifdef __KERNEL__
We shouldn't need __KERNEL__ in here, it's not a uapi header.
cheers
^ permalink raw reply
* Re: [PATCH v3 3/6] powerpc: move spinlock implementation to simple_spinlock
From: Michael Ellerman @ 2020-07-09 10:15 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev
Cc: linux-arch, Peter Zijlstra, Boqun Feng, linux-kernel,
Nicholas Piggin, virtualization, Ingo Molnar, kvm-ppc,
Waiman Long, Will Deacon
In-Reply-To: <20200706043540.1563616-4-npiggin@gmail.com>
Nicholas Piggin <npiggin@gmail.com> writes:
> To prepare for queued spinlocks. This is a simple rename except to update
> preprocessor guard name and a file reference.
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> arch/powerpc/include/asm/simple_spinlock.h | 292 ++++++++++++++++++
> .../include/asm/simple_spinlock_types.h | 21 ++
> arch/powerpc/include/asm/spinlock.h | 285 +----------------
> arch/powerpc/include/asm/spinlock_types.h | 12 +-
> 4 files changed, 315 insertions(+), 295 deletions(-)
> create mode 100644 arch/powerpc/include/asm/simple_spinlock.h
> create mode 100644 arch/powerpc/include/asm/simple_spinlock_types.h
>
> diff --git a/arch/powerpc/include/asm/simple_spinlock.h b/arch/powerpc/include/asm/simple_spinlock.h
> new file mode 100644
> index 000000000000..e048c041c4a9
> --- /dev/null
> +++ b/arch/powerpc/include/asm/simple_spinlock.h
> @@ -0,0 +1,292 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +#ifndef __ASM_SIMPLE_SPINLOCK_H
> +#define __ASM_SIMPLE_SPINLOCK_H
_ASM_POWERPC_SIMPLE_SPINLOCK_H
> +#ifdef __KERNEL__
Shouldn't be necessary.
> +/*
> + * Simple spin lock operations.
> + *
> + * Copyright (C) 2001-2004 Paul Mackerras <paulus@au.ibm.com>, IBM
> + * Copyright (C) 2001 Anton Blanchard <anton@au.ibm.com>, IBM
> + * Copyright (C) 2002 Dave Engebretsen <engebret@us.ibm.com>, IBM
> + * Rework to support virtual processors
> + *
> + * Type of int is used as a full 64b word is not necessary.
> + *
> + * (the type definitions are in asm/simple_spinlock_types.h)
> + */
> +#include <linux/irqflags.h>
> +#include <asm/paravirt.h>
> +#ifdef CONFIG_PPC64
> +#include <asm/paca.h>
> +#endif
I don't think paca.h needs a CONFIG_PPC64 guard, it contains one. I know
you're just moving the code, but still nice to cleanup slightly along
the way.
cheers
^ permalink raw reply
* Re: [PATCH v3 4/6] powerpc/64s: implement queued spinlocks and rwlocks
From: Michael Ellerman @ 2020-07-09 10:20 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev
Cc: linux-arch, Peter Zijlstra, Boqun Feng, linux-kernel,
Nicholas Piggin, virtualization, Ingo Molnar, kvm-ppc,
Waiman Long, Will Deacon
In-Reply-To: <20200706043540.1563616-5-npiggin@gmail.com>
Nicholas Piggin <npiggin@gmail.com> writes:
> These have shown significantly improved performance and fairness when
> spinlock contention is moderate to high on very large systems.
>
> [ Numbers hopefully forthcoming after more testing, but initial
> results look good ]
Would be good to have something here, even if it's preliminary.
> Thanks to the fast path, single threaded performance is not noticably
> hurt.
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> arch/powerpc/Kconfig | 13 ++++++++++++
> arch/powerpc/include/asm/Kbuild | 2 ++
> arch/powerpc/include/asm/qspinlock.h | 25 +++++++++++++++++++++++
> arch/powerpc/include/asm/spinlock.h | 5 +++++
> arch/powerpc/include/asm/spinlock_types.h | 5 +++++
> arch/powerpc/lib/Makefile | 3 +++
> include/asm-generic/qspinlock.h | 2 ++
Who's ack do we need for that part?
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index 24ac85c868db..17663ea57697 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -492,6 +494,17 @@ config HOTPLUG_CPU
>
> Say N if you are unsure.
>
> +config PPC_QUEUED_SPINLOCKS
> + bool "Queued spinlocks"
> + depends on SMP
> + default "y" if PPC_BOOK3S_64
Not sure about default y? At least until we've got a better idea of the
perf impact on a range of small/big new/old systems.
> + help
> + Say Y here to use to use queued spinlocks which are more complex
> + but give better salability and fairness on large SMP and NUMA
> + systems.
> +
> + If unsure, say "Y" if you have lots of cores, otherwise "N".
Would be nice if we could give a range for "lots".
> diff --git a/arch/powerpc/include/asm/Kbuild b/arch/powerpc/include/asm/Kbuild
> index dadbcf3a0b1e..1dd8b6adff5e 100644
> --- a/arch/powerpc/include/asm/Kbuild
> +++ b/arch/powerpc/include/asm/Kbuild
> @@ -6,5 +6,7 @@ generated-y += syscall_table_spu.h
> generic-y += export.h
> generic-y += local64.h
> generic-y += mcs_spinlock.h
> +generic-y += qrwlock.h
> +generic-y += qspinlock.h
The 2nd line spits a warning about a redundant entry. I think you want
to just drop it.
cheers
^ permalink raw reply
* Re: [PATCH v2 2/3] powerpc/64s: remove PROT_SAO support
From: Nicholas Piggin @ 2020-07-09 10:20 UTC (permalink / raw)
To: David Gibson, Paul Mackerras; +Cc: linux-api, linuxppc-dev, kvm-ppc, linux-mm
In-Reply-To: <20200709043406.GB2822576@thinks.paulus.ozlabs.org>
Excerpts from Paul Mackerras's message of July 9, 2020 2:34 pm:
> On Fri, Jul 03, 2020 at 11:19:57AM +1000, Nicholas Piggin wrote:
>> ISA v3.1 does not support the SAO storage control attribute required to
>> implement PROT_SAO. PROT_SAO was used by specialised system software
>> (Lx86) that has been discontinued for about 7 years, and is not thought
>> to be used elsewhere, so removal should not cause problems.
>>
>> We rather remove it than keep support for older processors, because
>> live migrating guest partitions to newer processors may not be possible
>> if SAO is in use (or worse allowed with silent races).
>
> This is actually a real problem for KVM, because now we have the
> capabilities of the host affecting the characteristics of the guest
> virtual machine in a manner which userspace (e.g. QEMU) is unable to
> control.
>
> It would probably be better to disallow SAO on all machines than have
> it available on some hosts and not others. (Yes I know there is a
> check on CPU_FTR_ARCH_206 in there, but that has been a no-op since we
> removed the PPC970 KVM support.)
This change doesn't change the SAO difference on the host processors
though, just tries to slightly improve it from silently broken to
maybe complaining a bit.
I didn't want to stop some very old image that uses this and is running
okay on an existing host from working, but maybe the existence of such
a thing would contradict my reasoning. But then if we don't care about
it why care about this KVM behaviour difference at all?
> Solving this properly will probably require creating a new KVM host
> capability and associated machine parameter in QEMU, along with a new
> machine type.
Rather than answer any of these questions, I might take the KVM change
out and that can be dealt with separately from guest SAO removal.
Thanks,
Nick
>
> [snip]
>
>> diff --git a/arch/powerpc/include/asm/kvm_book3s_64.h b/arch/powerpc/include/asm/kvm_book3s_64.h
>> index 9bb9bb370b53..fac39ff659d4 100644
>> --- a/arch/powerpc/include/asm/kvm_book3s_64.h
>> +++ b/arch/powerpc/include/asm/kvm_book3s_64.h
>> @@ -398,9 +398,10 @@ static inline bool hpte_cache_flags_ok(unsigned long hptel, bool is_ci)
>> {
>> unsigned int wimg = hptel & HPTE_R_WIMG;
>>
>> - /* Handle SAO */
>> + /* Handle SAO for POWER7,8,9 */
>> if (wimg == (HPTE_R_W | HPTE_R_I | HPTE_R_M) &&
>> - cpu_has_feature(CPU_FTR_ARCH_206))
>> + cpu_has_feature(CPU_FTR_ARCH_206) &&
>> + !cpu_has_feature(CPU_FTR_ARCH_31))
>> wimg = HPTE_R_M;
>
> Paul.
>
^ permalink raw reply
* Re: [PATCH] powerpc: select ARCH_HAS_MEMBARRIER_SYNC_CORE
From: Nicholas Piggin @ 2020-07-09 10:24 UTC (permalink / raw)
To: Mathieu Desnoyers; +Cc: linux-arch, linuxppc-dev
In-Reply-To: <407005394.1910.1594217551840.JavaMail.zimbra@efficios.com>
Excerpts from Mathieu Desnoyers's message of July 9, 2020 12:12 am:
> ----- On Jul 8, 2020, at 1:17 AM, Nicholas Piggin npiggin@gmail.com wrote:
>
>> Excerpts from Mathieu Desnoyers's message of July 7, 2020 9:25 pm:
>>> ----- On Jul 7, 2020, at 1:50 AM, Nicholas Piggin npiggin@gmail.com wrote:
>>>
> [...]
>>>> I should actually change the comment for 64-bit because soft masked
>>>> interrupt replay is an interesting case. I thought it was okay (because
>>>> the IPI would cause a hard interrupt which does do the rfi) but that
>>>> should at least be written.
>>>
>>> Yes.
>>>
>>>> The context synchronisation happens before
>>>> the Linux IPI function is called, but for the purpose of membarrier I
>>>> think that is okay (the membarrier just needs to have caused a memory
>>>> barrier + context synchronistaion by the time it has done).
>>>
>>> Can you point me to the code implementing this logic ?
>>
>> It's mostly in arch/powerpc/kernel/exception-64s.S and
>> powerpc/kernel/irq.c, but a lot of asm so easier to explain.
>>
>> When any Linux code does local_irq_disable(), we set interrupts as
>> software-masked in a per-cpu flag. When interrupts (including IPIs) come
>> in, the first thing we do is check that flag and if we are masked, then
>> record that the interrupt needs to be "replayed" in another per-cpu
>> flag. The interrupt handler then exits back using RFI (which is context
>> synchronising the CPU). Later, when the kernel code does
>> local_irq_enable(), it checks the replay flag to see if anything needs
>> to be done. At that point we basically just call the interrupt handler
>> code like a normal function, and when that returns there is no context
>> synchronising instruction.
>
> AFAIU this can only happen for interrupts nesting over irqoff sections,
> therefore over kernel code, never userspace, right ?
Right.
>> So membarrier IPI will always cause target CPUs to perform a context
>> synchronising instruction, but sometimes it happens before the IPI
>> handler function runs.
>
> If my understanding is correct, the replayed interrupt handler logic
> only nests over kernel code, which will eventually need to issue a
> context synchronizing instruction before returning to user-space.
Yes.
> All we care about is that starting from the membarrier, each core
> either:
>
> - interrupt user-space to issue the context synchronizing instruction if
> they were running userspace, or
> - _eventually_ issue a context synchronizing instruction before returning
> to user-space if they were running kernel code.
>
> So your earlier statement "the membarrier just needs to have caused a memory
> barrier + context synchronistaion by the time it has done" is not strictly
> correct: the context synchronizing instruction does not strictly need to
> happen on each core before membarrier returns. A similar line of thoughts
> can be followed for memory barriers.
Ah okay that makes it simpler, then no such speical comment is required
for the powerpc specific interrupt handling.
Thanks,
Nick
^ permalink raw reply
* Re: [PATCH v3 4/6] powerpc/64s: implement queued spinlocks and rwlocks
From: Peter Zijlstra @ 2020-07-09 10:33 UTC (permalink / raw)
To: Michael Ellerman
Cc: linux-arch, linuxppc-dev, Boqun Feng, linux-kernel,
Nicholas Piggin, virtualization, Ingo Molnar, kvm-ppc,
Waiman Long, Will Deacon
In-Reply-To: <877dvdvvkm.fsf@mpe.ellerman.id.au>
On Thu, Jul 09, 2020 at 08:20:25PM +1000, Michael Ellerman wrote:
> Nicholas Piggin <npiggin@gmail.com> writes:
> > These have shown significantly improved performance and fairness when
> > spinlock contention is moderate to high on very large systems.
> >
> > [ Numbers hopefully forthcoming after more testing, but initial
> > results look good ]
>
> Would be good to have something here, even if it's preliminary.
>
> > Thanks to the fast path, single threaded performance is not noticably
> > hurt.
> >
> > Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> > ---
> > arch/powerpc/Kconfig | 13 ++++++++++++
> > arch/powerpc/include/asm/Kbuild | 2 ++
> > arch/powerpc/include/asm/qspinlock.h | 25 +++++++++++++++++++++++
> > arch/powerpc/include/asm/spinlock.h | 5 +++++
> > arch/powerpc/include/asm/spinlock_types.h | 5 +++++
> > arch/powerpc/lib/Makefile | 3 +++
>
> > include/asm-generic/qspinlock.h | 2 ++
>
> Who's ack do we need for that part?
Mine I suppose would do, as discussed earlier, it probably isn't
required anymore, but I understand the paranoia of not wanting to change
too many things at once :-)
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
^ permalink raw reply
* Re: [PATCH v3 5/6] powerpc/pseries: implement paravirt qspinlocks for SPLPAR
From: Michael Ellerman @ 2020-07-09 10:53 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev
Cc: linux-arch, Peter Zijlstra, Boqun Feng, linux-kernel,
Nicholas Piggin, virtualization, Ingo Molnar, kvm-ppc,
Waiman Long, Will Deacon
In-Reply-To: <20200706043540.1563616-6-npiggin@gmail.com>
Nicholas Piggin <npiggin@gmail.com> writes:
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> arch/powerpc/include/asm/paravirt.h | 28 ++++++++
> arch/powerpc/include/asm/qspinlock.h | 66 +++++++++++++++++++
> arch/powerpc/include/asm/qspinlock_paravirt.h | 7 ++
> arch/powerpc/platforms/pseries/Kconfig | 5 ++
> arch/powerpc/platforms/pseries/setup.c | 6 +-
> include/asm-generic/qspinlock.h | 2 +
Another ack?
> diff --git a/arch/powerpc/include/asm/paravirt.h b/arch/powerpc/include/asm/paravirt.h
> index 7a8546660a63..f2d51f929cf5 100644
> --- a/arch/powerpc/include/asm/paravirt.h
> +++ b/arch/powerpc/include/asm/paravirt.h
> @@ -45,6 +55,19 @@ static inline void yield_to_preempted(int cpu, u32 yield_count)
> {
> ___bad_yield_to_preempted(); /* This would be a bug */
> }
> +
> +extern void ___bad_yield_to_any(void);
> +static inline void yield_to_any(void)
> +{
> + ___bad_yield_to_any(); /* This would be a bug */
> +}
Why do we do that rather than just not defining yield_to_any() at all
and letting the build fail on that?
There's a condition somewhere that we know will false at compile time
and drop the call before linking?
> diff --git a/arch/powerpc/include/asm/qspinlock_paravirt.h b/arch/powerpc/include/asm/qspinlock_paravirt.h
> new file mode 100644
> index 000000000000..750d1b5e0202
> --- /dev/null
> +++ b/arch/powerpc/include/asm/qspinlock_paravirt.h
> @@ -0,0 +1,7 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +#ifndef __ASM_QSPINLOCK_PARAVIRT_H
> +#define __ASM_QSPINLOCK_PARAVIRT_H
_ASM_POWERPC_QSPINLOCK_PARAVIRT_H please.
> +
> +EXPORT_SYMBOL(__pv_queued_spin_unlock);
Why's that in a header? Should that (eventually) go with the generic implementation?
> diff --git a/arch/powerpc/platforms/pseries/Kconfig b/arch/powerpc/platforms/pseries/Kconfig
> index 24c18362e5ea..756e727b383f 100644
> --- a/arch/powerpc/platforms/pseries/Kconfig
> +++ b/arch/powerpc/platforms/pseries/Kconfig
> @@ -25,9 +25,14 @@ config PPC_PSERIES
> select SWIOTLB
> default y
>
> +config PARAVIRT_SPINLOCKS
> + bool
> + default n
default n is the default.
> diff --git a/arch/powerpc/platforms/pseries/setup.c b/arch/powerpc/platforms/pseries/setup.c
> index 2db8469e475f..747a203d9453 100644
> --- a/arch/powerpc/platforms/pseries/setup.c
> +++ b/arch/powerpc/platforms/pseries/setup.c
> @@ -771,8 +771,12 @@ static void __init pSeries_setup_arch(void)
> if (firmware_has_feature(FW_FEATURE_LPAR)) {
> vpa_init(boot_cpuid);
>
> - if (lppaca_shared_proc(get_lppaca()))
> + if (lppaca_shared_proc(get_lppaca())) {
> static_branch_enable(&shared_processor);
> +#ifdef CONFIG_PARAVIRT_SPINLOCKS
> + pv_spinlocks_init();
> +#endif
> + }
We could avoid the ifdef with this I think?
diff --git a/arch/powerpc/include/asm/spinlock.h b/arch/powerpc/include/asm/spinlock.h
index 434615f1d761..6ec72282888d 100644
--- a/arch/powerpc/include/asm/spinlock.h
+++ b/arch/powerpc/include/asm/spinlock.h
@@ -10,5 +10,9 @@
#include <asm/simple_spinlock.h>
#endif
+#ifndef CONFIG_PARAVIRT_SPINLOCKS
+static inline void pv_spinlocks_init(void) { }
+#endif
+
#endif /* __KERNEL__ */
#endif /* __ASM_SPINLOCK_H */
cheers
^ permalink raw reply related
* Re: [PATCH 1/2] powerpc/powernv/idle: Exclude mfspr on HID1,4,5 on P9 and above
From: Gautham R Shenoy @ 2020-07-09 9:01 UTC (permalink / raw)
To: Pratik Rajesh Sampat
Cc: ego, pratik.r.sampat, linux-kernel, paulus, linuxppc-dev
In-Reply-To: <20200703124640.42820-1-psampat@linux.ibm.com>
On Fri, Jul 03, 2020 at 06:16:39PM +0530, Pratik Rajesh Sampat wrote:
> POWER9 onwards the support for the registers HID1, HID4, HID5 has been
> receded.
> Although mfspr on the above registers worked in Power9, In Power10
> simulator is unrecognized. Moving their assignment under the
> check for machines lower than Power9
>
> Signed-off-by: Pratik Rajesh Sampat <psampat@linux.ibm.com>
Nice catch.
Reviewed-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>
> ---
> arch/powerpc/platforms/powernv/idle.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/arch/powerpc/platforms/powernv/idle.c b/arch/powerpc/platforms/powernv/idle.c
> index 2dd467383a88..19d94d021357 100644
> --- a/arch/powerpc/platforms/powernv/idle.c
> +++ b/arch/powerpc/platforms/powernv/idle.c
> @@ -73,9 +73,6 @@ static int pnv_save_sprs_for_deep_states(void)
> */
> uint64_t lpcr_val = mfspr(SPRN_LPCR);
> uint64_t hid0_val = mfspr(SPRN_HID0);
> - uint64_t hid1_val = mfspr(SPRN_HID1);
> - uint64_t hid4_val = mfspr(SPRN_HID4);
> - uint64_t hid5_val = mfspr(SPRN_HID5);
> uint64_t hmeer_val = mfspr(SPRN_HMEER);
> uint64_t msr_val = MSR_IDLE;
> uint64_t psscr_val = pnv_deepest_stop_psscr_val;
> @@ -117,6 +114,9 @@ static int pnv_save_sprs_for_deep_states(void)
>
> /* Only p8 needs to set extra HID regiters */
> if (!cpu_has_feature(CPU_FTR_ARCH_300)) {
> + uint64_t hid1_val = mfspr(SPRN_HID1);
> + uint64_t hid4_val = mfspr(SPRN_HID4);
> + uint64_t hid5_val = mfspr(SPRN_HID5);
>
> rc = opal_slw_set_reg(pir, SPRN_HID1, hid1_val);
> if (rc != 0)
> --
> 2.25.4
>
--
Thanks and Regards
gautham.
^ permalink raw reply
* Re: [PATCH 2/2] powerpc/powernv/idle: save-restore DAWR0,DAWRX0 for P10
From: Gautham R Shenoy @ 2020-07-09 9:09 UTC (permalink / raw)
To: Pratik Rajesh Sampat
Cc: ego, pratik.r.sampat, linux-kernel, paulus, linuxppc-dev,
Ravi Bangoria
In-Reply-To: <20200703124640.42820-2-psampat@linux.ibm.com>
On Fri, Jul 03, 2020 at 06:16:40PM +0530, Pratik Rajesh Sampat wrote:
> Additional registers DAWR0, DAWRX0 may be lost on Power 10 for
> stop levels < 4.
Adding Ravi Bangoria <ravi.bangoria@linux.ibm.com> to the cc.
> Therefore save the values of these SPRs before entering a "stop"
> state and restore their values on wakeup.
>
> Signed-off-by: Pratik Rajesh Sampat <psampat@linux.ibm.com>
The saving and restoration looks good to me.
> ---
> arch/powerpc/platforms/powernv/idle.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/arch/powerpc/platforms/powernv/idle.c b/arch/powerpc/platforms/powernv/idle.c
> index 19d94d021357..471d4a65b1fa 100644
> --- a/arch/powerpc/platforms/powernv/idle.c
> +++ b/arch/powerpc/platforms/powernv/idle.c
> @@ -600,6 +600,8 @@ struct p9_sprs {
> u64 iamr;
> u64 amor;
> u64 uamor;
> + u64 dawr0;
> + u64 dawrx0;
> };
>
> static unsigned long power9_idle_stop(unsigned long psscr, bool mmu_on)
> @@ -677,6 +679,10 @@ static unsigned long power9_idle_stop(unsigned long psscr, bool mmu_on)
> sprs.tscr = mfspr(SPRN_TSCR);
> if (!firmware_has_feature(FW_FEATURE_ULTRAVISOR))
> sprs.ldbar = mfspr(SPRN_LDBAR);
> + if (cpu_has_feature(CPU_FTR_ARCH_31)) {
> + sprs.dawr0 = mfspr(SPRN_DAWR0);
> + sprs.dawrx0 = mfspr(SPRN_DAWRX0);
> + }
>
But this is within the if condition which says
if ((psscr & PSSCR_RL_MASK) >= pnv_first_spr_loss_level)
This if condition is meant for stop4 and stop5 since these are stop
levels that have OPAL_PM_LOSE_HYP_CONTEXT set.
Since we can lose DAWR*, on states that lose limited hypervisor
context, such as stop0-2, we need to unconditionally save them
like AMR, IAMR etc.
> sprs_saved = true;
>
> @@ -792,6 +798,10 @@ static unsigned long power9_idle_stop(unsigned long psscr, bool mmu_on)
> mtspr(SPRN_MMCR2, sprs.mmcr2);
> if (!firmware_has_feature(FW_FEATURE_ULTRAVISOR))
> mtspr(SPRN_LDBAR, sprs.ldbar);
> + if (cpu_has_feature(CPU_FTR_ARCH_31)) {
> + mtspr(SPRN_DAWR0, sprs.dawr0);
> + mtspr(SPRN_DAWRX0, sprs.dawrx0);
> + }
Likewise, we need to unconditionally restore these SPRs.
>
> mtspr(SPRN_SPRG3, local_paca->sprg_vdso);
>
> --
> 2.25.4
>
^ permalink raw reply
* Re: [PATCH v3 5/6] powerpc/pseries: implement paravirt qspinlocks for SPLPAR
From: Peter Zijlstra @ 2020-07-09 11:03 UTC (permalink / raw)
To: Michael Ellerman
Cc: linux-arch, linuxppc-dev, Boqun Feng, linux-kernel,
Nicholas Piggin, virtualization, Ingo Molnar, kvm-ppc,
Waiman Long, Will Deacon
In-Reply-To: <874kqhvu1v.fsf@mpe.ellerman.id.au>
On Thu, Jul 09, 2020 at 08:53:16PM +1000, Michael Ellerman wrote:
> Nicholas Piggin <npiggin@gmail.com> writes:
>
> > Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> > ---
> > arch/powerpc/include/asm/paravirt.h | 28 ++++++++
> > arch/powerpc/include/asm/qspinlock.h | 66 +++++++++++++++++++
> > arch/powerpc/include/asm/qspinlock_paravirt.h | 7 ++
> > arch/powerpc/platforms/pseries/Kconfig | 5 ++
> > arch/powerpc/platforms/pseries/setup.c | 6 +-
> > include/asm-generic/qspinlock.h | 2 +
>
> Another ack?
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
^ permalink raw reply
* Re: [PATCH 2/2] KVM: PPC: Book3S HV: rework secure mem slot dropping
From: Michael Ellerman @ 2020-07-09 11:13 UTC (permalink / raw)
To: Laurent Dufour, bharata
Cc: Ram Pai, linux-kernel, kvm-ppc, paulus, sathnaga, sukadev,
linuxppc-dev, bauerman
In-Reply-To: <0588d16a-8548-0f55-1132-400807a390a1@linux.ibm.com>
Laurent Dufour <ldufour@linux.ibm.com> writes:
> Le 08/07/2020 à 13:25, Bharata B Rao a écrit :
>> On Fri, Jul 03, 2020 at 05:59:14PM +0200, Laurent Dufour wrote:
>>> When a secure memslot is dropped, all the pages backed in the secure device
>>> (aka really backed by secure memory by the Ultravisor) should be paged out
>>> to a normal page. Previously, this was achieved by triggering the page
>>> fault mechanism which is calling kvmppc_svm_page_out() on each pages.
>>>
>>> This can't work when hot unplugging a memory slot because the memory slot
>>> is flagged as invalid and gfn_to_pfn() is then not trying to access the
>>> page, so the page fault mechanism is not triggered.
>>>
>>> Since the final goal is to make a call to kvmppc_svm_page_out() it seems
>>> simpler to directly calling it instead of triggering such a mechanism. This
>>> way kvmppc_uvmem_drop_pages() can be called even when hot unplugging a
>>> memslot.
>>
>> Yes, this appears much simpler.
>
> Thanks Bharata for reviewing this.
>
>>
>>>
>>> Since kvmppc_uvmem_drop_pages() is already holding kvm->arch.uvmem_lock,
>>> the call to __kvmppc_svm_page_out() is made.
>>> As __kvmppc_svm_page_out needs the vma pointer to migrate the pages, the
>>> VMA is fetched in a lazy way, to not trigger find_vma() all the time. In
>>> addition, the mmap_sem is help in read mode during that time, not in write
>>> mode since the virual memory layout is not impacted, and
>>> kvm->arch.uvmem_lock prevents concurrent operation on the secure device.
>>>
>>> Cc: Ram Pai <linuxram@us.ibm.com>
>>> Cc: Bharata B Rao <bharata@linux.ibm.com>
>>> Cc: Paul Mackerras <paulus@ozlabs.org>
>>> Signed-off-by: Laurent Dufour <ldufour@linux.ibm.com>
>>> ---
>>> arch/powerpc/kvm/book3s_hv_uvmem.c | 54 ++++++++++++++++++++----------
>>> 1 file changed, 37 insertions(+), 17 deletions(-)
>>>
>>> diff --git a/arch/powerpc/kvm/book3s_hv_uvmem.c b/arch/powerpc/kvm/book3s_hv_uvmem.c
>>> index 852cc9ae6a0b..479ddf16d18c 100644
>>> --- a/arch/powerpc/kvm/book3s_hv_uvmem.c
>>> +++ b/arch/powerpc/kvm/book3s_hv_uvmem.c
>>> @@ -533,35 +533,55 @@ static inline int kvmppc_svm_page_out(struct vm_area_struct *vma,
>>> * fault on them, do fault time migration to replace the device PTEs in
>>> * QEMU page table with normal PTEs from newly allocated pages.
>>> */
>>> -void kvmppc_uvmem_drop_pages(const struct kvm_memory_slot *free,
>>> +void kvmppc_uvmem_drop_pages(const struct kvm_memory_slot *slot,
>>> struct kvm *kvm, bool skip_page_out)
>>> {
>>> int i;
>>> struct kvmppc_uvmem_page_pvt *pvt;
>>> - unsigned long pfn, uvmem_pfn;
>>> - unsigned long gfn = free->base_gfn;
>>> + struct page *uvmem_page;
>>> + struct vm_area_struct *vma = NULL;
>>> + unsigned long uvmem_pfn, gfn;
>>> + unsigned long addr, end;
>>> +
>>> + down_read(&kvm->mm->mmap_sem);
>>
>> You should be using mmap_read_lock(kvm->mm) with recent kernels.
>
> Absolutely, shame on me, I reviewed Michel's series about that!
>
> Paul, Michael, could you fix that when pulling this patch or should I sent a
> whole new series?
Paul will take this series, so up to him.
cheers
^ permalink raw reply
* Re: [PATCH v5 1/4] riscv: Move kernel mapping to vmalloc zone
From: Alex Ghiti @ 2020-07-09 11:11 UTC (permalink / raw)
To: Palmer Dabbelt
Cc: aou, Anup Patel, linux-kernel, Atish Patra, paulus, zong.li,
Paul Walmsley, linux-riscv, linuxppc-dev
In-Reply-To: <mhng-831c4073-aefa-4aa0-a583-6a17f9aff9b7@palmerdabbelt-glaptop1>
Hi Palmer,
Le 7/9/20 à 1:05 AM, Palmer Dabbelt a écrit :
> On Sun, 07 Jun 2020 00:59:46 PDT (-0700), alex@ghiti.fr wrote:
>> This is a preparatory patch for relocatable kernel.
>>
>> The kernel used to be linked at PAGE_OFFSET address and used to be loaded
>> physically at the beginning of the main memory. Therefore, we could use
>> the linear mapping for the kernel mapping.
>>
>> But the relocated kernel base address will be different from PAGE_OFFSET
>> and since in the linear mapping, two different virtual addresses cannot
>> point to the same physical address, the kernel mapping needs to lie
>> outside
>> the linear mapping.
>
> I know it's been a while, but I keep opening this up to review it and just
> can't get over how ugly it is to put the kernel's linear map in the vmalloc
> region.
>
> I guess I don't understand why this is necessary at all. Specifically: why
> can't we just relocate the kernel within the linear map? That would let
> the
> bootloader put the kernel wherever it wants, modulo the physical memory
> size we
> support. We'd need to handle the regions that are coupled to the kernel's
> execution address, but we could just put them in an explicit memory region
> which is what we should probably be doing anyway.
Virtual relocation in the linear mapping requires to move the kernel
physically too. Zong implemented this physical move in its KASLR RFC
patchset, which is cumbersome since finding an available physical spot
is harder than just selecting a virtual range in the vmalloc range.
In addition, having the kernel mapping in the linear mapping prevents
the use of hugepage for the linear mapping resulting in performance loss
(at least for the GB that encompasses the kernel).
Why do you find this "ugly" ? The vmalloc region is just a bunch of
available virtual addresses to whatever purpose we want, and as noted by
Zong, arm64 uses the same scheme.
>
>> In addition, because modules and BPF must be close to the kernel (inside
>> +-2GB window), the kernel is placed at the end of the vmalloc zone minus
>> 2GB, which leaves room for modules and BPF. The kernel could not be
>> placed at the beginning of the vmalloc zone since other vmalloc
>> allocations from the kernel could get all the +-2GB window around the
>> kernel which would prevent new modules and BPF programs to be loaded.
>
> Well, that's not enough to make sure this doesn't happen -- it's just
> enough to
> make sure it doesn't happen very quickily. That's the same boat we're
> already
> in, though, so it's not like it's worse.
Indeed, that's not worse, I haven't found a way to reserve vmalloc area
without actually allocating it.
>
>> Signed-off-by: Alexandre Ghiti <alex@ghiti.fr>
>> Reviewed-by: Zong Li <zong.li@sifive.com>
>> ---
>> arch/riscv/boot/loader.lds.S | 3 +-
>> arch/riscv/include/asm/page.h | 10 +++++-
>> arch/riscv/include/asm/pgtable.h | 38 ++++++++++++++-------
>> arch/riscv/kernel/head.S | 3 +-
>> arch/riscv/kernel/module.c | 4 +--
>> arch/riscv/kernel/vmlinux.lds.S | 3 +-
>> arch/riscv/mm/init.c | 58 +++++++++++++++++++++++++-------
>> arch/riscv/mm/physaddr.c | 2 +-
>> 8 files changed, 88 insertions(+), 33 deletions(-)
>>
>> diff --git a/arch/riscv/boot/loader.lds.S b/arch/riscv/boot/loader.lds.S
>> index 47a5003c2e28..62d94696a19c 100644
>> --- a/arch/riscv/boot/loader.lds.S
>> +++ b/arch/riscv/boot/loader.lds.S
>> @@ -1,13 +1,14 @@
>> /* SPDX-License-Identifier: GPL-2.0 */
>>
>> #include <asm/page.h>
>> +#include <asm/pgtable.h>
>>
>> OUTPUT_ARCH(riscv)
>> ENTRY(_start)
>>
>> SECTIONS
>> {
>> - . = PAGE_OFFSET;
>> + . = KERNEL_LINK_ADDR;
>>
>> .payload : {
>> *(.payload)
>> diff --git a/arch/riscv/include/asm/page.h
>> b/arch/riscv/include/asm/page.h
>> index 2d50f76efe48..48bb09b6a9b7 100644
>> --- a/arch/riscv/include/asm/page.h
>> +++ b/arch/riscv/include/asm/page.h
>> @@ -90,18 +90,26 @@ typedef struct page *pgtable_t;
>>
>> #ifdef CONFIG_MMU
>> extern unsigned long va_pa_offset;
>> +extern unsigned long va_kernel_pa_offset;
>> extern unsigned long pfn_base;
>> #define ARCH_PFN_OFFSET (pfn_base)
>> #else
>> #define va_pa_offset 0
>> +#define va_kernel_pa_offset 0
>> #define ARCH_PFN_OFFSET (PAGE_OFFSET >> PAGE_SHIFT)
>> #endif /* CONFIG_MMU */
>>
>> extern unsigned long max_low_pfn;
>> extern unsigned long min_low_pfn;
>> +extern unsigned long kernel_virt_addr;
>>
>> #define __pa_to_va_nodebug(x) ((void *)((unsigned long) (x) +
>> va_pa_offset))
>> -#define __va_to_pa_nodebug(x) ((unsigned long)(x) - va_pa_offset)
>> +#define linear_mapping_va_to_pa(x) ((unsigned long)(x) -
>> va_pa_offset)
>> +#define kernel_mapping_va_to_pa(x) \
>> + ((unsigned long)(x) - va_kernel_pa_offset)
>> +#define __va_to_pa_nodebug(x) \
>> + (((x) >= PAGE_OFFSET) ? \
>> + linear_mapping_va_to_pa(x) : kernel_mapping_va_to_pa(x))
>>
>> #ifdef CONFIG_DEBUG_VIRTUAL
>> extern phys_addr_t __virt_to_phys(unsigned long x);
>> diff --git a/arch/riscv/include/asm/pgtable.h
>> b/arch/riscv/include/asm/pgtable.h
>> index 35b60035b6b0..94ef3b49dfb6 100644
>> --- a/arch/riscv/include/asm/pgtable.h
>> +++ b/arch/riscv/include/asm/pgtable.h
>> @@ -11,23 +11,29 @@
>>
>> #include <asm/pgtable-bits.h>
>>
>> -#ifndef __ASSEMBLY__
>> -
>> -/* Page Upper Directory not used in RISC-V */
>> -#include <asm-generic/pgtable-nopud.h>
>> -#include <asm/page.h>
>> -#include <asm/tlbflush.h>
>> -#include <linux/mm_types.h>
>> -
>> -#ifdef CONFIG_MMU
>> +#ifndef CONFIG_MMU
>> +#define KERNEL_VIRT_ADDR PAGE_OFFSET
>> +#define KERNEL_LINK_ADDR PAGE_OFFSET
>> +#else
>> +/*
>> + * Leave 2GB for modules and BPF that must lie within a 2GB range around
>> + * the kernel.
>> + */
>> +#define KERNEL_VIRT_ADDR (VMALLOC_END - SZ_2G + 1)
>> +#define KERNEL_LINK_ADDR KERNEL_VIRT_ADDR
>
> At a bare minimum this is going to make a mess of the 32-bit port, as
> non-relocatable kernels are now going to get linked at 1GiB which is
> where user
> code is supposed to live. That's an easy fix, though, as the 32-bit stuff
> doesn't need any module address restrictions.
Indeed, I will take a look at that.
>
>> #define VMALLOC_SIZE (KERN_VIRT_SIZE >> 1)
>> #define VMALLOC_END (PAGE_OFFSET - 1)
>> #define VMALLOC_START (PAGE_OFFSET - VMALLOC_SIZE)
>>
>> #define BPF_JIT_REGION_SIZE (SZ_128M)
>> -#define BPF_JIT_REGION_START (PAGE_OFFSET - BPF_JIT_REGION_SIZE)
>> -#define BPF_JIT_REGION_END (VMALLOC_END)
>> +#define BPF_JIT_REGION_START PFN_ALIGN((unsigned long)&_end)
>> +#define BPF_JIT_REGION_END (BPF_JIT_REGION_START +
>> BPF_JIT_REGION_SIZE)
>> +
>> +#ifdef CONFIG_64BIT
>> +#define VMALLOC_MODULE_START BPF_JIT_REGION_END
>> +#define VMALLOC_MODULE_END (((unsigned long)&_start & PAGE_MASK) +
>> SZ_2G)
>> +#endif
>>
>> /*
>> * Roughly size the vmemmap space to be large enough to fit enough
>> @@ -57,9 +63,16 @@
>> #define FIXADDR_SIZE PGDIR_SIZE
>> #endif
>> #define FIXADDR_START (FIXADDR_TOP - FIXADDR_SIZE)
>> -
>> #endif
>>
>> +#ifndef __ASSEMBLY__
>> +
>> +/* Page Upper Directory not used in RISC-V */
>> +#include <asm-generic/pgtable-nopud.h>
>> +#include <asm/page.h>
>> +#include <asm/tlbflush.h>
>> +#include <linux/mm_types.h>
>> +
>> #ifdef CONFIG_64BIT
>> #include <asm/pgtable-64.h>
>> #else
>> @@ -483,6 +496,7 @@ static inline void __kernel_map_pages(struct page
>> *page, int numpages, int enabl
>>
>> #define kern_addr_valid(addr) (1) /* FIXME */
>>
>> +extern char _start[];
>> extern void *dtb_early_va;
>> void setup_bootmem(void);
>> void paging_init(void);
>> diff --git a/arch/riscv/kernel/head.S b/arch/riscv/kernel/head.S
>> index 98a406474e7d..8f5bb7731327 100644
>> --- a/arch/riscv/kernel/head.S
>> +++ b/arch/riscv/kernel/head.S
>> @@ -49,7 +49,8 @@ ENTRY(_start)
>> #ifdef CONFIG_MMU
>> relocate:
>> /* Relocate return address */
>> - li a1, PAGE_OFFSET
>> + la a1, kernel_virt_addr
>> + REG_L a1, 0(a1)
>> la a2, _start
>> sub a1, a1, a2
>> add ra, ra, a1
>> diff --git a/arch/riscv/kernel/module.c b/arch/riscv/kernel/module.c
>> index 8bbe5dbe1341..1a8fbe05accf 100644
>> --- a/arch/riscv/kernel/module.c
>> +++ b/arch/riscv/kernel/module.c
>> @@ -392,12 +392,10 @@ int apply_relocate_add(Elf_Shdr *sechdrs, const
>> char *strtab,
>> }
>>
>> #if defined(CONFIG_MMU) && defined(CONFIG_64BIT)
>> -#define VMALLOC_MODULE_START \
>> - max(PFN_ALIGN((unsigned long)&_end - SZ_2G), VMALLOC_START)
>> void *module_alloc(unsigned long size)
>> {
>> return __vmalloc_node_range(size, 1, VMALLOC_MODULE_START,
>> - VMALLOC_END, GFP_KERNEL,
>> + VMALLOC_MODULE_END, GFP_KERNEL,
>> PAGE_KERNEL_EXEC, 0, NUMA_NO_NODE,
>> __builtin_return_address(0));
>> }
>> diff --git a/arch/riscv/kernel/vmlinux.lds.S
>> b/arch/riscv/kernel/vmlinux.lds.S
>> index 0339b6bbe11a..a9abde62909f 100644
>> --- a/arch/riscv/kernel/vmlinux.lds.S
>> +++ b/arch/riscv/kernel/vmlinux.lds.S
>> @@ -4,7 +4,8 @@
>> * Copyright (C) 2017 SiFive
>> */
>>
>> -#define LOAD_OFFSET PAGE_OFFSET
>> +#include <asm/pgtable.h>
>> +#define LOAD_OFFSET KERNEL_LINK_ADDR
>> #include <asm/vmlinux.lds.h>
>> #include <asm/page.h>
>> #include <asm/cache.h>
>> diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
>> index 736de6c8739f..71da78914645 100644
>> --- a/arch/riscv/mm/init.c
>> +++ b/arch/riscv/mm/init.c
>> @@ -22,6 +22,9 @@
>>
>> #include "../kernel/head.h"
>>
>> +unsigned long kernel_virt_addr = KERNEL_VIRT_ADDR;
>> +EXPORT_SYMBOL(kernel_virt_addr);
>> +
>> unsigned long empty_zero_page[PAGE_SIZE / sizeof(unsigned long)]
>> __page_aligned_bss;
>> EXPORT_SYMBOL(empty_zero_page);
>> @@ -178,8 +181,12 @@ void __init setup_bootmem(void)
>> }
>>
>> #ifdef CONFIG_MMU
>> +/* Offset between linear mapping virtual address and kernel load
>> address */
>> unsigned long va_pa_offset;
>> EXPORT_SYMBOL(va_pa_offset);
>> +/* Offset between kernel mapping virtual address and kernel load
>> address */
>> +unsigned long va_kernel_pa_offset;
>> +EXPORT_SYMBOL(va_kernel_pa_offset);
>> unsigned long pfn_base;
>> EXPORT_SYMBOL(pfn_base);
>>
>> @@ -271,7 +278,7 @@ static phys_addr_t __init alloc_pmd(uintptr_t va)
>> if (mmu_enabled)
>> return memblock_phys_alloc(PAGE_SIZE, PAGE_SIZE);
>>
>> - pmd_num = (va - PAGE_OFFSET) >> PGDIR_SHIFT;
>> + pmd_num = (va - kernel_virt_addr) >> PGDIR_SHIFT;
>> BUG_ON(pmd_num >= NUM_EARLY_PMDS);
>> return (uintptr_t)&early_pmd[pmd_num * PTRS_PER_PMD];
>> }
>> @@ -372,14 +379,30 @@ static uintptr_t __init
>> best_map_size(phys_addr_t base, phys_addr_t size)
>> #error "setup_vm() is called from head.S before relocate so it should
>> not use absolute addressing."
>> #endif
>>
>> +static uintptr_t load_pa, load_sz;
>> +
>> +static void __init create_kernel_page_table(pgd_t *pgdir, uintptr_t
>> map_size)
>> +{
>> + uintptr_t va, end_va;
>> +
>> + end_va = kernel_virt_addr + load_sz;
>> + for (va = kernel_virt_addr; va < end_va; va += map_size)
>> + create_pgd_mapping(pgdir, va,
>> + load_pa + (va - kernel_virt_addr),
>> + map_size, PAGE_KERNEL_EXEC);
>> +}
>> +
>> asmlinkage void __init setup_vm(uintptr_t dtb_pa)
>> {
>> uintptr_t va, end_va;
>> - uintptr_t load_pa = (uintptr_t)(&_start);
>> - uintptr_t load_sz = (uintptr_t)(&_end) - load_pa;
>> uintptr_t map_size = best_map_size(load_pa, MAX_EARLY_MAPPING_SIZE);
>>
>> + load_pa = (uintptr_t)(&_start);
>> + load_sz = (uintptr_t)(&_end) - load_pa;
>> +
>> va_pa_offset = PAGE_OFFSET - load_pa;
>> + va_kernel_pa_offset = kernel_virt_addr - load_pa;
>> +
>> pfn_base = PFN_DOWN(load_pa);
>>
>> /*
>> @@ -402,26 +425,22 @@ asmlinkage void __init setup_vm(uintptr_t dtb_pa)
>> create_pmd_mapping(fixmap_pmd, FIXADDR_START,
>> (uintptr_t)fixmap_pte, PMD_SIZE, PAGE_TABLE);
>> /* Setup trampoline PGD and PMD */
>> - create_pgd_mapping(trampoline_pg_dir, PAGE_OFFSET,
>> + create_pgd_mapping(trampoline_pg_dir, kernel_virt_addr,
>> (uintptr_t)trampoline_pmd, PGDIR_SIZE, PAGE_TABLE);
>> - create_pmd_mapping(trampoline_pmd, PAGE_OFFSET,
>> + create_pmd_mapping(trampoline_pmd, kernel_virt_addr,
>> load_pa, PMD_SIZE, PAGE_KERNEL_EXEC);
>> #else
>> /* Setup trampoline PGD */
>> - create_pgd_mapping(trampoline_pg_dir, PAGE_OFFSET,
>> + create_pgd_mapping(trampoline_pg_dir, kernel_virt_addr,
>> load_pa, PGDIR_SIZE, PAGE_KERNEL_EXEC);
>> #endif
>>
>> /*
>> - * Setup early PGD covering entire kernel which will allows
>> + * Setup early PGD covering entire kernel which will allow
>> * us to reach paging_init(). We map all memory banks later
>> * in setup_vm_final() below.
>> */
>> - end_va = PAGE_OFFSET + load_sz;
>> - for (va = PAGE_OFFSET; va < end_va; va += map_size)
>> - create_pgd_mapping(early_pg_dir, va,
>> - load_pa + (va - PAGE_OFFSET),
>> - map_size, PAGE_KERNEL_EXEC);
>> + create_kernel_page_table(early_pg_dir, map_size);
>>
>> /* Create fixed mapping for early FDT parsing */
>> end_va = __fix_to_virt(FIX_FDT) + FIX_FDT_SIZE;
>> @@ -441,6 +460,7 @@ static void __init setup_vm_final(void)
>> uintptr_t va, map_size;
>> phys_addr_t pa, start, end;
>> struct memblock_region *reg;
>> + static struct vm_struct vm_kernel = { 0 };
>>
>> /* Set mmu_enabled flag */
>> mmu_enabled = true;
>> @@ -467,10 +487,22 @@ static void __init setup_vm_final(void)
>> for (pa = start; pa < end; pa += map_size) {
>> va = (uintptr_t)__va(pa);
>> create_pgd_mapping(swapper_pg_dir, va, pa,
>> - map_size, PAGE_KERNEL_EXEC);
>> + map_size, PAGE_KERNEL);
>> }
>> }
>>
>> + /* Map the kernel */
>> + create_kernel_page_table(swapper_pg_dir, PMD_SIZE);
>> +
>> + /* Reserve the vmalloc area occupied by the kernel */
>> + vm_kernel.addr = (void *)kernel_virt_addr;
>> + vm_kernel.phys_addr = load_pa;
>> + vm_kernel.size = (load_sz + PMD_SIZE - 1) & ~(PMD_SIZE - 1);
>> + vm_kernel.flags = VM_MAP | VM_NO_GUARD;
>> + vm_kernel.caller = __builtin_return_address(0);
>> +
>> + vm_area_add_early(&vm_kernel);
>> +
>> /* Clear fixmap PTE and PMD mappings */
>> clear_fixmap(FIX_PTE);
>> clear_fixmap(FIX_PMD);
>> diff --git a/arch/riscv/mm/physaddr.c b/arch/riscv/mm/physaddr.c
>> index e8e4dcd39fed..35703d5ef5fd 100644
>> --- a/arch/riscv/mm/physaddr.c
>> +++ b/arch/riscv/mm/physaddr.c
>> @@ -23,7 +23,7 @@ EXPORT_SYMBOL(__virt_to_phys);
>>
>> phys_addr_t __phys_addr_symbol(unsigned long x)
>> {
>> - unsigned long kernel_start = (unsigned long)PAGE_OFFSET;
>> + unsigned long kernel_start = (unsigned long)kernel_virt_addr;
>> unsigned long kernel_end = (unsigned long)_end;
>>
>> /*
Alex
^ permalink raw reply
* Re: [PATCH 1/2] powerpc/vas: Report proper error for address translation failure
From: Michael Ellerman @ 2020-07-09 11:22 UTC (permalink / raw)
To: Haren Myneni; +Cc: tulioqm, abali, linuxppc-dev, rzinsly
In-Reply-To: <f8af60fd4167c9c04ee5ab47147b9e95bcb3b9ff.camel@linux.ibm.com>
Haren Myneni <haren@linux.ibm.com> writes:
> DMA controller uses CC=5 internally for translation fault handling. So
> OS should be using CC=250 and should report this error to the user space
> when NX encounters address translation failure on the request buffer.
That doesn't really explain *why* the OS must use CC=250.
Is it documented somewhere that 5 is for hardware use, and 250 is for
software?
> This patch defines CSB_CC_ADDRESS_TRANSLATION(250) and updates
> CSB.CC with this proper error code for user space.
We still have:
#define CSB_CC_TRANSLATION (5)
And it's very unclear where one or the other should be used.
Can one or the other get a name that makes the distinction clear.
cheers
> diff --git a/Documentation/powerpc/vas-api.rst b/Documentation/powerpc/vas-api.rst
> index 1217c2f..78627cc 100644
> --- a/Documentation/powerpc/vas-api.rst
> +++ b/Documentation/powerpc/vas-api.rst
> @@ -213,7 +213,7 @@ request buffers are not in memory. The operating system handles the fault by
> updating CSB with the following data:
>
> csb.flags = CSB_V;
> - csb.cc = CSB_CC_TRANSLATION;
> + csb.cc = CSB_CC_ADDRESS_TRANSLATION;
> csb.ce = CSB_CE_TERMINATION;
> csb.address = fault_address;
>
> diff --git a/arch/powerpc/include/asm/icswx.h b/arch/powerpc/include/asm/icswx.h
> index 965b1f3..b1c9a57 100644
> --- a/arch/powerpc/include/asm/icswx.h
> +++ b/arch/powerpc/include/asm/icswx.h
> @@ -77,6 +77,8 @@ struct coprocessor_completion_block {
> #define CSB_CC_CHAIN (37)
> #define CSB_CC_SEQUENCE (38)
> #define CSB_CC_HW (39)
> +/* User space address traslation failure */
> +#define CSB_CC_ADDRESS_TRANSLATION (250)
>
> #define CSB_SIZE (0x10)
> #define CSB_ALIGN CSB_SIZE
> diff --git a/arch/powerpc/platforms/powernv/vas-fault.c b/arch/powerpc/platforms/powernv/vas-fault.c
> index 266a6ca..33e89d4 100644
> --- a/arch/powerpc/platforms/powernv/vas-fault.c
> +++ b/arch/powerpc/platforms/powernv/vas-fault.c
> @@ -79,7 +79,7 @@ static void update_csb(struct vas_window *window,
> csb_addr = (void __user *)be64_to_cpu(crb->csb_addr);
>
> memset(&csb, 0, sizeof(csb));
> - csb.cc = CSB_CC_TRANSLATION;
> + csb.cc = CSB_CC_ADDRESS_TRANSLATION;
> csb.ce = CSB_CE_TERMINATION;
> csb.cs = 0;
> csb.count = 0;
> --
> 1.8.3.1
^ permalink raw reply
* Re: [PATCH 2/2] powerpc/powernv/idle: save-restore DAWR0,DAWRX0 for P10
From: Pratik Sampat @ 2020-07-09 11:21 UTC (permalink / raw)
To: ego; +Cc: Ravi Bangoria, pratik.r.sampat, linux-kernel, paulus,
linuxppc-dev
In-Reply-To: <20200709090948.GB24354@in.ibm.com>
On 09/07/20 2:39 pm, Gautham R Shenoy wrote:
> On Fri, Jul 03, 2020 at 06:16:40PM +0530, Pratik Rajesh Sampat wrote:
>> Additional registers DAWR0, DAWRX0 may be lost on Power 10 for
>> stop levels < 4.
> Adding Ravi Bangoria <ravi.bangoria@linux.ibm.com> to the cc.
>
>> Therefore save the values of these SPRs before entering a "stop"
>> state and restore their values on wakeup.
>>
>> Signed-off-by: Pratik Rajesh Sampat <psampat@linux.ibm.com>
>
> The saving and restoration looks good to me.
>> ---
>> arch/powerpc/platforms/powernv/idle.c | 10 ++++++++++
>> 1 file changed, 10 insertions(+)
>>
>> diff --git a/arch/powerpc/platforms/powernv/idle.c b/arch/powerpc/platforms/powernv/idle.c
>> index 19d94d021357..471d4a65b1fa 100644
>> --- a/arch/powerpc/platforms/powernv/idle.c
>> +++ b/arch/powerpc/platforms/powernv/idle.c
>> @@ -600,6 +600,8 @@ struct p9_sprs {
>> u64 iamr;
>> u64 amor;
>> u64 uamor;
>> + u64 dawr0;
>> + u64 dawrx0;
>> };
>>
>> static unsigned long power9_idle_stop(unsigned long psscr, bool mmu_on)
>> @@ -677,6 +679,10 @@ static unsigned long power9_idle_stop(unsigned long psscr, bool mmu_on)
>> sprs.tscr = mfspr(SPRN_TSCR);
>> if (!firmware_has_feature(FW_FEATURE_ULTRAVISOR))
>> sprs.ldbar = mfspr(SPRN_LDBAR);
>> + if (cpu_has_feature(CPU_FTR_ARCH_31)) {
>> + sprs.dawr0 = mfspr(SPRN_DAWR0);
>> + sprs.dawrx0 = mfspr(SPRN_DAWRX0);
>> + }
>>
>
> But this is within the if condition which says
>
> if ((psscr & PSSCR_RL_MASK) >= pnv_first_spr_loss_level)
>
> This if condition is meant for stop4 and stop5 since these are stop
> levels that have OPAL_PM_LOSE_HYP_CONTEXT set.
>
> Since we can lose DAWR*, on states that lose limited hypervisor
> context, such as stop0-2, we need to unconditionally save them
> like AMR, IAMR etc.
>
Right, shallow states too loose DAWR/X. Thanks for pointing it out.
I'll fix this and resend.
>> sprs_saved = true;
>>
>> @@ -792,6 +798,10 @@ static unsigned long power9_idle_stop(unsigned long psscr, bool mmu_on)
>> mtspr(SPRN_MMCR2, sprs.mmcr2);
>> if (!firmware_has_feature(FW_FEATURE_ULTRAVISOR))
>> mtspr(SPRN_LDBAR, sprs.ldbar);
>> + if (cpu_has_feature(CPU_FTR_ARCH_31)) {
>> + mtspr(SPRN_DAWR0, sprs.dawr0);
>> + mtspr(SPRN_DAWRX0, sprs.dawrx0);
>> + }
>
> Likewise, we need to unconditionally restore these SPRs.
>
>
>> mtspr(SPRN_SPRG3, local_paca->sprg_vdso);
>>
>> --
>> 2.25.4
>>
Thanks
Pratik
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox