Re: [PATCH 29/41] hw/intc/arm_gicv3_redist: Recalculate hppvlpi on VPENDBASER writes

[Richard Henderson]

On 4/8/22 07:15, Peter Maydell wrote:
> +    if (oldvalid && newvalid) {
> +        /*
> +         * Changing other fields while VALID is 1 is UNPREDICTABLE;
> +         * we choose to log and ignore the write.
> +         */
> +        if (cs->gicr_vpendbaser ^ newval) {
> +            qemu_log_mask(LOG_GUEST_ERROR,
> +                          "%s: Changing GICR_VPENDBASER when VALID=1 "
> +                          "is UNPREDICTABLE\n", __func__);
> +        }
> +        return;
> +    }

...

> @@ -493,10 +574,10 @@ static MemTxResult gicr_writel(GICv3CPUState *cs, hwaddr 
> offset,
>           cs->gicr_vpropbaser = deposit64(cs->gicr_vpropbaser, 32, 32, value);
>           return MEMTX_OK;
>       case GICR_VPENDBASER:
> -        cs->gicr_vpendbaser = deposit64(cs->gicr_vpendbaser, 0, 32, value);
> +        gicr_write_vpendbaser(cs, deposit64(cs->gicr_vpendbaser, 0, 32, 
> value));
>           return MEMTX_OK;
>       case GICR_VPENDBASER + 4:
> -        cs->gicr_vpendbaser = deposit64(cs->gicr_vpendbaser, 32, 32, value);
> +        gicr_write_vpendbaser(cs, deposit64(cs->gicr_vpendbaser, 32, 32, 
> value));
>           return MEMTX_OK;
>       default:
>           return MEMTX_ERROR;
> @@ -557,7 +638,7 @@ static MemTxResult gicr_writell(GICv3CPUState *cs, hwaddr 
> offset,
>           cs->gicr_vpropbaser = value;
>           return MEMTX_OK;
>       case GICR_VPENDBASER:
> -        cs->gicr_vpendbaser = value;
> +        gicr_write_vpendbaser(cs, value);
>           return MEMTX_OK;
>       default:
>           return MEMTX_ERROR;

It it really valid to write to vpendbaser with other than a 64-bit write?  I suppose it's 
possible to order the 32-bit writes to make sure the update to valid comes last...

Anyway, not really related to the real logic here,
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~