Message ID | 20230126211740.66874-3-shentey@gmail.com |
---|---|
State | New |
Headers | show |
Series | Resolve isabus global | expand |
On 26/01/2023 21:17, Bernhard Beschow wrote: > Both functions are always used together and in the same order. Let's > reflect this in the API. > > Inspired-by: <20210518215545.1793947-9-philmd@redhat.com> > 'hw/isa: Extract bus part from isa_register_portio_list()' > Signed-off-by: Bernhard Beschow <shentey@gmail.com> > --- > include/exec/ioport.h | 6 ++---- > hw/audio/adlib.c | 4 ++-- > hw/display/qxl.c | 5 ++--- > hw/display/vga.c | 8 ++++---- > hw/dma/i82374.c | 6 ++---- > hw/isa/isa-bus.c | 6 ++---- > hw/watchdog/wdt_ib700.c | 4 ++-- > softmmu/ioport.c | 19 +++++++------------ > 8 files changed, 23 insertions(+), 35 deletions(-) > > diff --git a/include/exec/ioport.h b/include/exec/ioport.h > index e34f668998..ec3e8e5942 100644 > --- a/include/exec/ioport.h > +++ b/include/exec/ioport.h > @@ -64,12 +64,10 @@ typedef struct PortioList { > > void portio_list_init(PortioList *piolist, Object *owner, > const struct MemoryRegionPortio *callbacks, > - void *opaque, const char *name); > + void *opaque, const char *name, > + MemoryRegion *address_space_io, uint16_t start); > void portio_list_set_flush_coalesced(PortioList *piolist); > void portio_list_destroy(PortioList *piolist); > -void portio_list_add(PortioList *piolist, > - struct MemoryRegion *address_space, > - uint32_t addr); > void portio_list_del(PortioList *piolist); > > #endif /* IOPORT_H */ > diff --git a/hw/audio/adlib.c b/hw/audio/adlib.c > index 5f979b1487..957abe3da7 100644 > --- a/hw/audio/adlib.c > +++ b/hw/audio/adlib.c > @@ -291,8 +291,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 (&s->port_list, OBJECT(s), adlib_portio_list, s, "adlib"); > - portio_list_add (&s->port_list, isa_address_space_io(&s->parent_obj), 0); > + portio_list_init(&s->port_list, OBJECT(s), adlib_portio_list, s, "adlib", > + 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 ec712d3ca2..6d5a931425 100644 > --- a/hw/display/qxl.c > +++ b/hw/display/qxl.c > @@ -2224,10 +2224,9 @@ static void qxl_realize_primary(PCIDevice *dev, Error **errp) > } > 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, > - vga, "vga"); > + portio_list_init(&qxl->vga_port_list, OBJECT(dev), qxl_vga_portio_list, vga, > + "vga", 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); > qxl->have_vga = true; > > vga->con = graphic_console_init(DEVICE(dev), 0, &qxl_ops, qxl); > diff --git a/hw/display/vga.c b/hw/display/vga.c > index 7a5fdff649..2b606a526c 100644 > --- a/hw/display/vga.c > +++ b/hw/display/vga.c > @@ -2309,12 +2309,12 @@ 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(&s->vga_port_list, obj, vga_ports, s, "vga"); > + portio_list_init(&s->vga_port_list, obj, vga_ports, s, "vga", > + address_space_io, 0x3b0); > 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(&s->vbe_port_list, obj, vbe_ports, s, "vbe"); > - portio_list_add(&s->vbe_port_list, address_space_io, 0x1ce); > + portio_list_init(&s->vbe_port_list, obj, vbe_ports, s, "vbe", > + address_space_io, 0x1ce); > } > } > diff --git a/hw/dma/i82374.c b/hw/dma/i82374.c > index 34c3aaf7d3..5914b34079 100644 > --- a/hw/dma/i82374.c > +++ b/hw/dma/i82374.c > @@ -131,10 +131,8 @@ static void i82374_realize(DeviceState *dev, Error **errp) > } > i8257_dma_init(isa_bus, true); > > - portio_list_init(&s->port_list, OBJECT(s), i82374_portio_list, s, > - "i82374"); > - portio_list_add(&s->port_list, isa_address_space_io(&s->parent_obj), > - s->iobase); > + portio_list_init(&s->port_list, OBJECT(s), i82374_portio_list, s, "i82374", > + isa_address_space_io(&s->parent_obj), s->iobase); > > memset(s->commands, 0, sizeof(s->commands)); > } > diff --git a/hw/isa/isa-bus.c b/hw/isa/isa-bus.c > index 1bee1a47f1..b3497dad61 100644 > --- a/hw/isa/isa-bus.c > +++ b/hw/isa/isa-bus.c > @@ -124,8 +124,6 @@ int isa_register_portio_list(ISADevice *dev, > const MemoryRegionPortio *pio_start, > void *opaque, const char *name) > { > - assert(piolist && !piolist->owner); > - > if (!isabus) { > return -ENODEV; > } > @@ -135,8 +133,8 @@ int isa_register_portio_list(ISADevice *dev, > 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); > + portio_list_init(piolist, OBJECT(dev), pio_start, opaque, name, > + isabus->address_space_io, start); > > return 0; > } > diff --git a/hw/watchdog/wdt_ib700.c b/hw/watchdog/wdt_ib700.c > index b116c3a3aa..e53e474b83 100644 > --- a/hw/watchdog/wdt_ib700.c > +++ b/hw/watchdog/wdt_ib700.c > @@ -115,8 +115,8 @@ static void wdt_ib700_realize(DeviceState *dev, Error **errp) > > s->timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, ib700_timer_expired, s); > > - 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); > + portio_list_init(&s->port_list, OBJECT(s), wdt_portio_list, s, "ib700", > + isa_address_space_io(&s->parent_obj), 0); > } > > static void wdt_ib700_reset(DeviceState *dev) > diff --git a/softmmu/ioport.c b/softmmu/ioport.c > index 215344467b..c92e3cb27d 100644 > --- a/softmmu/ioport.c > +++ b/softmmu/ioport.c > @@ -231,8 +231,13 @@ static void portio_list_add_1(PortioList *piolist, > > void portio_list_init(PortioList *piolist, Object *owner, > const MemoryRegionPortio *callbacks, > - void *opaque, const char *name) > + void *opaque, const char *name, > + MemoryRegion *address_space_io, uint16_t start) > { > + assert(piolist && !piolist->owner); > + > + const MemoryRegionPortio *pio, *pio_start = callbacks; > + unsigned int off_low, off_high, off_last, count; > unsigned n = 0; > > while (callbacks[n].size) { > @@ -242,21 +247,11 @@ void portio_list_init(PortioList *piolist, Object *owner, > piolist->ports = callbacks; > piolist->nr = 0; > piolist->regions = g_new0(MemoryRegion *, n); > - piolist->address_space = NULL; > + piolist->address_space = address_space_io; > piolist->opaque = opaque; > piolist->owner = owner; > piolist->name = name; > piolist->flush_coalesced_mmio = false; > -} > - > -void portio_list_add(PortioList *piolist, > - MemoryRegion *address_space, > - uint32_t start) > -{ > - const MemoryRegionPortio *pio, *pio_start = piolist->ports; > - unsigned int off_low, off_high, off_last, count; > - > - piolist->address_space = address_space; > > /* Handle the first entry specially. */ > off_last = off_low = pio_start->offset; As a general comment: is portio used outside of ISA bus devices at all? It seems to me that portio is really a wrapper around the the old x86 IO port functionality, and so I could see PortioList being moved exclusively to within ISADevice which could allow the address_space, opaque and owner to be resolved automatically with the PortioList being owned by ISADevice. ATB, Mark.
diff --git a/include/exec/ioport.h b/include/exec/ioport.h index e34f668998..ec3e8e5942 100644 --- a/include/exec/ioport.h +++ b/include/exec/ioport.h @@ -64,12 +64,10 @@ typedef struct PortioList { void portio_list_init(PortioList *piolist, Object *owner, const struct MemoryRegionPortio *callbacks, - void *opaque, const char *name); + void *opaque, const char *name, + MemoryRegion *address_space_io, uint16_t start); void portio_list_set_flush_coalesced(PortioList *piolist); void portio_list_destroy(PortioList *piolist); -void portio_list_add(PortioList *piolist, - struct MemoryRegion *address_space, - uint32_t addr); void portio_list_del(PortioList *piolist); #endif /* IOPORT_H */ diff --git a/hw/audio/adlib.c b/hw/audio/adlib.c index 5f979b1487..957abe3da7 100644 --- a/hw/audio/adlib.c +++ b/hw/audio/adlib.c @@ -291,8 +291,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 (&s->port_list, OBJECT(s), adlib_portio_list, s, "adlib"); - portio_list_add (&s->port_list, isa_address_space_io(&s->parent_obj), 0); + portio_list_init(&s->port_list, OBJECT(s), adlib_portio_list, s, "adlib", + 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 ec712d3ca2..6d5a931425 100644 --- a/hw/display/qxl.c +++ b/hw/display/qxl.c @@ -2224,10 +2224,9 @@ static void qxl_realize_primary(PCIDevice *dev, Error **errp) } 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, - vga, "vga"); + portio_list_init(&qxl->vga_port_list, OBJECT(dev), qxl_vga_portio_list, vga, + "vga", 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); qxl->have_vga = true; vga->con = graphic_console_init(DEVICE(dev), 0, &qxl_ops, qxl); diff --git a/hw/display/vga.c b/hw/display/vga.c index 7a5fdff649..2b606a526c 100644 --- a/hw/display/vga.c +++ b/hw/display/vga.c @@ -2309,12 +2309,12 @@ 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(&s->vga_port_list, obj, vga_ports, s, "vga"); + portio_list_init(&s->vga_port_list, obj, vga_ports, s, "vga", + address_space_io, 0x3b0); 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(&s->vbe_port_list, obj, vbe_ports, s, "vbe"); - portio_list_add(&s->vbe_port_list, address_space_io, 0x1ce); + portio_list_init(&s->vbe_port_list, obj, vbe_ports, s, "vbe", + address_space_io, 0x1ce); } } diff --git a/hw/dma/i82374.c b/hw/dma/i82374.c index 34c3aaf7d3..5914b34079 100644 --- a/hw/dma/i82374.c +++ b/hw/dma/i82374.c @@ -131,10 +131,8 @@ static void i82374_realize(DeviceState *dev, Error **errp) } i8257_dma_init(isa_bus, true); - portio_list_init(&s->port_list, OBJECT(s), i82374_portio_list, s, - "i82374"); - portio_list_add(&s->port_list, isa_address_space_io(&s->parent_obj), - s->iobase); + portio_list_init(&s->port_list, OBJECT(s), i82374_portio_list, s, "i82374", + isa_address_space_io(&s->parent_obj), s->iobase); memset(s->commands, 0, sizeof(s->commands)); } diff --git a/hw/isa/isa-bus.c b/hw/isa/isa-bus.c index 1bee1a47f1..b3497dad61 100644 --- a/hw/isa/isa-bus.c +++ b/hw/isa/isa-bus.c @@ -124,8 +124,6 @@ int isa_register_portio_list(ISADevice *dev, const MemoryRegionPortio *pio_start, void *opaque, const char *name) { - assert(piolist && !piolist->owner); - if (!isabus) { return -ENODEV; } @@ -135,8 +133,8 @@ int isa_register_portio_list(ISADevice *dev, 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); + portio_list_init(piolist, OBJECT(dev), pio_start, opaque, name, + isabus->address_space_io, start); return 0; } diff --git a/hw/watchdog/wdt_ib700.c b/hw/watchdog/wdt_ib700.c index b116c3a3aa..e53e474b83 100644 --- a/hw/watchdog/wdt_ib700.c +++ b/hw/watchdog/wdt_ib700.c @@ -115,8 +115,8 @@ static void wdt_ib700_realize(DeviceState *dev, Error **errp) s->timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, ib700_timer_expired, s); - 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); + portio_list_init(&s->port_list, OBJECT(s), wdt_portio_list, s, "ib700", + isa_address_space_io(&s->parent_obj), 0); } static void wdt_ib700_reset(DeviceState *dev) diff --git a/softmmu/ioport.c b/softmmu/ioport.c index 215344467b..c92e3cb27d 100644 --- a/softmmu/ioport.c +++ b/softmmu/ioport.c @@ -231,8 +231,13 @@ static void portio_list_add_1(PortioList *piolist, void portio_list_init(PortioList *piolist, Object *owner, const MemoryRegionPortio *callbacks, - void *opaque, const char *name) + void *opaque, const char *name, + MemoryRegion *address_space_io, uint16_t start) { + assert(piolist && !piolist->owner); + + const MemoryRegionPortio *pio, *pio_start = callbacks; + unsigned int off_low, off_high, off_last, count; unsigned n = 0; while (callbacks[n].size) { @@ -242,21 +247,11 @@ void portio_list_init(PortioList *piolist, Object *owner, piolist->ports = callbacks; piolist->nr = 0; piolist->regions = g_new0(MemoryRegion *, n); - piolist->address_space = NULL; + piolist->address_space = address_space_io; piolist->opaque = opaque; piolist->owner = owner; piolist->name = name; piolist->flush_coalesced_mmio = false; -} - -void portio_list_add(PortioList *piolist, - MemoryRegion *address_space, - uint32_t start) -{ - const MemoryRegionPortio *pio, *pio_start = piolist->ports; - unsigned int off_low, off_high, off_last, count; - - piolist->address_space = address_space; /* Handle the first entry specially. */ off_last = off_low = pio_start->offset;
Both functions are always used together and in the same order. Let's reflect this in the API. Inspired-by: <20210518215545.1793947-9-philmd@redhat.com> 'hw/isa: Extract bus part from isa_register_portio_list()' Signed-off-by: Bernhard Beschow <shentey@gmail.com> --- include/exec/ioport.h | 6 ++---- hw/audio/adlib.c | 4 ++-- hw/display/qxl.c | 5 ++--- hw/display/vga.c | 8 ++++---- hw/dma/i82374.c | 6 ++---- hw/isa/isa-bus.c | 6 ++---- hw/watchdog/wdt_ib700.c | 4 ++-- softmmu/ioport.c | 19 +++++++------------ 8 files changed, 23 insertions(+), 35 deletions(-)