Re: [PATCH] tee: add FOLL_LONGTERM for CMA case when alloc shm

[Xiaoming Ding (丁晓明)]

From 35fd062d5cbc4d182eee0183843cd6350d126788 Mon Sep 17 00:00:00 2001
From: Xiaoming Ding <xiaoming.ding@xxxxxxxxxxxx>
Date: Wed, 10 May 2023 10:15:23 +0800
Subject: [PATCH v2] tee: add FOLL_LONGTERM for CMA case when alloc shm

CMA is widely used on insufficient memory platform for 
secure media playback case, and FOLL_LONGTERM will 
avoid tee_shm alloc pages from CMA region.
without FOLL_LONGTERM, CMA region may alloc failed since 
tee_shm has a chance to use it in advance.

modify is verified on OPTEE XTEST and kinds of secure + clear playback 


Fixes: 033ddf12bcf5 ("tee: add register user memory")
Signed-off-by: Xiaoming Ding <xiaoming.ding@xxxxxxxxxxxx>
---
v1 -> v2: take off the ifdef and apply FOLL_LONGTERM by default

 drivers/tee/tee_shm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tee/tee_shm.c b/drivers/tee/tee_shm.c
index 673cf0359494..38878e549ca4 100644
--- a/drivers/tee/tee_shm.c
+++ b/drivers/tee/tee_shm.c
@@ -257,7 +257,7 @@ register_shm_helper(struct tee_context *ctx,
unsigned long addr,
 	}
 
 	if (flags & TEE_SHM_USER_MAPPED)
-		rc = pin_user_pages_fast(start, num_pages, FOLL_WRITE,
+		rc = pin_user_pages_fast(start, num_pages, FOLL_WRITE |
FOLL_LONGTERM,
 					 shm->pages);
 	else
 		rc = shm_get_kernel_pages(start, num_pages, shm-
>pages);
-- 
2.18.0

On Wed, 2023-05-17 at 15:49 +0530, Sumit Garg wrote:
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> 
> 
> On Wed, 17 May 2023 at 15:06, Christoph Hellwig <hch@xxxxxxxxxxxxx>
> wrote:
> > 
> > On Wed, May 17, 2023 at 02:56:13PM +0530, Sumit Garg wrote:
> > > Do you mean a pinned user-space page can be paged out
> > > automatically?
> > 
> > No, pinned pages can't be paged out.
> > 
> > But a short term pin implies it will be release after a short
> > delay,
> > and it is feasible for wait for the pin to go away.
> 
> Okay, I see. I would be interested to know the ranges for that short
> delay. I guess it may depend on how much memory pressure there is...
> 
> > 
> > For a long term pin waiting is not an option, and anyone wanting to
> > do something with the pinned page that requires it to not be pinned
> > must simply give up.
> > 
> > > Just FYI, the underlying use-case for TEE registered shared
> > > memory is
> > > that the references to pinned pages are provided to TEE
> > > implementation
> > > to operate upon. This can happen over multiple syscalls and we
> > > want
> > > the pinned pages to be always in RAM as otherwise the physical
> > > addresses may change if they are paged out in between.
> > 
> > That's a very use clear case for a long term pin.
> 
> ...however, thanks for the insights.
> 
> @Xiaoming,
> 
> Please use the following fixes tag for the v2 along with extending
> the
> commit description regarding the reliability provided by the long
> term
> flag.
> 
> Fixes: 033ddf12bcf5 ("tee: add register user memory")
> 
> -Sumit