From: Christophe Leroy <christophe.leroy@csgroup.eu>
To: Rohan McLure <rmclure@linux.ibm.com>,
"linuxppc-dev@lists.ozlabs.org" <linuxppc-dev@lists.ozlabs.org>
Cc: "linux-riscv@lists.infradead.org"
<linux-riscv@lists.infradead.org>,
"x86@kernel.org" <x86@kernel.org>,
"linux-arm-kernel@lists.infradead.org"
<linux-arm-kernel@lists.infradead.org>,
"linux-mm@kvack.org" <linux-mm@kvack.org>
Subject: Re: [PATCH v10 11/12] powerpc: mm: Use set_pte_at_unchecked() for early-boot / internal usages
Date: Wed, 13 Mar 2024 11:30:07 +0000 [thread overview]
Message-ID: <42bf1fb2-8a8e-4ded-97a6-da2617e89f5b@csgroup.eu> (raw)
In-Reply-To: <20240313042118.230397-12-rmclure@linux.ibm.com>
Le 13/03/2024 à 05:21, Rohan McLure a écrit :
> In the new set_ptes() API, set_pte_at() (a special case of set_ptes())
> is intended to be instrumented by the page table check facility. There
> are however several other routines that constitute the API for setting
> page table entries, including set_pmd_at() among others. Such routines
> are themselves implemented in terms of set_ptes_at().
>
> A future patch providing support for page table checking on powerpc
> must take care to avoid duplicate calls to
> page_table_check_p{te,md,ud}_set(). Allow for assignment of pte entries
> without instrumentation through the set_pte_at_unchecked() routine
> introduced in this patch.
>
> Cause API-facing routines that call set_pte_at() to instead call
> set_pte_at_unchecked(), which will remain uninstrumented by page
> table check. set_ptes() is itself implemented by calls to
> __set_pte_at(), so this eliminates redundant code.
>
> Also prefer set_pte_at_unchecked() in early-boot usages which should not be
> instrumented.
>
> Signed-off-by: Rohan McLure <rmclure@linux.ibm.com>
> ---
> v9: New patch
> v10: don't reuse __set_pte_at(), as that will not apply filters. Instead
> use new set_pte_at_unchecked().
Are filters needed at all in those usecases ?
> ---
> arch/powerpc/include/asm/pgtable.h | 2 ++
> arch/powerpc/mm/book3s64/hash_pgtable.c | 2 +-
> arch/powerpc/mm/book3s64/pgtable.c | 6 +++---
> arch/powerpc/mm/book3s64/radix_pgtable.c | 8 ++++----
> arch/powerpc/mm/nohash/book3e_pgtable.c | 2 +-
> arch/powerpc/mm/pgtable.c | 7 +++++++
> arch/powerpc/mm/pgtable_32.c | 2 +-
> 7 files changed, 19 insertions(+), 10 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/pgtable.h b/arch/powerpc/include/asm/pgtable.h
> index 3741a63fb82e..6ff1d8cfa216 100644
> --- a/arch/powerpc/include/asm/pgtable.h
> +++ b/arch/powerpc/include/asm/pgtable.h
> @@ -44,6 +44,8 @@ struct mm_struct;
> void set_ptes(struct mm_struct *mm, unsigned long addr, pte_t *ptep,
> pte_t pte, unsigned int nr);
> #define set_ptes set_ptes
> +void set_pte_at_unchecked(struct mm_struct *mm, unsigned long addr,
> + pte_t *ptep, pte_t pte);
> #define update_mmu_cache(vma, addr, ptep) \
> update_mmu_cache_range(NULL, vma, addr, ptep, 1)
>
> diff --git a/arch/powerpc/mm/book3s64/hash_pgtable.c b/arch/powerpc/mm/book3s64/hash_pgtable.c
> index 988948d69bc1..871472f99a01 100644
> --- a/arch/powerpc/mm/book3s64/hash_pgtable.c
> +++ b/arch/powerpc/mm/book3s64/hash_pgtable.c
> @@ -165,7 +165,7 @@ int hash__map_kernel_page(unsigned long ea, unsigned long pa, pgprot_t prot)
> ptep = pte_alloc_kernel(pmdp, ea);
> if (!ptep)
> return -ENOMEM;
> - set_pte_at(&init_mm, ea, ptep, pfn_pte(pa >> PAGE_SHIFT, prot));
> + set_pte_at_unchecked(&init_mm, ea, ptep, pfn_pte(pa >> PAGE_SHIFT, prot));
> } else {
> /*
> * If the mm subsystem is not fully up, we cannot create a
> diff --git a/arch/powerpc/mm/book3s64/pgtable.c b/arch/powerpc/mm/book3s64/pgtable.c
> index 3438ab72c346..25082ab6018b 100644
> --- a/arch/powerpc/mm/book3s64/pgtable.c
> +++ b/arch/powerpc/mm/book3s64/pgtable.c
> @@ -116,7 +116,7 @@ void set_pmd_at(struct mm_struct *mm, unsigned long addr,
> WARN_ON(!(pmd_large(pmd)));
> #endif
> trace_hugepage_set_pmd(addr, pmd_val(pmd));
> - return set_pte_at(mm, addr, pmdp_ptep(pmdp), pmd_pte(pmd));
> + return set_pte_at_unchecked(mm, addr, pmdp_ptep(pmdp), pmd_pte(pmd));
> }
>
> void set_pud_at(struct mm_struct *mm, unsigned long addr,
> @@ -133,7 +133,7 @@ void set_pud_at(struct mm_struct *mm, unsigned long addr,
> WARN_ON(!(pud_large(pud)));
> #endif
> trace_hugepage_set_pud(addr, pud_val(pud));
> - return set_pte_at(mm, addr, pudp_ptep(pudp), pud_pte(pud));
> + return set_pte_at_unchecked(mm, addr, pudp_ptep(pudp), pud_pte(pud));
> }
>
> static void do_serialize(void *arg)
> @@ -539,7 +539,7 @@ void ptep_modify_prot_commit(struct vm_area_struct *vma, unsigned long addr,
> if (radix_enabled())
> return radix__ptep_modify_prot_commit(vma, addr,
> ptep, old_pte, pte);
> - set_pte_at(vma->vm_mm, addr, ptep, pte);
> + set_pte_at_unchecked(vma->vm_mm, addr, ptep, pte);
> }
>
> #ifdef CONFIG_TRANSPARENT_HUGEPAGE
> diff --git a/arch/powerpc/mm/book3s64/radix_pgtable.c b/arch/powerpc/mm/book3s64/radix_pgtable.c
> index 46fa46ce6526..c661e42bb2f1 100644
> --- a/arch/powerpc/mm/book3s64/radix_pgtable.c
> +++ b/arch/powerpc/mm/book3s64/radix_pgtable.c
> @@ -109,7 +109,7 @@ static int early_map_kernel_page(unsigned long ea, unsigned long pa,
> ptep = pte_offset_kernel(pmdp, ea);
>
> set_the_pte:
> - set_pte_at(&init_mm, ea, ptep, pfn_pte(pfn, flags));
> + set_pte_at_unchecked(&init_mm, ea, ptep, pfn_pte(pfn, flags));
> asm volatile("ptesync": : :"memory");
> return 0;
> }
> @@ -1522,7 +1522,7 @@ void radix__ptep_modify_prot_commit(struct vm_area_struct *vma,
> (atomic_read(&mm->context.copros) > 0))
> radix__flush_tlb_page(vma, addr);
>
> - set_pte_at(mm, addr, ptep, pte);
> + set_pte_at_unchecked(mm, addr, ptep, pte);
> }
>
> int pud_set_huge(pud_t *pud, phys_addr_t addr, pgprot_t prot)
> @@ -1533,7 +1533,7 @@ int pud_set_huge(pud_t *pud, phys_addr_t addr, pgprot_t prot)
> if (!radix_enabled())
> return 0;
>
> - set_pte_at(&init_mm, 0 /* radix unused */, ptep, new_pud);
> + set_pte_at_unchecked(&init_mm, 0 /* radix unused */, ptep, new_pud);
>
> return 1;
> }
> @@ -1580,7 +1580,7 @@ int pmd_set_huge(pmd_t *pmd, phys_addr_t addr, pgprot_t prot)
> if (!radix_enabled())
> return 0;
>
> - set_pte_at(&init_mm, 0 /* radix unused */, ptep, new_pmd);
> + set_pte_at_unchecked(&init_mm, 0 /* radix unused */, ptep, new_pmd);
>
> return 1;
> }
> diff --git a/arch/powerpc/mm/nohash/book3e_pgtable.c b/arch/powerpc/mm/nohash/book3e_pgtable.c
> index 1c5e4ecbebeb..10d487b2b991 100644
> --- a/arch/powerpc/mm/nohash/book3e_pgtable.c
> +++ b/arch/powerpc/mm/nohash/book3e_pgtable.c
> @@ -111,7 +111,7 @@ int __ref map_kernel_page(unsigned long ea, phys_addr_t pa, pgprot_t prot)
> }
> ptep = pte_offset_kernel(pmdp, ea);
> }
> - set_pte_at(&init_mm, ea, ptep, pfn_pte(pa >> PAGE_SHIFT, prot));
> + set_pte_at_unchecked(&init_mm, ea, ptep, pfn_pte(pa >> PAGE_SHIFT, prot));
>
> smp_wmb();
> return 0;
> diff --git a/arch/powerpc/mm/pgtable.c b/arch/powerpc/mm/pgtable.c
> index e8e0289d7ab0..352679cf2684 100644
> --- a/arch/powerpc/mm/pgtable.c
> +++ b/arch/powerpc/mm/pgtable.c
> @@ -227,6 +227,13 @@ void set_ptes(struct mm_struct *mm, unsigned long addr, pte_t *ptep,
> }
> }
>
> +void set_pte_at_unchecked(struct mm_struct *mm, unsigned long addr,
> + pte_t *ptep, pte_t pte)
> +{
No need of the
VM_WARN_ON(pte_hw_valid(*ptep) && !pte_protnone(*ptep));
which is in set_ptes() ?
> + pte = set_pte_filter(pte, addr);
> + __set_pte_at(mm, addr, ptep, pte, 0);
> +}
> +
> void unmap_kernel_page(unsigned long va)
> {
> pmd_t *pmdp = pmd_off_k(va);
> diff --git a/arch/powerpc/mm/pgtable_32.c b/arch/powerpc/mm/pgtable_32.c
> index 4be97b4a44f9..a5a26faf91ec 100644
> --- a/arch/powerpc/mm/pgtable_32.c
> +++ b/arch/powerpc/mm/pgtable_32.c
> @@ -89,7 +89,7 @@ int __ref map_kernel_page(unsigned long va, phys_addr_t pa, pgprot_t prot)
> * hash table
> */
> BUG_ON((pte_present(*pg) | pte_hashpte(*pg)) && pgprot_val(prot));
> - set_pte_at(&init_mm, va, pg, pfn_pte(pa >> PAGE_SHIFT, prot));
> + set_pte_at_unchecked(&init_mm, va, pg, pfn_pte(pa >> PAGE_SHIFT, prot));
> }
> smp_wmb();
> return err;
next prev parent reply other threads:[~2024-03-13 11:31 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-03-13 4:21 [PATCH v10 00/12] Support page table check PowerPC Rohan McLure
2024-03-13 4:21 ` [PATCH v10 01/12] Revert "mm/page_table_check: remove unused parameter in [__]page_table_check_pud_set" Rohan McLure
2024-03-13 4:21 ` [PATCH v10 02/12] Revert "mm/page_table_check: remove unused parameter in [__]page_table_check_pmd_set" Rohan McLure
2024-03-13 4:21 ` [PATCH v10 03/12] mm: Provide addr parameter to page_table_check_pte_set() Rohan McLure
2024-03-13 4:21 ` [PATCH v10 04/12] Revert "mm/page_table_check: remove unused parameter in [__]page_table_check_pud_clear" Rohan McLure
2024-03-13 4:21 ` [PATCH v10 05/12] Revert "mm/page_table_check: remove unused parameter in [__]page_table_check_pmd_clear" Rohan McLure
2024-03-13 4:21 ` [PATCH v10 06/12] Revert "mm/page_table_check: remove unused parameter in [__]page_table_check_pte_clear" Rohan McLure
2024-03-13 4:21 ` [PATCH v10 07/12] mm: Provide address parameter to p{te,md,ud}_user_accessible_page() Rohan McLure
2024-03-13 4:21 ` [PATCH v10 08/12] powerpc: mm: Replace p{u,m,4}d_is_leaf with p{u,m,4}_leaf Rohan McLure
2024-03-13 10:33 ` Christophe Leroy
2024-03-13 4:21 ` [PATCH v10 09/12] powerpc: mm: Add common pud_pfn stub for all platforms Rohan McLure
2024-03-13 11:08 ` Christophe Leroy
2024-03-14 0:18 ` LTC IMAP
2024-03-13 4:21 ` [PATCH v10 10/12] poweprc: mm: Implement *_user_accessible_page() for ptes Rohan McLure
2024-03-13 11:19 ` Christophe Leroy
2024-03-13 4:21 ` [PATCH v10 11/12] powerpc: mm: Use set_pte_at_unchecked() for early-boot / internal usages Rohan McLure
2024-03-13 11:30 ` Christophe Leroy [this message]
2024-03-15 0:45 ` LTC IMAP
2024-03-13 4:21 ` [PATCH v10 12/12] powerpc: mm: Support page table check Rohan McLure
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=42bf1fb2-8a8e-4ded-97a6-da2617e89f5b@csgroup.eu \
--to=christophe.leroy@csgroup.eu \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-mm@kvack.org \
--cc=linux-riscv@lists.infradead.org \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=rmclure@linux.ibm.com \
--cc=x86@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).