2023-01-20 23:03:43

by Elliot Berman

[permalink] [raw]
Subject: [PATCH v9 23/27] virt: gunyah: Add IO handlers

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 <[email protected]>
Signed-off-by: Prakruthi Deepak Heragu <[email protected]>
Signed-off-by: Elliot Berman <[email protected]>
---
drivers/virt/gunyah/vm_mgr.c | 85 +++++++++++++++++++++++++++++++++++
drivers/virt/gunyah/vm_mgr.h | 4 ++
include/linux/gunyah_vm_mgr.h | 25 +++++++++++
3 files changed, 114 insertions(+)

diff --git a/drivers/virt/gunyah/vm_mgr.c b/drivers/virt/gunyah/vm_mgr.c
index ee593cf1b074..1dfe354bcc29 100644
--- a/drivers/virt/gunyah/vm_mgr.c
+++ b/drivers/virt/gunyah/vm_mgr.c
@@ -185,6 +185,91 @@ void ghvm_remove_resource_ticket(struct gunyah_vm *ghvm, struct gunyah_vm_resour
}
EXPORT_SYMBOL_GPL(ghvm_remove_resource_ticket);

+static inline bool gh_vm_io_handler_matches(struct gunyah_vm_io_handler *io_hdlr, u64 addr,
+ u64 len, u64 data)
+{
+ u64 mask = BIT_ULL(io_hdlr->len * BITS_PER_BYTE) - 1;
+
+ if (io_hdlr->addr != addr)
+ return false;
+
+ if (!io_hdlr->datamatch)
+ return true;
+
+ if (io_hdlr->len != len)
+ return false;
+
+ return (data & mask) == (io_hdlr->data & mask);
+}
+
+static struct gunyah_vm_io_handler *gh_vm_mgr_find_io_hdlr(struct gunyah_vm *ghvm, u64 addr,
+ u64 len, u64 data)
+{
+ struct gunyah_vm_io_handler *io_hdlr = NULL;
+ struct rb_node *root = NULL;
+
+ root = ghvm->mmio_handler_root.rb_node;
+ while (root) {
+ io_hdlr = rb_entry(root, struct gunyah_vm_io_handler, node);
+ if (addr < io_hdlr->addr)
+ root = root->rb_left;
+ else if (addr > io_hdlr->addr)
+ root = root->rb_right;
+ else if (gh_vm_io_handler_matches(io_hdlr, addr, len, data))
+ return io_hdlr;
+ }
+ return NULL;
+}
+
+int gh_vm_mgr_mmio_write(struct gunyah_vm *ghvm, u64 addr, u32 len, u64 data)
+{
+ struct gunyah_vm_io_handler *io_hdlr = NULL;
+
+ io_hdlr = gh_vm_mgr_find_io_hdlr(ghvm, addr, len, data);
+ if (!io_hdlr)
+ return -ENODEV;
+
+ if (io_hdlr->ops && io_hdlr->ops->write)
+ return io_hdlr->ops->write(io_hdlr, addr, len, data);
+
+ return -ENODEV;
+}
+EXPORT_SYMBOL_GPL(gh_vm_mgr_mmio_write);
+
+int gh_vm_mgr_add_io_handler(struct gunyah_vm *ghvm, struct gunyah_vm_io_handler *io_hdlr)
+{
+ struct rb_node **root, *parent = NULL;
+
+ if (io_hdlr->datamatch &&
+ (!io_hdlr->len || io_hdlr->len > (sizeof(io_hdlr->data) * BITS_PER_BYTE)))
+ return -EINVAL;
+
+ root = &ghvm->mmio_handler_root.rb_node;
+ while (*root) {
+ struct gunyah_vm_io_handler *curr = rb_entry(*root, struct gunyah_vm_io_handler,
+ node);
+
+ parent = *root;
+ if (io_hdlr->addr < curr->addr)
+ root = &((*root)->rb_left);
+ else if (io_hdlr->addr > curr->addr)
+ root = &((*root)->rb_right);
+ else
+ return -EEXIST;
+ }
+
+ rb_link_node(&io_hdlr->node, parent, root);
+ rb_insert_color(&io_hdlr->node, &ghvm->mmio_handler_root);
+ return 0;
+}
+EXPORT_SYMBOL_GPL(gh_vm_mgr_add_io_handler);
+
+void gh_vm_mgr_remove_io_handler(struct gunyah_vm *ghvm, struct gunyah_vm_io_handler *io_hdlr)
+{
+ rb_erase(&io_hdlr->node, &ghvm->mmio_handler_root);
+}
+EXPORT_SYMBOL_GPL(gh_vm_mgr_remove_io_handler);
+
static void ghvm_put(struct kref *kref)
{
struct gunyah_vm *ghvm = container_of(kref, struct gunyah_vm, kref);
diff --git a/drivers/virt/gunyah/vm_mgr.h b/drivers/virt/gunyah/vm_mgr.h
index f52350145010..eb17a2dda2a5 100644
--- a/drivers/virt/gunyah/vm_mgr.h
+++ b/drivers/virt/gunyah/vm_mgr.h
@@ -52,6 +52,8 @@ struct gunyah_vm {
struct list_head resources;
struct list_head resource_tickets;

+ struct rb_root mmio_handler_root;
+
struct list_head functions;
};

@@ -62,4 +64,6 @@ struct gunyah_vm_memory_mapping *gh_vm_mem_mapping_find(struct gunyah_vm *ghvm,
struct gunyah_vm_memory_mapping *gh_vm_mem_mapping_find_mapping(struct gunyah_vm *ghvm,
u64 gpa, u32 size);

+int gh_vm_mgr_mmio_write(struct gunyah_vm *ghvm, u64 addr, u32 len, u64 data);
+
#endif
diff --git a/include/linux/gunyah_vm_mgr.h b/include/linux/gunyah_vm_mgr.h
index 995a9b0eb6b7..dd719061f3bf 100644
--- a/include/linux/gunyah_vm_mgr.h
+++ b/include/linux/gunyah_vm_mgr.h
@@ -79,4 +79,29 @@ struct gunyah_vm_resource_ticket {
int ghvm_add_resource_ticket(struct gunyah_vm *ghvm, struct gunyah_vm_resource_ticket *ticket);
void ghvm_remove_resource_ticket(struct gunyah_vm *ghvm, struct gunyah_vm_resource_ticket *ticket);

+/*
+ * gunyah_vm_io_handler contains the info about an io device and its associated
+ * addr and the ops associated with the io device.
+ */
+struct gunyah_vm_io_handler {
+ struct rb_node node;
+ u64 addr;
+
+ bool datamatch;
+ u8 len;
+ u64 data;
+ struct gunyah_vm_io_handler_ops *ops;
+};
+
+/*
+ * gunyah_vm_io_handler_ops contains function pointers associated with an iodevice.
+ */
+struct gunyah_vm_io_handler_ops {
+ int (*read)(struct gunyah_vm_io_handler *io_dev, u64 addr, u32 len, u64 data);
+ int (*write)(struct gunyah_vm_io_handler *io_dev, u64 addr, u32 len, u64 data);
+};
+
+int gh_vm_mgr_add_io_handler(struct gunyah_vm *ghvm, struct gunyah_vm_io_handler *io_dev);
+void gh_vm_mgr_remove_io_handler(struct gunyah_vm *ghvm, struct gunyah_vm_io_handler *io_dev);
+
#endif
--
2.39.0


2023-02-06 10:47:07

by Srivatsa Vaddagiri

[permalink] [raw]
Subject: Re: [PATCH v9 23/27] virt: gunyah: Add IO handlers

* Elliot Berman <[email protected]> [2023-01-20 14:46:22]:

> +static inline bool gh_vm_io_handler_matches(struct gunyah_vm_io_handler *io_hdlr, u64 addr,
> + u64 len, u64 data)
> +{
> + u64 mask = BIT_ULL(io_hdlr->len * BITS_PER_BYTE) - 1;
> +
> + if (io_hdlr->addr != addr)

Isn't this test redundant (given that caller would have performed same test)?

> + return false;
> +
> + if (!io_hdlr->datamatch)
> + return true;
> +
> + if (io_hdlr->len != len)
> + return false;
> +
> + return (data & mask) == (io_hdlr->data & mask);
> +}
> +
> +static struct gunyah_vm_io_handler *gh_vm_mgr_find_io_hdlr(struct gunyah_vm *ghvm, u64 addr,
> + u64 len, u64 data)
> +{
> + struct gunyah_vm_io_handler *io_hdlr = NULL;
> + struct rb_node *root = NULL;
> +
> + root = ghvm->mmio_handler_root.rb_node;
> + while (root) {
> + io_hdlr = rb_entry(root, struct gunyah_vm_io_handler, node);
> + if (addr < io_hdlr->addr)
> + root = root->rb_left;
> + else if (addr > io_hdlr->addr)
> + root = root->rb_right;
> + else if (gh_vm_io_handler_matches(io_hdlr, addr, len, data))

In case of handler not matching, don't we need to modify root?
Otherwise we can be stuck in infinite loop here AFAICS.

> + return io_hdlr;
> + }
> + return NULL;
> +}

// snip

> +int gh_vm_mgr_add_io_handler(struct gunyah_vm *ghvm, struct gunyah_vm_io_handler *io_hdlr)
> +{
> + struct rb_node **root, *parent = NULL;
> +
> + if (io_hdlr->datamatch &&
> + (!io_hdlr->len || io_hdlr->len > (sizeof(io_hdlr->data) * BITS_PER_BYTE)))
> + return -EINVAL;
> +
> + root = &ghvm->mmio_handler_root.rb_node;
> + while (*root) {
> + struct gunyah_vm_io_handler *curr = rb_entry(*root, struct gunyah_vm_io_handler,
> + node);
> +
> + parent = *root;
> + if (io_hdlr->addr < curr->addr)
> + root = &((*root)->rb_left);
> + else if (io_hdlr->addr > curr->addr)
> + root = &((*root)->rb_right);
> + else

We should allow two io_handlers on the same addr, but with different data
matches I think.

> + return -EEXIST;
> + }
> +
> + rb_link_node(&io_hdlr->node, parent, root);
> + rb_insert_color(&io_hdlr->node, &ghvm->mmio_handler_root);
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(gh_vm_mgr_add_io_handler);

2023-02-07 03:59:50

by Elliot Berman

[permalink] [raw]
Subject: Re: [PATCH v9 23/27] virt: gunyah: Add IO handlers



On 2/6/2023 2:46 AM, Srivatsa Vaddagiri wrote:
> * Elliot Berman <[email protected]> [2023-01-20 14:46:22]:
>
>> +static inline bool gh_vm_io_handler_matches(struct gunyah_vm_io_handler *io_hdlr, u64 addr,
>> + u64 len, u64 data)
>> +{
>> + u64 mask = BIT_ULL(io_hdlr->len * BITS_PER_BYTE) - 1;
>> +
>> + if (io_hdlr->addr != addr)
>
> Isn't this test redundant (given that caller would have performed same test)?
>

Done.

>> + return false;
>> +
>> + if (!io_hdlr->datamatch)
>> + return true;
>> +
>> + if (io_hdlr->len != len)
>> + return false;
>> +
>> + return (data & mask) == (io_hdlr->data & mask);
>> +}
>> +
>> +static struct gunyah_vm_io_handler *gh_vm_mgr_find_io_hdlr(struct gunyah_vm *ghvm, u64 addr,
>> + u64 len, u64 data)
>> +{
>> + struct gunyah_vm_io_handler *io_hdlr = NULL;
>> + struct rb_node *root = NULL;
>> +
>> + root = ghvm->mmio_handler_root.rb_node;
>> + while (root) {
>> + io_hdlr = rb_entry(root, struct gunyah_vm_io_handler, node);
>> + if (addr < io_hdlr->addr)
>> + root = root->rb_left;
>> + else if (addr > io_hdlr->addr)
>> + root = root->rb_right;
>> + else if (gh_vm_io_handler_matches(io_hdlr, addr, len, data))
>
> In case of handler not matching, don't we need to modify root?
> Otherwise we can be stuck in infinite loop here AFAICS.
>

Done.

>> + return io_hdlr;
>> + }
>> + return NULL;
>> +}
>
> // snip
>
>> +int gh_vm_mgr_add_io_handler(struct gunyah_vm *ghvm, struct gunyah_vm_io_handler *io_hdlr)
>> +{
>> + struct rb_node **root, *parent = NULL;
>> +
>> + if (io_hdlr->datamatch &&
>> + (!io_hdlr->len || io_hdlr->len > (sizeof(io_hdlr->data) * BITS_PER_BYTE)))
>> + return -EINVAL;
>> +
>> + root = &ghvm->mmio_handler_root.rb_node;
>> + while (*root) {
>> + struct gunyah_vm_io_handler *curr = rb_entry(*root, struct gunyah_vm_io_handler,
>> + node);
>> +
>> + parent = *root;
>> + if (io_hdlr->addr < curr->addr)
>> + root = &((*root)->rb_left);
>> + else if (io_hdlr->addr > curr->addr)
>> + root = &((*root)->rb_right);
>> + else
>
> We should allow two io_handlers on the same addr, but with different data
> matches I think.
>

Done.

>> + return -EEXIST;
>> + }
>> +
>> + rb_link_node(&io_hdlr->node, parent, root);
>> + rb_insert_color(&io_hdlr->node, &ghvm->mmio_handler_root);
>> + return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(gh_vm_mgr_add_io_handler);

2023-02-07 12:19:47

by Srivatsa Vaddagiri

[permalink] [raw]
Subject: Re: [PATCH v9 23/27] virt: gunyah: Add IO handlers

* Elliot Berman <[email protected]> [2023-02-06 19:59:30]:

> > > +int gh_vm_mgr_add_io_handler(struct gunyah_vm *ghvm, struct gunyah_vm_io_handler *io_hdlr)
> > > +{
> > > + struct rb_node **root, *parent = NULL;
> > > +
> > > + if (io_hdlr->datamatch &&
> > > + (!io_hdlr->len || io_hdlr->len > (sizeof(io_hdlr->data) * BITS_PER_BYTE)))


io_hdlr->len represents length in bytes AFAICS so the above test should be:

(!io_hdlr->len || io_hdlr->len > (sizeof(io_hdlr->data) )))

?