Message ID | 20220111171048.3545974-3-peter.maydell@linaro.org |
---|---|
State | Superseded |
Headers | show |
Series | arm gicv3 ITS: Various bug fixes and refactorings | expand |
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~
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 --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 "