Re: [PATCH 26/35] target/arm: Implement FEAT_NV2 redirection of sysregs to RAM

[Richard Henderson]

On 12/18/23 22:32, Peter Maydell wrote:
> +    if (s->nv && s->nv2 && ri->nv2_redirect_offset) {

Again, s->nv test is redundant.

> +        /*
> +         * Some registers always redirect to memory; some only do so if
> +         * HCR_EL2.NV1 is 0, and some only if NV1 is 1 (these come in
> +         * pairs which share an offset; see the table in R_CSRPQ).
> +         */
> +        if (ri->nv2_redirect_offset & NV2_REDIR_NV1) {
> +            nv2_mem_redirect = s->nv1;
> +        } else if (ri->nv2_redirect_offset & NV2_REDIR_NO_NV1) {
> +            nv2_mem_redirect = !s->nv1;
> +        } else {
> +            nv2_mem_redirect = true;
> +        }

I wondered if it would be clearer with the "both" case having both bits set.  While I see 
that the first defined offset is 0x20, offset 0x00 is still reserved and *could* be used. 
At which point ri->nv2_redirect_offset would need a non-zero value for a zero offset.

Maybe clearer as

    nv2_mem_redirect = (ri->nv2_redirect_offset &
                        (s->nv1 ? NV2_REDIR_NV1_1 : NV2_REDIR_NV1_0));

?

This is more verbose for the (common?) case of redirect regardless of nv1, so 
maybe not.

> +        if (s->nv2_mem_be) {
> +            mop |= MO_BE;
> +        }

MO_BSWAP is host dependent -- needs

    mop |= (s->nv2_mem_be ? MO_BE : MO_LE);


r~