[Xen-devel] [PATCH 1/3] xen/arm: io: Distinguish unhandled IO from aborted one

[Julien Grall <julien.grall@xxxxxxx>]

Currently, Xen is considering that an IO could either be handled or
unhandled. When unhandled, the stage-2 abort function will try another
way to resolve the abort.

However, the MMIO emulation may return unhandled when the address
belongs to an emulated range but was not correct. In that case, Xen
should avodi to try another way and directly inject a guest data abort.

Introduce a tri-state return to distinguish the following state:
    * IO_ABORT: The IO was handled but resulted to an abort
    * IO_HANDLED: The IO was handled
    * IO_UNHANDLED: The IO was unhandled

For now, it is considered that an IO belonging to an emulated range
could either be handled or inject an abort. This could be revisit in the
future if overlapped region exist (or we want to try another way to
resolve the abort).

Signed-off-by: Julien Grall <julien.grall@xxxxxxx>
---
 xen/arch/arm/io.c          | 24 ++++++++++++++----------
 xen/arch/arm/traps.c       | 34 +++++++++++++++++++++++-----------
 xen/include/asm-arm/mmio.h |  9 ++++++++-
 3 files changed, 45 insertions(+), 22 deletions(-)

diff --git a/xen/arch/arm/io.c b/xen/arch/arm/io.c
index c748d8f5bf..a74ec21b86 100644
--- a/xen/arch/arm/io.c
+++ b/xen/arch/arm/io.c
@@ -23,8 +23,9 @@
 #include <asm/current.h>
 #include <asm/mmio.h>
 
-static int handle_read(const struct mmio_handler *handler, struct vcpu *v,
-                       mmio_info_t *info)
+static enum io_state handle_read(const struct mmio_handler *handler,
+                                 struct vcpu *v,
+                                 mmio_info_t *info)
 {
     const struct hsr_dabt dabt = info->dabt;
     struct cpu_user_regs *regs = guest_cpu_user_regs();
@@ -37,7 +38,7 @@ static int handle_read(const struct mmio_handler *handler, 
struct vcpu *v,
     uint8_t size = (1 << dabt.size) * 8;
 
     if ( !handler->ops->read(v, info, &r, handler->priv) )
-        return 0;
+        return IO_ABORT;
 
     /*
      * Sign extend if required.
@@ -57,17 +58,20 @@ static int handle_read(const struct mmio_handler *handler, 
struct vcpu *v,
 
     set_user_reg(regs, dabt.reg, r);
 
-    return 1;
+    return IO_HANDLED;
 }
 
-static int handle_write(const struct mmio_handler *handler, struct vcpu *v,
-                        mmio_info_t *info)
+static enum io_state handle_write(const struct mmio_handler *handler,
+                                  struct vcpu *v,
+                                  mmio_info_t *info)
 {
     const struct hsr_dabt dabt = info->dabt;
     struct cpu_user_regs *regs = guest_cpu_user_regs();
+    int ret;
 
-    return handler->ops->write(v, info, get_user_reg(regs, dabt.reg),
-                               handler->priv);
+    ret = handler->ops->write(v, info, get_user_reg(regs, dabt.reg),
+                              handler->priv);
+    return ( ret ) ? IO_HANDLED : IO_ABORT;
 }
 
 /* This function assumes that mmio regions are not overlapped */
@@ -100,14 +104,14 @@ static const struct mmio_handler 
*find_mmio_handler(struct domain *d,
     return handler;
 }
 
-int handle_mmio(mmio_info_t *info)
+enum io_state handle_mmio(mmio_info_t *info)
 {
     struct vcpu *v = current;
     const struct mmio_handler *handler = NULL;
 
     handler = find_mmio_handler(v->domain, info->gpa);
     if ( !handler )
-        return 0;
+        return IO_UNHANDLED;
 
     if ( info->dabt.write )
         return handle_write(handler, v, info);
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index c8534d6cff..843adf4959 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -1864,10 +1864,10 @@ 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)
-{
+static enum io_state 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,
@@ -1879,11 +1879,11 @@ static bool try_handle_mmio(struct cpu_user_regs *regs,
 
     /* stage-1 page table should never live in an emulated MMIO region */
     if ( dabt.s1ptw )
-        return false;
+        return IO_UNHANDLED;
 
     /* All the instructions used on emulated MMIO region should be valid */
     if ( !dabt.valid )
-        return false;
+        return IO_UNHANDLED;
 
     /*
      * Erratum 766422: Thumb store translation fault to Hypervisor may
@@ -1896,11 +1896,11 @@ static bool try_handle_mmio(struct cpu_user_regs *regs,
         if ( rc )
         {
             gprintk(XENLOG_DEBUG, "Unable to decode instruction\n");
-            return false;
+            return IO_ABORT;
         }
     }
 
-    return !!handle_mmio(&info);
+    return handle_mmio(&info);
 }
 
 /*
@@ -2005,10 +2005,21 @@ static void do_trap_stage2_abort_guest(struct 
cpu_user_regs *regs,
          *
          * Note that emulated region cannot be executed
          */
-        if ( is_data && try_handle_mmio(regs, hsr, gpa) )
+        if ( is_data )
         {
-            advance_pc(regs, hsr);
-            return;
+            enum io_state state = try_handle_mmio(regs, hsr, gpa);
+
+            switch ( state )
+            {
+            case IO_ABORT:
+                goto inject_abt;
+            case IO_HANDLED:
+                advance_pc(regs, hsr);
+                return;
+            case IO_UNHANDLED:
+                /* Nothing to do */
+                break;
+            }
         }
 
         /*
@@ -2029,6 +2040,7 @@ static void do_trap_stage2_abort_guest(struct 
cpu_user_regs *regs,
                 hsr.bits, xabt.fsc);
     }
 
+inject_abt:
     gdprintk(XENLOG_DEBUG, "HSR=0x%x pc=%#"PRIregister" gva=%#"PRIvaddr
              " gpa=%#"PRIpaddr"\n", hsr.bits, regs->pc, gva, gpa);
     if ( is_data )
diff --git a/xen/include/asm-arm/mmio.h b/xen/include/asm-arm/mmio.h
index 37e2b7a707..baaecf6931 100644
--- a/xen/include/asm-arm/mmio.h
+++ b/xen/include/asm-arm/mmio.h
@@ -32,6 +32,13 @@ typedef struct
     paddr_t gpa;
 } mmio_info_t;
 
+enum io_state
+{
+    IO_ABORT,       /* The IO was handled by the helper and lead to an abort. 
*/
+    IO_HANDLED,     /* The IO was successfully handled by the helper. */
+    IO_UNHANDLED,   /* The IO was not handled by the helper. */
+};
+
 typedef int (*mmio_read_t)(struct vcpu *v, mmio_info_t *info,
                            register_t *r, void *priv);
 typedef int (*mmio_write_t)(struct vcpu *v, mmio_info_t *info,
@@ -56,7 +63,7 @@ struct vmmio {
     struct mmio_handler *handlers;
 };
 
-extern int handle_mmio(mmio_info_t *info);
+extern enum io_state handle_mmio(mmio_info_t *info);
 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