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

[Alistair Francis]

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