[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



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.

I updated the Arm ARM reference to the online instruction decoding
table which will hopefully be more stable than the Arm ARM section
numbers.

Signed-off-by: Alex Bennée <alex.bennee@xxxxxxxxxx>
Cc: Manos Pitsidianakis <manos.pitsidianakis@xxxxxxxxxx>
---
 xen/arch/arm/decode.c | 20 ++++++++++++++++++++
 xen/arch/arm/decode.h | 38 +++++++++++++++++++++++++++++++++++---
 2 files changed, 55 insertions(+), 3 deletions(-)

diff --git a/xen/arch/arm/decode.c b/xen/arch/arm/decode.c
index 2537dbebc1..824025c24c 100644
--- a/xen/arch/arm/decode.c
+++ b/xen/arch/arm/decode.c
@@ -87,6 +87,26 @@ 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 & 0b0101) != 0b0100 )
+    {
+        gprintk(XENLOG_ERR, "Not a load/store instruction op1=%d",
+                opcode.top_level.op1);
+        goto bad_loadstore;
+    }
+
+    /*
+     * We are only expecting single register load/stores
+     */
+    if ( (opcode.ld_st.op0 & 0b0011) != 0b0011 )
+    {
+        gprintk(XENLOG_ERR, "Not single register load/store op0=%d",
+                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)
diff --git a/xen/arch/arm/decode.h b/xen/arch/arm/decode.h
index 13db8ac968..b1580178eb 100644
--- a/xen/arch/arm/decode.h
+++ b/xen/arch/arm/decode.h
@@ -24,9 +24,27 @@
 #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)".
+ * From:
+ * https://developer.arm.com/documentation/ddi0602/2023-12/Index-by-Encoding
+ *
+ * Top level encoding:
+ *
+ *   31  30  29 28  25 24                                             0
+ * ___________________________________________________________________
+ * |op0 | x  x |  op1 |                                               |
+ * |____|______|______|_______________________________________________|
+ *
+ * op0 = 0 is reserved
+ * op1 = x1x0 for Loads and Stores
+ *
+ * Loads and Stores
+ *
+ *  31    28 27   26   25  24             9 8                        0
+ * ___________________________________________________________________
+ * |  op0   | 1 | op1 | 0 |       op2      |                          |
+ * |________|___|_____|___|________________|__________________________|
+ *
+ * Load/store register (immediate post-indexed)
  *
  * 31 30 29  27 26 25  23   21 20              11   9         4       0
  * ___________________________________________________________________
@@ -35,6 +53,20 @@
  */
 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:9;
+        unsigned int op2:15;
+        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 */
-- 
2.39.2




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.