Re: [Qemu-devel] [PATCH v1 3/3] arm: xilinx_zynq: Use Sysbus Memory device for RAMs

[Igor Mammedov]

On Mon, 14 Apr 2014 19:22:11 -0700
Peter Crosthwaite <address@hidden> wrote:

> For consistency with other devices and completeness of system device
> tree.
> 
> Signed-off-by: Peter Crosthwaite <address@hidden>
> ---
> 
>  hw/arm/xilinx_zynq.c | 19 ++++++++++---------
>  1 file changed, 10 insertions(+), 9 deletions(-)
> 
> diff --git a/hw/arm/xilinx_zynq.c b/hw/arm/xilinx_zynq.c
> index 9ee21e7..7a0c951 100644
> --- a/hw/arm/xilinx_zynq.c
> +++ b/hw/arm/xilinx_zynq.c
> @@ -110,9 +110,6 @@ static void zynq_init(QEMUMachineInitArgs *args)
>      const char *initrd_filename = args->initrd_filename;
>      ObjectClass *cpu_oc;
>      ARMCPU *cpu;
> -    MemoryRegion *address_space_mem = get_system_memory();
> -    MemoryRegion *ext_ram = g_new(MemoryRegion, 1);
> -    MemoryRegion *ocm_ram = g_new(MemoryRegion, 1);
>      DeviceState *dev;
>      SysBusDevice *busdev;
>      qemu_irq pic[64];
add following hotplug-handler to board:

zynq_machine_plug_handler_callback(zynq_machine, dev) {
    if (type(dev) == dimm) {
        MemoryRegion *mr = DIMM_GET_CLASS(dev)->get_memory_region(dev);
        // do extra board specific placement checks

        // do actual mapping
        memory_region_add_subregion(zynq_machine->ram_address_space,
                                    object_property_get_int(dev, "start", NULL),
                                    mr);
        vmstate_register_ram_global(rm)
    }
}

> @@ -149,14 +146,18 @@ static void zynq_init(QEMUMachineInitArgs *args)
>      }
>  
>      /* DDR remapped to address zero.  */
> -    memory_region_init_ram(ext_ram, NULL, "zynq.ext_ram", ram_size);
> -    vmstate_register_ram_global(ext_ram);
> -    memory_region_add_subregion(address_space_mem, 0, ext_ram);
> +    dev = qdev_create(NULL, "sysbus-memory");
> +    qdev_prop_set_string(dev, "device-id", "zynq.ext_ram");
> +    qdev_prop_set_uint64(dev, "size", ram_size);
> +    qdev_init_nofail(dev);
> +    sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, 0);

could be replaced by hardcoded as above:

       object_add("ext_ram", QDict("memory-ram,size=foo"));
       dev = DEVICE(object_new("dimm"));
       qdev_prop_set_uint64(dev, "start", 0);
       object_property_set_bool(OBJECT(dev), true, "realized", &error_abort);

or throw away above section and use data driven way:

       -object memory-ram,id=ext_ram,size=ram_size
       -device dimm,id=zynq.ext_ram,memdev=ext_ram

with it one gets completely functional RAM device on board after
its realize has been completed.

>  
>      /* 256K of on-chip memory */
> -    memory_region_init_ram(ocm_ram, NULL, "zynq.ocm_ram", 256 << 10);
> -    vmstate_register_ram_global(ocm_ram);
> -    memory_region_add_subregion(address_space_mem, 0xFFFC0000, ocm_ram);
> +    dev = qdev_create(NULL, "sysbus-memory");
> +    qdev_prop_set_string(dev, "device-id", "zynq.ocm_ram");
> +    qdev_prop_set_uint64(dev, "size", 256 << 10);
> +    qdev_init_nofail(dev);
> +    sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, 0xFFFC0000);
>  
>      DriveInfo *dinfo = drive_get(IF_PFLASH, 0, 0);
>