On 02/02/18 14:34, Andre Przywara wrote:
Hi,

Hi,

On 02/02/18 10:14, Julien Grall wrote:
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 avoid 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 in an abort
     * IO_HANDLED: The IO was handled
     * IO_UNHANDLED: The IO was unhandled

I like that very much!


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.gr...@arm.com>

---
     Changes in v2:
         - Always return IO_ABORT when the check failed because we know
         it was targeted emulated IO.
         - Fix typoes
---
  xen/arch/arm/io.c          | 32 ++++++++++++++++++--------------
  xen/arch/arm/traps.c       | 18 +++++++++++++++---
  xen/include/asm-arm/mmio.h | 13 ++++++++++---
  3 files changed, 43 insertions(+), 20 deletions(-)

diff --git a/xen/arch/arm/io.c b/xen/arch/arm/io.c
index c3e9239ffe..1f4cb8f37d 100644
--- a/xen/arch/arm/io.c
+++ b/xen/arch/arm/io.c
@@ -26,8 +26,9 @@
#include "decode.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();
@@ -40,7 +41,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.
@@ -60,17 +61,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;

Why the brackets?

I was not feeling lazy that day ;). I will drop them.


Otherwise:

Reviewed-by: Andre Przywara <andre.przyw...@arm.com>

Cheers,

--
Julien Grall

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

Reply via email to