[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[PULL 10/20] Revert "hw/isa/vt82c686: Remove intermediate IRQ forwarder"
From: |
Philippe Mathieu-Daudé |
Subject: |
[PULL 10/20] Revert "hw/isa/vt82c686: Remove intermediate IRQ forwarder" |
Date: |
Wed, 8 Mar 2023 00:47:01 +0100 |
From: BALATON Zoltan <balaton@eik.bme.hu>
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 bb98e0f59cde ("hw/isa/vt82c686: 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]
928a8552-ab62-9e6c-a492-d6453e338b9d@redhat.com/">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/
Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
Tested-by: Rene Engel <ReneEngel80@emailn.de>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Message-Id:
<cdfb3c5a42e505450f6803124f27856434c5b298.1677628524.git.balaton@eik.bme.hu>
[PMD: Reworded description]
Inspired-by: Bernhard Beschow <shentey@gmail.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
hw/isa/vt82c686.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
index f4c40965cd..01e0148967 100644
--- a/hw/isa/vt82c686.c
+++ b/hw/isa/vt82c686.c
@@ -598,15 +598,23 @@ void via_isa_set_irq(PCIDevice *d, int n, int level)
qemu_set_irq(s->isa_irqs_in[n], level);
}
+static void via_isa_request_i8259_irq(void *opaque, int irq, int level)
+{
+ ViaISAState *s = opaque;
+ qemu_set_irq(s->cpu_intr, level);
+}
+
static void via_isa_realize(PCIDevice *d, Error **errp)
{
ViaISAState *s = VIA_ISA(d);
DeviceState *dev = DEVICE(d);
PCIBus *pci_bus = pci_get_bus(d);
+ qemu_irq *isa_irq;
ISABus *isa_bus;
int i;
qdev_init_gpio_out(dev, &s->cpu_intr, 1);
+ isa_irq = qemu_allocate_irqs(via_isa_request_i8259_irq, s, 1);
isa_bus = isa_bus_new(dev, pci_address_space(d), pci_address_space_io(d),
errp);
@@ -614,7 +622,7 @@ static void via_isa_realize(PCIDevice *d, Error **errp)
return;
}
- s->isa_irqs_in = i8259_init(isa_bus, s->cpu_intr);
+ s->isa_irqs_in = i8259_init(isa_bus, *isa_irq);
isa_bus_register_input_irqs(isa_bus, s->isa_irqs_in);
i8254_pit_init(isa_bus, 0x40, 0, NULL);
i8257_dma_init(isa_bus, 0);
--
2.38.1
- [PULL 00/20] MIPS patches for 2023-03-07, Philippe Mathieu-Daudé, 2023/03/07
- [PULL 01/20] docs/system: Remove "mips" board from target-mips.rst, Philippe Mathieu-Daudé, 2023/03/07
- [PULL 02/20] target/mips: Replace [g_]assert(0) -> g_assert_not_reached(), Philippe Mathieu-Daudé, 2023/03/07
- [PULL 03/20] target/mips: Fix JALS32/J32 instruction handling for microMIPS, Philippe Mathieu-Daudé, 2023/03/07
- [PULL 04/20] target/mips: Fix SWM32 handling for microMIPS, Philippe Mathieu-Daudé, 2023/03/07
- [PULL 05/20] target/mips: Implement CP0.Config7.WII bit support, Philippe Mathieu-Daudé, 2023/03/07
- [PULL 06/20] target/mips: Set correct CP0.Config[4, 5] values for M14K(c), Philippe Mathieu-Daudé, 2023/03/07
- [PULL 07/20] hw/mips: Declare all length properties as unsigned, Philippe Mathieu-Daudé, 2023/03/07
- [PULL 08/20] hw/mips/itu: Pass SAAR using QOM link property, Philippe Mathieu-Daudé, 2023/03/07
- [PULL 09/20] Revert "hw/isa/i82378: Remove intermediate IRQ forwarder", Philippe Mathieu-Daudé, 2023/03/07
- [PULL 10/20] Revert "hw/isa/vt82c686: Remove intermediate IRQ forwarder",
Philippe Mathieu-Daudé <=
- [PULL 11/20] hw/display/sm501: Add debug property to control pixman usage, Philippe Mathieu-Daudé, 2023/03/07
- [PULL 12/20] hw/intc/i8259: Implement legacy LTIM Edge/Level Bank Select, Philippe Mathieu-Daudé, 2023/03/07
- [PULL 13/20] hw/isa/vt82c686: Implement PCI IRQ routing, Philippe Mathieu-Daudé, 2023/03/07
- [PULL 14/20] hw/ppc/pegasos2: Fix PCI interrupt routing, Philippe Mathieu-Daudé, 2023/03/07
- [PULL 15/20] hw/usb/vt82c686-uhci-pci: Use PCI IRQ routing, Philippe Mathieu-Daudé, 2023/03/07
- [PULL 16/20] hw/audio/via-ac97: Basic implementation of audio playback, Philippe Mathieu-Daudé, 2023/03/07
- [PULL 18/20] ui/cocoa: Override windowDidResignKey, Philippe Mathieu-Daudé, 2023/03/07