Stefan Hajnoczi <stefa...@redhat.com> writes: > On Wed, Apr 28, 2021 at 04:18:17PM +0200, Markus Armbruster wrote: >> Stefan Hajnoczi <stefa...@redhat.com> writes:
[...] >> > The approach in this patch is okay but we should keep in mind it only >> > solves piix3-ide. ISA provides a non-qdev backdoor API and there may be >> > more instances of this type of bug. >> > >> > A qdev fix would address the root cause and make it possible to drop the >> > backdoor API, but that's probably too much work for little benefit. >> >> What do you mean by backdoor API? Global @isabus? > > Yes. It's also strange that isa_get_irq(ISADevice *dev, unsigned isairq) > accepts dev = NULL as a valid argument. @isabus is static in hw/isa/isa-bus.c. Uses: * Limit isa_bus_new() to one ISA bus. Arbitrary restriction; multiple ISA buses could work with suitable memory mapping and IRQ wiring. "Single ISA bus" assumptions could of course hide elsewhere in the code. * Implied argument to isa_get_irq(), isa_register_ioport(), isa_register_portio_list(), isa_address_space(), isa_address_space_io(). isa_get_irq() asserts that a non-null @dev is a child of @isabus. This means we don't actually need @isabus, except when @dev is null. I suspect two separate functions would be cleaner: one taking an ISABus * argument, and a wrapper taking an ISADevice * argument. isa_address_space() and isa_address_space_io() work the same, less the assertion. isa_register_ioport() and isa_register_portio_list() take a non-null @dev argument. They don't actually need @isabus. To eliminate global @isabus, we need to fix up the callers passing null @dev. Clean solution: plumb the ISABus returned by isa_bus_new() to the call sites. Where that's impractical, we can also get it from QOM, like build_dsdt_microvm() does.
