From: Ira Weiny ira.weiny@intel.com
get_kernel_pages() only had one caller [shm_get_kernel_pages()] which did not need the functionality it provided. Furthermore, it called kmap_to_page() which we are looking to removed.
Alter shm_get_kernel_pages() to no longer call get_kernel_pages() and remove get_kernel_pages(). Along the way it was noted that shm_get_kernel_pages() does not have any need to support vmalloc'ed addresses either. Remove that functionality to clean up the logic.
This series also fixes and uses is_kmap_addr().
Ira Weiny (4): highmem: Enhance is_kmap_addr() to check kmap_local_page() mappings tee: Remove vmalloc page support tee: Remove call to get_kernel_pages() mm: Remove get_kernel_pages()
drivers/tee/tee_shm.c | 41 ++++++++++++-------------------- include/linux/highmem-internal.h | 5 +++- include/linux/mm.h | 2 -- mm/swap.c | 30 ----------------------- 4 files changed, 19 insertions(+), 59 deletions(-)
base-commit: 274d7803837da78dfc911bcda0d593412676fc20
From: Ira Weiny ira.weiny@intel.com
is_kmap_addr() is only looking at the kmap() address range which may cause check_heap_object() to miss checking an overflow on a kmap_local_page() page.
Add a check for the kmap_local_page() address range to is_kmap_addr().
Cc: Matthew Wilcox willy@infradead.org Cc: Al Viro viro@zeniv.linux.org.uk Cc: "Fabio M. De Francesco" fmdefrancesco@gmail.com Cc: Thomas Gleixner tglx@linutronix.de Cc: Christoph Hellwig hch@lst.de Cc: Andrew Morton akpm@linux-foundation.org Signed-off-by: Ira Weiny ira.weiny@intel.com --- include/linux/highmem-internal.h | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/include/linux/highmem-internal.h b/include/linux/highmem-internal.h index 034b1106d022..5fd5cb58f109 100644 --- a/include/linux/highmem-internal.h +++ b/include/linux/highmem-internal.h @@ -152,7 +152,10 @@ static inline void totalhigh_pages_add(long count) static inline bool is_kmap_addr(const void *x) { unsigned long addr = (unsigned long)x; - return addr >= PKMAP_ADDR(0) && addr < PKMAP_ADDR(LAST_PKMAP); + + return (addr >= PKMAP_ADDR(0) && addr < PKMAP_ADDR(LAST_PKMAP)) || + (addr >= __fix_to_virt(FIX_KMAP_END) && + addr < __fix_to_virt(FIX_KMAP_BEGIN)); } #else /* CONFIG_HIGHMEM */
From: Ira Weiny ira.weiny@intel.com
The kernel pages used by shm_get_kernel_pages() are allocated using GFP_KERNEL through the following call stack:
trusted_instantiate() trusted_payload_alloc() -> GFP_KERNEL <trusted key op> tee_shm_register_kernel_buf() register_shm_helper() shm_get_kernel_pages()
Where <trusted key op> is one of:
trusted_key_unseal() trusted_key_get_random() trusted_key_seal()
Remove the vmalloc page support from shm_get_kernel_pages(). Replace with a warn on once.
Cc: Jens Wiklander jens.wiklander@linaro.org Cc: Al Viro viro@zeniv.linux.org.uk Cc: "Fabio M. De Francesco" fmdefrancesco@gmail.com Cc: Christoph Hellwig hch@lst.de Cc: Linus Torvalds torvalds@linux-foundation.org Signed-off-by: Ira Weiny ira.weiny@intel.com
--- Jens I went with the suggestion from Linus and Christoph and rejected vmalloc addresses. I did not hear back from you regarding Linus' question if the vmalloc page support was required by an up coming patch set or not. So I assumed it was something out of tree. --- drivers/tee/tee_shm.c | 36 ++++++++++++------------------------ 1 file changed, 12 insertions(+), 24 deletions(-)
diff --git a/drivers/tee/tee_shm.c b/drivers/tee/tee_shm.c index 27295bda3e0b..527a6eabc03e 100644 --- a/drivers/tee/tee_shm.c +++ b/drivers/tee/tee_shm.c @@ -24,37 +24,25 @@ static void shm_put_kernel_pages(struct page **pages, size_t page_count) static int shm_get_kernel_pages(unsigned long start, size_t page_count, struct page **pages) { + struct kvec *kiov; size_t n; int rc;
- 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; - } - rc = page_count; - } else { - struct kvec *kiov; - - kiov = kcalloc(page_count, sizeof(*kiov), GFP_KERNEL); - if (!kiov) - return -ENOMEM; + if (WARN_ON_ONCE(is_vmalloc_addr((void *)start))) + return -EINVAL;
- for (n = 0; n < page_count; n++) { - kiov[n].iov_base = (void *)(start + n * PAGE_SIZE); - kiov[n].iov_len = PAGE_SIZE; - } + kiov = kcalloc(page_count, sizeof(*kiov), GFP_KERNEL); + if (!kiov) + return -ENOMEM;
- rc = get_kernel_pages(kiov, page_count, 0, pages); - kfree(kiov); + for (n = 0; n < page_count; n++) { + kiov[n].iov_base = (void *)(start + n * PAGE_SIZE); + kiov[n].iov_len = PAGE_SIZE; }
+ rc = get_kernel_pages(kiov, page_count, 0, pages); + kfree(kiov); + return rc; }
Hi Ira,
On Sun, Oct 2, 2022 at 2:23 AM ira.weiny@intel.com wrote:
From: Ira Weiny ira.weiny@intel.com
The kernel pages used by shm_get_kernel_pages() are allocated using GFP_KERNEL through the following call stack:
trusted_instantiate() trusted_payload_alloc() -> GFP_KERNEL <trusted key op> tee_shm_register_kernel_buf() register_shm_helper() shm_get_kernel_pages()
Where <trusted key op> is one of:
trusted_key_unseal() trusted_key_get_random() trusted_key_seal()Remove the vmalloc page support from shm_get_kernel_pages(). Replace with a warn on once.
Cc: Jens Wiklander jens.wiklander@linaro.org Cc: Al Viro viro@zeniv.linux.org.uk Cc: "Fabio M. De Francesco" fmdefrancesco@gmail.com Cc: Christoph Hellwig hch@lst.de Cc: Linus Torvalds torvalds@linux-foundation.org Signed-off-by: Ira Weiny ira.weiny@intel.com
Jens I went with the suggestion from Linus and Christoph and rejected vmalloc addresses. I did not hear back from you regarding Linus' question if the vmalloc page support was required by an up coming patch set or not. So I assumed it was something out of tree.
Yes, that's correctly assumed, sorry for not confirming that earlier.
Reviewed-by: Jens Wiklander jens.wiklander@linaro.org
Thanks, Jens
drivers/tee/tee_shm.c | 36 ++++++++++++------------------------ 1 file changed, 12 insertions(+), 24 deletions(-)
diff --git a/drivers/tee/tee_shm.c b/drivers/tee/tee_shm.c index 27295bda3e0b..527a6eabc03e 100644 --- a/drivers/tee/tee_shm.c +++ b/drivers/tee/tee_shm.c @@ -24,37 +24,25 @@ static void shm_put_kernel_pages(struct page **pages, size_t page_count) static int shm_get_kernel_pages(unsigned long start, size_t page_count, struct page **pages) {
struct kvec *kiov; size_t n; int rc;
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;}rc = page_count;} else {struct kvec *kiov;kiov = kcalloc(page_count, sizeof(*kiov), GFP_KERNEL);if (!kiov)return -ENOMEM;
if (WARN_ON_ONCE(is_vmalloc_addr((void *)start)))return -EINVAL;
for (n = 0; n < page_count; n++) {kiov[n].iov_base = (void *)(start + n * PAGE_SIZE);kiov[n].iov_len = PAGE_SIZE;}
kiov = kcalloc(page_count, sizeof(*kiov), GFP_KERNEL);if (!kiov)return -ENOMEM;
rc = get_kernel_pages(kiov, page_count, 0, pages);kfree(kiov);
for (n = 0; n < page_count; n++) {kiov[n].iov_base = (void *)(start + n * PAGE_SIZE);kiov[n].iov_len = PAGE_SIZE; }rc = get_kernel_pages(kiov, page_count, 0, pages);kfree(kiov);return rc;}
-- 2.37.2
+ Phil
Hi Ira,
On Sun, 2 Oct 2022 at 05:53, ira.weiny@intel.com wrote:
From: Ira Weiny ira.weiny@intel.com
The kernel pages used by shm_get_kernel_pages() are allocated using GFP_KERNEL through the following call stack:
trusted_instantiate() trusted_payload_alloc() -> GFP_KERNEL <trusted key op> tee_shm_register_kernel_buf() register_shm_helper() shm_get_kernel_pages()
Where <trusted key op> is one of:
trusted_key_unseal() trusted_key_get_random() trusted_key_seal()Remove the vmalloc page support from shm_get_kernel_pages(). Replace with a warn on once.
Cc: Jens Wiklander jens.wiklander@linaro.org Cc: Al Viro viro@zeniv.linux.org.uk Cc: "Fabio M. De Francesco" fmdefrancesco@gmail.com Cc: Christoph Hellwig hch@lst.de Cc: Linus Torvalds torvalds@linux-foundation.org Signed-off-by: Ira Weiny ira.weiny@intel.com
Jens I went with the suggestion from Linus and Christoph and rejected vmalloc addresses. I did not hear back from you regarding Linus' question if the vmalloc page support was required by an up coming patch set or not. So I assumed it was something out of tree.
It looks like I wasn't CC'd to that conversation. IIRC, support for vmalloc addresses was added recently by Phil here [1]. So I would like to give him a chance if he is planning to post a corresponding kernel driver upstream.
[1] https://lists.trustedfirmware.org/archives/list/op-tee@lists.trustedfirmware...
-Sumit
drivers/tee/tee_shm.c | 36 ++++++++++++------------------------ 1 file changed, 12 insertions(+), 24 deletions(-)
diff --git a/drivers/tee/tee_shm.c b/drivers/tee/tee_shm.c index 27295bda3e0b..527a6eabc03e 100644 --- a/drivers/tee/tee_shm.c +++ b/drivers/tee/tee_shm.c @@ -24,37 +24,25 @@ static void shm_put_kernel_pages(struct page **pages, size_t page_count) static int shm_get_kernel_pages(unsigned long start, size_t page_count, struct page **pages) {
struct kvec *kiov; size_t n; int rc;
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;}rc = page_count;} else {struct kvec *kiov;kiov = kcalloc(page_count, sizeof(*kiov), GFP_KERNEL);if (!kiov)return -ENOMEM;
if (WARN_ON_ONCE(is_vmalloc_addr((void *)start)))return -EINVAL;
for (n = 0; n < page_count; n++) {kiov[n].iov_base = (void *)(start + n * PAGE_SIZE);kiov[n].iov_len = PAGE_SIZE;}
kiov = kcalloc(page_count, sizeof(*kiov), GFP_KERNEL);if (!kiov)return -ENOMEM;
rc = get_kernel_pages(kiov, page_count, 0, pages);kfree(kiov);
for (n = 0; n < page_count; n++) {kiov[n].iov_base = (void *)(start + n * PAGE_SIZE);kiov[n].iov_len = PAGE_SIZE; }rc = get_kernel_pages(kiov, page_count, 0, pages);kfree(kiov);return rc;}
-- 2.37.2
From: Ira Weiny ira.weiny@intel.com
The kernel pages used by shm_get_kernel_pages() are allocated using GFP_KERNEL through the following call stack:
trusted_instantiate() trusted_payload_alloc() -> GFP_KERNEL <trusted key op> tee_shm_register_kernel_buf() register_shm_helper() shm_get_kernel_pages()
Where <trusted key op> is one of:
trusted_key_unseal() trusted_key_get_random() trusted_key_seal()
Because the pages can't be from highmem get_kernel_pages() boils down to a get_page() call.
Remove the get_kernel_pages() call and open code the get_page().
In case a kmap page does slip through warn on once for a kmap address.
Cc: Jens Wiklander jens.wiklander@linaro.org Cc: Al Viro viro@zeniv.linux.org.uk Cc: "Fabio M. De Francesco" fmdefrancesco@gmail.com Cc: Christoph Hellwig hch@lst.de Cc: Linus Torvalds torvalds@linux-foundation.org Signed-off-by: Ira Weiny ira.weiny@intel.com --- drivers/tee/tee_shm.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/tee/tee_shm.c b/drivers/tee/tee_shm.c index 527a6eabc03e..45e6ff1a452e 100644 --- a/drivers/tee/tee_shm.c +++ b/drivers/tee/tee_shm.c @@ -11,6 +11,7 @@ #include <linux/tee_drv.h> #include <linux/uaccess.h> #include <linux/uio.h> +#include <linux/highmem.h> #include "tee_private.h"
static void shm_put_kernel_pages(struct page **pages, size_t page_count) @@ -26,9 +27,9 @@ static int shm_get_kernel_pages(unsigned long start, size_t page_count, { struct kvec *kiov; size_t n; - int rc;
- if (WARN_ON_ONCE(is_vmalloc_addr((void *)start))) + if (WARN_ON_ONCE(is_vmalloc_addr((void *)start) || + is_kmap_addr((void *)start))) return -EINVAL;
kiov = kcalloc(page_count, sizeof(*kiov), GFP_KERNEL); @@ -38,12 +39,12 @@ static int shm_get_kernel_pages(unsigned long start, size_t page_count, for (n = 0; n < page_count; n++) { kiov[n].iov_base = (void *)(start + n * PAGE_SIZE); kiov[n].iov_len = PAGE_SIZE; + pages[n] = virt_to_page(kiov[n].iov_base); + get_page(pages[n]); } - - rc = get_kernel_pages(kiov, page_count, 0, pages); kfree(kiov);
- return rc; + return page_count; }
static void release_registered_pages(struct tee_shm *shm)
On Sat, Oct 01, 2022 at 05:23:25PM -0700, ira.weiny@intel.com wrote:
kiov = kcalloc(page_count, sizeof(*kiov), GFP_KERNEL); @@ -38,12 +39,12 @@ static int shm_get_kernel_pages(unsigned long start, size_t page_count, for (n = 0; n < page_count; n++) { kiov[n].iov_base = (void *)(start + n * PAGE_SIZE); kiov[n].iov_len = PAGE_SIZE;
pages[n] = virt_to_page(kiov[n].iov_base);get_page(pages[n]);
- rc = get_kernel_pages(kiov, page_count, 0, pages); kfree(kiov);
IDGI. The only thing in kiov[...] you are every reading is ->iov_base. And you fetch it once, right after the assignment.
Why bother with allocating the array at all? pages[n] = virt_to_page((void *)start + n * PAGE_SIZE); would do just as well, not to mention the fact that since you reject vmalloc and kmap, you might simply do
page = virt_to_page(start); for (int n = 0; n < page_count; n++) get_page(pages[n] = page + n);
instead...
On Sun, Oct 02, 2022 at 01:46:41AM +0100, Al Viro wrote:
On Sat, Oct 01, 2022 at 05:23:25PM -0700, ira.weiny@intel.com wrote:
kiov = kcalloc(page_count, sizeof(*kiov), GFP_KERNEL); @@ -38,12 +39,12 @@ static int shm_get_kernel_pages(unsigned long start, size_t page_count, for (n = 0; n < page_count; n++) { kiov[n].iov_base = (void *)(start + n * PAGE_SIZE); kiov[n].iov_len = PAGE_SIZE;
pages[n] = virt_to_page(kiov[n].iov_base);get_page(pages[n]);
- rc = get_kernel_pages(kiov, page_count, 0, pages); kfree(kiov);
IDGI. The only thing in kiov[...] you are every reading is ->iov_base. And you fetch it once, right after the assignment.
:-( Good point. Thanks for catching that. I was too focused on just replacing get_kernel_pages() with get_page() and I should have refactored more.
Why bother with allocating the array at all? pages[n] = virt_to_page((void *)start + n * PAGE_SIZE); would do just as well, not to mention the fact that since you reject vmalloc and kmap, you might simply do
page = virt_to_page(start); for (int n = 0; n < page_count; n++) get_page(pages[n] = page + n);
I think I'd avoid the assignment in the parameter as I would miss that if I came back and looked at this code later.
I'll get rid of the kiov in v2.
Sorry for not cleaning it up more and thanks for the review!
Ira
On Sun, Oct 02, 2022 at 01:46:41AM +0100, Al Viro wrote:
IDGI. The only thing in kiov[...] you are every reading is ->iov_base. And you fetch it once, right after the assignment.
... and that's exactly what I pointed out on the last version of the patch ...
On Mon, Oct 03, 2022 at 09:17:18AM +0200, Christoph Hellwig wrote:
On Sun, Oct 02, 2022 at 01:46:41AM +0100, Al Viro wrote:
IDGI. The only thing in kiov[...] you are every reading is ->iov_base. And you fetch it once, right after the assignment.
... and that's exactly what I pointed out on the last version of the patch ...
Apologies I guess I missed it. Ira
From: Ira Weiny ira.weiny@intel.com
The only caller to get_kernel_pages() [shm_get_kernel_pages()] has been updated to not need it.
Remove get_kernel_pages().
Cc: Mel Gorman mgorman@suse.de Cc: Al Viro viro@zeniv.linux.org.uk Cc: "Fabio M. De Francesco" fmdefrancesco@gmail.com Cc: Christoph Hellwig hch@lst.de Cc: Andrew Morton akpm@linux-foundation.org Signed-off-by: Ira Weiny ira.weiny@intel.com --- include/linux/mm.h | 2 -- mm/swap.c | 30 ------------------------------ 2 files changed, 32 deletions(-)
diff --git a/include/linux/mm.h b/include/linux/mm.h index 8bbcccbc5565..9a06df4f057c 100644 --- a/include/linux/mm.h +++ b/include/linux/mm.h @@ -1969,8 +1969,6 @@ int __account_locked_vm(struct mm_struct *mm, unsigned long pages, bool inc, struct task_struct *task, bool bypass_rlim);
struct kvec; -int get_kernel_pages(const struct kvec *iov, int nr_pages, int write, - struct page **pages); struct page *get_dump_page(unsigned long addr);
bool folio_mark_dirty(struct folio *folio); diff --git a/mm/swap.c b/mm/swap.c index 955930f41d20..a9aa648eb0d0 100644 --- a/mm/swap.c +++ b/mm/swap.c @@ -157,36 +157,6 @@ void put_pages_list(struct list_head *pages) } EXPORT_SYMBOL(put_pages_list);
-/* - * get_kernel_pages() - pin kernel pages in memory - * @kiov: An array of struct kvec structures - * @nr_segs: number of segments to pin - * @write: pinning for read/write, currently ignored - * @pages: array that receives pointers to the pages pinned. - * Should be at least nr_segs long. - * - * Returns number of pages pinned. This may be fewer than the number requested. - * If nr_segs is 0 or negative, returns 0. If no pages were pinned, returns 0. - * Each page returned must be released with a put_page() call when it is - * finished with. - */ -int get_kernel_pages(const struct kvec *kiov, int nr_segs, int write, - struct page **pages) -{ - int seg; - - for (seg = 0; seg < nr_segs; seg++) { - if (WARN_ON(kiov[seg].iov_len != PAGE_SIZE)) - return seg; - - pages[seg] = kmap_to_page(kiov[seg].iov_base); - get_page(pages[seg]); - } - - return seg; -} -EXPORT_SYMBOL_GPL(get_kernel_pages); - typedef void (*move_fn_t)(struct lruvec *lruvec, struct folio *folio);
static void lru_add_fn(struct lruvec *lruvec, struct folio *folio)
On 10/1/22 17:23, ira.weiny@intel.com wrote:
From: Ira Weiny ira.weiny@intel.com
The only caller to get_kernel_pages() [shm_get_kernel_pages()] has been updated to not need it.
Remove get_kernel_pages().
Cc: Mel Gorman mgorman@suse.de Cc: Al Viro viro@zeniv.linux.org.uk Cc: "Fabio M. De Francesco" fmdefrancesco@gmail.com Cc: Christoph Hellwig hch@lst.de Cc: Andrew Morton akpm@linux-foundation.org Signed-off-by: Ira Weiny ira.weiny@intel.com
include/linux/mm.h | 2 -- mm/swap.c | 30 ------------------------------ 2 files changed, 32 deletions(-)
Good to see this removed (including the EXPORT), even if the functionality still remains in a less obvious form, over in shm.
The fewer "all your page are pinned" calls we need, the simpler things get. :)
Acked-by: John Hubbard jhubbard@nvidia.com
thanks,
Hi Ira,
On Sun, 2 Oct 2022 at 05:53, ira.weiny@intel.com wrote:
From: Ira Weiny ira.weiny@intel.com
get_kernel_pages() only had one caller [shm_get_kernel_pages()] which did not need the functionality it provided. Furthermore, it called kmap_to_page() which we are looking to removed.
Alter shm_get_kernel_pages() to no longer call get_kernel_pages() and remove get_kernel_pages(). Along the way it was noted that shm_get_kernel_pages() does not have any need to support vmalloc'ed addresses either. Remove that functionality to clean up the logic.
This series also fixes and uses is_kmap_addr().
From the above description, I am failing to see the motivation behind this change. Can you elaborate here?
Also, since you are targeting to remove kmap_to_page(), is there any alternative way to support highmem for future TEE bus drivers? As I can see higmem being enabled for multiple Arm defconfigs [1] which can also support TEE (an example which already does it: arch/arm/configs/imx_v6_v7_defconfig).
[1] git grep CONFIG_HIGHMEM arch/arm/
-Sumit
Ira Weiny (4): highmem: Enhance is_kmap_addr() to check kmap_local_page() mappings tee: Remove vmalloc page support tee: Remove call to get_kernel_pages() mm: Remove get_kernel_pages()
drivers/tee/tee_shm.c | 41 ++++++++++++-------------------- include/linux/highmem-internal.h | 5 +++- include/linux/mm.h | 2 -- mm/swap.c | 30 ----------------------- 4 files changed, 19 insertions(+), 59 deletions(-)
base-commit: 274d7803837da78dfc911bcda0d593412676fc20
2.37.2
On Mon, Oct 03, 2022 at 02:55:02PM +0530, Sumit Garg wrote:
Hi Ira,
On Sun, 2 Oct 2022 at 05:53, ira.weiny@intel.com wrote:
From: Ira Weiny ira.weiny@intel.com
get_kernel_pages() only had one caller [shm_get_kernel_pages()] which did not need the functionality it provided. Furthermore, it called kmap_to_page() which we are looking to removed.
Alter shm_get_kernel_pages() to no longer call get_kernel_pages() and remove get_kernel_pages(). Along the way it was noted that shm_get_kernel_pages() does not have any need to support vmalloc'ed addresses either. Remove that functionality to clean up the logic.
This series also fixes and uses is_kmap_addr().
From the above description, I am failing to see the motivation behind this change. Can you elaborate here?
Al Viro found[1] that kmap_to_page() is broken. But not only was it broken but it presents confusion over how highmem should be used because kmap() and friends should not be used for 'long term' mappings.
[1] https://lore.kernel.org/lkml/YzSSl1ItVlARDvG3@ZenIV
Also, since you are targeting to remove kmap_to_page(), is there any alternative way to support highmem for future TEE bus drivers? As I can see higmem being enabled for multiple Arm defconfigs [1] which can also support TEE (an example which already does it: arch/arm/configs/imx_v6_v7_defconfig).
With TEE how are the highmem pages used? Right now the code does not seem to use any user pages. So I can't really say how this should work. Why does the kernel need a mapping of those pages?
Ira
[1] git grep CONFIG_HIGHMEM arch/arm/
-Sumit
Ira Weiny (4): highmem: Enhance is_kmap_addr() to check kmap_local_page() mappings tee: Remove vmalloc page support tee: Remove call to get_kernel_pages() mm: Remove get_kernel_pages()
drivers/tee/tee_shm.c | 41 ++++++++++++-------------------- include/linux/highmem-internal.h | 5 +++- include/linux/mm.h | 2 -- mm/swap.c | 30 ----------------------- 4 files changed, 19 insertions(+), 59 deletions(-)
base-commit: 274d7803837da78dfc911bcda0d593412676fc20
2.37.2
On Mon, 3 Oct 2022 at 20:52, Ira Weiny ira.weiny@intel.com wrote:
On Mon, Oct 03, 2022 at 02:55:02PM +0530, Sumit Garg wrote:
Hi Ira,
On Sun, 2 Oct 2022 at 05:53, ira.weiny@intel.com wrote:
From: Ira Weiny ira.weiny@intel.com
get_kernel_pages() only had one caller [shm_get_kernel_pages()] which did not need the functionality it provided. Furthermore, it called kmap_to_page() which we are looking to removed.
Alter shm_get_kernel_pages() to no longer call get_kernel_pages() and remove get_kernel_pages(). Along the way it was noted that shm_get_kernel_pages() does not have any need to support vmalloc'ed addresses either. Remove that functionality to clean up the logic.
This series also fixes and uses is_kmap_addr().
From the above description, I am failing to see the motivation behind this change. Can you elaborate here?
Al Viro found[1] that kmap_to_page() is broken. But not only was it broken but it presents confusion over how highmem should be used because kmap() and friends should not be used for 'long term' mappings.
Thanks for the background info. This should be part of the cover letter.
Also, since you are targeting to remove kmap_to_page(), is there any alternative way to support highmem for future TEE bus drivers? As I can see higmem being enabled for multiple Arm defconfigs [1] which can also support TEE (an example which already does it: arch/arm/configs/imx_v6_v7_defconfig).
With TEE how are the highmem pages used? Right now the code does not seem to use any user pages. So I can't really say how this should work. Why does the kernel need a mapping of those pages?
Fair enough, I don't have a real kernel driver use-case for highmem which is required to be registered with TEE.
-Sumit
Ira
[1] git grep CONFIG_HIGHMEM arch/arm/
-Sumit
Ira Weiny (4): highmem: Enhance is_kmap_addr() to check kmap_local_page() mappings tee: Remove vmalloc page support tee: Remove call to get_kernel_pages() mm: Remove get_kernel_pages()
drivers/tee/tee_shm.c | 41 ++++++++++++-------------------- include/linux/highmem-internal.h | 5 +++- include/linux/mm.h | 2 -- mm/swap.c | 30 ----------------------- 4 files changed, 19 insertions(+), 59 deletions(-)
base-commit: 274d7803837da78dfc911bcda0d593412676fc20
2.37.2
op-tee@lists.trustedfirmware.org
-
Al Viro -
Christoph Hellwig -
Ira Weiny -
ira.weiny＠intel.com
-
Jens Wiklander -
John Hubbard -
Sumit Garg