From 848696bf353750899832c51005f1bd3540da5c29 Mon Sep 17 00:00:00 2001 From: Kirill Batuzov Date: Tue, 29 Apr 2014 17:38:39 +0400 Subject: [PATCH] PortioList: Store PortioList in device state MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Reviewed-by: Paolo Bonzini Signed-off-by: Andreas Färber --- hw/audio/adlib.c | 6 +++--- hw/display/qxl.c | 7 +++---- hw/display/qxl.h | 1 + hw/display/vga.c | 12 +++++------- hw/display/vga_int.h | 2 ++ hw/dma/i82374.c | 7 ++++--- hw/isa/isa-bus.c | 11 ++++++++--- hw/ppc/prep.c | 7 ++++--- hw/watchdog/wdt_ib700.c | 7 ++++--- 9 files changed, 34 insertions(+), 26 deletions(-) diff --git a/hw/audio/adlib.c b/hw/audio/adlib.c index 28eed81280..5dd739e541 100644 --- 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 e9c54d7399..7fb83e4eaf 100644 --- 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 c5de3d7075..412e346b68 100644 --- 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 c4c3238d5f..8cd6afe83a 100644 --- 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 d42ac926e3..5320abdc07 100644 --- 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 dc7a767ee2..b8ad2e64ec 100644 --- 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 55d01008d3..b28981bfde 100644 --- 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 e2436512f7..585937321f 100644 --- 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 bc994a4c32..68b33e12be 100644 --- 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)