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

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

 





On 2/6/2023 2:46 AM, Srivatsa Vaddagiri wrote:
* Elliot Berman <quic_eberman@xxxxxxxxxxx> [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);



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux