2024-01-17 19:58:01

by Alexandre Ghiti

[permalink] [raw]
Subject: [PATCH -fixes 0/2] svnapot fixes

While merging riscv napot and arm64 contpte support, I noticed we did
not abide by the specification which states that we should clear a
napot mapping before setting a new one, called "break before make" in
arm64 (patch 1). And also that we did not add the new hugetlb page size
added by napot in hugetlb_mask_last_page() (patch 2).

Alexandre Ghiti (2):
riscv: Fix set_huge_pte_at() for NAPOT mapping
riscv: Fix hugetlb_mask_last_page() when NAPOT is enabled

arch/riscv/mm/hugetlbpage.c | 62 +++++++++++++++++++++++++++++++++++--
1 file changed, 60 insertions(+), 2 deletions(-)

--
2.39.2



2024-01-17 19:59:02

by Alexandre Ghiti

[permalink] [raw]
Subject: [PATCH 1/2] riscv: Fix set_huge_pte_at() for NAPOT mapping

As stated by the privileged specification, we must clear a NAPOT
mapping and emit a sfence.vma before setting a new translation.

Fixes: 82a1a1f3bfb6 ("riscv: mm: support Svnapot in hugetlb page")
Signed-off-by: Alexandre Ghiti <[email protected]>
---
arch/riscv/mm/hugetlbpage.c | 42 +++++++++++++++++++++++++++++++++++--
1 file changed, 40 insertions(+), 2 deletions(-)

diff --git a/arch/riscv/mm/hugetlbpage.c b/arch/riscv/mm/hugetlbpage.c
index b52f0210481f..24c0179565d8 100644
--- a/arch/riscv/mm/hugetlbpage.c
+++ b/arch/riscv/mm/hugetlbpage.c
@@ -177,13 +177,36 @@ pte_t arch_make_huge_pte(pte_t entry, unsigned int shift, vm_flags_t flags)
return entry;
}

+static void clear_flush(struct mm_struct *mm,
+ unsigned long addr,
+ pte_t *ptep,
+ unsigned long pgsize,
+ unsigned long ncontig)
+{
+ struct vm_area_struct vma = TLB_FLUSH_VMA(mm, 0);
+ unsigned long i, saddr = addr;
+
+ for (i = 0; i < ncontig; i++, addr += pgsize, ptep++)
+ ptep_get_and_clear(mm, addr, ptep);
+
+ flush_tlb_range(&vma, saddr, addr);
+}
+
+/*
+ * When dealing with NAPOT mappings, the privileged specification indicates that
+ * "if an update needs to be made, the OS generally should first mark all of the
+ * PTEs invalid, then issue SFENCE.VMA instruction(s) covering all 4 KiB regions
+ * within the range, [...] then update the PTE(s), as described in Section
+ * 4.2.1.". That's the equivalent of the Break-Before-Make approach used by
+ * arm64.
+ */
void set_huge_pte_at(struct mm_struct *mm,
unsigned long addr,
pte_t *ptep,
pte_t pte,
unsigned long sz)
{
- unsigned long hugepage_shift;
+ unsigned long hugepage_shift, pgsize;
int i, pte_num;

if (sz >= PGDIR_SIZE)
@@ -198,7 +221,22 @@ void set_huge_pte_at(struct mm_struct *mm,
hugepage_shift = PAGE_SHIFT;

pte_num = sz >> hugepage_shift;
- for (i = 0; i < pte_num; i++, ptep++, addr += (1 << hugepage_shift))
+ pgsize = 1 << hugepage_shift;
+
+ if (!pte_present(pte)) {
+ for (i = 0; i < pte_num; i++, ptep++, addr += pgsize)
+ set_ptes(mm, addr, ptep, pte, 1);
+ return;
+ }
+
+ if (!pte_napot(pte)) {
+ set_ptes(mm, addr, ptep, pte, 1);
+ return;
+ }
+
+ clear_flush(mm, addr, ptep, pgsize, pte_num);
+
+ for (i = 0; i < pte_num; i++, ptep++, addr += pgsize)
set_pte_at(mm, addr, ptep, pte);
}

--
2.39.2


2024-01-17 20:00:04

by Alexandre Ghiti

[permalink] [raw]
Subject: [PATCH 2/2] riscv: Fix hugetlb_mask_last_page() when NAPOT is enabled

When NAPOT is enabled, a new hugepage size is available and then we need
to make hugetlb_mask_last_page() aware of that.

Fixes: 82a1a1f3bfb6 ("riscv: mm: support Svnapot in hugetlb page")
Signed-off-by: Alexandre Ghiti <[email protected]>
---
arch/riscv/mm/hugetlbpage.c | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)

diff --git a/arch/riscv/mm/hugetlbpage.c b/arch/riscv/mm/hugetlbpage.c
index 24c0179565d8..87af75ee7186 100644
--- a/arch/riscv/mm/hugetlbpage.c
+++ b/arch/riscv/mm/hugetlbpage.c
@@ -125,6 +125,26 @@ pte_t *huge_pte_offset(struct mm_struct *mm,
return pte;
}

+unsigned long hugetlb_mask_last_page(struct hstate *h)
+{
+ unsigned long hp_size = huge_page_size(h);
+
+ switch (hp_size) {
+#ifndef __PAGETABLE_PMD_FOLDED
+ case PUD_SIZE:
+ return P4D_SIZE - PUD_SIZE;
+#endif
+ case PMD_SIZE:
+ return PUD_SIZE - PMD_SIZE;
+ case napot_cont_size(NAPOT_CONT64KB_ORDER):
+ return PMD_SIZE - napot_cont_size(NAPOT_CONT64KB_ORDER);
+ default:
+ break;
+ }
+
+ return 0UL;
+}
+
static pte_t get_clear_contig(struct mm_struct *mm,
unsigned long addr,
pte_t *ptep,
--
2.39.2


Subject: Re: [PATCH -fixes 0/2] svnapot fixes

Hello:

This series was applied to riscv/linux.git (fixes)
by Palmer Dabbelt <[email protected]>:

On Wed, 17 Jan 2024 20:57:39 +0100 you wrote:
> While merging riscv napot and arm64 contpte support, I noticed we did
> not abide by the specification which states that we should clear a
> napot mapping before setting a new one, called "break before make" in
> arm64 (patch 1). And also that we did not add the new hugetlb page size
> added by napot in hugetlb_mask_last_page() (patch 2).
>
> Alexandre Ghiti (2):
> riscv: Fix set_huge_pte_at() for NAPOT mapping
> riscv: Fix hugetlb_mask_last_page() when NAPOT is enabled
>
> [...]

Here is the summary with links:
- [1/2] riscv: Fix set_huge_pte_at() for NAPOT mapping
https://git.kernel.org/riscv/c/1458eb2c9d88
- [2/2] riscv: Fix hugetlb_mask_last_page() when NAPOT is enabled
https://git.kernel.org/riscv/c/a179a4bfb694

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html