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

[Laurent Vivier]

Le 07/11/2020 à 00:51, Peter Maydell a écrit :
> 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.
> 
> thanks
> -- PMM
> 
> Peter Maydell (2):
>   hw/m68k/q800: Don't connect two qemu_irqs directly to the same input
>   hw/m68k/q800.c: Make the GLUE chip an actual QOM device
> 
>  hw/m68k/q800.c  | 92 ++++++++++++++++++++++++++++++++++++++++++-------
>  hw/m68k/Kconfig |  1 +
>  2 files changed, 80 insertions(+), 13 deletions(-)
> 

Applied to my m68k-for-6.0 branch

Thanks,
Laurent