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

2022-12-01 Thread Bin Meng 
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 

---

 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)) {
 uint32_t word = (addr - plic->pending_base) >> 2;
 
 return plic->pending[word];
@@ -202,7 +203,7 @@ static void sifive_plic_write(void *opaque, hwaddr addr, 
uint64_t value,
 sifive_plic_update(plic);
 }
 } else if (addr_between(addr, plic->pending_base,
-plic->num_sources >> 3)) {
+(plic->num_sources + 31) >> 3)) {
 qemu_log_mask(LOG_GUEST_ERROR,
   "%s: invalid pending write: 0x%" HWADDR_PRIx "",
   __func__, addr);
-- 
2.34.1

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

2022-12-01 Thread Wilfred Mallawa 
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 
> 
> ---
> 
>  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?

Wilfred,
>  uint32_t word = (addr - plic->pending_base) >> 2;
>  
>  return plic->pending[word];
> @@ -202,7 +203,7 @@ static void sifive_plic_write(void *opaque,
> hwaddr addr, uint64_t value,
>  sifive_plic_update(plic);
>  }
>  } else if (addr_between(addr, plic->pending_base,
> -    plic->num_sources >> 3)) {
> +    (plic->num_sources + 31) >> 3)) {
>  qemu_log_mask(LOG_GUEST_ERROR,
>    "%s: invalid pending write: 0x%" HWADDR_PRIx
> "",
>    __func__, addr);

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

2022-12-05 Thread Bin Meng 
On Fri, Dec 2, 2022 at 8:28 AM Wilfred Mallawa  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 
> >
> > ---
> >
> >  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.

Regards,
Bin

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

2022-12-05 Thread 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
>  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 
> > > 
> > > ---
> > > 
> > >  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

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

2022-12-06 Thread Alistair Francis 
On Fri, Dec 2, 2022 at 12:10 AM 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 

Reviewed-by: Alistair Francis 

Alistair

>
> ---
>
>  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)) {
>  uint32_t word = (addr - plic->pending_base) >> 2;
>
>  return plic->pending[word];
> @@ -202,7 +203,7 @@ static void sifive_plic_write(void *opaque, hwaddr addr, 
> uint64_t value,
>  sifive_plic_update(plic);
>  }
>  } else if (addr_between(addr, plic->pending_base,
> -plic->num_sources >> 3)) {
> +(plic->num_sources + 31) >> 3)) {
>  qemu_log_mask(LOG_GUEST_ERROR,
>"%s: invalid pending write: 0x%" HWADDR_PRIx "",
>__func__, addr);
> --
> 2.34.1
>
>