On Tue, 2 Jul 2024, Bernhard Beschow wrote:
Am 2. Juli 2024 18:42:23 UTC schrieb Bernhard Beschow <shen...@gmail.com>:
Am 1. Juli 2024 12:58:15 UTC schrieb Peter Maydell <peter.mayd...@linaro.org>:
On Sat, 29 Jun 2024 at 21:01, BALATON Zoltan <bala...@eik.bme.hu> wrote:
To avoid a warning about unfreed qemu_irq embed the i8259 irq in the
device state instead of allocating it.
Signed-off-by: BALATON Zoltan <bala...@eik.bme.hu>
---
hw/isa/vt82c686.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
index 8582ac0322..834051abeb 100644
--- a/hw/isa/vt82c686.c
+++ b/hw/isa/vt82c686.c
@@ -592,6 +592,8 @@ OBJECT_DECLARE_SIMPLE_TYPE(ViaISAState, VIA_ISA)
struct ViaISAState {
PCIDevice dev;
+
+ IRQState i8259_irq;
qemu_irq cpu_intr;
qemu_irq *isa_irqs_in;
uint16_t irq_state[ISA_NUM_IRQS];
@@ -715,13 +717,12 @@ 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);
qdev_init_gpio_in_named(dev, via_isa_pirq, "pirq", PCI_NUM_PINS);
- isa_irq = qemu_allocate_irqs(via_isa_request_i8259_irq, s, 1);
+ qemu_init_irq(&s->i8259_irq, via_isa_request_i8259_irq, s, 0);
isa_bus = isa_bus_new(dev, pci_address_space(d), pci_address_space_io(d),
errp);
So if I understand correctly, this IRQ line isn't visible
from outside this chip,
Actally it is, in the form of the INTR pin. Assuming similar naming
The INTR pin corresponds to qemu_irq cpu_intr not the i8259_irq.
conventions in vt82xx and piix, one can confirm this by consulting the
piix4 datasheet, "Figure 5. Interrupt Controller Block Diagram".
Moreover, the pegasos2 schematics (linked in the QEMU documentation)
suggest that this pin is actually used there, although not modeled in
QEMU.
Well, QEMU does actually wire the intr pin in the pegasos2 board code,
except that it isn't a named gpio like in piix4. If we allow this pin to
I could make that named to make it clearer, now it's the only output gpio
so did not name it as usually devices that only have one output don't use
named gpios for that.
be wired before the south bridge's realize we might be able to eliminate
the "intermediate irq forwarder" as Phil used to name it, resulting in
less and more efficient code. This solution would basically follow the
pattern I outlined under below link.
I think the problem here is that i8259 does not provide an output gpio for
this interrupt that the VT82xx could pass on but instead i8259_init()
needs a qemu_irq to be passed rhat the i8259 model will set. This seems to
be a legacy init function so the fix may be to Qdev-ify i8259 and add an
output irq to it then its users could instantiate and connect its IRQs as
usual and we don't need to create a qemu_irq to pass it to i8259_init().
Regards,
BALATON Zoltan
Best regards,
Bernhard
Compare this to how the "intr" pin is exposed by the piix4 device model and
wired in the Malta board.
we're just trying to wire together
two internal components of the chip? If so, I agree that
this seems a better way than creating a named GPIO that
we then have to document as a "not really an external
connection, don't try to use this" line. (We've done that
before I think in other devices, and it works but it's
a bit odd-looking.)
That said, I do notice that the via_isa_request_i8259_irq()
function doesn't do anything except pass the level onto
another qemu_irq, so I think the theoretical ideal would be
if we could arrange to plumb things directly through rather
than needing this extra qemu_irq and function. There's
probably a reason (order of device creation/connection?)
that doesn't work though.
I think there could be a general pattern of device creation/connection which
I've outlined here:
https://lore.kernel.org/qemu-devel/0ffb5fd2-08ce-4cec-9001-e7ac24407...@gmail.com/
Best regards,
Bernhard
-- PMM
