diff mbox series

target/arm: Fix CNTPOFF_EL2 trap to missing EL3

Message ID m3al6amhdkmsiy2f62w72ufth6dzn45xg5cz6xljceyibphnf4@ezmmpwk4tnhl (mailing list archive)
State New, archived
Headers show
Series target/arm: Fix CNTPOFF_EL2 trap to missing EL3 | expand

Commit Message

Pierre-Clément Tosi April 4, 2024, 4:36 p.m. UTC
EL2 accesses to CNTPOFF_EL2 should only ever trap to EL3 if EL3 is
present, as described by the reference manual (for MRS):

  /* ... */
  elsif PSTATE.EL == EL2 then
      if Halted() && HaveEL(EL3) && /*...*/ then
          UNDEFINED;
      elsif HaveEL(EL3) && SCR_EL3.ECVEn == '0' then
          /* ... */
      else
          X[t, 64] = CNTPOFF_EL2;

However, the existing implementation of gt_cntpoff_access() always
returns CP_ACCESS_TRAP_EL3 for EL2 accesses with SCR_EL3.ECVEn unset. In
pseudo-code terminology, this corresponds to assuming that HaveEL(EL3)
is always true, which is wrong. As a result, QEMU panics in
access_check_cp_reg() when started without EL3 and running EL2 code
accessing the register (e.g. any recent KVM booting a guest).

Therefore, add the HaveEL(EL3) check to gt_cntpoff_access().

Cc: qemu-stable@nongnu.org
Fixes: 2808d3b38a52 ("target/arm: Implement FEAT_ECV CNTPOFF_EL2 handling")
Signed-off-by: Pierre-Clément Tosi <ptosi@google.com>
---
 target/arm/helper.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Peter Maydell April 5, 2024, 2:23 p.m. UTC | #1
On Thu, 4 Apr 2024 at 17:36, Pierre-Clément Tosi <ptosi@google.com> wrote:
>
> EL2 accesses to CNTPOFF_EL2 should only ever trap to EL3 if EL3 is
> present, as described by the reference manual (for MRS):
>
>   /* ... */
>   elsif PSTATE.EL == EL2 then
>       if Halted() && HaveEL(EL3) && /*...*/ then
>           UNDEFINED;
>       elsif HaveEL(EL3) && SCR_EL3.ECVEn == '0' then
>           /* ... */
>       else
>           X[t, 64] = CNTPOFF_EL2;
>
> However, the existing implementation of gt_cntpoff_access() always
> returns CP_ACCESS_TRAP_EL3 for EL2 accesses with SCR_EL3.ECVEn unset. In
> pseudo-code terminology, this corresponds to assuming that HaveEL(EL3)
> is always true, which is wrong. As a result, QEMU panics in
> access_check_cp_reg() when started without EL3 and running EL2 code
> accessing the register (e.g. any recent KVM booting a guest).
>
> Therefore, add the HaveEL(EL3) check to gt_cntpoff_access().
>
> Cc: qemu-stable@nongnu.org
> Fixes: 2808d3b38a52 ("target/arm: Implement FEAT_ECV CNTPOFF_EL2 handling")
> Signed-off-by: Pierre-Clément Tosi <ptosi@google.com>

Oops, thanks for the fix. I'll get this in for the 9.0
release, so we won't need to backport it to stable branches
(the commit breaking this only went in in this cycle).



Applied to target-arm.next, thanks.

-- PMM
diff mbox series

Patch

diff --git a/target/arm/helper.c b/target/arm/helper.c
index 3f3a5b55d4..13ad90cac1 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -3452,7 +3452,8 @@  static CPAccessResult gt_cntpoff_access(CPUARMState *env,
                                         const ARMCPRegInfo *ri,
                                         bool isread)
 {
-    if (arm_current_el(env) == 2 && !(env->cp15.scr_el3 & SCR_ECVEN)) {
+    if (arm_current_el(env) == 2 && arm_feature(env, ARM_FEATURE_EL3) &&
+        !(env->cp15.scr_el3 & SCR_ECVEN)) {
         return CP_ACCESS_TRAP_EL3;
     }
     return CP_ACCESS_OK;