diff mbox series

[v2,02/13] hw/intc/arm_gicv3_its: Convert int ID check to num_intids convention

Message ID 20220111171048.3545974-3-peter.maydell@linaro.org
State Superseded
Headers show
Series arm gicv3 ITS: Various bug fixes and refactorings | expand

Commit Message

Peter Maydell Jan. 11, 2022, 5:10 p.m. UTC 
The bounds check on the number of interrupt IDs is correct, but
doesn't match our convention; change the variable name, initialize it
to the 2^n value rather than (2^n)-1, and use >= instead of > in the
comparison.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
---
 hw/intc/arm_gicv3_its.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Comments

Richard Henderson Jan. 28, 2022, 1:35 a.m. UTC | #1 
On 1/12/22 04:10, Peter Maydell wrote:
> The bounds check on the number of interrupt IDs is correct, but
> doesn't match our convention; change the variable name, initialize it
> to the 2^n value rather than (2^n)-1, and use >= instead of > in the
> comparison.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>   hw/intc/arm_gicv3_its.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c
> index 6d11fa02040..5919b1a3b7f 100644
> --- a/hw/intc/arm_gicv3_its.c
> +++ b/hw/intc/arm_gicv3_its.c
> @@ -338,7 +338,7 @@ static bool process_mapti(GICv3ITSState *s, uint64_t value, uint32_t offset,
>       uint32_t devid, eventid;
>       uint32_t pIntid = 0;
>       uint64_t num_eventids;
> -    uint32_t max_Intid;
> +    uint32_t num_intids;

Does this now need to be uint64_t, like num_eventids?

If not,
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~
Peter Maydell Jan. 28, 2022, 10:51 a.m. UTC | #2 
On Fri, 28 Jan 2022 at 01:35, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 1/12/22 04:10, Peter Maydell wrote:
> > The bounds check on the number of interrupt IDs is correct, but
> > doesn't match our convention; change the variable name, initialize it
> > to the 2^n value rather than (2^n)-1, and use >= instead of > in the
> > comparison.
> >
> > Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> > Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
> > ---
> >   hw/intc/arm_gicv3_its.c | 6 +++---
> >   1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c
> > index 6d11fa02040..5919b1a3b7f 100644
> > --- a/hw/intc/arm_gicv3_its.c
> > +++ b/hw/intc/arm_gicv3_its.c
> > @@ -338,7 +338,7 @@ static bool process_mapti(GICv3ITSState *s, uint64_t value, uint32_t offset,
> >       uint32_t devid, eventid;
> >       uint32_t pIntid = 0;
> >       uint64_t num_eventids;
> > -    uint32_t max_Intid;
> > +    uint32_t num_intids;
>
> Does this now need to be uint64_t, like num_eventids?

No, because GICD_TYPER_IDBITS is 0xf, so num_intids is 2^16
and still fits in a 32-bit integer.

thanks
-- PMM
diff mbox series

Patch

diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c
index 6d11fa02040..5919b1a3b7f 100644
--- a/hw/intc/arm_gicv3_its.c
+++ b/hw/intc/arm_gicv3_its.c
@@ -338,7 +338,7 @@  static bool process_mapti(GICv3ITSState *s, uint64_t value, uint32_t offset,
     uint32_t devid, eventid;
     uint32_t pIntid = 0;
     uint64_t num_eventids;
-    uint32_t max_Intid;
+    uint32_t num_intids;
     bool dte_valid;
     MemTxResult res = MEMTX_OK;
     uint16_t icid = 0;
@@ -379,11 +379,11 @@  static bool process_mapti(GICv3ITSState *s, uint64_t value, uint32_t offset,
     }
     dte_valid = FIELD_EX64(dte, DTE, VALID);
     num_eventids = 1ULL << (FIELD_EX64(dte, DTE, SIZE) + 1);
-    max_Intid = (1ULL << (GICD_TYPER_IDBITS + 1)) - 1;
+    num_intids = 1ULL << (GICD_TYPER_IDBITS + 1);
 
     if ((devid >= s->dt.num_ids) || (icid >= s->ct.num_ids)
             || !dte_valid || (eventid >= num_eventids) ||
-            (((pIntid < GICV3_LPI_INTID_START) || (pIntid > max_Intid)) &&
+            (((pIntid < GICV3_LPI_INTID_START) || (pIntid >= num_intids)) &&
              (pIntid != INTID_SPURIOUS))) {
         qemu_log_mask(LOG_GUEST_ERROR,
                       "%s: invalid command attributes "