qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PULL 08/30] target/arm: Add ptw_idx to S1Translate


From: Philippe Mathieu-Daudé
Subject: Re: [PULL 08/30] target/arm: Add ptw_idx to S1Translate
Date: Tue, 1 Nov 2022 11:10:29 +0100
User-agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0) Gecko/20100101 Thunderbird/102.4.0

On 1/11/22 00:14, Philippe Mathieu-Daudé wrote:
On 25/10/22 18:39, Peter Maydell wrote:
From: Richard Henderson <richard.henderson@linaro.org>

Hoist the computation of the mmu_idx for the ptw up to
get_phys_addr_with_struct and get_phys_addr_twostage.
This removes the duplicate check for stage2 disabled
from the middle of the walk, performing it only once.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Tested-by: Alex Bennée <alex.bennee@linaro.org>
Message-id: 20221024051851.3074715-3-richard.henderson@linaro.org
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
  target/arm/ptw.c | 71 ++++++++++++++++++++++++++++++++++++------------
  1 file changed, 54 insertions(+), 17 deletions(-)

diff --git a/target/arm/ptw.c b/target/arm/ptw.c
index 32d64125865..3c153f68318 100644
--- a/target/arm/ptw.c
+++ b/target/arm/ptw.c
@@ -17,6 +17,7 @@
  typedef struct S1Translate {
      ARMMMUIdx in_mmu_idx;
+    ARMMMUIdx in_ptw_idx;
      bool in_secure;
      bool in_debug;
      bool out_secure;
@@ -214,33 +215,24 @@ static bool S1_ptw_translate(CPUARMState *env, S1Translate *ptw,
  {
      bool is_secure = ptw->in_secure;
      ARMMMUIdx mmu_idx = ptw->in_mmu_idx;
-    ARMMMUIdx s2_mmu_idx = is_secure ? ARMMMUIdx_Stage2_S : ARMMMUIdx_Stage2;
-    bool s2_phys = false;
+    ARMMMUIdx s2_mmu_idx = ptw->in_ptw_idx;
      uint8_t pte_attrs;
      bool pte_secure;
-    if (!arm_mmu_idx_is_stage1_of_2(mmu_idx)
-        || regime_translation_disabled(env, s2_mmu_idx, is_secure)) {
-        s2_mmu_idx = is_secure ? ARMMMUIdx_Phys_S : ARMMMUIdx_Phys_NS;
-        s2_phys = true;
-    }
-
      if (unlikely(ptw->in_debug)) {
          /*
           * From gdbstub, do not use softmmu so that we don't modify the
           * state of the cpu at all, including softmmu tlb contents.
           */
-        if (s2_phys) {
-            ptw->out_phys = addr;
-            pte_attrs = 0;
-            pte_secure = is_secure;
-        } else {
+        if (regime_is_stage2(s2_mmu_idx)) {
              S1Translate s2ptw = {
                  .in_mmu_idx = s2_mmu_idx,
+                .in_ptw_idx = is_secure ? ARMMMUIdx_Phys_S : ARMMMUIdx_Phys_NS,
                  .in_secure = is_secure,
                  .in_debug = true,
              };
              GetPhysAddrResult s2 = { };
+
              if (!get_phys_addr_lpae(env, &s2ptw, addr, MMU_DATA_LOAD,
                                      false, &s2, fi)) {
                  goto fail;
@@ -248,6 +240,11 @@ static bool S1_ptw_translate(CPUARMState *env, S1Translate *ptw,
              ptw->out_phys = s2.f.phys_addr;
              pte_attrs = s2.cacheattrs.attrs;
              pte_secure = s2.f.attrs.secure;
+        } else {
+            /* Regime is physical. */
+            ptw->out_phys = addr;
+            pte_attrs = 0;
+            pte_secure = is_secure;
          }
          ptw->out_host = NULL;
      } else {
@@ -268,7 +265,7 @@ static bool S1_ptw_translate(CPUARMState *env, S1Translate *ptw,
          pte_secure = full->attrs.secure;
      }
-    if (!s2_phys) {
+    if (regime_is_stage2(s2_mmu_idx)) {
          uint64_t hcr = arm_hcr_el2_eff_secstate(env, is_secure);
          if ((hcr & HCR_PTW) && S2_attrs_are_device(hcr, pte_attrs)) {
@@ -1263,7 +1260,18 @@ static bool get_phys_addr_lpae(CPUARMState *env, S1Translate *ptw,
          descaddr |= (address >> (stride * (4 - level))) & indexmask;
          descaddr &= ~7ULL;
          nstable = extract32(tableattrs, 4, 1);
-        ptw->in_secure = !nstable;
+        if (!nstable) {
+            /*
+             * Stage2_S -> Stage2 or Phys_S -> Phys_NS
+             * Assert that the non-secure idx are even, and relative order.
+             */
+            QEMU_BUILD_BUG_ON((ARMMMUIdx_Phys_NS & 1) != 0);
+            QEMU_BUILD_BUG_ON((ARMMMUIdx_Stage2 & 1) != 0);
+            QEMU_BUILD_BUG_ON(ARMMMUIdx_Phys_NS + 1 != ARMMMUIdx_Phys_S); +            QEMU_BUILD_BUG_ON(ARMMMUIdx_Stage2 + 1 != ARMMMUIdx_Stage2_S);
+            ptw->in_ptw_idx &= ~1;
+            ptw->in_secure = false;
+        }
          descriptor = arm_ldq_ptw(env, ptw, descaddr, fi);
          if (fi->type != ARMFault_None) {
              goto do_fault;
@@ -2449,6 +2457,7 @@ static bool get_phys_addr_twostage(CPUARMState *env, S1Translate *ptw,
      is_el0 = ptw->in_mmu_idx == ARMMMUIdx_Stage1_E0;
      ptw->in_mmu_idx = s2walk_secure ? ARMMMUIdx_Stage2_S : ARMMMUIdx_Stage2; +    ptw->in_ptw_idx = s2walk_secure ? ARMMMUIdx_Phys_S : ARMMMUIdx_Phys_NS;
      ptw->in_secure = s2walk_secure;
      /*
@@ -2508,10 +2517,32 @@ static bool get_phys_addr_with_struct(CPUARMState *env, S1Translate *ptw,
                                        ARMMMUFaultInfo *fi)
  {
      ARMMMUIdx mmu_idx = ptw->in_mmu_idx;
-    ARMMMUIdx s1_mmu_idx = stage_1_mmu_idx(mmu_idx);
      bool is_secure = ptw->in_secure;
+    ARMMMUIdx s1_mmu_idx;
-    if (mmu_idx != s1_mmu_idx) {
+    switch (mmu_idx) {
+    case ARMMMUIdx_Phys_S:
+    case ARMMMUIdx_Phys_NS:
+        /* Checking Phys early avoids special casing later vs regime_el. */ +        return get_phys_addr_disabled(env, address, access_type, mmu_idx,
+                                      is_secure, result, fi);
+
+    case ARMMMUIdx_Stage1_E0:
+    case ARMMMUIdx_Stage1_E1:
+    case ARMMMUIdx_Stage1_E1_PAN:
+        /* First stage lookup uses second stage for ptw. */
+        ptw->in_ptw_idx = is_secure ? ARMMMUIdx_Stage2_S : ARMMMUIdx_Stage2;
+        break;
+
+    case ARMMMUIdx_E10_0:
+        s1_mmu_idx = ARMMMUIdx_Stage1_E0;
+        goto do_twostage;
+    case ARMMMUIdx_E10_1:
+        s1_mmu_idx = ARMMMUIdx_Stage1_E1;
+        goto do_twostage;
+    case ARMMMUIdx_E10_1_PAN:
+        s1_mmu_idx = ARMMMUIdx_Stage1_E1_PAN;
+    do_twostage:
          /*
           * Call ourselves recursively to do the stage 1 and then stage 2            * translations if mmu_idx is a two-stage regime, and EL2 present. @@ -2522,6 +2553,12 @@ static bool get_phys_addr_with_struct(CPUARMState *env, S1Translate *ptw,               return get_phys_addr_twostage(env, ptw, address, access_type,
                                            result, fi);
          }
+        /* fall through */
+
+    default:
+        /* Single stage and second stage uses physical for ptw. */
+        ptw->in_ptw_idx = is_secure ? ARMMMUIdx_Phys_S : ARMMMUIdx_Phys_NS;
+        break;
      }

Since this commit I can not boot Trusted Firmware on the SBSA-ref machine.

Do we need to set in_ptw_idx in get_phys_addr_with_secure()?



reply via email to

[Prev in Thread] Current Thread [Next in Thread]