On Wed, 3 Jul 2024, Bernhard Beschow wrote:
Am 3. Juli 2024 00:09:45 UTC schrieb BALATON Zoltan <bala...@eik.bme.hu>:
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().
I've implemented the approach avoiding the intermediate IRQ forwarders
here: https://github.com/shentok/qemu/commits/upstream/vt82c686-irq/ .
I'd send this series to the list as soon as I resolve some email
authentication issues.
This connects the gpio out before the device is realized. I don't think
that's the right fix and confuses all the users of this device as they
will need to remember to do this. I think the current interrupt forwarder
is OK until i8259 is Qdev-ified and solves this within the device. I'm OK
with the patch that makes intr named if you can submit just that.
Regards,
BALATON Zoltan
