Le 18/10/2023 à 06:55, Aneesh Kumar K.V a écrit :
> With commit 9fee28baa601 ("powerpc: implement the new page table range
> API") we added set_ptes to powerpc architecture but the implementation
> missed calling the pte filter for all the ptes we are setting in the
> range. set_pte_filter can be used for filter pte values and on some
> platforms which don't support coherent icache it clears the exec bit so
> that we can flush the icache on exec fault
>
> The patch also removes the usage of arch_enter/leave_lazy_mmu() because
> set_pte is not supposed to be used when updating a pte entry. Powerpc
> architecture uses this rule to skip the expensive tlb invalidate which
> is not needed when you are setting up the pte for the first time. See
> commit 56eecdb912b5 ("mm: Use ptep/pmdp_set_numa() for updating
> _PAGE_NUMA bit") for more details
>
> Fixes: 9fee28baa601 ("powerpc: implement the new page table range API")
> Signed-off-by: Aneesh Kumar K.V <aneesh.ku...@linux.ibm.com>
> ---
> arch/powerpc/mm/pgtable.c | 33 ++++++++++++++++++++-------------
> 1 file changed, 20 insertions(+), 13 deletions(-)
>
> diff --git a/arch/powerpc/mm/pgtable.c b/arch/powerpc/mm/pgtable.c
> index 3ba9fe411604..95ab20cca2da 100644
> --- a/arch/powerpc/mm/pgtable.c
> +++ b/arch/powerpc/mm/pgtable.c
> @@ -191,28 +191,35 @@ void set_ptes(struct mm_struct *mm, unsigned long addr,
> pte_t *ptep,
> pte_t pte, unsigned int nr)
> {
> /*
> - * Make sure hardware valid bit is not set. We don't do
> - * tlb flush for this update.
> + * We don't need to call arch_enter/leave_lazy_mmu_mode()
> + * because we expect set_ptes to be only be used on not present
> + * and not hw_valid ptes. Hence there is not translation cache flush
> + * involved that need to be batched.
> */
> - VM_WARN_ON(pte_hw_valid(*ptep) && !pte_protnone(*ptep));
> + for (;;) {
>
> - /* Note: mm->context.id might not yet have been assigned as
> - * this context might not have been activated yet when this
> - * is called.
> - */
> - pte = set_pte_filter(pte);
> + /*
> + * Make sure hardware valid bit is not set. We don't do
> + * tlb flush for this update.
> + */
> + VM_WARN_ON(pte_hw_valid(*ptep) && !pte_protnone(*ptep));
>
> - /* Perform the setting of the PTE */
> - arch_enter_lazy_mmu_mode();
> - for (;;) {
> + /* Note: mm->context.id might not yet have been assigned as
> + * this context might not have been activated yet when this
> + * is called.
> + */
> + pte = set_pte_filter(pte);
Why do you need to call set_pte_filter() inside the loop ?
The only difference between previous pte and next pte is the RPN, other
flags remain untouched so I can't see why you need to call
set_pte_filter() again.
> +
> + /* Perform the setting of the PTE */
> __set_pte_at(mm, addr, ptep, pte, 0);
> if (--nr == 0)
> break;
> ptep++;
> - pte = __pte(pte_val(pte) + (1UL << PTE_RPN_SHIFT));
> addr += PAGE_SIZE;
> + /* increment the pfn */
> + pte = __pte(pte_val(pte) + PAGE_SIZE);
PAGE_SIZE doesn't work on all platforms, see for instance e500.
see comment at
https://elixir.bootlin.com/linux/v6.3-rc2/source/arch/powerpc/include/asm/nohash/32/pgtable.h#L147
And then you see
https://elixir.bootlin.com/linux/v6.3-rc2/source/arch/powerpc/include/asm/nohash/pte-e500.h#L63
> +
> }
> - arch_leave_lazy_mmu_mode();
> }
>
> void unmap_kernel_page(unsigned long va)
Christophe
