Google Groups no longer supports new Usenet posts or subscriptions. Historical content remains viewable.
Dismiss

[PATCH] arm64: Add support for PTE contiguous bit.

319 views
Skip to first unread message

David Woods

unread,
Sep 15, 2015, 2:17:49 PM9/15/15
to Chris Metcalf, David Woods, Catalin Marinas, Will Deacon, Steve Capper, Marc Zyngier, Hugh Dickins, Mike Kravetz, Naoya Horiguchi, Andrew Morton, linux-ar...@lists.infradead.org, linux-...@vger.kernel.org, linu...@kvack.org
The arm64 MMU supports a Contiguous bit which is a hint that the TTE
is one of a set of contiguous entries which can be cached in a single
TLB entry. Supporting this bit adds new intermediate huge page sizes.

The set of huge page sizes available depends on the base page size.
Without using contiguous pages the huge page sizes are as follows.

4KB: 2MB 1GB
64KB: 512MB 4TB

With 4KB pages, the contiguous bit groups together sets of 16 pages
and with 64KB pages it groups sets of 32 pages. This enables two new
huge page sizes in each case, so that the full set of available sizes
is as follows.

4KB: 64KB 2MB 32MB 1GB
64KB: 2MB 512MB 16GB 4TB

If the base page size is set to 64KB then 2MB pages are enabled by
default. It is possible in the future to make 2MB the default huge
page size for both 4KB and 64KB pages.

Signed-off-by: David Woods <dwo...@ezchip.com>
Reviewed-by: Chris Metcalf <cmet...@ezchip.com>
---
arch/arm64/Kconfig | 3 -
arch/arm64/include/asm/hugetlb.h | 4 +
arch/arm64/include/asm/pgtable-hwdef.h | 15 +++
arch/arm64/include/asm/pgtable.h | 30 +++++-
arch/arm64/mm/hugetlbpage.c | 165 ++++++++++++++++++++++++++++++++-
5 files changed, 210 insertions(+), 7 deletions(-)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 7d95663..8310e38 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -447,9 +447,6 @@ config HW_PERF_EVENTS
config SYS_SUPPORTS_HUGETLBFS
def_bool y

-config ARCH_WANT_GENERAL_HUGETLB
- def_bool y
-
config ARCH_WANT_HUGE_PMD_SHARE
def_bool y if !ARM64_64K_PAGES

diff --git a/arch/arm64/include/asm/hugetlb.h b/arch/arm64/include/asm/hugetlb.h
index bb4052e..e5af553 100644
--- a/arch/arm64/include/asm/hugetlb.h
+++ b/arch/arm64/include/asm/hugetlb.h
@@ -97,4 +97,8 @@ static inline void arch_clear_hugepage_flags(struct page *page)
clear_bit(PG_dcache_clean, &page->flags);
}

+extern pte_t arch_make_huge_pte(pte_t entry, struct vm_area_struct *vma,
+ struct page *page, int writable);
+#define arch_make_huge_pte arch_make_huge_pte
+
#endif /* __ASM_HUGETLB_H */
diff --git a/arch/arm64/include/asm/pgtable-hwdef.h b/arch/arm64/include/asm/pgtable-hwdef.h
index 24154b0..da73243 100644
--- a/arch/arm64/include/asm/pgtable-hwdef.h
+++ b/arch/arm64/include/asm/pgtable-hwdef.h
@@ -55,6 +55,19 @@
#define SECTION_MASK (~(SECTION_SIZE-1))

/*
+ * Contiguous large page definitions.
+ */
+#ifdef CONFIG_ARM64_64K_PAGES
+#define CONTIG_SHIFT 5
+#define CONTIG_PAGES 32
+#else
+#define CONTIG_SHIFT 4
+#define CONTIG_PAGES 16
+#endif
+#define CONTIG_PTE_SIZE (CONTIG_PAGES * PAGE_SIZE)
+#define CONTIG_PTE_MASK (~(CONTIG_PTE_SIZE - 1))
+
+/*
* Hardware page table definitions.
*
* Level 1 descriptor (PUD).
@@ -83,6 +96,7 @@
#define PMD_SECT_S (_AT(pmdval_t, 3) << 8)
#define PMD_SECT_AF (_AT(pmdval_t, 1) << 10)
#define PMD_SECT_NG (_AT(pmdval_t, 1) << 11)
+#define PMD_SECT_CONTIG (_AT(pmdval_t, 1) << 52)
#define PMD_SECT_PXN (_AT(pmdval_t, 1) << 53)
#define PMD_SECT_UXN (_AT(pmdval_t, 1) << 54)

@@ -105,6 +119,7 @@
#define PTE_AF (_AT(pteval_t, 1) << 10) /* Access Flag */
#define PTE_NG (_AT(pteval_t, 1) << 11) /* nG */
#define PTE_DBM (_AT(pteval_t, 1) << 51) /* Dirty Bit Management */
+#define PTE_CONTIG (_AT(pteval_t, 1) << 52) /* Contiguous */
#define PTE_PXN (_AT(pteval_t, 1) << 53) /* Privileged XN */
#define PTE_UXN (_AT(pteval_t, 1) << 54) /* User XN */

diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
index 6900b2d9..df5ec64 100644
--- a/arch/arm64/include/asm/pgtable.h
+++ b/arch/arm64/include/asm/pgtable.h
@@ -144,6 +144,7 @@ extern struct page *empty_zero_page;
#define pte_special(pte) (!!(pte_val(pte) & PTE_SPECIAL))
#define pte_write(pte) (!!(pte_val(pte) & PTE_WRITE))
#define pte_exec(pte) (!(pte_val(pte) & PTE_UXN))
+#define pte_contig(pte) (!!(pte_val(pte) & PTE_CONTIG))

#ifdef CONFIG_ARM64_HW_AFDBM
#define pte_hw_dirty(pte) (!(pte_val(pte) & PTE_RDONLY))
@@ -206,6 +207,9 @@ static inline pte_t pte_mkspecial(pte_t pte)
return set_pte_bit(pte, __pgprot(PTE_SPECIAL));
}

+extern pte_t pte_mkcontig(pte_t pte);
+extern pmd_t pmd_mkcontig(pmd_t pmd);
+
static inline void set_pte(pte_t *ptep, pte_t pte)
{
*ptep = pte;
@@ -275,7 +279,7 @@ static inline void set_pte_at(struct mm_struct *mm, unsigned long addr,
/*
* Hugetlb definitions.
*/
-#define HUGE_MAX_HSTATE 2
+#define HUGE_MAX_HSTATE ((2 * CONFIG_PGTABLE_LEVELS) - 1)
#define HPAGE_SHIFT PMD_SHIFT
#define HPAGE_SIZE (_AC(1, UL) << HPAGE_SHIFT)
#define HPAGE_MASK (~(HPAGE_SIZE - 1))
@@ -372,7 +376,8 @@ extern pgprot_t phys_mem_access_prot(struct file *file, unsigned long pfn,
#define pmd_none(pmd) (!pmd_val(pmd))
#define pmd_present(pmd) (pmd_val(pmd))

-#define pmd_bad(pmd) (!(pmd_val(pmd) & 2))
+#define pmd_bad(pmd) (!(pmd_val(pmd) & \
+ (PMD_TABLE_BIT | PMD_SECT_CONTIG)))

#define pmd_table(pmd) ((pmd_val(pmd) & PMD_TYPE_MASK) == \
PMD_TYPE_TABLE)
@@ -500,7 +505,8 @@ static inline pud_t *pud_offset(pgd_t *pgd, unsigned long addr)
static inline pte_t pte_modify(pte_t pte, pgprot_t newprot)
{
const pteval_t mask = PTE_USER | PTE_PXN | PTE_UXN | PTE_RDONLY |
- PTE_PROT_NONE | PTE_WRITE | PTE_TYPE_MASK;
+ PTE_PROT_NONE | PTE_WRITE | PTE_TYPE_MASK |
+ PTE_CONTIG;
/* preserve the hardware dirty information */
if (pte_hw_dirty(pte))
newprot |= PTE_DIRTY;
@@ -513,6 +519,24 @@ static inline pmd_t pmd_modify(pmd_t pmd, pgprot_t newprot)
return pte_pmd(pte_modify(pmd_pte(pmd), newprot));
}

+static inline pte_t pte_modify_pfn(pte_t pte, unsigned long newpfn)
+{
+ const pteval_t mask = PHYS_MASK & PAGE_MASK;
+
+ pte_val(pte) = pfn_pte(newpfn, (pte_val(pte) & ~mask));
+ return pte;
+}
+
+#if CONFIG_PGTABLE_LEVELS > 2
+static inline pmd_t pmd_modify_pfn(pmd_t pmd, unsigned long newpfn)
+{
+ const pmdval_t mask = PHYS_MASK & PAGE_MASK;
+
+ pmd = pfn_pmd(newpfn, (pmd_val(pmd) & ~mask));
+ return pmd;
+}
+#endif
+
#ifdef CONFIG_ARM64_HW_AFDBM
/*
* Atomic pte/pmd modifications.
diff --git a/arch/arm64/mm/hugetlbpage.c b/arch/arm64/mm/hugetlbpage.c
index 383b03f..f5bbbbc 100644
--- a/arch/arm64/mm/hugetlbpage.c
+++ b/arch/arm64/mm/hugetlbpage.c
@@ -41,6 +41,155 @@ int pud_huge(pud_t pud)
#endif
}

+pte_t *huge_pte_alloc(struct mm_struct *mm,
+ unsigned long addr, unsigned long sz)
+{
+ pgd_t *pgd;
+ pud_t *pud;
+ pte_t *pte = NULL;
+ int i;
+
+ pgd = pgd_offset(mm, addr);
+ pud = pud_alloc(mm, pgd, addr);
+ if (pud) {
+ if (sz == PUD_SIZE) {
+ pte = (pte_t *)pud;
+ } else if (sz == PMD_SIZE) {
+#ifdef CONFIG_ARCH_WANT_HUGE_PMD_SHARE
+ if (pud_none(*pud))
+ pte = huge_pmd_share(mm, addr, pud);
+ else
+#endif
+ pte = (pte_t *)pmd_alloc(mm, pud, addr);
+ } else if (sz == (PAGE_SIZE * CONTIG_PAGES)) {
+ pmd_t *pmd = pmd_alloc(mm, pud, addr);
+
+ WARN_ON(addr & (sz - 1));
+ pte = pte_alloc_map(mm, NULL, pmd, addr);
+ if (pte_present(*pte)) {
+ unsigned long pfn;
+ *pte = pte_mkcontig(*pte);
+ pfn = pte_pfn(*pte);
+ for (i = 0; i < CONTIG_PAGES; i++) {
+ set_pte(&pte[i],
+ pte_modify_pfn(*pte, pfn + i));
+ }
+ }
+#if CONFIG_PGTABLE_LEVELS > 2
+ } else if (sz == (PMD_SIZE * CONTIG_PAGES)) {
+ pmd_t *pmd;
+
+ pmd = pmd_alloc(mm, pud, addr);
+ WARN_ON(addr & (sz - 1));
+ if (pmd && pmd_present(*pmd)) {
+ unsigned long pfn;
+ pmd_t pmdval;
+
+ pmdval = *pmd = pmd_mkcontig(*pmd);
+ pfn = pmd_pfn(*pmd);
+ for (i = 0; i < CONTIG_PAGES; i++) {
+ unsigned long newpfn = pfn +
+ (i << (PMD_SHIFT - PAGE_SHIFT));
+ if (!pmd_present(pmd[i]))
+ atomic_long_inc(&mm->nr_ptes);
+ set_pmd(&pmd[i],
+ pmd_modify_pfn(pmdval, newpfn));
+ }
+ }
+ return pmd;
+#endif
+ }
+ }
+
+ return pte;
+}
+
+pte_t *huge_pte_offset(struct mm_struct *mm, unsigned long addr)
+{
+ pgd_t *pgd;
+ pud_t *pud;
+ pmd_t *pmd = NULL;
+ pte_t *pte = NULL;
+
+ pgd = pgd_offset(mm, addr);
+ if (pgd_present(*pgd)) {
+ pud = pud_offset(pgd, addr);
+ if (pud_present(*pud)) {
+ if (pud_huge(*pud))
+ return (pte_t *)pud;
+ pmd = pmd_offset(pud, addr);
+ if (pmd_present(*pmd)) {
+ if (pmd_huge(*pmd))
+ return (pte_t *)pmd;
+ pte = pte_offset_kernel(pmd, addr);
+ if (pte_present(*pte) && pte_contig(*pte)) {
+ pte = pte_offset_kernel(
+ pmd, (addr & CONTIG_PTE_MASK));
+ return pte;
+ }
+ }
+ }
+ }
+ return (pte_t *) NULL;
+}
+
+pte_t arch_make_huge_pte(pte_t entry, struct vm_area_struct *vma,
+ struct page *page, int writable)
+{
+ size_t pagesize = huge_page_size(hstate_vma(vma));
+ pte_t nent = {0};
+
+ if (pagesize == PUD_SIZE || pagesize == PMD_SIZE)
+ nent = entry;
+ else if (pagesize == (PAGE_SIZE * CONTIG_PAGES))
+ nent = pte_mkcontig(entry);
+#if CONFIG_PGTABLE_LEVELS > 2
+ else if (pagesize == (PMD_SIZE * CONTIG_PAGES) ||
+ pagesize == (PUD_SIZE * CONTIG_PAGES))
+ nent = pmd_mkcontig(entry);
+#endif
+ else {
+ pr_warn("%s: unrecognized huge page size 0x%lx\n",
+ __func__, pagesize);
+ }
+ return nent;
+}
+
+pte_t pte_mkcontig(pte_t pte)
+{
+ pte = set_pte_bit(pte, __pgprot(PTE_CONTIG));
+ pte = set_pte_bit(pte, __pgprot(PTE_TYPE_PAGE));
+ return pte;
+}
+
+pmd_t pmd_mkcontig(pmd_t pmd)
+{
+ pmd = __pmd(pmd_val(pmd) | PMD_SECT_CONTIG);
+ return pmd;
+}
+
+struct page *follow_huge_pmd(struct mm_struct *mm, unsigned long address,
+ pmd_t *pmd, int write)
+{
+ struct page *page;
+
+ page = pte_page(*(pte_t *)pmd);
+ if (page)
+ page += ((address & ~PMD_MASK) >> PAGE_SHIFT);
+ return page;
+}
+
+struct page *follow_huge_pud(struct mm_struct *mm, unsigned long address,
+ pud_t *pud, int write)
+{
+ struct page *page;
+
+ page = pte_page(*(pte_t *)pud);
+ if (page)
+ page += ((address & ~PUD_MASK) >> PAGE_SHIFT);
+ return page;
+}
+
static __init int setup_hugepagesz(char *opt)
{
unsigned long ps = memparse(opt, &opt);
@@ -48,10 +197,24 @@ static __init int setup_hugepagesz(char *opt)
hugetlb_add_hstate(PMD_SHIFT - PAGE_SHIFT);
} else if (ps == PUD_SIZE) {
hugetlb_add_hstate(PUD_SHIFT - PAGE_SHIFT);
+ } else if (ps == (PAGE_SIZE * CONTIG_PAGES)) {
+ hugetlb_add_hstate(CONTIG_SHIFT);
+ } else if (ps == (PMD_SIZE * CONTIG_PAGES)) {
+ hugetlb_add_hstate((PMD_SHIFT + CONTIG_SHIFT) - PAGE_SHIFT);
} else {
- pr_err("hugepagesz: Unsupported page size %lu M\n", ps >> 20);
+ pr_err("hugepagesz: Unsupported page size %lu K\n", ps >> 10);
return 0;
}
return 1;
}
__setup("hugepagesz=", setup_hugepagesz);
+
+#ifdef CONFIG_ARM64_64K_PAGES
+static __init int add_default_hugepagesz(void)
+{
+ if (size_to_hstate(CONTIG_PAGES * PAGE_SIZE) == NULL)
+ hugetlb_add_hstate(CONTIG_SHIFT);
+ return 0;
+}
+arch_initcall(add_default_hugepagesz);
+#endif
--
2.1.2

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majo...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Steve Capper

unread,
Sep 16, 2015, 4:46:42 AM9/16/15
to David Woods, Chris Metcalf, Catalin Marinas, Will Deacon, Marc Zyngier, Hugh Dickins, Mike Kravetz, Naoya Horiguchi, Andrew Morton, linux-ar...@lists.infradead.org, linux-...@vger.kernel.org, linu...@kvack.org, Suzuki K. Poulose
On 15 September 2015 at 19:01, David Woods <dwo...@ezchip.com> wrote:
> The arm64 MMU supports a Contiguous bit which is a hint that the TTE
> is one of a set of contiguous entries which can be cached in a single
> TLB entry. Supporting this bit adds new intermediate huge page sizes.
>
> The set of huge page sizes available depends on the base page size.
> Without using contiguous pages the huge page sizes are as follows.
>
> 4KB: 2MB 1GB
> 64KB: 512MB 4TB

We just have 512MB for a 64KB granule.
As per [1] D4.2.6 - "The VMSAv8-64 translation table format" page D4-1668.

>
> With 4KB pages, the contiguous bit groups together sets of 16 pages
> and with 64KB pages it groups sets of 32 pages. This enables two new
> huge page sizes in each case, so that the full set of available sizes
> is as follows.
>
> 4KB: 64KB 2MB 32MB 1GB
> 64KB: 2MB 512MB 16GB 4TB
>
> If the base page size is set to 64KB then 2MB pages are enabled by
> default. It is possible in the future to make 2MB the default huge
> page size for both 4KB and 64KB pages.
>

Hi David,
Thanks for posting this, and apologies in advance for talking about
the ARM ARM[1]...

D4.4.2 "Other fields in the VMSAv8-64 translation table format
descriptors" (page D4-1715)
Only gives examples of the contiguous bit being used for level 3
descriptors (i.e. PTEs) when running with a 4KB and 64KB granule.

With a 16KB granule we *can* have a contiguous bit being used by level
2 descriptors (i.e. PMDs), so the pmd_contig logic could perhaps be
used in combination with Suzuki's 16KB PAGE_SIZE series at:
http://lists.infradead.org/pipermail/linux-arm-kernel/2015-September/370117.html

I will read through the rest of the patch and post more feedback

Cheers,
--
Steve

[1] - http://infocenter.arm.com/help/topic/com.arm.doc.ddi0487a.g/index.html

Steve Capper

unread,
Sep 16, 2015, 10:06:36 AM9/16/15
to David Woods, Chris Metcalf, Catalin Marinas, Will Deacon, Marc Zyngier, Hugh Dickins, Mike Kravetz, Naoya Horiguchi, Andrew Morton, linux-ar...@lists.infradead.org, linux-...@vger.kernel.org, linu...@kvack.org
Hi David,
Some initial comments below.

Cheers,
--
Steve
Careful here, CONTIG_PAGES should really be CONTIG_PTES.

If support is added for a 16KB granule case we are allowed:
128 x 16KB pages (ptes) to make a 2MB huge page, or
32 x 32MB blocks (pmds) to make a 1GB huge page.

i.e we CONTIG_PTES != CONTIG_PMDs

For 4KB or 64KB pages we are only allowed contiguous pte's so
CONTIG_PMDS == 0 in these cases.
I'm not sure about this. A contiguous pmd (which will be a block descriptor)
will no longer be bad?
We can probably get rid of these two functions, please see below.
Why are we writing pte's/pmd's in the huge_pte_alloc function?
What happened to set_huge_pte_at?

Also, rather than call pte_modify_pfn, I would recommend something like:

int loop;
unsigned long pfn = pte_pfn(pte);
pgprot_t hugeprot = __pgprot(pte_val(pfn_pte(pfn, 0) ^ pte_val(pte)));

for (loop = 0; loop < CONTIG_PTES; loop++) {
set_pte_at(mm, addr, ptep++, pfn_pte(pfn++, hugeprot));
addr += PAGE_SIZE;
}

i.e. extract a pgprot_t and combine with the pfn in the loop rather than
calling out.
Can these be folded into arch_make_huge_pte?

> +
> +struct page *follow_huge_pmd(struct mm_struct *mm, unsigned long address,
> + pmd_t *pmd, int write)
> +{
> + struct page *page;
> +
> + page = pte_page(*(pte_t *)pmd);
> + if (page)
> + page += ((address & ~PMD_MASK) >> PAGE_SHIFT);
> + return page;
> +}

Do we need to think about contiguous pmd's here?
It may be worth implementing follow_huge_addr?

> +
> +struct page *follow_huge_pud(struct mm_struct *mm, unsigned long address,
> + pud_t *pud, int write)
> +{
> + struct page *page;
> +
> + page = pte_page(*(pte_t *)pud);
> + if (page)
> + page += ((address & ~PUD_MASK) >> PAGE_SHIFT);
> + return page;
> +}
> +
> static __init int setup_hugepagesz(char *opt)
> {
> unsigned long ps = memparse(opt, &opt);
> @@ -48,10 +197,24 @@ static __init int setup_hugepagesz(char *opt)
> hugetlb_add_hstate(PMD_SHIFT - PAGE_SHIFT);
> } else if (ps == PUD_SIZE) {
> hugetlb_add_hstate(PUD_SHIFT - PAGE_SHIFT);
> + } else if (ps == (PAGE_SIZE * CONTIG_PAGES)) {
> + hugetlb_add_hstate(CONTIG_SHIFT);
> + } else if (ps == (PMD_SIZE * CONTIG_PAGES)) {

We need to distinguish between CONTIG_PTES and CONTIG_PMDS.

Will Deacon

unread,
Sep 16, 2015, 1:05:13 PM9/16/15
to David Woods, Chris Metcalf, Catalin Marinas, Steve Capper, Marc Zyngier, Hugh Dickins, Mike Kravetz, Naoya Horiguchi, Andrew Morton, linux-ar...@lists.infradead.org, linux-...@vger.kernel.org, linu...@kvack.org
Hi David,
I glanced briefly at this, and I think you'll need to do some extra work
for the CONFIG_HW_AFDBM=y case, where the CPU can set access/dirty bits
in any (i.e. not necessarily all) of the page table entries in a
contiguous mapping. In this case, things like huge_pte_dirty might need
overriding.

Will

David Woods

unread,
Sep 21, 2015, 12:59:56 PM9/21/15
to Steve Capper, Chris Metcalf, Catalin Marinas, Will Deacon, Marc Zyngier, Hugh Dickins, Mike Kravetz, Naoya Horiguchi, Andrew Morton, linux-ar...@lists.infradead.org, linux-...@vger.kernel.org, linu...@kvack.org, Suzuki K. Poulose

Steve,

Thanks for your review and comments. I take your points about the 16k
granule - it's helpful to know that support is in the works. However,
I'm not sure I agree with your reading of section 4.4.2. It's clear that
for 16k granules, the number of contiguous pages is different for the
PTE and PMD levels. But I don't see anywhere it says that for 4K and
64K that the contig bit is not supported at the PMD level - just that
the number of contiguous pages is the same at each level.

I tried using the tarmac trace module of the ARM simulator to support
this idea by turning on MMU tracing. Using 4k granule, I created 64k
and 32m pages and touched each location in the page. In both cases, the
trace recorded just one TLB fill (rather than the 16 you'd expect if the
contiguous bit were being ignored) and it indicated the expected page size.

1817498494 clk cpu2 TLB FILL cpu2.S1TLB 64K 0x2000000000_NS vmid=0, nG
asid=303:0x08fa360000_NS Normal InnerShareable
Inner=WriteBackWriteAllocate Outer=WriteBackWriteAllocate xn=0 pxn=1
ContiguousHint=1

1263366314 clk cpu2 TLB FILL cpu2.UTLB 32M 0x2000000000_NS vmid=0, nG
asid=300:0x08f6000000_NS Normal InnerShareable
Inner=WriteBackWriteAllocate Outer=WriteBackWriteAllocate xn=0 pxn=1
ContiguousHint=1

I'll try this with a 64k granule next. I'm not sure what will happen
with 16G pages since we are using an A53 model which I don't think
supports such large pages.

-Dave

Steve Capper

unread,
Sep 25, 2015, 1:53:21 PM9/25/15
to David Woods, Chris Metcalf, Catalin Marinas, Will Deacon, Marc Zyngier, Hugh Dickins, Mike Kravetz, Naoya Horiguchi, Andrew Morton, linux-ar...@lists.infradead.org, linux-...@vger.kernel.org, linu...@kvack.org, Suzuki K. Poulose
On 21 September 2015 at 09:44, David Woods <dwo...@ezchip.com> wrote:
>
> Steve,

Hi Dave,

>
> Thanks for your review and comments. I take your points about the 16k
> granule - it's helpful to know that support is in the works. However, I'm
> not sure I agree with your reading of section 4.4.2. It's clear that for 16k
> granules, the number of contiguous pages is different for the PTE and PMD
> levels. But I don't see anywhere it says that for 4K and 64K that the
> contig bit is not supported at the PMD level - just that the number of
> contiguous pages is the same at each level.

Many apologies, I appear to have led you down the garden path there.
Having double checked at ARM, the valid contiguous page sizes are indeed:
4K granule:
16 x ptes = 64K
16 x pmds = 32M
16 x puds = 16G

16K granule:
128 x ptes = 2M
32 x pmds = 1G

64K granule:
32 x ptes = 2M
32 x pmds = 16G

>
> I tried using the tarmac trace module of the ARM simulator to support this
> idea by turning on MMU tracing. Using 4k granule, I created 64k and 32m
> pages and touched each location in the page. In both cases, the trace
> recorded just one TLB fill (rather than the 16 you'd expect if the
> contiguous bit were being ignored) and it indicated the expected page size.
>
> 1817498494 clk cpu2 TLB FILL cpu2.S1TLB 64K 0x2000000000_NS vmid=0, nG
> asid=303:0x08fa360000_NS Normal InnerShareable Inner=WriteBackWriteAllocate
> Outer=WriteBackWriteAllocate xn=0 pxn=1 ContiguousHint=1
>
> 1263366314 clk cpu2 TLB FILL cpu2.UTLB 32M 0x2000000000_NS vmid=0, nG
> asid=300:0x08f6000000_NS Normal InnerShareable Inner=WriteBackWriteAllocate
> Outer=WriteBackWriteAllocate xn=0 pxn=1 ContiguousHint=1
>
> I'll try this with a 64k granule next. I'm not sure what will happen with
> 16G pages since we are using an A53 model which I don't think supports such
> large pages.

The Cortex-A53 supported TLB sizes can be found in the TRM:
http://infocenter.arm.com/help/topic/com.arm.doc.ddi0500f/Chddiifa.html

My understanding is that the core is allowed to ignore the contiguous
bit if it doesn't support the particular TLB entry size, or substitute
in a slightly smaller TLB entry than hinted possible. Anyway, do give
it a go :-).

Cheers,
--
Steve

David Woods

unread,
Oct 19, 2015, 2:59:15 PM10/19/15
to Steve Capper, Chris Metcalf, Catalin Marinas, Will Deacon, Marc Zyngier, Hugh Dickins, Mike Kravetz, Naoya Horiguchi, Andrew Morton, linux-ar...@lists.infradead.org, linux-...@vger.kernel.org, linu...@kvack.org
Steve,

Thanks for pointing this out. I changed it to allow for different
values for CONT_PTES and CONT_PMDS. As you say, that should
make it easier to merge with the 16K granule support.
Right, this was not correct. The problem was that on process exit,
it was only clearing the first PTE in each contiguous block. The other
15 (or 31) would still be non-zero and get reported as "bad" on the
console. Fixing huge_ptep_get_and_clear() solved that problem and
made this hack to pmd_bad() unnecessary.
Ok, I've changed it as you suggested.
I agree, it's better this way. I moved all this stuff out of
huge_pte_alloc()
and into set_huge_pte_at().

>> >+}
>> >+
>> >+pte_t pte_mkcontig(pte_t pte)
>> >+{
>> >+ pte = set_pte_bit(pte, __pgprot(PTE_CONTIG));
>> >+ pte = set_pte_bit(pte, __pgprot(PTE_TYPE_PAGE));
>> >+ return pte;
>> >+}
>> >+
>> >+pmd_t pmd_mkcontig(pmd_t pmd)
>> >+{
>> >+ pmd = __pmd(pmd_val(pmd) | PMD_SECT_CONTIG);
>> >+ return pmd;
>> >+}
> Can these be folded into arch_make_huge_pte?

I left these as separate functions but made them inline in pgtable.h
to follow Jeremy Linton's patch for the kernel linear mappings.

>
>> >+
>> >+struct page *follow_huge_pmd(struct mm_struct *mm, unsigned long address,
>> >+ pmd_t *pmd, int write)
>> >+{
>> >+ struct page *page;
>> >+
>> >+ page = pte_page(*(pte_t *)pmd);
>> >+ if (page)
>> >+ page += ((address & ~PMD_MASK) >> PAGE_SHIFT);
>> >+ return page;
>> >+}
> Do we need to think about contiguous pmd's here?
> It may be worth implementing follow_huge_addr?

It turned out to be unnecessary to override follow_huge_pmd/pud.

-Dave

David Woods

unread,
Oct 19, 2015, 4:09:43 PM10/19/15
to dwo...@ezchip.com, catalin...@arm.com, will....@arm.com, steve....@linaro.org, jeremy...@arm.com, linux-ar...@lists.infradead.org, linux-...@vger.kernel.org, linu...@kvack.org, cmet...@ezchip.com
The arm64 MMU supports a Contiguous bit which is a hint that the TTE
is one of a set of contiguous entries which can be cached in a single
TLB entry. Supporting this bit adds new intermediate huge page sizes.

The set of huge page sizes available depends on the base page size.
Without using contiguous pages the huge page sizes are as follows.

4KB: 2MB 1GB
64KB: 512MB

With a 4KB granule, the contiguous bit groups together sets of 16 pages
and with a 64KB granule it groups sets of 32 pages. This enables two new
huge page sizes in each case, so that the full set of available sizes
is as follows.

4KB: 64KB 2MB 32MB 1GB
64KB: 2MB 512MB 16GB

If a 16KB granule is used then the contiguous bit groups 128 pages
at the PTE level and 32 pages at the PMD level.

If the base page size is set to 64KB then 2MB pages are enabled by
default. It is possible in the future to make 2MB the default huge
page size for both 4KB and 64KB granules.

Signed-off-by: David Woods <dwo...@ezchip.com>
Reviewed-by: Chris Metcalf <cmet...@ezchip.com>
---
arch/arm64/Kconfig | 3 -
arch/arm64/include/asm/hugetlb.h | 30 ++---
arch/arm64/include/asm/pgtable-hwdef.h | 20 ++++
arch/arm64/include/asm/pgtable.h | 33 +++++-
arch/arm64/mm/hugetlbpage.c | 211 ++++++++++++++++++++++++++++++++-
5 files changed, 272 insertions(+), 25 deletions(-)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 07d1811..3aa151d 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -464,9 +464,6 @@ config HW_PERF_EVENTS
config SYS_SUPPORTS_HUGETLBFS
def_bool y

-config ARCH_WANT_GENERAL_HUGETLB
- def_bool y
-
config ARCH_WANT_HUGE_PMD_SHARE
def_bool y if !ARM64_64K_PAGES

diff --git a/arch/arm64/include/asm/hugetlb.h b/arch/arm64/include/asm/hugetlb.h
index bb4052e..2b153a9 100644
--- a/arch/arm64/include/asm/hugetlb.h
+++ b/arch/arm64/include/asm/hugetlb.h
@@ -26,12 +26,6 @@ static inline pte_t huge_ptep_get(pte_t *ptep)
return *ptep;
}

-static inline void set_huge_pte_at(struct mm_struct *mm, unsigned long addr,
- pte_t *ptep, pte_t pte)
-{
- set_pte_at(mm, addr, ptep, pte);
-}
-
static inline void huge_ptep_clear_flush(struct vm_area_struct *vma,
unsigned long addr, pte_t *ptep)
{
@@ -44,19 +38,6 @@ static inline void huge_ptep_set_wrprotect(struct mm_struct *mm,
ptep_set_wrprotect(mm, addr, ptep);
}

-static inline pte_t huge_ptep_get_and_clear(struct mm_struct *mm,
- unsigned long addr, pte_t *ptep)
-{
- return ptep_get_and_clear(mm, addr, ptep);
-}
-
-static inline int huge_ptep_set_access_flags(struct vm_area_struct *vma,
- unsigned long addr, pte_t *ptep,
- pte_t pte, int dirty)
-{
- return ptep_set_access_flags(vma, addr, ptep, pte, dirty);
-}
-
static inline void hugetlb_free_pgd_range(struct mmu_gather *tlb,
unsigned long addr, unsigned long end,
unsigned long floor,
@@ -97,4 +78,15 @@ static inline void arch_clear_hugepage_flags(struct page *page)
clear_bit(PG_dcache_clean, &page->flags);
}

+extern pte_t arch_make_huge_pte(pte_t entry, struct vm_area_struct *vma,
+ struct page *page, int writable);
+#define arch_make_huge_pte arch_make_huge_pte
+extern void set_huge_pte_at(struct mm_struct *mm, unsigned long addr,
+ pte_t *ptep, pte_t pte);
+extern int huge_ptep_set_access_flags(struct vm_area_struct *vma,
+ unsigned long addr, pte_t *ptep,
+ pte_t pte, int dirty);
+extern pte_t huge_ptep_get_and_clear(struct mm_struct *mm,
+ unsigned long addr, pte_t *ptep);
+
#endif /* __ASM_HUGETLB_H */
diff --git a/arch/arm64/include/asm/pgtable-hwdef.h b/arch/arm64/include/asm/pgtable-hwdef.h
index 24154b0..1b921a5 100644
--- a/arch/arm64/include/asm/pgtable-hwdef.h
+++ b/arch/arm64/include/asm/pgtable-hwdef.h
@@ -55,6 +55,24 @@
#define SECTION_MASK (~(SECTION_SIZE-1))

/*
+ * Contiguous page definitions.
+ */
+#ifdef CONFIG_ARM64_64K_PAGES
+#define CONT_PTE_SHIFT 5
+#define CONT_PMD_SHIFT 5
+#else
+#define CONT_PTE_SHIFT 4
+#define CONT_PMD_SHIFT 4
+#endif
+
+#define CONT_PTES (1 << CONT_PTE_SHIFT)
+#define CONT_PTE_SIZE (CONT_PTES * PAGE_SIZE)
+#define CONT_PTE_MASK (~(CONT_PTE_SIZE - 1))
+#define CONT_PMDS (1 << CONT_PMD_SHIFT)
+#define CONT_PMD_SIZE (CONT_PMDS * PMD_SIZE)
+#define CONT_PMD_MASK (~(CONT_PMD_SIZE - 1))
+
+/*
* Hardware page table definitions.
*
* Level 1 descriptor (PUD).
@@ -83,6 +101,7 @@
#define PMD_SECT_S (_AT(pmdval_t, 3) << 8)
#define PMD_SECT_AF (_AT(pmdval_t, 1) << 10)
#define PMD_SECT_NG (_AT(pmdval_t, 1) << 11)
+#define PMD_SECT_CONT (_AT(pmdval_t, 1) << 52)
#define PMD_SECT_PXN (_AT(pmdval_t, 1) << 53)
#define PMD_SECT_UXN (_AT(pmdval_t, 1) << 54)

@@ -105,6 +124,7 @@
#define PTE_AF (_AT(pteval_t, 1) << 10) /* Access Flag */
#define PTE_NG (_AT(pteval_t, 1) << 11) /* nG */
#define PTE_DBM (_AT(pteval_t, 1) << 51) /* Dirty Bit Management */
+#define PTE_CONT (_AT(pteval_t, 1) << 52) /* Contiguous */
#define PTE_PXN (_AT(pteval_t, 1) << 53) /* Privileged XN */
#define PTE_UXN (_AT(pteval_t, 1) << 54) /* User XN */

diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
index 26b0666..cf079a1 100644
--- a/arch/arm64/include/asm/pgtable.h
+++ b/arch/arm64/include/asm/pgtable.h
@@ -140,6 +140,7 @@ extern struct page *empty_zero_page;
#define pte_special(pte) (!!(pte_val(pte) & PTE_SPECIAL))
#define pte_write(pte) (!!(pte_val(pte) & PTE_WRITE))
#define pte_exec(pte) (!(pte_val(pte) & PTE_UXN))
+#define pte_cont(pte) (!!(pte_val(pte) & PTE_CONT))

#ifdef CONFIG_ARM64_HW_AFDBM
#define pte_hw_dirty(pte) (pte_write(pte) && !(pte_val(pte) & PTE_RDONLY))
@@ -202,6 +203,18 @@ static inline pte_t pte_mkspecial(pte_t pte)
return set_pte_bit(pte, __pgprot(PTE_SPECIAL));
}

+static inline pte_t pte_mkcont(pte_t pte)
+{
+ pte = set_pte_bit(pte, __pgprot(PTE_CONT));
+ return set_pte_bit(pte, __pgprot(PTE_TYPE_PAGE));
+ return pte;
+}
+
+static inline pmd_t pmd_mkcont(pmd_t pmd)
+{
+ return __pmd(pmd_val(pmd) | PMD_SECT_CONT);
+}
+
static inline void set_pte(pte_t *ptep, pte_t pte)
{
*ptep = pte;
@@ -271,7 +284,7 @@ static inline void set_pte_at(struct mm_struct *mm, unsigned long addr,
/*
* Hugetlb definitions.
*/
-#define HUGE_MAX_HSTATE 2
+#define HUGE_MAX_HSTATE ((2 * CONFIG_PGTABLE_LEVELS) - 1)
#define HPAGE_SHIFT PMD_SHIFT
#define HPAGE_SIZE (_AC(1, UL) << HPAGE_SHIFT)
#define HPAGE_MASK (~(HPAGE_SIZE - 1))
@@ -496,7 +509,7 @@ static inline pud_t *pud_offset(pgd_t *pgd, unsigned long addr)
static inline pte_t pte_modify(pte_t pte, pgprot_t newprot)
{
const pteval_t mask = PTE_USER | PTE_PXN | PTE_UXN | PTE_RDONLY |
- PTE_PROT_NONE | PTE_VALID | PTE_WRITE;
+ PTE_PROT_NONE | PTE_VALID | PTE_WRITE | PTE_CONT;
/* preserve the hardware dirty information */
if (pte_hw_dirty(pte))
pte = pte_mkdirty(pte);
@@ -509,6 +522,22 @@ static inline pmd_t pmd_modify(pmd_t pmd, pgprot_t newprot)
return pte_pmd(pte_modify(pmd_pte(pmd), newprot));
}

+static inline pte_t pte_modify_pfn(pte_t pte, unsigned long newpfn)
+{
+ const pteval_t mask = PHYS_MASK & PAGE_MASK;
+
+ pte_val(pte) = pfn_pte(newpfn, (pte_val(pte) & ~mask));
+ return pte;
+}
+
+static inline pmd_t pmd_modify_pfn(pmd_t pmd, unsigned long newpfn)
+{
+ const pmdval_t mask = PHYS_MASK & PAGE_MASK;
+
+ pmd = pfn_pmd(newpfn, (pmd_val(pmd) & ~mask));
+ return pmd;
+}
+
#ifdef CONFIG_ARM64_HW_AFDBM
/*
* Atomic pte/pmd modifications.
diff --git a/arch/arm64/mm/hugetlbpage.c b/arch/arm64/mm/hugetlbpage.c
index 383b03f..20fd34c 100644
--- a/arch/arm64/mm/hugetlbpage.c
+++ b/arch/arm64/mm/hugetlbpage.c
@@ -41,6 +41,201 @@ int pud_huge(pud_t pud)
#endif
}

+static int find_num_contig(struct mm_struct *mm, unsigned long addr,
+ pte_t *ptep, pte_t pte, size_t *pgsize)
+{
+ pgd_t *pgd = pgd_offset(mm, addr);
+ pud_t *pud;
+ pmd_t *pmd;
+
+ if (!pte_cont(pte))
+ return 1;
+
+ pud = pud_offset(pgd, addr);
+ pmd = pmd_offset(pud, addr);
+ if ((pte_t *)pmd == ptep) {
+ *pgsize = PMD_SIZE;
+ return CONT_PMDS;
+ }
+ *pgsize = PAGE_SIZE;
+ return CONT_PTES;
+}
+
+extern void set_huge_pte_at(struct mm_struct *mm, unsigned long addr,
+ pte_t *ptep, pte_t pte)
+{
+ size_t pgsize;
+ int ncontig = find_num_contig(mm, addr, ptep, pte, &pgsize);
+
+ if (ncontig == 1) {
+ set_pte_at(mm, addr, ptep, pte);
+ } else {
+ int i;
+ unsigned long pfn = pte_pfn(pte);
+ pgprot_t hugeprot =
+ __pgprot(pte_val(pfn_pte(pfn, 0) ^ pte_val(pte)));
+ for (i = 0; i < ncontig; i++) {
+ pr_debug("%s: set pte %p to 0x%llx\n", __func__, ptep,
+ pfn_pte(pfn, hugeprot));
+ set_pte_at(mm, addr, ptep, pfn_pte(pfn, hugeprot));
+ ptep++;
+ pfn += pgsize / PAGE_SIZE;
+ addr += pgsize;
+ }
+ }
+}
+
+pte_t *huge_pte_alloc(struct mm_struct *mm,
+ unsigned long addr, unsigned long sz)
+{
+ pgd_t *pgd;
+ pud_t *pud;
+ pte_t *pte = NULL;
+
+ pr_debug("%s: addr:0x%lx sz:0x%lx\n", __func__, addr, sz);
+ pgd = pgd_offset(mm, addr);
+ pud = pud_alloc(mm, pgd, addr);
+ if (pud) {
+ if (sz == PUD_SIZE) {
+ pte = (pte_t *)pud;
+ } else if (sz == (PAGE_SIZE * CONT_PTES)) {
+ pmd_t *pmd = pmd_alloc(mm, pud, addr);
+
+ WARN_ON(addr & (sz - 1));
+ pte = pte_alloc_map(mm, NULL, pmd, addr);
+ } else if (sz == PMD_SIZE) {
+#ifdef CONFIG_ARCH_WANT_HUGE_PMD_SHARE
+ if (pud_none(*pud))
+ pte = huge_pmd_share(mm, addr, pud);
+ else
+#endif
+ pte = (pte_t *)pmd_alloc(mm, pud, addr);
+ } else if (sz == (PMD_SIZE * CONT_PMDS)) {
+ pmd_t *pmd;
+
+ pmd = pmd_alloc(mm, pud, addr);
+ WARN_ON(addr & (sz - 1));
+ return (pte_t *)pmd;
+ }
+ }
+
+ pr_debug("%s: addr:0x%lx sz:0x%lx ret pte=%p/0x%llx\n", __func__, addr,
+ sz, pte, pte_val(*pte));
+ return pte;
+}
+
+pte_t *huge_pte_offset(struct mm_struct *mm, unsigned long addr)
+{
+ pgd_t *pgd;
+ pud_t *pud;
+ pmd_t *pmd = NULL;
+ pte_t *pte = NULL;
+
+ pgd = pgd_offset(mm, addr);
+ pr_debug("%s: addr:0x%lx pgd:%p\n", __func__, addr, pgd);
+ if (pgd_present(*pgd)) {
+ pud = pud_offset(pgd, addr);
+ if (pud_present(*pud)) {
+ if (pud_huge(*pud))
+ return (pte_t *)pud;
+ pmd = pmd_offset(pud, addr);
+ if (pmd_present(*pmd)) {
+ if (pte_cont(pmd_pte(*pmd))) {
+ pmd = pmd_offset(
+ pud, (addr & CONT_PMD_MASK));
+ return (pte_t *)pmd;
+ }
+ if (pmd_huge(*pmd))
+ return (pte_t *)pmd;
+ pte = pte_offset_kernel(pmd, addr);
+ if (pte_present(*pte) && pte_cont(*pte)) {
+ pte = pte_offset_kernel(
+ pmd, (addr & CONT_PTE_MASK));
+ }
+ return pte;
+ }
+ }
+ }
+ return (pte_t *) NULL;
+}
+
+pte_t arch_make_huge_pte(pte_t entry, struct vm_area_struct *vma,
+ struct page *page, int writable)
+{
+ size_t pagesize = huge_page_size(hstate_vma(vma));
+
+ if (pagesize == CONT_PTE_SIZE) {
+ entry = pte_mkcont(entry);
+ } else if (pagesize == CONT_PMD_SIZE) {
+ entry = pmd_pte(pmd_mkcont(pte_pmd(entry)));
+ } else if (pagesize != PUD_SIZE && pagesize != PMD_SIZE) {
+ pr_warn("%s: unrecognized huge page size 0x%lx\n",
+ __func__, pagesize);
+ }
+ return entry;
+}
+
+extern pte_t huge_ptep_get_and_clear(struct mm_struct *mm,
+ unsigned long addr, pte_t *ptep)
+{
+ pte_t pte = {0};
+
+ if (pte_cont(*ptep)) {
+ int ncontig, i;
+ size_t pgsize;
+ pte_t *cpte;
+ bool is_dirty = false;
+
+ cpte = huge_pte_offset(mm, addr);
+ ncontig = find_num_contig(mm, addr, cpte,
+ pte_val(*cpte), &pgsize);
+ /* save the 1st pte to return */
+ pte = ptep_get_and_clear(mm, addr, cpte);
+ for (i = 1; i < ncontig; ++i) {
+ if (pte_dirty(ptep_get_and_clear(mm, addr, ++cpte)))
+ is_dirty = true;
+ }
+ if (is_dirty)
+ return pte_mkdirty(pte);
+ else
+ return pte;
+ } else {
+ return ptep_get_and_clear(mm, addr, ptep);
+ }
+}
+
+int huge_ptep_set_access_flags(struct vm_area_struct *vma,
+ unsigned long addr, pte_t *ptep,
+ pte_t pte, int dirty)
+{
+ pte_t *cpte;
+
+ if (pte_cont(pte)) {
+ int ncontig, i, changed = 0;
+ size_t pgsize = 0;
+ unsigned long pfn = pte_pfn(pte);
+ /* Select all bits except the pfn */
+ pgprot_t hugeprot =
+ __pgprot(pte_val(pfn_pte(pfn, 0) ^ pte_val(pte)));
+
+ cpte = huge_pte_offset(vma->vm_mm, addr);
+ pfn = pte_pfn(*cpte);
+ ncontig = find_num_contig(vma->vm_mm, addr, cpte,
+ pte_val(*cpte), &pgsize);
+ for (i = 0; i < ncontig; ++i, ++cpte) {
+ changed = ptep_set_access_flags(vma, addr, cpte,
+ pfn_pte(pfn,
+ hugeprot),
+ dirty);
+ pfn += pgsize / PAGE_SIZE;
+ }
+ return changed;
+ } else {
+ return ptep_set_access_flags(vma, addr, ptep, pte, dirty);
+ }
+}
+
+
static __init int setup_hugepagesz(char *opt)
{
unsigned long ps = memparse(opt, &opt);
@@ -48,10 +243,24 @@ static __init int setup_hugepagesz(char *opt)
hugetlb_add_hstate(PMD_SHIFT - PAGE_SHIFT);
} else if (ps == PUD_SIZE) {
hugetlb_add_hstate(PUD_SHIFT - PAGE_SHIFT);
+ } else if (ps == (PAGE_SIZE * CONT_PTES)) {
+ hugetlb_add_hstate(CONT_PTE_SHIFT);
+ } else if (ps == (PMD_SIZE * CONT_PMDS)) {
+ hugetlb_add_hstate((PMD_SHIFT + CONT_PMD_SHIFT) - PAGE_SHIFT);
} else {
- pr_err("hugepagesz: Unsupported page size %lu M\n", ps >> 20);
+ pr_err("hugepagesz: Unsupported page size %lu K\n", ps >> 10);
return 0;
}
return 1;
}
__setup("hugepagesz=", setup_hugepagesz);
+
+#ifdef CONFIG_ARM64_64K_PAGES
+static __init int add_default_hugepagesz(void)
+{
+ if (size_to_hstate(CONT_PTES * PAGE_SIZE) == NULL)
+ hugetlb_add_hstate(CONT_PMD_SHIFT);
+ return 0;
+}
+arch_initcall(add_default_hugepagesz);
+#endif
--
2.1.2

Steve Capper

unread,
Oct 20, 2015, 8:16:43 AM10/20/15
to David Woods, catalin...@arm.com, will....@arm.com, jeremy...@arm.com, linux-ar...@lists.infradead.org, linux-...@vger.kernel.org, linu...@kvack.org, cmet...@ezchip.com
On Mon, Oct 19, 2015 at 04:09:09PM -0400, David Woods wrote:
> The arm64 MMU supports a Contiguous bit which is a hint that the TTE
> is one of a set of contiguous entries which can be cached in a single
> TLB entry. Supporting this bit adds new intermediate huge page sizes.
>
> The set of huge page sizes available depends on the base page size.
> Without using contiguous pages the huge page sizes are as follows.
>
> 4KB: 2MB 1GB
> 64KB: 512MB
>
> With a 4KB granule, the contiguous bit groups together sets of 16 pages
> and with a 64KB granule it groups sets of 32 pages. This enables two new
> huge page sizes in each case, so that the full set of available sizes
> is as follows.
>
> 4KB: 64KB 2MB 32MB 1GB
> 64KB: 2MB 512MB 16GB
>
> If a 16KB granule is used then the contiguous bit groups 128 pages
> at the PTE level and 32 pages at the PMD level.
>
> If the base page size is set to 64KB then 2MB pages are enabled by
> default. It is possible in the future to make 2MB the default huge
> page size for both 4KB and 64KB granules.

Thank you for the V2 David,
I have some comments below.

I would recommend running the next version of this series through
the libhugetlbfs test suite, as that may pick up a few things too.

Cheers,
--
Steve
The second return should be removed.

> +}
> +
> +static inline pmd_t pmd_mkcont(pmd_t pmd)
> +{
> + return __pmd(pmd_val(pmd) | PMD_SECT_CONT);
> +}
> +
> static inline void set_pte(pte_t *ptep, pte_t pte)
> {
> *ptep = pte;
> @@ -271,7 +284,7 @@ static inline void set_pte_at(struct mm_struct *mm, unsigned long addr,
> /*
> * Hugetlb definitions.
> */
> -#define HUGE_MAX_HSTATE 2
> +#define HUGE_MAX_HSTATE ((2 * CONFIG_PGTABLE_LEVELS) - 1)

Not sure about this definition. I would just go with the maximum possible
which is for a 4KB granule:
1 x 1GB pud
1 x 2MB pmd
16 x 2MB pmds
16 x 4KB ptes

So 4 for now?


> #define HPAGE_SHIFT PMD_SHIFT
> #define HPAGE_SIZE (_AC(1, UL) << HPAGE_SHIFT)
> #define HPAGE_MASK (~(HPAGE_SIZE - 1))
> @@ -496,7 +509,7 @@ static inline pud_t *pud_offset(pgd_t *pgd, unsigned long addr)
> static inline pte_t pte_modify(pte_t pte, pgprot_t newprot)
> {
> const pteval_t mask = PTE_USER | PTE_PXN | PTE_UXN | PTE_RDONLY |
> - PTE_PROT_NONE | PTE_VALID | PTE_WRITE;
> + PTE_PROT_NONE | PTE_VALID | PTE_WRITE | PTE_CONT;

Why has PTE_CONT been added to the pte_modify mask? This will allow
functions such as mprotect to remove the PTE_CONT bit.

> /* preserve the hardware dirty information */
> if (pte_hw_dirty(pte))
> pte = pte_mkdirty(pte);
> @@ -509,6 +522,22 @@ static inline pmd_t pmd_modify(pmd_t pmd, pgprot_t newprot)
> return pte_pmd(pte_modify(pmd_pte(pmd), newprot));
> }
>
> +static inline pte_t pte_modify_pfn(pte_t pte, unsigned long newpfn)
> +{
> + const pteval_t mask = PHYS_MASK & PAGE_MASK;
> +
> + pte_val(pte) = pfn_pte(newpfn, (pte_val(pte) & ~mask));
> + return pte;
> +}
> +
> +static inline pmd_t pmd_modify_pfn(pmd_t pmd, unsigned long newpfn)
> +{
> + const pmdval_t mask = PHYS_MASK & PAGE_MASK;
> +
> + pmd = pfn_pmd(newpfn, (pmd_val(pmd) & ~mask));
> + return pmd;
> +}

pte_modify_pfn and pmd_modify_pfn aren't referenced anywhere in the
patch so should be removed.

> +
> #ifdef CONFIG_ARM64_HW_AFDBM
> /*
> * Atomic pte/pmd modifications.
> diff --git a/arch/arm64/mm/hugetlbpage.c b/arch/arm64/mm/hugetlbpage.c
> index 383b03f..20fd34c 100644
> --- a/arch/arm64/mm/hugetlbpage.c
> +++ b/arch/arm64/mm/hugetlbpage.c
> @@ -41,6 +41,201 @@ int pud_huge(pud_t pud)
> #endif
> }
>
> +static int find_num_contig(struct mm_struct *mm, unsigned long addr,
> + pte_t *ptep, pte_t pte, size_t *pgsize)
> +{
> + pgd_t *pgd = pgd_offset(mm, addr);
> + pud_t *pud;
> + pmd_t *pmd;
> +
> + if (!pte_cont(pte))
> + return 1;
> +
> + pud = pud_offset(pgd, addr);
> + pmd = pmd_offset(pud, addr);

We need to check for pgd_present and pud_present as we walk.
I would be tempted to VM_BUG_ON if they are in an unexpected state.

> + if ((pte_t *)pmd == ptep) {
> + *pgsize = PMD_SIZE;
> + return CONT_PMDS;
> + }

I would check for pmd_present and VM_BUG_ON if it wasn't in an expected
state.

> + *pgsize = PAGE_SIZE;
> + return CONT_PTES;
> +}

Another approach would be something like:

struct vm_area_struct *vma = find_vma(mm, addr);
struct hstate *h = hstate_vma(vma);
size_t size = hpage_size(h);

But I think looking at the page table entries like you've done (with
some checking) may be a little better as it can supply some more robust
debugging with DEBUG_VM selected (and it doesn't need to find_vma).


> +
> +extern void set_huge_pte_at(struct mm_struct *mm, unsigned long addr,
> + pte_t *ptep, pte_t pte)

We don't need this extern.

> +{
> + size_t pgsize;
> + int ncontig = find_num_contig(mm, addr, ptep, pte, &pgsize);
> +
> + if (ncontig == 1) {
> + set_pte_at(mm, addr, ptep, pte);

We can return early here and avoid a level of indentation below.

> + } else {
> + int i;
> + unsigned long pfn = pte_pfn(pte);
> + pgprot_t hugeprot =
> + __pgprot(pte_val(pfn_pte(pfn, 0) ^ pte_val(pte)));
> + for (i = 0; i < ncontig; i++) {
> + pr_debug("%s: set pte %p to 0x%llx\n", __func__, ptep,
> + pfn_pte(pfn, hugeprot));
> + set_pte_at(mm, addr, ptep, pfn_pte(pfn, hugeprot));
> + ptep++;
> + pfn += pgsize / PAGE_SIZE;

nit: pgsize >> PAGE_SHIFT

> + addr += pgsize;
> + }
> + }
> +}

I see... so the contiguous pte and pmd cases are folded together.
The pgsize variable name could be changed, perhaps something like blocksize?
(I am terrible at picking names though :-)).

> +
> +pte_t *huge_pte_alloc(struct mm_struct *mm,
> + unsigned long addr, unsigned long sz)
> +{
> + pgd_t *pgd;
> + pud_t *pud;
> + pte_t *pte = NULL;
> +
> + pr_debug("%s: addr:0x%lx sz:0x%lx\n", __func__, addr, sz);
> + pgd = pgd_offset(mm, addr);
> + pud = pud_alloc(mm, pgd, addr);

Probably better to simplify the levels of indentation with:
if (!pud)
return NULL;
(or goto out before your pr_debug)

> + if (pud) {

Perhaps better to do something with switch(sz) below?

> + if (sz == PUD_SIZE) {
> + pte = (pte_t *)pud;
> + } else if (sz == (PAGE_SIZE * CONT_PTES)) {
> + pmd_t *pmd = pmd_alloc(mm, pud, addr);
> +
> + WARN_ON(addr & (sz - 1));
> + pte = pte_alloc_map(mm, NULL, pmd, addr);
> + } else if (sz == PMD_SIZE) {
> +#ifdef CONFIG_ARCH_WANT_HUGE_PMD_SHARE
> + if (pud_none(*pud))
> + pte = huge_pmd_share(mm, addr, pud);
> + else
> +#endif

This can be simplified to something like:

if (IS_ENABLED(CONFIG_ARCH_WANT_HUGE_PMD_SHARE)
&& pud_none(*pud))
else

So we can remove the preprocessor macros.
Again drop a level of indentation with:
if (!pgd_present(*pgd))
return NULL;

Similarly for pud_present and pmd_present.
I would go for switch(pagesize) here.

> + if (pagesize == CONT_PTE_SIZE) {
> + entry = pte_mkcont(entry);
> + } else if (pagesize == CONT_PMD_SIZE) {
> + entry = pmd_pte(pmd_mkcont(pte_pmd(entry)));
> + } else if (pagesize != PUD_SIZE && pagesize != PMD_SIZE) {
> + pr_warn("%s: unrecognized huge page size 0x%lx\n",
> + __func__, pagesize);
> + }
> + return entry;
> +}
> +
> +extern pte_t huge_ptep_get_and_clear(struct mm_struct *mm,
> + unsigned long addr, pte_t *ptep)
> +{
> + pte_t pte = {0};

nit: Do we need an initial value for pte?

> +
> + if (pte_cont(*ptep)) {
> + int ncontig, i;
> + size_t pgsize;
> + pte_t *cpte;
> + bool is_dirty = false;
> +
> + cpte = huge_pte_offset(mm, addr);
> + ncontig = find_num_contig(mm, addr, cpte,
> + pte_val(*cpte), &pgsize);
> + /* save the 1st pte to return */
> + pte = ptep_get_and_clear(mm, addr, cpte);
> + for (i = 1; i < ncontig; ++i) {
> + if (pte_dirty(ptep_get_and_clear(mm, addr, ++cpte)))
> + is_dirty = true;
> + }

Nice, we are keeping track of the dirty state. This looks to me like
it *should* work well with the dirty bit management patch that Catalin
introduced:
2f4b829 arm64: Add support for hardware updates of the access and dirty pte bits

Because ptep_get_and_clear will atomically get and clear the pte with
respect to the hardware dirty bit management thus we don't lose any
dirty information. huge_pte_dirty is then called on the extracted pte
by core code.

For a contiguous set of ptes/pmds the individual entry will be dirtied
by DBM rather than the complete set so it's good to check them all for
dirty when going through a get and clear.

Technically we don't need to track dirty if CONFIG_ARM64_HW_AFDBM is
not defined as the core code will fault and modify the entire set of
ptes otherwise.

I would be tempted to keep this code as is, but add a comment that
tracking the dirty variable here helps for when we switch on
CONFIG_ARM64_HW_AFDBM.
nit: pgsize >> PAGE_SHIFT

> + }

Similar thing here, we are folding the cont pte and pmd logic together,
probably best go for something like blocksize instead of pgsize.

> + return changed;
> + } else {
> + return ptep_set_access_flags(vma, addr, ptep, pte, dirty);
> + }
> +}
> +
> +
> static __init int setup_hugepagesz(char *opt)
> {
> unsigned long ps = memparse(opt, &opt);
> @@ -48,10 +243,24 @@ static __init int setup_hugepagesz(char *opt)
> hugetlb_add_hstate(PMD_SHIFT - PAGE_SHIFT);

Again switch statement here I think would be clearer.

> } else if (ps == PUD_SIZE) {
> hugetlb_add_hstate(PUD_SHIFT - PAGE_SHIFT);
> + } else if (ps == (PAGE_SIZE * CONT_PTES)) {
> + hugetlb_add_hstate(CONT_PTE_SHIFT);
> + } else if (ps == (PMD_SIZE * CONT_PMDS)) {
> + hugetlb_add_hstate((PMD_SHIFT + CONT_PMD_SHIFT) - PAGE_SHIFT);
> } else {
> - pr_err("hugepagesz: Unsupported page size %lu M\n", ps >> 20);
> + pr_err("hugepagesz: Unsupported page size %lu K\n", ps >> 10);
> return 0;
> }
> return 1;
> }
> __setup("hugepagesz=", setup_hugepagesz);
> +
> +#ifdef CONFIG_ARM64_64K_PAGES
> +static __init int add_default_hugepagesz(void)
> +{
> + if (size_to_hstate(CONT_PTES * PAGE_SIZE) == NULL)
> + hugetlb_add_hstate(CONT_PMD_SHIFT);
> + return 0;
> +}
> +arch_initcall(add_default_hugepagesz);
> +#endif
Why is this initcall defined? Was it for testing?

I think we are missing a few functions:
huge_ptep_set_access_flags
huge_ptep_set_wrprotect
huge_ptep_clear_flush

These functions need to loop through the contiguous set of ptes
or pmds. They should call into the ptep_ equivalents as they will
then work with the DBM patch.

David Woods

unread,
Nov 18, 2015, 3:34:33 PM11/18/15
to Steve Capper, catalin...@arm.com, will....@arm.com, jeremy...@arm.com, linux-ar...@lists.infradead.org, linux-...@vger.kernel.org, linu...@kvack.org, cmet...@ezchip.com
On 10/20/2015 08:16 AM, Steve Capper wrote:
> On Mon, Oct 19, 2015 at 04:09:09PM -0400, David Woods wrote:
>> >The arm64 MMU supports a Contiguous bit which is a hint that the TTE
>> >is one of a set of contiguous entries which can be cached in a single
>> >TLB entry. Supporting this bit adds new intermediate huge page sizes.
>> >
>> >The set of huge page sizes available depends on the base page size.
>> >Without using contiguous pages the huge page sizes are as follows.
>> >
>> > 4KB: 2MB 1GB
>> >64KB: 512MB
>> >
>> >With a 4KB granule, the contiguous bit groups together sets of 16 pages
>> >and with a 64KB granule it groups sets of 32 pages. This enables two new
>> >huge page sizes in each case, so that the full set of available sizes
>> >is as follows.
>> >
>> > 4KB: 64KB 2MB 32MB 1GB
>> >64KB: 2MB 512MB 16GB
>> >
>> >If a 16KB granule is used then the contiguous bit groups 128 pages
>> >at the PTE level and 32 pages at the PMD level.
>> >
>> >If the base page size is set to 64KB then 2MB pages are enabled by
>> >default. It is possible in the future to make 2MB the default huge
>> >page size for both 4KB and 64KB granules.
> Thank you for the V2 David,
> I have some comments below.
>
> I would recommend running the next version of this series through
> the libhugetlbfs test suite, as that may pick up a few things too.

Thanks Steve, for your detailed review. I did run the libhugetlbfs test
suite
and it turned up a bug which I'll point out below. I'll post a V3 shortly.

>
> Cheers,
> -- Steve
> >
> >+static inline pte_t pte_mkcont(pte_t pte)
> >+{
> >+ pte = set_pte_bit(pte, __pgprot(PTE_CONT));
> >+ return set_pte_bit(pte, __pgprot(PTE_TYPE_PAGE));
> >+ return pte;
> The second return should be removed.

Done.
>
> > /*
> > * Hugetlb definitions.
> > */
> >-#define HUGE_MAX_HSTATE 2
> >+#define HUGE_MAX_HSTATE ((2 * CONFIG_PGTABLE_LEVELS) - 1)
> Not sure about this definition. I would just go with the maximum possible
> which is for a 4KB granule:
> 1 x 1GB pud
> 1 x 2MB pmd
> 16 x 2MB pmds
> 16 x 4KB ptes
>
> So 4 for now?

This made some sense when I was thinking of supporting contiguous
PUDs. I've changed it to 4 as you suggest.
>> > #define HPAGE_SHIFT PMD_SHIFT
>> > #define HPAGE_SIZE (_AC(1, UL) << HPAGE_SHIFT)
>> > #define HPAGE_MASK (~(HPAGE_SIZE - 1))
>> >@@ -496,7 +509,7 @@ static inline pud_t *pud_offset(pgd_t *pgd, unsigned long addr)
>> > static inline pte_t pte_modify(pte_t pte, pgprot_t newprot)
>> > {
>> > const pteval_t mask = PTE_USER | PTE_PXN | PTE_UXN | PTE_RDONLY |
>> >- PTE_PROT_NONE | PTE_VALID | PTE_WRITE;
>> >+ PTE_PROT_NONE | PTE_VALID | PTE_WRITE | PTE_CONT;
> Why has PTE_CONT been added to the pte_modify mask? This will allow
> functions such as mprotect to remove the PTE_CONT bit.
Right, this is not needed anymore.
>
>
>
> >
> >+static inline pte_t pte_modify_pfn(pte_t pte, unsigned long newpfn)
> >+{
> >+ const pteval_t mask = PHYS_MASK & PAGE_MASK;
> >+
> >+ pte_val(pte) = pfn_pte(newpfn, (pte_val(pte) & ~mask));
> >+ return pte;
> >+}
> >+
> >+static inline pmd_t pmd_modify_pfn(pmd_t pmd, unsigned long newpfn)
> >+{
> >+ const pmdval_t mask = PHYS_MASK & PAGE_MASK;
> >+
> >+ pmd = pfn_pmd(newpfn, (pmd_val(pmd) & ~mask));
> >+ return pmd;
> >+}
> pte_modify_pfn and pmd_modify_pfn aren't referenced anywhere in the
> patch so should be removed.
Removed.
>
>> >
>> >+static int find_num_contig(struct mm_struct *mm, unsigned long addr,
>> >+ pte_t *ptep, pte_t pte, size_t *pgsize)
>> >+{
>> >+ pgd_t *pgd = pgd_offset(mm, addr);
>> >+ pud_t *pud;
>> >+ pmd_t *pmd;
>> >+
>> >+ if (!pte_cont(pte))
>> >+ return 1;
>> >+
>> >+ pud = pud_offset(pgd, addr);
>> >+ pmd = pmd_offset(pud, addr);
> We need to check for pgd_present and pud_present as we walk.
> I would be tempted to VM_BUG_ON if they are in an unexpected state.
Ok.
>
>> >+ if ((pte_t *)pmd == ptep) {
>> >+ *pgsize = PMD_SIZE;
>> >+ return CONT_PMDS;
>> >+ }
> I would check for pmd_present and VM_BUG_ON if it wasn't in an expected
> state.
>
>> >+ *pgsize = PAGE_SIZE;
>> >+ return CONT_PTES;
>> >+}
> Another approach would be something like:
>
> struct vm_area_struct *vma = find_vma(mm, addr);
> struct hstate *h = hstate_vma(vma);
> size_t size = hpage_size(h);
>
> But I think looking at the page table entries like you've done (with
> some checking) may be a little better as it can supply some more robust
> debugging with DEBUG_VM selected (and it doesn't need to find_vma).

I left it as-is with the appropriate checks added.
>> >+
>> >+extern void set_huge_pte_at(struct mm_struct *mm, unsigned long addr,
>> >+ pte_t *ptep, pte_t pte)
> We don't need this extern.

Ok.
>> >+{
>> >+ size_t pgsize;
>> >+ int ncontig = find_num_contig(mm, addr, ptep, pte, &pgsize);
>> >+
>> >+ if (ncontig == 1) {
>> >+ set_pte_at(mm, addr, ptep, pte);
> We can return early here and avoid a level of indentation below.
Ok.

>> >+ } else {
>> >+ int i;
>> >+ unsigned long pfn = pte_pfn(pte);
>> >+ pgprot_t hugeprot =
>> >+ __pgprot(pte_val(pfn_pte(pfn, 0) ^ pte_val(pte)));
>> >+ for (i = 0; i < ncontig; i++) {
>> >+ pr_debug("%s: set pte %p to 0x%llx\n", __func__, ptep,
>> >+ pfn_pte(pfn, hugeprot));
>> >+ set_pte_at(mm, addr, ptep, pfn_pte(pfn, hugeprot));
>> >+ ptep++;
>> >+ pfn += pgsize / PAGE_SIZE;
> nit: pgsize >> PAGE_SHIFT
>
>> >+ addr += pgsize;
>> >+ }
>> >+ }
>> >+}
> I see... so the contiguous pte and pmd cases are folded together.
> The pgsize variable name could be changed, perhaps something like blocksize?
> (I am terrible at picking names though :-)).

Well, isn't it still called a page even it it happens to be a
pmd level/huge page?

>
>> >+
>> >+pte_t *huge_pte_alloc(struct mm_struct *mm,
>> >+ unsigned long addr, unsigned long sz)
>> >+{
>> >+ pgd_t *pgd;
>> >+ pud_t *pud;
>> >+ pte_t *pte = NULL;
>> >+
>> >+ pr_debug("%s: addr:0x%lx sz:0x%lx\n", __func__, addr, sz);
>> >+ pgd = pgd_offset(mm, addr);
>> >+ pud = pud_alloc(mm, pgd, addr);
> Probably better to simplify the levels of indentation with:
> if (!pud)
> return NULL;
> (or goto out before your pr_debug)

Ok.
>
>> >+ if (pud) {
> Perhaps better to do something with switch(sz) below?

The problem with using switch is that depending on the number of
page table levels, some of the cases degenerate to the same value.
So we end up with compile time errors because of duplicate case
statements.
>
>> >+ if (sz == PUD_SIZE) {
>> >+ pte = (pte_t *)pud;
>> >+ } else if (sz == (PAGE_SIZE * CONT_PTES)) {
>> >+ pmd_t *pmd = pmd_alloc(mm, pud, addr);
>> >+
>> >+ WARN_ON(addr & (sz - 1));
>> >+ pte = pte_alloc_map(mm, NULL, pmd, addr);
>> >+ } else if (sz == PMD_SIZE) {
>> >+#ifdef CONFIG_ARCH_WANT_HUGE_PMD_SHARE
>> >+ if (pud_none(*pud))
>> >+ pte = huge_pmd_share(mm, addr, pud);
>> >+ else
>> >+#endif
> This can be simplified to something like:
>
> if (IS_ENABLED(CONFIG_ARCH_WANT_HUGE_PMD_SHARE)
> && pud_none(*pud))
> else
>
> So we can remove the preprocessor macros.
Ok.
> >+
> >+pte_t *huge_pte_offset(struct mm_struct *mm, unsigned long addr)
> >+{
> >+ pgd_t *pgd;
> >+ pud_t *pud;
> >+ pmd_t *pmd = NULL;
> >+ pte_t *pte = NULL;
> >+
> >+ pgd = pgd_offset(mm, addr);
> >+ pr_debug("%s: addr:0x%lx pgd:%p\n", __func__, addr, pgd);
> >+ if (pgd_present(*pgd)) {
> Again drop a level of indentation with:
> if (!pgd_present(*pgd))
> return NULL;
>
> Similarly for pud_present and pmd_present.
Ok.
>
>
> >+}
> >+
> >+pte_t arch_make_huge_pte(pte_t entry, struct vm_area_struct *vma,
> >+ struct page *page, int writable)
> >+{
> >+ size_t pagesize = huge_page_size(hstate_vma(vma));
> >+
> I would go for switch(pagesize) here.
Same as above.
>
>> >+ if (pagesize == CONT_PTE_SIZE) {
>> >+ entry = pte_mkcont(entry);
>> >+ } else if (pagesize == CONT_PMD_SIZE) {
>> >+ entry = pmd_pte(pmd_mkcont(pte_pmd(entry)));
>> >+ } else if (pagesize != PUD_SIZE && pagesize != PMD_SIZE) {
>> >+ pr_warn("%s: unrecognized huge page size 0x%lx\n",
>> >+ __func__, pagesize);
>> >+ }
>> >+ return entry;
>> >+}
>> >+
>> >+extern pte_t huge_ptep_get_and_clear(struct mm_struct *mm,
>> >+ unsigned long addr, pte_t *ptep)
>> >+{
>> >+ pte_t pte = {0};
> nit: Do we need an initial value for pte?

No, it's not necessary.
>
>> >+
>> >+ if (pte_cont(*ptep)) {
>> >+ int ncontig, i;
>> >+ size_t pgsize;
>> >+ pte_t *cpte;
>> >+ bool is_dirty = false;
>> >+
>> >+ cpte = huge_pte_offset(mm, addr);
>> >+ ncontig = find_num_contig(mm, addr, cpte,
>> >+ pte_val(*cpte), &pgsize);
>> >+ /* save the 1st pte to return */
>> >+ pte = ptep_get_and_clear(mm, addr, cpte);
>> >+ for (i = 1; i < ncontig; ++i) {
>> >+ if (pte_dirty(ptep_get_and_clear(mm, addr, ++cpte)))
>> >+ is_dirty = true;
>> >+ }
This is the bug I mentioned above which was caught by the test suite.
If CONFIG_ARM64_HW_AFDBM is defined then pte_dirty() becomes a
macro which evaluates its argument twice. I've got a side-effect in there
(++cpte) so it ends up clearing ptes that it shouldn't.

> Nice, we are keeping track of the dirty state. This looks to me like
> it*should* work well with the dirty bit management patch that Catalin
> introduced:
> 2f4b829 arm64: Add support for hardware updates of the access and dirty pte bits
>
> Because ptep_get_and_clear will atomically get and clear the pte with
> respect to the hardware dirty bit management thus we don't lose any
> dirty information. huge_pte_dirty is then called on the extracted pte
> by core code.
>
> For a contiguous set of ptes/pmds the individual entry will be dirtied
> by DBM rather than the complete set so it's good to check them all for
> dirty when going through a get and clear.
>
> Technically we don't need to track dirty if CONFIG_ARM64_HW_AFDBM is
> not defined as the core code will fault and modify the entire set of
> ptes otherwise.
>
> I would be tempted to keep this code as is, but add a comment that
> tracking the dirty variable here helps for when we switch on
> CONFIG_ARM64_HW_AFDBM.
I added a comment to try to make all this more clear.
>> >+
>> >+#ifdef CONFIG_ARM64_64K_PAGES
>> >+static __init int add_default_hugepagesz(void)
>> >+{
>> >+ if (size_to_hstate(CONT_PTES * PAGE_SIZE) == NULL)
>> >+ hugetlb_add_hstate(CONT_PMD_SHIFT);
>> >+ return 0;
>> >+}
>> >+arch_initcall(add_default_hugepagesz);
>> >+#endif
> Why is this initcall defined? Was it for testing?
This is intentional and in a way, the motivation for these changes. We're
expecting most of our customers to run with a 64K granule, but 512M is
too big as a huge page size in many cases. 2M is a lot more useful for
these applications and it's convenient because it is also the default huge
page size with a 4K granule. We think it's useful enough to enable by
default, but are interested to know your thoughts on that.

>
> I think we are missing a few functions:
> huge_ptep_set_access_flags
> huge_ptep_set_wrprotect
> huge_ptep_clear_flush
>
> These functions need to loop through the contiguous set of ptes
> or pmds. They should call into the ptep_ equivalents as they will
> then work with the DBM patch.
huge_ptep_set_access_flags() was there already, but I've added
the other two.

David Woods

unread,
Nov 19, 2015, 11:57:42 AM11/19/15
to dwo...@ezchip.com, catalin...@arm.com, will....@arm.com, steve....@linaro.org, jeremy...@arm.com, linux-ar...@lists.infradead.org, linux-...@vger.kernel.org, linu...@kvack.org, cmet...@ezchip.com
The arm64 MMU supports a Contiguous bit which is a hint that the TTE
is one of a set of contiguous entries which can be cached in a single
TLB entry. Supporting this bit adds new intermediate huge page sizes.

The set of huge page sizes available depends on the base page size.
Without using contiguous pages the huge page sizes are as follows.

4KB: 2MB 1GB
64KB: 512MB

With a 4KB granule, the contiguous bit groups together sets of 16 pages
and with a 64KB granule it groups sets of 32 pages. This enables two new
huge page sizes in each case, so that the full set of available sizes
is as follows.

4KB: 64KB 2MB 32MB 1GB
64KB: 2MB 512MB 16GB

If a 16KB granule is used then the contiguous bit groups 128 pages
at the PTE level and 32 pages at the PMD level.

If the base page size is set to 64KB then 2MB pages are enabled by
default. It is possible in the future to make 2MB the default huge
page size for both 4KB and 64KB granules.

Signed-off-by: David Woods <dwo...@ezchip.com>
Reviewed-by: Chris Metcalf <cmet...@ezchip.com>
---

This patch should resolve the comments on v2 and is now based on on the
arm64 next tree which includes 16K granule support. I've added definitions
which should enable 2M and 1G huge page sizes with a 16K granule.
Unfortunately, the A53 model we have does not support 16K so I don't
have a way to test this.

arch/arm64/Kconfig | 3 -
arch/arm64/include/asm/hugetlb.h | 44 ++----
arch/arm64/include/asm/pgtable-hwdef.h | 18 ++-
arch/arm64/include/asm/pgtable.h | 10 +-
arch/arm64/mm/hugetlbpage.c | 267 ++++++++++++++++++++++++++++++++-
include/linux/hugetlb.h | 2 -
6 files changed, 306 insertions(+), 38 deletions(-)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 40e1151..077bb7c 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -480,9 +480,6 @@ config HW_PERF_EVENTS
config SYS_SUPPORTS_HUGETLBFS
def_bool y

-config ARCH_WANT_GENERAL_HUGETLB
- def_bool y
-
config ARCH_WANT_HUGE_PMD_SHARE
def_bool y if ARM64_4K_PAGES || (ARM64_16K_PAGES && !ARM64_VA_BITS_36)

diff --git a/arch/arm64/include/asm/hugetlb.h b/arch/arm64/include/asm/hugetlb.h
index bb4052e..bbc1e35 100644
--- a/arch/arm64/include/asm/hugetlb.h
+++ b/arch/arm64/include/asm/hugetlb.h
@@ -26,36 +26,7 @@ static inline pte_t huge_ptep_get(pte_t *ptep)
return *ptep;
}

-static inline void set_huge_pte_at(struct mm_struct *mm, unsigned long addr,
- pte_t *ptep, pte_t pte)
-{
- set_pte_at(mm, addr, ptep, pte);
-}
-
-static inline void huge_ptep_clear_flush(struct vm_area_struct *vma,
- unsigned long addr, pte_t *ptep)
-{
- ptep_clear_flush(vma, addr, ptep);
-}
-
-static inline void huge_ptep_set_wrprotect(struct mm_struct *mm,
- unsigned long addr, pte_t *ptep)
-{
- ptep_set_wrprotect(mm, addr, ptep);
-}

-static inline pte_t huge_ptep_get_and_clear(struct mm_struct *mm,
- unsigned long addr, pte_t *ptep)
-{
- return ptep_get_and_clear(mm, addr, ptep);
-}
-
-static inline int huge_ptep_set_access_flags(struct vm_area_struct *vma,
- unsigned long addr, pte_t *ptep,
- pte_t pte, int dirty)
-{
- return ptep_set_access_flags(vma, addr, ptep, pte, dirty);
-}

static inline void hugetlb_free_pgd_range(struct mmu_gather *tlb,
unsigned long addr, unsigned long end,
@@ -97,4 +68,19 @@ static inline void arch_clear_hugepage_flags(struct page *page)
clear_bit(PG_dcache_clean, &page->flags);
}

+extern pte_t arch_make_huge_pte(pte_t entry, struct vm_area_struct *vma,
+ struct page *page, int writable);
+#define arch_make_huge_pte arch_make_huge_pte
+extern void set_huge_pte_at(struct mm_struct *mm, unsigned long addr,
+ pte_t *ptep, pte_t pte);
+extern int huge_ptep_set_access_flags(struct vm_area_struct *vma,
+ unsigned long addr, pte_t *ptep,
+ pte_t pte, int dirty);
+extern pte_t huge_ptep_get_and_clear(struct mm_struct *mm,
+ unsigned long addr, pte_t *ptep);
+extern void huge_ptep_set_wrprotect(struct mm_struct *mm,
+ unsigned long addr, pte_t *ptep);
+extern void huge_ptep_clear_flush(struct vm_area_struct *vma,
+ unsigned long addr, pte_t *ptep);
+
#endif /* __ASM_HUGETLB_H */
diff --git a/arch/arm64/include/asm/pgtable-hwdef.h b/arch/arm64/include/asm/pgtable-hwdef.h
index d6739e8..5c25b83 100644
--- a/arch/arm64/include/asm/pgtable-hwdef.h
+++ b/arch/arm64/include/asm/pgtable-hwdef.h
@@ -90,7 +90,23 @@
/*
* Contiguous page definitions.
*/
-#define CONT_PTES (_AC(1, UL) << CONT_SHIFT)
+#ifdef CONFIG_ARM64_64K_PAGES
+#define CONT_PTE_SHIFT 5
+#define CONT_PMD_SHIFT 5
+#elif defined(CONFIG_ARM64_16K_PAGES)
+#define CONT_PTE_SHIFT 7
+#define CONT_PMD_SHIFT 5
+#else
+#define CONT_PTE_SHIFT 4
+#define CONT_PMD_SHIFT 4
+#endif
+
+#define CONT_PTES (1 << CONT_PTE_SHIFT)
+#define CONT_PTE_SIZE (CONT_PTES * PAGE_SIZE)
+#define CONT_PTE_MASK (~(CONT_PTE_SIZE - 1))
+#define CONT_PMDS (1 << CONT_PMD_SHIFT)
+#define CONT_PMD_SIZE (CONT_PMDS * PMD_SIZE)
+#define CONT_PMD_MASK (~(CONT_PMD_SIZE - 1))
/* the the numerical offset of the PTE within a range of CONT_PTES */
#define CONT_RANGE_OFFSET(addr) (((addr)>>PAGE_SHIFT)&(CONT_PTES-1))

diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
index 1c99d56..d259332 100644
--- a/arch/arm64/include/asm/pgtable.h
+++ b/arch/arm64/include/asm/pgtable.h
@@ -214,7 +214,8 @@ static inline pte_t pte_mkspecial(pte_t pte)

static inline pte_t pte_mkcont(pte_t pte)
{
- return set_pte_bit(pte, __pgprot(PTE_CONT));
+ pte = set_pte_bit(pte, __pgprot(PTE_CONT));
+ return set_pte_bit(pte, __pgprot(PTE_TYPE_PAGE));
}

static inline pte_t pte_mknoncont(pte_t pte)
@@ -222,6 +223,11 @@ static inline pte_t pte_mknoncont(pte_t pte)
return clear_pte_bit(pte, __pgprot(PTE_CONT));
}

+static inline pmd_t pmd_mkcont(pmd_t pmd)
+{
+ return __pmd(pmd_val(pmd) | PMD_SECT_CONT);
+}
+
static inline void set_pte(pte_t *ptep, pte_t pte)
{
*ptep = pte;
@@ -291,7 +297,7 @@ static inline void set_pte_at(struct mm_struct *mm, unsigned long addr,
/*
* Hugetlb definitions.
*/
-#define HUGE_MAX_HSTATE 2
+#define HUGE_MAX_HSTATE 4
#define HPAGE_SHIFT PMD_SHIFT
#define HPAGE_SIZE (_AC(1, UL) << HPAGE_SHIFT)
#define HPAGE_MASK (~(HPAGE_SIZE - 1))
diff --git a/arch/arm64/mm/hugetlbpage.c b/arch/arm64/mm/hugetlbpage.c
index 383b03f..1688445 100644
--- a/arch/arm64/mm/hugetlbpage.c
+++ b/arch/arm64/mm/hugetlbpage.c
@@ -41,17 +41,282 @@ int pud_huge(pud_t pud)
#endif
}

+static int find_num_contig(struct mm_struct *mm, unsigned long addr,
+ pte_t *ptep, pte_t pte, size_t *pgsize)
+{
+ pgd_t *pgd = pgd_offset(mm, addr);
+ pud_t *pud;
+ pmd_t *pmd;
+
+ if (!pte_cont(pte))
+ return 1;
+ if (!pgd_present(*pgd)) {
+ VM_BUG_ON(!pgd_present(*pgd));
+ return 1;
+ }
+ pud = pud_offset(pgd, addr);
+ if (!pud_present(*pud)) {
+ VM_BUG_ON(!pud_present(*pud));
+ return 1;
+ }
+ pmd = pmd_offset(pud, addr);
+ if (!pmd_present(*pmd)) {
+ VM_BUG_ON(!pmd_present(*pmd));
+ return 1;
+ }
+ if ((pte_t *)pmd == ptep) {
+ *pgsize = PMD_SIZE;
+ return CONT_PMDS;
+ }
+ *pgsize = PAGE_SIZE;
+ return CONT_PTES;
+}
+
+void set_huge_pte_at(struct mm_struct *mm, unsigned long addr,
+ pte_t *ptep, pte_t pte)
+{
+ size_t pgsize;
+ int i;
+ int ncontig = find_num_contig(mm, addr, ptep, pte, &pgsize);
+ unsigned long pfn;
+ pgprot_t hugeprot;
+
+ if (ncontig == 1) {
+ set_pte_at(mm, addr, ptep, pte);
+ return;
+ }
+
+ pfn = pte_pfn(pte);
+ hugeprot = __pgprot(pte_val(pfn_pte(pfn, 0) ^ pte_val(pte)));
+ for (i = 0; i < ncontig; i++) {
+ pr_debug("%s: set pte %p to 0x%llx\n", __func__, ptep,
+ pfn_pte(pfn, hugeprot));
+ set_pte_at(mm, addr, ptep, pfn_pte(pfn, hugeprot));
+ ptep++;
+ pfn += pgsize >> PAGE_SHIFT;
+ addr += pgsize;
+ }
+}
+
+pte_t *huge_pte_alloc(struct mm_struct *mm,
+ unsigned long addr, unsigned long sz)
+{
+ pgd_t *pgd;
+ pud_t *pud;
+ pte_t *pte = NULL;
+
+ pr_debug("%s: addr:0x%lx sz:0x%lx\n", __func__, addr, sz);
+ pgd = pgd_offset(mm, addr);
+ pud = pud_alloc(mm, pgd, addr);
+ if (!pud)
+ return NULL;
+
+ if (sz == PUD_SIZE) {
+ pte = (pte_t *)pud;
+ } else if (sz == (PAGE_SIZE * CONT_PTES)) {
+ pmd_t *pmd = pmd_alloc(mm, pud, addr);
+
+ WARN_ON(addr & (sz - 1));
+ pte = pte_alloc_map(mm, NULL, pmd, addr);
+ } else if (sz == PMD_SIZE) {
+ if (IS_ENABLED(CONFIG_ARCH_WANT_HUGE_PMD_SHARE) &&
+ pud_none(*pud))
+ pte = huge_pmd_share(mm, addr, pud);
+ else
+ pte = (pte_t *)pmd_alloc(mm, pud, addr);
+ } else if (sz == (PMD_SIZE * CONT_PMDS)) {
+ pmd_t *pmd;
+
+ pmd = pmd_alloc(mm, pud, addr);
+ WARN_ON(addr & (sz - 1));
+ return (pte_t *)pmd;
+ }
+
+ pr_debug("%s: addr:0x%lx sz:0x%lx ret pte=%p/0x%llx\n", __func__, addr,
+ sz, pte, pte_val(*pte));
+ return pte;
+}
+
+pte_t *huge_pte_offset(struct mm_struct *mm, unsigned long addr)
+{
+ pgd_t *pgd;
+ pud_t *pud;
+ pmd_t *pmd = NULL;
+ pte_t *pte = NULL;
+
+ pgd = pgd_offset(mm, addr);
+ pr_debug("%s: addr:0x%lx pgd:%p\n", __func__, addr, pgd);
+ if (!pgd_present(*pgd))
+ return NULL;
+ pud = pud_offset(pgd, addr);
+ if (!pud_present(*pud))
+ return NULL;
+
+ if (pud_huge(*pud))
+ return (pte_t *)pud;
+ pmd = pmd_offset(pud, addr);
+ if (!pmd_present(*pmd))
+ return NULL;
+
+ if (pte_cont(pmd_pte(*pmd))) {
+ pmd = pmd_offset(
+ pud, (addr & CONT_PMD_MASK));
+ return (pte_t *)pmd;
+ }
+ if (pmd_huge(*pmd))
+ return (pte_t *)pmd;
+ pte = pte_offset_kernel(pmd, addr);
+ if (pte_present(*pte) && pte_cont(*pte)) {
+ pte = pte_offset_kernel(
+ pmd, (addr & CONT_PTE_MASK));
+ }
+ return pte;
+}
+
+pte_t arch_make_huge_pte(pte_t entry, struct vm_area_struct *vma,
+ struct page *page, int writable)
+{
+ size_t pagesize = huge_page_size(hstate_vma(vma));
+
+ if (pagesize == CONT_PTE_SIZE) {
+ entry = pte_mkcont(entry);
+ } else if (pagesize == CONT_PMD_SIZE) {
+ entry = pmd_pte(pmd_mkcont(pte_pmd(entry)));
+ } else if (pagesize != PUD_SIZE && pagesize != PMD_SIZE) {
+ pr_warn("%s: unrecognized huge page size 0x%lx\n",
+ __func__, pagesize);
+ }
+ return entry;
+}
+
+pte_t huge_ptep_get_and_clear(struct mm_struct *mm,
+ unsigned long addr, pte_t *ptep)
+{
+ pte_t pte;
+
+ if (pte_cont(*ptep)) {
+ int ncontig, i;
+ size_t pgsize;
+ pte_t *cpte;
+ bool is_dirty = false;
+
+ cpte = huge_pte_offset(mm, addr);
+ ncontig = find_num_contig(mm, addr, cpte,
+ pte_val(*cpte), &pgsize);
+ /* save the 1st pte to return */
+ pte = ptep_get_and_clear(mm, addr, cpte);
+ for (i = 1; i < ncontig; ++i) {
+ /*
+ * If HW_AFDBM is enabled, then the HW could
+ * turn on the dirty bit for any of the page
+ * in the set, so check them all.
+ */
+ ++cpte;
+ if (pte_dirty(ptep_get_and_clear(mm, addr, cpte)))
+ is_dirty = true;
+ }
+ if (is_dirty)
+ return pte_mkdirty(pte);
+ else
+ return pte;
+ } else {
+ return ptep_get_and_clear(mm, addr, ptep);
+ }
+}
+
+int huge_ptep_set_access_flags(struct vm_area_struct *vma,
+ unsigned long addr, pte_t *ptep,
+ pte_t pte, int dirty)
+{
+ pte_t *cpte;
+
+ if (pte_cont(pte)) {
+ int ncontig, i, changed = 0;
+ size_t pgsize = 0;
+ unsigned long pfn = pte_pfn(pte);
+ /* Select all bits except the pfn */
+ pgprot_t hugeprot =
+ __pgprot(pte_val(pfn_pte(pfn, 0) ^ pte_val(pte)));
+
+ cpte = huge_pte_offset(vma->vm_mm, addr);
+ pfn = pte_pfn(*cpte);
+ ncontig = find_num_contig(vma->vm_mm, addr, cpte,
+ pte_val(*cpte), &pgsize);
+ for (i = 0; i < ncontig; ++i, ++cpte) {
+ changed = ptep_set_access_flags(vma, addr, cpte,
+ pfn_pte(pfn,
+ hugeprot),
+ dirty);
+ pfn += pgsize >> PAGE_SHIFT;
+ }
+ return changed;
+ } else {
+ return ptep_set_access_flags(vma, addr, ptep, pte, dirty);
+ }
+}
+
+void huge_ptep_set_wrprotect(struct mm_struct *mm,
+ unsigned long addr, pte_t *ptep)
+{
+ if (pte_cont(*ptep)) {
+ int ncontig, i;
+ pte_t *cpte;
+ size_t pgsize = 0;
+
+ cpte = huge_pte_offset(mm, addr);
+ ncontig = find_num_contig(mm, addr, cpte,
+ pte_val(*cpte), &pgsize);
+ for (i = 0; i < ncontig; ++i, ++cpte)
+ ptep_set_wrprotect(mm, addr, cpte);
+ } else {
+ ptep_set_wrprotect(mm, addr, ptep);
+ }
+}
+
+void huge_ptep_clear_flush(struct vm_area_struct *vma,
+ unsigned long addr, pte_t *ptep)
+{
+ if (pte_cont(*ptep)) {
+ int ncontig, i;
+ pte_t *cpte;
+ size_t pgsize = 0;
+
+ cpte = huge_pte_offset(vma->vm_mm, addr);
+ ncontig = find_num_contig(vma->vm_mm, addr, cpte,
+ pte_val(*cpte), &pgsize);
+ for (i = 0; i < ncontig; ++i, ++cpte)
+ ptep_clear_flush(vma, addr, cpte);
+ } else {
+ ptep_clear_flush(vma, addr, ptep);
+ }
+}
+
static __init int setup_hugepagesz(char *opt)
{
unsigned long ps = memparse(opt, &opt);
+
if (ps == PMD_SIZE) {
hugetlb_add_hstate(PMD_SHIFT - PAGE_SHIFT);
} else if (ps == PUD_SIZE) {
hugetlb_add_hstate(PUD_SHIFT - PAGE_SHIFT);
+ } else if (ps == (PAGE_SIZE * CONT_PTES)) {
+ hugetlb_add_hstate(CONT_PTE_SHIFT);
+ } else if (ps == (PMD_SIZE * CONT_PMDS)) {
+ hugetlb_add_hstate((PMD_SHIFT + CONT_PMD_SHIFT) - PAGE_SHIFT);
} else {
- pr_err("hugepagesz: Unsupported page size %lu M\n", ps >> 20);
+ pr_err("hugepagesz: Unsupported page size %lu K\n", ps >> 10);
return 0;
}
return 1;
}
__setup("hugepagesz=", setup_hugepagesz);
+
+#ifdef CONFIG_ARM64_64K_PAGES
+static __init int add_default_hugepagesz(void)
+{
+ if (size_to_hstate(CONT_PTES * PAGE_SIZE) == NULL)
+ hugetlb_add_hstate(CONT_PMD_SHIFT);
+ return 0;
+}
+arch_initcall(add_default_hugepagesz);
+#endif
diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
index 5e35379..4342cc2 100644
--- a/include/linux/hugetlb.h
+++ b/include/linux/hugetlb.h
@@ -96,9 +96,7 @@ u32 hugetlb_fault_mutex_hash(struct hstate *h, struct mm_struct *mm,
struct address_space *mapping,
pgoff_t idx, unsigned long address);

-#ifdef CONFIG_ARCH_WANT_HUGE_PMD_SHARE
pte_t *huge_pmd_share(struct mm_struct *mm, unsigned long addr, pud_t *pud);
-#endif

extern int hugepages_treat_as_movable;
extern int sysctl_hugetlb_shm_group;
--
2.1.2

yalin wang

unread,
Nov 20, 2015, 5:07:44 AM11/20/15
to David Woods, catalin...@arm.com, will....@arm.com, steve....@linaro.org, jeremy...@arm.com, linux-ar...@lists.infradead.org, linux-...@vger.kernel.org, linu...@kvack.org, cmet...@ezchip.com
is this should be pte_val(pfn_pte(pfn, 0)) ^ pte_val(pte) ?

> + for (i = 0; i < ncontig; i++) {
> + pr_debug("%s: set pte %p to 0x%llx\n", __func__, ptep,
> + pfn_pte(pfn, hugeprot));
> + set_pte_at(mm, addr, ptep, pfn_pte(pfn, hugeprot));
> + ptep++;
> + pfn += pgsize >> PAGE_SHIFT;
> + addr += pgsize;
> + }
> +}
>

David Woods

unread,
Nov 20, 2015, 11:21:16 AM11/20/15
to yalin wang, catalin...@arm.com, will....@arm.com, steve....@linaro.org, jeremy...@arm.com, linux-ar...@lists.infradead.org, linux-...@vger.kernel.org, linu...@kvack.org, cmet...@ezchip.com
On 11/20/2015 05:07 AM, yalin wang wrote:
>> +
>> +void set_huge_pte_at(struct mm_struct *mm, unsigned long addr,
>> + pte_t *ptep, pte_t pte)
>> +{
>> + size_t pgsize;
>> + int i;
>> + int ncontig = find_num_contig(mm, addr, ptep, pte, &pgsize);
>> + unsigned long pfn;
>> + pgprot_t hugeprot;
>> +
>> + if (ncontig == 1) {
>> + set_pte_at(mm, addr, ptep, pte);
>> + return;
>> + }
>> +
>> + pfn = pte_pfn(pte);
>> + hugeprot = __pgprot(pte_val(pfn_pte(pfn, 0) ^ pte_val(pte)));
> is this should be pte_val(pfn_pte(pfn, 0)) ^ pte_val(pte) ?
>
The code generated is identical either way, but I agree your way looks
better.

-Dave
0 new messages