PortioList: Store PortioList in device state

PortioList is an abstraction used for construction of MemoryRegionPortioList
from MemoryRegionPortio. It can be used later to unmap created memory regions.
It also requires proper cleanup because some of the memory inside is allocated
dynamically.

By moving PortioList ot device state we make it possible to cleanup later and
avoid leaking memory.

This change spans several target platforms.  The following testcases cover all
changed lines:
  qemu-system-ppc -M prep
  qemu-system-i386 -vga qxl
  qemu-system-i386 -M isapc -soundhw adlib -device ib700,id=watchdog0,bus=isa.0

Signed-off-by: Kirill Batuzov <batuzovk@ispras.ru>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Andreas Färber <afaerber@suse.de>

diff --git a/hw/audio/adlib.c b/hw/audio/adlib.c
index 28eed81280a92b1ef2babea038e35638cd0d5a61..5dd739e541d8435fd86cdadb3044de0e0e441241 100644 (file)
--- a/hw/audio/adlib.c
+++ b/hw/audio/adlib.c
@@ -86,6 +86,7 @@ typedef struct {
 #ifndef HAS_YMF262
     FM_OPL *opl;
 #endif
+    PortioList port_list;
 } AdlibState;
 
 static AdlibState *glob_adlib;
@@ -293,7 +294,6 @@ static MemoryRegionPortio adlib_portio_list[] = {
 static void adlib_realizefn (DeviceState *dev, Error **errp)
 {
     AdlibState *s = ADLIB(dev);
-    PortioList *port_list = g_new(PortioList, 1);
     struct audsettings as;
 
     if (glob_adlib) {
@@ -349,8 +349,8 @@ static void adlib_realizefn (DeviceState *dev, Error **errp)
 
     adlib_portio_list[0].offset = s->port;
     adlib_portio_list[1].offset = s->port + 8;
-    portio_list_init (port_list, OBJECT(s), adlib_portio_list, s, "adlib");
-    portio_list_add (port_list, isa_address_space_io(&s->parent_obj), 0);
+    portio_list_init (&s->port_list, OBJECT(s), adlib_portio_list, s, "adlib");
+    portio_list_add (&s->port_list, isa_address_space_io(&s->parent_obj), 0);
 }
 
 static Property adlib_properties[] = {

diff --git a/hw/display/qxl.c b/hw/display/qxl.c
index e9c54d7399d57edc9080401f228ec394d80f38c1..7fb83e4eafcdc786ed2d0ea3e2bddfa043435eaa 100644 (file)
--- a/hw/display/qxl.c
+++ b/hw/display/qxl.c
@@ -2055,7 +2055,6 @@ static int qxl_init_primary(PCIDevice *dev)
 {
     PCIQXLDevice *qxl = DO_UPCAST(PCIQXLDevice, pci, dev);
     VGACommonState *vga = &qxl->vga;
-    PortioList *qxl_vga_port_list = g_new(PortioList, 1);
     int rc;
 
     qxl->id = 0;
@@ -2064,10 +2063,10 @@ static int qxl_init_primary(PCIDevice *dev)
     vga_common_init(vga, OBJECT(dev), true);
     vga_init(vga, OBJECT(dev),
              pci_address_space(dev), pci_address_space_io(dev), false);
-    portio_list_init(qxl_vga_port_list, OBJECT(dev), qxl_vga_portio_list,
+    portio_list_init(&qxl->vga_port_list, OBJECT(dev), qxl_vga_portio_list,
                      vga, "vga");
-    portio_list_set_flush_coalesced(qxl_vga_port_list);
-    portio_list_add(qxl_vga_port_list, pci_address_space_io(dev), 0x3b0);
+    portio_list_set_flush_coalesced(&qxl->vga_port_list);
+    portio_list_add(&qxl->vga_port_list, pci_address_space_io(dev), 0x3b0);
 
     vga->con = graphic_console_init(DEVICE(dev), 0, &qxl_ops, qxl);
     qemu_spice_display_init_common(&qxl->ssd);

diff --git a/hw/display/qxl.h b/hw/display/qxl.h
index c5de3d707590a7c9edac468923c1f3412c3e078e..412e346b687fdf6d1387001eefd6d7526daa6d8d 100644 (file)
--- a/hw/display/qxl.h
+++ b/hw/display/qxl.h
@@ -32,6 +32,7 @@ enum qxl_mode {
 
 typedef struct PCIQXLDevice {
     PCIDevice          pci;
+    PortioList         vga_port_list;
     SimpleSpiceDisplay ssd;
     int                id;
     uint32_t           debug;

diff --git a/hw/display/vga.c b/hw/display/vga.c
index c4c3238d5ffff989ce72f7ab6d9b0072796f8c36..8cd6afe83ade241444754c9eb7798729e688ccab 100644 (file)
--- a/hw/display/vga.c
+++ b/hw/display/vga.c
@@ -2355,8 +2355,6 @@ void vga_init(VGACommonState *s, Object *obj, MemoryRegion *address_space,
 {
     MemoryRegion *vga_io_memory;
     const MemoryRegionPortio *vga_ports, *vbe_ports;
-    PortioList *vga_port_list = g_new(PortioList, 1);
-    PortioList *vbe_port_list = g_new(PortioList, 1);
 
     qemu_register_reset(vga_reset, s);
 
@@ -2371,13 +2369,13 @@ void vga_init(VGACommonState *s, Object *obj, MemoryRegion *address_space,
                                         1);
     memory_region_set_coalescing(vga_io_memory);
     if (init_vga_ports) {
-        portio_list_init(vga_port_list, obj, vga_ports, s, "vga");
-        portio_list_set_flush_coalesced(vga_port_list);
-        portio_list_add(vga_port_list, address_space_io, 0x3b0);
+        portio_list_init(&s->vga_port_list, obj, vga_ports, s, "vga");
+        portio_list_set_flush_coalesced(&s->vga_port_list);
+        portio_list_add(&s->vga_port_list, address_space_io, 0x3b0);
     }
     if (vbe_ports) {
-        portio_list_init(vbe_port_list, obj, vbe_ports, s, "vbe");
-        portio_list_add(vbe_port_list, address_space_io, 0x1ce);
+        portio_list_init(&s->vbe_port_list, obj, vbe_ports, s, "vbe");
+        portio_list_add(&s->vbe_port_list, address_space_io, 0x1ce);
     }
 }
 

diff --git a/hw/display/vga_int.h b/hw/display/vga_int.h
index d42ac926e3540beb5fdfea3f8b300c502ec8439d..5320abdc076c221fd35c4aa714fc385a9e70e90e 100644 (file)
--- a/hw/display/vga_int.h
+++ b/hw/display/vga_int.h
@@ -124,6 +124,8 @@ typedef struct VGACommonState {
     void (*get_resolution)(struct VGACommonState *s,
                         int *pwidth,
                         int *pheight);
+    PortioList vga_port_list;
+    PortioList vbe_port_list;
     /* bochs vbe state */
     uint16_t vbe_index;
     uint16_t vbe_regs[VBE_DISPI_INDEX_NB];

diff --git a/hw/dma/i82374.c b/hw/dma/i82374.c
index dc7a767ee28186c3abb1afb6303e0d34186cc382..b8ad2e64ec55414473823fd15b7bde5b29762743 100644 (file)
--- a/hw/dma/i82374.c
+++ b/hw/dma/i82374.c
@@ -39,6 +39,7 @@ do { fprintf(stderr, "i82374 ERROR: " fmt , ## __VA_ARGS__); } while (0)
 typedef struct I82374State {
     uint8_t commands[8];
     qemu_irq out;
+    PortioList port_list;
 } I82374State;
 
 static const VMStateDescription vmstate_i82374 = {
@@ -137,10 +138,10 @@ static void i82374_isa_realize(DeviceState *dev, Error **errp)
 {
     ISAi82374State *isa = I82374(dev);
     I82374State *s = &isa->state;
-    PortioList *port_list = g_new(PortioList, 1);
 
-    portio_list_init(port_list, OBJECT(isa), i82374_portio_list, s, "i82374");
-    portio_list_add(port_list, isa_address_space_io(&isa->parent_obj),
+    portio_list_init(&s->port_list, OBJECT(isa), i82374_portio_list, s,
+                     "i82374");
+    portio_list_add(&s->port_list, isa_address_space_io(&isa->parent_obj),
                     isa->iobase);
 
     i82374_realize(s, errp);

diff --git a/hw/isa/isa-bus.c b/hw/isa/isa-bus.c
index 55d01008d3827fafed2d71266b34e4273b3a9b09..b28981bfdea6811d49fb68bb013c303345ee71a4 100644 (file)
--- a/hw/isa/isa-bus.c
+++ b/hw/isa/isa-bus.c
@@ -108,15 +108,20 @@ void isa_register_portio_list(ISADevice *dev, uint16_t start,
                               const MemoryRegionPortio *pio_start,
                               void *opaque, const char *name)
 {
-    PortioList *piolist = g_new(PortioList, 1);
+    PortioList piolist;
 
     /* START is how we should treat DEV, regardless of the actual
        contents of the portio array.  This is how the old code
        actually handled e.g. the FDC device.  */
     isa_init_ioport(dev, start);
 
-    portio_list_init(piolist, OBJECT(dev), pio_start, opaque, name);
-    portio_list_add(piolist, isabus->address_space_io, start);
+    /* FIXME: the device should store created PortioList in its state.  Note
+       that DEV can be NULL here and that single device can register several
+       portio lists.  Current implementation is leaking memory allocated
+       in portio_list_init.  The leak is not critical because it happens only
+       at initialization time.  */
+    portio_list_init(&piolist, OBJECT(dev), pio_start, opaque, name);
+    portio_list_add(&piolist, isabus->address_space_io, start);
 }
 
 static void isa_device_init(Object *obj)

diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
index e2436512f78b2c14acc69b220396509c120ceebb..585937321fc23580d5d39d42763382b1c052cedf 100644 (file)
--- a/hw/ppc/prep.c
+++ b/hw/ppc/prep.c
@@ -361,6 +361,8 @@ static const MemoryRegionPortio prep_portio_list[] = {
     PORTIO_END_OF_LIST(),
 };
 
+static PortioList prep_port_list;
+
 /* PowerPC PREP hardware initialisation */
 static void ppc_prep_init(QEMUMachineInitArgs *args)
 {
@@ -375,7 +377,6 @@ static void ppc_prep_init(QEMUMachineInitArgs *args)
     CPUPPCState *env = NULL;
     nvram_t nvram;
     M48t59State *m48t59;
-    PortioList *port_list = g_new(PortioList, 1);
 #if 0
     MemoryRegion *xcsr = g_new(MemoryRegion, 1);
 #endif
@@ -542,8 +543,8 @@ static void ppc_prep_init(QEMUMachineInitArgs *args)
     cpu = POWERPC_CPU(first_cpu);
     sysctrl->reset_irq = cpu->env.irq_inputs[PPC6xx_INPUT_HRESET];
 
-    portio_list_init(port_list, NULL, prep_portio_list, sysctrl, "prep");
-    portio_list_add(port_list, isa_address_space_io(isa), 0x0);
+    portio_list_init(&prep_port_list, NULL, prep_portio_list, sysctrl, "prep");
+    portio_list_add(&prep_port_list, isa_address_space_io(isa), 0x0);
 
     /* PowerPC control and status register group */
 #if 0

diff --git a/hw/watchdog/wdt_ib700.c b/hw/watchdog/wdt_ib700.c
index bc994a4c32e46e92470cd3f6d2f0a6a2a98d5df5..68b33e12be48eef6cc5f45fe487656dc7e509118 100644 (file)
--- a/hw/watchdog/wdt_ib700.c
+++ b/hw/watchdog/wdt_ib700.c
@@ -42,6 +42,8 @@ typedef struct IB700state {
     ISADevice parent_obj;
 
     QEMUTimer *timer;
+
+    PortioList port_list;
 } IB700State;
 
 /* This is the timer.  We use a global here because the watchdog
@@ -106,14 +108,13 @@ static const MemoryRegionPortio wdt_portio_list[] = {
 static void wdt_ib700_realize(DeviceState *dev, Error **errp)
 {
     IB700State *s = IB700(dev);
-    PortioList *port_list = g_new(PortioList, 1);
 
     ib700_debug("watchdog init\n");
 
     s->timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, ib700_timer_expired, s);
 
-    portio_list_init(port_list, OBJECT(s), wdt_portio_list, s, "ib700");
-    portio_list_add(port_list, isa_address_space_io(&s->parent_obj), 0);
+    portio_list_init(&s->port_list, OBJECT(s), wdt_portio_list, s, "ib700");
+    portio_list_add(&s->port_list, isa_address_space_io(&s->parent_obj), 0);
 }
 
 static void wdt_ib700_reset(DeviceState *dev)