Hi Tyler, This fix is incorrect.
In PLIC spec, Interrupt Source Priority Memory Map is 0x000000: Reserved (interrupt source 0 does not exist) 0x000004: Interrupt source 1 priority 0x000008: Interrupt source 2 priority Current RISC-V machines (virt, sifive_u) use 0x4 as priority_base, so current formula "irq = ((addr - plic->priority_base) >> 2) + 1" will take offset 0x4 as IRQ source 1, which is correct. Your fix will cause the bug in existing machines. Thanks, Jim Shu On Tue, Sep 6, 2022 at 11:21 PM Tyler Ng <t...@rivosinc.com> wrote: > > Here's the patch SHA that introduced the offset: > 0feb4a7129eb4f120c75849ddc9e50495c50cb63 > > -Tyler > > On Mon, Sep 5, 2022 at 6:15 AM Andrew Jones <ajo...@ventanamicro.com> wrote: >> >> On Thu, Sep 01, 2022 at 03:50:06PM -0700, Tyler Ng wrote: >> > Fixes a bug in which the index of the interrupt priority is off by 1. >> > For example, using an IRQ number of 3 with a priority of 1 is supposed to >> > set >> > plic->source_priority[2] = 1, but instead it sets >> > plic->source_priority[3] = 1. When an interrupt is claimed to be >> > serviced, it checks the index 2 instead of 3. >> > >> > Signed-off-by: Tyler Ng <t...@rivosinc.com> >> >> Fixes tag? >> >> Thanks, >> drew >> >> > --- >> > hw/intc/sifive_plic.c | 2 +- >> > 1 file changed, 1 insertion(+), 1 deletion(-) >> > >> > diff --git a/hw/intc/sifive_plic.c b/hw/intc/sifive_plic.c >> > index af4ae3630e..e75c47300a 100644 >> > --- a/hw/intc/sifive_plic.c >> > +++ b/hw/intc/sifive_plic.c >> > @@ -178,7 +178,7 @@ static void sifive_plic_write(void *opaque, hwaddr >> > addr, uint64_t value, >> > SiFivePLICState *plic = opaque; >> > >> > if (addr_between(addr, plic->priority_base, plic->num_sources << 2)) { >> > - uint32_t irq = ((addr - plic->priority_base) >> 2) + 1; >> > + uint32_t irq = ((addr - plic->priority_base) >> 2) + 0; >> > >> > plic->source_priority[irq] = value & 7; >> > sifive_plic_update(plic); >> > -- >> > 2.30.2 >> >