Re: [Qemu-devel] [PATCH v4 03/13] target-arm: Add support for AArch32 S2 negative t0sz

[Peter Maydell]

On 14 October 2015 at 23:55, Edgar E. Iglesias <address@hidden> wrote:
> From: "Edgar E. Iglesias" <address@hidden>
>
> Add support for AArch32 S2 negative t0sz. In preparation for
> using 40bit IPAs on AArch32.
>
> Signed-off-by: Edgar E. Iglesias <address@hidden>
> ---
>  target-arm/helper.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
>
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index 4e19838..a8a46db 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -6475,6 +6475,17 @@ static bool get_phys_addr_lpae(CPUARMState *env, 
> target_ulong address,
>      if (va_size == 64) {
>          t0sz = MIN(t0sz, 39);
>          t0sz = MAX(t0sz, 16);
> +    } else {
> +        bool sext = extract32(t0sz, 4, 1);
> +        bool sign = extract32(t0sz, 3, 1);
> +        t0sz = sextract32(t0sz, 0, 4);
> +
> +        /* If the sign-extend bit is not the same as t0sz[3], the result
> +         * is unpredictable. Flag this as a guest error.  */
> +        if (sign != sext) {
> +            qemu_log_mask(LOG_GUEST_ERROR,
> +                          "AArch32: VTCR.S / VTCR.T0SZ[3] missmatch\n");
> +        }

Shouldn't this be guarded by a check on whether this is an s2
translation, since the 4-bit signed T0SZ and the S bit are only for
the VTCR, not for the normal TTBCRs ?

That is, we have 3 cases here for determining t0sz:
 * AArch64 6-bit unsigned field
 * AArch32 stage 1 3-bit unsigned field
 * AArch32 stage 2 4-bit signed field
so we need more than just a single if/else.

It's true that bits 3 and 4 are RES0 for TTBCR, but if we're
going to actually start logging guest errors here maybe we
should actually report the real problem (RES0 bits being set)
for that case.

thanks
-- PMM