On Mon, May 26, 2025 at 11:33 AM Sumit Garg <sumit.g...@kernel.org> wrote:
>
> On Mon, May 26, 2025 at 11:21:47AM +0200, Jens Wiklander wrote:
> > On Mon, May 26, 2025 at 9:22 AM Sumit Garg <sumit.g...@kernel.org> wrote:
> > >
> > > On Tue, May 20, 2025 at 05:16:49PM +0200, Jens Wiklander wrote:
> > > > Add tee_shm_alloc_dma_mem() to allocate DMA memory. The memory is
> > > > represented by a tee_shm object using the new flag TEE_SHM_DMA_MEM to
> > > > identify it as DMA memory. The allocated memory will later be lent to
> > > > the TEE to be used as protected memory.
> > > >
> > > > Signed-off-by: Jens Wiklander <jens.wiklan...@linaro.org>
> > > > ---
> > > > drivers/tee/tee_shm.c | 74 ++++++++++++++++++++++++++++++++++++++--
> > > > include/linux/tee_core.h | 5 +++
> > > > 2 files changed, 77 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/drivers/tee/tee_shm.c b/drivers/tee/tee_shm.c
> > > > index e1ed52ee0a16..92a6a35e1a1e 100644
> > > > --- a/drivers/tee/tee_shm.c
> > > > +++ b/drivers/tee/tee_shm.c
> > > > @@ -5,6 +5,8 @@
> > > > #include <linux/anon_inodes.h>
> > > > #include <linux/device.h>
> > > > #include <linux/dma-buf.h>
> > > > +#include <linux/dma-mapping.h>
> > > > +#include <linux/highmem.h>
> > > > #include <linux/idr.h>
> > > > #include <linux/io.h>
> > > > #include <linux/mm.h>
> > > > @@ -13,9 +15,14 @@
> > > > #include <linux/tee_core.h>
> > > > #include <linux/uaccess.h>
> > > > #include <linux/uio.h>
> > > > -#include <linux/highmem.h>
> > > > #include "tee_private.h"
> > > >
> > > > +struct tee_shm_dma_mem {
> > > > + struct tee_shm shm;
> > > > + dma_addr_t dma_addr;
> > > > + struct page *page;
> > > > +};
> > > > +
> > > > static void shm_put_kernel_pages(struct page **pages, size_t
> > > > page_count)
> > > > {
> > > > size_t n;
> > > > @@ -49,7 +56,14 @@ static void tee_shm_release(struct tee_device
> > > > *teedev, struct tee_shm *shm)
> > > > struct tee_shm *parent_shm = NULL;
> > > > void *p = shm;
> > > >
> > > > - if (shm->flags & TEE_SHM_DMA_BUF) {
> > > > + if (shm->flags & TEE_SHM_DMA_MEM) {
> > > > + struct tee_shm_dma_mem *dma_mem;
> > > > +
> > > > + dma_mem = container_of(shm, struct tee_shm_dma_mem, shm);
> > > > + p = dma_mem;
> > > > + dma_free_pages(&teedev->dev, shm->size, dma_mem->page,
> > > > + dma_mem->dma_addr, DMA_BIDIRECTIONAL);
> > >
> > > Although the kernel bot already found a randconfig issue, it looks like
> > > we need to add Kconfig dependencies like HAS_DMA, DMA_CMA etc.
> > >
> > > Also, I was thinking if we should rather add a new TEE subsystem
> > > specific Kconfig option like: TEE_DMABUF_HEAPS which can then be used to
> > > select whatever dependency is needed as well as act as a gating Kconfig
> > > for relevant features.
> >
> > You mean something like this?
> >
> > --- a/drivers/tee/Kconfig
> > +++ b/drivers/tee/Kconfig
> > @@ -13,6 +13,14 @@ menuconfig TEE
> >
> > if TEE
> >
> > +config TEE_DMABUF_HEAPS
> > + bool
> > + depends on HAS_DMA && DMABUF_HEAPS
>
> Yeah this looks fine to me but needs to be tested if DMA_CMA is a
> dependency here too.
Why? It can work without CMA for small allocations.
>
> > +
> > +config TEE_STATIC_PROTMEM_POOL
> > + bool
> > + depends on HAS_IOMEM && TEE_DMABUF_HEAPS
>
> The static and dynamic protected memory pools should get auto enabled if
> TEE_DMABUF_HEAPS is enabled since they are pre-requisite to provide the
> protected heaps support. Something like:
>
> +config TEE_STATIC_PROTMEM_POOL
> + bool
> + default y if TEE_DMABUF_HEAPS
> + depends on HAS_IOMEM
Right, I'll update as needed.
Cheers,
Jens
