qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 1/1] hw/arm/boot: set initrd with #[address/size]-cells type in fdt
@ 2022-11-29 10:48 Schspa Shi
  2022-11-29 11:07 ` Philippe Mathieu-Daudé
  2022-11-29 14:48 ` Peter Maydell
  0 siblings, 2 replies; 5+ messages in thread
From: Schspa Shi @ 2022-11-29 10:48 UTC (permalink / raw)
  To: peter.maydell; +Cc: qemu-arm, qemu-devel, Schspa Shi

We use 32bit value for linux,initrd-[start/end], when we have
loader_start > 4GB, there will be a wrong initrd_start passed
to the kernel, and the kernel will report the following warning.

[    0.000000] ------------[ cut here ]------------
[    0.000000] initrd not fully accessible via the linear mapping -- please check your bootloader ...
[    0.000000] WARNING: CPU: 0 PID: 0 at arch/arm64/mm/init.c:355 arm64_memblock_init+0x158/0x244
[    0.000000] Modules linked in:
[    0.000000] CPU: 0 PID: 0 Comm: swapper Tainted: G        W          6.1.0-rc3-13250-g30a0b95b1335-dirty #28
[    0.000000] Hardware name: Horizon Sigi Virtual development board (DT)
[    0.000000] pstate: 600000c5 (nZCv daIF -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[    0.000000] pc : arm64_memblock_init+0x158/0x244
[    0.000000] lr : arm64_memblock_init+0x158/0x244
[    0.000000] sp : ffff800009273df0
[    0.000000] x29: ffff800009273df0 x28: 0000001000cc0010 x27: 0000800000000000
[    0.000000] x26: 000000000050a3e2 x25: ffff800008b46000 x24: ffff800008b46000
[    0.000000] x23: ffff800008a53000 x22: ffff800009420000 x21: ffff800008a53000
[    0.000000] x20: 0000000004000000 x19: 0000000004000000 x18: 00000000ffff1020
[    0.000000] x17: 6568632065736165 x16: 6c70202d2d20676e x15: 697070616d207261
[    0.000000] x14: 656e696c20656874 x13: 0a2e2e2e20726564 x12: 0000000000000000
[    0.000000] x11: 0000000000000000 x10: 00000000ffffffff x9 : 0000000000000000
[    0.000000] x8 : 0000000000000000 x7 : 796c6c756620746f x6 : 6e20647274696e69
[    0.000000] x5 : ffff8000093c7c47 x4 : ffff800008a2102f x3 : ffff800009273a88
[    0.000000] x2 : 80000000fffff038 x1 : 00000000000000c0 x0 : 0000000000000056
[    0.000000] Call trace:
[    0.000000]  arm64_memblock_init+0x158/0x244
[    0.000000]  setup_arch+0x164/0x1cc
[    0.000000]  start_kernel+0x94/0x4ac
[    0.000000]  __primary_switched+0xb4/0xbc
[    0.000000] ---[ end trace 0000000000000000 ]---
[    0.000000] Zone ranges:
[    0.000000]   DMA      [mem 0x0000001000000000-0x0000001007ffffff]

To fix it, we can change it to #[address/size]-cells type.

Signed-off-by: Schspa Shi <schspa@gmail.com>

-- 

Changelog:
v1 -> v2:
        - Use #[address/size]-cells for data type.
---
 hw/arm/boot.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index 57efb61ee419..98cd1fdad2c6 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -638,15 +638,17 @@ int arm_load_dtb(hwaddr addr, const struct arm_boot_info *binfo,
     }
 
     if (binfo->initrd_size) {
-        rc = qemu_fdt_setprop_cell(fdt, "/chosen", "linux,initrd-start",
-                                   binfo->initrd_start);
+        rc = qemu_fdt_setprop_sized_cells(fdt, "/chosen", "linux,initrd-start",
+                                     acells, binfo->initrd_start);
         if (rc < 0) {
             fprintf(stderr, "couldn't set /chosen/linux,initrd-start\n");
             goto fail;
         }
 
-        rc = qemu_fdt_setprop_cell(fdt, "/chosen", "linux,initrd-end",
-                                   binfo->initrd_start + binfo->initrd_size);
+        rc = qemu_fdt_setprop_sized_cells(fdt, "/chosen", "linux,initrd-end",
+                                          scells,
+                                          binfo->initrd_start +
+                                          binfo->initrd_size);
         if (rc < 0) {
             fprintf(stderr, "couldn't set /chosen/linux,initrd-end\n");
             goto fail;
-- 
2.37.3



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v2 1/1] hw/arm/boot: set initrd with #[address/size]-cells type in fdt
  2022-11-29 10:48 [PATCH v2 1/1] hw/arm/boot: set initrd with #[address/size]-cells type in fdt Schspa Shi
@ 2022-11-29 11:07 ` Philippe Mathieu-Daudé
  2022-11-29 11:46   ` Schspa Shi
  2022-11-29 14:48 ` Peter Maydell
  1 sibling, 1 reply; 5+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-11-29 11:07 UTC (permalink / raw)
  To: Schspa Shi, peter.maydell; +Cc: qemu-arm, qemu-devel

On 29/11/22 11:48, Schspa Shi wrote:
> We use 32bit value for linux,initrd-[start/end], when we have
> loader_start > 4GB, there will be a wrong initrd_start passed
> to the kernel, and the kernel will report the following warning.
> 
> [    0.000000] ------------[ cut here ]------------
> [    0.000000] initrd not fully accessible via the linear mapping -- please check your bootloader ...
> [    0.000000] WARNING: CPU: 0 PID: 0 at arch/arm64/mm/init.c:355 arm64_memblock_init+0x158/0x244
> [    0.000000] Modules linked in:
> [    0.000000] CPU: 0 PID: 0 Comm: swapper Tainted: G        W          6.1.0-rc3-13250-g30a0b95b1335-dirty #28
> [    0.000000] Hardware name: Horizon Sigi Virtual development board (DT)
> [    0.000000] pstate: 600000c5 (nZCv daIF -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> [    0.000000] pc : arm64_memblock_init+0x158/0x244
> [    0.000000] lr : arm64_memblock_init+0x158/0x244
> [    0.000000] sp : ffff800009273df0
> [    0.000000] x29: ffff800009273df0 x28: 0000001000cc0010 x27: 0000800000000000
> [    0.000000] x26: 000000000050a3e2 x25: ffff800008b46000 x24: ffff800008b46000
> [    0.000000] x23: ffff800008a53000 x22: ffff800009420000 x21: ffff800008a53000
> [    0.000000] x20: 0000000004000000 x19: 0000000004000000 x18: 00000000ffff1020
> [    0.000000] x17: 6568632065736165 x16: 6c70202d2d20676e x15: 697070616d207261
> [    0.000000] x14: 656e696c20656874 x13: 0a2e2e2e20726564 x12: 0000000000000000
> [    0.000000] x11: 0000000000000000 x10: 00000000ffffffff x9 : 0000000000000000
> [    0.000000] x8 : 0000000000000000 x7 : 796c6c756620746f x6 : 6e20647274696e69
> [    0.000000] x5 : ffff8000093c7c47 x4 : ffff800008a2102f x3 : ffff800009273a88
> [    0.000000] x2 : 80000000fffff038 x1 : 00000000000000c0 x0 : 0000000000000056
> [    0.000000] Call trace:
> [    0.000000]  arm64_memblock_init+0x158/0x244
> [    0.000000]  setup_arch+0x164/0x1cc
> [    0.000000]  start_kernel+0x94/0x4ac
> [    0.000000]  __primary_switched+0xb4/0xbc
> [    0.000000] ---[ end trace 0000000000000000 ]---
> [    0.000000] Zone ranges:
> [    0.000000]   DMA      [mem 0x0000001000000000-0x0000001007ffffff]
> 
> To fix it, we can change it to #[address/size]-cells type.
> 
> Signed-off-by: Schspa Shi <schspa@gmail.com>
> 

Something is odd with this patch, using Thunderbird + Colored Diffs 
add-on can't see the patch content, but I can see it archived:
https://lore.kernel.org/qemu-devel/20221129104809.84860-1-schspa@gmail.com/

Maybe because you used '--' instead of the git '---' separator?


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2 1/1] hw/arm/boot: set initrd with #[address/size]-cells type in fdt
  2022-11-29 11:07 ` Philippe Mathieu-Daudé
@ 2022-11-29 11:46   ` Schspa Shi
  0 siblings, 0 replies; 5+ messages in thread
From: Schspa Shi @ 2022-11-29 11:46 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé; +Cc: peter.maydell, qemu-arm, qemu-devel


Philippe Mathieu-Daudé <philmd@linaro.org> writes:

> On 29/11/22 11:48, Schspa Shi wrote:
>> We use 32bit value for linux,initrd-[start/end], when we have
>> loader_start > 4GB, there will be a wrong initrd_start passed
>> to the kernel, and the kernel will report the following warning.
>> [    0.000000] ------------[ cut here ]------------
>> [    0.000000] initrd not fully accessible via the linear mapping -- please check your bootloader ...
>> [    0.000000] WARNING: CPU: 0 PID: 0 at arch/arm64/mm/init.c:355 arm64_memblock_init+0x158/0x244
>> [    0.000000] Modules linked in:
>> [    0.000000] CPU: 0 PID: 0 Comm: swapper Tainted: G        W          6.1.0-rc3-13250-g30a0b95b1335-dirty #28
>> [    0.000000] Hardware name: Horizon Sigi Virtual development board (DT)
>> [    0.000000] pstate: 600000c5 (nZCv daIF -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
>> [    0.000000] pc : arm64_memblock_init+0x158/0x244
>> [    0.000000] lr : arm64_memblock_init+0x158/0x244
>> [    0.000000] sp : ffff800009273df0
>> [    0.000000] x29: ffff800009273df0 x28: 0000001000cc0010 x27: 0000800000000000
>> [    0.000000] x26: 000000000050a3e2 x25: ffff800008b46000 x24: ffff800008b46000
>> [    0.000000] x23: ffff800008a53000 x22: ffff800009420000 x21: ffff800008a53000
>> [    0.000000] x20: 0000000004000000 x19: 0000000004000000 x18: 00000000ffff1020
>> [    0.000000] x17: 6568632065736165 x16: 6c70202d2d20676e x15: 697070616d207261
>> [    0.000000] x14: 656e696c20656874 x13: 0a2e2e2e20726564 x12: 0000000000000000
>> [    0.000000] x11: 0000000000000000 x10: 00000000ffffffff x9 : 0000000000000000
>> [    0.000000] x8 : 0000000000000000 x7 : 796c6c756620746f x6 : 6e20647274696e69
>> [    0.000000] x5 : ffff8000093c7c47 x4 : ffff800008a2102f x3 : ffff800009273a88
>> [    0.000000] x2 : 80000000fffff038 x1 : 00000000000000c0 x0 : 0000000000000056
>> [    0.000000] Call trace:
>> [    0.000000]  arm64_memblock_init+0x158/0x244
>> [    0.000000]  setup_arch+0x164/0x1cc
>> [    0.000000]  start_kernel+0x94/0x4ac
>> [    0.000000]  __primary_switched+0xb4/0xbc
>> [    0.000000] ---[ end trace 0000000000000000 ]---
>> [    0.000000] Zone ranges:
>> [    0.000000]   DMA      [mem 0x0000001000000000-0x0000001007ffffff]
>> To fix it, we can change it to #[address/size]-cells type.
>> Signed-off-by: Schspa Shi <schspa@gmail.com>
>> 
>
> Something is odd with this patch, using Thunderbird + Colored Diffs add-on can't
> see the patch content, but I can see it archived:
> https://lore.kernel.org/qemu-devel/20221129104809.84860-1-schspa@gmail.com/
>
> Maybe because you used '--' instead of the git '---' separator?

This maybe cause by the seprator above the Change log line.
I use '-- ' as seprator.

Should I resend a new V3 patch to fix this ?

-- 
BRs
Schspa Shi


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2 1/1] hw/arm/boot: set initrd with #[address/size]-cells type in fdt
  2022-11-29 10:48 [PATCH v2 1/1] hw/arm/boot: set initrd with #[address/size]-cells type in fdt Schspa Shi
  2022-11-29 11:07 ` Philippe Mathieu-Daudé
@ 2022-11-29 14:48 ` Peter Maydell
  2022-11-29 16:07   ` Schspa Shi
  1 sibling, 1 reply; 5+ messages in thread
From: Peter Maydell @ 2022-11-29 14:48 UTC (permalink / raw)
  To: Schspa Shi; +Cc: qemu-arm, qemu-devel

On Tue, 29 Nov 2022 at 10:48, Schspa Shi <schspa@gmail.com> wrote:
>
> We use 32bit value for linux,initrd-[start/end], when we have
> loader_start > 4GB, there will be a wrong initrd_start passed
> to the kernel, and the kernel will report the following warning.
>
> [    0.000000] ------------[ cut here ]------------
> [    0.000000] initrd not fully accessible via the linear mapping -- please check your bootloader ...
> [    0.000000] WARNING: CPU: 0 PID: 0 at arch/arm64/mm/init.c:355 arm64_memblock_init+0x158/0x244
> [    0.000000] Modules linked in:
> [    0.000000] CPU: 0 PID: 0 Comm: swapper Tainted: G        W          6.1.0-rc3-13250-g30a0b95b1335-dirty #28
> [    0.000000] Hardware name: Horizon Sigi Virtual development board (DT)
> [    0.000000] pstate: 600000c5 (nZCv daIF -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> [    0.000000] pc : arm64_memblock_init+0x158/0x244
> [    0.000000] lr : arm64_memblock_init+0x158/0x244
> [    0.000000] sp : ffff800009273df0
> [    0.000000] x29: ffff800009273df0 x28: 0000001000cc0010 x27: 0000800000000000
> [    0.000000] x26: 000000000050a3e2 x25: ffff800008b46000 x24: ffff800008b46000
> [    0.000000] x23: ffff800008a53000 x22: ffff800009420000 x21: ffff800008a53000
> [    0.000000] x20: 0000000004000000 x19: 0000000004000000 x18: 00000000ffff1020
> [    0.000000] x17: 6568632065736165 x16: 6c70202d2d20676e x15: 697070616d207261
> [    0.000000] x14: 656e696c20656874 x13: 0a2e2e2e20726564 x12: 0000000000000000
> [    0.000000] x11: 0000000000000000 x10: 00000000ffffffff x9 : 0000000000000000
> [    0.000000] x8 : 0000000000000000 x7 : 796c6c756620746f x6 : 6e20647274696e69
> [    0.000000] x5 : ffff8000093c7c47 x4 : ffff800008a2102f x3 : ffff800009273a88
> [    0.000000] x2 : 80000000fffff038 x1 : 00000000000000c0 x0 : 0000000000000056
> [    0.000000] Call trace:
> [    0.000000]  arm64_memblock_init+0x158/0x244
> [    0.000000]  setup_arch+0x164/0x1cc
> [    0.000000]  start_kernel+0x94/0x4ac
> [    0.000000]  __primary_switched+0xb4/0xbc
> [    0.000000] ---[ end trace 0000000000000000 ]---
> [    0.000000] Zone ranges:
> [    0.000000]   DMA      [mem 0x0000001000000000-0x0000001007ffffff]
>
> To fix it, we can change it to #[address/size]-cells type.
>
> Signed-off-by: Schspa Shi <schspa@gmail.com>
>
> --
>
> Changelog:
> v1 -> v2:
>         - Use #[address/size]-cells for data type.
> ---
>  hw/arm/boot.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/hw/arm/boot.c b/hw/arm/boot.c
> index 57efb61ee419..98cd1fdad2c6 100644
> --- a/hw/arm/boot.c
> +++ b/hw/arm/boot.c
> @@ -638,15 +638,17 @@ int arm_load_dtb(hwaddr addr, const struct arm_boot_info *binfo,
>      }
>
>      if (binfo->initrd_size) {
> -        rc = qemu_fdt_setprop_cell(fdt, "/chosen", "linux,initrd-start",
> -                                   binfo->initrd_start);
> +        rc = qemu_fdt_setprop_sized_cells(fdt, "/chosen", "linux,initrd-start",
> +                                     acells, binfo->initrd_start);

Indentation here doesn't look quite right.

>          if (rc < 0) {
>              fprintf(stderr, "couldn't set /chosen/linux,initrd-start\n");
>              goto fail;
>          }
>
> -        rc = qemu_fdt_setprop_cell(fdt, "/chosen", "linux,initrd-end",
> -                                   binfo->initrd_start + binfo->initrd_size);
> +        rc = qemu_fdt_setprop_sized_cells(fdt, "/chosen", "linux,initrd-end",
> +                                          scells,
> +                                          binfo->initrd_start +
> +                                          binfo->initrd_size);

This should use 'acells', not 'scells', because it's an address,
not a size.

thanks
-- PMM


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2 1/1] hw/arm/boot: set initrd with #[address/size]-cells type in fdt
  2022-11-29 14:48 ` Peter Maydell
@ 2022-11-29 16:07   ` Schspa Shi
  0 siblings, 0 replies; 5+ messages in thread
From: Schspa Shi @ 2022-11-29 16:07 UTC (permalink / raw)
  To: Peter Maydell; +Cc: qemu-arm, qemu-devel


Peter Maydell <peter.maydell@linaro.org> writes:

> On Tue, 29 Nov 2022 at 10:48, Schspa Shi <schspa@gmail.com> wrote:
>>
>> We use 32bit value for linux,initrd-[start/end], when we have
>> loader_start > 4GB, there will be a wrong initrd_start passed
>> to the kernel, and the kernel will report the following warning.
>>
>> [    0.000000] ------------[ cut here ]------------
>> [    0.000000] initrd not fully accessible via the linear mapping -- please check your bootloader ...
>> [    0.000000] WARNING: CPU: 0 PID: 0 at arch/arm64/mm/init.c:355 arm64_memblock_init+0x158/0x244
>> [    0.000000] Modules linked in:
>> [    0.000000] CPU: 0 PID: 0 Comm: swapper Tainted: G        W          6.1.0-rc3-13250-g30a0b95b1335-dirty #28
>> [    0.000000] Hardware name: Horizon Sigi Virtual development board (DT)
>> [    0.000000] pstate: 600000c5 (nZCv daIF -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
>> [    0.000000] pc : arm64_memblock_init+0x158/0x244
>> [    0.000000] lr : arm64_memblock_init+0x158/0x244
>> [    0.000000] sp : ffff800009273df0
>> [    0.000000] x29: ffff800009273df0 x28: 0000001000cc0010 x27: 0000800000000000
>> [    0.000000] x26: 000000000050a3e2 x25: ffff800008b46000 x24: ffff800008b46000
>> [    0.000000] x23: ffff800008a53000 x22: ffff800009420000 x21: ffff800008a53000
>> [    0.000000] x20: 0000000004000000 x19: 0000000004000000 x18: 00000000ffff1020
>> [    0.000000] x17: 6568632065736165 x16: 6c70202d2d20676e x15: 697070616d207261
>> [    0.000000] x14: 656e696c20656874 x13: 0a2e2e2e20726564 x12: 0000000000000000
>> [    0.000000] x11: 0000000000000000 x10: 00000000ffffffff x9 : 0000000000000000
>> [    0.000000] x8 : 0000000000000000 x7 : 796c6c756620746f x6 : 6e20647274696e69
>> [    0.000000] x5 : ffff8000093c7c47 x4 : ffff800008a2102f x3 : ffff800009273a88
>> [    0.000000] x2 : 80000000fffff038 x1 : 00000000000000c0 x0 : 0000000000000056
>> [    0.000000] Call trace:
>> [    0.000000]  arm64_memblock_init+0x158/0x244
>> [    0.000000]  setup_arch+0x164/0x1cc
>> [    0.000000]  start_kernel+0x94/0x4ac
>> [    0.000000]  __primary_switched+0xb4/0xbc
>> [    0.000000] ---[ end trace 0000000000000000 ]---
>> [    0.000000] Zone ranges:
>> [    0.000000]   DMA      [mem 0x0000001000000000-0x0000001007ffffff]
>>
>> To fix it, we can change it to #[address/size]-cells type.
>>
>> Signed-off-by: Schspa Shi <schspa@gmail.com>
>>
>> --
>>
>> Changelog:
>> v1 -> v2:
>>         - Use #[address/size]-cells for data type.
>> ---
>>  hw/arm/boot.c | 10 ++++++----
>>  1 file changed, 6 insertions(+), 4 deletions(-)
>>
>> diff --git a/hw/arm/boot.c b/hw/arm/boot.c
>> index 57efb61ee419..98cd1fdad2c6 100644
>> --- a/hw/arm/boot.c
>> +++ b/hw/arm/boot.c
>> @@ -638,15 +638,17 @@ int arm_load_dtb(hwaddr addr, const struct arm_boot_info *binfo,
>>      }
>>
>>      if (binfo->initrd_size) {
>> -        rc = qemu_fdt_setprop_cell(fdt, "/chosen", "linux,initrd-start",
>> -                                   binfo->initrd_start);
>> +        rc = qemu_fdt_setprop_sized_cells(fdt, "/chosen", "linux,initrd-start",
>> +                                     acells, binfo->initrd_start);
>
> Indentation here doesn't look quite right.
>

My mistake, and fixed in the v3 patch.

>>          if (rc < 0) {
>>              fprintf(stderr, "couldn't set /chosen/linux,initrd-start\n");
>>              goto fail;
>>          }
>>
>> -        rc = qemu_fdt_setprop_cell(fdt, "/chosen", "linux,initrd-end",
>> -                                   binfo->initrd_start + binfo->initrd_size);
>> +        rc = qemu_fdt_setprop_sized_cells(fdt, "/chosen", "linux,initrd-end",
>> +                                          scells,
>> +                                          binfo->initrd_start +
>> +                                          binfo->initrd_size);
>
> This should use 'acells', not 'scells', because it's an address,
> not a size.
>

It fixed in the v3 patch too.

> thanks
> -- PMM

-- 
BRs
Schspa Shi


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2022-11-29 16:10 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-11-29 10:48 [PATCH v2 1/1] hw/arm/boot: set initrd with #[address/size]-cells type in fdt Schspa Shi
2022-11-29 11:07 ` Philippe Mathieu-Daudé
2022-11-29 11:46   ` Schspa Shi
2022-11-29 14:48 ` Peter Maydell
2022-11-29 16:07   ` Schspa Shi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).