[Xen-devel] [PATCH v3 1/4] xen/arm: traps: Merge try_handle_mmio() and handle_mmio()

[Julien Grall <julien.grall@xxxxxxx>]

At the moment, try_handle_mmio() will do check on the HSR and bail out
if one check fail. This means that another method will be tried to
handle the fault even for bad access on emulated region. While this
should not be an issue, this is not future proof.

Move the checks of try_handle_mmio() in handle_mmio() after we identified
the fault to target an emulated MMIO. While this does not fix the potential
fall-through, a follow-up patch will do by distinguish the potential error.

Note that the handle_mmio() was renamed to try_handle_mmio() and the
prototype adapted.

While merging the 2 functions, remove the check whether the fault is
stage-2 abort on stage-1 translation walk because the instruction
syndrome will always be invalid (see B3-1433 in DDI 0406C.c and
D10-2460 in DDI 0487C.a).

Signed-off-by: Julien Grall <julien.grall@xxxxxxx>

---
    Changes in v2:
        - Patch added
---
 xen/arch/arm/io.c          | 43 ++++++++++++++++++++++++++++++++++++++-----
 xen/arch/arm/traps.c       | 41 -----------------------------------------
 xen/include/asm-arm/mmio.h |  4 +++-
 3 files changed, 41 insertions(+), 47 deletions(-)

diff --git a/xen/arch/arm/io.c b/xen/arch/arm/io.c
index c748d8f5bf..c3e9239ffe 100644
--- a/xen/arch/arm/io.c
+++ b/xen/arch/arm/io.c
@@ -20,9 +20,12 @@
 #include <xen/spinlock.h>
 #include <xen/sched.h>
 #include <xen/sort.h>
+#include <asm/cpuerrata.h>
 #include <asm/current.h>
 #include <asm/mmio.h>
 
+#include "decode.h"
+
 static int handle_read(const struct mmio_handler *handler, struct vcpu *v,
                        mmio_info_t *info)
 {
@@ -100,19 +103,49 @@ static const struct mmio_handler 
*find_mmio_handler(struct domain *d,
     return handler;
 }
 
-int handle_mmio(mmio_info_t *info)
+int try_handle_mmio(struct cpu_user_regs *regs,
+                    const union hsr hsr,
+                    paddr_t gpa)
 {
     struct vcpu *v = current;
     const struct mmio_handler *handler = NULL;
+    const struct hsr_dabt dabt = hsr.dabt;
+    mmio_info_t info = {
+        .gpa = gpa,
+        .dabt = dabt
+    };
+
+    ASSERT(hsr.ec == HSR_EC_DATA_ABORT_LOWER_EL);
 
-    handler = find_mmio_handler(v->domain, info->gpa);
+    handler = find_mmio_handler(v->domain, info.gpa);
     if ( !handler )
         return 0;
 
-    if ( info->dabt.write )
-        return handle_write(handler, v, info);
+    /* All the instructions used on emulated MMIO region should be valid */
+    if ( !dabt.valid )
+        return 0;
+
+    /*
+     * Erratum 766422: Thumb store translation fault to Hypervisor may
+     * not have correct HSR Rt value.
+     */
+    if ( check_workaround_766422() && (regs->cpsr & PSR_THUMB) &&
+         dabt.write )
+    {
+        int rc;
+
+        rc = decode_instruction(regs, &info.dabt);
+        if ( rc )
+        {
+            gprintk(XENLOG_DEBUG, "Unable to decode instruction\n");
+            return 0;
+        }
+    }
+
+    if ( info.dabt.write )
+        return handle_write(handler, v, &info);
     else
-        return handle_read(handler, v, info);
+        return handle_read(handler, v, &info);
 }
 
 void register_mmio_handler(struct domain *d,
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index c8534d6cff..2f8d790bb3 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -51,8 +51,6 @@
 #include <asm/vgic.h>
 #include <asm/vtimer.h>
 
-#include "decode.h"
-
 /* The base of the stack must always be double-word aligned, which means
  * that both the kernel half of struct cpu_user_regs (which is pushed in
  * entry.S) and struct cpu_info (which lives at the bottom of a Xen
@@ -1864,45 +1862,6 @@ static inline bool hpfar_is_valid(bool s1ptw, uint8_t 
fsc)
     return s1ptw || (fsc == FSC_FLT_TRANS && !check_workaround_834220());
 }
 
-static bool try_handle_mmio(struct cpu_user_regs *regs,
-                            const union hsr hsr,
-                            paddr_t gpa)
-{
-    const struct hsr_dabt dabt = hsr.dabt;
-    mmio_info_t info = {
-        .gpa = gpa,
-        .dabt = dabt
-    };
-    int rc;
-
-    ASSERT(hsr.ec == HSR_EC_DATA_ABORT_LOWER_EL);
-
-    /* stage-1 page table should never live in an emulated MMIO region */
-    if ( dabt.s1ptw )
-        return false;
-
-    /* All the instructions used on emulated MMIO region should be valid */
-    if ( !dabt.valid )
-        return false;
-
-    /*
-     * Erratum 766422: Thumb store translation fault to Hypervisor may
-     * not have correct HSR Rt value.
-     */
-    if ( check_workaround_766422() && (regs->cpsr & PSR_THUMB) &&
-         dabt.write )
-    {
-        rc = decode_instruction(regs, &info.dabt);
-        if ( rc )
-        {
-            gprintk(XENLOG_DEBUG, "Unable to decode instruction\n");
-            return false;
-        }
-    }
-
-    return !!handle_mmio(&info);
-}
-
 /*
  * When using ACPI, most of the MMIO regions will be mapped on-demand
  * in stage-2 page tables for the hardware domain because Xen is not
diff --git a/xen/include/asm-arm/mmio.h b/xen/include/asm-arm/mmio.h
index 37e2b7a707..c941073257 100644
--- a/xen/include/asm-arm/mmio.h
+++ b/xen/include/asm-arm/mmio.h
@@ -56,7 +56,9 @@ struct vmmio {
     struct mmio_handler *handlers;
 };
 
-extern int handle_mmio(mmio_info_t *info);
+int try_handle_mmio(struct cpu_user_regs *regs,
+                    const union hsr hsr,
+                    paddr_t gpa);
 void register_mmio_handler(struct domain *d,
                            const struct mmio_handler_ops *ops,
                            paddr_t addr, paddr_t size, void *priv);
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel