On Thu, Jun 18, 2020 at 2:07 PM Jessica Clarke <[email protected]> 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 <[email protected]>
Looks good to me! Applied to the RISC-V tree Reviewed-by: Alistair Francis <[email protected]> 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 > >
