Series comparison

-[PULL 0/1] target-arm queue
+[PULL 0/2] target-arm queue
-Just one patch for rc2, a revert.
+This bug seemed worth fixing for 8.0 since we need an rc4 anyway:
 we were using uninitialized data for the guarded bit when
 combining stage 1 and stage 2 attrs.
+thanks
 -- PMM
-The following changes since commit 49aaac3548bc5a4632a14de939d5312b28dc1ba2:
+The following changes since commit 08dede07030973c1053868bc64de7e10bfa02ad6:
-  Merge tag 'linux-user-for-6.2-pull-request' of git://github.com/vivier/qemu into staging (2021-11-22 10:33:13 +0100)
+  Merge tag 'pull-ppc-20230409' of https://github.com/legoater/qemu into staging (2023-04-10 11:47:52 +0100)
 are available in the Git repository at:
-  https://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-target-arm-20211122
+  https://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-target-arm-20230410
-for you to fetch changes up to 4825eaae4fdd56fba0febdfbdd7bf9684ae3ee0d:
+for you to fetch changes up to 8539dc00552e8ea60420856fc1262c8299bc6308:
-  Revert "arm: tcg: Adhere to SMCCC 1.3 section 5.2" (2021-11-22 13:41:48 +0000)
+  target/arm: Copy guarded bit in combine_cacheattrs (2023-04-10 14:31:40 +0100)
 ----------------------------------------------------------------
-target-arm queue:
+target-arm: Fix bug where we weren't initializing
- * revert SMCCC/PSCI change, as it regresses some usecases for some boards
+            guarded bit state when combining S1/S2 attrs
 ----------------------------------------------------------------
-Peter Maydell (1):
+Richard Henderson (2):
-      Revert "arm: tcg: Adhere to SMCCC 1.3 section 5.2"
+      target/arm: PTE bit GP only applies to stage1
       target/arm: Copy guarded bit in combine_cacheattrs
- target/arm/psci.c | 35 +++++++++++++++++++++++++++++------
+ target/arm/ptw.c | 11 ++++++-----
-file changed, 29 insertions(+), 6 deletions(-)
+file changed, 6 insertions(+), 5 deletions(-)

-[PULL 1/1] Revert "arm: tcg: Adhere to SMCCC 1.3 section 5.2"
+[PULL 1/2] target/arm: PTE bit GP only applies to stage1
-This reverts commit 9fcd15b9193e819b6cc2fd0a45e3506148812bb4.
+From: Richard Henderson <richard.henderson@linaro.org>
-This change turns out to cause regressions, for instance on the
+Only perform the extract of GP during the stage1 walk.
 imx6ul boards as described here:
 https://lore.kernel.org/qemu-devel/c8b89685-7490-328b-51a3-48711c140a84@tribudubois.net/
-The primary cause of that regression is that the guest code running
+Reported-by: Peter Maydell <peter.maydell@linaro.org>
-at EL3 expects SMCs (not related to PSCI) to do what they would if
+Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
-our PSCI emulation was not present at all, but after this change
+Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
-they instead set a value in R0/X0 and continue.
+Message-id: 20230407185149.3253946-2-richard.henderson@linaro.org
 Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
 ---
  target/arm/ptw.c | 10 +++++-----
 file changed, 5 insertions(+), 5 deletions(-)
-We could fix that by a refactoring that allowed us to only turn on
+diff --git a/target/arm/ptw.c b/target/arm/ptw.c
 the PSCI emulation if we weren't booting the guest at EL3, but there
 is a more tangled problem with the highbank board, which:
  (1) wants to enable PSCI emulation
  (2) has a bit of guest code that it wants to run at EL3 and
      to perform SMC calls that trap to the monitor vector table:
      this is the boot stub code that is written to memory by
      arm_write_secure_board_setup_dummy_smc() and which the
      highbank board enables by setting bootinfo->secure_board_setup
 We can't satisfy both of those and also have the PSCI emulation
 handle all SMC instruction executions regardless of function
 identifier value.
 This is too tricky to try to sort out before 6.2 is released;
 revert this commit so we can take the time to get it right in
 the 7.0 release.
 Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
 Message-id: 20211119163419.557623-1-peter.maydell@linaro.org
 ---
  target/arm/psci.c | 35 +++++++++++++++++++++++++++++------
 file changed, 29 insertions(+), 6 deletions(-)
 diff --git a/target/arm/psci.c b/target/arm/psci.c
 index XXXXXXX..XXXXXXX 100644
---- a/target/arm/psci.c
+--- a/target/arm/ptw.c
-+++ b/target/arm/psci.c
++++ b/target/arm/ptw.c
-@@ -XXX,XX +XXX,XX @@
+@@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_lpae(CPUARMState *env, S1Translate *ptw,
+         result->f.attrs.secure = false;
  bool arm_is_psci_call(ARMCPU *cpu, int excp_type)
  {
 -    /*
 -     * Return true if the exception type matches the configured PSCI conduit.
 -     * This is called before the SMC/HVC instruction is executed, to decide
 -     * whether we should treat it as a PSCI call or with the architecturally
 +    /* Return true if the r0/x0 value indicates a PSCI call and
 +     * the exception type matches the configured PSCI conduit. This is
 +     * called before the SMC/HVC instruction is executed, to decide whether
 +     * we should treat it as a PSCI call or with the architecturally
       * defined behaviour for an SMC or HVC (which might be UNDEF or trap
       * to EL2 or to EL3).
       */
 +    CPUARMState *env = &cpu->env;
 +    uint64_t param = is_a64(env) ? env->xregs[0] : env->regs[0];
      switch (excp_type) {
      case EXCP_HVC:
@@ -XXX,XX +XXX,XX @@ bool arm_is_psci_call(ARMCPU *cpu, int excp_type)
          return false;
      }
--    return true;
+-    /* When in aarch64 mode, and BTI is enabled, remember GP in the TLB.  */
-+    switch (param) {
+-    if (aarch64 && cpu_isar_feature(aa64_bti, cpu)) {
-+    case QEMU_PSCI_0_2_FN_PSCI_VERSION:
+-        result->f.guarded = extract64(attrs, 50, 1); /* GP */
-+    case QEMU_PSCI_0_2_FN_MIGRATE_INFO_TYPE:
+-    }
-+    case QEMU_PSCI_0_2_FN_AFFINITY_INFO:
+-
-+    case QEMU_PSCI_0_2_FN64_AFFINITY_INFO:
+     if (regime_is_stage2(mmu_idx)) {
-+    case QEMU_PSCI_0_2_FN_SYSTEM_RESET:
+         result->cacheattrs.is_s2_format = true;
-+    case QEMU_PSCI_0_2_FN_SYSTEM_OFF:
+         result->cacheattrs.attrs = extract32(attrs, 2, 4);
-+    case QEMU_PSCI_0_1_FN_CPU_ON:
+@@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_lpae(CPUARMState *env, S1Translate *ptw,
-+    case QEMU_PSCI_0_2_FN_CPU_ON:
+         assert(attrindx <= 7);
-+    case QEMU_PSCI_0_2_FN64_CPU_ON:
+         result->cacheattrs.is_s2_format = false;
-+    case QEMU_PSCI_0_1_FN_CPU_OFF:
+         result->cacheattrs.attrs = extract64(mair, attrindx * 8, 8);
-+    case QEMU_PSCI_0_2_FN_CPU_OFF:
++
-+    case QEMU_PSCI_0_1_FN_CPU_SUSPEND:
++        /* When in aarch64 mode, and BTI is enabled, remember GP in the TLB. */
-+    case QEMU_PSCI_0_2_FN_CPU_SUSPEND:
++        if (aarch64 && cpu_isar_feature(aa64_bti, cpu)) {
-+    case QEMU_PSCI_0_2_FN64_CPU_SUSPEND:
++            result->f.guarded = extract64(attrs, 50, 1); /* GP */
-+    case QEMU_PSCI_0_1_FN_MIGRATE:
++        }
 +    case QEMU_PSCI_0_2_FN_MIGRATE:
 +        return true;
 +    default:
 +        return false;
 +    }
  }
  void arm_handle_psci_call(ARMCPU *cpu)
@@ -XXX,XX +XXX,XX @@ void arm_handle_psci_call(ARMCPU *cpu)
          break;
      case QEMU_PSCI_0_1_FN_MIGRATE:
      case QEMU_PSCI_0_2_FN_MIGRATE:
 -    default:
          ret = QEMU_PSCI_RET_NOT_SUPPORTED;
          break;
 +    default:
 +        g_assert_not_reached();
      }
- err:
+     /*
 --
-.25.1
+.34.1

-New patch
+[PULL 2/2] target/arm: Copy guarded bit in combine_cacheattrs
+From: Richard Henderson <richard.henderson@linaro.org>
+The guarded bit comes from the stage1 walk.
+Fixes: Coverity CID 1507929
+Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
+Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
+Message-id: 20230407185149.3253946-3-richard.henderson@linaro.org
+Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
+---
+ target/arm/ptw.c | 1 +
+file changed, 1 insertion(+)
+diff --git a/target/arm/ptw.c b/target/arm/ptw.c
+index XXXXXXX..XXXXXXX 100644
+--- a/target/arm/ptw.c
++++ b/target/arm/ptw.c
+@@ -XXX,XX +XXX,XX @@ static ARMCacheAttrs combine_cacheattrs(uint64_t hcr,
+     assert(!s1.is_s2_format);
+     ret.is_s2_format = false;
++    ret.guarded = s1.guarded;
+     if (s1.attrs == 0xf0) {
+         tagged = true;
+--
+.34.1

This reverts commit 9fcd15b9193e819b6cc2fd0a45e3506148812bb4.

This change turns out to cause regressions, for instance on the
imx6ul boards as described here:
https://lore.kernel.org/qemu-devel/c8b89685-7490-328b-51a3-48711c140a84@tribudubois.net/

The primary cause of that regression is that the guest code running
at EL3 expects SMCs (not related to PSCI) to do what they would if
our PSCI emulation was not present at all, but after this change
they instead set a value in R0/X0 and continue.

We could fix that by a refactoring that allowed us to only turn on
the PSCI emulation if we weren't booting the guest at EL3, but there
is a more tangled problem with the highbank board, which:
 (1) wants to enable PSCI emulation
 (2) has a bit of guest code that it wants to run at EL3 and
     to perform SMC calls that trap to the monitor vector table:
     this is the boot stub code that is written to memory by
     arm_write_secure_board_setup_dummy_smc() and which the
     highbank board enables by setting bootinfo->secure_board_setup

We can't satisfy both of those and also have the PSCI emulation
handle all SMC instruction executions regardless of function
identifier value.

This is too tricky to try to sort out before 6.2 is released;
revert this commit so we can take the time to get it right in
the 7.0 release.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Message-id: 20211119163419.557623-1-peter.maydell@linaro.org
---
 target/arm/psci.c | 35 +++++++++++++++++++++++++++++------
 1 file changed, 29 insertions(+), 6 deletions(-)

diff --git a/target/arm/psci.c b/target/arm/psci.c
index XXXXXXX..XXXXXXX 100644
--- a/target/arm/psci.c
+++ b/target/arm/psci.c
@@ -XXX,XX +XXX,XX @@
 
 bool arm_is_psci_call(ARMCPU *cpu, int excp_type)
 {
-    /*
-     * Return true if the exception type matches the configured PSCI conduit.
-     * This is called before the SMC/HVC instruction is executed, to decide
-     * whether we should treat it as a PSCI call or with the architecturally
+    /* Return true if the r0/x0 value indicates a PSCI call and
+     * the exception type matches the configured PSCI conduit. This is
+     * called before the SMC/HVC instruction is executed, to decide whether
+     * we should treat it as a PSCI call or with the architecturally
      * defined behaviour for an SMC or HVC (which might be UNDEF or trap
      * to EL2 or to EL3).
      */
+    CPUARMState *env = &cpu->env;
+    uint64_t param = is_a64(env) ? env->xregs[0] : env->regs[0];
 
     switch (excp_type) {
     case EXCP_HVC:
@@ -XXX,XX +XXX,XX @@ bool arm_is_psci_call(ARMCPU *cpu, int excp_type)
         return false;
     }
 
-    return true;
+    switch (param) {
+    case QEMU_PSCI_0_2_FN_PSCI_VERSION:
+    case QEMU_PSCI_0_2_FN_MIGRATE_INFO_TYPE:
+    case QEMU_PSCI_0_2_FN_AFFINITY_INFO:
+    case QEMU_PSCI_0_2_FN64_AFFINITY_INFO:
+    case QEMU_PSCI_0_2_FN_SYSTEM_RESET:
+    case QEMU_PSCI_0_2_FN_SYSTEM_OFF:
+    case QEMU_PSCI_0_1_FN_CPU_ON:
+    case QEMU_PSCI_0_2_FN_CPU_ON:
+    case QEMU_PSCI_0_2_FN64_CPU_ON:
+    case QEMU_PSCI_0_1_FN_CPU_OFF:
+    case QEMU_PSCI_0_2_FN_CPU_OFF:
+    case QEMU_PSCI_0_1_FN_CPU_SUSPEND:
+    case QEMU_PSCI_0_2_FN_CPU_SUSPEND:
+    case QEMU_PSCI_0_2_FN64_CPU_SUSPEND:
+    case QEMU_PSCI_0_1_FN_MIGRATE:
+    case QEMU_PSCI_0_2_FN_MIGRATE:
+        return true;
+    default:
+        return false;
+    }
 }
 
 void arm_handle_psci_call(ARMCPU *cpu)
@@ -XXX,XX +XXX,XX @@ void arm_handle_psci_call(ARMCPU *cpu)
         break;
     case QEMU_PSCI_0_1_FN_MIGRATE:
     case QEMU_PSCI_0_2_FN_MIGRATE:
-    default:
         ret = QEMU_PSCI_RET_NOT_SUPPORTED;
         break;
+    default:
+        g_assert_not_reached();
     }
 
 err:
-- 
2.25.1

This bug seemed worth fixing for 8.0 since we need an rc4 anyway:
we were using uninitialized data for the guarded bit when
combining stage 1 and stage 2 attrs.

thanks
-- PMM

The following changes since commit 08dede07030973c1053868bc64de7e10bfa02ad6:

Merge tag 'pull-ppc-20230409' of https://github.com/legoater/qemu into staging (2023-04-10 11:47:52 +0100)

are available in the Git repository at:

https://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-target-arm-20230410

for you to fetch changes up to 8539dc00552e8ea60420856fc1262c8299bc6308:

target/arm: Copy guarded bit in combine_cacheattrs (2023-04-10 14:31:40 +0100)

----------------------------------------------------------------
target-arm: Fix bug where we weren't initializing
            guarded bit state when combining S1/S2 attrs

----------------------------------------------------------------
Richard Henderson (2):
      target/arm: PTE bit GP only applies to stage1
      target/arm: Copy guarded bit in combine_cacheattrs

target/arm/ptw.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

From: Richard Henderson <richard.henderson@linaro.org>

Only perform the extract of GP during the stage1 walk.

Reported-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Message-id: 20230407185149.3253946-2-richard.henderson@linaro.org
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target/arm/ptw.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/target/arm/ptw.c b/target/arm/ptw.c
index XXXXXXX..XXXXXXX 100644
--- a/target/arm/ptw.c
+++ b/target/arm/ptw.c
@@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_lpae(CPUARMState *env, S1Translate *ptw,
         result->f.attrs.secure = false;
     }
 
-    /* When in aarch64 mode, and BTI is enabled, remember GP in the TLB.  */
-    if (aarch64 && cpu_isar_feature(aa64_bti, cpu)) {
-        result->f.guarded = extract64(attrs, 50, 1); /* GP */
-    }
-
     if (regime_is_stage2(mmu_idx)) {
         result->cacheattrs.is_s2_format = true;
         result->cacheattrs.attrs = extract32(attrs, 2, 4);
@@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_lpae(CPUARMState *env, S1Translate *ptw,
         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.guarded = extract64(attrs, 50, 1); /* GP */
+        }
     }
 
     /*
-- 
2.34.1