* [PATCH V2 0/2] mm/debug_vm_pgtable: Some minor updates
@ 2020-12-01 12:19 Anshuman Khandual
2020-12-01 12:19 ` [PATCH V2 1/2] mm/debug_vm_pgtable/basic: Add validation for dirtiness after write protect Anshuman Khandual
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Anshuman Khandual @ 2020-12-01 12:19 UTC (permalink / raw)
To: linux-mm, akpm
Cc: linux-kernel, catalin.marinas, steven.price, christophe.leroy,
gerald.schaefer, vgupta, paul.walmsley, Anshuman Khandual
This series contains some cleanups and new test suggestions from Catalin
from an earlier discussion.
https://lore.kernel.org/linux-mm/20201123142237.GF17833@gaia/
This series is based on v5.10-rc6 and has been tested on arm64 and x86 but
has only been build tested on riscv, s390, arc etc. It would be great if
folks could test this on these platforms as well. Thank you.
Cc: Christophe Leroy <christophe.leroy@csgroup.eu>
Cc: Gerald Schaefer <gerald.schaefer@linux.ibm.com>
Cc: Vineet Gupta <vgupta@synopsys.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Paul Walmsley <paul.walmsley@sifive.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Changes in V2:
- Added some missing tests in pud_basic_tests()
- Added bidirectional dirty check after pxx_wrprotect() per Catalin
- Moved WARN_ON(pte_dirty(pte_wrprotect(pte))) just after page table entry creation
Changes in V1:
https://lore.kernel.org/linux-mm/1606453584-15399-1-git-send-email-anshuman.khandual@arm.com/
Anshuman Khandual (2):
mm/debug_vm_pgtable/basic: Add validation for dirtiness after write protect
mm/debug_vm_pgtable/basic: Iterate over entire protection_map[]
Documentation/vm/arch_pgtable_helpers.rst | 8 +-
mm/debug_vm_pgtable.c | 89 ++++++++++++++++++++---
2 files changed, 81 insertions(+), 16 deletions(-)
--
2.20.1
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH V2 1/2] mm/debug_vm_pgtable/basic: Add validation for dirtiness after write protect 2020-12-01 12:19 [PATCH V2 0/2] mm/debug_vm_pgtable: Some minor updates Anshuman Khandual @ 2020-12-01 12:19 ` Anshuman Khandual 2020-12-02 11:16 ` Steven Price 2020-12-01 12:19 ` [PATCH V2 2/2] mm/debug_vm_pgtable/basic: Iterate over entire protection_map[] Anshuman Khandual 2020-12-09 2:41 ` [PATCH V2 0/2] mm/debug_vm_pgtable: Some minor updates Anshuman Khandual 2 siblings, 1 reply; 7+ messages in thread From: Anshuman Khandual @ 2020-12-01 12:19 UTC (permalink / raw) To: linux-mm, akpm Cc: linux-kernel, catalin.marinas, steven.price, christophe.leroy, gerald.schaefer, vgupta, paul.walmsley, Anshuman Khandual This adds validation tests for dirtiness after write protect conversion for each page table level. There are two new separate test types involved here. The first test ensures that a given page table entry does not become dirty after pxx_wrprotect(). This is important for platforms like arm64 which transfers and drops the hardware dirty bit (!PTE_RDONLY) to the software dirty bit while making it an write protected one. This test ensures that no fresh page table entry could be created with hardware dirty bit set. The second test ensures that a given page table entry always preserve the dirty information across pxx_wrprotect(). This adds two previously missing PUD level basic tests and while here fixes pxx_wrprotect() related typos in the documentation file. Cc: Andrew Morton <akpm@linux-foundation.org> Cc: linux-mm@kvack.org Cc: linux-kernel@vger.kernel.org Suggested-by: Catalin Marinas <catalin.marinas@arm.com> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com> --- Documentation/vm/arch_pgtable_helpers.rst | 8 ++--- mm/debug_vm_pgtable.c | 42 +++++++++++++++++++++++ 2 files changed, 46 insertions(+), 4 deletions(-) diff --git a/Documentation/vm/arch_pgtable_helpers.rst b/Documentation/vm/arch_pgtable_helpers.rst index f3591ee3aaa8..552567d863b8 100644 --- a/Documentation/vm/arch_pgtable_helpers.rst +++ b/Documentation/vm/arch_pgtable_helpers.rst @@ -50,7 +50,7 @@ PTE Page Table Helpers +---------------------------+--------------------------------------------------+ | pte_mkwrite | Creates a writable PTE | +---------------------------+--------------------------------------------------+ -| pte_mkwrprotect | Creates a write protected PTE | +| pte_wrprotect | Creates a write protected PTE | +---------------------------+--------------------------------------------------+ | pte_mkspecial | Creates a special PTE | +---------------------------+--------------------------------------------------+ @@ -120,7 +120,7 @@ PMD Page Table Helpers +---------------------------+--------------------------------------------------+ | pmd_mkwrite | Creates a writable PMD | +---------------------------+--------------------------------------------------+ -| pmd_mkwrprotect | Creates a write protected PMD | +| pmd_wrprotect | Creates a write protected PMD | +---------------------------+--------------------------------------------------+ | pmd_mkspecial | Creates a special PMD | +---------------------------+--------------------------------------------------+ @@ -186,7 +186,7 @@ PUD Page Table Helpers +---------------------------+--------------------------------------------------+ | pud_mkwrite | Creates a writable PUD | +---------------------------+--------------------------------------------------+ -| pud_mkwrprotect | Creates a write protected PUD | +| pud_wrprotect | Creates a write protected PUD | +---------------------------+--------------------------------------------------+ | pud_mkdevmap | Creates a ZONE_DEVICE mapped PUD | +---------------------------+--------------------------------------------------+ @@ -224,7 +224,7 @@ HugeTLB Page Table Helpers +---------------------------+--------------------------------------------------+ | huge_pte_mkwrite | Creates a writable HugeTLB | +---------------------------+--------------------------------------------------+ -| huge_pte_mkwrprotect | Creates a write protected HugeTLB | +| huge_pte_wrprotect | Creates a write protected HugeTLB | +---------------------------+--------------------------------------------------+ | huge_ptep_get_and_clear | Clears a HugeTLB | +---------------------------+--------------------------------------------------+ diff --git a/mm/debug_vm_pgtable.c b/mm/debug_vm_pgtable.c index c05d9dcf7891..c6fffea54522 100644 --- a/mm/debug_vm_pgtable.c +++ b/mm/debug_vm_pgtable.c @@ -63,6 +63,17 @@ static void __init pte_basic_tests(unsigned long pfn, pgprot_t prot) pte_t pte = pfn_pte(pfn, prot); pr_debug("Validating PTE basic\n"); + + /* + * This test needs to execute right after the given page + * table entry is created with pfn_pte() to make sure that + * protection_map[idx] does not have the dirty bit enabled + * from the beginning. This is particularly important for + * platforms like arm64 where (!PTE_RDONLY) indicate dirty + * bit being set. + */ + WARN_ON(pte_dirty(pte_wrprotect(pte))); + WARN_ON(!pte_same(pte, pte)); WARN_ON(!pte_young(pte_mkyoung(pte_mkold(pte)))); WARN_ON(!pte_dirty(pte_mkdirty(pte_mkclean(pte)))); @@ -70,6 +81,8 @@ static void __init pte_basic_tests(unsigned long pfn, pgprot_t prot) WARN_ON(pte_young(pte_mkold(pte_mkyoung(pte)))); WARN_ON(pte_dirty(pte_mkclean(pte_mkdirty(pte)))); WARN_ON(pte_write(pte_wrprotect(pte_mkwrite(pte)))); + WARN_ON(pte_dirty(pte_wrprotect(pte_mkclean(pte)))); + WARN_ON(!pte_dirty(pte_wrprotect(pte_mkdirty(pte)))); } static void __init pte_advanced_tests(struct mm_struct *mm, @@ -137,6 +150,18 @@ static void __init pmd_basic_tests(unsigned long pfn, pgprot_t prot) return; pr_debug("Validating PMD basic\n"); + + /* + * This test needs to execute right after the given page + * table entry is created with pfn_pmd() to make sure that + * protection_map[idx] does not have the dirty bit enabled + * from the beginning. This is particularly important for + * platforms like arm64 where (!PTE_RDONLY) indicate dirty + * bit being set. + */ + WARN_ON(pmd_dirty(pmd_wrprotect(pmd))); + + WARN_ON(!pmd_same(pmd, pmd)); WARN_ON(!pmd_young(pmd_mkyoung(pmd_mkold(pmd)))); WARN_ON(!pmd_dirty(pmd_mkdirty(pmd_mkclean(pmd)))); @@ -144,6 +169,8 @@ static void __init pmd_basic_tests(unsigned long pfn, pgprot_t prot) WARN_ON(pmd_young(pmd_mkold(pmd_mkyoung(pmd)))); WARN_ON(pmd_dirty(pmd_mkclean(pmd_mkdirty(pmd)))); WARN_ON(pmd_write(pmd_wrprotect(pmd_mkwrite(pmd)))); + WARN_ON(pmd_dirty(pmd_wrprotect(pmd_mkclean(pmd)))); + WARN_ON(!pmd_dirty(pmd_wrprotect(pmd_mkdirty(pmd)))); /* * A huge page does not point to next level page table * entry. Hence this must qualify as pmd_bad(). @@ -257,11 +284,26 @@ static void __init pud_basic_tests(unsigned long pfn, pgprot_t prot) return; pr_debug("Validating PUD basic\n"); + + /* + * This test needs to execute right after the given page + * table entry is created with pfn_pud() to make sure that + * protection_map[idx] does not have the dirty bit enabled + * from the beginning. This is particularly important for + * platforms like arm64 where (!PTE_RDONLY) indicate dirty + * bit being set. + */ + WARN_ON(pud_dirty(pud_wrprotect(pud))); + WARN_ON(!pud_same(pud, pud)); WARN_ON(!pud_young(pud_mkyoung(pud_mkold(pud)))); + WARN_ON(!pud_dirty(pud_mkdirty(pud_mkclean(pud)))); + WARN_ON(pud_dirty(pud_mkclean(pud_mkdirty(pud)))); WARN_ON(!pud_write(pud_mkwrite(pud_wrprotect(pud)))); WARN_ON(pud_write(pud_wrprotect(pud_mkwrite(pud)))); WARN_ON(pud_young(pud_mkold(pud_mkyoung(pud)))); + WARN_ON(pud_dirty(pud_wrprotect(pud_mkclean(pud)))); + WARN_ON(!pud_dirty(pud_wrprotect(pud_mkdirty(pud)))); if (mm_pmd_folded(mm)) return; -- 2.20.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH V2 1/2] mm/debug_vm_pgtable/basic: Add validation for dirtiness after write protect 2020-12-01 12:19 ` [PATCH V2 1/2] mm/debug_vm_pgtable/basic: Add validation for dirtiness after write protect Anshuman Khandual @ 2020-12-02 11:16 ` Steven Price 2020-12-02 12:04 ` Anshuman Khandual 0 siblings, 1 reply; 7+ messages in thread From: Steven Price @ 2020-12-02 11:16 UTC (permalink / raw) To: Anshuman Khandual, linux-mm, akpm Cc: linux-kernel, catalin.marinas, christophe.leroy, gerald.schaefer, vgupta, paul.walmsley On 01/12/2020 12:19, Anshuman Khandual wrote: > This adds validation tests for dirtiness after write protect conversion for > each page table level. There are two new separate test types involved here. > > The first test ensures that a given page table entry does not become dirty > after pxx_wrprotect(). This is important for platforms like arm64 which > transfers and drops the hardware dirty bit (!PTE_RDONLY) to the software > dirty bit while making it an write protected one. This test ensures that > no fresh page table entry could be created with hardware dirty bit set. > The second test ensures that a given page table entry always preserve the > dirty information across pxx_wrprotect(). > > This adds two previously missing PUD level basic tests and while here fixes > pxx_wrprotect() related typos in the documentation file. > > Cc: Andrew Morton <akpm@linux-foundation.org> > Cc: linux-mm@kvack.org > Cc: linux-kernel@vger.kernel.org > Suggested-by: Catalin Marinas <catalin.marinas@arm.com> > Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com> > --- > Documentation/vm/arch_pgtable_helpers.rst | 8 ++--- > mm/debug_vm_pgtable.c | 42 +++++++++++++++++++++++ > 2 files changed, 46 insertions(+), 4 deletions(-) > > diff --git a/Documentation/vm/arch_pgtable_helpers.rst b/Documentation/vm/arch_pgtable_helpers.rst > index f3591ee3aaa8..552567d863b8 100644 > --- a/Documentation/vm/arch_pgtable_helpers.rst > +++ b/Documentation/vm/arch_pgtable_helpers.rst > @@ -50,7 +50,7 @@ PTE Page Table Helpers > +---------------------------+--------------------------------------------------+ > | pte_mkwrite | Creates a writable PTE | > +---------------------------+--------------------------------------------------+ > -| pte_mkwrprotect | Creates a write protected PTE | > +| pte_wrprotect | Creates a write protected PTE | > +---------------------------+--------------------------------------------------+ > | pte_mkspecial | Creates a special PTE | > +---------------------------+--------------------------------------------------+ > @@ -120,7 +120,7 @@ PMD Page Table Helpers > +---------------------------+--------------------------------------------------+ > | pmd_mkwrite | Creates a writable PMD | > +---------------------------+--------------------------------------------------+ > -| pmd_mkwrprotect | Creates a write protected PMD | > +| pmd_wrprotect | Creates a write protected PMD | > +---------------------------+--------------------------------------------------+ > | pmd_mkspecial | Creates a special PMD | > +---------------------------+--------------------------------------------------+ > @@ -186,7 +186,7 @@ PUD Page Table Helpers > +---------------------------+--------------------------------------------------+ > | pud_mkwrite | Creates a writable PUD | > +---------------------------+--------------------------------------------------+ > -| pud_mkwrprotect | Creates a write protected PUD | > +| pud_wrprotect | Creates a write protected PUD | > +---------------------------+--------------------------------------------------+ > | pud_mkdevmap | Creates a ZONE_DEVICE mapped PUD | > +---------------------------+--------------------------------------------------+ > @@ -224,7 +224,7 @@ HugeTLB Page Table Helpers > +---------------------------+--------------------------------------------------+ > | huge_pte_mkwrite | Creates a writable HugeTLB | > +---------------------------+--------------------------------------------------+ > -| huge_pte_mkwrprotect | Creates a write protected HugeTLB | > +| huge_pte_wrprotect | Creates a write protected HugeTLB | > +---------------------------+--------------------------------------------------+ > | huge_ptep_get_and_clear | Clears a HugeTLB | > +---------------------------+--------------------------------------------------+ > diff --git a/mm/debug_vm_pgtable.c b/mm/debug_vm_pgtable.c > index c05d9dcf7891..c6fffea54522 100644 > --- a/mm/debug_vm_pgtable.c > +++ b/mm/debug_vm_pgtable.c > @@ -63,6 +63,17 @@ static void __init pte_basic_tests(unsigned long pfn, pgprot_t prot) > pte_t pte = pfn_pte(pfn, prot); > > pr_debug("Validating PTE basic\n"); > + > + /* > + * This test needs to execute right after the given page > + * table entry is created with pfn_pte() to make sure that > + * protection_map[idx] does not have the dirty bit enabled > + * from the beginning. This is particularly important for > + * platforms like arm64 where (!PTE_RDONLY) indicate dirty > + * bit being set. > + */ Unless I'm seriously mistaken this comment is misleading - the likes of pte_wrprotect() take the PTE *by value* and return the modified version. So none of these tests actually modify the variable 'pte'. So there shouldn't actually be any restrictions on the ordering. Or am I missing something? Steve > + WARN_ON(pte_dirty(pte_wrprotect(pte))); > + > WARN_ON(!pte_same(pte, pte)); > WARN_ON(!pte_young(pte_mkyoung(pte_mkold(pte)))); > WARN_ON(!pte_dirty(pte_mkdirty(pte_mkclean(pte)))); > @@ -70,6 +81,8 @@ static void __init pte_basic_tests(unsigned long pfn, pgprot_t prot) > WARN_ON(pte_young(pte_mkold(pte_mkyoung(pte)))); > WARN_ON(pte_dirty(pte_mkclean(pte_mkdirty(pte)))); > WARN_ON(pte_write(pte_wrprotect(pte_mkwrite(pte)))); > + WARN_ON(pte_dirty(pte_wrprotect(pte_mkclean(pte)))); > + WARN_ON(!pte_dirty(pte_wrprotect(pte_mkdirty(pte)))); > } > > static void __init pte_advanced_tests(struct mm_struct *mm, > @@ -137,6 +150,18 @@ static void __init pmd_basic_tests(unsigned long pfn, pgprot_t prot) > return; > > pr_debug("Validating PMD basic\n"); > + > + /* > + * This test needs to execute right after the given page > + * table entry is created with pfn_pmd() to make sure that > + * protection_map[idx] does not have the dirty bit enabled > + * from the beginning. This is particularly important for > + * platforms like arm64 where (!PTE_RDONLY) indicate dirty > + * bit being set. > + */ > + WARN_ON(pmd_dirty(pmd_wrprotect(pmd))); > + > + > WARN_ON(!pmd_same(pmd, pmd)); > WARN_ON(!pmd_young(pmd_mkyoung(pmd_mkold(pmd)))); > WARN_ON(!pmd_dirty(pmd_mkdirty(pmd_mkclean(pmd)))); > @@ -144,6 +169,8 @@ static void __init pmd_basic_tests(unsigned long pfn, pgprot_t prot) > WARN_ON(pmd_young(pmd_mkold(pmd_mkyoung(pmd)))); > WARN_ON(pmd_dirty(pmd_mkclean(pmd_mkdirty(pmd)))); > WARN_ON(pmd_write(pmd_wrprotect(pmd_mkwrite(pmd)))); > + WARN_ON(pmd_dirty(pmd_wrprotect(pmd_mkclean(pmd)))); > + WARN_ON(!pmd_dirty(pmd_wrprotect(pmd_mkdirty(pmd)))); > /* > * A huge page does not point to next level page table > * entry. Hence this must qualify as pmd_bad(). > @@ -257,11 +284,26 @@ static void __init pud_basic_tests(unsigned long pfn, pgprot_t prot) > return; > > pr_debug("Validating PUD basic\n"); > + > + /* > + * This test needs to execute right after the given page > + * table entry is created with pfn_pud() to make sure that > + * protection_map[idx] does not have the dirty bit enabled > + * from the beginning. This is particularly important for > + * platforms like arm64 where (!PTE_RDONLY) indicate dirty > + * bit being set. > + */ > + WARN_ON(pud_dirty(pud_wrprotect(pud))); > + > WARN_ON(!pud_same(pud, pud)); > WARN_ON(!pud_young(pud_mkyoung(pud_mkold(pud)))); > + WARN_ON(!pud_dirty(pud_mkdirty(pud_mkclean(pud)))); > + WARN_ON(pud_dirty(pud_mkclean(pud_mkdirty(pud)))); > WARN_ON(!pud_write(pud_mkwrite(pud_wrprotect(pud)))); > WARN_ON(pud_write(pud_wrprotect(pud_mkwrite(pud)))); > WARN_ON(pud_young(pud_mkold(pud_mkyoung(pud)))); > + WARN_ON(pud_dirty(pud_wrprotect(pud_mkclean(pud)))); > + WARN_ON(!pud_dirty(pud_wrprotect(pud_mkdirty(pud)))); > > if (mm_pmd_folded(mm)) > return; > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH V2 1/2] mm/debug_vm_pgtable/basic: Add validation for dirtiness after write protect 2020-12-02 11:16 ` Steven Price @ 2020-12-02 12:04 ` Anshuman Khandual 0 siblings, 0 replies; 7+ messages in thread From: Anshuman Khandual @ 2020-12-02 12:04 UTC (permalink / raw) To: Steven Price, linux-mm, akpm Cc: linux-kernel, catalin.marinas, christophe.leroy, gerald.schaefer, vgupta, paul.walmsley On 12/2/20 4:46 PM, Steven Price wrote: > On 01/12/2020 12:19, Anshuman Khandual wrote: >> This adds validation tests for dirtiness after write protect conversion for >> each page table level. There are two new separate test types involved here. >> >> The first test ensures that a given page table entry does not become dirty >> after pxx_wrprotect(). This is important for platforms like arm64 which >> transfers and drops the hardware dirty bit (!PTE_RDONLY) to the software >> dirty bit while making it an write protected one. This test ensures that >> no fresh page table entry could be created with hardware dirty bit set. >> The second test ensures that a given page table entry always preserve the >> dirty information across pxx_wrprotect(). >> >> This adds two previously missing PUD level basic tests and while here fixes >> pxx_wrprotect() related typos in the documentation file. >> >> Cc: Andrew Morton <akpm@linux-foundation.org> >> Cc: linux-mm@kvack.org >> Cc: linux-kernel@vger.kernel.org >> Suggested-by: Catalin Marinas <catalin.marinas@arm.com> >> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com> >> --- >> Documentation/vm/arch_pgtable_helpers.rst | 8 ++--- >> mm/debug_vm_pgtable.c | 42 +++++++++++++++++++++++ >> 2 files changed, 46 insertions(+), 4 deletions(-) >> >> diff --git a/Documentation/vm/arch_pgtable_helpers.rst b/Documentation/vm/arch_pgtable_helpers.rst >> index f3591ee3aaa8..552567d863b8 100644 >> --- a/Documentation/vm/arch_pgtable_helpers.rst >> +++ b/Documentation/vm/arch_pgtable_helpers.rst >> @@ -50,7 +50,7 @@ PTE Page Table Helpers >> +---------------------------+--------------------------------------------------+ >> | pte_mkwrite | Creates a writable PTE | >> +---------------------------+--------------------------------------------------+ >> -| pte_mkwrprotect | Creates a write protected PTE | >> +| pte_wrprotect | Creates a write protected PTE | >> +---------------------------+--------------------------------------------------+ >> | pte_mkspecial | Creates a special PTE | >> +---------------------------+--------------------------------------------------+ >> @@ -120,7 +120,7 @@ PMD Page Table Helpers >> +---------------------------+--------------------------------------------------+ >> | pmd_mkwrite | Creates a writable PMD | >> +---------------------------+--------------------------------------------------+ >> -| pmd_mkwrprotect | Creates a write protected PMD | >> +| pmd_wrprotect | Creates a write protected PMD | >> +---------------------------+--------------------------------------------------+ >> | pmd_mkspecial | Creates a special PMD | >> +---------------------------+--------------------------------------------------+ >> @@ -186,7 +186,7 @@ PUD Page Table Helpers >> +---------------------------+--------------------------------------------------+ >> | pud_mkwrite | Creates a writable PUD | >> +---------------------------+--------------------------------------------------+ >> -| pud_mkwrprotect | Creates a write protected PUD | >> +| pud_wrprotect | Creates a write protected PUD | >> +---------------------------+--------------------------------------------------+ >> | pud_mkdevmap | Creates a ZONE_DEVICE mapped PUD | >> +---------------------------+--------------------------------------------------+ >> @@ -224,7 +224,7 @@ HugeTLB Page Table Helpers >> +---------------------------+--------------------------------------------------+ >> | huge_pte_mkwrite | Creates a writable HugeTLB | >> +---------------------------+--------------------------------------------------+ >> -| huge_pte_mkwrprotect | Creates a write protected HugeTLB | >> +| huge_pte_wrprotect | Creates a write protected HugeTLB | >> +---------------------------+--------------------------------------------------+ >> | huge_ptep_get_and_clear | Clears a HugeTLB | >> +---------------------------+--------------------------------------------------+ >> diff --git a/mm/debug_vm_pgtable.c b/mm/debug_vm_pgtable.c >> index c05d9dcf7891..c6fffea54522 100644 >> --- a/mm/debug_vm_pgtable.c >> +++ b/mm/debug_vm_pgtable.c >> @@ -63,6 +63,17 @@ static void __init pte_basic_tests(unsigned long pfn, pgprot_t prot) >> pte_t pte = pfn_pte(pfn, prot); >> pr_debug("Validating PTE basic\n"); >> + >> + /* >> + * This test needs to execute right after the given page >> + * table entry is created with pfn_pte() to make sure that >> + * protection_map[idx] does not have the dirty bit enabled >> + * from the beginning. This is particularly important for >> + * platforms like arm64 where (!PTE_RDONLY) indicate dirty >> + * bit being set. >> + */ > > Unless I'm seriously mistaken this comment is misleading - the likes of pte_wrprotect() take the PTE *by value* and return the modified version. So none of these tests actually modify the variable 'pte'. So there shouldn't actually be any restrictions on the ordering. > > Or am I missing something? No, you are right. Seems like I might have confused this for other page table entry altering tests here (via pointers). Although it might still be better to have these tests at the beginning as not to miss a freshly created page table entry. That way the test would not be forgotten and invalidated, in case the subsequent tests in the function change in the future. So yes, the there is no restriction on the ordering here, as I might have mentioned previously. Looking at the comments again. It still seems to be applicable and valid as it does not explicitly refer to the restriction on ordering here. It just stresses on the point that it needs to execute right after creation to test what was in protection_map[idx]. ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH V2 2/2] mm/debug_vm_pgtable/basic: Iterate over entire protection_map[] 2020-12-01 12:19 [PATCH V2 0/2] mm/debug_vm_pgtable: Some minor updates Anshuman Khandual 2020-12-01 12:19 ` [PATCH V2 1/2] mm/debug_vm_pgtable/basic: Add validation for dirtiness after write protect Anshuman Khandual @ 2020-12-01 12:19 ` Anshuman Khandual 2020-12-09 2:41 ` [PATCH V2 0/2] mm/debug_vm_pgtable: Some minor updates Anshuman Khandual 2 siblings, 0 replies; 7+ messages in thread From: Anshuman Khandual @ 2020-12-01 12:19 UTC (permalink / raw) To: linux-mm, akpm Cc: linux-kernel, catalin.marinas, steven.price, christophe.leroy, gerald.schaefer, vgupta, paul.walmsley, Anshuman Khandual Currently the basic tests just validate various page table transformations after starting with vm_get_page_prot(VM_READ|VM_WRITE|VM_EXEC) protection. Instead scan over the entire protection_map[] for better coverage. It also makes sure that all these basic page table tranformations checks hold true irrespective of the starting protection value for the page table entry. There is also a slight change in the debug print format for basic tests to capture the protection value it is being tested with. The modified output looks something like [pte_basic_tests ]: Validating PTE basic () [pte_basic_tests ]: Validating PTE basic (read) [pte_basic_tests ]: Validating PTE basic (write) [pte_basic_tests ]: Validating PTE basic (read|write) [pte_basic_tests ]: Validating PTE basic (exec) [pte_basic_tests ]: Validating PTE basic (read|exec) [pte_basic_tests ]: Validating PTE basic (write|exec) [pte_basic_tests ]: Validating PTE basic (read|write|exec) [pte_basic_tests ]: Validating PTE basic (shared) [pte_basic_tests ]: Validating PTE basic (read|shared) [pte_basic_tests ]: Validating PTE basic (write|shared) [pte_basic_tests ]: Validating PTE basic (read|write|shared) [pte_basic_tests ]: Validating PTE basic (exec|shared) [pte_basic_tests ]: Validating PTE basic (read|exec|shared) [pte_basic_tests ]: Validating PTE basic (write|exec|shared) [pte_basic_tests ]: Validating PTE basic (read|write|exec|shared) This adds a missing argument 'struct mm_struct *' in pud_basic_tests() test . This never got exposed before as PUD based THP is available only on X86 platform where mm_pmd_folded(mm) call gets macro replaced without requiring the mm_struct i.e __is_defined(__PAGETABLE_PMD_FOLDED). Cc: Andrew Morton <akpm@linux-foundation.org> Cc: linux-mm@kvack.org Cc: linux-kernel@vger.kernel.org Reviewed-by: Steven Price <steven.price@arm.com> Suggested-by: Catalin Marinas <catalin.marinas@arm.com> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com> --- mm/debug_vm_pgtable.c | 47 ++++++++++++++++++++++++++++++++----------- 1 file changed, 35 insertions(+), 12 deletions(-) diff --git a/mm/debug_vm_pgtable.c b/mm/debug_vm_pgtable.c index c6fffea54522..a96b5270e451 100644 --- a/mm/debug_vm_pgtable.c +++ b/mm/debug_vm_pgtable.c @@ -58,11 +58,13 @@ #define RANDOM_ORVALUE (GENMASK(BITS_PER_LONG - 1, 0) & ~ARCH_SKIP_MASK) #define RANDOM_NZVALUE GENMASK(7, 0) -static void __init pte_basic_tests(unsigned long pfn, pgprot_t prot) +static void __init pte_basic_tests(unsigned long pfn, int idx) { + pgprot_t prot = protection_map[idx]; pte_t pte = pfn_pte(pfn, prot); + unsigned long val = idx, *ptr = &val; - pr_debug("Validating PTE basic\n"); + pr_debug("Validating PTE basic (%pGv)\n", ptr); /* * This test needs to execute right after the given page @@ -142,14 +144,16 @@ static void __init pte_savedwrite_tests(unsigned long pfn, pgprot_t prot) } #ifdef CONFIG_TRANSPARENT_HUGEPAGE -static void __init pmd_basic_tests(unsigned long pfn, pgprot_t prot) +static void __init pmd_basic_tests(unsigned long pfn, int idx) { + pgprot_t prot = protection_map[idx]; pmd_t pmd = pfn_pmd(pfn, prot); + unsigned long val = idx, *ptr = &val; if (!has_transparent_hugepage()) return; - pr_debug("Validating PMD basic\n"); + pr_debug("Validating PMD basic (%pGv)\n", ptr); /* * This test needs to execute right after the given page @@ -276,14 +280,16 @@ static void __init pmd_savedwrite_tests(unsigned long pfn, pgprot_t prot) } #ifdef CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD -static void __init pud_basic_tests(unsigned long pfn, pgprot_t prot) +static void __init pud_basic_tests(struct mm_struct *mm, unsigned long pfn, int idx) { + pgprot_t prot = protection_map[idx]; pud_t pud = pfn_pud(pfn, prot); + unsigned long val = idx, *ptr = &val; if (!has_transparent_hugepage()) return; - pr_debug("Validating PUD basic\n"); + pr_debug("Validating PUD basic (%pGv)\n", ptr); /* * This test needs to execute right after the given page @@ -401,7 +407,7 @@ static void __init pud_huge_tests(pud_t *pudp, unsigned long pfn, pgprot_t prot) #endif /* !CONFIG_HAVE_ARCH_HUGE_VMAP */ #else /* !CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD */ -static void __init pud_basic_tests(unsigned long pfn, pgprot_t prot) { } +static void __init pud_basic_tests(struct mm_struct *mm, unsigned long pfn, int idx) { } static void __init pud_advanced_tests(struct mm_struct *mm, struct vm_area_struct *vma, pud_t *pudp, unsigned long pfn, unsigned long vaddr, @@ -414,8 +420,8 @@ static void __init pud_huge_tests(pud_t *pudp, unsigned long pfn, pgprot_t prot) } #endif /* CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD */ #else /* !CONFIG_TRANSPARENT_HUGEPAGE */ -static void __init pmd_basic_tests(unsigned long pfn, pgprot_t prot) { } -static void __init pud_basic_tests(unsigned long pfn, pgprot_t prot) { } +static void __init pmd_basic_tests(unsigned long pfn, int idx) { } +static void __init pud_basic_tests(struct mm_struct *mm, unsigned long pfn, int idx) { } static void __init pmd_advanced_tests(struct mm_struct *mm, struct vm_area_struct *vma, pmd_t *pmdp, unsigned long pfn, unsigned long vaddr, @@ -941,6 +947,7 @@ static int __init debug_vm_pgtable(void) unsigned long vaddr, pte_aligned, pmd_aligned; unsigned long pud_aligned, p4d_aligned, pgd_aligned; spinlock_t *ptl = NULL; + int idx; pr_info("Validating architecture page table helpers\n"); prot = vm_get_page_prot(VMFLAGS); @@ -1005,9 +1012,25 @@ static int __init debug_vm_pgtable(void) saved_pmdp = pmd_offset(pudp, 0UL); saved_ptep = pmd_pgtable(pmd); - pte_basic_tests(pte_aligned, prot); - pmd_basic_tests(pmd_aligned, prot); - pud_basic_tests(pud_aligned, prot); + /* + * Iterate over the protection_map[] to make sure that all + * the basic page table transformation validations just hold + * true irrespective of the starting protection value for a + * given page table entry. + */ + for (idx = 0; idx < ARRAY_SIZE(protection_map); idx++) { + pte_basic_tests(pte_aligned, idx); + pmd_basic_tests(pmd_aligned, idx); + pud_basic_tests(mm, pud_aligned, idx); + } + + /* + * Both P4D and PGD level tests are very basic which do not + * involve creating page table entries from the protection + * value and the given pfn. Hence just keep them out from + * the above iteration for now to save some test execution + * time. + */ p4d_basic_tests(p4d_aligned, prot); pgd_basic_tests(pgd_aligned, prot); -- 2.20.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH V2 0/2] mm/debug_vm_pgtable: Some minor updates 2020-12-01 12:19 [PATCH V2 0/2] mm/debug_vm_pgtable: Some minor updates Anshuman Khandual 2020-12-01 12:19 ` [PATCH V2 1/2] mm/debug_vm_pgtable/basic: Add validation for dirtiness after write protect Anshuman Khandual 2020-12-01 12:19 ` [PATCH V2 2/2] mm/debug_vm_pgtable/basic: Iterate over entire protection_map[] Anshuman Khandual @ 2020-12-09 2:41 ` Anshuman Khandual 2020-12-09 15:25 ` Gerald Schaefer 2 siblings, 1 reply; 7+ messages in thread From: Anshuman Khandual @ 2020-12-09 2:41 UTC (permalink / raw) To: linux-mm, akpm Cc: linux-kernel, catalin.marinas, steven.price, christophe.leroy, gerald.schaefer, vgupta, paul.walmsley On 12/1/20 5:49 PM, Anshuman Khandual wrote: > This series contains some cleanups and new test suggestions from Catalin > from an earlier discussion. > > https://lore.kernel.org/linux-mm/20201123142237.GF17833@gaia/ > > This series is based on v5.10-rc6 and has been tested on arm64 and x86 but > has only been build tested on riscv, s390, arc etc. It would be great if > folks could test this on these platforms as well. Thank you. > > Cc: Christophe Leroy <christophe.leroy@csgroup.eu> > Cc: Gerald Schaefer <gerald.schaefer@linux.ibm.com> > Cc: Vineet Gupta <vgupta@synopsys.com> > Cc: Catalin Marinas <catalin.marinas@arm.com> > Cc: Paul Walmsley <paul.walmsley@sifive.com> > Cc: Andrew Morton <akpm@linux-foundation.org> Hello Gerald/Christophe/Vineet/Paul, Could you please give this series a quick test on s390, ppc, arc, and riscv platforms. Thank you. - Anshuman ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH V2 0/2] mm/debug_vm_pgtable: Some minor updates 2020-12-09 2:41 ` [PATCH V2 0/2] mm/debug_vm_pgtable: Some minor updates Anshuman Khandual @ 2020-12-09 15:25 ` Gerald Schaefer 0 siblings, 0 replies; 7+ messages in thread From: Gerald Schaefer @ 2020-12-09 15:25 UTC (permalink / raw) To: Anshuman Khandual Cc: linux-mm, akpm, linux-kernel, catalin.marinas, steven.price, christophe.leroy, vgupta, paul.walmsley On Wed, 9 Dec 2020 08:11:13 +0530 Anshuman Khandual <anshuman.khandual@arm.com> wrote: > > > On 12/1/20 5:49 PM, Anshuman Khandual wrote: > > This series contains some cleanups and new test suggestions from Catalin > > from an earlier discussion. > > > > https://lore.kernel.org/linux-mm/20201123142237.GF17833@gaia/ > > > > This series is based on v5.10-rc6 and has been tested on arm64 and x86 but > > has only been build tested on riscv, s390, arc etc. It would be great if > > folks could test this on these platforms as well. Thank you. > > > > Cc: Christophe Leroy <christophe.leroy@csgroup.eu> > > Cc: Gerald Schaefer <gerald.schaefer@linux.ibm.com> > > Cc: Vineet Gupta <vgupta@synopsys.com> > > Cc: Catalin Marinas <catalin.marinas@arm.com> > > Cc: Paul Walmsley <paul.walmsley@sifive.com> > > Cc: Andrew Morton <akpm@linux-foundation.org> > > Hello Gerald/Christophe/Vineet/Paul, > > Could you please give this series a quick test on s390, ppc, arc, > and riscv platforms. Thank you. > Hi Anshuman, works fine for s390. Tested-by: Gerald Schaefer <gerald.schaefer@de.ibm.com> [s390] ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-12-09 15:27 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-12-01 12:19 [PATCH V2 0/2] mm/debug_vm_pgtable: Some minor updates Anshuman Khandual 2020-12-01 12:19 ` [PATCH V2 1/2] mm/debug_vm_pgtable/basic: Add validation for dirtiness after write protect Anshuman Khandual 2020-12-02 11:16 ` Steven Price 2020-12-02 12:04 ` Anshuman Khandual 2020-12-01 12:19 ` [PATCH V2 2/2] mm/debug_vm_pgtable/basic: Iterate over entire protection_map[] Anshuman Khandual 2020-12-09 2:41 ` [PATCH V2 0/2] mm/debug_vm_pgtable: Some minor updates Anshuman Khandual 2020-12-09 15:25 ` Gerald Schaefer
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).