Message ID | 20220201193207.2771604-12-peter.maydell@linaro.org |
---|---|
State | Superseded |
Headers | show |
Series | hw/intc/arm_gicv3_its: more cleanups, bugfixes | expand |
On 2/2/22 06:32, Peter Maydell wrote: > In the MAPC command, if V=0 this is a request to delete a collection > table entry and the rdbase field of the command packet will not be > used. In particular, the specification says that the "UNPREDICTABLE > if rdbase is not valid" only applies for V=1. > > We were doing a check-and-log-guest-error on rdbase regardless of > whether the V bit was set, and also (harmlessly but confusingly) > storing the contents of the rdbase field into the updated collection > table entry. Update the code so that if V=0 we don't check or use > the rdbase field value. > > Signed-off-by: Peter Maydell<peter.maydell@linaro.org> > --- > hw/intc/arm_gicv3_its.c | 24 ++++++++++++------------ > 1 file changed, 12 insertions(+), 12 deletions(-) Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~
diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c index 9735d609df2..069991f7f36 100644 --- a/hw/intc/arm_gicv3_its.c +++ b/hw/intc/arm_gicv3_its.c @@ -468,21 +468,21 @@ static ItsCmdResult process_mapc(GICv3ITSState *s, const uint64_t *cmdpkt) CTEntry cte; icid = cmdpkt[2] & ICID_MASK; - - cte.rdbase = (cmdpkt[2] & R_MAPC_RDBASE_MASK) >> R_MAPC_RDBASE_SHIFT; - cte.rdbase &= RDBASE_PROCNUM_MASK; - cte.valid = cmdpkt[2] & CMD_FIELD_VALID_MASK; + if (cte.valid) { + cte.rdbase = (cmdpkt[2] & R_MAPC_RDBASE_MASK) >> R_MAPC_RDBASE_SHIFT; + cte.rdbase &= RDBASE_PROCNUM_MASK; + } else { + cte.rdbase = 0; + } - if ((icid >= s->ct.num_entries) || (cte.rdbase >= s->gicv3->num_cpu)) { + if (icid >= s->ct.num_entries) { + qemu_log_mask(LOG_GUEST_ERROR, "ITS MAPC: invalid ICID 0x%d", icid); + return CMD_CONTINUE; + } + if (cte.valid && cte.rdbase >= s->gicv3->num_cpu) { qemu_log_mask(LOG_GUEST_ERROR, - "ITS MAPC: invalid collection table attributes " - "icid %d rdbase %u\n", icid, cte.rdbase); - /* - * in this implementation, in case of error - * we ignore this command and move onto the next - * command in the queue - */ + "ITS MAPC: invalid RDBASE %u ", cte.rdbase); return CMD_CONTINUE; }
In the MAPC command, if V=0 this is a request to delete a collection table entry and the rdbase field of the command packet will not be used. In particular, the specification says that the "UNPREDICTABLE if rdbase is not valid" only applies for V=1. We were doing a check-and-log-guest-error on rdbase regardless of whether the V bit was set, and also (harmlessly but confusingly) storing the contents of the rdbase field into the updated collection table entry. Update the code so that if V=0 we don't check or use the rdbase field value. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/intc/arm_gicv3_its.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-)