hw/arm: Pack the QEMU generated device tree

[PATCH 1/3] hw/arm: Refactor struct arm_boot_info::get_dtb()

Posted by Bin Meng 10 months ago

At present we expect struct arm_boot_info::get_dtb() to return the
device tree pointer as well as the device tree size. However, this
is not necessary as we can get the device tree size via the device
tree header directly. Change get_dtb() signature to drop the *size
argument, and get the size by ourselves.

Signed-off-by: Bin Meng <bin.meng@windriver.com>
---

 include/hw/arm/boot.h     | 8 ++++----
 hw/arm/boot.c             | 3 ++-
 hw/arm/sbsa-ref.c         | 3 +--
 hw/arm/virt.c             | 4 +---
 hw/arm/xlnx-versal-virt.c | 4 +---
 5 files changed, 9 insertions(+), 13 deletions(-)

diff --git a/include/hw/arm/boot.h b/include/hw/arm/boot.h
index 80c492d742..37fd1b520e 100644
--- a/include/hw/arm/boot.h
+++ b/include/hw/arm/boot.h
@@ -82,11 +82,11 @@ struct arm_boot_info {
                                      const struct arm_boot_info *info);
     /* if a board is able to create a dtb without a dtb file then it
      * sets get_dtb. This will only be used if no dtb file is provided
-     * by the user. On success, sets *size to the length of the created
-     * dtb, and returns a pointer to it. (The caller must free this memory
-     * with g_free() when it has finished with it.) On failure, returns NULL.
+     * by the user. On success, returns a pointer to it. (The caller must
+     * free this memory with g_free() when it has finished with it.)
+     * On failure, returns NULL.
      */
-    void *(*get_dtb)(const struct arm_boot_info *info, int *size);
+    void *(*get_dtb)(const struct arm_boot_info *info);
     /* if a board needs to be able to modify a device tree provided by
      * the user it should implement this hook.
      */
diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index 84ea6a807a..ff1173299f 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -538,11 +538,12 @@ int arm_load_dtb(hwaddr addr, const struct arm_boot_info *binfo,
         }
         g_free(filename);
     } else {
-        fdt = binfo->get_dtb(binfo, &size);
+        fdt = binfo->get_dtb(binfo);
         if (!fdt) {
             fprintf(stderr, "Board was unable to create a dtb blob\n");
             goto fail;
         }
+        size = fdt_totalsize(fdt);
     }
 
     if (addr_limit > addr && size > (addr_limit - addr)) {
diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c
index 477dca0637..c5023871a7 100644
--- a/hw/arm/sbsa-ref.c
+++ b/hw/arm/sbsa-ref.c
@@ -681,12 +681,11 @@ static void create_pcie(SBSAMachineState *sms)
     create_smmu(sms, pci->bus);
 }
 
-static void *sbsa_ref_dtb(const struct arm_boot_info *binfo, int *fdt_size)
+static void *sbsa_ref_dtb(const struct arm_boot_info *binfo)
 {
     const SBSAMachineState *board = container_of(binfo, SBSAMachineState,
                                                  bootinfo);
 
-    *fdt_size = board->fdt_size;
     return board->fdt;
 }
 
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 2793121cb4..1996fffa99 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -1577,14 +1577,12 @@ static void create_secure_ram(VirtMachineState *vms,
     g_free(nodename);
 }
 
-static void *machvirt_dtb(const struct arm_boot_info *binfo, int *fdt_size)
+static void *machvirt_dtb(const struct arm_boot_info *binfo)
 {
     const VirtMachineState *board = container_of(binfo, VirtMachineState,
                                                  bootinfo);
     MachineState *ms = MACHINE(board);
 
-
-    *fdt_size = board->fdt_size;
     return ms->fdt;
 }
 
diff --git a/hw/arm/xlnx-versal-virt.c b/hw/arm/xlnx-versal-virt.c
index 537118224f..1e043c813e 100644
--- a/hw/arm/xlnx-versal-virt.c
+++ b/hw/arm/xlnx-versal-virt.c
@@ -551,12 +551,10 @@ static void versal_virt_modify_dtb(const struct arm_boot_info *binfo,
     fdt_add_memory_nodes(s, fdt, binfo->ram_size);
 }
 
-static void *versal_virt_get_dtb(const struct arm_boot_info *binfo,
-                                  int *fdt_size)
+static void *versal_virt_get_dtb(const struct arm_boot_info *binfo)
 {
     const VersalVirt *board = container_of(binfo, VersalVirt, binfo);
 
-    *fdt_size = board->fdt_size;
     return board->fdt;
 }
 
-- 
2.34.1

Re: [PATCH 1/3] hw/arm: Refactor struct arm_boot_info::get_dtb()

Posted by Alistair Francis 9 months, 3 weeks ago

On Mon, Jan 15, 2024 at 2:36 PM Bin Meng <bin.meng@windriver.com> wrote:
>
> At present we expect struct arm_boot_info::get_dtb() to return the
> device tree pointer as well as the device tree size. However, this
> is not necessary as we can get the device tree size via the device
> tree header directly. Change get_dtb() signature to drop the *size
> argument, and get the size by ourselves.
>
> Signed-off-by: Bin Meng <bin.meng@windriver.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>
>  include/hw/arm/boot.h     | 8 ++++----
>  hw/arm/boot.c             | 3 ++-
>  hw/arm/sbsa-ref.c         | 3 +--
>  hw/arm/virt.c             | 4 +---
>  hw/arm/xlnx-versal-virt.c | 4 +---
>  5 files changed, 9 insertions(+), 13 deletions(-)
>
> diff --git a/include/hw/arm/boot.h b/include/hw/arm/boot.h
> index 80c492d742..37fd1b520e 100644
> --- a/include/hw/arm/boot.h
> +++ b/include/hw/arm/boot.h
> @@ -82,11 +82,11 @@ struct arm_boot_info {
>                                       const struct arm_boot_info *info);
>      /* if a board is able to create a dtb without a dtb file then it
>       * sets get_dtb. This will only be used if no dtb file is provided
> -     * by the user. On success, sets *size to the length of the created
> -     * dtb, and returns a pointer to it. (The caller must free this memory
> -     * with g_free() when it has finished with it.) On failure, returns NULL.
> +     * by the user. On success, returns a pointer to it. (The caller must
> +     * free this memory with g_free() when it has finished with it.)
> +     * On failure, returns NULL.
>       */
> -    void *(*get_dtb)(const struct arm_boot_info *info, int *size);
> +    void *(*get_dtb)(const struct arm_boot_info *info);
>      /* if a board needs to be able to modify a device tree provided by
>       * the user it should implement this hook.
>       */
> diff --git a/hw/arm/boot.c b/hw/arm/boot.c
> index 84ea6a807a..ff1173299f 100644
> --- a/hw/arm/boot.c
> +++ b/hw/arm/boot.c
> @@ -538,11 +538,12 @@ int arm_load_dtb(hwaddr addr, const struct arm_boot_info *binfo,
>          }
>          g_free(filename);
>      } else {
> -        fdt = binfo->get_dtb(binfo, &size);
> +        fdt = binfo->get_dtb(binfo);
>          if (!fdt) {
>              fprintf(stderr, "Board was unable to create a dtb blob\n");
>              goto fail;
>          }
> +        size = fdt_totalsize(fdt);
>      }
>
>      if (addr_limit > addr && size > (addr_limit - addr)) {
> diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c
> index 477dca0637..c5023871a7 100644
> --- a/hw/arm/sbsa-ref.c
> +++ b/hw/arm/sbsa-ref.c
> @@ -681,12 +681,11 @@ static void create_pcie(SBSAMachineState *sms)
>      create_smmu(sms, pci->bus);
>  }
>
> -static void *sbsa_ref_dtb(const struct arm_boot_info *binfo, int *fdt_size)
> +static void *sbsa_ref_dtb(const struct arm_boot_info *binfo)
>  {
>      const SBSAMachineState *board = container_of(binfo, SBSAMachineState,
>                                                   bootinfo);
>
> -    *fdt_size = board->fdt_size;
>      return board->fdt;
>  }
>
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 2793121cb4..1996fffa99 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -1577,14 +1577,12 @@ static void create_secure_ram(VirtMachineState *vms,
>      g_free(nodename);
>  }
>
> -static void *machvirt_dtb(const struct arm_boot_info *binfo, int *fdt_size)
> +static void *machvirt_dtb(const struct arm_boot_info *binfo)
>  {
>      const VirtMachineState *board = container_of(binfo, VirtMachineState,
>                                                   bootinfo);
>      MachineState *ms = MACHINE(board);
>
> -
> -    *fdt_size = board->fdt_size;
>      return ms->fdt;
>  }
>
> diff --git a/hw/arm/xlnx-versal-virt.c b/hw/arm/xlnx-versal-virt.c
> index 537118224f..1e043c813e 100644
> --- a/hw/arm/xlnx-versal-virt.c
> +++ b/hw/arm/xlnx-versal-virt.c
> @@ -551,12 +551,10 @@ static void versal_virt_modify_dtb(const struct arm_boot_info *binfo,
>      fdt_add_memory_nodes(s, fdt, binfo->ram_size);
>  }
>
> -static void *versal_virt_get_dtb(const struct arm_boot_info *binfo,
> -                                  int *fdt_size)
> +static void *versal_virt_get_dtb(const struct arm_boot_info *binfo)
>  {
>      const VersalVirt *board = container_of(binfo, VersalVirt, binfo);
>
> -    *fdt_size = board->fdt_size;
>      return board->fdt;
>  }
>
> --
> 2.34.1
>
>

Re: [PATCH 1/3] hw/arm: Refactor struct arm_boot_info::get_dtb()

Posted by Philippe Mathieu-Daudé 10 months ago

On 15/1/24 05:34, Bin Meng wrote:
> At present we expect struct arm_boot_info::get_dtb() to return the
> device tree pointer as well as the device tree size. However, this
> is not necessary as we can get the device tree size via the device
> tree header directly. Change get_dtb() signature to drop the *size
> argument, and get the size by ourselves.
> 
> Signed-off-by: Bin Meng <bin.meng@windriver.com>
> ---
> 
>   include/hw/arm/boot.h     | 8 ++++----
>   hw/arm/boot.c             | 3 ++-
>   hw/arm/sbsa-ref.c         | 3 +--
>   hw/arm/virt.c             | 4 +---
>   hw/arm/xlnx-versal-virt.c | 4 +---
>   5 files changed, 9 insertions(+), 13 deletions(-)

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

[PATCH 1/3] hw/arm: Refactor struct arm_boot_info::get_dtb()
[PATCH 2/3] hw/arm: Pack the QEMU generated device tree
[PATCH 3/3] tests/acpi: Update virt/SSDT.memhp