lkml.org 
[lkml]   [2023]   [Oct]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [PATCH v3 04/15] firmware: qcom: add a dedicated TrustZone buffer allocator
    On Mon, Oct 9, 2023 at 11:28 PM Andrew Halaney <ahalaney@redhat.com> wrote:
    >
    > On Mon, Oct 09, 2023 at 05:34:16PM +0200, Bartosz Golaszewski wrote:
    > > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
    > >
    > > We have several SCM calls that require passing buffers to the TrustZone
    > > on top of the SMC core which allocates memory for calls that require
    > > more than 4 arguments.
    > >
    > > Currently every user does their own thing which leads to code
    > > duplication. Many users call dma_alloc_coherent() for every call which
    > > is terribly unperformant (speed- and size-wise).
    > >
    > > Provide a set of library functions for creating and managing pool of
    > > memory which is suitable for sharing with the TrustZone, that is:
    > > page-aligned, contiguous and non-cachable as well as provides a way of
    > > mapping of kernel virtual addresses to physical space.
    > >
    > > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
    > > ---

    [snip]

    >
    > I got these warnings with this series:
    >
    > ahalaney@fedora ~/git/linux-next (git)-[7204cc6c3d73] % ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- make W=1 C=2 drivers/firmware/qcom/
    > drivers/firmware/qcom/qcom_tzmem.c:137: warning: Function parameter or member 'size' not described in 'qcom_tzmem_pool_new'
    > CHECK drivers/firmware/qcom/qcom_tzmem.c
    > drivers/firmware/qcom/qcom_tzmem.c:204:17: warning: incorrect type in assignment (different address spaces)
    > drivers/firmware/qcom/qcom_tzmem.c:204:17: expected void **slot
    > drivers/firmware/qcom/qcom_tzmem.c:204:17: got void [noderef] __rcu **
    > drivers/firmware/qcom/qcom_tzmem.c:204:17: warning: incorrect type in assignment (different address spaces)
    > drivers/firmware/qcom/qcom_tzmem.c:204:17: expected void **slot
    > drivers/firmware/qcom/qcom_tzmem.c:204:17: got void [noderef] __rcu **
    > drivers/firmware/qcom/qcom_tzmem.c:204:17: warning: incorrect type in argument 1 (different address spaces)
    > drivers/firmware/qcom/qcom_tzmem.c:204:17: expected void [noderef] __rcu **slot
    > drivers/firmware/qcom/qcom_tzmem.c:204:17: got void **slot
    > drivers/firmware/qcom/qcom_tzmem.c:204:17: warning: incorrect type in assignment (different address spaces)
    > drivers/firmware/qcom/qcom_tzmem.c:204:17: expected void **slot
    > drivers/firmware/qcom/qcom_tzmem.c:204:17: got void [noderef] __rcu **
    > drivers/firmware/qcom/qcom_tzmem.c:339:13: warning: context imbalance in 'qcom_tzmem_to_phys' - wrong count at exit
    >
    >
    > All are confusing me, size seems described, I don't know much about
    > radix tree usage / rcu, and the locking in qcom_tzmem_to_phys seems sane
    > to me but I'm still grappling with the new syntax.
    >
    > For the one address space one, I _think_ maybe a diff like this is in
    > order?
    >
    > diff --git a/drivers/firmware/qcom/qcom_tzmem.c b/drivers/firmware/qcom/qcom_tzmem.c
    > index b3137844fe43..5b409615198d 100644
    > --- a/drivers/firmware/qcom/qcom_tzmem.c
    > +++ b/drivers/firmware/qcom/qcom_tzmem.c
    > @@ -193,7 +193,7 @@ void qcom_tzmem_pool_free(struct qcom_tzmem_pool *pool)
    > struct qcom_tzmem_chunk *chunk;
    > struct radix_tree_iter iter;
    > bool non_empty = false;
    > - void **slot;
    > + void __rcu **slot;
    >
    > if (!pool)
    > return;
    > @@ -202,7 +202,7 @@ void qcom_tzmem_pool_free(struct qcom_tzmem_pool *pool)
    >
    > scoped_guard(spinlock_irqsave, &qcom_tzmem_chunks_lock) {
    > radix_tree_for_each_slot(slot, &qcom_tzmem_chunks, &iter, 0) {
    > - chunk = *slot;
    > + chunk = radix_tree_deref_slot_protected(slot, &qcom_tzmem_chunks_lock);
    >
    > if (chunk->owner == pool)
    > non_empty = true;
    >

    Ah, I was thinking about it but then figured that I already use a
    spinlock and I didn't see these errors on my side so decided to
    dereference it normally.

    I'll check it again.

    Bart

    >
    > Still planning on reviewing/testing the rest, but got tripped up there
    > so thought I'd highlight it before doing the rest.
    >
    > Thanks,
    > Andrew
    >

    \
     
     \ /
      Last update: 2023-10-10 08:44    [W:3.134 / U:0.112 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site