2022-11-07 16:00:14

by Conor Dooley

[permalink] [raw]
Subject: [PATCH v1] riscv: fix reserved memory setup

Currently, RISC-V sets up reserved memory using the "early" copy of the
device tree. As a result, when trying to get a reserved memory region
using of_reserved_mem_lookup(), the pointer to reserved memory regions
is using the early, pre-virtual-memory address which causes a kernel
panic when trying to use the buffer's name:

Unable to handle kernel paging request at virtual address 00000000401c31ac
Oops [#1]
Modules linked in:
CPU: 0 PID: 0 Comm: swapper Not tainted 6.0.0-rc1-00001-g0d9d6953d834 #1
Hardware name: Microchip PolarFire-SoC Icicle Kit (DT)
epc : string+0x4a/0xea
ra : vsnprintf+0x1e4/0x336
epc : ffffffff80335ea0 ra : ffffffff80338936 sp : ffffffff81203be0
gp : ffffffff812e0a98 tp : ffffffff8120de40 t0 : 0000000000000000
t1 : ffffffff81203e28 t2 : 7265736572203a46 s0 : ffffffff81203c20
s1 : ffffffff81203e28 a0 : ffffffff81203d22 a1 : 0000000000000000
a2 : ffffffff81203d08 a3 : 0000000081203d21 a4 : ffffffffffffffff
a5 : 00000000401c31ac a6 : ffff0a00ffffff04 a7 : ffffffffffffffff
s2 : ffffffff81203d08 s3 : ffffffff81203d00 s4 : 0000000000000008
s5 : ffffffff000000ff s6 : 0000000000ffffff s7 : 00000000ffffff00
s8 : ffffffff80d9821a s9 : ffffffff81203d22 s10: 0000000000000002
s11: ffffffff80d9821c t3 : ffffffff812f3617 t4 : ffffffff812f3617
t5 : ffffffff812f3618 t6 : ffffffff81203d08
status: 0000000200000100 badaddr: 00000000401c31ac cause: 000000000000000d
[<ffffffff80338936>] vsnprintf+0x1e4/0x336
[<ffffffff80055ae2>] vprintk_store+0xf6/0x344
[<ffffffff80055d86>] vprintk_emit+0x56/0x192
[<ffffffff80055ed8>] vprintk_default+0x16/0x1e
[<ffffffff800563d2>] vprintk+0x72/0x80
[<ffffffff806813b2>] _printk+0x36/0x50
[<ffffffff8068af48>] print_reserved_mem+0x1c/0x24
[<ffffffff808057ec>] paging_init+0x528/0x5bc
[<ffffffff808031ae>] setup_arch+0xd0/0x592
[<ffffffff8080070e>] start_kernel+0x82/0x73c

early_init_fdt_scan_reserved_mem() takes no arguments as it operates on
initial_boot_params, which is populated by early_init_dt_verify(). On
RISC-V, early_init_dt_verify() is called twice. Once, directly, in
setup_arch() if CONFIG_BUILTIN_DTB is not enabled and once indirectly,
very early in the boot process, by parse_dtb() when it calls
early_init_dt_scan_nodes().

This first call uses dtb_early_va to set initial_boot_params, which is
not usable later in the boot process when
early_init_fdt_scan_reserved_mem() is called. On arm64 for example, the
corresponding call to early_init_dt_scan_nodes() uses fixmap addresses
and doesn't suffer the same fate.

Move early_init_fdt_scan_reserved_mem() further along the boot sequence,
after the direct call to early_init_dt_verify() in setup_arch() so that
the names use the correct virtual memory addresses. The above supposed
that CONFIG_BUILTIN_DTB was not set, but should work equally in the case
where it is - unflatted_and_copy_device_tree() also updates
initial_boot_params.

Reported-by: Valentina Fernandez <[email protected]>
Reported-by: Evgenii Shatokhin <[email protected]>
Link: https://lore.kernel.org/linux-riscv/[email protected]/
Fixes: 922b0375fc93 ("riscv: Fix memblock reservation for device tree blob")
Signed-off-by: Conor Dooley <[email protected]>
---
arch/riscv/kernel/setup.c | 1 +
arch/riscv/mm/init.c | 1 -
2 files changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c
index ad76bb59b059..67ec1fadcfe2 100644
--- a/arch/riscv/kernel/setup.c
+++ b/arch/riscv/kernel/setup.c
@@ -283,6 +283,7 @@ void __init setup_arch(char **cmdline_p)
else
pr_err("No DTB found in kernel mappings\n");
#endif
+ early_init_fdt_scan_reserved_mem();
misc_mem_init();

init_resources();
diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
index b56a0a75533f..50a1b6edd491 100644
--- a/arch/riscv/mm/init.c
+++ b/arch/riscv/mm/init.c
@@ -262,7 +262,6 @@ static void __init setup_bootmem(void)
memblock_reserve(dtb_early_pa, fdt_totalsize(dtb_early_va));
}

- early_init_fdt_scan_reserved_mem();
dma_contiguous_reserve(dma32_phys_limit);
if (IS_ENABLED(CONFIG_64BIT))
hugetlb_cma_reserve(PUD_SHIFT - PAGE_SHIFT);
--
2.38.0



2022-11-07 22:13:25

by Evgenii Shatokhin

[permalink] [raw]
Subject: Re: [PATCH v1] riscv: fix reserved memory setup

Hi,

On 07.11.2022 18:15, Conor Dooley wrote:
> «Внимание! Данное письмо от внешнего адресата!»
>
> Currently, RISC-V sets up reserved memory using the "early" copy of the
> device tree. As a result, when trying to get a reserved memory region
> using of_reserved_mem_lookup(), the pointer to reserved memory regions
> is using the early, pre-virtual-memory address which causes a kernel
> panic when trying to use the buffer's name:
>
> Unable to handle kernel paging request at virtual address 00000000401c31ac
> Oops [#1]
> Modules linked in:
> CPU: 0 PID: 0 Comm: swapper Not tainted 6.0.0-rc1-00001-g0d9d6953d834 #1
> Hardware name: Microchip PolarFire-SoC Icicle Kit (DT)
> epc : string+0x4a/0xea
> ra : vsnprintf+0x1e4/0x336
> epc : ffffffff80335ea0 ra : ffffffff80338936 sp : ffffffff81203be0
> gp : ffffffff812e0a98 tp : ffffffff8120de40 t0 : 0000000000000000
> t1 : ffffffff81203e28 t2 : 7265736572203a46 s0 : ffffffff81203c20
> s1 : ffffffff81203e28 a0 : ffffffff81203d22 a1 : 0000000000000000
> a2 : ffffffff81203d08 a3 : 0000000081203d21 a4 : ffffffffffffffff
> a5 : 00000000401c31ac a6 : ffff0a00ffffff04 a7 : ffffffffffffffff
> s2 : ffffffff81203d08 s3 : ffffffff81203d00 s4 : 0000000000000008
> s5 : ffffffff000000ff s6 : 0000000000ffffff s7 : 00000000ffffff00
> s8 : ffffffff80d9821a s9 : ffffffff81203d22 s10: 0000000000000002
> s11: ffffffff80d9821c t3 : ffffffff812f3617 t4 : ffffffff812f3617
> t5 : ffffffff812f3618 t6 : ffffffff81203d08
> status: 0000000200000100 badaddr: 00000000401c31ac cause: 000000000000000d
> [<ffffffff80338936>] vsnprintf+0x1e4/0x336
> [<ffffffff80055ae2>] vprintk_store+0xf6/0x344
> [<ffffffff80055d86>] vprintk_emit+0x56/0x192
> [<ffffffff80055ed8>] vprintk_default+0x16/0x1e
> [<ffffffff800563d2>] vprintk+0x72/0x80
> [<ffffffff806813b2>] _printk+0x36/0x50
> [<ffffffff8068af48>] print_reserved_mem+0x1c/0x24
> [<ffffffff808057ec>] paging_init+0x528/0x5bc
> [<ffffffff808031ae>] setup_arch+0xd0/0x592
> [<ffffffff8080070e>] start_kernel+0x82/0x73c
>
> early_init_fdt_scan_reserved_mem() takes no arguments as it operates on
> initial_boot_params, which is populated by early_init_dt_verify(). On
> RISC-V, early_init_dt_verify() is called twice. Once, directly, in
> setup_arch() if CONFIG_BUILTIN_DTB is not enabled and once indirectly,
> very early in the boot process, by parse_dtb() when it calls
> early_init_dt_scan_nodes().
>
> This first call uses dtb_early_va to set initial_boot_params, which is
> not usable later in the boot process when
> early_init_fdt_scan_reserved_mem() is called. On arm64 for example, the
> corresponding call to early_init_dt_scan_nodes() uses fixmap addresses
> and doesn't suffer the same fate.

Thank you for looking into this!

As I wrote earlier, I ran into this issue too, while working on a
remoteproc driver for our RISC-V-based SoC.

The proposed patch fixes the bug for me and seems to have no problematic
side-effects. So:

Tested-by: Evgenii Shatokhin <[email protected]>

>
> Move early_init_fdt_scan_reserved_mem() further along the boot sequence,
> after the direct call to early_init_dt_verify() in setup_arch() so that
> the names use the correct virtual memory addresses. The above supposed
> that CONFIG_BUILTIN_DTB was not set, but should work equally in the case
> where it is - unflatted_and_copy_device_tree() also updates
> initial_boot_params.
>
> Reported-by: Valentina Fernandez <[email protected]>
> Reported-by: Evgenii Shatokhin <[email protected]>
> Link: https://lore.kernel.org/linux-riscv/[email protected]/
> Fixes: 922b0375fc93 ("riscv: Fix memblock reservation for device tree blob")
> Signed-off-by: Conor Dooley <[email protected]>
> ---
> arch/riscv/kernel/setup.c | 1 +
> arch/riscv/mm/init.c | 1 -
> 2 files changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c
> index ad76bb59b059..67ec1fadcfe2 100644
> --- a/arch/riscv/kernel/setup.c
> +++ b/arch/riscv/kernel/setup.c
> @@ -283,6 +283,7 @@ void __init setup_arch(char **cmdline_p)
> else
> pr_err("No DTB found in kernel mappings\n");
> #endif
> + early_init_fdt_scan_reserved_mem();
> misc_mem_init();
>
> init_resources();
> diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> index b56a0a75533f..50a1b6edd491 100644
> --- a/arch/riscv/mm/init.c
> +++ b/arch/riscv/mm/init.c
> @@ -262,7 +262,6 @@ static void __init setup_bootmem(void)
> memblock_reserve(dtb_early_pa, fdt_totalsize(dtb_early_va));
> }
>
> - early_init_fdt_scan_reserved_mem();
> dma_contiguous_reserve(dma32_phys_limit);
> if (IS_ENABLED(CONFIG_64BIT))
> hugetlb_cma_reserve(PUD_SHIFT - PAGE_SHIFT);
> --
> 2.38.0
>


2022-11-08 08:27:35

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v1] riscv: fix reserved memory setup

On Tue, Nov 08, 2022 at 12:40:19AM +0300, Evgenii Shatokhin wrote:
> Hi,

> Thank you for looking into this!
>
> As I wrote earlier, I ran into this issue too, while working on a remoteproc
> driver for our RISC-V-based SoC.
>
> The proposed patch fixes the bug for me and seems to have no problematic
> side-effects. So:
>
> Tested-by: Evgenii Shatokhin <[email protected]>

Apologies for forgetting to add the T-b tag & thanks for re-sending it.
Conor.

2022-11-10 23:08:32

by Palmer Dabbelt

[permalink] [raw]
Subject: Re: [PATCH v1] riscv: fix reserved memory setup

On Mon, 7 Nov 2022 15:15:25 +0000, Conor Dooley wrote:
> Currently, RISC-V sets up reserved memory using the "early" copy of the
> device tree. As a result, when trying to get a reserved memory region
> using of_reserved_mem_lookup(), the pointer to reserved memory regions
> is using the early, pre-virtual-memory address which causes a kernel
> panic when trying to use the buffer's name:
>
> Unable to handle kernel paging request at virtual address 00000000401c31ac
> Oops [#1]
> Modules linked in:
> CPU: 0 PID: 0 Comm: swapper Not tainted 6.0.0-rc1-00001-g0d9d6953d834 #1
> Hardware name: Microchip PolarFire-SoC Icicle Kit (DT)
> epc : string+0x4a/0xea
> ra : vsnprintf+0x1e4/0x336
> epc : ffffffff80335ea0 ra : ffffffff80338936 sp : ffffffff81203be0
> gp : ffffffff812e0a98 tp : ffffffff8120de40 t0 : 0000000000000000
> t1 : ffffffff81203e28 t2 : 7265736572203a46 s0 : ffffffff81203c20
> s1 : ffffffff81203e28 a0 : ffffffff81203d22 a1 : 0000000000000000
> a2 : ffffffff81203d08 a3 : 0000000081203d21 a4 : ffffffffffffffff
> a5 : 00000000401c31ac a6 : ffff0a00ffffff04 a7 : ffffffffffffffff
> s2 : ffffffff81203d08 s3 : ffffffff81203d00 s4 : 0000000000000008
> s5 : ffffffff000000ff s6 : 0000000000ffffff s7 : 00000000ffffff00
> s8 : ffffffff80d9821a s9 : ffffffff81203d22 s10: 0000000000000002
> s11: ffffffff80d9821c t3 : ffffffff812f3617 t4 : ffffffff812f3617
> t5 : ffffffff812f3618 t6 : ffffffff81203d08
> status: 0000000200000100 badaddr: 00000000401c31ac cause: 000000000000000d
> [<ffffffff80338936>] vsnprintf+0x1e4/0x336
> [<ffffffff80055ae2>] vprintk_store+0xf6/0x344
> [<ffffffff80055d86>] vprintk_emit+0x56/0x192
> [<ffffffff80055ed8>] vprintk_default+0x16/0x1e
> [<ffffffff800563d2>] vprintk+0x72/0x80
> [<ffffffff806813b2>] _printk+0x36/0x50
> [<ffffffff8068af48>] print_reserved_mem+0x1c/0x24
> [<ffffffff808057ec>] paging_init+0x528/0x5bc
> [<ffffffff808031ae>] setup_arch+0xd0/0x592
> [<ffffffff8080070e>] start_kernel+0x82/0x73c
>
> [...]

Applied, thanks!

[1/1] riscv: fix reserved memory setup
https://git.kernel.org/palmer/c/50e63dd8ed92

Best regards,
--
Palmer Dabbelt <[email protected]>

Subject: Re: [PATCH v1] riscv: fix reserved memory setup

Hello:

This patch was applied to riscv/linux.git (fixes)
by Palmer Dabbelt <[email protected]>:

On Mon, 7 Nov 2022 15:15:25 +0000 you wrote:
> Currently, RISC-V sets up reserved memory using the "early" copy of the
> device tree. As a result, when trying to get a reserved memory region
> using of_reserved_mem_lookup(), the pointer to reserved memory regions
> is using the early, pre-virtual-memory address which causes a kernel
> panic when trying to use the buffer's name:
>
> Unable to handle kernel paging request at virtual address 00000000401c31ac
> Oops [#1]
> Modules linked in:
> CPU: 0 PID: 0 Comm: swapper Not tainted 6.0.0-rc1-00001-g0d9d6953d834 #1
> Hardware name: Microchip PolarFire-SoC Icicle Kit (DT)
> epc : string+0x4a/0xea
> ra : vsnprintf+0x1e4/0x336
> epc : ffffffff80335ea0 ra : ffffffff80338936 sp : ffffffff81203be0
> gp : ffffffff812e0a98 tp : ffffffff8120de40 t0 : 0000000000000000
> t1 : ffffffff81203e28 t2 : 7265736572203a46 s0 : ffffffff81203c20
> s1 : ffffffff81203e28 a0 : ffffffff81203d22 a1 : 0000000000000000
> a2 : ffffffff81203d08 a3 : 0000000081203d21 a4 : ffffffffffffffff
> a5 : 00000000401c31ac a6 : ffff0a00ffffff04 a7 : ffffffffffffffff
> s2 : ffffffff81203d08 s3 : ffffffff81203d00 s4 : 0000000000000008
> s5 : ffffffff000000ff s6 : 0000000000ffffff s7 : 00000000ffffff00
> s8 : ffffffff80d9821a s9 : ffffffff81203d22 s10: 0000000000000002
> s11: ffffffff80d9821c t3 : ffffffff812f3617 t4 : ffffffff812f3617
> t5 : ffffffff812f3618 t6 : ffffffff81203d08
> status: 0000000200000100 badaddr: 00000000401c31ac cause: 000000000000000d
> [<ffffffff80338936>] vsnprintf+0x1e4/0x336
> [<ffffffff80055ae2>] vprintk_store+0xf6/0x344
> [<ffffffff80055d86>] vprintk_emit+0x56/0x192
> [<ffffffff80055ed8>] vprintk_default+0x16/0x1e
> [<ffffffff800563d2>] vprintk+0x72/0x80
> [<ffffffff806813b2>] _printk+0x36/0x50
> [<ffffffff8068af48>] print_reserved_mem+0x1c/0x24
> [<ffffffff808057ec>] paging_init+0x528/0x5bc
> [<ffffffff808031ae>] setup_arch+0xd0/0x592
> [<ffffffff8080070e>] start_kernel+0x82/0x73c
>
> [...]

Here is the summary with links:
- [v1] riscv: fix reserved memory setup
https://git.kernel.org/riscv/c/50e63dd8ed92

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html