* [PATCH 1/2] hw/riscv: spike: Add '/chosen/stdout-path' in device tree unconditionally
@ 2022-04-21 5:56 ` Bin Meng
0 siblings, 0 replies; 11+ messages in thread
From: Bin Meng @ 2022-04-21 5:56 UTC (permalink / raw)
To: Alistair Francis, qemu-devel, qemu-riscv
Cc: Bin Meng, Philippe Mathieu-Daudé
From: Bin Meng <bin.meng@windriver.com>
At present the adding '/chosen/stdout-path' property in device tree
is determined by whether a kernel command line is provided, which is
wrong. It should be added unconditionally.
Fixes: 8d8897accb1c ("hw/riscv: spike: Allow using binary firmware as bios")
Signed-off-by: Bin Meng <bin.meng@windriver.com>
---
hw/riscv/spike.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c
index d059a67f9b..1562b000bb 100644
--- a/hw/riscv/spike.c
+++ b/hw/riscv/spike.c
@@ -174,10 +174,11 @@ static void create_fdt(SpikeState *s, const MemMapEntry *memmap,
riscv_socket_fdt_write_distance_matrix(mc, fdt);
+ qemu_fdt_add_subnode(fdt, "/chosen");
+ qemu_fdt_setprop_string(fdt, "/chosen", "stdout-path", "/htif");
+
if (cmdline) {
- qemu_fdt_add_subnode(fdt, "/chosen");
qemu_fdt_setprop_string(fdt, "/chosen", "bootargs", cmdline);
- qemu_fdt_setprop_string(fdt, "/chosen", "stdout-path", "/htif");
}
}
--
2.25.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 1/2] hw/riscv: spike: Add '/chosen/stdout-path' in device tree unconditionally
@ 2022-04-21 5:56 ` Bin Meng
0 siblings, 0 replies; 11+ messages in thread
From: Bin Meng @ 2022-04-21 5:56 UTC (permalink / raw)
To: Alistair Francis, qemu-devel, qemu-riscv
Cc: Philippe Mathieu-Daudé, Bin Meng
From: Bin Meng <bin.meng@windriver.com>
At present the adding '/chosen/stdout-path' property in device tree
is determined by whether a kernel command line is provided, which is
wrong. It should be added unconditionally.
Fixes: 8d8897accb1c ("hw/riscv: spike: Allow using binary firmware as bios")
Signed-off-by: Bin Meng <bin.meng@windriver.com>
---
hw/riscv/spike.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c
index d059a67f9b..1562b000bb 100644
--- a/hw/riscv/spike.c
+++ b/hw/riscv/spike.c
@@ -174,10 +174,11 @@ static void create_fdt(SpikeState *s, const MemMapEntry *memmap,
riscv_socket_fdt_write_distance_matrix(mc, fdt);
+ qemu_fdt_add_subnode(fdt, "/chosen");
+ qemu_fdt_setprop_string(fdt, "/chosen", "stdout-path", "/htif");
+
if (cmdline) {
- qemu_fdt_add_subnode(fdt, "/chosen");
qemu_fdt_setprop_string(fdt, "/chosen", "bootargs", cmdline);
- qemu_fdt_setprop_string(fdt, "/chosen", "stdout-path", "/htif");
}
}
--
2.25.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 2/2] hw/riscv: Don't add empty bootargs to device tree
2022-04-21 5:56 ` Bin Meng
@ 2022-04-21 5:56 ` Bin Meng
-1 siblings, 0 replies; 11+ messages in thread
From: Bin Meng @ 2022-04-21 5:56 UTC (permalink / raw)
To: Alistair Francis, qemu-devel, qemu-riscv
Cc: Bin Meng, Philippe Mathieu-Daudé
From: Bin Meng <bin.meng@windriver.com>
Commit 7c28f4da20e5 ("RISC-V: Don't add NULL bootargs to device-tree")
tried to avoid adding *NULL* bootargs to device tree, but unfortunately
the changes were entirely useless, due to MachineState::kernel_cmdline
can't be NULL at all as the default value is given as an empty string.
(see hw/core/machine.c::machine_initfn()).
Note the wording of *NULL* bootargs is wrong. It can't be NULL otherwise
a segfault had already been observed by dereferencing the NULL pointer.
It should be worded as *empty" bootargs.
Fixes: 7c28f4da20e5 ("RISC-V: Don't add NULL bootargs to device-tree")
Signed-off-by: Bin Meng <bin.meng@windriver.com>
---
hw/riscv/microchip_pfsoc.c | 2 +-
hw/riscv/sifive_u.c | 2 +-
hw/riscv/spike.c | 2 +-
hw/riscv/virt.c | 2 +-
4 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/hw/riscv/microchip_pfsoc.c b/hw/riscv/microchip_pfsoc.c
index cafd1fc9ae..10a5d0e501 100644
--- a/hw/riscv/microchip_pfsoc.c
+++ b/hw/riscv/microchip_pfsoc.c
@@ -571,7 +571,7 @@ static void microchip_icicle_kit_machine_init(MachineState *machine)
"linux,initrd-end", end);
}
- if (machine->kernel_cmdline) {
+ if (machine->kernel_cmdline && *machine->kernel_cmdline) {
qemu_fdt_setprop_string(machine->fdt, "/chosen",
"bootargs", machine->kernel_cmdline);
}
diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
index 7fbc7dea42..cc8c7637cb 100644
--- a/hw/riscv/sifive_u.c
+++ b/hw/riscv/sifive_u.c
@@ -511,7 +511,7 @@ static void create_fdt(SiFiveUState *s, const MemMapEntry *memmap,
g_free(nodename);
update_bootargs:
- if (cmdline) {
+ if (cmdline && *cmdline) {
qemu_fdt_setprop_string(fdt, "/chosen", "bootargs", cmdline);
}
}
diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c
index 1562b000bb..068ba3493e 100644
--- a/hw/riscv/spike.c
+++ b/hw/riscv/spike.c
@@ -177,7 +177,7 @@ static void create_fdt(SpikeState *s, const MemMapEntry *memmap,
qemu_fdt_add_subnode(fdt, "/chosen");
qemu_fdt_setprop_string(fdt, "/chosen", "stdout-path", "/htif");
- if (cmdline) {
+ if (cmdline && *cmdline) {
qemu_fdt_setprop_string(fdt, "/chosen", "bootargs", cmdline);
}
}
diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index da50cbed43..a628a3abdf 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -998,7 +998,7 @@ static void create_fdt(RISCVVirtState *s, const MemMapEntry *memmap,
create_fdt_flash(s, memmap);
update_bootargs:
- if (cmdline) {
+ if (cmdline && *cmdline) {
qemu_fdt_setprop_string(mc->fdt, "/chosen", "bootargs", cmdline);
}
}
--
2.25.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 2/2] hw/riscv: Don't add empty bootargs to device tree
@ 2022-04-21 5:56 ` Bin Meng
0 siblings, 0 replies; 11+ messages in thread
From: Bin Meng @ 2022-04-21 5:56 UTC (permalink / raw)
To: Alistair Francis, qemu-devel, qemu-riscv
Cc: Philippe Mathieu-Daudé, Bin Meng
From: Bin Meng <bin.meng@windriver.com>
Commit 7c28f4da20e5 ("RISC-V: Don't add NULL bootargs to device-tree")
tried to avoid adding *NULL* bootargs to device tree, but unfortunately
the changes were entirely useless, due to MachineState::kernel_cmdline
can't be NULL at all as the default value is given as an empty string.
(see hw/core/machine.c::machine_initfn()).
Note the wording of *NULL* bootargs is wrong. It can't be NULL otherwise
a segfault had already been observed by dereferencing the NULL pointer.
It should be worded as *empty" bootargs.
Fixes: 7c28f4da20e5 ("RISC-V: Don't add NULL bootargs to device-tree")
Signed-off-by: Bin Meng <bin.meng@windriver.com>
---
hw/riscv/microchip_pfsoc.c | 2 +-
hw/riscv/sifive_u.c | 2 +-
hw/riscv/spike.c | 2 +-
hw/riscv/virt.c | 2 +-
4 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/hw/riscv/microchip_pfsoc.c b/hw/riscv/microchip_pfsoc.c
index cafd1fc9ae..10a5d0e501 100644
--- a/hw/riscv/microchip_pfsoc.c
+++ b/hw/riscv/microchip_pfsoc.c
@@ -571,7 +571,7 @@ static void microchip_icicle_kit_machine_init(MachineState *machine)
"linux,initrd-end", end);
}
- if (machine->kernel_cmdline) {
+ if (machine->kernel_cmdline && *machine->kernel_cmdline) {
qemu_fdt_setprop_string(machine->fdt, "/chosen",
"bootargs", machine->kernel_cmdline);
}
diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
index 7fbc7dea42..cc8c7637cb 100644
--- a/hw/riscv/sifive_u.c
+++ b/hw/riscv/sifive_u.c
@@ -511,7 +511,7 @@ static void create_fdt(SiFiveUState *s, const MemMapEntry *memmap,
g_free(nodename);
update_bootargs:
- if (cmdline) {
+ if (cmdline && *cmdline) {
qemu_fdt_setprop_string(fdt, "/chosen", "bootargs", cmdline);
}
}
diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c
index 1562b000bb..068ba3493e 100644
--- a/hw/riscv/spike.c
+++ b/hw/riscv/spike.c
@@ -177,7 +177,7 @@ static void create_fdt(SpikeState *s, const MemMapEntry *memmap,
qemu_fdt_add_subnode(fdt, "/chosen");
qemu_fdt_setprop_string(fdt, "/chosen", "stdout-path", "/htif");
- if (cmdline) {
+ if (cmdline && *cmdline) {
qemu_fdt_setprop_string(fdt, "/chosen", "bootargs", cmdline);
}
}
diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index da50cbed43..a628a3abdf 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -998,7 +998,7 @@ static void create_fdt(RISCVVirtState *s, const MemMapEntry *memmap,
create_fdt_flash(s, memmap);
update_bootargs:
- if (cmdline) {
+ if (cmdline && *cmdline) {
qemu_fdt_setprop_string(mc->fdt, "/chosen", "bootargs", cmdline);
}
}
--
2.25.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] hw/riscv: Don't add empty bootargs to device tree
2022-04-21 5:56 ` Bin Meng
(?)
@ 2022-04-21 6:13 ` Bin Meng
-1 siblings, 0 replies; 11+ messages in thread
From: Bin Meng @ 2022-04-21 6:13 UTC (permalink / raw)
To: Alistair Francis, qemu-devel@nongnu.org Developers, open list:RISC-V
Cc: Bin Meng, Philippe Mathieu-Daudé
+ Philippe's another email address as the redhat one is unreachable
On Thu, Apr 21, 2022 at 1:56 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> From: Bin Meng <bin.meng@windriver.com>
>
> Commit 7c28f4da20e5 ("RISC-V: Don't add NULL bootargs to device-tree")
> tried to avoid adding *NULL* bootargs to device tree, but unfortunately
> the changes were entirely useless, due to MachineState::kernel_cmdline
> can't be NULL at all as the default value is given as an empty string.
> (see hw/core/machine.c::machine_initfn()).
>
> Note the wording of *NULL* bootargs is wrong. It can't be NULL otherwise
> a segfault had already been observed by dereferencing the NULL pointer.
> It should be worded as *empty" bootargs.
>
> Fixes: 7c28f4da20e5 ("RISC-V: Don't add NULL bootargs to device-tree")
> Signed-off-by: Bin Meng <bin.meng@windriver.com>
> ---
>
> hw/riscv/microchip_pfsoc.c | 2 +-
> hw/riscv/sifive_u.c | 2 +-
> hw/riscv/spike.c | 2 +-
> hw/riscv/virt.c | 2 +-
> 4 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/hw/riscv/microchip_pfsoc.c b/hw/riscv/microchip_pfsoc.c
> index cafd1fc9ae..10a5d0e501 100644
> --- a/hw/riscv/microchip_pfsoc.c
> +++ b/hw/riscv/microchip_pfsoc.c
> @@ -571,7 +571,7 @@ static void microchip_icicle_kit_machine_init(MachineState *machine)
> "linux,initrd-end", end);
> }
>
> - if (machine->kernel_cmdline) {
> + if (machine->kernel_cmdline && *machine->kernel_cmdline) {
> qemu_fdt_setprop_string(machine->fdt, "/chosen",
> "bootargs", machine->kernel_cmdline);
> }
> diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
> index 7fbc7dea42..cc8c7637cb 100644
> --- a/hw/riscv/sifive_u.c
> +++ b/hw/riscv/sifive_u.c
> @@ -511,7 +511,7 @@ static void create_fdt(SiFiveUState *s, const MemMapEntry *memmap,
> g_free(nodename);
>
> update_bootargs:
> - if (cmdline) {
> + if (cmdline && *cmdline) {
> qemu_fdt_setprop_string(fdt, "/chosen", "bootargs", cmdline);
> }
> }
> diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c
> index 1562b000bb..068ba3493e 100644
> --- a/hw/riscv/spike.c
> +++ b/hw/riscv/spike.c
> @@ -177,7 +177,7 @@ static void create_fdt(SpikeState *s, const MemMapEntry *memmap,
> qemu_fdt_add_subnode(fdt, "/chosen");
> qemu_fdt_setprop_string(fdt, "/chosen", "stdout-path", "/htif");
>
> - if (cmdline) {
> + if (cmdline && *cmdline) {
> qemu_fdt_setprop_string(fdt, "/chosen", "bootargs", cmdline);
> }
> }
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index da50cbed43..a628a3abdf 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -998,7 +998,7 @@ static void create_fdt(RISCVVirtState *s, const MemMapEntry *memmap,
> create_fdt_flash(s, memmap);
>
> update_bootargs:
> - if (cmdline) {
> + if (cmdline && *cmdline) {
> qemu_fdt_setprop_string(mc->fdt, "/chosen", "bootargs", cmdline);
> }
> }
> --
Regards,
Bin
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] hw/riscv: spike: Add '/chosen/stdout-path' in device tree unconditionally
2022-04-21 5:56 ` Bin Meng
@ 2022-04-22 0:59 ` Alistair Francis
-1 siblings, 0 replies; 11+ messages in thread
From: Alistair Francis @ 2022-04-22 0:59 UTC (permalink / raw)
To: Bin Meng
Cc: open list:RISC-V, Bin Meng, Alistair Francis,
qemu-devel@nongnu.org Developers, Philippe Mathieu-Daudé
On Thu, Apr 21, 2022 at 3:57 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> From: Bin Meng <bin.meng@windriver.com>
>
> At present the adding '/chosen/stdout-path' property in device tree
> is determined by whether a kernel command line is provided, which is
> wrong. It should be added unconditionally.
>
> Fixes: 8d8897accb1c ("hw/riscv: spike: Allow using binary firmware as bios")
> Signed-off-by: Bin Meng <bin.meng@windriver.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
>
> hw/riscv/spike.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c
> index d059a67f9b..1562b000bb 100644
> --- a/hw/riscv/spike.c
> +++ b/hw/riscv/spike.c
> @@ -174,10 +174,11 @@ static void create_fdt(SpikeState *s, const MemMapEntry *memmap,
>
> riscv_socket_fdt_write_distance_matrix(mc, fdt);
>
> + qemu_fdt_add_subnode(fdt, "/chosen");
> + qemu_fdt_setprop_string(fdt, "/chosen", "stdout-path", "/htif");
> +
> if (cmdline) {
> - qemu_fdt_add_subnode(fdt, "/chosen");
> qemu_fdt_setprop_string(fdt, "/chosen", "bootargs", cmdline);
> - qemu_fdt_setprop_string(fdt, "/chosen", "stdout-path", "/htif");
> }
> }
>
> --
> 2.25.1
>
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] hw/riscv: spike: Add '/chosen/stdout-path' in device tree unconditionally
@ 2022-04-22 0:59 ` Alistair Francis
0 siblings, 0 replies; 11+ messages in thread
From: Alistair Francis @ 2022-04-22 0:59 UTC (permalink / raw)
To: Bin Meng
Cc: Alistair Francis, qemu-devel@nongnu.org Developers,
open list:RISC-V, Bin Meng, Philippe Mathieu-Daudé
On Thu, Apr 21, 2022 at 3:57 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> From: Bin Meng <bin.meng@windriver.com>
>
> At present the adding '/chosen/stdout-path' property in device tree
> is determined by whether a kernel command line is provided, which is
> wrong. It should be added unconditionally.
>
> Fixes: 8d8897accb1c ("hw/riscv: spike: Allow using binary firmware as bios")
> Signed-off-by: Bin Meng <bin.meng@windriver.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
>
> hw/riscv/spike.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c
> index d059a67f9b..1562b000bb 100644
> --- a/hw/riscv/spike.c
> +++ b/hw/riscv/spike.c
> @@ -174,10 +174,11 @@ static void create_fdt(SpikeState *s, const MemMapEntry *memmap,
>
> riscv_socket_fdt_write_distance_matrix(mc, fdt);
>
> + qemu_fdt_add_subnode(fdt, "/chosen");
> + qemu_fdt_setprop_string(fdt, "/chosen", "stdout-path", "/htif");
> +
> if (cmdline) {
> - qemu_fdt_add_subnode(fdt, "/chosen");
> qemu_fdt_setprop_string(fdt, "/chosen", "bootargs", cmdline);
> - qemu_fdt_setprop_string(fdt, "/chosen", "stdout-path", "/htif");
> }
> }
>
> --
> 2.25.1
>
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] hw/riscv: Don't add empty bootargs to device tree
2022-04-21 5:56 ` Bin Meng
@ 2022-04-22 1:07 ` Alistair Francis
-1 siblings, 0 replies; 11+ messages in thread
From: Alistair Francis @ 2022-04-22 1:07 UTC (permalink / raw)
To: Bin Meng
Cc: open list:RISC-V, Bin Meng, Alistair Francis,
qemu-devel@nongnu.org Developers, Philippe Mathieu-Daudé
On Thu, Apr 21, 2022 at 3:58 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> From: Bin Meng <bin.meng@windriver.com>
>
> Commit 7c28f4da20e5 ("RISC-V: Don't add NULL bootargs to device-tree")
> tried to avoid adding *NULL* bootargs to device tree, but unfortunately
> the changes were entirely useless, due to MachineState::kernel_cmdline
> can't be NULL at all as the default value is given as an empty string.
> (see hw/core/machine.c::machine_initfn()).
>
> Note the wording of *NULL* bootargs is wrong. It can't be NULL otherwise
> a segfault had already been observed by dereferencing the NULL pointer.
> It should be worded as *empty" bootargs.
>
> Fixes: 7c28f4da20e5 ("RISC-V: Don't add NULL bootargs to device-tree")
> Signed-off-by: Bin Meng <bin.meng@windriver.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
>
> hw/riscv/microchip_pfsoc.c | 2 +-
> hw/riscv/sifive_u.c | 2 +-
> hw/riscv/spike.c | 2 +-
> hw/riscv/virt.c | 2 +-
> 4 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/hw/riscv/microchip_pfsoc.c b/hw/riscv/microchip_pfsoc.c
> index cafd1fc9ae..10a5d0e501 100644
> --- a/hw/riscv/microchip_pfsoc.c
> +++ b/hw/riscv/microchip_pfsoc.c
> @@ -571,7 +571,7 @@ static void microchip_icicle_kit_machine_init(MachineState *machine)
> "linux,initrd-end", end);
> }
>
> - if (machine->kernel_cmdline) {
> + if (machine->kernel_cmdline && *machine->kernel_cmdline) {
> qemu_fdt_setprop_string(machine->fdt, "/chosen",
> "bootargs", machine->kernel_cmdline);
> }
> diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
> index 7fbc7dea42..cc8c7637cb 100644
> --- a/hw/riscv/sifive_u.c
> +++ b/hw/riscv/sifive_u.c
> @@ -511,7 +511,7 @@ static void create_fdt(SiFiveUState *s, const MemMapEntry *memmap,
> g_free(nodename);
>
> update_bootargs:
> - if (cmdline) {
> + if (cmdline && *cmdline) {
> qemu_fdt_setprop_string(fdt, "/chosen", "bootargs", cmdline);
> }
> }
> diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c
> index 1562b000bb..068ba3493e 100644
> --- a/hw/riscv/spike.c
> +++ b/hw/riscv/spike.c
> @@ -177,7 +177,7 @@ static void create_fdt(SpikeState *s, const MemMapEntry *memmap,
> qemu_fdt_add_subnode(fdt, "/chosen");
> qemu_fdt_setprop_string(fdt, "/chosen", "stdout-path", "/htif");
>
> - if (cmdline) {
> + if (cmdline && *cmdline) {
> qemu_fdt_setprop_string(fdt, "/chosen", "bootargs", cmdline);
> }
> }
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index da50cbed43..a628a3abdf 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -998,7 +998,7 @@ static void create_fdt(RISCVVirtState *s, const MemMapEntry *memmap,
> create_fdt_flash(s, memmap);
>
> update_bootargs:
> - if (cmdline) {
> + if (cmdline && *cmdline) {
> qemu_fdt_setprop_string(mc->fdt, "/chosen", "bootargs", cmdline);
> }
> }
> --
> 2.25.1
>
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] hw/riscv: Don't add empty bootargs to device tree
@ 2022-04-22 1:07 ` Alistair Francis
0 siblings, 0 replies; 11+ messages in thread
From: Alistair Francis @ 2022-04-22 1:07 UTC (permalink / raw)
To: Bin Meng
Cc: Alistair Francis, qemu-devel@nongnu.org Developers,
open list:RISC-V, Bin Meng, Philippe Mathieu-Daudé
On Thu, Apr 21, 2022 at 3:58 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> From: Bin Meng <bin.meng@windriver.com>
>
> Commit 7c28f4da20e5 ("RISC-V: Don't add NULL bootargs to device-tree")
> tried to avoid adding *NULL* bootargs to device tree, but unfortunately
> the changes were entirely useless, due to MachineState::kernel_cmdline
> can't be NULL at all as the default value is given as an empty string.
> (see hw/core/machine.c::machine_initfn()).
>
> Note the wording of *NULL* bootargs is wrong. It can't be NULL otherwise
> a segfault had already been observed by dereferencing the NULL pointer.
> It should be worded as *empty" bootargs.
>
> Fixes: 7c28f4da20e5 ("RISC-V: Don't add NULL bootargs to device-tree")
> Signed-off-by: Bin Meng <bin.meng@windriver.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
>
> hw/riscv/microchip_pfsoc.c | 2 +-
> hw/riscv/sifive_u.c | 2 +-
> hw/riscv/spike.c | 2 +-
> hw/riscv/virt.c | 2 +-
> 4 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/hw/riscv/microchip_pfsoc.c b/hw/riscv/microchip_pfsoc.c
> index cafd1fc9ae..10a5d0e501 100644
> --- a/hw/riscv/microchip_pfsoc.c
> +++ b/hw/riscv/microchip_pfsoc.c
> @@ -571,7 +571,7 @@ static void microchip_icicle_kit_machine_init(MachineState *machine)
> "linux,initrd-end", end);
> }
>
> - if (machine->kernel_cmdline) {
> + if (machine->kernel_cmdline && *machine->kernel_cmdline) {
> qemu_fdt_setprop_string(machine->fdt, "/chosen",
> "bootargs", machine->kernel_cmdline);
> }
> diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
> index 7fbc7dea42..cc8c7637cb 100644
> --- a/hw/riscv/sifive_u.c
> +++ b/hw/riscv/sifive_u.c
> @@ -511,7 +511,7 @@ static void create_fdt(SiFiveUState *s, const MemMapEntry *memmap,
> g_free(nodename);
>
> update_bootargs:
> - if (cmdline) {
> + if (cmdline && *cmdline) {
> qemu_fdt_setprop_string(fdt, "/chosen", "bootargs", cmdline);
> }
> }
> diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c
> index 1562b000bb..068ba3493e 100644
> --- a/hw/riscv/spike.c
> +++ b/hw/riscv/spike.c
> @@ -177,7 +177,7 @@ static void create_fdt(SpikeState *s, const MemMapEntry *memmap,
> qemu_fdt_add_subnode(fdt, "/chosen");
> qemu_fdt_setprop_string(fdt, "/chosen", "stdout-path", "/htif");
>
> - if (cmdline) {
> + if (cmdline && *cmdline) {
> qemu_fdt_setprop_string(fdt, "/chosen", "bootargs", cmdline);
> }
> }
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index da50cbed43..a628a3abdf 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -998,7 +998,7 @@ static void create_fdt(RISCVVirtState *s, const MemMapEntry *memmap,
> create_fdt_flash(s, memmap);
>
> update_bootargs:
> - if (cmdline) {
> + if (cmdline && *cmdline) {
> qemu_fdt_setprop_string(mc->fdt, "/chosen", "bootargs", cmdline);
> }
> }
> --
> 2.25.1
>
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] hw/riscv: Don't add empty bootargs to device tree
2022-04-21 5:56 ` Bin Meng
@ 2022-04-22 23:03 ` Alistair Francis
-1 siblings, 0 replies; 11+ messages in thread
From: Alistair Francis @ 2022-04-22 23:03 UTC (permalink / raw)
To: Bin Meng
Cc: open list:RISC-V, Bin Meng, Alistair Francis,
qemu-devel@nongnu.org Developers, Philippe Mathieu-Daudé
On Thu, Apr 21, 2022 at 3:58 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> From: Bin Meng <bin.meng@windriver.com>
>
> Commit 7c28f4da20e5 ("RISC-V: Don't add NULL bootargs to device-tree")
> tried to avoid adding *NULL* bootargs to device tree, but unfortunately
> the changes were entirely useless, due to MachineState::kernel_cmdline
> can't be NULL at all as the default value is given as an empty string.
> (see hw/core/machine.c::machine_initfn()).
>
> Note the wording of *NULL* bootargs is wrong. It can't be NULL otherwise
> a segfault had already been observed by dereferencing the NULL pointer.
> It should be worded as *empty" bootargs.
>
> Fixes: 7c28f4da20e5 ("RISC-V: Don't add NULL bootargs to device-tree")
> Signed-off-by: Bin Meng <bin.meng@windriver.com>
Thanks!
Applied to riscv-to-apply.next
Alistair
> ---
>
> hw/riscv/microchip_pfsoc.c | 2 +-
> hw/riscv/sifive_u.c | 2 +-
> hw/riscv/spike.c | 2 +-
> hw/riscv/virt.c | 2 +-
> 4 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/hw/riscv/microchip_pfsoc.c b/hw/riscv/microchip_pfsoc.c
> index cafd1fc9ae..10a5d0e501 100644
> --- a/hw/riscv/microchip_pfsoc.c
> +++ b/hw/riscv/microchip_pfsoc.c
> @@ -571,7 +571,7 @@ static void microchip_icicle_kit_machine_init(MachineState *machine)
> "linux,initrd-end", end);
> }
>
> - if (machine->kernel_cmdline) {
> + if (machine->kernel_cmdline && *machine->kernel_cmdline) {
> qemu_fdt_setprop_string(machine->fdt, "/chosen",
> "bootargs", machine->kernel_cmdline);
> }
> diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
> index 7fbc7dea42..cc8c7637cb 100644
> --- a/hw/riscv/sifive_u.c
> +++ b/hw/riscv/sifive_u.c
> @@ -511,7 +511,7 @@ static void create_fdt(SiFiveUState *s, const MemMapEntry *memmap,
> g_free(nodename);
>
> update_bootargs:
> - if (cmdline) {
> + if (cmdline && *cmdline) {
> qemu_fdt_setprop_string(fdt, "/chosen", "bootargs", cmdline);
> }
> }
> diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c
> index 1562b000bb..068ba3493e 100644
> --- a/hw/riscv/spike.c
> +++ b/hw/riscv/spike.c
> @@ -177,7 +177,7 @@ static void create_fdt(SpikeState *s, const MemMapEntry *memmap,
> qemu_fdt_add_subnode(fdt, "/chosen");
> qemu_fdt_setprop_string(fdt, "/chosen", "stdout-path", "/htif");
>
> - if (cmdline) {
> + if (cmdline && *cmdline) {
> qemu_fdt_setprop_string(fdt, "/chosen", "bootargs", cmdline);
> }
> }
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index da50cbed43..a628a3abdf 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -998,7 +998,7 @@ static void create_fdt(RISCVVirtState *s, const MemMapEntry *memmap,
> create_fdt_flash(s, memmap);
>
> update_bootargs:
> - if (cmdline) {
> + if (cmdline && *cmdline) {
> qemu_fdt_setprop_string(mc->fdt, "/chosen", "bootargs", cmdline);
> }
> }
> --
> 2.25.1
>
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] hw/riscv: Don't add empty bootargs to device tree
@ 2022-04-22 23:03 ` Alistair Francis
0 siblings, 0 replies; 11+ messages in thread
From: Alistair Francis @ 2022-04-22 23:03 UTC (permalink / raw)
To: Bin Meng
Cc: Alistair Francis, qemu-devel@nongnu.org Developers,
open list:RISC-V, Bin Meng, Philippe Mathieu-Daudé
On Thu, Apr 21, 2022 at 3:58 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> From: Bin Meng <bin.meng@windriver.com>
>
> Commit 7c28f4da20e5 ("RISC-V: Don't add NULL bootargs to device-tree")
> tried to avoid adding *NULL* bootargs to device tree, but unfortunately
> the changes were entirely useless, due to MachineState::kernel_cmdline
> can't be NULL at all as the default value is given as an empty string.
> (see hw/core/machine.c::machine_initfn()).
>
> Note the wording of *NULL* bootargs is wrong. It can't be NULL otherwise
> a segfault had already been observed by dereferencing the NULL pointer.
> It should be worded as *empty" bootargs.
>
> Fixes: 7c28f4da20e5 ("RISC-V: Don't add NULL bootargs to device-tree")
> Signed-off-by: Bin Meng <bin.meng@windriver.com>
Thanks!
Applied to riscv-to-apply.next
Alistair
> ---
>
> hw/riscv/microchip_pfsoc.c | 2 +-
> hw/riscv/sifive_u.c | 2 +-
> hw/riscv/spike.c | 2 +-
> hw/riscv/virt.c | 2 +-
> 4 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/hw/riscv/microchip_pfsoc.c b/hw/riscv/microchip_pfsoc.c
> index cafd1fc9ae..10a5d0e501 100644
> --- a/hw/riscv/microchip_pfsoc.c
> +++ b/hw/riscv/microchip_pfsoc.c
> @@ -571,7 +571,7 @@ static void microchip_icicle_kit_machine_init(MachineState *machine)
> "linux,initrd-end", end);
> }
>
> - if (machine->kernel_cmdline) {
> + if (machine->kernel_cmdline && *machine->kernel_cmdline) {
> qemu_fdt_setprop_string(machine->fdt, "/chosen",
> "bootargs", machine->kernel_cmdline);
> }
> diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
> index 7fbc7dea42..cc8c7637cb 100644
> --- a/hw/riscv/sifive_u.c
> +++ b/hw/riscv/sifive_u.c
> @@ -511,7 +511,7 @@ static void create_fdt(SiFiveUState *s, const MemMapEntry *memmap,
> g_free(nodename);
>
> update_bootargs:
> - if (cmdline) {
> + if (cmdline && *cmdline) {
> qemu_fdt_setprop_string(fdt, "/chosen", "bootargs", cmdline);
> }
> }
> diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c
> index 1562b000bb..068ba3493e 100644
> --- a/hw/riscv/spike.c
> +++ b/hw/riscv/spike.c
> @@ -177,7 +177,7 @@ static void create_fdt(SpikeState *s, const MemMapEntry *memmap,
> qemu_fdt_add_subnode(fdt, "/chosen");
> qemu_fdt_setprop_string(fdt, "/chosen", "stdout-path", "/htif");
>
> - if (cmdline) {
> + if (cmdline && *cmdline) {
> qemu_fdt_setprop_string(fdt, "/chosen", "bootargs", cmdline);
> }
> }
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index da50cbed43..a628a3abdf 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -998,7 +998,7 @@ static void create_fdt(RISCVVirtState *s, const MemMapEntry *memmap,
> create_fdt_flash(s, memmap);
>
> update_bootargs:
> - if (cmdline) {
> + if (cmdline && *cmdline) {
> qemu_fdt_setprop_string(mc->fdt, "/chosen", "bootargs", cmdline);
> }
> }
> --
> 2.25.1
>
>
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2022-04-22 23:05 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-21 5:56 [PATCH 1/2] hw/riscv: spike: Add '/chosen/stdout-path' in device tree unconditionally Bin Meng
2022-04-21 5:56 ` Bin Meng
2022-04-21 5:56 ` [PATCH 2/2] hw/riscv: Don't add empty bootargs to device tree Bin Meng
2022-04-21 5:56 ` Bin Meng
2022-04-21 6:13 ` Bin Meng
2022-04-22 1:07 ` Alistair Francis
2022-04-22 1:07 ` Alistair Francis
2022-04-22 23:03 ` Alistair Francis
2022-04-22 23:03 ` Alistair Francis
2022-04-22 0:59 ` [PATCH 1/2] hw/riscv: spike: Add '/chosen/stdout-path' in device tree unconditionally Alistair Francis
2022-04-22 0:59 ` Alistair Francis
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.