[PATCH] target/arm: fix IL bit for data abort exceptions

[Jeff Kubascik]

The Instruction Length bit of the Exception Syndrome Register was fixed to 1
for data aborts. This bit is used by the Xen hypervisor to determine how to
increment the program counter after a mmio handler is successful and returns
control back to the guest virtual machine. With this value fixed to 1, the
hypervisor would always increment the program counter by 0x4. This is a
problem when the guest virtual machine is using Thumb instructions, as the
instruction that caused the exception may be 16 bits.

This adds a is_16bit flag to the disassembler context to keep track of the
current instruction length. For load/store instructions, the instruction
length bit is stored with the instruction syndrome data, to be later used if
the data abort occurs.

Signed-off-by: Jeff Kubascik <address@hidden>
---
Hello,

I am using the ARMv8 version of QEMU to run the Xen hypervisor with a guest
virtual machine compiled for AArch32/Thumb code. I have noticed that when
the guest VM tries to write to an emulated PL011 register, the mmio handler
always increments the program counter by 4, even if the store instruction
that caused the exception was a 16-bit Thumb instruction.

I have traced this back to the IL bit in the ESR_EL2 register. Xen uses the
IL bit to determine how to increment the program counter. However, QEMU does
not correctly emulate this bit, always setting it to 1 (32-bit instruction).

The above patch works for my setup. However, I am not very familiar with the
QEMU code base, so it may not be the best way to do it, or even be correct.
Any feedback would be greatly appreciated.

Sincerely,
Jeff Kubascik
---
 target/arm/tlb_helper.c    | 2 +-
 target/arm/translate-a64.c | 1 +
 target/arm/translate.c     | 4 +++-
 target/arm/translate.h     | 2 ++
 4 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/target/arm/tlb_helper.c b/target/arm/tlb_helper.c
index 5feb312941..e63f8bda29 100644
--- a/target/arm/tlb_helper.c
+++ b/target/arm/tlb_helper.c
@@ -44,7 +44,7 @@ static inline uint32_t merge_syn_data_abort(uint32_t 
template_syn,
         syn = syn_data_abort_with_iss(same_el,
                                       0, 0, 0, 0, 0,
                                       ea, 0, s1ptw, is_write, fsc,
-                                      false);
+                                      true);
         /* Merge the runtime syndrome with the template syndrome.  */
         syn |= template_syn;
     }
diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index d4bebbe629..a3c618fdd9 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -14045,6 +14045,7 @@ static void disas_a64_insn(CPUARMState *env, 
DisasContext *s)
     s->pc_curr = s->base.pc_next;
     insn = arm_ldl_code(env, s->base.pc_next, s->sctlr_b);
     s->insn = insn;
+    s->is_16bit = false;
     s->base.pc_next += 4;
 
     s->fp_access_checked = false;
diff --git a/target/arm/translate.c b/target/arm/translate.c
index 2b6c1f91bf..300480f1b7 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -8555,7 +8555,7 @@ static ISSInfo make_issinfo(DisasContext *s, int rd, bool 
p, bool w)
 
     /* ISS not valid if writeback */
     if (p && !w) {
-        ret = rd;
+        ret = rd | (s->is_16bit ? ISSIs16Bit : 0);
     } else {
         ret = ISSInvalid;
     }
@@ -11057,6 +11057,7 @@ static void arm_tr_translate_insn(DisasContextBase 
*dcbase, CPUState *cpu)
     dc->pc_curr = dc->base.pc_next;
     insn = arm_ldl_code(env, dc->base.pc_next, dc->sctlr_b);
     dc->insn = insn;
+    dc->is_16bit = false;
     dc->base.pc_next += 4;
     disas_arm_insn(dc, insn);
 
@@ -11126,6 +11127,7 @@ static void thumb_tr_translate_insn(DisasContextBase 
*dcbase, CPUState *cpu)
     dc->pc_curr = dc->base.pc_next;
     insn = arm_lduw_code(env, dc->base.pc_next, dc->sctlr_b);
     is_16bit = thumb_insn_is_16bit(dc, dc->base.pc_next, insn);
+    dc->is_16bit = is_16bit;
     dc->base.pc_next += 2;
     if (!is_16bit) {
         uint32_t insn2 = arm_lduw_code(env, dc->base.pc_next, dc->sctlr_b);
diff --git a/target/arm/translate.h b/target/arm/translate.h
index b837b7fcbf..c16f434477 100644
--- a/target/arm/translate.h
+++ b/target/arm/translate.h
@@ -14,6 +14,8 @@ typedef struct DisasContext {
     target_ulong pc_curr;
     target_ulong page_start;
     uint32_t insn;
+    /* 16-bit instruction flag */
+    bool is_16bit;
     /* Nonzero if this instruction has been conditionally skipped.  */
     int condjmp;
     /* The label that will be jumped to when the instruction is skipped.  */
-- 
2.17.1