[PATCH 1/2] hw/ssi: ibex_spi_host: Clear the interrupt even if disabled

Alistair Francis posted 2 patches 1 year ago
Maintainers: Alistair Francis <alistair@alistair23.me>, Palmer Dabbelt <palmer@dabbelt.com>, Bin Meng <bin.meng@windriver.com>, Weiwei Li <liweiwei@iscas.ac.cn>, Daniel Henrique Barboza <dbarboza@ventanamicro.com>, Liu Zhiwei <zhiwei_liu@linux.alibaba.com>
[PATCH 1/2] hw/ssi: ibex_spi_host: Clear the interrupt even if disabled
Posted by Alistair Francis 1 year ago
We currently don't clear the interrupts if they are disabled. This means
that if an interrupt occurs and the guest disables interrupts the QEMU
IRQ will remain high.

This doesn't immediately affect guests, but if the
guest re-enables interrupts it's possible that we will miss an
interrupt as it always remains set.

Let's update the logic to always call qemu_set_irq() even if the
interrupts are disabled to ensure we set the level low. The level will
never be high unless interrupts are enabled, so we won't generate
interrupts when we shouldn't.

Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
 hw/ssi/ibex_spi_host.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/hw/ssi/ibex_spi_host.c b/hw/ssi/ibex_spi_host.c
index 1ee7d88c22..c300ec294d 100644
--- a/hw/ssi/ibex_spi_host.c
+++ b/hw/ssi/ibex_spi_host.c
@@ -205,9 +205,10 @@ static void ibex_spi_host_irq(IbexSPIHostState *s)
         if (err_irq) {
             s->regs[IBEX_SPI_HOST_INTR_STATE] |= R_INTR_STATE_ERROR_MASK;
         }
-        qemu_set_irq(s->host_err, err_irq);
     }
 
+    qemu_set_irq(s->host_err, err_irq);
+
     /* Event IRQ Enabled and Event IRQ Cleared */
     if (event_en && !status_pending) {
         if (FIELD_EX32(intr_test_reg, INTR_STATE,  SPI_EVENT)) {
@@ -229,8 +230,9 @@ static void ibex_spi_host_irq(IbexSPIHostState *s)
         if (event_irq) {
             s->regs[IBEX_SPI_HOST_INTR_STATE] |= R_INTR_STATE_SPI_EVENT_MASK;
         }
-        qemu_set_irq(s->event, event_irq);
     }
+
+    qemu_set_irq(s->event, event_irq);
 }
 
 static void ibex_spi_host_transfer(IbexSPIHostState *s)
-- 
2.41.0
Re: [PATCH 1/2] hw/ssi: ibex_spi_host: Clear the interrupt even if disabled
Posted by Daniel Henrique Barboza 1 year ago

On 11/1/23 21:34, Alistair Francis wrote:
> We currently don't clear the interrupts if they are disabled. This means
> that if an interrupt occurs and the guest disables interrupts the QEMU
> IRQ will remain high.
> 
> This doesn't immediately affect guests, but if the
> guest re-enables interrupts it's possible that we will miss an
> interrupt as it always remains set.
> 
> Let's update the logic to always call qemu_set_irq() even if the
> interrupts are disabled to ensure we set the level low. The level will
> never be high unless interrupts are enabled, so we won't generate
> interrupts when we shouldn't.
> 
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> ---

Reviewed-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>

>   hw/ssi/ibex_spi_host.c | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/ssi/ibex_spi_host.c b/hw/ssi/ibex_spi_host.c
> index 1ee7d88c22..c300ec294d 100644
> --- a/hw/ssi/ibex_spi_host.c
> +++ b/hw/ssi/ibex_spi_host.c
> @@ -205,9 +205,10 @@ static void ibex_spi_host_irq(IbexSPIHostState *s)
>           if (err_irq) {
>               s->regs[IBEX_SPI_HOST_INTR_STATE] |= R_INTR_STATE_ERROR_MASK;
>           }
> -        qemu_set_irq(s->host_err, err_irq);
>       }
>   
> +    qemu_set_irq(s->host_err, err_irq);
> +
>       /* Event IRQ Enabled and Event IRQ Cleared */
>       if (event_en && !status_pending) {
>           if (FIELD_EX32(intr_test_reg, INTR_STATE,  SPI_EVENT)) {
> @@ -229,8 +230,9 @@ static void ibex_spi_host_irq(IbexSPIHostState *s)
>           if (event_irq) {
>               s->regs[IBEX_SPI_HOST_INTR_STATE] |= R_INTR_STATE_SPI_EVENT_MASK;
>           }
> -        qemu_set_irq(s->event, event_irq);
>       }
> +
> +    qemu_set_irq(s->event, event_irq);
>   }
>   
>   static void ibex_spi_host_transfer(IbexSPIHostState *s)