xen/arm: traps: Merge try_handle_mmio() and handle_mmio()

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@arm.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Reviewed-by: Andre Przywara <andre.przywara@arm.com>

diff --git a/xen/arch/arm/io.c b/xen/arch/arm/io.c
index c748d8f5bf22bbb53b5b60d779f761024b6f65cd..c3e9239ffe53b5d1b01a0e0c4eb115b765de1c26 100644 (file)
--- 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 c8534d6cff78526b59a5493724663ce5d3138b30..2f8d790bb319ab716f23deffaee8bebf36fec8d9 100644 (file)
--- 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 37e2b7a7070a9aaf5b74aed0e8b67a09f1a4276c..c9410732576f0956c47d3d15128ee4ba4d02fae3 100644 (file)
--- 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);