Re: [PATCH 2/4] target/arm: Update MSR access to UAO

[Richard Henderson <richard.henderson@linaro.org>]

On 12/6/19 10:30 AM, Peter Maydell wrote:
>> +    if (cpu_isar_feature(aa64_uao, cpu)) {
>> +        static const ARMCPRegInfo uao_reginfo[] = {
>> +            { .name = "UAO", .state = ARM_CP_STATE_AA64,
>> +              .opc0 = 3, .opc1 = 0, .crn = 4, .crm = 2, .opc2 = 4,
>> +              .type = ARM_CP_NO_RAW, .access = PL1_RW,
>> +              .readfn = aa64_uao_read, .writefn = aa64_uao_write, },
>> +            REGINFO_SENTINEL
>> +        };
> 
> This could just be a file-scope global, right?
> Also, you can just use define_one_arm_cp_reg() rather
> than having a list with one entry. (cf zcr_el1_reginfo).

Can do.

>> +    case 0x03: /* UAO */
>> +        if (!dc_isar_feature(aa64_uao, s) || s->current_el == 0) {
>> +            goto do_unallocated;
>> +        }
>> +        if (crm & 1) {
>> +            set_pstate_bits(PSTATE_UAO);
>> +        } else {
>> +            clear_pstate_bits(PSTATE_UAO);
>> +        }
>> +        t1 = tcg_const_i32(s->current_el);
>> +        gen_helper_rebuild_hflags_a64(cpu_env, t1);
>> +        tcg_temp_free_i32(t1);
>> +        break;
> 
> Do we also need to end the TB since we've messed with
> the hflags, or is some bit of code not in the patch
> context handling that?

Outside the patch context, at the start of the function, we default to ending
the TB.

> Does the "on exception entry PSTATE.UAO is zeroed" behaviour
> fall out automatically for us? How about "on exception entry
> from aarch32 to aarch64 SPSR_ELx.UAO is set to zero" ?

Yes to both -- new_mode (perhaps better renamed as new_pstate) is initialized
as 0 + stuff required to be non-zero.  Thus PAN required special handling but
UAO does not.

> I think we may also want a minor code change so that an exception
> return from aarch64 to aarch32 doesn't copy a bogus SPSR UAO==1
> into the pstate/cpsr.

Ah, good catch.


r~