Re: [Qemu-arm] [PATCH 03/17] target/arm: Add MTE system registers

[Richard Henderson]

On 2/5/19 11:27 AM, Peter Maydell wrote:
>> +#ifdef TARGET_AARCH64
>> +        uint64_t tfsr_el[4]; /* tfsrel0_el1 is index 0.  */
>> +        uint64_t gcr_el1;
>> +        uint64_t rgsr_el1;
>> +#endif
> 
> Are we going to add more fields inside this #ifdef or is it only
> saving 12 words?

Just the 12 words here.  We've got plenty of other ifdefs though...

>> +/* Set bits within PSTATE.  */
>> +static inline void set_pstate_bits(uint32_t bits)
>> +{
>> +    TCGv_i32 p = tcg_temp_new_i32();
>> +
>> +    tcg_gen_ld_i32(p, cpu_env, offsetof(CPUARMState, pstate));
>> +    tcg_gen_ori_i32(p, p, bits);
>> +    tcg_gen_st_i32(p, cpu_env, offsetof(CPUARMState, pstate));
>> +    tcg_temp_free_i32(p);
> 
> Maybe assert() that all the bits in the input are in the
> set that we actually store in env->pstate, to catch attempts
> to set NZCV, nRW, etc this way ?

I suppose.  There's the clear_pstate_bits just below, which has a couple of 
users.

>> +      .type = ARM_CP_NO_RAW,
>> +      .access = PL0_RW, .readfn = tco_read, .writefn = tco_write },
>> +    REGINFO_SENTINEL
> 
> Missing GMID_EL1 ?

Err..  that's not in 00eac5, at least.

>> +    case 0x1c: /* TCO */
>> +        if (!dc_isar_feature(aa64_mte_insn_reg, s)) {
>> +            goto do_unallocated;
>> +        }
>> +        if (crm & 1) {
>> +            set_pstate_bits(PSTATE_TCO);
>> +        } else {
>> +            clear_pstate_bits(PSTATE_TCO);
>> +        }
>> +        break;
> 
> Don't we need to break the TB here or something to pick up
> the new value of TCO when we generate code for a following
> load or store?

Yep.  It's included in the (quite complex) MTE_ACTIVE.


r~