lkml.org 
[lkml]   [2023]   [Aug]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [RFT 1/2] RISC-V: handle missing "no-map" properties for OpenSBI's PMP protected regions
    On Wed, Aug 2, 2023 at 4:14 AM Conor Dooley <conor.dooley@microchip.com> wrote:
    >
    > 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.
    >

    We have different mechanisms of DT being passed to the kernel.

    1. A prior stage(e.g U-Boot SPL) to M-mode runtime firmware (e.g.
    OpenSBI, rustSBI) passes the DT to M-mode runtime firmware and it
    passes to the next stage.
    In this case, M-mode runtime firmware gets a chance to update the
    no-map property in DT that the kernel can parse.

    2. User loads the DT from the boot loader (e.g EDK2, U-Boot proper).
    Any DT patching done by the M-mode firmware is useless. If these DTBs
    don't have the no-map
    property, hibernation or EFI booting will have issues as well.

    We are trying to solve only one part of problem #1 in this patch. I
    don't think any other M-mode runtime firmware patches DT with no-map
    property as well.
    Please let me know if I am wrong about that. The problem is not
    restricted to [v0.8 to v1.3) of OpenSBI.

    The booting doc now says that no-map property must be set and any
    usage of DTBs without that (via #1 or #2) will have unintended
    consequences.

    > Reported-by: Song Shuai <suagrfillet@gmail.com>
    > Link: https://lore.kernel.org/all/CAAYs2=gQvkhTeioMmqRDVGjdtNF_vhB+vm_1dHJxPNi75YDQ_Q@mail.gmail.com/
    > Reported-by: JeeHeng Sia <jeeheng.sia@starfivetech.com>
    > Link: https://groups.google.com/a/groups.riscv.org/g/sw-dev/c/ITXwaKfA6z8
    > Reported-by: Petr Tesarik <petrtesarik@huaweicloud.com>
    > Closes: https://lore.kernel.org/linux-riscv/76ff0f51-d6c1-580d-f943-061e93073306@huaweicloud.com/ [1]
    > CC: stable@vger.kernel.org
    > Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
    > ---
    > 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
    > +
    > 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
    >

    \
     
     \ /
      Last update: 2023-08-08 21:53    [W:3.865 / U:0.040 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site