Re: [PATCH] hw/ppc: sam460ex.c: store all GPIO lines in mal_irqs[]

[BALATON Zoltan <balaton@eik.bme.hu>]

[-- Attachment #1: Type: text/plain, Size: 2014 bytes --]

On Wed, 3 Aug 2022, Daniel Henrique Barboza wrote:
> We're not storing all GPIO lines we're retrieving with
> qdev_get_gpio_in() in mal_irqs[]. We're storing just the last one in the
> first index:
>
>    for (i = 0; i < ARRAY_SIZE(mal_irqs); i++) {
>        mal_irqs[0] = qdev_get_gpio_in(uic[2], 3 + i);
>    }
>    ppc4xx_mal_init(env, 4, 16, mal_irqs);

Indeed, this used to be ppc4xx_mal_init(env, 4, 16, &uic[2][3]); before 
706e944206d7 and this typo slipped thorugh unnoticed, likely because the 
MAL is only there for the firmware to be happy. I think it would be used 
by the EMAC Ethernet port or maybe SATA which are not emulated so probably 
nothing really uses the MAL.

Acked-by: BALATON Zoltan <balaton@eik.bme.hu>

>
> mal_irqs is used in ppc4xx_mal_init() to assign the IRQs to MAL:
>
>    for (i = 0; i < 4; i++) {
>        mal->irqs[i] = irqs[i];
>    }
>
> Since only irqs[0] has been initialized, mal->irqs[1,2,3] are being
> zeroed.
>
> This doesn´t seem to trigger any apparent issues at this moment, but
> Cedric's QOMification of the MAL device [1] is executing a
> sysbus_connect_irq() that will fail if we do not store all GPIO lines
> properly.
>
> [1] https://lists.gnu.org/archive/html/qemu-devel/2022-08/msg00497.html
>
> Cc: Peter Maydell <peter.maydell@linaro.org>
> Cc: BALATON Zoltan <balaton@eik.bme.hu>
> Fixes: 706e944206d7 ("hw/ppc/sam460ex: Drop use of ppcuic_init()")
> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
> ---
> hw/ppc/sam460ex.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/ppc/sam460ex.c b/hw/ppc/sam460ex.c
> index 7e8da657c2..0357ee077f 100644
> --- a/hw/ppc/sam460ex.c
> +++ b/hw/ppc/sam460ex.c
> @@ -384,7 +384,7 @@ static void sam460ex_init(MachineState *machine)
>
>     /* MAL */
>     for (i = 0; i < ARRAY_SIZE(mal_irqs); i++) {
> -        mal_irqs[0] = qdev_get_gpio_in(uic[2], 3 + i);
> +        mal_irqs[i] = qdev_get_gpio_in(uic[2], 3 + i);
>     }
>     ppc4xx_mal_init(env, 4, 16, mal_irqs);
>
>