Re: [PATCH 09/26] hw/intc/arm_gicv3_its: Correct handling of MAPI

["Alex Bennée" <alex.bennee@linaro.org>]

Peter Maydell <peter.maydell@linaro.org> writes:

> The MAPI command takes arguments DeviceID, EventID, ICID, and is
> defined to be equivalent to MAPTI DeviceID, EventID, EventID, ICID.
> (That is, where MAPTI takes an explicit pINTID, MAPI uses the EventID
> as the pINTID.)
>
> We didn't quite get this right.  In particular the error checks for
> MAPI include "EventID does not specify a valid LPI identifier", which
> is the same as MAPTI's error check for the pINTID field.  QEMU's code
> skips the pINTID error check entirely in the MAPI case.
>
> We can fix this bug and in the process simplify the code by switching
> to the obvious implementation of setting pIntid = eventid early
> if ignore_pInt is true.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  hw/intc/arm_gicv3_its.c | 18 +++++++-----------
>  1 file changed, 7 insertions(+), 11 deletions(-)
>
> diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c
> index 15eb72a0a15..6f21c56fba2 100644
> --- a/hw/intc/arm_gicv3_its.c
> +++ b/hw/intc/arm_gicv3_its.c
> @@ -354,7 +354,9 @@ static bool process_mapti(GICv3ITSState *s, uint64_t value, uint32_t offset,
>  
>      eventid = (value & EVENTID_MASK);
>  
> -    if (!ignore_pInt) {
> +    if (ignore_pInt) {
> +        pIntid = eventid;
> +    } else {
>          pIntid = ((value & pINTID_MASK) >> pINTID_SHIFT);

This would be worth cleaning up with field accessors at some point.

Anyway:

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

-- 
Alex Bennée