diff options
| author | Philippe Mathieu-Daudé <philmd@linaro.org> | 2023-03-07 12:10:08 +0100 |
|---|---|---|
| committer | Philippe Mathieu-Daudé <philmd@linaro.org> | 2023-03-08 00:37:48 +0100 |
| commit | d1396cc74935bec473282fdcaeae3cb52910187b (patch) | |
| tree | 729795979cc613218ec99615fde41517262a19d1 /hw/isa/i82378.c | |
| parent | 4c921e3fb2a9f53cbc97318487844b48ad3781f8 (diff) | |
| download | qemu-d1396cc74935bec473282fdcaeae3cb52910187b.zip qemu-d1396cc74935bec473282fdcaeae3cb52910187b.tar.gz qemu-d1396cc74935bec473282fdcaeae3cb52910187b.tar.bz2 | |
Revert "hw/isa/i82378: Remove intermediate IRQ forwarder"
To be 'usable', QDev objects (which are QOM objects) must be
1/ initialized (at this point their properties can be modified), then
2/ realized (properties are consumed).
Some devices (objects) might depend on other devices. When creating
the 'QOM composition tree', parent objects can't be 'realized' until
all their children are. We might also have circular dependencies.
A common circular dependency occurs with IRQs. Device (A) has an
output IRQ wired to device (B), and device (B) has one to device (A).
When (A) is realized and connects its IRQ to an unrealized (B), the
IRQ handler on (B) is not yet created. QEMU pass IRQ between objects
as pointer. When (A) poll (B)'s IRQ, it is NULL. Later (B) is realized
and its IRQ pointers are populated, but (A) keeps a reference to a
NULL pointer.
A common pattern to bypass this circular limitation is to use 'proxy'
objects. Proxy (P) is created (and realized) before (A) and (B). Then
(A) and (B) can be created in different order, it doesn't matter: (P)
pointers are already populated.
Commit cef2e7148e ("hw/isa/i82378: Remove intermediate IRQ forwarder")
neglected the QOM/QDev circular dependency issue, and removed the
'proxy' between the southbridge, its PCI functions and the interrupt
controller, resulting in PCI functions wiring output IRQs to
'NULL', leading to guest failures (IRQ never delivered) [1] [2].
Since we are entering feature freeze, it is safer to revert the
offending patch until we figure a way to strengthen our APIs.
[1] https://lore.kernel.org/qemu-devel/928a8552-ab62-9e6c-a492-d6453e338b9d@redhat.com/
[2] https://lore.kernel.org/qemu-devel/cover.1677628524.git.balaton@eik.bme.hu/
This reverts commit cef2e7148e32d61338de0220619d308bf42af770.
Reported-by: Thomas Huth <thuth@redhat.com>
Inspired-by: Bernhard Beschow <shentey@gmail.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Diffstat (limited to 'hw/isa/i82378.c')
| -rw-r--r-- | hw/isa/i82378.c | 10 |
1 files changed, 9 insertions, 1 deletions
diff --git a/hw/isa/i82378.c b/hw/isa/i82378.c index 233059c..5432ab5 100644 --- a/hw/isa/i82378.c +++ b/hw/isa/i82378.c @@ -47,6 +47,12 @@ static const VMStateDescription vmstate_i82378 = { }, }; +static void i82378_request_out0_irq(void *opaque, int irq, int level) +{ + I82378State *s = opaque; + qemu_set_irq(s->cpu_intr, level); +} + static void i82378_request_pic_irq(void *opaque, int irq, int level) { DeviceState *dev = opaque; @@ -88,7 +94,9 @@ static void i82378_realize(PCIDevice *pci, Error **errp) */ /* 2 82C59 (irq) */ - s->isa_irqs_in = i8259_init(isabus, s->cpu_intr); + s->isa_irqs_in = i8259_init(isabus, + qemu_allocate_irq(i82378_request_out0_irq, + s, 0)); isa_bus_register_input_irqs(isabus, s->isa_irqs_in); /* 1 82C54 (pit) */ |