Re: [PATCH 0/2] m68k/q800: make the GLUE chip a QOM device

[Peter Maydell]

On Fri, 6 Nov 2020 at 23:51, Peter Maydell <peter.maydell@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