On Tue, Jul 31, 2018 at 01:31:46AM +0200, BALATON Zoltan wrote: > On Tue, 31 Jul 2018, Peter Maydell wrote: > > On 30 July 2018 at 23:37, BALATON Zoltan <bala...@eik.bme.hu> wrote: > > QEMU's implementation of qemu_irq signal lines is that the destination > > end provides the qemu_irq, which under the hood is a pointer to a > > struct containing a pointer to the function in the destination device > > which gets called when the source end says "the line level has changed". > > This means that there won't be a compile time or runtime error if you > > pass that qemu_irq to multiple sources (ie device outputs) simultaneously. > > But the behaviour will be wrong, because the destination will see all > > the "level is 0", "level is 1" calls from all the sources intermingled, eg > > > > device A output: ____|^^^^^^^^^^^^^|______ > > > > device B output: _______|^^^^^|___________ > > > > destination sees: ____|^^^^^^^^|___________ > > > > (because the destination gets the "level now 0" call when B's output > > goes to zero). To get the desired behaviour: > > > > logical OR: ____|^^^^^^^^^^^^^|_____ > > > > you need an OR gate. (I'm assuming wired-OR because that's the > > usual thing when several devices share an interrupt.) > > > > If your testing involves a setup which doesn't happen to assert > > several of the interrupt lines simultaneously you won't notice this > > problem. > > I think the testing with SATA and USB mouse on a PCI card is likely to > assert several interrupts simultanelously (eg. when moving the mouse while > loading stuff from disk) but the OS might have some ways to still recover > from this so maybe we won't notice it anyway. As this is now confirmed that > using the same irq multiple times is wrong I think we need a better fix. > > David, can you please drop this patch, we'll come up with a different fix.
Done. Should have looked at that patch a bit closer.
> > > Probably we can just change the map function in ppc440_pcix.c to always
> > > return the first line then what's specified for other lines should not
> > > matter and the above problem is avoided. We could even get rid of those
> > > additional irqs by changing ppc440_pcix.c to only model a single line but
> > > I'd need someone with better understanding of this to confirm that I got
> > > this right.
> >
> > I think that would also fix the bug. The logically preferable
> > approach would depend rather on what the actual hardware does:
> > is there a PCI controller chip with 4 interrupt outputs which
> > the board wires together to a single interrupt controller line,
> > or does the PCI controller chip itself have just one output
> > and do the merging of the different PCI interrupts itself?
>
> Hmm, don't really know. The PCI controller is part of the SoC but we don't
> have the manual of this SoC. The physical board itself also has a single PCI
> slot so however it's wired internally it's only using one irq for that. Not
> sure what other internal PCI devices are there. A comment in U-Boot sources
> says this (it lists PCIB twice but maybe that's meant to be PCID?):
>
> // IRQ2 = PCI_INT (PCIA, PCIB, PCIC, PCIB)
>
> So that suggests probably there are 4 PCI irqs that are wired together but
> probably this is inside the SoC. It could also be read as there's only a
> single PCI_INT that delivers all four PCI interrupts. So if we go from that
> either adding an OR gate to sam460ex.c that ORs together the PCI lines and
> connects it to uic[1][0] would work, or changing ppc440_pcix.c to provide a
> single PCI interrupt? We don't really have definitive info other than this
> comment so whatever Sebastian prefers and you approve is fine with me.
>
> Thank you,
> BALATON Zoltan
>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
