[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <3e066032031e4552b4b7903755deb669@EXMBX066.cuchost.com>
Date: Mon, 7 Aug 2023 00:44:07 +0000
From: JeeHeng Sia <jeeheng.sia@...rfivetech.com>
To: Conor Dooley <conor.dooley@...rochip.com>,
"palmer@...belt.com" <palmer@...belt.com>
CC: "conor@...nel.org" <conor@...nel.org>,
Paul Walmsley <paul.walmsley@...ive.com>,
Atish Patra <atishp@...osinc.com>,
Anup Patel <apatel@...tanamicro.com>,
Alexandre Ghiti <alexghiti@...osinc.com>,
Björn Töpel <bjorn@...osinc.com>,
Song Shuai <suagrfillet@...il.com>,
Petr Tesarik <petrtesarik@...weicloud.com>,
"linux-riscv@...ts.infradead.org" <linux-riscv@...ts.infradead.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"stable@...r.kernel.org" <stable@...r.kernel.org>
Subject: RE: [RFT 1/2] RISC-V: handle missing "no-map" properties for
OpenSBI's PMP protected regions
> -----Original Message-----
> From: Conor Dooley <conor.dooley@...rochip.com>
> Sent: Wednesday, August 2, 2023 7:13 PM
> To: palmer@...belt.com
> Cc: conor@...nel.org; conor.dooley@...rochip.com; Paul Walmsley <paul.walmsley@...ive.com>; Atish Patra
> <atishp@...osinc.com>; Anup Patel <apatel@...tanamicro.com>; Alexandre Ghiti <alexghiti@...osinc.com>; Björn Töpel
> <bjorn@...osinc.com>; Song Shuai <suagrfillet@...il.com>; JeeHeng Sia <jeeheng.sia@...rfivetech.com>; Petr Tesarik
> <petrtesarik@...weicloud.com>; linux-riscv@...ts.infradead.org; linux-kernel@...r.kernel.org; stable@...r.kernel.org
> 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@...il.com>
> Link: https://lore.kernel.org/all/CAAYs2=gQvkhTeioMmqRDVGjdtNF_vhB+vm_1dHJxPNi75YDQ_Q@mail.gmail.com/
> Reported-by: JeeHeng Sia <jeeheng.sia@...rfivetech.com>
> Link: https://groups.google.com/a/groups.riscv.org/g/sw-dev/c/ITXwaKfA6z8
> Reported-by: Petr Tesarik <petrtesarik@...weicloud.com>
> Closes: https://lore.kernel.org/linux-riscv/76ff0f51-d6c1-580d-f943-061e93073306@huaweicloud.com/ [1]
> CC: stable@...r.kernel.org
> Signed-off-by: Conor Dooley <conor.dooley@...rochip.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
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
Powered by blists - more mailing lists