On 15/01/2020 15:07, Igor Mammedov wrote:
> memory_region_allocate_system_memory() API is going away, so
> replace it with memdev allocated MemoryRegion. The later is
> initialized by generic code, so board only needs to opt in
> to memdev scheme by providing
> MachineClass::default_ram_id
> and using MachineState::ram instead of manually initializing
> RAM memory region.
>
> Patch moves ram size check into sun4m_hw_init() and drops
> ram_init() moving remainder to sun4m_hw_init() as well,
> as it was the only place that called it.
>
> Also it rewrites impl. of RamDevice a little bit, which
> could serve as an example of frontend device for RAM backend.
> (Caveats are:
> 1. it doesn't check for memdev backend being mapped
> since it's been usurped by generic machine to handle
> majority of machines which don't have RAM frontend device
> 2. it still lacks 'addr' property and still has hardcoded
> sysbus_mmio_map() in board init. If done right, board should
> set 'addr' property and bus/machine plug handler should map
> it during device realize time.
> )
> Further improvements were left as exercise for the future,
> since frontends are out scope of RAM conversion to memdev.
>
> Signed-off-by: Igor Mammedov <imamm...@redhat.com>
> ---
> CC: mark.cave-ayl...@ilande.co.uk
> CC: atar4q...@gmail.com
> ---
> hw/sparc/sun4m.c | 73
> ++++++++++++++++++++++++++++----------------------------
> 1 file changed, 36 insertions(+), 37 deletions(-)
>
> diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
> index 2aaa5bf..834ad2a 100644
> --- a/hw/sparc/sun4m.c
> +++ b/hw/sparc/sun4m.c
> @@ -777,63 +777,42 @@ static const TypeInfo prom_info = {
>
> typedef struct RamDevice {
> SysBusDevice parent_obj;
> -
> - MemoryRegion ram;
> - uint64_t size;
> + HostMemoryBackend *memdev;
> } RamDevice;
>
> /* System RAM */
> static void ram_realize(DeviceState *dev, Error **errp)
> {
> RamDevice *d = SUN4M_RAM(dev);
> - SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
> + MemoryRegion *ram = host_memory_backend_get_memory(d->memdev);
>
> - memory_region_allocate_system_memory(&d->ram, OBJECT(d), "sun4m.ram",
> - d->size);
> - sysbus_init_mmio(sbd, &d->ram);
> + sysbus_init_mmio(SYS_BUS_DEVICE(dev), ram);
> }
>
> -static void ram_init(hwaddr addr, ram_addr_t RAM_size,
> - uint64_t max_mem)
> +static void ram_initfn(Object *obj)
> {
> - DeviceState *dev;
> - SysBusDevice *s;
> - RamDevice *d;
> -
> - /* allocate RAM */
> - if ((uint64_t)RAM_size > max_mem) {
> - error_report("Too much memory for this machine: %" PRId64 ","
> - " maximum %" PRId64,
> - RAM_size / MiB, max_mem / MiB);
> - exit(1);
> - }
> - dev = qdev_create(NULL, "memory");
> - s = SYS_BUS_DEVICE(dev);
> -
> - d = SUN4M_RAM(dev);
> - d->size = RAM_size;
> - qdev_init_nofail(dev);
> -
> - sysbus_mmio_map(s, 0, addr);
> + RamDevice *d = SUN4M_RAM(obj);
> + object_property_add_link(obj, "memdev", TYPE_MEMORY_BACKEND,
> + (Object **)&d->memdev,
> + object_property_allow_set_link,
> + OBJ_PROP_LINK_STRONG, &error_abort);
> + object_property_set_description(obj, "memdev", "Set RAM backend"
> + "Valid value is ID of a hostmem backend",
> + &error_abort);
> }
>
> -static Property ram_properties[] = {
> - DEFINE_PROP_UINT64("size", RamDevice, size, 0),
> - DEFINE_PROP_END_OF_LIST(),
> -};
> -
> static void ram_class_init(ObjectClass *klass, void *data)
> {
> DeviceClass *dc = DEVICE_CLASS(klass);
>
> dc->realize = ram_realize;
> - dc->props = ram_properties;
> }
>
> static const TypeInfo ram_info = {
> .name = TYPE_SUN4M_MEMORY,
> .parent = TYPE_SYS_BUS_DEVICE,
> .instance_size = sizeof(RamDevice),
> + .instance_init = ram_initfn,
> .class_init = ram_class_init,
> };
>
> @@ -880,6 +859,13 @@ static void sun4m_hw_init(const struct sun4m_hwdef
> *hwdef,
> unsigned int smp_cpus = machine->smp.cpus;
> unsigned int max_cpus = machine->smp.max_cpus;
>
> + if (machine->ram_size > hwdef->max_mem) {
> + error_report("Too much memory for this machine: %" PRId64 ","
> + " maximum %" PRId64,
> + machine->ram_size / MiB, hwdef->max_mem / MiB);
> + exit(1);
> + }
> +
> /* init CPUs */
> for(i = 0; i < smp_cpus; i++) {
> cpu_devinit(machine->cpu_type, i, hwdef->slavio_base, &cpu_irqs[i]);
> @@ -888,9 +874,13 @@ static void sun4m_hw_init(const struct sun4m_hwdef
> *hwdef,
> for (i = smp_cpus; i < MAX_CPUS; i++)
> cpu_irqs[i] = qemu_allocate_irqs(dummy_cpu_set_irq, NULL, MAX_PILS);
>
> + /* Create and map RAM frontend */
> + dev = qdev_create(NULL, "memory");
> + object_property_set_link(OBJECT(dev), OBJECT(machine->ram_memdev),
> + "memdev", &error_fatal);
> + qdev_init_nofail(dev);
> + sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, 0);
>
> - /* set up devices */
> - ram_init(0, machine->ram_size, hwdef->max_mem);
> /* models without ECC don't trap when missing ram is accessed */
> if (!hwdef->ecc_base) {
> empty_slot_init(machine->ram_size, hwdef->max_mem -
> machine->ram_size);
> @@ -1078,7 +1068,7 @@ static void sun4m_hw_init(const struct sun4m_hwdef
> *hwdef,
>
> fw_cfg_add_i16(fw_cfg, FW_CFG_NB_CPUS, (uint16_t)smp_cpus);
> fw_cfg_add_i16(fw_cfg, FW_CFG_MAX_CPUS, (uint16_t)max_cpus);
> - fw_cfg_add_i64(fw_cfg, FW_CFG_RAM_SIZE, (uint64_t)ram_size);
> + fw_cfg_add_i64(fw_cfg, FW_CFG_RAM_SIZE, (uint64_t)machine->ram_size);
> fw_cfg_add_i16(fw_cfg, FW_CFG_MACHINE_ID, hwdef->machine_id);
> fw_cfg_add_i16(fw_cfg, FW_CFG_SUN4M_DEPTH, graphic_depth);
> fw_cfg_add_i16(fw_cfg, FW_CFG_SUN4M_WIDTH, graphic_width);
> @@ -1415,6 +1405,7 @@ static void ss5_class_init(ObjectClass *oc, void *data)
> mc->default_boot_order = "c";
> mc->default_cpu_type = SPARC_CPU_TYPE_NAME("Fujitsu-MB86904");
> mc->default_display = "tcx";
> + mc->default_ram_id = "sun4m.ram";
> }
>
> static const TypeInfo ss5_type = {
> @@ -1434,6 +1425,7 @@ static void ss10_class_init(ObjectClass *oc, void *data)
> mc->default_boot_order = "c";
> mc->default_cpu_type = SPARC_CPU_TYPE_NAME("TI-SuperSparc-II");
> mc->default_display = "tcx";
> + mc->default_ram_id = "sun4m.ram";
> }
>
> static const TypeInfo ss10_type = {
> @@ -1453,6 +1445,7 @@ static void ss600mp_class_init(ObjectClass *oc, void
> *data)
> mc->default_boot_order = "c";
> mc->default_cpu_type = SPARC_CPU_TYPE_NAME("TI-SuperSparc-II");
> mc->default_display = "tcx";
> + mc->default_ram_id = "sun4m.ram";
> }
>
> static const TypeInfo ss600mp_type = {
> @@ -1472,6 +1465,7 @@ static void ss20_class_init(ObjectClass *oc, void *data)
> mc->default_boot_order = "c";
> mc->default_cpu_type = SPARC_CPU_TYPE_NAME("TI-SuperSparc-II");
> mc->default_display = "tcx";
> + mc->default_ram_id = "sun4m.ram";
> }
>
> static const TypeInfo ss20_type = {
> @@ -1490,6 +1484,7 @@ static void voyager_class_init(ObjectClass *oc, void
> *data)
> mc->default_boot_order = "c";
> mc->default_cpu_type = SPARC_CPU_TYPE_NAME("Fujitsu-MB86904");
> mc->default_display = "tcx";
> + mc->default_ram_id = "sun4m.ram";
> }
>
> static const TypeInfo voyager_type = {
> @@ -1508,6 +1503,7 @@ static void ss_lx_class_init(ObjectClass *oc, void
> *data)
> mc->default_boot_order = "c";
> mc->default_cpu_type = SPARC_CPU_TYPE_NAME("TI-MicroSparc-I");
> mc->default_display = "tcx";
> + mc->default_ram_id = "sun4m.ram";
> }
>
> static const TypeInfo ss_lx_type = {
> @@ -1526,6 +1522,7 @@ static void ss4_class_init(ObjectClass *oc, void *data)
> mc->default_boot_order = "c";
> mc->default_cpu_type = SPARC_CPU_TYPE_NAME("Fujitsu-MB86904");
> mc->default_display = "tcx";
> + mc->default_ram_id = "sun4m.ram";
> }
>
> static const TypeInfo ss4_type = {
> @@ -1544,6 +1541,7 @@ static void scls_class_init(ObjectClass *oc, void *data)
> mc->default_boot_order = "c";
> mc->default_cpu_type = SPARC_CPU_TYPE_NAME("TI-MicroSparc-I");
> mc->default_display = "tcx";
> + mc->default_ram_id = "sun4m.ram";
> }
>
> static const TypeInfo scls_type = {
> @@ -1562,6 +1560,7 @@ static void sbook_class_init(ObjectClass *oc, void
> *data)
> mc->default_boot_order = "c";
> mc->default_cpu_type = SPARC_CPU_TYPE_NAME("TI-MicroSparc-I");
> mc->default_display = "tcx";
> + mc->default_ram_id = "sun4m.ram";
> }
>
> static const TypeInfo sbook_type = {
As you say it's a little bit more complicated here, however it looks sensible
to me.
Acked-by: Mark Cave-Ayland <mark.cave-ayl...@ilande.co.uk>
ATB,
Mark.
