| Message ID | 20220803233204.2724202-1-danielhb413@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | hw/ppc: sam460ex.c: store all GPIO lines in mal_irqs[] | expand |
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); > >
On 8/4/22 01:32, 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); > > 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> Yes, I found the same issue when fixing ppc4xx_mal_init(). Reviewed-by: Cédric Le Goater <clg@kaod.org> Thanks, C. > --- > 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); >
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);
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); 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(-)