diff mbox series

[01/10] target/arm: Improve debug logging of AArch32 exception return

Message ID 20181012144235.19646-2-peter.maydell@linaro.org
State Superseded
Headers show
Series target/arm: more HCR bits, improve syndrome reporting | expand

Commit Message

Peter Maydell Oct. 12, 2018, 2:42 p.m. UTC 
For AArch32, exception return happens through certain kinds
of CPSR write. We don't currently have any CPU_LOG_INT logging
of these events (unlike AArch64, where we log in the ERET
instruction). Add some suitable logging.

This will log exception returns like this:
Exception return from AArch32 hyp to usr PC 0x80100374

paralleling the existing logging in the exception_return
helper for AArch64 exception returns:
Exception return from AArch64 EL2 to AArch64 EL0 PC 0x8003045c
Exception return from AArch64 EL2 to AArch32 EL0 PC 0x8003045c

(Note that an AArch32 exception return can only be
AArch32->AArch32, never to AArch64.)

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

---
 target/arm/internals.h | 18 ++++++++++++++++++
 target/arm/helper.c    | 10 ++++++++++
 target/arm/translate.c |  7 +------
 3 files changed, 29 insertions(+), 6 deletions(-)

-- 
2.19.0

Comments

Richard Henderson Oct. 14, 2018, 4:12 p.m. UTC | #1 
On 10/12/18 7:42 AM, Peter Maydell wrote:
> For AArch32, exception return happens through certain kinds

> of CPSR write. We don't currently have any CPU_LOG_INT logging

> of these events (unlike AArch64, where we log in the ERET

> instruction). Add some suitable logging.

> 

> This will log exception returns like this:

> Exception return from AArch32 hyp to usr PC 0x80100374

> 

> paralleling the existing logging in the exception_return

> helper for AArch64 exception returns:

> Exception return from AArch64 EL2 to AArch64 EL0 PC 0x8003045c

> Exception return from AArch64 EL2 to AArch32 EL0 PC 0x8003045c

> 

> (Note that an AArch32 exception return can only be

> AArch32->AArch32, never to AArch64.)

> 

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

> ---


Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


> +    static const char * const cpu_mode_names[16] = {

> +        "usr", "fiq", "irq", "svc", "???", "???", "mon", "abt",

> +        "???", "???", "hyp", "und", "???", "???", "???", "sys"

> +    };


Nit: Better as static const char cpu_mode_names[16][4].

For tiny strings like this, the pointer to a separate string is larger than the
string itself.


r~
diff mbox series

Patch

diff --git a/target/arm/internals.h b/target/arm/internals.h
index a4fc709bcc7..abe4d73b59c 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -840,4 +840,22 @@  static inline uint32_t v7m_sp_limit(CPUARMState *env)
     }
 }
 
+/**
+ * aarch32_mode_name(): Return name of the AArch32 CPU mode
+ * @psr: Program Status Register indicating CPU mode
+ *
+ * Returns, for debug logging purposes, a printable representation
+ * of the AArch32 CPU mode ("svc", "usr", etc) as indicated by
+ * the low bits of the specified PSR.
+ */
+static inline const char *aarch32_mode_name(uint32_t psr)
+{
+    static const char * const cpu_mode_names[16] = {
+        "usr", "fiq", "irq", "svc", "???", "???", "mon", "abt",
+        "???", "???", "hyp", "und", "???", "???", "???", "sys"
+    };
+
+    return cpu_mode_names[psr & 0xf];
+}
+
 #endif
diff --git a/target/arm/helper.c b/target/arm/helper.c
index e3368e7edc5..0fa5ac0450f 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -6205,7 +6205,17 @@  void cpsr_write(CPUARMState *env, uint32_t val, uint32_t mask,
                 mask |= CPSR_IL;
                 val |= CPSR_IL;
             }
+            qemu_log_mask(LOG_GUEST_ERROR,
+                          "Illegal AArch32 mode switch attempt from %s to %s\n",
+                          aarch32_mode_name(env->uncached_cpsr),
+                          aarch32_mode_name(val));
         } else {
+            qemu_log_mask(CPU_LOG_INT, "%s %s to %s PC 0x%" PRIx32 "\n",
+                          write_type == CPSRWriteExceptionReturn ?
+                          "Exception return from AArch32" :
+                          "AArch32 mode switch from",
+                          aarch32_mode_name(env->uncached_cpsr),
+                          aarch32_mode_name(val), env->regs[15]);
             switch_mode(env, val & CPSR_M);
         }
     }
diff --git a/target/arm/translate.c b/target/arm/translate.c
index 1b4bacb522b..7c7d920e331 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -13092,11 +13092,6 @@  void gen_intermediate_code(CPUState *cpu, TranslationBlock *tb)
     translator_loop(ops, &dc.base, cpu, tb);
 }
 
-static const char *cpu_mode_names[16] = {
-  "usr", "fiq", "irq", "svc", "???", "???", "mon", "abt",
-  "???", "???", "hyp", "und", "???", "???", "???", "sys"
-};
-
 void arm_cpu_dump_state(CPUState *cs, FILE *f, fprintf_function cpu_fprintf,
                         int flags)
 {
@@ -13162,7 +13157,7 @@  void arm_cpu_dump_state(CPUState *cs, FILE *f, fprintf_function cpu_fprintf,
                     psr & CPSR_V ? 'V' : '-',
                     psr & CPSR_T ? 'T' : 'A',
                     ns_status,
-                    cpu_mode_names[psr & 0xf], (psr & 0x10) ? 32 : 26);
+                    aarch32_mode_name(psr), (psr & 0x10) ? 32 : 26);
     }
 
     if (flags & CPU_DUMP_FPU) {