| Message ID | 20230523091350.292221-2-arnaud.pouliquen@foss.st.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | introduction of a remoteproc tee to load signed firmware images | expand |
On Tue, May 23, 2023 at 11:13:47AM +0200, Arnaud Pouliquen wrote:
> This patch revert commit c83900393aa1 ("tee: Remove vmalloc page support")
As per the discussion back then: don't just blindly do the same dumb
thing again and fix the interfae to actually pass in a page array,
or iov_iter or an actually useful container that fits.
Hello Christoph, On 5/24/23 08:46, Christoph Hellwig wrote: > On Tue, May 23, 2023 at 11:13:47AM +0200, Arnaud Pouliquen wrote: >> This patch revert commit c83900393aa1 ("tee: Remove vmalloc page support") > > As per the discussion back then: don't just blindly do the same dumb > thing again and fix the interfae to actually pass in a page array, > or iov_iter or an actually useful container that fits. > I suppose your are speaking about this discussion: https://lore.kernel.org/all/20221002002326.946620-3-ira.weiny@intel.com/ If I'm not mistaken, I should modify at tee_shm_register_kernel_buf API and register_shm_helper inernal function, right? Seems that Jens has also pointed out the free part... What about having equivalent of shm_get_kernel_pages in an external helper (to defined where to put it), could it be an alternative of the upadate of the tee_shm API? Thanks, Arnaud
On Wed, May 24, 2023 at 04:01:14PM +0200, Arnaud POULIQUEN wrote: > > As per the discussion back then: don't just blindly do the same dumb > > thing again and fix the interfae to actually pass in a page array, > > or iov_iter or an actually useful container that fits. > > > > I suppose your are speaking about this discussion: > https://lore.kernel.org/all/20221002002326.946620-3-ira.weiny@intel.com/ Yes. > > If I'm not mistaken, I should modify at tee_shm_register_kernel_buf API and > register_shm_helper inernal function, right? > > What about having equivalent of shm_get_kernel_pages in an external helper (to > defined where to put it), could it be an alternative of the upadate of the > tee_shm API? I think the fundamentally right thing is to pass an iov_iter to register_shm_helper, and then use the new as of 6.3 iov_iter_extract_pages helper to extract the pages from that. For the kernel users you can then simply pass down an ITER_BVEC iter that you can fill with vmalloc pages if you want.
On 5/26/23 14:37, Christoph Hellwig wrote: > On Wed, May 24, 2023 at 04:01:14PM +0200, Arnaud POULIQUEN wrote: >>> As per the discussion back then: don't just blindly do the same dumb >>> thing again and fix the interfae to actually pass in a page array, >>> or iov_iter or an actually useful container that fits. >>> >> >> I suppose your are speaking about this discussion: >> https://lore.kernel.org/all/20221002002326.946620-3-ira.weiny@intel.com/ > > Yes. > >> >> If I'm not mistaken, I should modify at tee_shm_register_kernel_buf API and >> register_shm_helper inernal function, right? >> > >> What about having equivalent of shm_get_kernel_pages in an external helper (to >> defined where to put it), could it be an alternative of the upadate of the >> tee_shm API? > > I think the fundamentally right thing is to pass an iov_iter to > register_shm_helper, and then use the new as of 6.3 > iov_iter_extract_pages helper to extract the pages from that. For > the kernel users you can then simply pass down an ITER_BVEC iter > that you can fill with vmalloc pages if you want. > Thanks for the advice! Regards, Arnaud
diff --git a/drivers/tee/tee_shm.c b/drivers/tee/tee_shm.c index 673cf0359494..b2d349ac17b4 100644 --- a/drivers/tee/tee_shm.c +++ b/drivers/tee/tee_shm.c @@ -28,14 +28,26 @@ static int shm_get_kernel_pages(unsigned long start, size_t page_count, struct page *page; size_t n; - if (WARN_ON_ONCE(is_vmalloc_addr((void *)start) || - is_kmap_addr((void *)start))) + if (WARN_ON_ONCE(is_kmap_addr((void *)start))) return -EINVAL; - page = virt_to_page((void *)start); - for (n = 0; n < page_count; n++) { - pages[n] = page + n; - get_page(pages[n]); + if (is_vmalloc_addr((void *)start)) { + struct page *page; + + for (n = 0; n < page_count; n++) { + page = vmalloc_to_page((void *)(start + PAGE_SIZE * n)); + if (!page) + return -ENOMEM; + + get_page(page); + pages[n] = page; + } + } else { + page = virt_to_page((void *)start); + for (n = 0; n < page_count; n++) { + pages[n] = page + n; + get_page(pages[n]); + } } return page_count;
This patch revert commit c83900393aa1 ("tee: Remove vmalloc page support") The firmware framework uses vmalloc page to store an image of a firmware, got from the file system. To be able to give this firmware to OP-TEE without an extra copy, the vmalloc page support needs to be reintroduce. Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@foss.st.com> --- drivers/tee/tee_shm.c | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-)