Re: [PATCH 15/15] hw/intc: sifive_plic: Fix the pending register range check

[Wilfred Mallawa]

On Mon, 2022-12-05 at 16:21 +0800, Bin Meng wrote:
> On Fri, Dec 2, 2022 at 8:28 AM Wilfred Mallawa
> <wilfred.mallawa@wdc.com> wrote:
> > 
> > On Thu, 2022-12-01 at 22:08 +0800, Bin Meng wrote:
> > > The pending register upper limit is currently set to
> > > plic->num_sources >> 3, which is wrong, e.g.: considering
> > > plic->num_sources is 7, the upper limit becomes 0 which fails
> > > the range check if reading the pending register at pending_base.
> > > 
> > > Fixes: 1e24429e40df ("SiFive RISC-V PLIC Block")
> > > Signed-off-by: Bin Meng <bmeng@tinylab.org>
> > > 
> > > ---
> > > 
> > >  hw/intc/sifive_plic.c | 5 +++--
> > >  1 file changed, 3 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/hw/intc/sifive_plic.c b/hw/intc/sifive_plic.c
> > > index 7a6a358c57..a3fc8222c7 100644
> > > --- a/hw/intc/sifive_plic.c
> > > +++ b/hw/intc/sifive_plic.c
> > > @@ -143,7 +143,8 @@ static uint64_t sifive_plic_read(void
> > > *opaque,
> > > hwaddr addr, unsigned size)
> > >          uint32_t irq = (addr - plic->priority_base) >> 2;
> > > 
> > >          return plic->source_priority[irq];
> > > -    } else if (addr_between(addr, plic->pending_base, plic-
> > > > num_sources >> 3)) {
> > > +    } else if (addr_between(addr, plic->pending_base,
> > > +                            (plic->num_sources + 31) >> 3)) {
> > why does adding specifically 31 work here?
> > 
> 
> Each pending register is 32-bit for 32 interrupt sources. Adding 31
> is
> to round up to next pending register offset.
> 
Ah I see, thanks for that.

Regards,
Wilfred
> Regards,
> Bin