diff mbox series

[v2,19/21] target/arm: Move device detection earlier in get_phys_addr_lpae

Message ID 20241005200600.493604-20-richard.henderson@linaro.org
State Superseded
Headers show
Series accel/tcg: Introduce tlb_fill_align hook | expand

Commit Message

Richard Henderson Oct. 5, 2024, 8:05 p.m. UTC 
Determine cache attributes, and thence Device vs Normal memory,
earlier in the function.  We have an existing regime_is_stage2
if block into which this can be slotted.

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

Comments

Helge Deller Oct. 7, 2024, 9:25 p.m. UTC | #1 
On 10/5/24 22:05, Richard Henderson wrote:
> Determine cache attributes, and thence Device vs Normal memory,

"thence" ?

Other than that I have no arm knowledge to review the patch below....

Helge

> earlier in the function.  We have an existing regime_is_stage2
> if block into which this can be slotted.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   target/arm/ptw.c | 49 ++++++++++++++++++++++++------------------------
>   1 file changed, 25 insertions(+), 24 deletions(-)
>
> diff --git a/target/arm/ptw.c b/target/arm/ptw.c
> index 238b2c92a9..0a1a820362 100644
> --- a/target/arm/ptw.c
> +++ b/target/arm/ptw.c
> @@ -2029,8 +2029,20 @@ static bool get_phys_addr_lpae(CPUARMState *env, S1Translate *ptw,
>               xn = extract64(attrs, 53, 2);
>               result->f.prot = get_S2prot(env, ap, xn, ptw->in_s1_is_el0);
>           }
> +
> +        result->cacheattrs.is_s2_format = true;
> +        result->cacheattrs.attrs = extract32(attrs, 2, 4);
> +        /*
> +         * Security state does not really affect HCR_EL2.FWB;
> +         * we only need to filter FWB for aa32 or other FEAT.
> +         */
> +        device = S2_attrs_are_device(arm_hcr_el2_eff(env),
> +                                     result->cacheattrs.attrs);
>       } else {
>           int nse, ns = extract32(attrs, 5, 1);
> +        uint8_t attrindx;
> +        uint64_t mair;
> +
>           switch (out_space) {
>           case ARMSS_Root:
>               /*
> @@ -2102,6 +2114,19 @@ static bool get_phys_addr_lpae(CPUARMState *env, S1Translate *ptw,
>            */
>           result->f.prot = get_S1prot(env, mmu_idx, aarch64, ap, xn, pxn,
>                                       result->f.attrs.space, out_space);
> +
> +        /* Index into MAIR registers for cache attributes */
> +        attrindx = extract32(attrs, 2, 3);
> +        mair = env->cp15.mair_el[regime_el(env, mmu_idx)];
> +        assert(attrindx <= 7);
> +        result->cacheattrs.is_s2_format = false;
> +        result->cacheattrs.attrs = extract64(mair, attrindx * 8, 8);
> +
> +        /* When in aarch64 mode, and BTI is enabled, remember GP in the TLB. */
> +        if (aarch64 && cpu_isar_feature(aa64_bti, cpu)) {
> +            result->f.extra.arm.guarded = extract64(attrs, 50, 1); /* GP */
> +        }
> +        device = S1_attrs_are_device(result->cacheattrs.attrs);
>       }
>
>       if (!(result->f.prot & (1 << access_type))) {
> @@ -2131,30 +2156,6 @@ static bool get_phys_addr_lpae(CPUARMState *env, S1Translate *ptw,
>       result->f.attrs.space = out_space;
>       result->f.attrs.secure = arm_space_is_secure(out_space);
>
> -    if (regime_is_stage2(mmu_idx)) {
> -        result->cacheattrs.is_s2_format = true;
> -        result->cacheattrs.attrs = extract32(attrs, 2, 4);
> -        /*
> -         * Security state does not really affect HCR_EL2.FWB;
> -         * we only need to filter FWB for aa32 or other FEAT.
> -         */
> -        device = S2_attrs_are_device(arm_hcr_el2_eff(env),
> -                                     result->cacheattrs.attrs);
> -    } else {
> -        /* Index into MAIR registers for cache attributes */
> -        uint8_t attrindx = extract32(attrs, 2, 3);
> -        uint64_t mair = env->cp15.mair_el[regime_el(env, mmu_idx)];
> -        assert(attrindx <= 7);
> -        result->cacheattrs.is_s2_format = false;
> -        result->cacheattrs.attrs = extract64(mair, attrindx * 8, 8);
> -
> -        /* When in aarch64 mode, and BTI is enabled, remember GP in the TLB. */
> -        if (aarch64 && cpu_isar_feature(aa64_bti, cpu)) {
> -            result->f.extra.arm.guarded = extract64(attrs, 50, 1); /* GP */
> -        }
> -        device = S1_attrs_are_device(result->cacheattrs.attrs);
> -    }
> -
>       /*
>        * Enable alignment checks on Device memory.
>        *
Peter Maydell Oct. 8, 2024, 2:22 p.m. UTC | #2 
On Sat, 5 Oct 2024 at 21:06, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Determine cache attributes, and thence Device vs Normal memory,
> earlier in the function.  We have an existing regime_is_stage2
> if block into which this can be slotted.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/arm/ptw.c | 49 ++++++++++++++++++++++++------------------------
>  1 file changed, 25 insertions(+), 24 deletions(-)

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM
diff mbox series

Patch

diff --git a/target/arm/ptw.c b/target/arm/ptw.c
index 238b2c92a9..0a1a820362 100644
--- a/target/arm/ptw.c
+++ b/target/arm/ptw.c
@@ -2029,8 +2029,20 @@  static bool get_phys_addr_lpae(CPUARMState *env, S1Translate *ptw,
             xn = extract64(attrs, 53, 2);
             result->f.prot = get_S2prot(env, ap, xn, ptw->in_s1_is_el0);
         }
+
+        result->cacheattrs.is_s2_format = true;
+        result->cacheattrs.attrs = extract32(attrs, 2, 4);
+        /*
+         * Security state does not really affect HCR_EL2.FWB;
+         * we only need to filter FWB for aa32 or other FEAT.
+         */
+        device = S2_attrs_are_device(arm_hcr_el2_eff(env),
+                                     result->cacheattrs.attrs);
     } else {
         int nse, ns = extract32(attrs, 5, 1);
+        uint8_t attrindx;
+        uint64_t mair;
+
         switch (out_space) {
         case ARMSS_Root:
             /*
@@ -2102,6 +2114,19 @@  static bool get_phys_addr_lpae(CPUARMState *env, S1Translate *ptw,
          */
         result->f.prot = get_S1prot(env, mmu_idx, aarch64, ap, xn, pxn,
                                     result->f.attrs.space, out_space);
+
+        /* Index into MAIR registers for cache attributes */
+        attrindx = extract32(attrs, 2, 3);
+        mair = env->cp15.mair_el[regime_el(env, mmu_idx)];
+        assert(attrindx <= 7);
+        result->cacheattrs.is_s2_format = false;
+        result->cacheattrs.attrs = extract64(mair, attrindx * 8, 8);
+
+        /* When in aarch64 mode, and BTI is enabled, remember GP in the TLB. */
+        if (aarch64 && cpu_isar_feature(aa64_bti, cpu)) {
+            result->f.extra.arm.guarded = extract64(attrs, 50, 1); /* GP */
+        }
+        device = S1_attrs_are_device(result->cacheattrs.attrs);
     }
 
     if (!(result->f.prot & (1 << access_type))) {
@@ -2131,30 +2156,6 @@  static bool get_phys_addr_lpae(CPUARMState *env, S1Translate *ptw,
     result->f.attrs.space = out_space;
     result->f.attrs.secure = arm_space_is_secure(out_space);
 
-    if (regime_is_stage2(mmu_idx)) {
-        result->cacheattrs.is_s2_format = true;
-        result->cacheattrs.attrs = extract32(attrs, 2, 4);
-        /*
-         * Security state does not really affect HCR_EL2.FWB;
-         * we only need to filter FWB for aa32 or other FEAT.
-         */
-        device = S2_attrs_are_device(arm_hcr_el2_eff(env),
-                                     result->cacheattrs.attrs);
-    } else {
-        /* Index into MAIR registers for cache attributes */
-        uint8_t attrindx = extract32(attrs, 2, 3);
-        uint64_t mair = env->cp15.mair_el[regime_el(env, mmu_idx)];
-        assert(attrindx <= 7);
-        result->cacheattrs.is_s2_format = false;
-        result->cacheattrs.attrs = extract64(mair, attrindx * 8, 8);
-
-        /* When in aarch64 mode, and BTI is enabled, remember GP in the TLB. */
-        if (aarch64 && cpu_isar_feature(aa64_bti, cpu)) {
-            result->f.extra.arm.guarded = extract64(attrs, 50, 1); /* GP */
-        }
-        device = S1_attrs_are_device(result->cacheattrs.attrs);
-    }
-
     /*
      * Enable alignment checks on Device memory.
      *