[RFC PATCH v2] xen/arm: improve handling of load/store instruction decoding

[Alex Bennée <alex.bennee@xxxxxxxxxx>]

While debugging VirtIO on Arm we ran into a warning due to memory
being memcpy'd across MMIO space. While the bug was in the mappings
the warning was a little confusing:

  (XEN) d47v2 Rn should not be equal to Rt except for r31
  (XEN) d47v2 unhandled Arm instruction 0x3d800000
  (XEN) d47v2 Unable to decode instruction

The Rn == Rt warning is only applicable to single register load/stores
so add some verification steps before to weed out unexpected accesses.

While at it update the Arm ARM references to the latest version of the
documentation.

Signed-off-by: Alex Bennée <alex.bennee@xxxxxxxxxx>
Cc: Manos Pitsidianakis <manos.pitsidianakis@xxxxxxxxxx>

---
v2
  - use single line comments where applicable
  - update Arm ARM references
  - use #defines for magic numbers
---
 xen/arch/arm/decode.c | 35 ++++++++++++++++++++------
 xen/arch/arm/decode.h | 57 ++++++++++++++++++++++++++++++++++++++-----
 2 files changed, 79 insertions(+), 13 deletions(-)

diff --git a/xen/arch/arm/decode.c b/xen/arch/arm/decode.c
index 2537dbebc1..73a88e4701 100644
--- a/xen/arch/arm/decode.c
+++ b/xen/arch/arm/decode.c
@@ -87,15 +87,36 @@ static int decode_arm64(register_t pc, mmio_info_t *info)
         return 1;
     }
 
+    /* Check this is a load/store of some sort */
+    if ( (opcode.top_level.op1 & TL_LDST_OP1_MASK) != TL_LDST_OP1_VALUE )
+    {
+        gprintk(XENLOG_ERR, "Not a load/store instruction op1=%u\n",
+                opcode.top_level.op1);
+        goto bad_loadstore;
+    }
+
+    /* We are only expecting single register load/stores */
+    if ( (opcode.ld_st.op0 & LS_SREG_OP0_MASK) != LS_SREG_OP0_VALUE )
+    {
+        gprintk(XENLOG_ERR, "Not single register load/store op0=%u\n",
+                opcode.ld_st.op0);
+        goto bad_loadstore;
+    }
+
     /*
-     * Refer Arm v8 ARM DDI 0487G.b, Page - C6-1107
-     * "Shared decode for all encodings" (under ldr immediate)
-     * If n == t && n != 31, then the return value is implementation defined
-     * (can be WBSUPPRESS, UNKNOWN, UNDEFINED or NOP). Thus, we do not support
-     * this. This holds true for ldrb/ldrh immediate as well.
+     * Refer Arm v8 ARM DDI 0487J.a, Page - K1-12586
+     *
+     * STR (immediate) CONSTRAINED UNPREDICTABLE behaviour
+     *
+     * "If the instruction encoding specifies pre-indexed addressing or
+     * post-indexed addressing, and n == t && n != 31, then one of the
+     * following behaviors must occur:" UNDEFINED, NOP or UNKNOWN
+     *
+     * Execution @ EL0/EL1 when HCR_EL2.TIDCP is 1 traps to EL2 with
+     * EC = 0.
      *
-     * Also refer, Page - C6-1384, the above described behaviour is same for
-     * str immediate. This holds true for strb/strh immediate as well
+     * This also hold true for LDR (immediate), Page K1-12581 and
+     * the RB/RH variants of both.
      */
     if ( (opcode.ldr_str.rn == opcode.ldr_str.rt) && (opcode.ldr_str.rn != 31) 
)
     {
diff --git a/xen/arch/arm/decode.h b/xen/arch/arm/decode.h
index 13db8ac968..188114a71e 100644
--- a/xen/arch/arm/decode.h
+++ b/xen/arch/arm/decode.h
@@ -24,17 +24,54 @@
 #include <asm/processor.h>
 
 /*
- * Refer to the ARMv8 ARM (DDI 0487G.b), Section C4.1.4 Loads and Stores
- * Page 318 specifies the following bit pattern for
- * "load/store register (immediate post-indexed)".
+ * Refer to the ARMv8 ARM (DDI 0487J.a)
  *
- * 31 30 29  27 26 25  23   21 20              11   9         4       0
+ * Section C A64 Instruct Set Encoding
+ *
+ * C4.1 A64 instruction set encoding:
+ *
+ *   31  30  29 28  25 24                                             0
  * ___________________________________________________________________
- * |size|1 1 1 |V |0 0 |opc |0 |      imm9     |0 1 |  Rn     |  Rt   |
- * |____|______|__|____|____|__|_______________|____|_________|_______|
+ * |op0 | x  x |  op1 |                                               |
+ * |____|______|______|_______________________________________________|
+ *
+ * op0 = 0 is reserved
+ * op1 = x1x0 for Loads and Stores
+ *
+ * Section C4.1.88 Loads and Stores
+ *
+ *  31    28 27   26   25  24 23 22 21      16 15  12 11 10 9        0
+ * ___________________________________________________________________
+ * |  op0   | 1 | op1 | 0 | op2 |  |    op3   |      | op4 |          |
+ * |________|___|_____|___|_____|__|__________|______|_____|__________|
+ *
+ * Page C4-653 Load/store register (immediate post-indexed)
+ *
+ * 31 30 29  27 26 25 24 23 22 21 20           12 11 10 9    5 4     0
+ * ___________________________________________________________________
+ * |size|1 1 1 |V | 0 0 | opc |0 |      imm9     | 0 1 |  Rn  |  Rt   |
+ * |____|______|__|_____|_____|__|_______________|_____|______|_______|
  */
 union instr {
     uint32_t value;
+    struct {
+        unsigned int ign2:25;
+        unsigned int op1:4;     /* instruction class */
+        unsigned int ign1:2;
+        unsigned int op0:1;     /* value = 1b */
+    } top_level;
+    struct {
+        unsigned int ign1:10;
+        unsigned int op4:2;
+        unsigned int ign2:4;
+        unsigned int op3:6;
+        unsigned int ign3:1;
+        unsigned int op2:2;
+        unsigned int fixed1:1; /* value = 0b */
+        unsigned int op1:1;
+        unsigned int fixed2:1; /* value = 1b */
+        unsigned int op0:4;
+    } ld_st;
     struct {
         unsigned int rt:5;     /* Rt register */
         unsigned int rn:5;     /* Rn register */
@@ -49,6 +86,14 @@ union instr {
     } ldr_str;
 };
 
+/* Top level load/store encoding */
+#define TL_LDST_OP1_MASK        0b0101
+#define TL_LDST_OP1_VALUE       0b0100
+
+/* Load/store single reg encoding */
+#define LS_SREG_OP0_MASK        0b0011
+#define LS_SREG_OP0_VALUE       0b0011
+
 #define POST_INDEX_FIXED_MASK   0x3B200C00
 #define POST_INDEX_FIXED_VALUE  0x38000400
 
-- 
2.39.2