[v4] target/arm: Implement ARMv8.1-VHE

[PATCH v4 29/40] target/arm: Flush tlb for ASID changes in EL2&0 translation regime

Posted by Richard Henderson 5 years, 11 months ago

Since we only support a single ASID, flush the tlb when it changes.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/arm/helper.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/target/arm/helper.c b/target/arm/helper.c
index 9df55a8d6b..2a4d4c2c0d 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -3740,6 +3740,15 @@ static void vmsa_ttbr_write(CPUARMState *env, const ARMCPRegInfo *ri,
 static void vmsa_tcr_ttbr_el2_write(CPUARMState *env, const ARMCPRegInfo *ri,
                                     uint64_t value)
 {
+    /*
+     * If we are running with E2&0 regime, then the ASID is active.
+     * Flush if that changes.
+     */
+    if ((arm_hcr_el2_eff(env) & HCR_E2H) &&
+        extract64(raw_read(env, ri) ^ value, 48, 16)) {
+        tlb_flush_by_mmuidx(env_cpu(env),
+                            ARMMMUIdxBit_EL20_2 | ARMMMUIdxBit_EL20_0);
+    }
     raw_write(env, ri, value);
 }
 
-- 
2.17.1

Re: [PATCH v4 29/40] target/arm: Flush tlb for ASID changes in EL2&0 translation regime

Posted by Peter Maydell 5 years, 11 months ago

On Tue, 3 Dec 2019 at 02:30, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Since we only support a single ASID, flush the tlb when it changes.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/arm/helper.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
>
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index 9df55a8d6b..2a4d4c2c0d 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -3740,6 +3740,15 @@ static void vmsa_ttbr_write(CPUARMState *env, const ARMCPRegInfo *ri,
>  static void vmsa_tcr_ttbr_el2_write(CPUARMState *env, const ARMCPRegInfo *ri,
>                                      uint64_t value)
>  {
> +    /*
> +     * If we are running with E2&0 regime, then the ASID is active.
> +     * Flush if that changes.
> +     */
> +    if ((arm_hcr_el2_eff(env) & HCR_E2H) &&
> +        extract64(raw_read(env, ri) ^ value, 48, 16)) {
> +        tlb_flush_by_mmuidx(env_cpu(env),
> +                            ARMMMUIdxBit_EL20_2 | ARMMMUIdxBit_EL20_0);
> +    }
>      raw_write(env, ri, value);
>  }

For the existing EL1 setup we have separate write functions
for TTBR registers and for TCR_EL1 (vmsa_tcr_el1_write()
and vmsa_ttbr_write()), rather than a single one, and they
don't do the same thing. Why do we use a single writefn
here? It looks particularly odd because we're actually looking
at the value written here.

thanks
-- PMM

Re: [PATCH v4 29/40] target/arm: Flush tlb for ASID changes in EL2&0 translation regime

Posted by Richard Henderson 5 years, 9 months ago

On 12/6/19 9:05 AM, Peter Maydell wrote:
> On Tue, 3 Dec 2019 at 02:30, Richard Henderson
> <richard.henderson@linaro.org> wrote:
>>
>> Since we only support a single ASID, flush the tlb when it changes.
>>
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>>  target/arm/helper.c | 9 +++++++++
>>  1 file changed, 9 insertions(+)
>>
>> diff --git a/target/arm/helper.c b/target/arm/helper.c
>> index 9df55a8d6b..2a4d4c2c0d 100644
>> --- a/target/arm/helper.c
>> +++ b/target/arm/helper.c
>> @@ -3740,6 +3740,15 @@ static void vmsa_ttbr_write(CPUARMState *env, const ARMCPRegInfo *ri,
>>  static void vmsa_tcr_ttbr_el2_write(CPUARMState *env, const ARMCPRegInfo *ri,
>>                                      uint64_t value)
>>  {
>> +    /*
>> +     * If we are running with E2&0 regime, then the ASID is active.
>> +     * Flush if that changes.
>> +     */
>> +    if ((arm_hcr_el2_eff(env) & HCR_E2H) &&
>> +        extract64(raw_read(env, ri) ^ value, 48, 16)) {
>> +        tlb_flush_by_mmuidx(env_cpu(env),
>> +                            ARMMMUIdxBit_EL20_2 | ARMMMUIdxBit_EL20_0);
>> +    }
>>      raw_write(env, ri, value);
>>  }
> 
> For the existing EL1 setup we have separate write functions
> for TTBR registers and for TCR_EL1 (vmsa_tcr_el1_write()
> and vmsa_ttbr_write()), rather than a single one, and they
> don't do the same thing. Why do we use a single writefn
> here? It looks particularly odd because we're actually looking
> at the value written here.

Yes, Laurent noticed the same problem wrt patch 4.
Fixed.


r~