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

[PATCH v2 08/39] xen/riscv: introduce device-agnostic MMIO emulation dispatch



RISC-V guests can expose several virtual interrupt controllers at
distinct GPA ranges: vPLIC (hasn't been introduced yet) for legacy machines,
vAPLIC and vIMSIC for AIA-compliant ones (are being introduced in the follow
up patches). Routing MMIO faults via a per-device is_access() check in the
trap handler would couple it to every device it must serve, requiring a
new conditional branch in the fault path each time a new emulated device is
added.

Introduce a per-domain MMIO handler registration table, modeled
after the equivalent ARM framework, so that virtual devices
self-register their GPA ranges and read/write callbacks at domain
creation time. The MMIO fault path delegates to a single
try_handle_mmio() entry point and remains agnostic of which device
owns a particular address.

A subsequent patch wires this into the MMIO fault path in traps.c.

Signed-off-by: Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>
---
Changes in v2:
 - Drop copyright from mmio.c as it will go stale anyway as code moves
   around.
 - Drop the max_count parameter of domain_io_init() (MAX_IO_HANDLER is a
   global boundary) and embed the handler array directly in struct vmmio
   as struct mmio_handler handlers[MAX_IO_HANDLER].  This removes the
   xvzalloc_array() allocation, the max_num_entries field and
   domain_io_free() altogether; domain_io_init() consequently cannot fail
   any longer and now returns void. Note this goes slightly beyond the
   suggested variant in that max_num_entries is dropped as well, since
   ARRAY_SIZE(vmmio->handlers) serves the same purpose.
 - Drop the separate register_t argument of mmio_read_t/mmio_write_t;
   handlers now produce and consume the value through info->data.
   handle_read()/handle_write() are gone as a result, with
   try_handle_mmio() invoking ops->read()/ops->write() directly.
 - Turn mmio_read_t/mmio_write_t into function types rather than
   pointer-to-function types, so that pointer-ness is visible at the use
   sites in struct mmio_handler_ops. Constify the mmio_info_t * of the
   write callback, which has no reason to modify it any more.
 - register_mmio_handler() returns int instead of BUG_ON()ing on a full
   table: -ENOSPC now lets the caller fail domain creation. It also
   validates its inputs, rejecting a NULL ops (or one with a missing
   read/write callback) as well as zero-sized and address-wrapping
   regions with -EINVAL.
 - Guarantee the non-overlap property that cmp_mmio_handler() relies on:
   register_mmio_handler() checks the new region against both neighbours
   of its insertion slot and returns -EEXIST on overlap.
 - Replace the sort() call per registration with an insertion into the
   already sorted array: locate the slot and memmove() the tail up by
   one. sort(), swap_mmio_handler() and <xen/sort.h> are gone.
 - Extend cmp_mmio_handler()'s comment to state that it is a bsearch()
   comparator and to explain the key/elem asymmetry; document why the
   neighbours' addr + size cannot overflow.
 - Fix over-long lines and a mis-indented label.
---
---
 xen/arch/riscv/Makefile             |   1 +
 xen/arch/riscv/domain.c             |   3 +
 xen/arch/riscv/include/asm/domain.h |   3 +
 xen/arch/riscv/include/asm/mmio.h   |  63 ++++++++++
 xen/arch/riscv/mmio.c               | 176 ++++++++++++++++++++++++++++
 5 files changed, 246 insertions(+)
 create mode 100644 xen/arch/riscv/include/asm/mmio.h
 create mode 100644 xen/arch/riscv/mmio.c

diff --git a/xen/arch/riscv/Makefile b/xen/arch/riscv/Makefile
index 3b948c11dd61..ce6410a299a4 100644
--- a/xen/arch/riscv/Makefile
+++ b/xen/arch/riscv/Makefile
@@ -14,6 +14,7 @@ obj-y += intc.o
 obj-y += irq.o
 obj-y += kernel.init.o
 obj-y += mm.o
+obj-y += mmio.o
 obj-y += p2m.o
 obj-y += paging.o
 obj-y += pt.o
diff --git a/xen/arch/riscv/domain.c b/xen/arch/riscv/domain.c
index 57c37cb2dfc2..ec327a5e8a23 100644
--- a/xen/arch/riscv/domain.c
+++ b/xen/arch/riscv/domain.c
@@ -12,6 +12,7 @@
 #include <asm/cpufeature.h>
 #include <asm/csr.h>
 #include <asm/intc.h>
+#include <asm/mmio.h>
 #include <asm/riscv_encoding.h>
 #include <asm/vtimer.h>
 
@@ -316,6 +317,8 @@ int arch_domain_create(struct domain *d,
     if ( (rc = p2m_init(d, config)) != 0)
         goto fail;
 
+    domain_io_init(d);
+
     if ( (rc = domain_vintc_init(d)) )
         goto fail;
 
diff --git a/xen/arch/riscv/include/asm/domain.h 
b/xen/arch/riscv/include/asm/domain.h
index e035b33ddfdc..15e8fa19685e 100644
--- a/xen/arch/riscv/include/asm/domain.h
+++ b/xen/arch/riscv/include/asm/domain.h
@@ -9,6 +9,7 @@
 
 #include <asm/cpufeature.h>
 #include <asm/guest-layout.h>
+#include <asm/mmio.h>
 #include <asm/p2m.h>
 #include <asm/vtimer.h>
 
@@ -101,6 +102,8 @@ struct arch_domain {
     const unsigned long *isa;
 
     struct vintc *vintc;
+
+    struct vmmio vmmio;
 };
 
 #include <xen/sched.h>
diff --git a/xen/arch/riscv/include/asm/mmio.h 
b/xen/arch/riscv/include/asm/mmio.h
new file mode 100644
index 000000000000..582969e5351b
--- /dev/null
+++ b/xen/arch/riscv/include/asm/mmio.h
@@ -0,0 +1,63 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+#ifndef RISCV_MMIO_H
+#define RISCV_MMIO_H
+
+#include <xen/lib.h>
+#include <xen/rwlock.h>
+
+struct domain;
+struct vcpu;
+
+#define MAX_IO_HANDLER  16
+
+typedef struct {
+    paddr_t gpa;
+    unsigned int len;  /* access width in bytes (1, 2, 4, 8) */
+    bool is_write;
+    /* store: value to write; load: value read (set by handler) */
+    register_t data;
+} mmio_info_t;
+
+enum io_state
+{
+    IO_ABORT,       /* The IO was handled and led to an abort. */
+    IO_HANDLED,     /* The IO was successfully handled. */
+    IO_UNHANDLED,   /* No handler found for the IO. */
+};
+
+typedef enum io_state (mmio_read_t)(struct vcpu *v, mmio_info_t *info);
+typedef enum io_state (mmio_write_t)(struct vcpu *v, const mmio_info_t *info);
+
+struct mmio_handler_ops {
+    mmio_read_t *read;
+    mmio_write_t *write;
+};
+
+struct mmio_handler {
+    paddr_t addr;
+    paddr_t size;
+    const struct mmio_handler_ops *ops;
+};
+
+struct vmmio {
+    unsigned int num_entries;
+    rwlock_t lock;
+    struct mmio_handler handlers[MAX_IO_HANDLER];
+};
+
+int do_mmio(mmio_info_t *info, paddr_t fault_addr, unsigned int len);
+int register_mmio_handler(struct domain *d,
+                          const struct mmio_handler_ops *ops,
+                          paddr_t addr, paddr_t size);
+void domain_io_init(struct domain *d);
+
+#endif /* RISCV_MMIO_H */
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/arch/riscv/mmio.c b/xen/arch/riscv/mmio.c
new file mode 100644
index 000000000000..d241ab5ea13d
--- /dev/null
+++ b/xen/arch/riscv/mmio.c
@@ -0,0 +1,176 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+
+#include <xen/bsearch.h>
+#include <xen/lib.h>
+#include <xen/rwlock.h>
+#include <xen/sched.h>
+#include <xen/string.h>
+
+#include <asm/current.h>
+#include <asm/mmio.h>
+
+/*
+ * bsearch() comparator: @key holds the address to look up in its addr field,
+ * @elem is an entry of vmmio->handlers. Relies on the regions not
+ * overlapping, which register_mmio_handler() enforces.
+ */
+static int cmp_mmio_handler(const void *key, const void *elem)
+{
+    const struct mmio_handler *handler0 = key;
+    const struct mmio_handler *handler1 = elem;
+
+    if ( handler0->addr < handler1->addr )
+        return -1;
+
+    if ( handler0->addr >= (handler1->addr + handler1->size) )
+        return 1;
+
+    return 0;
+}
+
+/*
+ * Return a copy of the matching handler rather than a pointer into
+ * vmmio->handlers: a concurrent register_mmio_handler() shifts entries
+ * up to keep the array sorted, so an escaped pointer could refer to a
+ * different (or torn) entry once the lock is dropped. The copy stays
+ * valid as the ops structures are never freed.
+ */
+static bool find_mmio_handler(struct domain *d, paddr_t gpa,
+                              struct mmio_handler *out)
+{
+    struct vmmio *vmmio = &d->arch.vmmio;
+    struct mmio_handler key = { .addr = gpa };
+    const struct mmio_handler *handler;
+
+    read_lock(&vmmio->lock);
+    handler = bsearch(&key, vmmio->handlers, vmmio->num_entries,
+                      sizeof(*handler), cmp_mmio_handler);
+    if ( handler )
+        *out = *handler;
+    read_unlock(&vmmio->lock);
+
+    return handler != NULL;
+}
+
+static enum io_state try_handle_mmio(mmio_info_t *info)
+{
+    struct vcpu *v = current;
+    struct mmio_handler handler = {};
+
+    if ( !find_mmio_handler(v->domain, info->gpa, &handler) )
+        return IO_UNHANDLED;
+
+    if ( info->is_write )
+        return handler.ops->write(v, info);
+    else
+        return handler.ops->read(v, info);
+}
+
+/*
+ * Check alignment and dispatch a decoded MMIO access to a registered
+ * handler. On success (0), info->data holds the read value for loads.
+ *
+ * There is no "retry" outcome to handle: find_mmio_handler() returns a
+ * copy of the matching handler taken under vmmio->lock and the ops
+ * structures are never freed, so the lookup result cannot go stale
+ * between finding the handler and invoking it.
+ */
+int do_mmio(mmio_info_t *info, paddr_t fault_addr, unsigned int len)
+{
+    /* Fault address should be aligned to length of MMIO */
+    if ( fault_addr & (len - 1) )
+        return -EIO;
+
+    info->gpa = fault_addr;
+    info->len = len;
+
+    switch ( try_handle_mmio(info) )
+    {
+    case IO_HANDLED:
+        return 0;
+
+    case IO_ABORT:
+        return -EIO;
+
+    default:
+        return -EOPNOTSUPP;
+    }
+}
+
+int register_mmio_handler(struct domain *d,
+                          const struct mmio_handler_ops *ops,
+                          paddr_t addr, paddr_t size)
+{
+    struct vmmio *vmmio = &d->arch.vmmio;
+    struct mmio_handler *handlers = vmmio->handlers;
+    paddr_t end = addr + size;
+    unsigned int i;
+    int rc = 0;
+    bool overlap;
+
+    if ( !ops || !ops->read || !ops->write || !size || end < addr )
+        return -EINVAL;
+
+    write_lock(&vmmio->lock);
+
+    if ( vmmio->num_entries >= ARRAY_SIZE(vmmio->handlers) )
+    {
+        rc = -ENOSPC;
+        goto out;
+    }
+
+    /*
+     * The array is kept sorted by base address, so rather than appending and
+     * re-sorting, find the slot the new region belongs to and shift the tail
+     * up by one.
+     */
+    for ( i = vmmio->num_entries;
+          i > 0 && handlers[i - 1].addr > addr;
+          i-- )
+        /* Nothing */;
+
+    /*
+     * Regions are required not to overlap; check both neighbours. Their
+     * addr + size cannot overflow, as such regions are rejected above when
+     * they get registered.
+     */
+    overlap = (i > 0 && handlers[i - 1].addr + handlers[i - 1].size > addr) ||
+              (i < vmmio->num_entries && end > handlers[i].addr);
+
+    if ( overlap )
+    {
+        rc = -EEXIST;
+        goto out;
+    }
+
+    memmove(&handlers[i + 1], &handlers[i],
+            (vmmio->num_entries - i) * sizeof(*handlers));
+
+    handlers[i] = (struct mmio_handler){
+        .addr = addr,
+        .size = size,
+        .ops = ops,
+    };
+
+    vmmio->num_entries++;
+
+ out:
+    write_unlock(&vmmio->lock);
+
+    return rc;
+}
+
+void domain_io_init(struct domain *d)
+{
+    rwlock_init(&d->arch.vmmio.lock);
+    d->arch.vmmio.num_entries = 0;
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
-- 
2.55.0




 


Rackspace

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