[v2,73/86] sparc:sun4m: use memdev for RAM
diff mbox series

Message ID 1579100861-73692-74-git-send-email-imammedo@redhat.com
State New
Headers show
Series
  • refactor main RAM allocation to use hostmem backend
Related show

Commit Message

Igor Mammedov Jan. 15, 2020, 3:07 p.m. UTC
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 <imammedo@redhat.com>
---
CC: mark.cave-ayland@ilande.co.uk
CC: atar4qemu@gmail.com
---
 hw/sparc/sun4m.c | 73 ++++++++++++++++++++++++++++----------------------------
 1 file changed, 36 insertions(+), 37 deletions(-)

Comments

Mark Cave-Ayland Jan. 16, 2020, 9:12 a.m. UTC | #1
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 <imammedo@redhat.com>
> ---
> CC: mark.cave-ayland@ilande.co.uk
> CC: atar4qemu@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-ayland@ilande.co.uk>


ATB,

Mark.

Patch
diff mbox series

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