diff mbox series

[v9,23/27] virt: gunyah: Add IO handlers

Message ID 20230120224627.4053418-24-quic_eberman@quicinc.com
State New
Headers show
Series Drivers for gunyah hypervisor | expand

Commit Message

Elliot Berman Jan. 20, 2023, 10:46 p.m. UTC 
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  | 85 +++++++++++++++++++++++++++++++++++
 drivers/virt/gunyah/vm_mgr.h  |  4 ++
 include/linux/gunyah_vm_mgr.h | 25 +++++++++++
 3 files changed, 114 insertions(+)

Comments

Srivatsa Vaddagiri Feb. 6, 2023, 10:46 a.m. UTC | #1 
* Elliot Berman <quic_eberman@quicinc.com> [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);
Elliot Berman Feb. 7, 2023, 3:59 a.m. UTC | #2 
On 2/6/2023 2:46 AM, Srivatsa Vaddagiri wrote:
> * Elliot Berman <quic_eberman@quicinc.com> [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);
Srivatsa Vaddagiri Feb. 7, 2023, 12:19 p.m. UTC | #3 
* Elliot Berman <quic_eberman@quicinc.com> [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) )))

?
diff mbox series

Patch

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