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

[BALATON Zoltan]

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 <bala...@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.mayd...@linaro.org>
Cc: BALATON Zoltan <bala...@eik.bme.hu>
Fixes: 706e944206d7 ("hw/ppc/sam460ex: Drop use of ppcuic_init()")
Signed-off-by: Daniel Henrique Barboza <danielhb...@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);