Re: [PATCH 2/4] target/arm: Allow writes to FNG1, FNG0, A2

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

On 12/9/25 09:04, Jim MacArthur wrote:
> On 12/5/25 15:30, Richard Henderson wrote:
>> On 12/4/25 12:04, Jim MacArthur wrote:
>>> @@ -6121,8 +6131,16 @@ static void tcr2_el2_write(CPUARMState *env, const ARMCPRegInfo 
>>> *ri,
>>>       if (cpu_isar_feature(aa64_mec, cpu)) {
>>>           valid_mask |= TCR2_AMEC0 | TCR2_AMEC1;
>>>       }
>>> +    if (cpu_isar_feature(aa64_asid2, cpu)) {
>>> +        valid_mask |= TCR2_FNG1 | TCR2_FNG0 | TCR2_A2;
>>> +        require_flush = true;
>>> +    }
>>>       value &= valid_mask;
>>>       raw_write(env, ri, value);
>>> +
>>> +    if (require_flush) {
>>> +        tlb_flush(CPU(cpu));
>>> +    }
>>
>> Just because A2 is valid doesn't mean the A2 bit changed.
>>
>> Compare, for instance, vmsa_ttbr_write, where we notice if the ASID has changed before 
>> performing the flush.
>>
>> Note as well that we don't need to flush all tlbs.  In tcr2_el1_write we know that we 
>> are only affecting the EL1&0 regime (alle1_tlbmask). In tcr2_el2_write, we know that we 
>> are only affecting the EL2&0 regime (see the E2H part of vae2_tlbmask).
>>
>>
>> r~
>>
> 
> Before I make a full patch series, can I check this looks correct?
> 
> In tcr2_el1_write:
> 
>      if (cpu_isar_feature(aa64_asid2, cpu)) {
>          uint64_t asid_nonglobal_flags = TCR2_FNG1 | TCR2_FNG0 | TCR2_A2;
>          valid_mask |= asid_nonglobal_flags;
>          require_flush = ((raw_read(env, ri) ^ value) & asid_nonglobal_flags) != 0;
>      }
>      value &= valid_mask;
>      raw_write(env, ri, value);
> 
>      if (require_flush) {
>          tlb_flush_by_mmuidx(CPU(cpu), alle1_tlbmask(env));
>      }
> 
> And then in tcr_el2_write, the same check but flushing by: ARMMMUIdxBit_E20_2 | 
> ARMMMUIdxBit_E20_2_PAN | ARMMMUIdxBit_E20_2_GCS | ARMMMUIdxBit_E20_0 | 
> ARMMMUIdxBit_E20_0_GCS, as used in vmsa_tcr_ttbr_el2_write. This could be factored out 
> into a constant function like alle1_tlbmask.

You don't actually need the require_flush boolean. You could just as well perform the 
flush immediately -- there's nothing about tlb_flush that requires the raw_write to happen 
first.

The FNG[01] bits don't affect ASID selection, so you don't need to flush when they change, 
only when the A2 bit changes.


r~