diff mbox series

[v1,1/4] target/arm: handle M-profile semihosting at translate time

Message ID 20190703155244.28166-2-alex.bennee@linaro.org
State Superseded
Headers show
Series arm semihosting cleanups | expand

Commit Message

Alex Bennée July 3, 2019, 3:52 p.m. UTC 
We do this for other semihosting calls so we might as well do it for
M-profile as well.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

---
 target/arm/helper.c    | 18 ++++++------------
 target/arm/translate.c | 20 +++++++++++++++++++-
 2 files changed, 25 insertions(+), 13 deletions(-)

-- 
2.20.1

Comments

Richard Henderson July 3, 2019, 4:26 p.m. UTC | #1 
On 7/3/19 5:52 PM, Alex Bennée wrote:
> +static inline void gen_thumb_bkpt(DisasContext *s, int imm8)

> +{

> +    if (arm_dc_feature(s, ARM_FEATURE_M) &&

> +        semihosting_enabled() &&

> +#ifndef CONFIG_USER_ONLY

> +        s->current_el != 0 &&

> +#endif

> +        (imm8 == 0xab)) {


Extra parenthesis.  Otherwise,
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>



r~
Alex Bennée July 4, 2019, 10:21 a.m. UTC | #2 
Richard Henderson <richard.henderson@linaro.org> writes:

> On 7/3/19 5:52 PM, Alex Bennée wrote:

>> +static inline void gen_thumb_bkpt(DisasContext *s, int imm8)

>> +{

>> +    if (arm_dc_feature(s, ARM_FEATURE_M) &&

>> +        semihosting_enabled() &&

>> +#ifndef CONFIG_USER_ONLY

>> +        s->current_el != 0 &&

>> +#endif

>> +        (imm8 == 0xab)) {

>

> Extra parenthesis.


The wrapping on imm8 == 0xab? Do you want that cleaned up on the other
patches as well?

> Otherwise,

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

>

>

> r~



--
Alex Bennée
Philippe Mathieu-Daudé July 4, 2019, 10:25 a.m. UTC | #3 
On 7/4/19 12:21 PM, Alex Bennée wrote:
> 

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

> 

>> On 7/3/19 5:52 PM, Alex Bennée wrote:

>>> +static inline void gen_thumb_bkpt(DisasContext *s, int imm8)

>>> +{

>>> +    if (arm_dc_feature(s, ARM_FEATURE_M) &&

>>> +        semihosting_enabled() &&

>>> +#ifndef CONFIG_USER_ONLY

>>> +        s->current_el != 0 &&

>>> +#endif

>>> +        (imm8 == 0xab)) {

>>

>> Extra parenthesis.

> 

> The wrapping on imm8 == 0xab? Do you want that cleaned up on the other

> patches as well?


I understood this comment for "(s->current_el != 0) &&"
Richard Henderson July 5, 2019, 3:09 p.m. UTC | #4 
On 7/4/19 12:25 PM, Philippe Mathieu-Daudé wrote:
> 

> 

> On 7/4/19 12:21 PM, Alex Bennée wrote:

>>

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

>>

>>> On 7/3/19 5:52 PM, Alex Bennée wrote:

>>>> +static inline void gen_thumb_bkpt(DisasContext *s, int imm8)

>>>> +{

>>>> +    if (arm_dc_feature(s, ARM_FEATURE_M) &&

>>>> +        semihosting_enabled() &&

>>>> +#ifndef CONFIG_USER_ONLY

>>>> +        s->current_el != 0 &&

>>>> +#endif

>>>> +        (imm8 == 0xab)) {

>>>

>>> Extra parenthesis.

>>

>> The wrapping on imm8 == 0xab? Do you want that cleaned up on the other

>> patches as well?

> 

> I understood this comment for "(s->current_el != 0) &&"

> 


No, I meant imm8 == 0xab.  And, sure, cleaning the other patches would be good.


r~
diff mbox series

Patch

diff --git a/target/arm/helper.c b/target/arm/helper.c
index df4276f5f6..ad29dc4072 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -9692,19 +9692,13 @@  void arm_v7m_cpu_do_interrupt(CPUState *cs)
             break;
         }
         break;
+    case EXCP_SEMIHOST:
+        qemu_log_mask(CPU_LOG_INT,
+                      "...handling as semihosting call 0x%x\n",
+                      env->regs[0]);
+        env->regs[0] = do_arm_semihosting(env);
+        return;
     case EXCP_BKPT:
-        if (semihosting_enabled()) {
-            int nr;
-            nr = arm_lduw_code(env, env->regs[15], arm_sctlr_b(env)) & 0xff;
-            if (nr == 0xab) {
-                env->regs[15] += 2;
-                qemu_log_mask(CPU_LOG_INT,
-                              "...handling as semihosting call 0x%x\n",
-                              env->regs[0]);
-                env->regs[0] = do_arm_semihosting(env);
-                return;
-            }
-        }
         armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_DEBUG, false);
         break;
     case EXCP_IRQ:
diff --git a/target/arm/translate.c b/target/arm/translate.c
index 4750b9fa1b..aaab043636 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -10977,6 +10977,24 @@  illegal_op:
                        default_exception_el(s));
 }
 
+/*
+ * Thumb BKPT. On M-profile CPUs this may be a semihosting call which
+ * we can process much the same way as gen_hlt() above.
+ */
+static inline void gen_thumb_bkpt(DisasContext *s, int imm8)
+{
+    if (arm_dc_feature(s, ARM_FEATURE_M) &&
+        semihosting_enabled() &&
+#ifndef CONFIG_USER_ONLY
+        s->current_el != 0 &&
+#endif
+        (imm8 == 0xab)) {
+        gen_exception_internal_insn(s, 0, EXCP_SEMIHOST);
+        return;
+    }
+    gen_exception_bkpt_insn(s, 2, syn_aa32_bkpt(imm8, true));
+}
+
 static void disas_thumb_insn(DisasContext *s, uint32_t insn)
 {
     uint32_t val, op, rm, rn, rd, shift, cond;
@@ -11605,7 +11623,7 @@  static void disas_thumb_insn(DisasContext *s, uint32_t insn)
         {
             int imm8 = extract32(insn, 0, 8);
             ARCH(5);
-            gen_exception_bkpt_insn(s, 2, syn_aa32_bkpt(imm8, true));
+            gen_thumb_bkpt(s, imm8);
             break;
         }