Re: [PATCH v6 02/13] firmware: qcom: add a dedicated TrustZone buffer allocator

[Bartosz Golaszewski]

On Sat, Dec 16, 2023 at 8:00 PM Om Prakash Singh
<quic_omprsing@xxxxxxxxxxx> wrote:
>
>
>
> On 11/27/2023 7:45 PM, 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>
> > Reviewed-by: Andrew Halaney <ahalaney@xxxxxxxxxx>
> > Tested-by: Andrew Halaney <ahalaney@xxxxxxxxxx> # sc8280xp-lenovo-thinkpad-x13s
> > ---

[snip]

> > +
> > +#include "qcom_tzmem.h"
> > +
> > +struct qcom_tzmem_pool {
> > +     void *vbase;
> > +     dma_addr_t pbase;
> > +     size_t size;
> > +     struct gen_pool *pool;
> > +     void *priv;
> priv is not being using any where.

It's here for other implementations of the TZMem pools to store
private data under. SHM Bridge uses it in one of the later patches in
this series but it makes more logical sense to put it here.

[snip]

> > diff --git a/include/linux/firmware/qcom/qcom_tzmem.h b/include/linux/firmware/qcom/qcom_tzmem.h
> > new file mode 100644
> > index 000000000000..8e7fddab8cb4
> > --- /dev/null
> > +++ b/include/linux/firmware/qcom/qcom_tzmem.h
> > @@ -0,0 +1,28 @@
> > +/* SPDX-License-Identifier: GPL-2.0-or-later */
> > +/*
> > + * Copyright (C) 2023 Linaro Ltd.
> > + */
> > +
> > +#ifndef __QCOM_TZMEM_H
> > +#define __QCOM_TZMEM_H
> > +
> > +#include <linux/cleanup.h>
> > +#include <linux/gfp.h>
> > +#include <linux/types.h>
> > +
> > +struct device;
> > +struct qcom_tzmem_pool;
> qcom_tzmem_pool stucture definition should be moved to header file.

No, why would you do this?

Bart

[snip]