Re: [PATCH 1/2] hw/intc/allwinner-a10-pic: Handle IRQ levels other than 0 or 1

[Guenter Roeck]

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
>