[v11,21/26] virt: gunyah: Add IO handlers
Commit Message
Add framework for VM functions to handle stage-2 write faults from Gunyah
guest virtual machines. IO handlers have a range of addresses which they
apply to. Optionally, they may apply to only when the value written
matches the IO handler's value.
Co-developed-by: Prakruthi Deepak Heragu <quic_pheragu@quicinc.com>
Signed-off-by: Prakruthi Deepak Heragu <quic_pheragu@quicinc.com>
Signed-off-by: Elliot Berman <quic_eberman@quicinc.com>
---
drivers/virt/gunyah/vm_mgr.c | 94 +++++++++++++++++++++++++++++++++++
drivers/virt/gunyah/vm_mgr.h | 4 ++
include/linux/gunyah_vm_mgr.h | 25 ++++++++++
3 files changed, 123 insertions(+)
Comments
On 3/3/23 7:06 PM, Elliot Berman wrote:
> Add framework for VM functions to handle stage-2 write faults from Gunyah
> guest virtual machines. IO handlers have a range of addresses which they
> apply to. Optionally, they may apply to only when the value written
> matches the IO handler's value.
>
> Co-developed-by: Prakruthi Deepak Heragu <quic_pheragu@quicinc.com>
> Signed-off-by: Prakruthi Deepak Heragu <quic_pheragu@quicinc.com>
> Signed-off-by: Elliot Berman <quic_eberman@quicinc.com>
Two (related) bugs and a suggestion that might help avoid
adding the same problem in the future. (Or maybe I made
that suggestion elsewhere? Anyway, you'll see.)
-Alex
> ---
> drivers/virt/gunyah/vm_mgr.c | 94 +++++++++++++++++++++++++++++++++++
> drivers/virt/gunyah/vm_mgr.h | 4 ++
> include/linux/gunyah_vm_mgr.h | 25 ++++++++++
> 3 files changed, 123 insertions(+)
>
> diff --git a/drivers/virt/gunyah/vm_mgr.c b/drivers/virt/gunyah/vm_mgr.c
> index 0269bcdaf692..b31fac15ff45 100644
> --- a/drivers/virt/gunyah/vm_mgr.c
> +++ b/drivers/virt/gunyah/vm_mgr.c
> @@ -233,6 +233,100 @@ static void gh_vm_add_resource(struct gh_vm *ghvm, struct gh_resource *ghrsc)
> mutex_unlock(&ghvm->resources_lock);
> }
>
> +static int _gh_vm_io_handler_compare(const struct rb_node *node, const struct rb_node *parent)
> +{
> + struct gh_vm_io_handler *n = container_of(node, struct gh_vm_io_handler, node);
> + struct gh_vm_io_handler *p = container_of(parent, struct gh_vm_io_handler, node);
> +
> + if (n->addr < p->addr)
> + return -1;
> + if (n->addr > p->addr)
> + return 1;
> + if ((n->len && !p->len) || (!n->len && p->len))
> + return 0;
> + if (n->len < p->len)
> + return -1;
> + if (n->len > p->len)
> + return 1;
The datamatch field in a gh_vm_io_handler structure is Boolean.
If this is what you intend, it would be better to not treat
them as integer values (i.e., don't use < and >).
However I *think* what you want is to be comparing the
data fields here. If so, this is a BUG.
I think you should maybe use "data" in the gh_fn_ioeventfd_arg
structure rather than "datamatch". And then use "datamatch"
consistently as a Boolean indicating whether to do matching,
and "data" to be the value used in matching.
> + if (n->datamatch < p->datamatch)
> + return -1;
> + if (n->datamatch > p->datamatch)
> + return 1;
> + return 0;
> +}
> +
> +static int gh_vm_io_handler_compare(struct rb_node *node, const struct rb_node *parent)
> +{
> + return _gh_vm_io_handler_compare(node, parent);
> +}
> +
> +static int gh_vm_io_handler_find(const void *key, const struct rb_node *node)
> +{
> + const struct gh_vm_io_handler *k = key;
> +
> + return _gh_vm_io_handler_compare(&k->node, node);
> +}
> +
> +static struct gh_vm_io_handler *gh_vm_mgr_find_io_hdlr(struct gh_vm *ghvm, u64 addr,
> + u64 len, u64 data)
> +{
> + struct gh_vm_io_handler key = {
> + .addr = addr,
> + .len = len,
> + .datamatch = data,
The datamatch field here is Boolean. I'm pretty sure you
want to assign the data field instead, in which case, this
is a BUG.
If you *do* intend to treat the data assigned as Boolean,
please use !!data to make this obvious.
> + };
> + struct rb_node *node;
> +
> + node = rb_find(&key, &ghvm->mmio_handler_root, gh_vm_io_handler_find);
> + if (!node)
> + return NULL;
> +
> + return container_of(node, struct gh_vm_io_handler, node);
> +}
> +
> +int gh_vm_mmio_write(struct gh_vm *ghvm, u64 addr, u32 len, u64 data)
> +{
> + struct gh_vm_io_handler *io_hdlr = NULL;
> + int ret;
> +
> + down_read(&ghvm->mmio_handler_lock);
> + io_hdlr = gh_vm_mgr_find_io_hdlr(ghvm, addr, len, data);
> + if (!io_hdlr || !io_hdlr->ops || !io_hdlr->ops->write) {
> + ret = -ENODEV;
> + goto out;
> + }
> +
> + ret = io_hdlr->ops->write(io_hdlr, addr, len, data);
> +
> +out:
> + up_read(&ghvm->mmio_handler_lock);
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(gh_vm_mmio_write);
. . .
@@ -233,6 +233,100 @@ static void gh_vm_add_resource(struct gh_vm *ghvm, struct gh_resource *ghrsc)
mutex_unlock(&ghvm->resources_lock);
}
+static int _gh_vm_io_handler_compare(const struct rb_node *node, const struct rb_node *parent)
+{
+ struct gh_vm_io_handler *n = container_of(node, struct gh_vm_io_handler, node);
+ struct gh_vm_io_handler *p = container_of(parent, struct gh_vm_io_handler, node);
+
+ if (n->addr < p->addr)
+ return -1;
+ if (n->addr > p->addr)
+ return 1;
+ if ((n->len && !p->len) || (!n->len && p->len))
+ return 0;
+ if (n->len < p->len)
+ return -1;
+ if (n->len > p->len)
+ return 1;
+ if (n->datamatch < p->datamatch)
+ return -1;
+ if (n->datamatch > p->datamatch)
+ return 1;
+ return 0;
+}
+
+static int gh_vm_io_handler_compare(struct rb_node *node, const struct rb_node *parent)
+{
+ return _gh_vm_io_handler_compare(node, parent);
+}
+
+static int gh_vm_io_handler_find(const void *key, const struct rb_node *node)
+{
+ const struct gh_vm_io_handler *k = key;
+
+ return _gh_vm_io_handler_compare(&k->node, node);
+}
+
+static struct gh_vm_io_handler *gh_vm_mgr_find_io_hdlr(struct gh_vm *ghvm, u64 addr,
+ u64 len, u64 data)
+{
+ struct gh_vm_io_handler key = {
+ .addr = addr,
+ .len = len,
+ .datamatch = data,
+ };
+ struct rb_node *node;
+
+ node = rb_find(&key, &ghvm->mmio_handler_root, gh_vm_io_handler_find);
+ if (!node)
+ return NULL;
+
+ return container_of(node, struct gh_vm_io_handler, node);
+}
+
+int gh_vm_mmio_write(struct gh_vm *ghvm, u64 addr, u32 len, u64 data)
+{
+ struct gh_vm_io_handler *io_hdlr = NULL;
+ int ret;
+
+ down_read(&ghvm->mmio_handler_lock);
+ io_hdlr = gh_vm_mgr_find_io_hdlr(ghvm, addr, len, data);
+ if (!io_hdlr || !io_hdlr->ops || !io_hdlr->ops->write) {
+ ret = -ENODEV;
+ goto out;
+ }
+
+ ret = io_hdlr->ops->write(io_hdlr, addr, len, data);
+
+out:
+ up_read(&ghvm->mmio_handler_lock);
+ return ret;
+}
+EXPORT_SYMBOL_GPL(gh_vm_mmio_write);
+
+int gh_vm_add_io_handler(struct gh_vm *ghvm, struct gh_vm_io_handler *io_hdlr)
+{
+ struct rb_node *found;
+
+ if (io_hdlr->datamatch && (!io_hdlr->len || io_hdlr->len > sizeof(io_hdlr->data)))
+ return -EINVAL;
+
+ down_write(&ghvm->mmio_handler_lock);
+ found = rb_find_add(&io_hdlr->node, &ghvm->mmio_handler_root, gh_vm_io_handler_compare);
+ up_write(&ghvm->mmio_handler_lock);
+
+ return found ? -EEXIST : 0;
+}
+EXPORT_SYMBOL_GPL(gh_vm_add_io_handler);
+
+void gh_vm_remove_io_handler(struct gh_vm *ghvm, struct gh_vm_io_handler *io_hdlr)
+{
+ down_write(&ghvm->mmio_handler_lock);
+ rb_erase(&io_hdlr->node, &ghvm->mmio_handler_root);
+ up_write(&ghvm->mmio_handler_lock);
+}
+EXPORT_SYMBOL_GPL(gh_vm_remove_io_handler);
+
static int gh_vm_rm_notification_status(struct gh_vm *ghvm, void *data)
{
struct gh_rm_vm_status_payload *payload = data;
@@ -55,6 +55,8 @@ struct gh_vm {
struct mutex resources_lock;
struct list_head resources;
struct list_head resource_tickets;
+ struct rb_root mmio_handler_root;
+ struct rw_semaphore mmio_handler_lock;
};
int gh_vm_mem_alloc(struct gh_vm *ghvm, struct gh_userspace_memory_region *region);
@@ -63,4 +65,6 @@ int gh_vm_mem_free(struct gh_vm *ghvm, u32 label);
struct gh_vm_mem *gh_vm_mem_find_by_label(struct gh_vm *ghvm, u32 label);
struct gh_vm_mem *gh_vm_mem_find_by_addr(struct gh_vm *ghvm, u64 guest_phys_addr, u32 size);
+int gh_vm_mmio_write(struct gh_vm *ghvm, u64 addr, u32 len, u64 data);
+
#endif
@@ -84,4 +84,29 @@ struct gh_vm_resource_ticket {
int gh_vm_add_resource_ticket(struct gh_vm *ghvm, struct gh_vm_resource_ticket *ticket);
void gh_vm_remove_resource_ticket(struct gh_vm *ghvm, struct gh_vm_resource_ticket *ticket);
+/*
+ * gh_vm_io_handler contains the info about an io device and its associated
+ * addr and the ops associated with the io device.
+ */
+struct gh_vm_io_handler {
+ struct rb_node node;
+ u64 addr;
+
+ bool datamatch;
+ u8 len;
+ u64 data;
+ struct gh_vm_io_handler_ops *ops;
+};
+
+/*
+ * gh_vm_io_handler_ops contains function pointers associated with an iodevice.
+ */
+struct gh_vm_io_handler_ops {
+ int (*read)(struct gh_vm_io_handler *io_dev, u64 addr, u32 len, u64 data);
+ int (*write)(struct gh_vm_io_handler *io_dev, u64 addr, u32 len, u64 data);
+};
+
+int gh_vm_add_io_handler(struct gh_vm *ghvm, struct gh_vm_io_handler *io_dev);
+void gh_vm_remove_io_handler(struct gh_vm *ghvm, struct gh_vm_io_handler *io_dev);
+
#endif