| Message ID | 20230530080425.18612-2-alexghiti@rivosinc.com |
|---|---|
| State | Rejected, archived |
| Headers | show |
| Series | fdt: Mark "/reserved-memory" nodes as nosave if !reusable | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/patch-applied | fail | build log |
On Tue, May 30, 2023 at 10:04:25AM +0200, Alexandre Ghiti wrote: > The hibernation process will access those reserved memory regions if > they are part of the linear mapping, but as described in > devicetree/bindings/reserved-memory/reserved-memory.yaml, > "/reserved-memory" nodes should not be used as normal memory, unless they > are marked as reusable which means the kernel can access it at some point. > > Otherwise those regions are only used by drivers which should do what's > necessary when the hibernation process is started, or they can contain > the firmware reserved memory regions which should not be accessed at all. Hibernation is only one case. Speculative accesses could also occur. I think some of the memory debugging stuff will walk memory as well. If something can't be accessed, it better have 'no-map'. > > Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com> > --- > arch/riscv/kernel/setup.c | 2 + How is this specific to Risc-V? Hint, it's not. > drivers/of/fdt.c | 77 +++++++++++++++++++++++++++++++++++++++ > include/linux/of_fdt.h | 1 + > 3 files changed, 80 insertions(+) > > diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c > index 36b026057503..642f1035b5ce 100644 > --- a/arch/riscv/kernel/setup.c > +++ b/arch/riscv/kernel/setup.c > @@ -299,6 +299,8 @@ void __init setup_arch(char **cmdline_p) > if (IS_ENABLED(CONFIG_RISCV_ISA_ZICBOM) && > riscv_isa_extension_available(NULL, ZICBOM)) > riscv_noncoherent_supported(); > + > + early_init_fdt_nosave_reserved_mem(); > } > > static int __init topology_init(void) > diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c > index bf502ba8da95..863de7e6b10c 100644 > --- a/drivers/of/fdt.c > +++ b/drivers/of/fdt.c > @@ -26,6 +26,7 @@ > #include <linux/serial_core.h> > #include <linux/sysfs.h> > #include <linux/random.h> > +#include <linux/suspend.h> > > #include <asm/setup.h> /* for COMMAND_LINE_SIZE */ > #include <asm/page.h> > @@ -494,6 +495,43 @@ static int __init early_init_dt_reserve_memory(phys_addr_t base, > return memblock_reserve(base, size); > } > > +/* > + * __reserved_mem_nosave_reg() - Make all memory described in 'reg' property as > + * nosave, unless it is "reusable". > + */ > +static void __init __reserved_mem_nosave_reg(unsigned long node, > + const char *uname) > +{ > + int t_len = (dt_root_addr_cells + dt_root_size_cells) * sizeof(__be32); > + phys_addr_t base, size; > + int len; > + const __be32 *prop; > + bool reusable; > + > + prop = of_get_flat_dt_prop(node, "reg", &len); > + if (!prop) > + return; > + > + if (len && len % t_len != 0) { > + pr_err("Reserved memory: invalid reg property in '%s', skipping node.\n", > + uname); > + return; > + } > + > + reusable = of_get_flat_dt_prop(node, "reusable", NULL) != NULL; > + > + while (len >= t_len) { > + base = dt_mem_next_cell(dt_root_addr_cells, &prop); > + size = dt_mem_next_cell(dt_root_size_cells, &prop); > + > + if (size && !reusable) > + register_nosave_region(phys_to_pfn(base), > + phys_to_pfn(base + size)); > + > + len -= t_len; > + } > +} > + > /* > * __reserved_mem_reserve_reg() - reserve all memory described in 'reg' property > */ > @@ -596,6 +634,38 @@ static int __init fdt_scan_reserved_mem(void) > return 0; > } > > +/* > + * fdt_nosave_reserved_mem() - scan a single FDT node to mark reserved memory > + * as nosave. > + */ > +static int __init fdt_nosave_reserved_mem(void) > +{ > + int node, child; > + const void *fdt = initial_boot_params; > + > + node = fdt_path_offset(fdt, "/reserved-memory"); > + if (node < 0) > + return -ENODEV; > + > + if (__reserved_mem_check_root(node) != 0) { > + pr_err("Reserved memory: unsupported node format, ignoring\n"); > + return -EINVAL; > + } > + > + fdt_for_each_subnode(child, fdt, node) { > + const char *uname; > + > + if (!of_fdt_device_is_available(fdt, child)) > + continue; > + > + uname = fdt_get_name(fdt, child, NULL); > + > + __reserved_mem_nosave_reg(child, uname); > + } > + > + return 0; > +} > + > /* > * fdt_reserve_elfcorehdr() - reserves memory for elf core header > * > @@ -649,6 +719,13 @@ void __init early_init_fdt_scan_reserved_mem(void) > fdt_init_reserved_mem(); > } > > +void __init early_init_fdt_nosave_reserved_mem(void) > +{ > +#ifdef CONFIG_HIBERNATION > + fdt_nosave_reserved_mem(); > +#endif > +} > + > /** > * early_init_fdt_reserve_self() - reserve the memory used by the FDT blob > */ > diff --git a/include/linux/of_fdt.h b/include/linux/of_fdt.h > index d69ad5bb1eb1..55eb5a0f7305 100644 > --- a/include/linux/of_fdt.h > +++ b/include/linux/of_fdt.h > @@ -63,6 +63,7 @@ extern int early_init_dt_scan_memory(void); > extern void early_init_dt_check_for_usable_mem_range(void); > extern int early_init_dt_scan_chosen_stdout(void); > extern void early_init_fdt_scan_reserved_mem(void); > +extern void early_init_fdt_nosave_reserved_mem(void); > extern void early_init_fdt_reserve_self(void); > extern void early_init_dt_add_memory_arch(u64 base, u64 size); > extern u64 dt_mem_next_cell(int s, const __be32 **cellp); > -- > 2.39.2 >
diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c index 36b026057503..642f1035b5ce 100644 --- a/arch/riscv/kernel/setup.c +++ b/arch/riscv/kernel/setup.c @@ -299,6 +299,8 @@ void __init setup_arch(char **cmdline_p) if (IS_ENABLED(CONFIG_RISCV_ISA_ZICBOM) && riscv_isa_extension_available(NULL, ZICBOM)) riscv_noncoherent_supported(); + + early_init_fdt_nosave_reserved_mem(); } static int __init topology_init(void) diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c index bf502ba8da95..863de7e6b10c 100644 --- a/drivers/of/fdt.c +++ b/drivers/of/fdt.c @@ -26,6 +26,7 @@ #include <linux/serial_core.h> #include <linux/sysfs.h> #include <linux/random.h> +#include <linux/suspend.h> #include <asm/setup.h> /* for COMMAND_LINE_SIZE */ #include <asm/page.h> @@ -494,6 +495,43 @@ static int __init early_init_dt_reserve_memory(phys_addr_t base, return memblock_reserve(base, size); } +/* + * __reserved_mem_nosave_reg() - Make all memory described in 'reg' property as + * nosave, unless it is "reusable". + */ +static void __init __reserved_mem_nosave_reg(unsigned long node, + const char *uname) +{ + int t_len = (dt_root_addr_cells + dt_root_size_cells) * sizeof(__be32); + phys_addr_t base, size; + int len; + const __be32 *prop; + bool reusable; + + prop = of_get_flat_dt_prop(node, "reg", &len); + if (!prop) + return; + + if (len && len % t_len != 0) { + pr_err("Reserved memory: invalid reg property in '%s', skipping node.\n", + uname); + return; + } + + reusable = of_get_flat_dt_prop(node, "reusable", NULL) != NULL; + + while (len >= t_len) { + base = dt_mem_next_cell(dt_root_addr_cells, &prop); + size = dt_mem_next_cell(dt_root_size_cells, &prop); + + if (size && !reusable) + register_nosave_region(phys_to_pfn(base), + phys_to_pfn(base + size)); + + len -= t_len; + } +} + /* * __reserved_mem_reserve_reg() - reserve all memory described in 'reg' property */ @@ -596,6 +634,38 @@ static int __init fdt_scan_reserved_mem(void) return 0; } +/* + * fdt_nosave_reserved_mem() - scan a single FDT node to mark reserved memory + * as nosave. + */ +static int __init fdt_nosave_reserved_mem(void) +{ + int node, child; + const void *fdt = initial_boot_params; + + node = fdt_path_offset(fdt, "/reserved-memory"); + if (node < 0) + return -ENODEV; + + if (__reserved_mem_check_root(node) != 0) { + pr_err("Reserved memory: unsupported node format, ignoring\n"); + return -EINVAL; + } + + fdt_for_each_subnode(child, fdt, node) { + const char *uname; + + if (!of_fdt_device_is_available(fdt, child)) + continue; + + uname = fdt_get_name(fdt, child, NULL); + + __reserved_mem_nosave_reg(child, uname); + } + + return 0; +} + /* * fdt_reserve_elfcorehdr() - reserves memory for elf core header * @@ -649,6 +719,13 @@ void __init early_init_fdt_scan_reserved_mem(void) fdt_init_reserved_mem(); } +void __init early_init_fdt_nosave_reserved_mem(void) +{ +#ifdef CONFIG_HIBERNATION + fdt_nosave_reserved_mem(); +#endif +} + /** * early_init_fdt_reserve_self() - reserve the memory used by the FDT blob */ diff --git a/include/linux/of_fdt.h b/include/linux/of_fdt.h index d69ad5bb1eb1..55eb5a0f7305 100644 --- a/include/linux/of_fdt.h +++ b/include/linux/of_fdt.h @@ -63,6 +63,7 @@ extern int early_init_dt_scan_memory(void); extern void early_init_dt_check_for_usable_mem_range(void); extern int early_init_dt_scan_chosen_stdout(void); extern void early_init_fdt_scan_reserved_mem(void); +extern void early_init_fdt_nosave_reserved_mem(void); extern void early_init_fdt_reserve_self(void); extern void early_init_dt_add_memory_arch(u64 base, u64 size); extern u64 dt_mem_next_cell(int s, const __be32 **cellp);
The hibernation process will access those reserved memory regions if they are part of the linear mapping, but as described in devicetree/bindings/reserved-memory/reserved-memory.yaml, "/reserved-memory" nodes should not be used as normal memory, unless they are marked as reusable which means the kernel can access it at some point. Otherwise those regions are only used by drivers which should do what's necessary when the hibernation process is started, or they can contain the firmware reserved memory regions which should not be accessed at all. Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com> --- arch/riscv/kernel/setup.c | 2 + drivers/of/fdt.c | 77 +++++++++++++++++++++++++++++++++++++++ include/linux/of_fdt.h | 1 + 3 files changed, 80 insertions(+)