On Fri, 6 Nov 2020 at 23:51, Peter Maydell <peter.mayd...@linaro.org> wrote: > > This series is 6.0 material really I think. It's a bit of cleanup > prompted by a Coverity issue, CID 1421883. There are another half > dozen or so similar issues, where Coverity is complaining that we > allocate an array of qemu_irqs with qemu_allocate_irqs() in a board > init function -- in this case the 'pic' array in q800_init() -- and > then we return from the board init function and the memory is leaked, > in the sense that nobody has a pointer to it any more. > > The leak isn't real, in that board init functions are called only > once, and the array of qemu_irqs really does need to stay around for > the life of the simulation, so these are pretty much insignificant > as Coverity issues go. But this coding style which uses a free-floating > set of qemu_irqs is not very "modern QEMU", so the issues act as > a nudge that we should clean the code up by encapsulating the > interrupt-line behaviour in a QOM device. In the q800 case there > actually is already a GLUEState struct, it just needs to be turned > into a QOM device with GPIO input lines. Patch 2 does that. > > Patch 1 fixes a bug I noticed while doing this work -- it's > not valid to connect two qemu_irq lines directly to the same > input (here both ESCC irq lines go to pic[3]) because it produces > weird behaviour like "both lines are asserted but the device > consuming the interrupt sees the line deassert when one of the > two inputs goes low, rather than only when they both go low". > You need to put an explicit OR gate in, assuming that logical-OR > is the desired behaviour, which it usually is. > > Tested only with 'make check' and 'make check-acceptance', > but the latter does have a q800 bootup test.
Laurent, did you want to take this series as an m68k patchset, or would you rather I just put it in via the target-arm queue? thanks -- PMM