| Message ID | 20230606104609.3692557-2-peter.maydell@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | allwinner-a10: Fix interrupt controller regression | expand |
On Tue, Jun 06, 2023 at 11:46:08AM +0100, Peter Maydell wrote: > In commit 2c5fa0778c3b430 we fixed an endianness bug in the Allwinner > A10 PIC model; however in the process we introduced a regression. > This is because the old code was robust against the incoming 'level' > argument being something other than 0 or 1, whereas the new code was > not. > > In particular, the allwinner-sdhost code treats its IRQ line > as 0-vs-non-0 rather than 0-vs-1, so when the SD controller > set its IRQ line for any reason other than transmit the > interrupt controller would ignore it. The observed effect > was a guest timeout when rebooting the guest kernel. > > Handle level values other than 0 or 1, to restore the old > behaviour. > > Fixes: 2c5fa0778c3b430 ("hw/intc/allwinner-a10-pic: Don't use set_bit()/clear_bit()") > Cc: qemu-stable@nongnu.org > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Tested-by: Guenter Roeck <linux@roeck-us.net> Tested on top of 8.0.2, both this patch alone as well as this patch plus the second patch in the series. Guenter > --- > hw/intc/allwinner-a10-pic.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/hw/intc/allwinner-a10-pic.c b/hw/intc/allwinner-a10-pic.c > index 4875e68ba6a..d0bf8d545ba 100644 > --- a/hw/intc/allwinner-a10-pic.c > +++ b/hw/intc/allwinner-a10-pic.c > @@ -51,7 +51,7 @@ static void aw_a10_pic_set_irq(void *opaque, int irq, int level) > AwA10PICState *s = opaque; > uint32_t *pending_reg = &s->irq_pending[irq / 32]; > > - *pending_reg = deposit32(*pending_reg, irq % 32, 1, level); > + *pending_reg = deposit32(*pending_reg, irq % 32, 1, !!level); > aw_a10_pic_update(s); > } > > -- > 2.34.1 >
diff --git a/hw/intc/allwinner-a10-pic.c b/hw/intc/allwinner-a10-pic.c index 4875e68ba6a..d0bf8d545ba 100644 --- a/hw/intc/allwinner-a10-pic.c +++ b/hw/intc/allwinner-a10-pic.c @@ -51,7 +51,7 @@ static void aw_a10_pic_set_irq(void *opaque, int irq, int level) AwA10PICState *s = opaque; uint32_t *pending_reg = &s->irq_pending[irq / 32]; - *pending_reg = deposit32(*pending_reg, irq % 32, 1, level); + *pending_reg = deposit32(*pending_reg, irq % 32, 1, !!level); aw_a10_pic_update(s); }
In commit 2c5fa0778c3b430 we fixed an endianness bug in the Allwinner A10 PIC model; however in the process we introduced a regression. This is because the old code was robust against the incoming 'level' argument being something other than 0 or 1, whereas the new code was not. In particular, the allwinner-sdhost code treats its IRQ line as 0-vs-non-0 rather than 0-vs-1, so when the SD controller set its IRQ line for any reason other than transmit the interrupt controller would ignore it. The observed effect was a guest timeout when rebooting the guest kernel. Handle level values other than 0 or 1, to restore the old behaviour. Fixes: 2c5fa0778c3b430 ("hw/intc/allwinner-a10-pic: Don't use set_bit()/clear_bit()") Cc: qemu-stable@nongnu.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/intc/allwinner-a10-pic.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)