On Mon, Oct 9, 2023 at 11:28 PM Andrew Halaney <ahalaney@xxxxxxxxxx> wrote:
>
> On Mon, Oct 09, 2023 at 05:34:16PM +0200, Bartosz Golaszewski wrote:
> > From: Bartosz Golaszewski <bartosz.golaszewski@xxxxxxxxxx>
> >
> > 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@xxxxxxxxxx>
> > ---
[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
I fixed the other ones but this one I think comes from CHECK not
dealing correctly with the spinlock guard.
>
>
> 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);
We need to keep the lock taken for the duration of the looping so we
can use the regular radix_tree_deref_slot().
Bart
>
> if (chunk->owner == pool)
> non_empty = true;
>
>
> Still planning on reviewing/testing the rest, but got tripped up there
> so thought I'd highlight it before doing the rest.
>
> Thanks,
> Andrew
>
