qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 1/2] hw/riscv: Skip re-generating DT nodes for a given DTB
@ 2023-02-28  7:45 Bin Meng
  2023-02-28  7:45 ` [PATCH v2 2/2] hw/riscv: Move the dtb load bits outside of create_fdt() Bin Meng
  2023-03-02  1:29 ` [PATCH v2 1/2] hw/riscv: Skip re-generating DT nodes for a given DTB Palmer Dabbelt
  0 siblings, 2 replies; 4+ messages in thread
From: Bin Meng @ 2023-02-28  7:45 UTC (permalink / raw)
  To: qemu-devel
  Cc: Daniel Henrique Barboza, Alistair Francis, Bin Meng, Liu Zhiwei,
	Palmer Dabbelt, Weiwei Li, qemu-riscv

Launch qemu-system-riscv64 with a given dtb for 'sifive_u' and 'virt'
machines, QEMU complains:

  qemu_fdt_add_subnode: Failed to create subnode /soc: FDT_ERR_EXISTS

The whole DT generation logic should be skipped when a given DTB is
present.

Fixes: b1f19f238cae ("hw/riscv: write bootargs 'chosen' FDT after riscv_load_kernel()")
Signed-off-by: Bin Meng <bmeng@tinylab.org>
Reviewed-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
---

(no changes since v1)

 hw/riscv/sifive_u.c | 1 +
 hw/riscv/virt.c     | 1 +
 2 files changed, 2 insertions(+)

diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
index ad3bb35b34..76db5ed3dd 100644
--- a/hw/riscv/sifive_u.c
+++ b/hw/riscv/sifive_u.c
@@ -118,6 +118,7 @@ static void create_fdt(SiFiveUState *s, const MemMapEntry *memmap,
             error_report("load_device_tree() failed");
             exit(1);
         }
+        return;
     } else {
         fdt = ms->fdt = create_device_tree(&fdt_size);
         if (!fdt) {
diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index 86c4adc0c9..0c7b4a1e46 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -1014,6 +1014,7 @@ static void create_fdt(RISCVVirtState *s, const MemMapEntry *memmap)
             error_report("load_device_tree() failed");
             exit(1);
         }
+        return;
     } else {
         ms->fdt = create_device_tree(&s->fdt_size);
         if (!ms->fdt) {
-- 
2.25.1



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

* [PATCH v2 2/2] hw/riscv: Move the dtb load bits outside of create_fdt()
  2023-02-28  7:45 [PATCH v2 1/2] hw/riscv: Skip re-generating DT nodes for a given DTB Bin Meng
@ 2023-02-28  7:45 ` Bin Meng
  2023-02-28  9:15   ` Daniel Henrique Barboza
  2023-03-02  1:29 ` [PATCH v2 1/2] hw/riscv: Skip re-generating DT nodes for a given DTB Palmer Dabbelt
  1 sibling, 1 reply; 4+ messages in thread
From: Bin Meng @ 2023-02-28  7:45 UTC (permalink / raw)
  To: qemu-devel
  Cc: Daniel Henrique Barboza, Alistair Francis, Bin Meng, Liu Zhiwei,
	Palmer Dabbelt, Weiwei Li, qemu-riscv

Move the dtb load bits outside of create_fdt(), and put it explicitly
in sifive_u_machine_init() and virt_machine_init(). With such change
create_fdt() does exactly what its function name tells us.

Suggested-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
Signed-off-by: Bin Meng <bmeng@tinylab.org>
---

Changes in v2:
- new patch: Move the dtb load bits outside of create_fdt()

 include/hw/riscv/sifive_u.h |  1 +
 hw/riscv/sifive_u.c         | 31 +++++++++++++++----------------
 hw/riscv/virt.c             | 29 ++++++++++++++---------------
 3 files changed, 30 insertions(+), 31 deletions(-)

diff --git a/include/hw/riscv/sifive_u.h b/include/hw/riscv/sifive_u.h
index 65af306963..0696f85942 100644
--- a/include/hw/riscv/sifive_u.h
+++ b/include/hw/riscv/sifive_u.h
@@ -68,6 +68,7 @@ typedef struct SiFiveUState {
 
     /*< public >*/
     SiFiveUSoCState soc;
+    int fdt_size;
 
     bool start_in_flash;
     uint32_t msel;
diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
index 76db5ed3dd..35a335b8d0 100644
--- a/hw/riscv/sifive_u.c
+++ b/hw/riscv/sifive_u.c
@@ -99,7 +99,7 @@ static void create_fdt(SiFiveUState *s, const MemMapEntry *memmap,
     MachineState *ms = MACHINE(s);
     uint64_t mem_size = ms->ram_size;
     void *fdt;
-    int cpu, fdt_size;
+    int cpu;
     uint32_t *cells;
     char *nodename;
     uint32_t plic_phandle, prci_phandle, gpio_phandle, phandle = 1;
@@ -112,19 +112,10 @@ static void create_fdt(SiFiveUState *s, const MemMapEntry *memmap,
         "sifive,plic-1.0.0", "riscv,plic0"
     };
 
-    if (ms->dtb) {
-        fdt = ms->fdt = load_device_tree(ms->dtb, &fdt_size);
-        if (!fdt) {
-            error_report("load_device_tree() failed");
-            exit(1);
-        }
-        return;
-    } else {
-        fdt = ms->fdt = create_device_tree(&fdt_size);
-        if (!fdt) {
-            error_report("create_device_tree() failed");
-            exit(1);
-        }
+    fdt = ms->fdt = create_device_tree(&s->fdt_size);
+    if (!fdt) {
+        error_report("create_device_tree() failed");
+        exit(1);
     }
 
     qemu_fdt_setprop_string(fdt, "/", "model", "SiFive HiFive Unleashed A00");
@@ -561,8 +552,16 @@ static void sifive_u_machine_init(MachineState *machine)
     qdev_connect_gpio_out(DEVICE(&(s->soc.gpio)), 10,
                           qemu_allocate_irq(sifive_u_machine_reset, NULL, 0));
 
-    /* create device tree */
-    create_fdt(s, memmap, riscv_is_32bit(&s->soc.u_cpus));
+    /* load/create device tree */
+    if (machine->dtb) {
+        machine->fdt = load_device_tree(machine->dtb, &s->fdt_size);
+        if (!machine->fdt) {
+            error_report("load_device_tree() failed");
+            exit(1);
+        }
+    } else {
+        create_fdt(s, memmap, riscv_is_32bit(&s->soc.u_cpus));
+    }
 
     if (s->start_in_flash) {
         /*
diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index 0c7b4a1e46..53ed2e8369 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -1008,19 +1008,10 @@ static void create_fdt(RISCVVirtState *s, const MemMapEntry *memmap)
     uint32_t irq_pcie_phandle = 1, irq_virtio_phandle = 1;
     uint8_t rng_seed[32];
 
-    if (ms->dtb) {
-        ms->fdt = load_device_tree(ms->dtb, &s->fdt_size);
-        if (!ms->fdt) {
-            error_report("load_device_tree() failed");
-            exit(1);
-        }
-        return;
-    } else {
-        ms->fdt = create_device_tree(&s->fdt_size);
-        if (!ms->fdt) {
-            error_report("create_device_tree() failed");
-            exit(1);
-        }
+    ms->fdt = create_device_tree(&s->fdt_size);
+    if (!ms->fdt) {
+        error_report("create_device_tree() failed");
+        exit(1);
     }
 
     qemu_fdt_setprop_string(ms->fdt, "/", "model", "riscv-virtio,qemu");
@@ -1505,8 +1496,16 @@ static void virt_machine_init(MachineState *machine)
     }
     virt_flash_map(s, system_memory);
 
-    /* create device tree */
-    create_fdt(s, memmap);
+    /* load/create device tree */
+    if (machine->dtb) {
+        machine->fdt = load_device_tree(machine->dtb, &s->fdt_size);
+        if (!machine->fdt) {
+            error_report("load_device_tree() failed");
+            exit(1);
+        }
+    } else {
+        create_fdt(s, memmap);
+    }
 
     s->machine_done.notify = virt_machine_done;
     qemu_add_machine_init_done_notifier(&s->machine_done);
-- 
2.25.1



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

* Re: [PATCH v2 2/2] hw/riscv: Move the dtb load bits outside of create_fdt()
  2023-02-28  7:45 ` [PATCH v2 2/2] hw/riscv: Move the dtb load bits outside of create_fdt() Bin Meng
@ 2023-02-28  9:15   ` Daniel Henrique Barboza
  0 siblings, 0 replies; 4+ messages in thread
From: Daniel Henrique Barboza @ 2023-02-28  9:15 UTC (permalink / raw)
  To: Bin Meng, qemu-devel
  Cc: Alistair Francis, Bin Meng, Liu Zhiwei, Palmer Dabbelt, Weiwei Li,
	qemu-riscv



On 2/28/23 04:45, Bin Meng wrote:
> Move the dtb load bits outside of create_fdt(), and put it explicitly
> in sifive_u_machine_init() and virt_machine_init(). With such change
> create_fdt() does exactly what its function name tells us.
> 
> Suggested-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> Signed-off-by: Bin Meng <bmeng@tinylab.org>
> ---

Reviewed-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>

> 
> Changes in v2:
> - new patch: Move the dtb load bits outside of create_fdt()
> 
>   include/hw/riscv/sifive_u.h |  1 +
>   hw/riscv/sifive_u.c         | 31 +++++++++++++++----------------
>   hw/riscv/virt.c             | 29 ++++++++++++++---------------
>   3 files changed, 30 insertions(+), 31 deletions(-)
> 
> diff --git a/include/hw/riscv/sifive_u.h b/include/hw/riscv/sifive_u.h
> index 65af306963..0696f85942 100644
> --- a/include/hw/riscv/sifive_u.h
> +++ b/include/hw/riscv/sifive_u.h
> @@ -68,6 +68,7 @@ typedef struct SiFiveUState {
>   
>       /*< public >*/
>       SiFiveUSoCState soc;
> +    int fdt_size;
>   
>       bool start_in_flash;
>       uint32_t msel;
> diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
> index 76db5ed3dd..35a335b8d0 100644
> --- a/hw/riscv/sifive_u.c
> +++ b/hw/riscv/sifive_u.c
> @@ -99,7 +99,7 @@ static void create_fdt(SiFiveUState *s, const MemMapEntry *memmap,
>       MachineState *ms = MACHINE(s);
>       uint64_t mem_size = ms->ram_size;
>       void *fdt;
> -    int cpu, fdt_size;
> +    int cpu;
>       uint32_t *cells;
>       char *nodename;
>       uint32_t plic_phandle, prci_phandle, gpio_phandle, phandle = 1;
> @@ -112,19 +112,10 @@ static void create_fdt(SiFiveUState *s, const MemMapEntry *memmap,
>           "sifive,plic-1.0.0", "riscv,plic0"
>       };
>   
> -    if (ms->dtb) {
> -        fdt = ms->fdt = load_device_tree(ms->dtb, &fdt_size);
> -        if (!fdt) {
> -            error_report("load_device_tree() failed");
> -            exit(1);
> -        }
> -        return;
> -    } else {
> -        fdt = ms->fdt = create_device_tree(&fdt_size);
> -        if (!fdt) {
> -            error_report("create_device_tree() failed");
> -            exit(1);
> -        }
> +    fdt = ms->fdt = create_device_tree(&s->fdt_size);
> +    if (!fdt) {
> +        error_report("create_device_tree() failed");
> +        exit(1);
>       }
>   
>       qemu_fdt_setprop_string(fdt, "/", "model", "SiFive HiFive Unleashed A00");
> @@ -561,8 +552,16 @@ static void sifive_u_machine_init(MachineState *machine)
>       qdev_connect_gpio_out(DEVICE(&(s->soc.gpio)), 10,
>                             qemu_allocate_irq(sifive_u_machine_reset, NULL, 0));
>   
> -    /* create device tree */
> -    create_fdt(s, memmap, riscv_is_32bit(&s->soc.u_cpus));
> +    /* load/create device tree */
> +    if (machine->dtb) {
> +        machine->fdt = load_device_tree(machine->dtb, &s->fdt_size);
> +        if (!machine->fdt) {
> +            error_report("load_device_tree() failed");
> +            exit(1);
> +        }
> +    } else {
> +        create_fdt(s, memmap, riscv_is_32bit(&s->soc.u_cpus));
> +    }
>   
>       if (s->start_in_flash) {
>           /*
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index 0c7b4a1e46..53ed2e8369 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -1008,19 +1008,10 @@ static void create_fdt(RISCVVirtState *s, const MemMapEntry *memmap)
>       uint32_t irq_pcie_phandle = 1, irq_virtio_phandle = 1;
>       uint8_t rng_seed[32];
>   
> -    if (ms->dtb) {
> -        ms->fdt = load_device_tree(ms->dtb, &s->fdt_size);
> -        if (!ms->fdt) {
> -            error_report("load_device_tree() failed");
> -            exit(1);
> -        }
> -        return;
> -    } else {
> -        ms->fdt = create_device_tree(&s->fdt_size);
> -        if (!ms->fdt) {
> -            error_report("create_device_tree() failed");
> -            exit(1);
> -        }
> +    ms->fdt = create_device_tree(&s->fdt_size);
> +    if (!ms->fdt) {
> +        error_report("create_device_tree() failed");
> +        exit(1);
>       }
>   
>       qemu_fdt_setprop_string(ms->fdt, "/", "model", "riscv-virtio,qemu");
> @@ -1505,8 +1496,16 @@ static void virt_machine_init(MachineState *machine)
>       }
>       virt_flash_map(s, system_memory);
>   
> -    /* create device tree */
> -    create_fdt(s, memmap);
> +    /* load/create device tree */
> +    if (machine->dtb) {
> +        machine->fdt = load_device_tree(machine->dtb, &s->fdt_size);
> +        if (!machine->fdt) {
> +            error_report("load_device_tree() failed");
> +            exit(1);
> +        }
> +    } else {
> +        create_fdt(s, memmap);
> +    }
>   
>       s->machine_done.notify = virt_machine_done;
>       qemu_add_machine_init_done_notifier(&s->machine_done);


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

* Re: [PATCH v2 1/2] hw/riscv: Skip re-generating DT nodes for a given DTB
  2023-02-28  7:45 [PATCH v2 1/2] hw/riscv: Skip re-generating DT nodes for a given DTB Bin Meng
  2023-02-28  7:45 ` [PATCH v2 2/2] hw/riscv: Move the dtb load bits outside of create_fdt() Bin Meng
@ 2023-03-02  1:29 ` Palmer Dabbelt
  1 sibling, 0 replies; 4+ messages in thread
From: Palmer Dabbelt @ 2023-03-02  1:29 UTC (permalink / raw)
  To: bmeng
  Cc: qemu-devel, dbarboza, Alistair Francis, bin.meng, zhiwei_liu,
	liweiwei, qemu-riscv

On Mon, 27 Feb 2023 23:45:21 PST (-0800), bmeng@tinylab.org wrote:
> Launch qemu-system-riscv64 with a given dtb for 'sifive_u' and 'virt'
> machines, QEMU complains:
>
>   qemu_fdt_add_subnode: Failed to create subnode /soc: FDT_ERR_EXISTS
>
> The whole DT generation logic should be skipped when a given DTB is
> present.
>
> Fixes: b1f19f238cae ("hw/riscv: write bootargs 'chosen' FDT after riscv_load_kernel()")
> Signed-off-by: Bin Meng <bmeng@tinylab.org>
> Reviewed-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> ---
>
> (no changes since v1)
>
>  hw/riscv/sifive_u.c | 1 +
>  hw/riscv/virt.c     | 1 +
>  2 files changed, 2 insertions(+)
>
> diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
> index ad3bb35b34..76db5ed3dd 100644
> --- a/hw/riscv/sifive_u.c
> +++ b/hw/riscv/sifive_u.c
> @@ -118,6 +118,7 @@ static void create_fdt(SiFiveUState *s, const MemMapEntry *memmap,
>              error_report("load_device_tree() failed");
>              exit(1);
>          }
> +        return;
>      } else {
>          fdt = ms->fdt = create_device_tree(&fdt_size);
>          if (!fdt) {
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index 86c4adc0c9..0c7b4a1e46 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -1014,6 +1014,7 @@ static void create_fdt(RISCVVirtState *s, const MemMapEntry *memmap)
>              error_report("load_device_tree() failed");
>              exit(1);
>          }
> +        return;
>      } else {
>          ms->fdt = create_device_tree(&s->fdt_size);
>          if (!ms->fdt) {

Thanks, these two are queued up.


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

end of thread, other threads:[~2023-03-02  1:31 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-02-28  7:45 [PATCH v2 1/2] hw/riscv: Skip re-generating DT nodes for a given DTB Bin Meng
2023-02-28  7:45 ` [PATCH v2 2/2] hw/riscv: Move the dtb load bits outside of create_fdt() Bin Meng
2023-02-28  9:15   ` Daniel Henrique Barboza
2023-03-02  1:29 ` [PATCH v2 1/2] hw/riscv: Skip re-generating DT nodes for a given DTB Palmer Dabbelt

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).