On Wed, Jul 16, 2025 at 11:54:11AM +0200, Luc Michel wrote:
> Refactor the DDR aperture regions creation using the VersalMap
> structure. Device creation and FDT node creation are split into two
> functions because the later must happen during ARM virtual bootloader
> modify_dtb callback.
>
> Signed-off-by: Luc Michel <luc.mic...@amd.com>
Reviewed-by: Francisco Iglesias <francisco.igles...@amd.com>
> ---
> include/hw/arm/xlnx-versal.h | 7 +---
> hw/arm/xlnx-versal-virt.c | 79 +-----------------------------------
> hw/arm/xlnx-versal.c | 73 ++++++++++++++++++++++-----------
> 3 files changed, 53 insertions(+), 106 deletions(-)
>
> diff --git a/include/hw/arm/xlnx-versal.h b/include/hw/arm/xlnx-versal.h
> index 7be5a6ccf4d..a3bc967c352 100644
> --- a/include/hw/arm/xlnx-versal.h
> +++ b/include/hw/arm/xlnx-versal.h
> @@ -41,15 +41,10 @@ struct Versal {
>
> /*< public >*/
> GArray *intc;
> MemoryRegion mr_ps;
>
> - struct {
> - /* 4 ranges to access DDR. */
> - MemoryRegion mr_ddr_ranges[4];
> - } noc;
> -
> struct {
> uint32_t clk_25mhz;
> uint32_t clk_125mhz;
> uint32_t gic;
> } phandle;
> @@ -71,10 +66,12 @@ static inline void versal_set_fdt(Versal *s, void *fdt)
> {
> g_assert(!qdev_is_realized(DEVICE(s)));
> s->cfg.fdt = fdt;
> }
>
> +void versal_fdt_add_memory_nodes(Versal *s, uint64_t ram_size);
> +
> void versal_sdhci_plug_card(Versal *s, int sd_idx, BlockBackend *blk);
> void versal_efuse_attach_drive(Versal *s, BlockBackend *blk);
> void versal_bbram_attach_drive(Versal *s, BlockBackend *blk);
> void versal_ospi_create_flash(Versal *s, int flash_idx, const char
> *flash_mdl,
> BlockBackend *blk);
> diff --git a/hw/arm/xlnx-versal-virt.c b/hw/arm/xlnx-versal-virt.c
> index cad345b98e0..7f40c197072 100644
> --- a/hw/arm/xlnx-versal-virt.c
> +++ b/hw/arm/xlnx-versal-virt.c
> @@ -104,92 +104,17 @@ static void fdt_nop_memory_nodes(void *fdt, Error
> **errp)
> n++;
> }
> g_strfreev(node_path);
> }
>
> -static void fdt_add_memory_nodes(VersalVirt *s, void *fdt, uint64_t ram_size)
> -{
> - /* Describes the various split DDR access regions. */
> - static const struct {
> - uint64_t base;
> - uint64_t size;
> - } addr_ranges[] = {
> - { MM_TOP_DDR, MM_TOP_DDR_SIZE },
> - { MM_TOP_DDR_2, MM_TOP_DDR_2_SIZE },
> - { MM_TOP_DDR_3, MM_TOP_DDR_3_SIZE },
> - { MM_TOP_DDR_4, MM_TOP_DDR_4_SIZE }
> - };
> - uint64_t mem_reg_prop[8] = {0};
> - uint64_t size = ram_size;
> - Error *err = NULL;
> - char *name;
> - int i;
> -
> - fdt_nop_memory_nodes(fdt, &err);
> - if (err) {
> - error_report_err(err);
> - return;
> - }
> -
> - name = g_strdup_printf("/memory@%x", MM_TOP_DDR);
> - for (i = 0; i < ARRAY_SIZE(addr_ranges) && size; i++) {
> - uint64_t mapsize;
> -
> - mapsize = size < addr_ranges[i].size ? size : addr_ranges[i].size;
> -
> - mem_reg_prop[i * 2] = addr_ranges[i].base;
> - mem_reg_prop[i * 2 + 1] = mapsize;
> - size -= mapsize;
> - }
> - qemu_fdt_add_subnode(fdt, name);
> - qemu_fdt_setprop_string(fdt, name, "device_type", "memory");
> -
> - switch (i) {
> - case 1:
> - qemu_fdt_setprop_sized_cells(fdt, name, "reg",
> - 2, mem_reg_prop[0],
> - 2, mem_reg_prop[1]);
> - break;
> - case 2:
> - qemu_fdt_setprop_sized_cells(fdt, name, "reg",
> - 2, mem_reg_prop[0],
> - 2, mem_reg_prop[1],
> - 2, mem_reg_prop[2],
> - 2, mem_reg_prop[3]);
> - break;
> - case 3:
> - qemu_fdt_setprop_sized_cells(fdt, name, "reg",
> - 2, mem_reg_prop[0],
> - 2, mem_reg_prop[1],
> - 2, mem_reg_prop[2],
> - 2, mem_reg_prop[3],
> - 2, mem_reg_prop[4],
> - 2, mem_reg_prop[5]);
> - break;
> - case 4:
> - qemu_fdt_setprop_sized_cells(fdt, name, "reg",
> - 2, mem_reg_prop[0],
> - 2, mem_reg_prop[1],
> - 2, mem_reg_prop[2],
> - 2, mem_reg_prop[3],
> - 2, mem_reg_prop[4],
> - 2, mem_reg_prop[5],
> - 2, mem_reg_prop[6],
> - 2, mem_reg_prop[7]);
> - break;
> - default:
> - g_assert_not_reached();
> - }
> - g_free(name);
> -}
> -
> static void versal_virt_modify_dtb(const struct arm_boot_info *binfo,
> void *fdt)
> {
> VersalVirt *s = container_of(binfo, VersalVirt, binfo);
>
> - fdt_add_memory_nodes(s, fdt, binfo->ram_size);
> + fdt_nop_memory_nodes(s->fdt, &error_abort);
> + versal_fdt_add_memory_nodes(&s->soc, binfo->ram_size);
> }
>
> static void *versal_virt_get_dtb(const struct arm_boot_info *binfo,
> int *fdt_size)
> {
> diff --git a/hw/arm/xlnx-versal.c b/hw/arm/xlnx-versal.c
> index f46c73ac8e7..bf680077e48 100644
> --- a/hw/arm/xlnx-versal.c
> +++ b/hw/arm/xlnx-versal.c
> @@ -115,10 +115,15 @@ typedef struct VersalCpuClusterMap {
> } VersalCpuClusterMap;
>
> typedef struct VersalMap {
> VersalMemMap ocm;
>
> + struct VersalDDRMap {
> + VersalMemMap chan[4];
> + size_t num_chan;
> + } ddr;
> +
> VersalCpuClusterMap apu;
> VersalCpuClusterMap rpu;
>
> VersalSimplePeriphMap uart[2];
> size_t num_uart;
> @@ -219,10 +224,18 @@ static const VersalMap VERSAL_MAP = {
> .ocm = {
> .addr = 0xfffc0000,
> .size = 0x40000,
> },
>
> + .ddr = {
> + .chan[0] = { .addr = 0x0, .size = 2 * GiB },
> + .chan[1] = { .addr = 0x800000000ull, .size = 32 * GiB },
> + .chan[2] = { .addr = 0xc00000000ull, .size = 256 * GiB },
> + .chan[3] = { .addr = 0x10000000000ull, .size = 734 * GiB },
> + .num_chan = 4,
> + },
> +
> .apu = {
> .name = "apu",
> .cpu_model = ARM_CPU_TYPE_NAME("cortex-a72"),
> .num_cluster = 1,
> .num_core = 2,
> @@ -1480,50 +1493,62 @@ static inline void versal_create_crl(Versal *s)
> sysbus_mmio_get_region(SYS_BUS_DEVICE(dev),
> 0));
>
> versal_sysbus_connect_irq(s, SYS_BUS_DEVICE(dev), 0, map->crl.irq);
> }
>
> -/* This takes the board allocated linear DDR memory and creates aliases
> +/*
> + * This takes the board allocated linear DDR memory and creates aliases
> * for each split DDR range/aperture on the Versal address map.
> */
> -static void versal_map_ddr(Versal *s)
> +static void versal_map_ddr(Versal *s, const struct VersalDDRMap *map)
> {
> uint64_t size = memory_region_size(s->cfg.mr_ddr);
> - /* Describes the various split DDR access regions. */
> - static const struct {
> - uint64_t base;
> - uint64_t size;
> - } addr_ranges[] = {
> - { MM_TOP_DDR, MM_TOP_DDR_SIZE },
> - { MM_TOP_DDR_2, MM_TOP_DDR_2_SIZE },
> - { MM_TOP_DDR_3, MM_TOP_DDR_3_SIZE },
> - { MM_TOP_DDR_4, MM_TOP_DDR_4_SIZE }
> - };
> uint64_t offset = 0;
> int i;
>
> - assert(ARRAY_SIZE(addr_ranges) == ARRAY_SIZE(s->noc.mr_ddr_ranges));
> - for (i = 0; i < ARRAY_SIZE(addr_ranges) && size; i++) {
> - char *name;
> + for (i = 0; i < map->num_chan && size; i++) {
> uint64_t mapsize;
> + MemoryRegion *alias;
> +
> + mapsize = MIN(size, map->chan[i].size);
>
> - mapsize = size < addr_ranges[i].size ? size : addr_ranges[i].size;
> - name = g_strdup_printf("noc-ddr-range%d", i);
> /* Create the MR alias. */
> - memory_region_init_alias(&s->noc.mr_ddr_ranges[i], OBJECT(s),
> - name, s->cfg.mr_ddr,
> - offset, mapsize);
> + alias = g_new(MemoryRegion, 1);
> + memory_region_init_alias(alias, OBJECT(s), "noc-ddr-range",
> + s->cfg.mr_ddr, offset, mapsize);
>
> /* Map it onto the NoC MR. */
> - memory_region_add_subregion(&s->mr_ps, addr_ranges[i].base,
> - &s->noc.mr_ddr_ranges[i]);
> + memory_region_add_subregion(&s->mr_ps, map->chan[i].addr, alias);
> offset += mapsize;
> size -= mapsize;
> - g_free(name);
> }
> }
>
> +void versal_fdt_add_memory_nodes(Versal *s, uint64_t size)
> +{
> + const struct VersalDDRMap *map = &versal_get_map(s)->ddr;
> + g_autofree char *node;
> + g_autofree uint64_t *reg;
> + int i;
> +
> + reg = g_new(uint64_t, map->num_chan * 2);
> +
> + for (i = 0; i < map->num_chan && size; i++) {
> + uint64_t mapsize;
> +
> + mapsize = MIN(size, map->chan[i].size);
> +
> + reg[i * 2] = cpu_to_be64(map->chan[i].addr);
> + reg[i * 2 + 1] = cpu_to_be64(mapsize);
> +
> + size -= mapsize;
> + }
> +
> + node = versal_fdt_add_subnode(s, "/memory", 0, "memory",
> sizeof("memory"));
> + qemu_fdt_setprop(s->cfg.fdt, node, "reg", reg, sizeof(uint64_t) * i * 2);
> +}
> +
> static void versal_unimp_area(Versal *s, const char *name,
> MemoryRegion *mr,
> hwaddr base, hwaddr size)
> {
> DeviceState *dev = qdev_new(TYPE_UNIMPLEMENTED_DEVICE);
> @@ -1687,11 +1712,11 @@ static void versal_realize(DeviceState *dev, Error
> **errp)
> versal_create_trng(s, &map->trng);
> versal_create_rtc(s, &map->rtc);
> versal_create_cfu(s, &map->cfu);
> versal_create_crl(s);
>
> - versal_map_ddr(s);
> + versal_map_ddr(s, &map->ddr);
> versal_unimp(s);
>
> /* Create the On Chip Memory (OCM). */
> ocm = g_new(MemoryRegion, 1);
> memory_region_init_ram(ocm, OBJECT(s), "ocm", map->ocm.size,
> &error_fatal);
> --
> 2.50.0
>
