Re: [PATCH] riscv: reserve DTB before possible memblock allocation

Conor Dooley
2023-06-07T18:17:41Z

[-- Attachment #1.1: Type: text/plain, Size: 2777 bytes --]

+CC Alex, you should take a look at this patch.

On Wed, Jun 07, 2023 at 09:35:19PM +0800, Woody Zhang wrote:
> It's possible that early_init_fdt_scan_reserved_mem() allocates memory
> from memblock for dynamic reserved memory in `/reserved-memory` node.
> Any fixed reservation must be done before that to avoid potential
> conflicts.
>
> Reserve the DTB in memblock just after early scanning it.

The rationale makes sense to me, I am just wondering what compelling
reason there is to move it away from the memblock_reserve()s for the
initd and vmlinux? Moving it above early_init_fdt_scan_reserved_mem()
should be the sufficient minimum & would keep things together.

Cheers,
Conor.

>
> Signed-off-by: Woody Zhang <woodylab@foxmail.com>
> ---
> arch/riscv/kernel/setup.c | 10 ++++++++++
> arch/riscv/mm/init.c | 9 ---------
> 2 files changed, 10 insertions(+), 9 deletions(-)
>
> diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c
> index 36b026057503..c147fa8da929 100644
> --- a/arch/riscv/kernel/setup.c
> +++ b/arch/riscv/kernel/setup.c
> @@ -16,6 +16,7 @@
> #include <linux/console.h>
> #include <linux/screen_info.h>
> #include <linux/of_fdt.h>
> +#include <linux/libfdt.h>
> #include <linux/sched/task.h>
> #include <linux/smp.h>
> #include <linux/efi.h>
> @@ -256,6 +257,15 @@ static void __init parse_dtb(void)
> pr_err("No DTB passed to the kernel\n");
> }
>
> + /*
> + * If DTB is built in, no need to reserve its memblock.
> + * Otherwise, do reserve it but avoid using
> + * early_init_fdt_reserve_self() since __pa() does
> + * not work for DTB pointers that are fixmap addresses
> + */
> + if (!IS_ENABLED(CONFIG_BUILTIN_DTB))
> + memblock_reserve(dtb_early_pa, fdt_totalsize(dtb_early_va));
> +
> #ifdef CONFIG_CMDLINE_FORCE
> strscpy(boot_command_line, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
> pr_info("Forcing kernel command line to: %s\n", boot_command_line);
> diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> index c6bb966e4123..f8c9a79acd94 100644
> --- a/arch/riscv/mm/init.c
> +++ b/arch/riscv/mm/init.c
> @@ -254,15 +254,6 @@ static void __init setup_bootmem(void)
> */
> early_init_fdt_scan_reserved_mem();
>
> - /*
> - * If DTB is built in, no need to reserve its memblock.
> - * Otherwise, do reserve it but avoid using
> - * early_init_fdt_reserve_self() since __pa() does
> - * not work for DTB pointers that are fixmap addresses
> - */
> - if (!IS_ENABLED(CONFIG_BUILTIN_DTB))
> - memblock_reserve(dtb_early_pa, fdt_totalsize(dtb_early_va));
> -
> dma_contiguous_reserve(dma32_phys_limit);
> if (IS_ENABLED(CONFIG_64BIT))
> hugetlb_cma_reserve(PUD_SHIFT - PAGE_SHIFT);
> --
> 2.39.2
>

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]