restore large page mappings in direct map for secretmem
Subsystems (4)
cover restore large page mappings in direct map for secretmem
Hi all,
secretmem removes the pages backing secretmem mappings from the direct map.
Removing one base page from the direct map can split the covering large mapping down to PTE mappings. Repeated splits can leave more of the direct map mapped with PTEs, meaning more TLB entries for the same range and potentially more TLB pressure.
This series adds an arch hook so secretmem can ask the architecture to try to restore large page mappings whenever secretmem restores a folio to the direct map.
Thanks,
Lance
Lance Yang (2):
mm/secretmem: try to restore large page mappings in direct map x86/mm: restore large page mappings for secretmem
arch/x86/include/asm/set_memory.h | 2 ++ arch/x86/mm/pat/set_memory.c | 27 +++++++++++++++++++++++++++ include/linux/set_memory.h | 6 ++++++ mm/secretmem.c | 12 ++++++++++-- 4 files changed, 45 insertions(+), 2 deletions(-)
Signature · 1 line
2.49.0
RFC PATCH 1/2 mm/secretmem: try to restore large page mappings in direct map
From: Lance Yang <[email protected]>
secretmem removes the pages backing secretmem mappings from the direct map.
Removing one base page from the direct map can split the covering large mapping down to PTE mappings. Repeated splits can leave more of the direct map mapped with PTEs, meaning more TLB entries for the same range and potentially more TLB pressure.
So let's try to restore large page mappings whenever secretmem restores a folio to the direct map.
--- include/linux/set_memory.h | 6 ++++++ mm/secretmem.c | 12 ++++++++++-- 2 files changed, 16 insertions(+), 2 deletions(-)
include/linux/set_memory.h +6 -0
@@ -58,6 +58,12 @@ static inline bool can_set_direct_map(void)#endif #endif /* CONFIG_ARCH_HAS_SET_DIRECT_MAP */ +#ifndef arch_try_collapse_direct_map +static inline void arch_try_collapse_direct_map (struct page *page) +{ +} +#endif +#ifdef CONFIG_X86_64 int set_mce_nospec (unsigned long pfn);int clear_mce_nospec (unsigned long pfn);
mm/secretmem.c +10 -2
@@ -47,6 +47,14 @@ bool secretmem_active(void)return !!atomic_read (&secretmem_users); } +static void secretmem_restore_direct_map (struct folio *folio) +{ +struct page *page =folio_page (folio,0 ); + +if (!set_direct_map_default_noflush (page)) +arch_try_collapse_direct_map (page); +} +static vm_fault_t secretmem_fault (struct vm_fault *vmf) {struct address_space *mapping = vmf->vma->vm_file->f_mapping;
@@ -87,7 +95,7 @@ static vm_fault_t secretmem_fault(struct vm_fault *vmf)* already happened when we marked the page invalid * which guarantees that this call won't fail */ -set_direct_map_default_noflush (folio_page (folio,0 )); +secretmem_restore_direct_map (folio);folio_put (folio);if (err == -EEXIST ) goto retry;
@@ -151,7 +159,7 @@ static int secretmem_migrate_folio(struct address_space *mapping,static void secretmem_free_folio (struct folio *folio) { -set_direct_map_default_noflush (folio_page (folio,0 )); +secretmem_restore_direct_map (folio);folio_zero_segment (folio,0 ,folio_size (folio)); }
Signature · 1 line
2.49.0
Mike Rapoport
maintainerRe: [RFC PATCH 1/2] mm/secretmem: try to restore large page mappings in direct map
Hi Lance,
Quoted 24 lines · Lance Yang
From: Lance Yang <[email protected]>
secretmem removes the pages backing secretmem mappings from the direct map.
Removing one base page from the direct map can split the covering large mapping down to PTE mappings. Repeated splits can leave more of the direct map mapped with PTEs, meaning more TLB entries for the same range and potentially more TLB pressure.
So let's try to restore large page mappings whenever secretmem restores a folio to the direct map.
Tested-by Xueyuan Chen <[email protected]>Signed-off-by Lance Yang <[email protected]>--- include/linux/set_memory.h | 6 ++++++ mm/secretmem.c | 12 ++++++++++-- 2 files changed, 16 insertions(+), 2 deletions(-)include/linux/set_memory.h +0
-0
include/linux/set_memory.h static inline bool can_set_direct_map(void) · line 65
Quoted 8 lines
include/linux/set_memory.h +5
-0@@ -58,6 +58,12 @@ static inline bool can_set_direct_map(void)#endif #endif /* CONFIG_ARCH_HAS_SET_DIRECT_MAP */ +#ifndef arch_try_collapse_direct_map +static inline void arch_try_collapse_direct_map (struct page *page) +{ +} +#endif
Did you explore what would it mean to hook collapse_large_pages() into set_direct_map_default_noflush()?
Signature · 2 lines
Sincerely yours, Mike.
Re: [RFC PATCH 1/2] mm/secretmem: try to restore large page mappings in direct map
Quoted 1 line · Mike Rapoport
Hi Lance,
Hey Mike,
Quoted 28 lines · Lance Yang
Quoted 27 lines
From: Lance Yang <[email protected]>
secretmem removes the pages backing secretmem mappings from the direct map.
Removing one base page from the direct map can split the covering large mapping down to PTE mappings. Repeated splits can leave more of the direct map mapped with PTEs, meaning more TLB entries for the same range and potentially more TLB pressure.
So let's try to restore large page mappings whenever secretmem restores a folio to the direct map.
Tested-by Xueyuan Chen <[email protected]>Signed-off-by Lance Yang <[email protected]>--- include/linux/set_memory.h | 6 ++++++ mm/secretmem.c | 12 ++++++++++-- 2 files changed, 16 insertions(+), 2 deletions(-)include/linux/set_memory.h +0
-0@@ -58,6 +58,12 @@ static inline bool can_set_direct_map(void)#endif #endif /* CONFIG_ARCH_HAS_SET_DIRECT_MAP */
Quoted 8 lines
include/linux/set_memory.h static inline bool can_set_direct_map(void) · line 65
Quoted 5 lines
include/linux/set_memory.h +5
-0+#ifndef arch_try_collapse_direct_map +static inline void arch_try_collapse_direct_map (struct page *page) +{ +} +#endif Did you explore what would it mean to hook
collapse_large_pages()intoset_direct_map_default_noflush()?
Good point, I kept it separate on purpose :)
Putting collapse into set_direct_map_default_noflush() would change the semantics of that helper a bit, IMHO.
I would expect arch_try_collapse_direct_map() to also be useful for cases where a direct-map permission change could split a large maping first, and the user wants to try restoring the large mapping after changing it back. One example[1] is making a direct-map range read-only for security, which I am also working on :)
[1] https://lore.kernel.org/linux-mm/[email protected]/
Cheers, Lance
Mike Rapoport
maintainerRe: [RFC PATCH 1/2] mm/secretmem: try to restore large page mappings in direct map
Quoted 38 lines · Lance Yang
Quoted 28 lines · Mike Rapoport
Quoted 27 lines · Lance Yang
From: Lance Yang <[email protected]>
secretmem removes the pages backing secretmem mappings from the direct map.
Removing one base page from the direct map can split the covering large mapping down to PTE mappings. Repeated splits can leave more of the direct map mapped with PTEs, meaning more TLB entries for the same range and potentially more TLB pressure.
So let's try to restore large page mappings whenever secretmem restores a folio to the direct map.
Tested-by Xueyuan Chen <[email protected]>Signed-off-by Lance Yang <[email protected]>--- include/linux/set_memory.h | 6 ++++++ mm/secretmem.c | 12 ++++++++++-- 2 files changed, 16 insertions(+), 2 deletions(-)include/linux/set_memory.h +0
-0@@ -58,6 +58,12 @@ static inline bool can_set_direct_map(void)#endif #endif /* CONFIG_ARCH_HAS_SET_DIRECT_MAP */ Quoted 8 lines
include/linux/set_memory.h static inline bool can_set_direct_map(void) · line 65
Quoted 5 lines
include/linux/set_memory.h +5
-0+#ifndef arch_try_collapse_direct_map +static inline void arch_try_collapse_direct_map (struct page *page) +{ +} +#endif Did you explore what would it mean to hook
collapse_large_pages()intoset_direct_map_default_noflush()?
Quoted 4 lines
Good point, I kept it separate on purpose :)
Putting collapse into
set_direct_map_default_noflush()would change the semantics of that helper a bit, IMHO.
For x86 default means present + rw + PSE, so yuu can look at it as actually better enforcing the semantics :)
Quoted 5 lines
I would expect
arch_try_collapse_direct_map()to also be useful for cases where a direct-map permission change could split a large maping first, and the user wants to try restoring the large mapping after changing it back. One example[1] is making a direct-map range read-only for security, which I am also working on :)
I don't think users should care. The users care for particular permissions of a range in the direct map. It should be up to the architecture to select most suitable mapping size. The splits are implicit, I don't see why collapses can't be implicit as well.
Quoted 4 lines
[1] https://lore.kernel.org/linux-mm/[email protected]/
Cheers, Lance
Signature · 2 lines
Sincerely yours, Mike.
Re: [RFC PATCH 1/2] mm/secretmem: try to restore large page mappings in direct map
Quoted 39 lines · Mike Rapoport
Quoted 38 lines · Lance Yang
Quoted 36 lines · Mike Rapoport
Quoted 24 lines · Lance Yang
From: Lance Yang <[email protected]>
secretmem removes the pages backing secretmem mappings from the direct map.
Removing one base page from the direct map can split the covering large mapping down to PTE mappings. Repeated splits can leave more of the direct map mapped with PTEs, meaning more TLB entries for the same range and potentially more TLB pressure.
So let's try to restore large page mappings whenever secretmem restores a folio to the direct map.
Tested-by Xueyuan Chen <[email protected]>Signed-off-by Lance Yang <[email protected]>--- include/linux/set_memory.h | 6 ++++++ mm/secretmem.c | 12 ++++++++++-- 2 files changed, 16 insertions(+), 2 deletions(-)include/linux/set_memory.h +0
-0include/linux/set_memory.h static inline bool can_set_direct_map(void) · line 65
Quoted 8 lines
include/linux/set_memory.h +5
-0@@ -58,6 +58,12 @@ static inline bool can_set_direct_map(void)#endif #endif /* CONFIG_ARCH_HAS_SET_DIRECT_MAP */ +#ifndef arch_try_collapse_direct_map +static inline void arch_try_collapse_direct_map (struct page *page) +{ +} +#endif Did you explore what would it mean to hook
collapse_large_pages()intoset_direct_map_default_noflush()?
Quoted 7 lines
Quoted 4 lines
Good point, I kept it separate on purpose :)
Putting collapse into
set_direct_map_default_noflush()would change the semantics of that helper a bit, IMHO.For x86 default means present + rw + PSE, so yuu can look at it as actually better enforcing the semantics :)
Yep. One x86 detail though, default seems to miss _PAGE_GLOBAL today. Not sure if that is intentional or just historical. See patch #02.
Quoted 10 lines
Quoted 5 lines
I would expect
arch_try_collapse_direct_map()to also be useful for cases where a direct-map permission change could split a large maping first, and the user wants to try restoring the large mapping after changing it back. One example[1] is making a direct-map range read-only for security, which I am also working on :)I don't think users should care. The users care for particular permissions of a range in the direct map. It should be up to the architecture to select most suitable mapping size. The splits are implicit, I don't see why collapses can't be implicit as well.
And agreed, users should not care about the final mapping size, that is up to the arch.
TBH, my concern is making the collapse cost implicit for every set_direct_map_default_noflush() caller. I still lean toward keeping it opt-in, but happy to hear what folks prefer :)
Quoted 6 lines
Quoted 5 lines
[1] https://lore.kernel.org/linux-mm/[email protected]/
Cheers, Lance
Re: [RFC PATCH 1/2] mm/secretmem: try to restore large page mappings in direct map
Quoted 27 lines · Lance Yang
Quoted 9 lines · Mike Rapoport
Quoted 5 lines · Lance Yang
Good point, I kept it separate on purpose :)
Putting collapse into
set_direct_map_default_noflush()would change the semantics of that helper a bit, IMHO.For x86 default means present + rw + PSE, so yuu can look at it as actually better enforcing the semantics :)
Yep. One x86 detail though, default seems to miss
_PAGE_GLOBALtoday. Not sure if that is intentional or just historical. See patch #02.Quoted 10 lines
Quoted 5 lines
I would expect
arch_try_collapse_direct_map()to also be useful for cases where a direct-map permission change could split a large maping first, and the user wants to try restoring the large mapping after changing it back. One example[1] is making a direct-map range read-only for security, which I am also working on :)I don't think users should care. The users care for particular permissions of a range in the direct map. It should be up to the architecture to select most suitable mapping size. The splits are implicit, I don't see why collapses can't be implicit as well.
Quoted 6 lines
And agreed, users should not care about the final mapping size, that is up to the arch.
TBH, my concern is making the collapse cost implicit for every
set_direct_map_default_noflush()caller. I still lean toward keeping it opt-in, but happy to hear what folks prefer :)
If we could easily do that automatically, that would likely be preferable.
Especially given that we are getting other users of direct-map removal soon that would face similar problems (e.g., guest_memfd).
What the performance impact of trying to collapse after every directmap update? Imagine we have a full PMD range with directmap-removed PTEs?
Signature · 3 lines
Cheers, David
Re: [RFC PATCH 1/2] mm/secretmem: try to restore large page mappings in direct map
Quoted 31 lines · David Hildenbrand (Arm)
Quoted 30 lines · Lance Yang
Quoted 9 lines · Mike Rapoport
Quoted 5 lines · Lance Yang
Good point, I kept it separate on purpose :)
Putting collapse into
set_direct_map_default_noflush()would change the semantics of that helper a bit, IMHO.For x86 default means present + rw + PSE, so yuu can look at it as actually better enforcing the semantics :)
Yep. One x86 detail though, default seems to miss
_PAGE_GLOBALtoday. Not sure if that is intentional or just historical. See patch #02.Quoted 10 lines
Quoted 5 lines
I would expect
arch_try_collapse_direct_map()to also be useful for cases where a direct-map permission change could split a large maping first, and the user wants to try restoring the large mapping after changing it back. One example[1] is making a direct-map range read-only for security, which I am also working on :)I don't think users should care. The users care for particular permissions of a range in the direct map. It should be up to the architecture to select most suitable mapping size. The splits are implicit, I don't see why collapses can't be implicit as well.
And agreed, users should not care about the final mapping size, that is up to the arch.
Quoted 8 lines
Quoted 3 lines
TBH, my concern is making the collapse cost implicit for every
set_direct_map_default_noflush()caller. I still lean toward keeping it opt-in, but happy to hear what folks prefer :)If we could easily do that automatically, that would likely be preferable.
Especially given that we are getting other users of direct-map removal soon that would face similar problems (e.g.,
guest_memfd).
Yeah. Makes sense to me ;)
Quoted 3 lines
What the performance impact of trying to collapse after every directmap update? Imagine we have a full PMD range with directmap-removed PTEs?
Collapse can turn 512 PTEs into one PMD, and, if the large range is compatible, 512 PMDs into one PUD.
Eeah try walks the page tables, takes pgd_lock, and scans entries until it hits a non-present entry, mismatched flags, or non-contiguous PFN. The same kind of check is done for PMD entries before a PUD collapse.
If nothing collapses, it balis out without flush_tlb_all(). If at least one collapse succeeds, flush_tlb_all() is called once before freeing the old page tables, and that is probably the expensive part :)
So failed tries are cheaper that a real collapse, but not free.
Not sure how often the remove/restore cycles happens, whether automatic collapse is worth it depends on that. Keeping it explicit lets callers take that cost only when they know the collapse is really useful ...
Thanks, Lance
Mike Rapoport
maintainerRe: [RFC PATCH 1/2] mm/secretmem: try to restore large page mappings in direct map
Quoted 58 lines · Lance Yang
Quoted 31 lines · David Hildenbrand (Arm)
Quoted 30 lines · Lance Yang
Quoted 9 lines · Mike Rapoport
Quoted 5 lines · Lance Yang
Good point, I kept it separate on purpose :)
Putting collapse into
set_direct_map_default_noflush()would change the semantics of that helper a bit, IMHO.For x86 default means present + rw + PSE, so yuu can look at it as actually better enforcing the semantics :)
Yep. One x86 detail though, default seems to miss
_PAGE_GLOBALtoday. Not sure if that is intentional or just historical. See patch #02.Quoted 10 lines
Quoted 5 lines
I would expect
arch_try_collapse_direct_map()to also be useful for cases where a direct-map permission change could split a large maping first, and the user wants to try restoring the large mapping after changing it back. One example[1] is making a direct-map range read-only for security, which I am also working on :)I don't think users should care. The users care for particular permissions of a range in the direct map. It should be up to the architecture to select most suitable mapping size. The splits are implicit, I don't see why collapses can't be implicit as well.
And agreed, users should not care about the final mapping size, that is up to the arch.
Quoted 8 lines
Quoted 3 lines
TBH, my concern is making the collapse cost implicit for every
set_direct_map_default_noflush()caller. I still lean toward keeping it opt-in, but happy to hear what folks prefer :)If we could easily do that automatically, that would likely be preferable.
Especially given that we are getting other users of direct-map removal soon that would face similar problems (e.g.,
guest_memfd).Yeah. Makes sense to me ;)
Quoted 3 lines
What the performance impact of trying to collapse after every directmap update? Imagine we have a full PMD range with directmap-removed PTEs?
Collapse can turn 512 PTEs into one PMD, and, if the large range is compatible, 512 PMDs into one PUD.
Eeah try walks the page tables, takes
pgd_lock, and scans entries until it hits a non-present entry, mismatched flags, or non-contiguous PFN. The same kind of check is done for PMD entries before a PUD collapse.If nothing collapses, it balis out without
flush_tlb_all(). If at least one collapse succeeds,flush_tlb_all()is called once before freeing the old page tables, and that is probably the expensive part :)
Quoted 5 lines
So failed tries are cheaper that a real collapse, but not free.
Not sure how often the remove/restore cycles happens, whether automatic collapse is worth it depends on that. Keeping it explicit lets callers take that cost only when they know the collapse is really useful ...
The callers don't have any clue if the collapse is useful.
In secretmem case, it changes permissions of a single 4k page. How should it decide whether to collapse or not? Or any other caller of set_memory_* APIs for that matter?
Moreover, secretmem won't know if there are bpf allocations in the same PUD that also hammer direct map permissions?
It's either we decide that large mappings in the direct map are worth taking the cost of collapse or we live with the fragmented direct map.
And even though it's hard to measure, we'd need some numbers for at least some use cases to get a feeling of what's involved.
Quoted 2 lines
Thanks, Lance
Signature · 2 lines
Sincerely yours, Mike.
RFC PATCH 2/2 x86/mm: restore large page mappings for secretmem
In reply to Lance YangFrom: Lance Yang <[email protected]>
secretmem can ask the architecture to restore large page mappings in the direct map after a page has been restored.
On x86, set_direct_map_invalid_noflush() clears _PAGE_PRESENT, _PAGE_RW and _PAGE_DIRTY through CPA. CPA also clears _PAGE_GLOBAL after the PTE becomes non-present, because that bit is used for PROTNONE.
set_direct_map_default_noflush() only restores _PAGE_PRESENT and _PAGE_RW, so the restored PTE can still differ from the rest of the direct map. Put back the missing PAGE_KERNEL bits before trying to collapse the range.
Use PAGE_KERNEL so the restored PTE keeps _PAGE_GLOBAL when PAGE_KERNEL has it, and does not get it when PTI clears _PAGE_GLOBAL from __default_kernel_pte_mask.
--- arch/x86/include/asm/set_memory.h | 2 ++ arch/x86/mm/pat/set_memory.c | 27 +++++++++++++++++++++++++++ 2 files changed, 29 insertions(+)
arch/x86/include/asm/set_memory.h +2 -0
@@ -89,6 +89,8 @@ int set_pages_rw(struct page *page, int numpages);int set_direct_map_invalid_noflush (struct page *page);int set_direct_map_default_noflush (struct page *page);int set_direct_map_valid_noflush (struct page *page,unsigned nr,bool valid); +void arch_try_collapse_direct_map (struct page *page); +#define arch_try_collapse_direct_map arch_try_collapse_direct_mapbool kernel_page_present (struct page *page);extern int kernel_set_to_readonly;
arch/x86/mm/pat/set_memory.c +27 -0
@@ -2654,6 +2654,33 @@ int set_direct_map_default_noflush(struct page *page)return __set_pages_p (page,1 ); } +void arch_try_collapse_direct_map (struct page *page) +{ +pgprotval_t mask =pgprot_val (PAGE_KERNEL ) & (_PAGE_DIRTY | _PAGE_GLOBAL); +unsigned long addr; +struct cpa_data cpa = { .vaddr = &addr, + .pgd =NULL , + .numpages =1 , + .mask_set =__pgprot (mask), + .mask_clr =__pgprot (0 ), + .flags =CPA_NO_CHECK_ALIAS }; + +if (PageHighMem (page) ||debug_pagealloc_enabled ()) +return ; + + addr = (unsigned long )page_address (page); + +/* +* set_direct_map_default_noflush() only restores _PAGE_PRESENT and +* _PAGE_RW. Put back the other PAGE_KERNEL bits needed for a restored +* direct-map PTE to match the rest of the range. +*/ +if (__change_page_attr_set_clr (&cpa,1 )) +return ; + +cpa_collapse_large_pages (&cpa); +} +int set_direct_map_valid_noflush (struct page *page,unsigned nr,bool valid) {if (valid)
Signature · 1 line
2.49.0