Peter Maydell <[email protected]> writes:
> In process_its_cmd() and process_mapti() we must check the > event ID against a limit defined by the size field in the DTE, > which specifies the number of ID bits minus one. Convert > this code to our num_foo convention: > * change the variable names > * use uint64_t and 1ULL when calculating the number > of valid event IDs, because DTE.SIZE is 5 bits and > so num_eventids may be up to 2^32 > * fix the off-by-one error in the comparison > > Signed-off-by: Peter Maydell <[email protected]> > --- > hw/intc/arm_gicv3_its.c | 18 ++++++++++-------- > 1 file changed, 10 insertions(+), 8 deletions(-) > > diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c > index fa3cdb57554..6d11fa02040 100644 > --- a/hw/intc/arm_gicv3_its.c > +++ b/hw/intc/arm_gicv3_its.c > @@ -225,7 +225,7 @@ static bool process_its_cmd(GICv3ITSState *s, uint64_t > value, uint32_t offset, > MemTxResult res = MEMTX_OK; > bool dte_valid; > uint64_t dte = 0; > - uint32_t max_eventid; > + uint64_t num_eventids; > uint16_t icid = 0; > uint32_t pIntid = 0; > bool ite_valid = false; > @@ -258,7 +258,7 @@ static bool process_its_cmd(GICv3ITSState *s, uint64_t > value, uint32_t offset, > dte_valid = FIELD_EX64(dte, DTE, VALID); > > if (dte_valid) { > - max_eventid = 1UL << (FIELD_EX64(dte, DTE, SIZE) + 1); > + num_eventids = 1ULL << (FIELD_EX64(dte, DTE, SIZE) + 1); > > ite_valid = get_ite(s, eventid, dte, &icid, &pIntid, &res); > > @@ -299,10 +299,11 @@ static bool process_its_cmd(GICv3ITSState *s, uint64_t > value, uint32_t offset, > dte_valid ? "valid" : "invalid", > ite_valid ? "valid" : "invalid", > cte_valid ? "valid" : "invalid"); > - } else if (eventid > max_eventid) { > + } else if (eventid >= num_eventids) { > qemu_log_mask(LOG_GUEST_ERROR, > - "%s: invalid command attributes: eventid %d > %d\n", > - __func__, eventid, max_eventid); > + "%s: invalid command attributes: eventid %d >= %" > + PRId64 "\n", > + __func__, eventid, num_eventids); > } else { > /* > * Current implementation only supports rdbase == procnum > @@ -336,7 +337,8 @@ static bool process_mapti(GICv3ITSState *s, uint64_t > value, uint32_t offset, > AddressSpace *as = &s->gicv3->dma_as; > uint32_t devid, eventid; > uint32_t pIntid = 0; > - uint32_t max_eventid, max_Intid; > + uint64_t num_eventids; > + uint32_t max_Intid; > bool dte_valid; > MemTxResult res = MEMTX_OK; > uint16_t icid = 0; > @@ -376,11 +378,11 @@ static bool process_mapti(GICv3ITSState *s, uint64_t > value, uint32_t offset, > return result; > } > dte_valid = FIELD_EX64(dte, DTE, VALID); > - max_eventid = 1UL << (FIELD_EX64(dte, DTE, SIZE) + 1); > + num_eventids = 1ULL << (FIELD_EX64(dte, DTE, SIZE) + 1); > max_Intid = (1ULL << (GICD_TYPER_IDBITS + 1)) - 1; > > if ((devid >= s->dt.num_ids) || (icid >= s->ct.num_ids) > - || !dte_valid || (eventid > max_eventid) || > + || !dte_valid || (eventid >= num_eventids) || > (((pIntid < GICV3_LPI_INTID_START) || (pIntid > max_Intid)) && > (pIntid != INTID_SPURIOUS))) { It seems process_mapti has the similar "catch all the errors" if leg that I split apart in process_its_command to make it easier to see what failed. However: Reviewed-by: Alex Bennée <[email protected]> -- Alex Bennée
