Re: [PATCH v2 02/47] hw/arm/xlnx-versal: prepare for FDT creation

["Edgar E. Iglesias" <edgar.iglesias@amd.com>]

On Wed, Aug 20, 2025 at 10:25:01AM +0200, Luc Michel wrote:
> The following commits will move FDT creation logic from the
> xlnx-versal-virt machine to the xlnx-versal SoC itself. Prepare this by
> passing the FDT handle to the SoC before it is realized. If no FDT is
> passed, a dummy one is created internally as a stub to the fdt function
> calls.
> 
> For now the SoC only creates the two clock nodes. The ones from the
> xlnx-versal virt machine are renamed with a `old-' prefix and will be
> removed once they are not referenced anymore.


Hi Luc,

I was trying this but I see this on this patch:
../qemu/hw/arm/xlnx-versal.c: In function ‘versal_realize’:
../qemu/hw/arm/xlnx-versal.c:945:11: error: unknown type name ‘VersalMap’
  945 |     const VersalMap *map = versal_get_map(s);
      |           ^~~~~~~~~
../qemu/hw/arm/xlnx-versal.c:945:28: error: implicit declaration of function ‘versal_get_map’; did you mean ‘versal_set_fdt’? [-Werror=implicit-function-declaration]
  945 |     const VersalMap *map = versal_get_map(s);
      |                            ^~~~~~~~~~~~~~
      |                            versal_set_fdt
../qemu/hw/arm/xlnx-versal.c:945:28: error: nested extern declaration of ‘versal_get_map’ [-Werror=nested-externs]
../qemu/hw/arm/xlnx-versal.c:945:28: error: initialization of ‘const int *’ from ‘int’ makes pointer from integer without a cast [-Werror=int-conversion]
../qemu/hw/arm/xlnx-versal.c:946:12: error: unused variable ‘i’ [-Werror=unused-variable]
  946 |     size_t i;
      |            ^
../qemu/hw/arm/xlnx-versal.c:945:22: error: unused variable ‘map’ [-Werror=unused-variable]
  945 |     const VersalMap *map = versal_get_map(s);
      |                      ^~~





> 
> Signed-off-by: Luc Michel <luc.michel@amd.com>
> Reviewed-by: Francisco Iglesias <francisco.iglesias@amd.com>
> ---
>  include/hw/arm/xlnx-versal.h | 12 ++++++++++++
>  hw/arm/xlnx-versal-virt.c    | 11 +++++++----
>  hw/arm/xlnx-versal.c         | 30 ++++++++++++++++++++++++++++++
>  3 files changed, 49 insertions(+), 4 deletions(-)
> 
> diff --git a/include/hw/arm/xlnx-versal.h b/include/hw/arm/xlnx-versal.h
> index 1f92e314d6c..f2a62b43552 100644
> --- a/include/hw/arm/xlnx-versal.h
> +++ b/include/hw/arm/xlnx-versal.h
> @@ -134,21 +134,33 @@ struct Versal {
>          XlnxVersalCFrameBcastReg cframe_bcast;
>  
>          OrIRQState apb_irq_orgate;
>      } pmc;
>  
> +    struct {
> +        uint32_t clk_25mhz;
> +        uint32_t clk_125mhz;
> +    } phandle;
> +
>      struct {
>          MemoryRegion *mr_ddr;
> +        void *fdt;
>      } cfg;
>  };
>  
>  struct VersalClass {
>      SysBusDeviceClass parent;
>  
>      VersalVersion version;
>  };
>  
> +static inline void versal_set_fdt(Versal *s, void *fdt)
> +{
> +    g_assert(!qdev_is_realized(DEVICE(s)));
> +    s->cfg.fdt = fdt;
> +}
> +
>  /* Memory-map and IRQ definitions. Copied a subset from
>   * auto-generated files.  */
>  
>  #define VERSAL_GIC_MAINT_IRQ        9
>  #define VERSAL_TIMER_VIRT_IRQ       11
> diff --git a/hw/arm/xlnx-versal-virt.c b/hw/arm/xlnx-versal-virt.c
> index adadbb72902..69f3bb401b9 100644
> --- a/hw/arm/xlnx-versal-virt.c
> +++ b/hw/arm/xlnx-versal-virt.c
> @@ -1,9 +1,10 @@
>  /*
>   * Xilinx Versal Virtual board.
>   *
>   * Copyright (c) 2018 Xilinx Inc.
> + * Copyright (c) 2025 Advanced Micro Devices, Inc.
>   * Written by Edgar E. Iglesias
>   *
>   * This program is free software; you can redistribute it and/or modify
>   * it under the terms of the GNU General Public License version 2 or
>   * (at your option) any later version.
> @@ -695,14 +696,13 @@ static void versal_virt_init(MachineState *machine)
>                               &error_abort);
>      object_property_set_link(OBJECT(&s->soc), "canbus0", OBJECT(s->canbus[0]),
>                               &error_abort);
>      object_property_set_link(OBJECT(&s->soc), "canbus1", OBJECT(s->canbus[1]),
>                               &error_abort);
> -    sysbus_realize(SYS_BUS_DEVICE(&s->soc), &error_fatal);
>  
>      fdt_create(s);
> -    create_virtio_regions(s);
> +    versal_set_fdt(&s->soc, s->fdt);
>      fdt_add_gem_nodes(s);
>      fdt_add_uart_nodes(s);
>      fdt_add_canfd_nodes(s);
>      fdt_add_gic_nodes(s);
>      fdt_add_timer_nodes(s);
> @@ -712,12 +712,15 @@ static void versal_virt_init(MachineState *machine)
>      fdt_add_rtc_node(s);
>      fdt_add_bbram_node(s);
>      fdt_add_efuse_ctrl_node(s);
>      fdt_add_efuse_cache_node(s);
>      fdt_add_cpu_nodes(s, psci_conduit);
> -    fdt_add_clk_node(s, "/clk125", 125000000, s->phandle.clk_125Mhz);
> -    fdt_add_clk_node(s, "/clk25", 25000000, s->phandle.clk_25Mhz);
> +    fdt_add_clk_node(s, "/old-clk125", 125000000, s->phandle.clk_125Mhz);
> +    fdt_add_clk_node(s, "/old-clk25", 25000000, s->phandle.clk_25Mhz);
> +
> +    sysbus_realize(SYS_BUS_DEVICE(&s->soc), &error_fatal);
> +    create_virtio_regions(s);
>  
>      /* Make the APU cpu address space visible to virtio and other
>       * modules unaware of multiple address-spaces.  */
>      memory_region_add_subregion_overlap(get_system_memory(),
>                                          0, &s->soc.fpd.apu.mr, 0);
> diff --git a/hw/arm/xlnx-versal.c b/hw/arm/xlnx-versal.c
> index 4da656318f6..7bb55751e5c 100644
> --- a/hw/arm/xlnx-versal.c
> +++ b/hw/arm/xlnx-versal.c
> @@ -22,10 +22,12 @@
>  #include "hw/misc/unimp.h"
>  #include "hw/arm/xlnx-versal.h"
>  #include "qemu/log.h"
>  #include "target/arm/cpu-qom.h"
>  #include "target/arm/gtimer.h"
> +#include "system/device_tree.h"
> +#include "hw/arm/fdt.h"
>  
>  #define XLNX_VERSAL_ACPU_TYPE ARM_CPU_TYPE_NAME("cortex-a72")
>  #define XLNX_VERSAL_RCPU_TYPE ARM_CPU_TYPE_NAME("cortex-r5f")
>  #define GEM_REVISION        0x40070106
>  
> @@ -917,14 +919,42 @@ static void versal_unimp(Versal *s)
>      qdev_connect_gpio_out_named(DEVICE(&s->pmc.iou.slcr),
>                                  SYSBUS_DEVICE_GPIO_IRQ, 0,
>                                  gpio_in);
>  }
>  
> +static uint32_t fdt_add_clk_node(Versal *s, const char *name,
> +                                 unsigned int freq_hz)
> +{
> +    uint32_t phandle;
> +
> +    phandle = qemu_fdt_alloc_phandle(s->cfg.fdt);
> +
> +    qemu_fdt_add_subnode(s->cfg.fdt, name);
> +    qemu_fdt_setprop_cell(s->cfg.fdt, name, "phandle", phandle);
> +    qemu_fdt_setprop_cell(s->cfg.fdt, name, "clock-frequency", freq_hz);
> +    qemu_fdt_setprop_cell(s->cfg.fdt, name, "#clock-cells", 0x0);
> +    qemu_fdt_setprop_string(s->cfg.fdt, name, "compatible", "fixed-clock");
> +    qemu_fdt_setprop(s->cfg.fdt, name, "u-boot,dm-pre-reloc", NULL, 0);
> +
> +    return phandle;
> +}
> +
>  static void versal_realize(DeviceState *dev, Error **errp)
>  {
>      Versal *s = XLNX_VERSAL_BASE(dev);
>      qemu_irq pic[XLNX_VERSAL_NR_IRQS];
> +    const VersalMap *map = versal_get_map(s);
> +    size_t i;
> +
> +    if (s->cfg.fdt == NULL) {
> +        int fdt_size;
> +
> +        s->cfg.fdt = create_device_tree(&fdt_size);
> +    }
> +
> +    s->phandle.clk_25mhz = fdt_add_clk_node(s, "/clk25", 25 * 1000 * 1000);
> +    s->phandle.clk_125mhz = fdt_add_clk_node(s, "/clk125", 125 * 1000 * 1000);
>  
>      versal_create_apu_cpus(s);
>      versal_create_apu_gic(s, pic);
>      versal_create_rpu_cpus(s);
>      versal_create_uarts(s, pic);
> -- 
> 2.50.1
>