Re: [PATCH] riscv: plic: Add a couple of mising sifive_plic_update calls

[Alistair Francis]

On Thu, Jun 18, 2020 at 2:07 PM Jessica Clarke <jrtc27@jrtc27.com> wrote:
>
> Claiming an interrupt and changing the source priority both potentially
> affect whether an interrupt is pending, thus we must re-compute xEIP.
> Note that we don't put the sifive_plic_update inside sifive_plic_claim
> so that the logging of a claim (and the resulting IRQ) happens before
> the state update, making the causal effect clear, and that we drop the
> explicit call to sifive_plic_print_state when claiming since
> sifive_plic_update already does that automatically at the end for us.
>
> This can result in both spurious interrupt storms if you fail to
> complete an IRQ before enabling interrupts (and no other actions occur
> that result in a call to sifive_plic_update), but also more importantly
> lost interrupts if a disabled interrupt is pending and then becomes
> enabled.
>
> Signed-off-by: Jessica Clarke <jrtc27@jrtc27.com>

Looks good to me!

Applied to the RISC-V tree

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  hw/riscv/sifive_plic.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/hw/riscv/sifive_plic.c b/hw/riscv/sifive_plic.c
> index d91e82b8ab..c20c192034 100644
> --- a/hw/riscv/sifive_plic.c
> +++ b/hw/riscv/sifive_plic.c
> @@ -255,8 +255,8 @@ static uint64_t sifive_plic_read(void *opaque, hwaddr 
> addr, unsigned size)
>                      plic->addr_config[addrid].hartid,
>                      mode_to_char(plic->addr_config[addrid].mode),
>                      value);
> -                sifive_plic_print_state(plic);
>              }
> +            sifive_plic_update(plic);
>              return value;
>          }
>      }
> @@ -287,6 +287,7 @@ static void sifive_plic_write(void *opaque, hwaddr addr, 
> uint64_t value,
>              qemu_log("plic: write priority: irq=%d priority=%d\n",
>                  irq, plic->source_priority[irq]);
>          }
> +        sifive_plic_update(plic);
>          return;
>      } else if (addr >= plic->pending_base && /* 1 bit per source */
>                 addr < plic->pending_base + (plic->num_sources >> 3))
> --
> 2.20.1
>
>