[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20230206104637.GG332@quicinc.com>
Date: Mon, 6 Feb 2023 16:16:37 +0530
From: Srivatsa Vaddagiri <quic_svaddagi@...cinc.com>
To: Elliot Berman <quic_eberman@...cinc.com>
CC: Bjorn Andersson <quic_bjorande@...cinc.com>,
Alex Elder <elder@...aro.org>,
Murali Nalajala <quic_mnalajal@...cinc.com>,
Trilok Soni <quic_tsoni@...cinc.com>,
Carl van Schaik <quic_cvanscha@...cinc.com>,
Prakruthi Deepak Heragu <quic_pheragu@...cinc.com>,
Dmitry Baryshkov <dmitry.baryshkov@...aro.org>,
Arnd Bergmann <arnd@...db.de>,
"Greg Kroah-Hartman" <gregkh@...uxfoundation.org>,
Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
Jonathan Corbet <corbet@....net>,
Bagas Sanjaya <bagasdotme@...il.com>,
Catalin Marinas <catalin.marinas@....com>,
Will Deacon <will@...nel.org>, Marc Zyngier <maz@...nel.org>,
Jassi Brar <jassisinghbrar@...il.com>,
Sudeep Holla <sudeep.holla@....com>,
<linux-arm-msm@...r.kernel.org>, <devicetree@...r.kernel.org>,
<linux-kernel@...r.kernel.org>, <linux-doc@...r.kernel.org>,
<linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH v9 23/27] virt: gunyah: Add IO handlers
* Elliot Berman <quic_eberman@...cinc.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);
Powered by blists - more mailing lists