> -----Original Message----- > From: Conor Dooley <conor.dooley@xxxxxxxxxxxxx> > Sent: Wednesday, August 2, 2023 7:13 PM > To: palmer@xxxxxxxxxxx > Cc: conor@xxxxxxxxxx; conor.dooley@xxxxxxxxxxxxx; Paul Walmsley <paul.walmsley@xxxxxxxxxx>; Atish Patra > <atishp@xxxxxxxxxxxx>; Anup Patel <apatel@xxxxxxxxxxxxxxxx>; Alexandre Ghiti <alexghiti@xxxxxxxxxxxx>; Björn Töpel > <bjorn@xxxxxxxxxxxx>; Song Shuai <suagrfillet@xxxxxxxxx>; JeeHeng Sia <jeeheng.sia@xxxxxxxxxxxxxxxx>; Petr Tesarik > <petrtesarik@xxxxxxxxxxxxxxx>; linux-riscv@xxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; stable@xxxxxxxxxxxxxxx > Subject: [RFT 1/2] RISC-V: handle missing "no-map" properties for OpenSBI's PMP protected regions > > Add an erratum for versions [v0.8 to v1.3) of OpenSBI which fail to add > the "no-map" property to the reserved memory nodes for the regions it > has protected using PMPs. > > Our existing fix sweeping hibernation under the carpet by marking it > NONPORTABLE is insufficient as there are other ways to generate > accesses to these reserved memory regions, as Petr discovered [1] > while testing crash kernels & kdump. > > Intercede during the boot process when the afflicted versions of OpenSBI > are present & set the "no-map" property in all "mmode_resv" nodes before > the kernel does its reserved memory region initialisation. > > Reported-by: Song Shuai <suagrfillet@xxxxxxxxx> > Link: https://lore.kernel.org/all/CAAYs2=gQvkhTeioMmqRDVGjdtNF_vhB+vm_1dHJxPNi75YDQ_Q@xxxxxxxxxxxxxx/ > Reported-by: JeeHeng Sia <jeeheng.sia@xxxxxxxxxxxxxxxx> > Link: https://groups.google.com/a/groups.riscv.org/g/sw-dev/c/ITXwaKfA6z8 > Reported-by: Petr Tesarik <petrtesarik@xxxxxxxxxxxxxxx> > Closes: https://lore.kernel.org/linux-riscv/76ff0f51-d6c1-580d-f943-061e93073306@xxxxxxxxxxxxxxx/ [1] > CC: stable@xxxxxxxxxxxxxxx > Signed-off-by: Conor Dooley <conor.dooley@xxxxxxxxxxxxx> > --- > arch/riscv/include/asm/sbi.h | 5 +++++ > arch/riscv/kernel/sbi.c | 42 +++++++++++++++++++++++++++++++++++- > arch/riscv/mm/init.c | 3 +++ > 3 files changed, 49 insertions(+), 1 deletion(-) > > diff --git a/arch/riscv/include/asm/sbi.h b/arch/riscv/include/asm/sbi.h > index 5b4a1bf5f439..5360f3476278 100644 > --- a/arch/riscv/include/asm/sbi.h > +++ b/arch/riscv/include/asm/sbi.h > @@ -252,6 +252,9 @@ enum sbi_pmu_ctr_type { > #define SBI_ERR_ALREADY_STARTED -7 > #define SBI_ERR_ALREADY_STOPPED -8 > > +/* SBI implementation IDs */ > +#define SBI_IMP_OPENSBI 1 I would suggest to create an enum struct for the SBI Imp ID in the sbi.h file. What do you think? > + > extern unsigned long sbi_spec_version; > struct sbiret { > long error; > @@ -259,6 +262,8 @@ struct sbiret { > }; > > void sbi_init(void); > +void sbi_apply_reserved_mem_erratum(void *dtb_va); > + > struct sbiret sbi_ecall(int ext, int fid, unsigned long arg0, > unsigned long arg1, unsigned long arg2, > unsigned long arg3, unsigned long arg4, > diff --git a/arch/riscv/kernel/sbi.c b/arch/riscv/kernel/sbi.c > index c672c8ba9a2a..aeb27263fa53 100644 > --- a/arch/riscv/kernel/sbi.c > +++ b/arch/riscv/kernel/sbi.c > @@ -5,8 +5,10 @@ > * Copyright (c) 2020 Western Digital Corporation or its affiliates. > */ > > +#include <linux/acpi.h> > #include <linux/bits.h> > #include <linux/init.h> > +#include <linux/libfdt.h> > #include <linux/pm.h> > #include <linux/reboot.h> > #include <asm/sbi.h> > @@ -583,6 +585,40 @@ long sbi_get_mimpid(void) > } > EXPORT_SYMBOL_GPL(sbi_get_mimpid); > > +static long sbi_firmware_id; > +static long sbi_firmware_version; > + > +/* > + * For devicetrees patched by OpenSBI a "mmode_resv" node is added to cover > + * the region OpenSBI has protected by means of a PMP. Some versions of OpenSBI, > + * [v0.8 to v1.3), omitted the "no-map" property, but this trips up hibernation > + * among other things. > + */ > +void __init sbi_apply_reserved_mem_erratum(void *dtb_pa) > +{ > + int child, reserved_mem; > + > + if (sbi_firmware_id != SBI_IMP_OPENSBI) > + return; > + > + if (!acpi_disabled) > + return; > + > + if (sbi_firmware_version >= 0x10003 || sbi_firmware_version < 0x8) > + return; > + > + reserved_mem = fdt_path_offset((void *)dtb_pa, "/reserved-memory"); > + if (reserved_mem < 0) > + return; > + > + fdt_for_each_subnode(child, (void *)dtb_pa, reserved_mem) { > + const char *name = fdt_get_name((void *)dtb_pa, child, NULL); > + > + if (!strncmp(name, "mmode_resv", 10)) > + fdt_setprop((void *)dtb_pa, child, "no-map", NULL, 0); > + } > +}; > + > void __init sbi_init(void) > { > int ret; > @@ -596,8 +632,12 @@ void __init sbi_init(void) > sbi_major_version(), sbi_minor_version()); > > if (!sbi_spec_is_0_1()) { > + sbi_firmware_id = sbi_get_firmware_id(); > + sbi_firmware_version = sbi_get_firmware_version(); > + > pr_info("SBI implementation ID=0x%lx Version=0x%lx\n", > - sbi_get_firmware_id(), sbi_get_firmware_version()); > + sbi_firmware_id, sbi_firmware_version); > + > if (sbi_probe_extension(SBI_EXT_TIME)) { > __sbi_set_timer = __sbi_set_timer_v02; > pr_info("SBI TIME extension detected\n"); > diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c > index 70fb31960b63..cb16bfdeacdb 100644 > --- a/arch/riscv/mm/init.c > +++ b/arch/riscv/mm/init.c > @@ -29,6 +29,7 @@ > #include <asm/tlbflush.h> > #include <asm/sections.h> > #include <asm/soc.h> > +#include <asm/sbi.h> > #include <asm/io.h> > #include <asm/ptdump.h> > #include <asm/numa.h> > @@ -253,6 +254,8 @@ static void __init setup_bootmem(void) > * in the device tree, otherwise the allocation could end up in a > * reserved region. > */ > + > + sbi_apply_reserved_mem_erratum(dtb_early_va); > early_init_fdt_scan_reserved_mem(); > > /* > -- > 2.40.1
