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

Oleksii Kurochko <[email protected]>
Newsgroups gmane.comp.emulators.xen.devel
Message-ID <a74ff916f76fb95e1e44bfadd45fbc477df00c51.1787838835.git.oleksii.kurochko@gmail.com>
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 <[email protected]>
---
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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.