[RFC-PATCH V2] ARM hugetlb support

[RFC-PATCH V2] ARM hugetlb support

[Bill Carson]

Hi All

This is 2nd RFC PATCH for ARM huge page support. 2MB(two 1MB page)/16MB
huge page are supported, Versatile Express Cortex-A9x4 tile is used as test 
board. Verification is done with libhugetlbfs with both arm/thumb2 instruction.

ChangeLog From V1:
    - Drop L_PTE_HUGEPAGE defines
    - Check whether super-section is supported through mmfr3
    - Using mmu_context_t to store huge linux pte instead of mm_struct

Any suggestions would be truly welcome :)

Bill Carson(3)
    - Add hugetlb high level hooks
    - Using mmu_context_t to store huge linux pte
    - Introduce set_hugepte_ext to set hardware pte

Signed-off-by: Bill Carson <bill4carson@gmail.com>
---
 arch/arm/Kconfig                      |   29 ++++++
 arch/arm/include/asm/glue-proc.h      |    3 +
 arch/arm/include/asm/hugetlb.h        |  178 +++++++++++++++++++++++++++++++++
 arch/arm/include/asm/mmu.h            |    4 +
 arch/arm/include/asm/page.h           |   15 +++
 arch/arm/include/asm/pgtable-2level.h |    3 +
 arch/arm/include/asm/pgtable.h        |    9 ++
 arch/arm/include/asm/proc-fns.h       |    3 +
 arch/arm/mm/Makefile                  |    1 +
 arch/arm/mm/dma-mapping.c             |    3 -
 arch/arm/mm/fault.c                   |   15 +++
 arch/arm/mm/hugetlb.c                 |  132 ++++++++++++++++++++++++
 arch/arm/mm/pgd.c                     |   11 ++
 arch/arm/mm/proc-v7-2level.S          |   87 ++++++++++++++++
 14 files changed, 490 insertions(+), 3 deletions(-)
 create mode 100644 arch/arm/include/asm/hugetlb.h
 create mode 100644 arch/arm/mm/hugetlb.c

[PATCH 1/7] Add various hugetlb arm high level hooks

[Bill Carson]

Signed-off-by: Bill Carson <bill4carson@gmail.com>
---
 arch/arm/include/asm/hugetlb.h |  178 ++++++++++++++++++++++++++++++++++++++++
 arch/arm/include/asm/page.h    |   15 ++++
 arch/arm/mm/hugetlb.c          |  132 +++++++++++++++++++++++++++++
 3 files changed, 325 insertions(+), 0 deletions(-)
 create mode 100644 arch/arm/include/asm/hugetlb.h
 create mode 100644 arch/arm/mm/hugetlb.c

diff --git a/arch/arm/include/asm/hugetlb.h b/arch/arm/include/asm/hugetlb.h
new file mode 100644
index 0000000..3c34528
--- /dev/null
+++ b/arch/arm/include/asm/hugetlb.h
@@ -0,0 +1,178 @@
+/*
+ * hugetlb.h, ARM Huge Tlb Page support.
+ *
+ * Copyright (c) Bill Carson
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
+ * See the GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ *
+ */
+
+#ifndef __ASM_HUGETLB_H
+#define __ASM_HUGETLB_H
+
+#include <asm/page.h>
+#include <asm/pgtable-2level.h>
+#include <asm/tlb.h>
+
+
+/* 2M and 16M hugepage linux ptes are stored in mmu_context_t->huge_linux_pte
+ *
+ * 2M hugepage
+ * ===========
+ * one huge linux pte caters to two HW ptes,
+ *
+ * 16M hugepage
+ * ============
+ * one huge linux pte caters for sixteen HW ptes,
+ *
+ * The number of huge linux ptes depends on PAGE_OFFSET configuration
+ * which is defined as following:
+ */
+#define HUGE_LINUX_PTE_COUNT	( PAGE_OFFSET >> HPAGE_SHIFT)
+#define HUGE_LINUX_PTE_SIZE		(HUGE_LINUX_PTE_COUNT * sizeof(pte_t *))
+#define HUGE_LINUX_PTE_INDEX(addr) (addr >> HPAGE_SHIFT)
+
+static inline int is_hugepage_only_range(struct mm_struct *mm,
+					 unsigned long addr,
+					 unsigned long len)
+{
+	return 0;
+}
+
+static inline int prepare_hugepage_range(struct file *file,
+					 unsigned long addr,
+					 unsigned long len)
+{
+	struct hstate *h = hstate_file(file);
+	/* addr/len should be aligned with huge page size */
+	if (len & ~huge_page_mask(h))
+		return -EINVAL;
+	if (addr & ~huge_page_mask(h))
+		return -EINVAL;
+
+	return 0;
+}
+
+static inline void hugetlb_prefault_arch_hook(struct mm_struct *mm)
+{
+}
+
+static inline void hugetlb_free_pgd_range(struct mmu_gather *tlb,
+			unsigned long addr, unsigned long end,
+			unsigned long floor, unsigned long ceiling)
+{
+}
+
+static inline void set_huge_pte_at(struct mm_struct *mm, unsigned long addr,
+				   pte_t *ptep, pte_t pte)
+{
+	pgd_t *pgd;
+	pud_t *pud;
+	pmd_t *pmd;
+	pte_t *linuxpte = mm->context.huge_linux_pte;
+
+	BUG_ON(linuxpte == NULL);
+	BUG_ON(HUGE_LINUX_PTE_INDEX(addr) >= HUGE_LINUX_PTE_COUNT);
+	BUG_ON(ptep != &linuxpte[HUGE_LINUX_PTE_INDEX(addr)]);
+
+	/* set huge linux pte first */
+	*ptep = pte;
+	
+	/* then set hardware pte */
+	addr &= HPAGE_MASK;
+	pgd = pgd_offset(mm, addr);
+	pud = pud_offset(pgd, addr);
+	pmd = pmd_offset(pud, addr);
+	set_hugepte_at(mm, addr, pmd, pte);
+}
+
+static inline pte_t huge_ptep_get_and_clear(struct mm_struct *mm,
+					    unsigned long addr, pte_t *ptep)
+{
+	pte_t pte = *ptep;
+	pte_t fake = L_PTE_YOUNG;
+	pgd_t *pgd;
+	pud_t *pud;
+	pmd_t *pmd;
+
+	/* clear linux pte */
+	*ptep = 0;
+
+	/* let set_hugepte_at clear HW entry */
+	addr &= HPAGE_MASK;
+	pgd = pgd_offset(mm, addr);
+	pud = pud_offset(pgd, addr);
+	pmd = pmd_offset(pud, addr);
+	set_hugepte_at(mm, addr, pmd, fake);
+	return pte;
+}
+
+static inline void huge_ptep_clear_flush(struct vm_area_struct *vma,
+					 unsigned long addr, pte_t *ptep)
+{
+	if (HPAGE_SHIFT == SUPERSECTION_SHIFT)
+		flush_tlb_page(vma, addr & SUPERSECTION_MASK);
+	else {
+		flush_tlb_page(vma, addr & SECTION_MASK);
+		flush_tlb_page(vma, (addr & SECTION_MASK)^0x100000);
+	}
+}
+
+static inline int huge_pte_none(pte_t pte)
+{
+	return pte_none(pte);
+}
+
+static inline pte_t huge_pte_wrprotect(pte_t pte)
+{
+	return pte_wrprotect(pte);
+}
+
+static inline void huge_ptep_set_wrprotect(struct mm_struct *mm,
+					   unsigned long addr, pte_t *ptep)
+{
+	pte_t old_pte = *ptep;
+	set_huge_pte_at(mm, addr, ptep, pte_wrprotect(old_pte));
+}
+
+static inline pte_t huge_ptep_get(pte_t *ptep)
+{
+	return *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)
+{
+	int changed = !pte_same(huge_ptep_get(ptep), pte);
+	if (changed) {
+		set_huge_pte_at(vma->vm_mm, addr, ptep, pte);
+		huge_ptep_clear_flush(vma, addr, &pte);
+	}
+
+	return changed;
+}
+
+static inline int arch_prepare_hugepage(struct page *page)
+{
+	return 0;
+}
+
+static inline void arch_release_hugepage(struct page *page)
+{
+}
+
+#endif /* __ASM_HUGETLB_H */
+
diff --git a/arch/arm/include/asm/page.h b/arch/arm/include/asm/page.h
index 97b440c..3e6769a 100644
--- a/arch/arm/include/asm/page.h
+++ b/arch/arm/include/asm/page.h
@@ -15,6 +15,21 @@
 #define PAGE_SIZE		(_AC(1,UL) << PAGE_SHIFT)
 #define PAGE_MASK		(~(PAGE_SIZE-1))
 
+#ifdef CONFIG_HUGEPAGE_SIZE_2MB
+/* we have 2MB hugepage for two 1MB section mapping */
+#define HPAGE_SHIFT		(SECTION_SHIFT + 1)
+#define HPAGE_SIZE		(_AC(1, UL) << HPAGE_SHIFT)
+#define HPAGE_MASK		(~(HPAGE_SIZE - 1))
+#define HUGETLB_PAGE_ORDER	(HPAGE_SHIFT - PAGE_SHIFT)
+#endif
+
+#ifdef CONFIG_HUGEPAGE_SIZE_16MB
+#define HPAGE_SHIFT		SUPERSECTION_SHIFT
+#define HPAGE_SIZE		(_AC(1, UL) << HPAGE_SHIFT)
+#define HPAGE_MASK		(~(HPAGE_SIZE - 1))
+#define HUGETLB_PAGE_ORDER	(HPAGE_SHIFT - PAGE_SHIFT)
+#endif
+
 #ifndef __ASSEMBLY__
 
 #ifndef CONFIG_MMU
diff --git a/arch/arm/mm/hugetlb.c b/arch/arm/mm/hugetlb.c
new file mode 100644
index 0000000..165bd8f
--- /dev/null
+++ b/arch/arm/mm/hugetlb.c
@@ -0,0 +1,132 @@
+/*
+ * hugetlb.c, ARM Huge Tlb Page support.
+ *
+ * Copyright (c) Bill Carson
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
+ * See the GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ *
+ */
+
+#include <linux/hugetlb.h>
+
+pte_t *huge_pte_alloc(struct mm_struct *mm, unsigned long addr,
+		      unsigned long sz)
+{
+	pte_t *linuxpte = mm->context.huge_linux_pte;
+	int index;
+
+	if (linuxpte == NULL) {
+		linuxpte = kzalloc(HUGE_LINUX_PTE_SIZE, GFP_ATOMIC);
+		if (linuxpte == NULL) {
+			printk(KERN_ERR "Cannot allocate memory for huge linux pte\n");
+			return NULL;
+		}
+		mm->context.huge_linux_pte = linuxpte;
+	}
+	/* huge page mapping only cover user space address */
+	BUG_ON(HUGE_LINUX_PTE_INDEX(addr) >= HUGE_LINUX_PTE_COUNT);
+	index = HUGE_LINUX_PTE_INDEX(addr);
+	return &linuxpte[HUGE_LINUX_PTE_INDEX(addr)];
+}
+
+pte_t *huge_pte_offset(struct mm_struct *mm, unsigned long addr)
+{
+	pgd_t *pgd;
+	pud_t *pud;
+	pmd_t *pmd = NULL;
+	pte_t *linuxpte = mm->context.huge_linux_pte;
+
+	/* check this mapping exist at pmd level */
+	pgd = pgd_offset(mm, addr);
+	if (pgd_present(*pgd)) {
+		pud = pud_offset(pgd, addr);
+		pmd = pmd_offset(pud, addr);
+		if (!pmd_present(*pmd))
+			return NULL;
+	}
+
+	BUG_ON(HUGE_LINUX_PTE_INDEX(addr) >= HUGE_LINUX_PTE_COUNT);
+	BUG_ON((*pmd & PMD_TYPE_MASK) != PMD_TYPE_SECT);
+	return &linuxpte[HUGE_LINUX_PTE_INDEX(addr)];
+}
+
+int huge_pmd_unshare(struct mm_struct *mm, unsigned long *addr, pte_t *ptep)
+{
+	return 0;
+}
+
+struct page *follow_huge_addr(struct mm_struct *mm, unsigned long address,
+				int write)
+{
+	return ERR_PTR(-EINVAL);
+}
+
+int pmd_huge(pmd_t pmd)
+{
+	return (pmd_val(pmd) & PMD_TYPE_MASK) == PMD_TYPE_SECT;
+}
+
+int pud_huge(pud_t pud)
+{
+	return  0; } struct page * follow_huge_pmd(struct mm_struct *mm, unsigned long address, pmd_t *pmd, int write)
+{
+	struct page *page = NULL;
+	unsigned long pfn;
+
+	BUG_ON((pmd_val(*pmd) & PMD_TYPE_MASK) != PMD_TYPE_SECT);
+	pfn = ((pmd_val(*pmd) & HPAGE_MASK) >> PAGE_SHIFT);
+	page = pfn_to_page(pfn);
+	return page;
+}
+
+static int __init add_huge_page_size(unsigned long long size)
+{
+	int shift = __ffs(size);
+	u32 mmfr3 = 0;
+
+	/* Check that it is a page size supported by the hardware and
+	 * that it fits within pagetable and slice limits. */
+	if (!is_power_of_2(size) || (shift != HPAGE_SHIFT))
+		return -EINVAL;
+
+	/* If user wants super-section support, then check if our cpu
+	 * has this feature supported in ID_MMFR3 */
+	if (shift == SUPERSECTION_SHIFT) {
+		__asm__("mrc p15, 0, %0, c0, c1, 7\n" : "=r" (mmfr3));
+		if (mmfr3 & 0xF0000000) {
+			printk("Super-Section is NOT supported by this CPU, mmfr3:0x%x\n", mmfr3);
+			return -EINVAL;
+		}
+	}
+
+	/* Return if huge page size has already been setup */
+	if (size_to_hstate(size))
+		return 0;
+
+	hugetlb_add_hstate(shift - PAGE_SHIFT);
+	return 0;
+}
+
+static int __init hugepage_setup_sz(char *str)
+{
+	unsigned long long size;
+
+	size = memparse(str, &str);
+	if (add_huge_page_size(size) != 0)
+		printk(KERN_WARNING "Invalid huge page size specified(%llu)\n",
+			 size);
+
+	return 1;
+}
+__setup("hugepagesz=", hugepage_setup_sz);
-- 
1.7.1

[PATCH 1/7] Add various hugetlb arm high level hooks

[Catalin Marinas]

On Mon, Feb 13, 2012 at 09:44:22AM +0000, Bill Carson wrote:
> diff --git a/arch/arm/include/asm/page.h b/arch/arm/include/asm/page.h
> index 97b440c..3e6769a 100644
> --- a/arch/arm/include/asm/page.h
> +++ b/arch/arm/include/asm/page.h
> @@ -15,6 +15,21 @@
>  #define PAGE_SIZE		(_AC(1,UL) << PAGE_SHIFT)
>  #define PAGE_MASK		(~(PAGE_SIZE-1))
>  
> +#ifdef CONFIG_HUGEPAGE_SIZE_2MB
> +/* we have 2MB hugepage for two 1MB section mapping */
> +#define HPAGE_SHIFT		(SECTION_SHIFT + 1)
> +#define HPAGE_SIZE		(_AC(1, UL) << HPAGE_SHIFT)
> +#define HPAGE_MASK		(~(HPAGE_SIZE - 1))
> +#define HUGETLB_PAGE_ORDER	(HPAGE_SHIFT - PAGE_SHIFT)
> +#endif
> +
> +#ifdef CONFIG_HUGEPAGE_SIZE_16MB
> +#define HPAGE_SHIFT		SUPERSECTION_SHIFT
> +#define HPAGE_SIZE		(_AC(1, UL) << HPAGE_SHIFT)
> +#define HPAGE_MASK		(~(HPAGE_SIZE - 1))
> +#define HUGETLB_PAGE_ORDER	(HPAGE_SHIFT - PAGE_SHIFT)
> +#endif

Ah, you still have these just config time options. Can you not make an
hpage_shift variable like PowerPC or IA-64?

(I haven't yet reviewed the rest of the code)

-- 
Catalin

[PATCH 1/7] Add various hugetlb arm high level hooks

[bill4carson]

On 2012?02?29? 01:30, Catalin Marinas wrote:
> On Mon, Feb 13, 2012 at 09:44:22AM +0000, Bill Carson wrote:
>> diff --git a/arch/arm/include/asm/page.h b/arch/arm/include/asm/page.h
>> index 97b440c..3e6769a 100644
>> --- a/arch/arm/include/asm/page.h
>> +++ b/arch/arm/include/asm/page.h
>> @@ -15,6 +15,21 @@
>>   #define PAGE_SIZE		(_AC(1,UL)<<  PAGE_SHIFT)
>>   #define PAGE_MASK		(~(PAGE_SIZE-1))
>>
>> +#ifdef CONFIG_HUGEPAGE_SIZE_2MB
>> +/* we have 2MB hugepage for two 1MB section mapping */
>> +#define HPAGE_SHIFT		(SECTION_SHIFT + 1)
>> +#define HPAGE_SIZE		(_AC(1, UL)<<  HPAGE_SHIFT)
>> +#define HPAGE_MASK		(~(HPAGE_SIZE - 1))
>> +#define HUGETLB_PAGE_ORDER	(HPAGE_SHIFT - PAGE_SHIFT)
>> +#endif
>> +
>> +#ifdef CONFIG_HUGEPAGE_SIZE_16MB
>> +#define HPAGE_SHIFT		SUPERSECTION_SHIFT
>> +#define HPAGE_SIZE		(_AC(1, UL)<<  HPAGE_SHIFT)
>> +#define HPAGE_MASK		(~(HPAGE_SIZE - 1))
>> +#define HUGETLB_PAGE_ORDER	(HPAGE_SHIFT - PAGE_SHIFT)
>> +#endif
>
> Ah, you still have these just config time options. Can you not make an
> hpage_shift variable like PowerPC or IA-64?
>
Hi, Catalin

Thank you for reviewing this patch.


unsigned int hpage_shift = SECTION_SHIFT + 1; /* default to 2MB page */
#define HPAGE_SHIFT hpage_shift

then user could configure hpage_shift through "hugepagesz" parameter.
I think this is what I am supposed to do,
If so, hugepage_setup_sz/cpu_v7_set_hugepte_ext needs modification


> (I haven't yet reviewed the rest of the code)
>


So, should I send another version now, or wait until you walk through
the reset of patch set?

And again, thanks for the patch reviewing:)


-- 
I am a slow learner
but I will keep trying to fight for my dreams!

--bill

[PATCH 1/7] Add various hugetlb arm high level hooks

[Catalin Marinas]

On Wed, Feb 29, 2012 at 02:34:34AM +0000, bill4carson wrote:
> On 2012?02?29? 01:30, Catalin Marinas wrote:
> > On Mon, Feb 13, 2012 at 09:44:22AM +0000, Bill Carson wrote:
> >> diff --git a/arch/arm/include/asm/page.h b/arch/arm/include/asm/page.h
> >> index 97b440c..3e6769a 100644
> >> --- a/arch/arm/include/asm/page.h
> >> +++ b/arch/arm/include/asm/page.h
> >> @@ -15,6 +15,21 @@
> >>   #define PAGE_SIZE		(_AC(1,UL)<<  PAGE_SHIFT)
> >>   #define PAGE_MASK		(~(PAGE_SIZE-1))
> >>
> >> +#ifdef CONFIG_HUGEPAGE_SIZE_2MB
> >> +/* we have 2MB hugepage for two 1MB section mapping */
> >> +#define HPAGE_SHIFT		(SECTION_SHIFT + 1)
> >> +#define HPAGE_SIZE		(_AC(1, UL)<<  HPAGE_SHIFT)
> >> +#define HPAGE_MASK		(~(HPAGE_SIZE - 1))
> >> +#define HUGETLB_PAGE_ORDER	(HPAGE_SHIFT - PAGE_SHIFT)
> >> +#endif
> >> +
> >> +#ifdef CONFIG_HUGEPAGE_SIZE_16MB
> >> +#define HPAGE_SHIFT		SUPERSECTION_SHIFT
> >> +#define HPAGE_SIZE		(_AC(1, UL)<<  HPAGE_SHIFT)
> >> +#define HPAGE_MASK		(~(HPAGE_SIZE - 1))
> >> +#define HUGETLB_PAGE_ORDER	(HPAGE_SHIFT - PAGE_SHIFT)
> >> +#endif
> >
> > Ah, you still have these just config time options. Can you not make an
> > hpage_shift variable like PowerPC or IA-64?
> 
> Thank you for reviewing this patch.

I haven't finished yet (another email to come :)).

> unsigned int hpage_shift = SECTION_SHIFT + 1; /* default to 2MB page */
> #define HPAGE_SHIFT hpage_shift

Something like that. You could even use PMD_SHIFT which is 21 already
and it makes it clearer that we use huge pages at the pmd level.

> So, should I send another version now, or wait until you walk through
> the reset of patch set?

Just wait, I'll have a look through all the patches.

BTW, there are several coding style issues, I won't go through them but
it would help if you have a look at Documentation/CodingStyle
(especially comment style, if/else brackets).

Thanks.

-- 
Catalin

[PATCH 1/7] Add various hugetlb arm high level hooks

[bill4carson]

On 2012?02?29? 17:39, Catalin Marinas wrote:
> On Wed, Feb 29, 2012 at 02:34:34AM +0000, bill4carson wrote:
>> On 2012?02?29? 01:30, Catalin Marinas wrote:
>>> On Mon, Feb 13, 2012 at 09:44:22AM +0000, Bill Carson wrote:
>>>> diff --git a/arch/arm/include/asm/page.h b/arch/arm/include/asm/page.h
>>>> index 97b440c..3e6769a 100644
>>>> --- a/arch/arm/include/asm/page.h
>>>> +++ b/arch/arm/include/asm/page.h
>>>> @@ -15,6 +15,21 @@
>>>>    #define PAGE_SIZE		(_AC(1,UL)<<   PAGE_SHIFT)
>>>>    #define PAGE_MASK		(~(PAGE_SIZE-1))
>>>>
>>>> +#ifdef CONFIG_HUGEPAGE_SIZE_2MB
>>>> +/* we have 2MB hugepage for two 1MB section mapping */
>>>> +#define HPAGE_SHIFT		(SECTION_SHIFT + 1)
>>>> +#define HPAGE_SIZE		(_AC(1, UL)<<   HPAGE_SHIFT)
>>>> +#define HPAGE_MASK		(~(HPAGE_SIZE - 1))
>>>> +#define HUGETLB_PAGE_ORDER	(HPAGE_SHIFT - PAGE_SHIFT)
>>>> +#endif
>>>> +
>>>> +#ifdef CONFIG_HUGEPAGE_SIZE_16MB
>>>> +#define HPAGE_SHIFT		SUPERSECTION_SHIFT
>>>> +#define HPAGE_SIZE		(_AC(1, UL)<<   HPAGE_SHIFT)
>>>> +#define HPAGE_MASK		(~(HPAGE_SIZE - 1))
>>>> +#define HUGETLB_PAGE_ORDER	(HPAGE_SHIFT - PAGE_SHIFT)
>>>> +#endif
>>>
>>> Ah, you still have these just config time options. Can you not make an
>>> hpage_shift variable like PowerPC or IA-64?
>>
>> Thank you for reviewing this patch.
>
> I haven't finished yet (another email to come :)).
>
I'm looking forward any comments/suggestions ^_^


>> unsigned int hpage_shift = SECTION_SHIFT + 1; /* default to 2MB page */
>> #define HPAGE_SHIFT hpage_shift
>
> Something like that. You could even use PMD_SHIFT which is 21 already
> and it makes it clearer that we use huge pages at the pmd level.
>
OK, I will keep that in mind for V3.


>> So, should I send another version now, or wait until you walk through
>> the reset of patch set?
>
> Just wait, I'll have a look through all the patches.
>
> BTW, there are several coding style issues, I won't go through them but
> it would help if you have a look at Documentation/CodingStyle
> (especially comment style, if/else brackets).
>

Yes, I forgot to run checkpatch.pl to sanitize this patch set.
Will keep in mind for all next version.

thanks

> Thanks.
>

-- 
I am a slow learner
but I will keep trying to fight for my dreams!

--bill

[PATCH 1/7] Add various hugetlb arm high level hooks

[Catalin Marinas]

On 29 February 2012 10:21, bill4carson <bill4carson@gmail.com> wrote:
> On 2012?02?29? 17:39, Catalin Marinas wrote:
>> BTW, there are several coding style issues, I won't go through them but
>> it would help if you have a look at Documentation/CodingStyle
>> (especially comment style, if/else brackets).
>
> Yes, I forgot to run checkpatch.pl to sanitize this patch set.

Don't just rely on checkpatch.pl, have a look at the CodingStyle doc :)

-- 
Catalin

[PATCH 1/7] Add various hugetlb arm high level hooks

[Catalin Marinas]

On Mon, Feb 13, 2012 at 09:44:22AM +0000, Bill Carson wrote:
> --- /dev/null
> +++ b/arch/arm/include/asm/hugetlb.h
...
> +#include <asm/page.h>
> +#include <asm/pgtable-2level.h>

Just include asm/pgtable.h, it includes the right 2level.h file
automatically (and I plan to add LPAE support as well).

> +static inline void set_huge_pte_at(struct mm_struct *mm, unsigned long addr,
> +				   pte_t *ptep, pte_t pte)
> +{
> +	pgd_t *pgd;
> +	pud_t *pud;
> +	pmd_t *pmd;
> +	pte_t *linuxpte = mm->context.huge_linux_pte;
> +
> +	BUG_ON(linuxpte == NULL);
> +	BUG_ON(HUGE_LINUX_PTE_INDEX(addr) >= HUGE_LINUX_PTE_COUNT);
> +	BUG_ON(ptep != &linuxpte[HUGE_LINUX_PTE_INDEX(addr)]);
> +
> +	/* set huge linux pte first */
> +	*ptep = pte;
> +	
> +	/* then set hardware pte */
> +	addr &= HPAGE_MASK;
> +	pgd = pgd_offset(mm, addr);
> +	pud = pud_offset(pgd, addr);
> +	pmd = pmd_offset(pud, addr);
> +	set_hugepte_at(mm, addr, pmd, pte);

You may want to add a comment here that we only have two levels of page
tables (and there is no need for pud_none() checks).

Also I would say the set_hugepte_at function name is easily confused
with set_huge_pte_at(), maybe change it to __set_huge_pte_at() or
something else.

> +static inline pte_t huge_ptep_get_and_clear(struct mm_struct *mm,
> +					    unsigned long addr, pte_t *ptep)
> +{
> +	pte_t pte = *ptep;
> +	pte_t fake = L_PTE_YOUNG;
> +	pgd_t *pgd;
> +	pud_t *pud;
> +	pmd_t *pmd;
> +
> +	/* clear linux pte */
> +	*ptep = 0;
> +
> +	/* let set_hugepte_at clear HW entry */
> +	addr &= HPAGE_MASK;
> +	pgd = pgd_offset(mm, addr);
> +	pud = pud_offset(pgd, addr);
> +	pmd = pmd_offset(pud, addr);
> +	set_hugepte_at(mm, addr, pmd, fake);

Same here.

> +static inline void huge_ptep_set_wrprotect(struct mm_struct *mm,
> +					   unsigned long addr, pte_t *ptep)
> +{
> +	pte_t old_pte = *ptep;
> +	set_huge_pte_at(mm, addr, ptep, pte_wrprotect(old_pte));
> +}

You could use the generic ptep_set_wrprotect()

> +static inline int huge_ptep_set_access_flags(struct vm_area_struct *vma,
> +					     unsigned long addr,
> +					     pte_t *ptep, pte_t pte,
> +					     int dirty)
> +{
> +	int changed = !pte_same(huge_ptep_get(ptep), pte);
> +	if (changed) {
> +		set_huge_pte_at(vma->vm_mm, addr, ptep, pte);
> +		huge_ptep_clear_flush(vma, addr, &pte);
> +	}
> +
> +	return changed;
> +}

I could also use the generic ptep_set_access_flags().

> --- /dev/null
> +++ b/arch/arm/mm/hugetlb.c
...
> +pte_t *huge_pte_alloc(struct mm_struct *mm, unsigned long addr,
> +		      unsigned long sz)
> +{
> +	pte_t *linuxpte = mm->context.huge_linux_pte;
> +	int index;
> +
> +	if (linuxpte == NULL) {
> +		linuxpte = kzalloc(HUGE_LINUX_PTE_SIZE, GFP_ATOMIC);
> +		if (linuxpte == NULL) {
> +			printk(KERN_ERR "Cannot allocate memory for huge linux pte\n");

pr_err()?

> +			return NULL;
> +		}
> +		mm->context.huge_linux_pte = linuxpte;
> +	}
> +	/* huge page mapping only cover user space address */
> +	BUG_ON(HUGE_LINUX_PTE_INDEX(addr) >= HUGE_LINUX_PTE_COUNT);
> +	index = HUGE_LINUX_PTE_INDEX(addr);
> +	return &linuxpte[HUGE_LINUX_PTE_INDEX(addr)];
> +}
> +
> +pte_t *huge_pte_offset(struct mm_struct *mm, unsigned long addr)
> +{
> +	pgd_t *pgd;
> +	pud_t *pud;
> +	pmd_t *pmd = NULL;
> +	pte_t *linuxpte = mm->context.huge_linux_pte;
> +
> +	/* check this mapping exist at pmd level */
> +	pgd = pgd_offset(mm, addr);
> +	if (pgd_present(*pgd)) {
> +		pud = pud_offset(pgd, addr);
> +		pmd = pmd_offset(pud, addr);
> +		if (!pmd_present(*pmd))
> +			return NULL;
> +	}

You could add checks for the pud as well, they would be optimised out by
the compiler but it would be easier to add support for LPAE as well. In
my LPAE hugetlb implementation, I have something like this:

	pgd = pgd_offset(mm, addr);
	if (pgd_present(*pgd)) {
		pud = pud_offset(pgd, addr);
		if (pud_present(*pud))
			pmd = pmd_offset(pud, addr);
	}

> +	BUG_ON(HUGE_LINUX_PTE_INDEX(addr) >= HUGE_LINUX_PTE_COUNT);
> +	BUG_ON((*pmd & PMD_TYPE_MASK) != PMD_TYPE_SECT);
> +	return &linuxpte[HUGE_LINUX_PTE_INDEX(addr)];

You could add a macro to make it easier for LPAE:

#define huge_pte(mm, pmd, addr)		\
	(&mm->context.huge_linux_pte(HUGE_LINUX_PTE_INDEX(addr)))

With LPAE, it would simply be a (pte_t *)pmd cast.

> +int pud_huge(pud_t pud)
> +{
> +	return  0; } struct page * follow_huge_pmd(struct mm_struct *mm, unsigned long address, pmd_t *pmd, int write)

Something went wrong around here.

> +{
> +	struct page *page = NULL;

You don't need to initialise page here.

> +	unsigned long pfn;
> +
> +	BUG_ON((pmd_val(*pmd) & PMD_TYPE_MASK) != PMD_TYPE_SECT);
> +	pfn = ((pmd_val(*pmd) & HPAGE_MASK) >> PAGE_SHIFT);
> +	page = pfn_to_page(pfn);
> +	return page;
> +}
> +
> +static int __init add_huge_page_size(unsigned long long size)
> +{
> +	int shift = __ffs(size);
> +	u32 mmfr3 = 0;
> +
> +	/* Check that it is a page size supported by the hardware and
> +	 * that it fits within pagetable and slice limits. */
> +	if (!is_power_of_2(size) || (shift != HPAGE_SHIFT))
> +		return -EINVAL;

You could use get_order() instead of __ffs(), the latter just finds the
first bit set.

But here you should have set hpage_shift.

-- 
Catalin

[PATCH 1/7] Add various hugetlb arm high level hooks

[bill4carson]

On 2012?02?29? 18:32, Catalin Marinas wrote:
> On Mon, Feb 13, 2012 at 09:44:22AM +0000, Bill Carson wrote:
>> --- /dev/null
>> +++ b/arch/arm/include/asm/hugetlb.h
> ...
>> +#include<asm/page.h>
>> +#include<asm/pgtable-2level.h>
>
> Just include asm/pgtable.h, it includes the right 2level.h file
> automatically (and I plan to add LPAE support as well).
>
>> +static inline void set_huge_pte_at(struct mm_struct *mm, unsigned long addr,
>> +				   pte_t *ptep, pte_t pte)
>> +{
>> +	pgd_t *pgd;
>> +	pud_t *pud;
>> +	pmd_t *pmd;
>> +	pte_t *linuxpte = mm->context.huge_linux_pte;
>> +
>> +	BUG_ON(linuxpte == NULL);
>> +	BUG_ON(HUGE_LINUX_PTE_INDEX(addr)>= HUGE_LINUX_PTE_COUNT);
>> +	BUG_ON(ptep !=&linuxpte[HUGE_LINUX_PTE_INDEX(addr)]);
>> +
>> +	/* set huge linux pte first */
>> +	*ptep = pte;
>> +	
>> +	/* then set hardware pte */
>> +	addr&= HPAGE_MASK;
>> +	pgd = pgd_offset(mm, addr);
>> +	pud = pud_offset(pgd, addr);
>> +	pmd = pmd_offset(pud, addr);
>> +	set_hugepte_at(mm, addr, pmd, pte);
>
> You may want to add a comment here that we only have two levels of page
> tables (and there is no need for pud_none() checks).
>
I will add a comment to clarify this.

> Also I would say the set_hugepte_at function name is easily confused
> with set_huge_pte_at(), maybe change it to __set_huge_pte_at() or
> something else.
>
yes, I will make that change.


>> +static inline pte_t huge_ptep_get_and_clear(struct mm_struct *mm,
>> +					    unsigned long addr, pte_t *ptep)
>> +{
>> +	pte_t pte = *ptep;
>> +	pte_t fake = L_PTE_YOUNG;
>> +	pgd_t *pgd;
>> +	pud_t *pud;
>> +	pmd_t *pmd;
>> +
>> +	/* clear linux pte */
>> +	*ptep = 0;
>> +
>> +	/* let set_hugepte_at clear HW entry */
>> +	addr&= HPAGE_MASK;
>> +	pgd = pgd_offset(mm, addr);
>> +	pud = pud_offset(pgd, addr);
>> +	pmd = pmd_offset(pud, addr);
>> +	set_hugepte_at(mm, addr, pmd, fake);
>
> Same here.
>
>> +static inline void huge_ptep_set_wrprotect(struct mm_struct *mm,
>> +					   unsigned long addr, pte_t *ptep)
>> +{
>> +	pte_t old_pte = *ptep;
>> +	set_huge_pte_at(mm, addr, ptep, pte_wrprotect(old_pte));
>> +}
>
> You could use the generic ptep_set_wrprotect()

I'm a bit of confused about this.

generic ptep_set_wrprotect() can not set huge pte, that's why
set_huge_pte_at is used instead here.


>
>> +static inline int huge_ptep_set_access_flags(struct vm_area_struct *vma,
>> +					     unsigned long addr,
>> +					     pte_t *ptep, pte_t pte,
>> +					     int dirty)
>> +{
>> +	int changed = !pte_same(huge_ptep_get(ptep), pte);
>> +	if (changed) {
>> +		set_huge_pte_at(vma->vm_mm, addr, ptep, pte);
>> +		huge_ptep_clear_flush(vma, addr,&pte);
>> +	}
>> +
>> +	return changed;
>> +}
>
> I could also use the generic ptep_set_access_flags().

Same as above.

IMHO, cannot use generic hooks here, cause we are setting huge pte
with a different set pte API than set_pte_at.


>
>> --- /dev/null
>> +++ b/arch/arm/mm/hugetlb.c
> ...
>> +pte_t *huge_pte_alloc(struct mm_struct *mm, unsigned long addr,
>> +		      unsigned long sz)
>> +{
>> +	pte_t *linuxpte = mm->context.huge_linux_pte;
>> +	int index;
>> +
>> +	if (linuxpte == NULL) {
>> +		linuxpte = kzalloc(HUGE_LINUX_PTE_SIZE, GFP_ATOMIC);
>> +		if (linuxpte == NULL) {
>> +			printk(KERN_ERR "Cannot allocate memory for huge linux pte\n");
>
> pr_err()?
Yes, pr_err should be used in here.
thanks


>
>> +			return NULL;
>> +		}
>> +		mm->context.huge_linux_pte = linuxpte;
>> +	}
>> +	/* huge page mapping only cover user space address */
>> +	BUG_ON(HUGE_LINUX_PTE_INDEX(addr)>= HUGE_LINUX_PTE_COUNT);
>> +	index = HUGE_LINUX_PTE_INDEX(addr);
>> +	return&linuxpte[HUGE_LINUX_PTE_INDEX(addr)];
>> +}
>> +
>> +pte_t *huge_pte_offset(struct mm_struct *mm, unsigned long addr)
>> +{
>> +	pgd_t *pgd;
>> +	pud_t *pud;
>> +	pmd_t *pmd = NULL;
>> +	pte_t *linuxpte = mm->context.huge_linux_pte;
>> +
>> +	/* check this mapping exist at pmd level */
>> +	pgd = pgd_offset(mm, addr);
>> +	if (pgd_present(*pgd)) {
>> +		pud = pud_offset(pgd, addr);
>> +		pmd = pmd_offset(pud, addr);
>> +		if (!pmd_present(*pmd))
>> +			return NULL;
>> +	}
>
> You could add checks for the pud as well, they would be optimised out by
> the compiler but it would be easier to add support for LPAE as well. In
> my LPAE hugetlb implementation, I have something like this:
>
> 	pgd = pgd_offset(mm, addr);
> 	if (pgd_present(*pgd)) {
> 		pud = pud_offset(pgd, addr);
> 		if (pud_present(*pud))
> 			pmd = pmd_offset(pud, addr);
> 	}
>
Ok, I will add the pud checks as per your comment.



>> +	BUG_ON(HUGE_LINUX_PTE_INDEX(addr)>= HUGE_LINUX_PTE_COUNT);
>> +	BUG_ON((*pmd&  PMD_TYPE_MASK) != PMD_TYPE_SECT);
>> +	return&linuxpte[HUGE_LINUX_PTE_INDEX(addr)];
>
> You could add a macro to make it easier for LPAE:
>
> #define huge_pte(mm, pmd, addr)		\
> 	(&mm->context.huge_linux_pte(HUGE_LINUX_PTE_INDEX(addr)))
>
Nice, I will keep this in V3.


> With LPAE, it would simply be a (pte_t *)pmd cast.
>
>> +int pud_huge(pud_t pud)
>> +{
>> +	return  0; } struct page * follow_huge_pmd(struct mm_struct *mm, unsigned long address, pmd_t *pmd, int write)
>
> Something went wrong around here.
>
crap! I will make it cleaner next time. I promise!


>> +{
>> +	struct page *page = NULL;
>
> You don't need to initialise page here.
OK, I will drop the "NULL".

>
>> +	unsigned long pfn;
>> +
>> +	BUG_ON((pmd_val(*pmd)&  PMD_TYPE_MASK) != PMD_TYPE_SECT);
>> +	pfn = ((pmd_val(*pmd)&  HPAGE_MASK)>>  PAGE_SHIFT);
>> +	page = pfn_to_page(pfn);
>> +	return page;
>> +}
>> +
>> +static int __init add_huge_page_size(unsigned long long size)
>> +{
>> +	int shift = __ffs(size);
>> +	u32 mmfr3 = 0;
>> +
>> +	/* Check that it is a page size supported by the hardware and
>> +	 * that it fits within pagetable and slice limits. */
>> +	if (!is_power_of_2(size) || (shift != HPAGE_SHIFT))
>> +		return -EINVAL;
>
> You could use get_order() instead of __ffs(), the latter just finds the
> first bit set.
With all due respect, I'm afraid I can't agree with you on this.
here, we should use __ffs to return this "shift" not the order.

For "hugepagesz=2M", hpage_shift/HPAGE_SHIFT should be set to 21,
*not* the order 9(21-12), that's what HUGETLB_PAGE_ORDER for.


>
> But here you should have set hpage_shift.
>

-- 
I am a slow learner
but I will keep trying to fight for my dreams!

--bill

[PATCH 1/7] Add various hugetlb arm high level hooks

[Catalin Marinas]

On Wed, Feb 29, 2012 at 11:28:30AM +0000, bill4carson wrote:
> On 2012?02?29? 18:32, Catalin Marinas wrote:
> > On Mon, Feb 13, 2012 at 09:44:22AM +0000, Bill Carson wrote:
> >> +static int __init add_huge_page_size(unsigned long long size)
> >> +{
> >> +	int shift = __ffs(size);
> >> +	u32 mmfr3 = 0;
> >> +
> >> +	/* Check that it is a page size supported by the hardware and
> >> +	 * that it fits within pagetable and slice limits. */
> >> +	if (!is_power_of_2(size) || (shift != HPAGE_SHIFT))
> >> +		return -EINVAL;
> >
> > You could use get_order() instead of __ffs(), the latter just finds the
> > first bit set.
> 
> With all due respect, I'm afraid I can't agree with you on this.
> here, we should use __ffs to return this "shift" not the order.
> 
> For "hugepagesz=2M", hpage_shift/HPAGE_SHIFT should be set to 21,
> *not* the order 9(21-12), that's what HUGETLB_PAGE_ORDER for.

I agree (I got confused by get_order() and log2()).

-- 
Catalin

[PATCH 1/7] Add various hugetlb arm high level hooks

[Catalin Marinas]

On Wed, Feb 29, 2012 at 11:28:30AM +0000, bill4carson wrote:
> On 2012?02?29? 18:32, Catalin Marinas wrote:
> > On Mon, Feb 13, 2012 at 09:44:22AM +0000, Bill Carson wrote:
> >> +static inline void huge_ptep_set_wrprotect(struct mm_struct *mm,
> >> +					   unsigned long addr, pte_t *ptep)
> >> +{
> >> +	pte_t old_pte = *ptep;
> >> +	set_huge_pte_at(mm, addr, ptep, pte_wrprotect(old_pte));
> >> +}
> >
> > You could use the generic ptep_set_wrprotect()
> 
> I'm a bit of confused about this.
> 
> generic ptep_set_wrprotect() can not set huge pte, that's why
> set_huge_pte_at is used instead here.

Ah, the generic one can only work with with LPAE where set_huge_pte_at()
is just a set_pte_at(). So this part looks good.

-- 
Catalin

[PATCH 1/7] Add various hugetlb arm high level hooks

[bill4carson]

On 2012?02?29? 23:38, Catalin Marinas wrote:
> On Wed, Feb 29, 2012 at 11:28:30AM +0000, bill4carson wrote:
>> On 2012?02?29? 18:32, Catalin Marinas wrote:
>>> On Mon, Feb 13, 2012 at 09:44:22AM +0000, Bill Carson wrote:
>>>> +static inline void huge_ptep_set_wrprotect(struct mm_struct *mm,
>>>> +					   unsigned long addr, pte_t *ptep)
>>>> +{
>>>> +	pte_t old_pte = *ptep;
>>>> +	set_huge_pte_at(mm, addr, ptep, pte_wrprotect(old_pte));
>>>> +}
>>>
>>> You could use the generic ptep_set_wrprotect()
>>
>> I'm a bit of confused about this.
>>
>> generic ptep_set_wrprotect() can not set huge pte, that's why
>> set_huge_pte_at is used instead here.
>
> Ah, the generic one can only work with with LPAE where set_huge_pte_at()
> is just a set_pte_at(). So this part looks good.
>
Hi, Catalin

Thanks for your time to review PATCH1/7 :)

I'm wondering if reset of this patch set is ok,
I would like to start working on V3 as per your comments.



-- 
I am a slow learner
but I will keep trying to fight for my dreams!

--bill

[PATCH 1/7] Add various hugetlb arm high level hooks

[Catalin Marinas]

On Thu, Mar 08, 2012 at 12:35:36AM +0000, bill4carson wrote:
> On 2012?02?29? 23:38, Catalin Marinas wrote:
> > On Wed, Feb 29, 2012 at 11:28:30AM +0000, bill4carson wrote:
> >> On 2012?02?29? 18:32, Catalin Marinas wrote:
> >>> On Mon, Feb 13, 2012 at 09:44:22AM +0000, Bill Carson wrote:
> >>>> +static inline void huge_ptep_set_wrprotect(struct mm_struct *mm,
> >>>> +					   unsigned long addr, pte_t *ptep)
> >>>> +{
> >>>> +	pte_t old_pte = *ptep;
> >>>> +	set_huge_pte_at(mm, addr, ptep, pte_wrprotect(old_pte));
> >>>> +}
> >>>
> >>> You could use the generic ptep_set_wrprotect()
> >>
> >> I'm a bit of confused about this.
> >>
> >> generic ptep_set_wrprotect() can not set huge pte, that's why
> >> set_huge_pte_at is used instead here.
> >
> > Ah, the generic one can only work with with LPAE where set_huge_pte_at()
> > is just a set_pte_at(). So this part looks good.
> 
> Thanks for your time to review PATCH1/7 :)
> 
> I'm wondering if reset of this patch set is ok,
> I would like to start working on V3 as per your comments.

I haven't finished looking through the patches. Probably I will by the
end of this week.

-- 
Catalin

[PATCH 1/7] Add various hugetlb arm high level hooks

[Arnd Bergmann]

On Monday 13 February 2012, Bill Carson wrote:
> +
> +/* 2M and 16M hugepage linux ptes are stored in mmu_context_t->huge_linux_pte
> + *
> + * 2M hugepage
> + * ===========
> + * one huge linux pte caters to two HW ptes,
> + *

I think this needs more explanation. Why do you use a 2MB hugepage with two ptes here?
Wouldn't it be more logical to use 1MB hugepages?

	Arnd

[PATCH 1/7] Add various hugetlb arm high level hooks

[Catalin Marinas]

On Wed, Feb 29, 2012 at 12:31:51PM +0000, Arnd Bergmann wrote:
> On Monday 13 February 2012, Bill Carson wrote:
> > +
> > +/* 2M and 16M hugepage linux ptes are stored in mmu_context_t->huge_linux_pte
> > + *
> > + * 2M hugepage
> > + * ===========
> > + * one huge linux pte caters to two HW ptes,
> > + *
> 
> I think this needs more explanation. Why do you use a 2MB hugepage with two ptes here?
> Wouldn't it be more logical to use 1MB hugepages?

That's because how pgd_t is defined for the classic MMU (PMD_SHIFT also
being 21). I think Bill explained in the first series of the patch why
using 1MB for huge pages doesn't go that well. But I agree, the
explanation should go in here as well.

-- 
Catalin

[PATCH 2/7] Add various hugetlb page table fix

[Bill Carson]

Signed-off-by: Bill Carson <bill4carson@gmail.com>
---
 arch/arm/include/asm/pgtable-2level.h |    3 +++
 arch/arm/include/asm/pgtable.h        |    9 +++++++++
 2 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/arch/arm/include/asm/pgtable-2level.h b/arch/arm/include/asm/pgtable-2level.h
index 2317a71..031c50e 100644
--- a/arch/arm/include/asm/pgtable-2level.h
+++ b/arch/arm/include/asm/pgtable-2level.h
@@ -178,6 +178,9 @@ static inline pmd_t *pmd_offset(pud_t *pud, unsigned long addr)
 #define pmd_addr_end(addr,end) (end)
 
 #define set_pte_ext(ptep,pte,ext) cpu_set_pte_ext(ptep,pte,ext)
+#ifdef CONFIG_ARM_HUGETLB_SUPPORT
+#define set_hugepte_ext(ptep,pte,ext) cpu_set_hugepte_ext(ptep,pte,ext)
+#endif
 
 #endif /* __ASSEMBLY__ */
 
diff --git a/arch/arm/include/asm/pgtable.h b/arch/arm/include/asm/pgtable.h
index f66626d..a119ac6 100644
--- a/arch/arm/include/asm/pgtable.h
+++ b/arch/arm/include/asm/pgtable.h
@@ -213,6 +213,14 @@ static inline void set_pte_at(struct mm_struct *mm, unsigned long addr,
 		set_pte_ext(ptep, pteval, PTE_EXT_NG);
 	}
 }
+#ifdef CONFIG_ARM_HUGETLB_SUPPORT
+static inline void set_hugepte_at(struct mm_struct *mm, unsigned long addr,
+			      pte_t *ptep, pte_t pteval)
+{
+	__sync_icache_dcache(pteval);
+	set_hugepte_ext(ptep, pteval, PTE_EXT_NG);
+}
+#endif
 
 #define pte_none(pte)		(!pte_val(pte))
 #define pte_present(pte)	(pte_val(pte) & L_PTE_PRESENT)
@@ -236,6 +244,7 @@ PTE_BIT_FUNC(mkdirty,   |= L_PTE_DIRTY);
 PTE_BIT_FUNC(mkold,     &= ~L_PTE_YOUNG);
 PTE_BIT_FUNC(mkyoung,   |= L_PTE_YOUNG);
 
+static inline pte_t pte_mkhuge(pte_t pte) { return pte; }
 static inline pte_t pte_mkspecial(pte_t pte) { return pte; }
 
 static inline pte_t pte_modify(pte_t pte, pgprot_t newprot)
-- 
1.7.1

[PATCH 2/7] Add various hugetlb page table fix

[Catalin Marinas]

On Mon, Feb 13, 2012 at 09:44:23AM +0000, Bill Carson wrote:
> --- a/arch/arm/include/asm/pgtable.h
> +++ b/arch/arm/include/asm/pgtable.h
> @@ -236,6 +244,7 @@ PTE_BIT_FUNC(mkdirty,   |= L_PTE_DIRTY);
>  PTE_BIT_FUNC(mkold,     &= ~L_PTE_YOUNG);
>  PTE_BIT_FUNC(mkyoung,   |= L_PTE_YOUNG);
>  
> +static inline pte_t pte_mkhuge(pte_t pte) { return pte; }

Do we need to set the PMD_TYPE_SECT here or you just do it via
set_huge_pte()?

-- 
Catalin

[PATCH 3/7] Introduce set_hugepte_ext api to setup huge hardware pmds

[Bill Carson]

Signed-off-by: Bill Carson <bill4carson@gmail.com>
---
 arch/arm/include/asm/glue-proc.h |    3 +
 arch/arm/include/asm/proc-fns.h  |    3 +
 arch/arm/mm/proc-v7-2level.S     |   87 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 93 insertions(+), 0 deletions(-)

diff --git a/arch/arm/include/asm/glue-proc.h b/arch/arm/include/asm/glue-proc.h
index e2be7f1..2bbd452 100644
--- a/arch/arm/include/asm/glue-proc.h
+++ b/arch/arm/include/asm/glue-proc.h
@@ -256,6 +256,9 @@
 #define cpu_dcache_clean_area		__glue(CPU_NAME,_dcache_clean_area)
 #define cpu_do_switch_mm		__glue(CPU_NAME,_switch_mm)
 #define cpu_set_pte_ext			__glue(CPU_NAME,_set_pte_ext)
+#ifdef CONFIG_ARM_HUGETLB_SUPPORT
+#define cpu_set_hugepte_ext		__glue(CPU_NAME,_set_hugepte_ext)
+#endif
 #define cpu_suspend_size		__glue(CPU_NAME,_suspend_size)
 #define cpu_do_suspend			__glue(CPU_NAME,_do_suspend)
 #define cpu_do_resume			__glue(CPU_NAME,_do_resume)
diff --git a/arch/arm/include/asm/proc-fns.h b/arch/arm/include/asm/proc-fns.h
index f3628fb..75bd755 100644
--- a/arch/arm/include/asm/proc-fns.h
+++ b/arch/arm/include/asm/proc-fns.h
@@ -87,6 +87,9 @@ extern void cpu_do_switch_mm(unsigned long pgd_phys, struct mm_struct *mm);
 extern void cpu_set_pte_ext(pte_t *ptep, pte_t pte);
 #else
 extern void cpu_set_pte_ext(pte_t *ptep, pte_t pte, unsigned int ext);
+#ifdef CONFIG_ARM_HUGETLB_SUPPORT
+extern void cpu_set_hugepte_ext(pte_t *ptep, pte_t pte, unsigned int ext);
+#endif
 #endif
 extern void cpu_reset(unsigned long addr) __attribute__((noreturn));
 
diff --git a/arch/arm/mm/proc-v7-2level.S b/arch/arm/mm/proc-v7-2level.S
index 3a4b3e7..4dc1554 100644
--- a/arch/arm/mm/proc-v7-2level.S
+++ b/arch/arm/mm/proc-v7-2level.S
@@ -111,6 +111,93 @@ ENTRY(cpu_v7_set_pte_ext)
 	mov	pc, lr
 ENDPROC(cpu_v7_set_pte_ext)
 
+#ifdef CONFIG_ARM_HUGETLB_SUPPORT
+ENTRY(cpu_v7_set_hugepte_ext)
+	@mask out AP[2:0] TEX[2:0] in first level section descriptor
+	bic	r3, r1, #0x0000fc00
+	@clear NX/IMP
+	bic	r3, r3,#0x210
+
+	@clear BIT1:0
+	bic	r3, r3, #PMD_TYPE_MASK
+
+	@set extension bit
+	orr	r3, r3, #PMD_SECT_nG @HUGEPAGE always non-global
+
+	@set SECT mapping,1M section or 16M supersection
+	orr	r3, r3, #PMD_SECT_AP_WRITE
+	orr	r3, r3, #PMD_TYPE_SECT
+
+	@BIT18 1: 16M supersection 0: 1M section
+	bic	r3, r3, #PMD_SECT_SUPER
+
+	@ shared bit
+	tst	r1,#L_PTE_SHARED
+	orrne	r3,r3,#PMD_SECT_S
+
+	@ shared device ?
+	tst	r1, #1 << 4
+	orrne	r3, r3, #PMD_SECT_TEX(1)
+
+	eor	r1, r1,  #L_PTE_DIRTY
+	tst	r1, #L_PTE_RDONLY | L_PTE_DIRTY
+	orrne	r3, r3, #PMD_SECT_APX
+
+	tst	r1, #L_PTE_USER
+	orrne	r3, r3, #PMD_SECT_AP_READ
+#ifdef CONFIG_CPU_USE_DOMAINS
+	tstne	r3, #PMD_SECT_APX
+	bicne	r3, r3, #PMD_SECT_APX | PMD_SECT_AP_WRITE
+#endif
+
+	@set domain
+	bic 	r3, r3, #(0xf << 5)
+	orr	r3, r3, #PMD_DOMAIN(0x1)
+
+#ifdef CONFIG_HUGEPAGE_SIZE_16MB
+	@ for supersection mapping
+	@ clear domain setting and extend addr
+	@ set BIT18 to denote supersection
+	ldr	r2, =((0xf << 5) | (0xf << 20))
+	bic	r3, r3, r2
+	orr	r3, r3, #PMD_SECT_SUPER
+#endif
+
+	tst	r1, #L_PTE_XN
+	orrne	r3, r3, #PMD_SECT_XN
+
+	tst	r1, #L_PTE_YOUNG
+	tstne	r1, #L_PTE_PRESENT
+	moveq	r3, #0
+
+#ifdef CONFIG_HUGEPAGE_SIZE_16MB
+	@ set 16M huge page
+	.rept 16
+ 	str r3, [r0]
+	mcr	p15, 0, r0, c7, c10, 1		@ flush_pte
+	add	r0, r0, #4
+	.endr
+#endif
+
+#ifdef CONFIG_HUGEPAGE_SIZE_2MB
+	@ set 2M huge page
+	@ 1st 1MB mapping
+ 	str r3, [r0]
+	mcr	p15, 0, r0, c7, c10, 1		@ flush_pte
+	@ 2st 1MB mapping
+	cmp	r3,#0
+	movne	r2,#0x100000
+	addne	r3, r3, r2
+
+	add	r0, r0, #4
+ 	str r3, [r0]
+	mcr	p15, 0, r0, c7, c10, 1		@ flush_pte
+#endif
+	mov pc, lr
+#endif /*COFNIG_ARM_HUGETLB_SUPPORT*/
+
+ENDPROC(cpu_v7_set_hugepte_ext)
+
 	/*
 	 * Memory region attributes with SCTLR.TRE=1
 	 *
-- 
1.7.1

[PATCH 4/7] Store huge page linux pte in mmu_context_t

[Bill Carson]

Signed-off-by: Bill Carson <bill4carson@gmail.com>
---
 arch/arm/include/asm/mmu.h |    4 ++++
 arch/arm/mm/pgd.c          |   11 +++++++++++
 2 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/arch/arm/include/asm/mmu.h b/arch/arm/include/asm/mmu.h
index 1496565..579bf82 100644
--- a/arch/arm/include/asm/mmu.h
+++ b/arch/arm/include/asm/mmu.h
@@ -9,6 +9,10 @@ typedef struct {
 	raw_spinlock_t id_lock;
 #endif
 	unsigned int kvm_seq;
+#ifdef CONFIG_ARM_HUGETLB_SUPPORT
+	/* we place hugepage linux pte at mm_struct.context  */
+	pte_t *huge_linux_pte;
+#endif
 } mm_context_t;
 
 #ifdef CONFIG_CPU_HAS_ASID
diff --git a/arch/arm/mm/pgd.c b/arch/arm/mm/pgd.c
index a3e78cc..895bb3f 100644
--- a/arch/arm/mm/pgd.c
+++ b/arch/arm/mm/pgd.c
@@ -91,6 +91,12 @@ pgd_t *pgd_alloc(struct mm_struct *mm)
 		pte_unmap(new_pte);
 	}
 
+#ifdef CONFIG_ARM_HUGETLB_SUPPORT
+	/* reset the hugepage linux pte pointer
+	 * for new mm_struct when we do the fork
+	 */
+	mm->context.huge_linux_pte = NULL;
+#endif
 	return new_pgd;
 
 no_pte:
@@ -135,6 +141,11 @@ no_pud:
 	pgd_clear(pgd);
 	pud_free(mm, pud);
 no_pgd:
+#ifdef CONFIG_ARM_HUGETLB_SUPPORT
+	/* free huge linux pte table */
+	if (mm->context.huge_linux_pte != NULL)
+		kfree(mm->context.huge_linux_pte);
+#endif
 #ifdef CONFIG_ARM_LPAE
 	/*
 	 * Free modules/pkmap or identity pmd tables.
-- 
1.7.1

[PATCH 5/7] Using do_page_fault for section fault handling

[Bill Carson]

Signed-off-by: Bill Carson <bill4carson@gmail.com>
---
 arch/arm/mm/fault.c |   15 +++++++++++++++
 1 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mm/fault.c b/arch/arm/mm/fault.c
index bb7eac3..af6703d 100644
--- a/arch/arm/mm/fault.c
+++ b/arch/arm/mm/fault.c
@@ -19,6 +19,7 @@
 #include <linux/sched.h>
 #include <linux/highmem.h>
 #include <linux/perf_event.h>
+#include <linux/hugetlb.h>
 
 #include <asm/exception.h>
 #include <asm/system.h>
@@ -485,6 +486,7 @@ do_translation_fault(unsigned long addr, unsigned int fsr,
 }
 #endif					/* CONFIG_MMU */
 
+#ifndef CONFIG_ARM_HUGETLB_SUPPORT
 /*
  * Some section permission faults need to be handled gracefully.
  * They can happen due to a __{get,put}_user during an oops.
@@ -496,6 +498,19 @@ do_sect_fault(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
 	return 0;
 }
 
+#else
+
+/* Since normal 4K page based vma will never fault into section traps,
+ * This will enable us to use do_page_fault for section permission fault.
+ */
+static int
+do_sect_fault(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
+{
+	do_page_fault(addr, fsr, regs);
+	return 0;
+}
+#endif
+
 /*
  * This abort handler always returns "fault".
  */
-- 
1.7.1

[PATCH 6/7] Add hugetlb Kconfig option

[Bill Carson]

Signed-off-by: Bill Carson <bill4carson@gmail.com>
---
 arch/arm/Kconfig |   29 +++++++++++++++++++++++++++++
 1 files changed, 29 insertions(+), 0 deletions(-)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index a48aecc..161bca6 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -985,6 +985,35 @@ config ARCH_ZYNQ
 	  Support for Xilinx Zynq ARM Cortex A9 Platform
 endchoice
 
+config SYS_SUPPORTS_HUGETLBFS
+		def_bool n
+
+config ARM_HUGETLB_SUPPORT
+	bool "Support HUGETLB for ARMv7 (EXPERIMENTAL)"
+	depends on CPU_V7 && EXPERIMENTAL
+	select SYS_SUPPORTS_HUGETLBFS
+	select HUGETLBFS
+	default y
+
+choice
+	prompt "Huge Page Size"
+	depends on ARM_HUGETLB_SUPPORT
+	default HUGEPAGE_SIZE_2MB
+
+config HUGEPAGE_SIZE_2MB
+	bool "2MB"
+	depends on ARM_HUGETLB_SUPPORT
+	help
+	 This option select huge page size in 2MB units
+
+config HUGEPAGE_SIZE_16MB
+	bool "16MB"
+	depends on ARM_HUGETLB_SUPPORT
+	help
+	  This option select huge page size in 16MB units
+
+endchoice
+
 #
 # This is sorted alphabetically by mach-* pathname.  However, plat-*
 # Kconfigs may be included either alphabetically (according to the
-- 
1.7.1

[PATCH 6/7] Add hugetlb Kconfig option

[Sergei Shtylyov]

Hello.

On 13-02-2012 13:44, Bill Carson wrote:

> Signed-off-by: Bill Carson<bill4carson@gmail.com>
> ---
>   arch/arm/Kconfig |   29 +++++++++++++++++++++++++++++
>   1 files changed, 29 insertions(+), 0 deletions(-)

> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index a48aecc..161bca6 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -985,6 +985,35 @@ config ARCH_ZYNQ
>   	  Support for Xilinx Zynq ARM Cortex A9 Platform
>   endchoice
>
> +config SYS_SUPPORTS_HUGETLBFS
> +		def_bool n

    This is probably overindented.

WBR, Sergei

[PATCH 6/7] Add hugetlb Kconfig option

[bill4carson]

On 2012?02?13? 19:00, Sergei Shtylyov wrote:
> Hello.
>
> On 13-02-2012 13:44, Bill Carson wrote:
>
>> Signed-off-by: Bill Carson<bill4carson@gmail.com>
>> ---
>> arch/arm/Kconfig | 29 +++++++++++++++++++++++++++++
>> 1 files changed, 29 insertions(+), 0 deletions(-)
>
>> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
>> index a48aecc..161bca6 100644
>> --- a/arch/arm/Kconfig
>> +++ b/arch/arm/Kconfig
>> @@ -985,6 +985,35 @@ config ARCH_ZYNQ
>> Support for Xilinx Zynq ARM Cortex A9 Platform
>> endchoice
>>
>> +config SYS_SUPPORTS_HUGETLBFS
>> + def_bool n
>
> This is probably overindented.
>
Hi, Sergei

Thanks for your point :)

I'm sure there are other coding style issues like this, I'll pay
attention to it in the next review round.




> WBR, Sergei
>

-- 
I am a slow learner
but I will keep trying to fight for my dreams!

--bill

[PATCH 7/7] Minor compiling fix

[Bill Carson]

Signed-off-by: Bill Carson <bill4carson@gmail.com>
---
 arch/arm/mm/Makefile      |    1 +
 arch/arm/mm/dma-mapping.c |    3 ---
 2 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mm/Makefile b/arch/arm/mm/Makefile
index bca7e61..9348e1e 100644
--- a/arch/arm/mm/Makefile
+++ b/arch/arm/mm/Makefile
@@ -99,4 +99,5 @@ AFLAGS_proc-v7.o	:=-Wa,-march=armv7-a
 obj-$(CONFIG_CACHE_FEROCEON_L2)	+= cache-feroceon-l2.o
 obj-$(CONFIG_CACHE_L2X0)	+= cache-l2x0.o
 obj-$(CONFIG_CACHE_XSC3L2)	+= cache-xsc3l2.o
+obj-$(CONFIG_HUGETLB_PAGE)	+= hugetlb.o
 obj-$(CONFIG_CACHE_TAUROS2)	+= cache-tauros2.o
diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
index 1aa664a..8dc5fb4 100644
--- a/arch/arm/mm/dma-mapping.c
+++ b/arch/arm/mm/dma-mapping.c
@@ -152,9 +152,6 @@ static struct arm_vmregion_head consistent_head = {
 	.vm_end		= CONSISTENT_END,
 };
 
-#ifdef CONFIG_HUGETLB_PAGE
-#error ARM Coherent DMA allocator does not (yet) support huge TLB
-#endif
 
 /*
  * Initialise the consistent memory allocation.
-- 
1.7.1

[PATCH 7/7] Minor compiling fix

[Arnd Bergmann]

On Monday 13 February 2012, Bill Carson wrote:
> --- a/arch/arm/mm/dma-mapping.c
> +++ b/arch/arm/mm/dma-mapping.c
> @@ -152,9 +152,6 @@ static struct arm_vmregion_head consistent_head = {
>         .vm_end         = CONSISTENT_END,
>  };
>  
> -#ifdef CONFIG_HUGETLB_PAGE
> -#error ARM Coherent DMA allocator does not (yet) support huge TLB
> -#endif

This one needs an explanation, too. What has changed that now allows
dma_alloc_coherent to work with CONFIG_HUGETLB_PAGE set that did not
work before?

In general, please add a description with each patch you send, following
the instructions in Documentation/SubmittingPatches.

	Arnd

[PATCH 7/7] Minor compiling fix

[bill4carson]

On 2012?02?29? 20:35, Arnd Bergmann wrote:
> On Monday 13 February 2012, Bill Carson wrote:
>> --- a/arch/arm/mm/dma-mapping.c
>> +++ b/arch/arm/mm/dma-mapping.c
>> @@ -152,9 +152,6 @@ static struct arm_vmregion_head consistent_head = {
>>          .vm_end         = CONSISTENT_END,
>>   };
>>
>> -#ifdef CONFIG_HUGETLB_PAGE
>> -#error ARM Coherent DMA allocator does not (yet) support huge TLB
>> -#endif
>
> This one needs an explanation, too. What has changed that now allows
> dma_alloc_coherent to work with CONFIG_HUGETLB_PAGE set that did not
> work before?
>

AFAIK, __dma_alloc has the finest granularity at 4k page size even when
user requests 1M bytes memory.

but current hugetlb low level set_hugepte_at *ONLY*/*MUST* support 2M 
page mapping, so it can't be used to create huge dma mapping in a more
general way.

comments this error out doesn't mean that it could support huge page dma
mapping now with this patch, just shut up gcc when compiling.

Adding a single low level hook to create 1M page mapping can satisfy
the needs, but apparently that's out of hugetlb scope.
And it's on my TODO list :)



> In general, please add a description with each patch you send, following
> the instructions in Documentation/SubmittingPatches.
>

Thanks, I will follow the rules.

> 	Arnd
>

-- 
I am a slow learner
but I will keep trying to fight for my dreams!

--bill

[PATCH 7/7] Minor compiling fix

[Arnd Bergmann]

On Thursday 01 March 2012, bill4carson wrote:
> On 2012?02?29? 20:35, Arnd Bergmann wrote:
> > On Monday 13 February 2012, Bill Carson wrote:
> >> --- a/arch/arm/mm/dma-mapping.c
> >> +++ b/arch/arm/mm/dma-mapping.c
> >> @@ -152,9 +152,6 @@ static struct arm_vmregion_head consistent_head = {
> >>          .vm_end         = CONSISTENT_END,
> >>   };
> >>
> >> -#ifdef CONFIG_HUGETLB_PAGE
> >> -#error ARM Coherent DMA allocator does not (yet) support huge TLB
> >> -#endif
> >
> > This one needs an explanation, too. What has changed that now allows
> > dma_alloc_coherent to work with CONFIG_HUGETLB_PAGE set that did not
> > work before?
> >
> 
> AFAIK, __dma_alloc has the finest granularity at 4k page size even when
> user requests 1M bytes memory.
> 
> but current hugetlb low level set_hugepte_at *ONLY*/*MUST* support 2M 
> page mapping, so it can't be used to create huge dma mapping in a more
> general way.
> 
> comments this error out doesn't mean that it could support huge page dma
> mapping now with this patch, just shut up gcc when compiling.
> 
> Adding a single low level hook to create 1M page mapping can satisfy
> the needs, but apparently that's out of hugetlb scope.
> And it's on my TODO list :)

Hmm, I'm still not sure I'm following. The coherent DMA area is currently
limited to 2MB or a small multiple of that, so you would not be able
to allocate coherent hugepages anyway.

Further, I don't think there are any device drivers that call
dma_alloc_coherent and expect to map the resulting page into
user space, or that we even have an interface for that.

This entire chunk of code is also getting changed with the addition of CMA.

	Arnd

[PATCH 7/7] Minor compiling fix

[carson bill]

2012/3/1 Arnd Bergmann <arnd@arndb.de>
>
> On Thursday 01 March 2012, bill4carson wrote:
> > On 2012?02?29? 20:35, Arnd Bergmann wrote:
> > > On Monday 13 February 2012, Bill Carson wrote:
> > >> --- a/arch/arm/mm/dma-mapping.c
> > >> +++ b/arch/arm/mm/dma-mapping.c
> > >> @@ -152,9 +152,6 @@ static struct arm_vmregion_head consistent_head = {
> > >>          .vm_end         = CONSISTENT_END,
> > >>   };
> > >>
> > >> -#ifdef CONFIG_HUGETLB_PAGE
> > >> -#error ARM Coherent DMA allocator does not (yet) support huge TLB
> > >> -#endif
> > >
> > > This one needs an explanation, too. What has changed that now allows
> > > dma_alloc_coherent to work with CONFIG_HUGETLB_PAGE set that did not
> > > work before?
> > >
> >
> > AFAIK, __dma_alloc has the finest granularity at 4k page size even when
> > user requests 1M bytes memory.
> >
> > but current hugetlb low level set_hugepte_at *ONLY*/*MUST* support 2M
> > page mapping, so it can't be used to create huge dma mapping in a more
> > general way.
> >
> > comments this error out doesn't mean that it could support huge page dma
> > mapping now with this patch, just shut up gcc when compiling.
> >
> > Adding a single low level hook to create 1M page mapping can satisfy
> > the needs, but apparently that's out of hugetlb scope.
> > And it's on my TODO list :)
>
> Hmm, I'm still not sure I'm following. The coherent DMA area is currently
> limited to 2MB or a small multiple of that, so you would not be able
> to allocate coherent hugepages anyway.
>
>
> Further, I don't think there are any device drivers that call
> dma_alloc_coherent and expect to map the resulting page into
> user space, or that we even have an interface for that.
>

This "ifdef" is used to remind dma implementation should use hugetlb low
API to set up huge page mappings one day hugetlb is supported, just it!

For now, I don't see any direct relationship dma alloc has to do with hugetlb
in a more general way other than both could share low level page table
management.


> This entire chunk of code is also getting changed with the addition of CMA.
>
>        Arnd

[RFC] ARM hugetlb support

[bill4carson at gmail.com]

Hi All


This patch aims to support huge page for ARM. For now, 2MB(two 1MB page)/16MB
huge page are supported, Versatile Express Cortex-A9x4 tile is used as test 
board. Verifications are running with libhugetlbfs and ltp.

Any suggestions would be welcome.


Signed-off-by: Bill Carson <bill4carson@gmail.com>
---
 arch/arm/Kconfig                      |   29 ++++
 arch/arm/include/asm/glue-proc.h      |    3 +
 arch/arm/include/asm/hugetlb.h        |  240 +++++++++++++++++++++++++++++++++
 arch/arm/include/asm/page.h           |   15 ++
 arch/arm/include/asm/pgtable-2level.h |    8 +
 arch/arm/include/asm/pgtable.h        |   28 ++++
 arch/arm/include/asm/proc-fns.h       |    3 +
 arch/arm/mm/Makefile                  |    1 +
 arch/arm/mm/dma-mapping.c             |    3 -
 arch/arm/mm/fault.c                   |   15 ++
 arch/arm/mm/hugetlb.c                 |  187 +++++++++++++++++++++++++
 arch/arm/mm/pgd.c                     |   28 ++++
 arch/arm/mm/proc-v7-2level.S          |   96 +++++++++++++
 include/linux/mm_types.h              |   11 ++
 14 files changed, 664 insertions(+), 3 deletions(-)
 create mode 100644 arch/arm/include/asm/hugetlb.h
 create mode 100644 arch/arm/mm/hugetlb.c

[PATCH 2/7] Add various hugetlb page table fix

[bill4carson at gmail.com]

From: Bill Carson <bill4carson@gmail.com>

    - Add L_PTE_huge page to mark huge page
    - Modify pte_pfn for hugetlb
    - Add set_hugepte_at for hugetlb arm high levle hooks use

Signed-off-by: Bill Carson <bill4carson@gmail.com>
---
 arch/arm/include/asm/pgtable-2level.h |    8 ++++++++
 arch/arm/include/asm/pgtable.h        |   28 ++++++++++++++++++++++++++++
 2 files changed, 36 insertions(+), 0 deletions(-)

diff --git a/arch/arm/include/asm/pgtable-2level.h b/arch/arm/include/asm/pgtable-2level.h
index 2317a71..062c93c 100644
--- a/arch/arm/include/asm/pgtable-2level.h
+++ b/arch/arm/include/asm/pgtable-2level.h
@@ -123,6 +123,11 @@
 #define L_PTE_USER		(_AT(pteval_t, 1) << 8)
 #define L_PTE_XN		(_AT(pteval_t, 1) << 9)
 #define L_PTE_SHARED		(_AT(pteval_t, 1) << 10)	/* shared(v6), coherent(xsc3) */
+#ifdef CONFIG_ARM_HUGETLB_SUPPORT
+#define L_PTE_HUGEPAGE	(_AT(pteval_t, 1) << 11) /* mark hugepage */
+#define L_PTE_HPAGE_2M  (_AT(pteval_t, 1) << 12) /* only when HUGEPAGE set */
+#define L_PTE_HPAGE_16M (_AT(pteval_t, 1) << 13) /* only when HUGEPAGE set */
+#endif
 
 /*
  * These are the memory types, defined to be compatible with
@@ -178,6 +183,9 @@ static inline pmd_t *pmd_offset(pud_t *pud, unsigned long addr)
 #define pmd_addr_end(addr,end) (end)
 
 #define set_pte_ext(ptep,pte,ext) cpu_set_pte_ext(ptep,pte,ext)
+#ifdef CONFIG_ARM_HUGETLB_SUPPORT
+#define set_hugepte_ext(ptep,pte,ext) cpu_set_hugepte_ext(ptep,pte,ext)
+#endif
 
 #endif /* __ASSEMBLY__ */
 
diff --git a/arch/arm/include/asm/pgtable.h b/arch/arm/include/asm/pgtable.h
index f66626d..da875d8 100644
--- a/arch/arm/include/asm/pgtable.h
+++ b/arch/arm/include/asm/pgtable.h
@@ -187,7 +187,21 @@ static inline pte_t *pmd_page_vaddr(pmd_t pmd)
 #define pte_offset_map(pmd,addr)	(__pte_map(pmd) + pte_index(addr))
 #define pte_unmap(pte)			__pte_unmap(pte)
 
+#ifdef CONFIG_ARM_HUGETLB_SUPPORT
+
+#ifdef CONFIG_HUGEPAGE_SIZE_2MB
+#define hugepte_pfn(pte)    ((pte_val(pte) & SECTION_MASK) >> PAGE_SHIFT)
+#endif
+#ifdef CONFIG_HUGEPAGE_SIZE_16MB
+#define hugepte_pfn(pte)    ((pte_val(pte) & SUPERSECTION_MASK) >> PAGE_SHIFT)
+#endif
+#define pte_is_huge(pte)	(pte_val(pte) & L_PTE_HUGEPAGE)
+#define pte_pfn(pte)	    (pte_is_huge(pte) ? \
+				hugepte_pfn(pte) : ((pte_val(pte) & PHYS_MASK) >> PAGE_SHIFT))
+#else
 #define pte_pfn(pte)		((pte_val(pte) & PHYS_MASK) >> PAGE_SHIFT)
+#endif /*!CONFIG_ARM_HUGETLB_SUPPORT*/
+
 #define pfn_pte(pfn,prot)	__pte(__pfn_to_phys(pfn) | pgprot_val(prot))
 
 #define pte_page(pte)		pfn_to_page(pte_pfn(pte))
@@ -213,6 +227,14 @@ static inline void set_pte_at(struct mm_struct *mm, unsigned long addr,
 		set_pte_ext(ptep, pteval, PTE_EXT_NG);
 	}
 }
+#ifdef CONFIG_ARM_HUGETLB_SUPPORT
+static inline void set_hugepte_at(struct mm_struct *mm, unsigned long addr,
+			      pte_t *ptep, pte_t pteval)
+{
+	__sync_icache_dcache(pteval);
+	set_hugepte_ext(ptep, pteval, PTE_EXT_NG);
+}
+#endif
 
 #define pte_none(pte)		(!pte_val(pte))
 #define pte_present(pte)	(pte_val(pte) & L_PTE_PRESENT)
@@ -235,6 +257,12 @@ PTE_BIT_FUNC(mkclean,   &= ~L_PTE_DIRTY);
 PTE_BIT_FUNC(mkdirty,   |= L_PTE_DIRTY);
 PTE_BIT_FUNC(mkold,     &= ~L_PTE_YOUNG);
 PTE_BIT_FUNC(mkyoung,   |= L_PTE_YOUNG);
+#ifdef CONFIG_HUGEPAGE_SIZE_2MB
+PTE_BIT_FUNC(mkhuge,    |= L_PTE_HUGEPAGE | L_PTE_HPAGE_2M);
+#endif
+#ifdef CONFIG_HUGEPAGE_SIZE_16MB
+PTE_BIT_FUNC(mkhuge,    |= L_PTE_HUGEPAGE | L_PTE_HPAGE_16M);
+#endif
 
 static inline pte_t pte_mkspecial(pte_t pte) { return pte; }
 
-- 
1.7.1

[PATCH 2/7] Add various hugetlb page table fix

[Catalin Marinas]

On 30 January 2012 07:57,  <bill4carson@gmail.com> wrote:
> diff --git a/arch/arm/include/asm/pgtable-2level.h b/arch/arm/include/asm/pgtable-2level.h
> index 2317a71..062c93c 100644
> --- a/arch/arm/include/asm/pgtable-2level.h
> +++ b/arch/arm/include/asm/pgtable-2level.h
> @@ -123,6 +123,11 @@
> ?#define L_PTE_USER ? ? ? ? ? ? (_AT(pteval_t, 1) << 8)
> ?#define L_PTE_XN ? ? ? ? ? ? ? (_AT(pteval_t, 1) << 9)
> ?#define L_PTE_SHARED ? ? ? ? ? (_AT(pteval_t, 1) << 10) ? ? ? ?/* shared(v6), coherent(xsc3) */
> +#ifdef CONFIG_ARM_HUGETLB_SUPPORT
> +#define L_PTE_HUGEPAGE (_AT(pteval_t, 1) << 11) /* mark hugepage */

Do we actually need this bit? The checks are done starting from the
top pgd/pud/pmd. So we stop at the pmd level and check whether it is
table or a section mapping. I don't think we ever check a pte entry
for whether it is huge or not (as,@least with the implementation
you posted, it does not support 64K pages).

> +#define L_PTE_HPAGE_2M ?(_AT(pteval_t, 1) << 12) /* only when HUGEPAGE set */
> +#define L_PTE_HPAGE_16M (_AT(pteval_t, 1) << 13) /* only when HUGEPAGE set */

If we go for 2MB only, we can ignore both definitions here.

-- 
Catalin

[PATCH 2/7] Add various hugetlb page table fix

[Russell King - ARM Linux]

On Mon, Jan 30, 2012 at 03:57:13PM +0800, bill4carson at gmail.com wrote:
> diff --git a/arch/arm/include/asm/pgtable-2level.h b/arch/arm/include/asm/pgtable-2level.h
> index 2317a71..062c93c 100644
> --- a/arch/arm/include/asm/pgtable-2level.h
> +++ b/arch/arm/include/asm/pgtable-2level.h
> @@ -123,6 +123,11 @@
>  #define L_PTE_USER		(_AT(pteval_t, 1) << 8)
>  #define L_PTE_XN		(_AT(pteval_t, 1) << 9)
>  #define L_PTE_SHARED		(_AT(pteval_t, 1) << 10)	/* shared(v6), coherent(xsc3) */
> +#ifdef CONFIG_ARM_HUGETLB_SUPPORT
> +#define L_PTE_HUGEPAGE	(_AT(pteval_t, 1) << 11) /* mark hugepage */
> +#define L_PTE_HPAGE_2M  (_AT(pteval_t, 1) << 12) /* only when HUGEPAGE set */
> +#define L_PTE_HPAGE_16M (_AT(pteval_t, 1) << 13) /* only when HUGEPAGE set */
> +#endif

(1) How does this work when normal pages can have bit 11 set if they're an
odd PFN?

(2) How do we even get to PTE level when a 2 or 16MB section doesn't have
a pte table (as the L1 entry is used for the section or supersection
mapping) ?

[PATCH 2/7] Add various hugetlb page table fix

[Catalin Marinas]

On 31 January 2012 09:58, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Mon, Jan 30, 2012 at 03:57:13PM +0800, bill4carson at gmail.com wrote:
>> diff --git a/arch/arm/include/asm/pgtable-2level.h b/arch/arm/include/asm/pgtable-2level.h
>> index 2317a71..062c93c 100644
>> --- a/arch/arm/include/asm/pgtable-2level.h
>> +++ b/arch/arm/include/asm/pgtable-2level.h
>> @@ -123,6 +123,11 @@
>> ?#define L_PTE_USER ? ? ? ? ? (_AT(pteval_t, 1) << 8)
>> ?#define L_PTE_XN ? ? ? ? ? ? (_AT(pteval_t, 1) << 9)
>> ?#define L_PTE_SHARED ? ? ? ? (_AT(pteval_t, 1) << 10) ? ? ? ?/* shared(v6), coherent(xsc3) */
>> +#ifdef CONFIG_ARM_HUGETLB_SUPPORT
>> +#define L_PTE_HUGEPAGE ? ? ? (_AT(pteval_t, 1) << 11) /* mark hugepage */
>> +#define L_PTE_HPAGE_2M ?(_AT(pteval_t, 1) << 12) /* only when HUGEPAGE set */
>> +#define L_PTE_HPAGE_16M (_AT(pteval_t, 1) << 13) /* only when HUGEPAGE set */
>> +#endif
>
> (1) How does this work when normal pages can have bit 11 set if they're an
> odd PFN?

Isn't that bit 12?

> (2) How do we even get to PTE level when a 2 or 16MB section doesn't have
> a pte table (as the L1 entry is used for the section or supersection
> mapping) ?

We don't, that's why I think we don't even need this bit defined.

-- 
Catalin

[PATCH 2/7] Add various hugetlb page table fix

[bill4carson]

On 2012?01?31? 20:25, Catalin Marinas wrote:
> On 31 January 2012 09:58, Russell King - ARM Linux
> <linux@arm.linux.org.uk>  wrote:
>> On Mon, Jan 30, 2012 at 03:57:13PM +0800, bill4carson at gmail.com wrote:
>>> diff --git a/arch/arm/include/asm/pgtable-2level.h b/arch/arm/include/asm/pgtable-2level.h
>>> index 2317a71..062c93c 100644
>>> --- a/arch/arm/include/asm/pgtable-2level.h
>>> +++ b/arch/arm/include/asm/pgtable-2level.h
>>> @@ -123,6 +123,11 @@
>>>   #define L_PTE_USER           (_AT(pteval_t, 1)<<  8)
>>>   #define L_PTE_XN             (_AT(pteval_t, 1)<<  9)
>>>   #define L_PTE_SHARED         (_AT(pteval_t, 1)<<  10)        /* shared(v6), coherent(xsc3) */
>>> +#ifdef CONFIG_ARM_HUGETLB_SUPPORT
>>> +#define L_PTE_HUGEPAGE       (_AT(pteval_t, 1)<<  11) /* mark hugepage */
>>> +#define L_PTE_HPAGE_2M  (_AT(pteval_t, 1)<<  12) /* only when HUGEPAGE set */
>>> +#define L_PTE_HPAGE_16M (_AT(pteval_t, 1)<<  13) /* only when HUGEPAGE set */
>>> +#endif
>> (1) How does this work when normal pages can have bit 11 set if they're an
>> odd PFN?
> Isn't that bit 12?
>
>> (2) How do we even get to PTE level when a 2 or 16MB section doesn't have
>> a pte table (as the L1 entry is used for the section or supersection
>> mapping) ?
> We don't, that's why I think we don't even need this bit defined.
>
First, thanks for Russel/Catalin to take time review this patch:)


a:
By pte used in here, I mean linux pte. linux pte is needed by generic mm 
layer
whether this mapping is 4K, 1MB or 16MB.


b:
Why L_PTE_HUGEPAGE is needed?

hugetlb subsystem will call pte_page to derive the corresponding page 
struct from
a given pte, and pte_pfn is used first to convert pte into a page frame 
number.
This is where need to be careful. Normal page based pte upper [31:12] 
bits is pfn,
huge page(1MB) based pte upper[31:20] is pfn, so one bit MUST 
distinguish normal
page based pte with huge page based pte.

This one bit will *only* be BIT11, one last unused PTE flag within 
normal page.
that's why L_PTE_HUGEPAGE is defined.

If L_PTE_HUGEPAGE is set, then BIT19:12 is enough to be used to mark 
this pte is 1MB
or 16MB huge page.



-- 
I am a slow learner
but I will keep trying to fight for my dreams!

--bill

[PATCH 2/7] Add various hugetlb page table fix

[Catalin Marinas]

On Wed, Feb 01, 2012 at 03:10:21AM +0000, bill4carson wrote:
> Why L_PTE_HUGEPAGE is needed?
> 
> hugetlb subsystem will call pte_page to derive the corresponding page 
> struct from a given pte, and pte_pfn is used first to convert pte into
> a page frame number.

Are you sure the pte_pfn() conversion is right? Does it need to be
different from the 4K pfn? I haven't seen any other architecture doing
shifts other than PAGE_SHIFT even for huge pages.

-- 
Catalin

[PATCH 2/7] Add various hugetlb page table fix

[bill4carson]

On 2012?02?07? 00:26, Catalin Marinas wrote:
> On Wed, Feb 01, 2012 at 03:10:21AM +0000, bill4carson wrote:
>> Why L_PTE_HUGEPAGE is needed?
>>
>> hugetlb subsystem will call pte_page to derive the corresponding page
>> struct from a given pte, and pte_pfn is used first to convert pte into
>> a page frame number.
>
> Are you sure the pte_pfn() conversion is right? Does it need to be
> different from the 4K pfn?

Hello, Catalin

Let me take a few words to make this clear for us.

pte_page is defined as following to derive page struct from a given pte.
This macro is used both in generic mm as well as hugetlb sub-system, so
we need do the switch in pte_pfn to mark huge page based linux pte out
of normal page based linux pte, that's what L_PTE_HUGEPAGE for.

#define pte_page(pte)		pfn_to_page(pte_pfn(pte))


So L_PTE_HUGEPAGE is *NOT* set in normal page based linux pte,
linux pte bits[31:12] is the page frame number;
otherwise, we got a huge page based linux pte, and linux pte
bits[31:20] is page frame number for SECTION mapping, and bits[31:24]
is page frame number for SUPER-SECTION mapping.

I think this the full story of following code:

#ifdef CONFIG_ARM_HUGETLB_SUPPORT

#ifdef CONFIG_HUGEPAGE_SIZE_2MB
#define hugepte_pfn(pte)    ((pte_val(pte) & SECTION_MASK) >> PAGE_SHIFT)
#endif
#ifdef CONFIG_HUGEPAGE_SIZE_16MB
#define hugepte_pfn(pte)    ((pte_val(pte) & SUPERSECTION_MASK) >> 
PAGE_SHIFT)
#endif
#define pte_is_huge(pte)	(pte_val(pte) & L_PTE_HUGEPAGE)
#define pte_pfn(pte)	    (pte_is_huge(pte) ? \
				hugepte_pfn(pte) : ((pte_val(pte) & PHYS_MASK) >> PAGE_SHIFT))
#else
#define pte_pfn(pte)		((pte_val(pte) & PHYS_MASK) >> PAGE_SHIFT)
#endif /*!CONFIG_ARM_HUGETLB_SUPPORT*/


#define pte_page(pte)		pfn_to_page(pte_pfn(pte))




> I haven't seen any other architecture doing
> shifts other than PAGE_SHIFT even for huge pages.
>

#define L_PTE_HUGEPAGE	(_AT(pteval_t, 1) << 11) /* mark hugepage */
#define L_PTE_HPAGE_2M  (_AT(pteval_t, 1) << 12) /* only when HUGEPAGE 
set */
#define L_PTE_HPAGE_16M (_AT(pteval_t, 1) << 13) /* only when HUGEPAGE 
set */

See linux pte BIT12 is used to denote 2MB huge page, BIT13 is used for
16MB page, that's why PAGE_SHIFT is not enough for do the shifting.

I hope I understand your question and give the right answer :)



-- 
I am a slow learner
but I will keep trying to fight for my dreams!

--bill

[PATCH 2/7] Add various hugetlb page table fix

[Catalin Marinas]

On Tue, Feb 07, 2012 at 01:42:01AM +0000, bill4carson wrote:
> On 2012?02?07? 00:26, Catalin Marinas wrote:
> > On Wed, Feb 01, 2012 at 03:10:21AM +0000, bill4carson wrote:
> >> Why L_PTE_HUGEPAGE is needed?
> >>
> >> hugetlb subsystem will call pte_page to derive the corresponding page
> >> struct from a given pte, and pte_pfn is used first to convert pte into
> >> a page frame number.
> >
> > Are you sure the pte_pfn() conversion is right? Does it need to be
> > different from the 4K pfn?
...
> pte_page is defined as following to derive page struct from a given pte.
> This macro is used both in generic mm as well as hugetlb sub-system, so
> we need do the switch in pte_pfn to mark huge page based linux pte out
> of normal page based linux pte, that's what L_PTE_HUGEPAGE for.
> 
> #define pte_page(pte)		pfn_to_page(pte_pfn(pte))
> 
> So L_PTE_HUGEPAGE is *NOT* set in normal page based linux pte,
> linux pte bits[31:12] is the page frame number;

I agree.

> otherwise, we got a huge page based linux pte, and linux pte
> bits[31:20] is page frame number for SECTION mapping, and bits[31:24]
> is page frame number for SUPER-SECTION mapping.

Actually it is still 31:12 but with bits 19:12 or 23:12 masked out. So
you do the correct shift by PAGE_SHIFT with the additional masking for
huge pages (harmless).

But do we actually need this masking? Do the huge_pte_offset() or
huge_pte_alloc() functions return the Linux pte (pmd) for the huge page?
If yes, can we not ensure that bits 19:12 are already zero? This
shouldn't be any different from the 4K Linux pte but with an address
aligned to 1MB.

-- 
Catalin

[PATCH 2/7] Add various hugetlb page table fix

[carson bill]

2012/2/7, Catalin Marinas <catalin.marinas@arm.com>:
> On Tue, Feb 07, 2012 at 01:42:01AM +0000, bill4carson wrote:
>> On 2012?02?07? 00:26, Catalin Marinas wrote:
>> > On Wed, Feb 01, 2012 at 03:10:21AM +0000, bill4carson wrote:
>> >> Why L_PTE_HUGEPAGE is needed?
>> >>
>> >> hugetlb subsystem will call pte_page to derive the corresponding page
>> >> struct from a given pte, and pte_pfn is used first to convert pte into
>> >> a page frame number.
>> >
>> > Are you sure the pte_pfn() conversion is right? Does it need to be
>> > different from the 4K pfn?
> ...
>> pte_page is defined as following to derive page struct from a given pte.
>> This macro is used both in generic mm as well as hugetlb sub-system, so
>> we need do the switch in pte_pfn to mark huge page based linux pte out
>> of normal page based linux pte, that's what L_PTE_HUGEPAGE for.
>>
>> #define pte_page(pte)		pfn_to_page(pte_pfn(pte))
>>
>> So L_PTE_HUGEPAGE is *NOT* set in normal page based linux pte,
>> linux pte bits[31:12] is the page frame number;
>
> I agree.
>
>> otherwise, we got a huge page based linux pte, and linux pte
>> bits[31:20] is page frame number for SECTION mapping, and bits[31:24]
>> is page frame number for SUPER-SECTION mapping.
>
> Actually it is still 31:12 but with bits 19:12 or 23:12 masked out. So
> you do the correct shift by PAGE_SHIFT with the additional masking for
> huge pages (harmless).
>
> But do we actually need this masking? Do the huge_pte_offset() or
> huge_pte_alloc() functions return the Linux pte (pmd) for the huge page?
> If yes, can we not ensure that bits 19:12 are already zero? This
> shouldn't be any different from the 4K Linux pte but with an address
> aligned to 1MB.
>

I'm afraid there is some misunderstanding.
huge_pte_offset() returns the huge linux pte address if they exist;
huge_pte_alloc()  allocates a location to store huge linux pte, and
return this address;
non of above functions return huge linux pte *value*.

make_huge_pte() will return huge linux pte for a given page and vma
protection bits,
please notice pte_mkhuge is used to mark this pte as huge linux pte by setting
L_PTE_HUGEPAGE, then set_huge_pte_at() is used to set huge linux pte as well
huge hardware pte.


2113static pte_t make_huge_pte(struct vm_area_struct *vma, struct page *page,
2114                                int writable)
2115{
2116        pte_t entry;
2117
2118        if (writable) {
2119                entry =
2120                    pte_mkwrite(pte_mkdirty(mk_pte(page,
vma->vm_page_prot)));
2121        } else {
2122                entry = huge_pte_wrprotect(mk_pte(page, vma->vm_page_prot));
2123        }
2124        entry = pte_mkyoung(entry);
2125        entry = pte_mkhuge(entry);
2126
2127        return entry;
2128}

Hence, normal linux pte must has L_PTE_HUGEPAE cleared;
A huge linux pte must has L_PTE_HUGEPAGE(BIT11) set
This could lead to L_PTE_HPAGE_2M(BIT12) or L_PTE_HPAGE_16M(BIT13) set
respectively, that's why the masking is needed for pte_pfn.


+#ifdef CONFIG_ARM_HUGETLB_SUPPORT
+#define L_PTE_HUGEPAGE	(_AT(pteval_t, 1) << 11) /* mark hugepage */
+#define L_PTE_HPAGE_2M  (_AT(pteval_t, 1) << 12) /* only when HUGEPAGE set */
+#define L_PTE_HPAGE_16M (_AT(pteval_t, 1) << 13) /* only when HUGEPAGE set */
+#endif


> --
> Catalin
>

[PATCH 2/7] Add various hugetlb page table fix

[Catalin Marinas]

On Tue, Feb 07, 2012 at 01:24:09PM +0000, carson bill wrote:
> 2012/2/7, Catalin Marinas <catalin.marinas@arm.com>:
> > On Tue, Feb 07, 2012 at 01:42:01AM +0000, bill4carson wrote:
> >> On 2012?02?07? 00:26, Catalin Marinas wrote:
> >> > On Wed, Feb 01, 2012 at 03:10:21AM +0000, bill4carson wrote:
> >> >> Why L_PTE_HUGEPAGE is needed?
> >> >>
> >> >> hugetlb subsystem will call pte_page to derive the corresponding page
> >> >> struct from a given pte, and pte_pfn is used first to convert pte into
> >> >> a page frame number.
> >> >
> >> > Are you sure the pte_pfn() conversion is right? Does it need to be
> >> > different from the 4K pfn?
> > ...
> >> pte_page is defined as following to derive page struct from a given pte.
> >> This macro is used both in generic mm as well as hugetlb sub-system, so
> >> we need do the switch in pte_pfn to mark huge page based linux pte out
> >> of normal page based linux pte, that's what L_PTE_HUGEPAGE for.
> >>
> >> #define pte_page(pte)		pfn_to_page(pte_pfn(pte))
> >>
> >> So L_PTE_HUGEPAGE is *NOT* set in normal page based linux pte,
> >> linux pte bits[31:12] is the page frame number;
> >
> > I agree.
> >
> >> otherwise, we got a huge page based linux pte, and linux pte
> >> bits[31:20] is page frame number for SECTION mapping, and bits[31:24]
> >> is page frame number for SUPER-SECTION mapping.
> >
> > Actually it is still 31:12 but with bits 19:12 or 23:12 masked out. So
> > you do the correct shift by PAGE_SHIFT with the additional masking for
> > huge pages (harmless).
> >
> > But do we actually need this masking? Do the huge_pte_offset() or
> > huge_pte_alloc() functions return the Linux pte (pmd) for the huge page?
> > If yes, can we not ensure that bits 19:12 are already zero? This
> > shouldn't be any different from the 4K Linux pte but with an address
> > aligned to 1MB.
> 
> I'm afraid there is some misunderstanding.
> huge_pte_offset() returns the huge linux pte address if they exist;
> huge_pte_alloc()  allocates a location to store huge linux pte, and
> return this address;
> non of above functions return huge linux pte *value*.

I agree, huge_pte_offset() returns a pointer to the Linux pte/pmd if it
exists. My point is that the values stored in Linux pte/pmd have bits
20:12 cleared already as the address is at least 2MB aligned (well,
apart from the additional L_PTE_HPAGE_* bits that you declared). Is this
correct? If yes, then you don't need any additional masking for
pte_pfn() even if it is passed a Linux pmd.

> make_huge_pte() will return huge linux pte for a given page and vma
> protection bits,
> please notice pte_mkhuge is used to mark this pte as huge linux pte by setting
> L_PTE_HUGEPAGE, then set_huge_pte_at() is used to set huge linux pte as well
> huge hardware pte.
> 
> 
> 2113static pte_t make_huge_pte(struct vm_area_struct *vma, struct page *page,
> 2114                                int writable)
> 2115{
> 2116        pte_t entry;
> 2117
> 2118        if (writable) {
> 2119                entry =
> 2120                    pte_mkwrite(pte_mkdirty(mk_pte(page,
> vma->vm_page_prot)));
> 2121        } else {
> 2122                entry = huge_pte_wrprotect(mk_pte(page, vma->vm_page_prot));
> 2123        }
> 2124        entry = pte_mkyoung(entry);
> 2125        entry = pte_mkhuge(entry);
> 2126
> 2127        return entry;
> 2128}
> 
> Hence, normal linux pte must has L_PTE_HUGEPAE cleared;
> A huge linux pte must has L_PTE_HUGEPAGE(BIT11) set
> This could lead to L_PTE_HPAGE_2M(BIT12) or L_PTE_HPAGE_16M(BIT13) set
> respectively, that's why the masking is needed for pte_pfn.

But if you avoid setting L_PTE_HPAGE_*, than we don't need the masking
for pte_pfn. In which case, we don't need to differentiate between a
normal and a huge pte in pte_pfn(), so no need for L_PTE_HUGEPAGE. The
set_huge_pte_at() function is only called with a huge pte, so it doesn't
need to check the L_PTE_HUGEPAGE bit either.

-- 
Catalin

[PATCH 2/7] Add various hugetlb page table fix

[carson bill]

2012/2/7, Catalin Marinas <catalin.marinas@arm.com>:
> On Tue, Feb 07, 2012 at 01:24:09PM +0000, carson bill wrote:
>> 2012/2/7, Catalin Marinas <catalin.marinas@arm.com>:
>> > On Tue, Feb 07, 2012 at 01:42:01AM +0000, bill4carson wrote:
>> >> On 2012?02?07? 00:26, Catalin Marinas wrote:
>> >> > On Wed, Feb 01, 2012 at 03:10:21AM +0000, bill4carson wrote:
>> >> >> Why L_PTE_HUGEPAGE is needed?
>> >> >>
>> >> >> hugetlb subsystem will call pte_page to derive the corresponding
>> >> >> page
>> >> >> struct from a given pte, and pte_pfn is used first to convert pte
>> >> >> into
>> >> >> a page frame number.
>> >> >
>> >> > Are you sure the pte_pfn() conversion is right? Does it need to be
>> >> > different from the 4K pfn?
>> > ...
>> >> pte_page is defined as following to derive page struct from a given
>> >> pte.
>> >> This macro is used both in generic mm as well as hugetlb sub-system, so
>> >> we need do the switch in pte_pfn to mark huge page based linux pte out
>> >> of normal page based linux pte, that's what L_PTE_HUGEPAGE for.
>> >>
>> >> #define pte_page(pte)		pfn_to_page(pte_pfn(pte))
>> >>
>> >> So L_PTE_HUGEPAGE is *NOT* set in normal page based linux pte,
>> >> linux pte bits[31:12] is the page frame number;
>> >
>> > I agree.
>> >
>> >> otherwise, we got a huge page based linux pte, and linux pte
>> >> bits[31:20] is page frame number for SECTION mapping, and bits[31:24]
>> >> is page frame number for SUPER-SECTION mapping.
>> >
>> > Actually it is still 31:12 but with bits 19:12 or 23:12 masked out. So
>> > you do the correct shift by PAGE_SHIFT with the additional masking for
>> > huge pages (harmless).
>> >
>> > But do we actually need this masking? Do the huge_pte_offset() or
>> > huge_pte_alloc() functions return the Linux pte (pmd) for the huge page?
>> > If yes, can we not ensure that bits 19:12 are already zero? This
>> > shouldn't be any different from the 4K Linux pte but with an address
>> > aligned to 1MB.
>>
>> I'm afraid there is some misunderstanding.
>> huge_pte_offset() returns the huge linux pte address if they exist;
>> huge_pte_alloc()  allocates a location to store huge linux pte, and
>> return this address;
>> non of above functions return huge linux pte *value*.
>
> I agree, huge_pte_offset() returns a pointer to the Linux pte/pmd if it
> exists. My point is that the values stored in Linux pte/pmd have bits
> 20:12 cleared already as the address is at least 2MB aligned (well,
> apart from the additional L_PTE_HPAGE_* bits that you declared). Is this
> correct? If yes, then you don't need any additional masking for
> pte_pfn() even if it is passed a Linux pmd.

Yes, pte_pfn doesn't need any modification if we don't need any L_PTE_HPAGE_*).


>
>> make_huge_pte() will return huge linux pte for a given page and vma
>> protection bits,
>> please notice pte_mkhuge is used to mark this pte as huge linux pte by
>> setting
>> L_PTE_HUGEPAGE, then set_huge_pte_at() is used to set huge linux pte as
>> well
>> huge hardware pte.
>>
>>
>> 2113static pte_t make_huge_pte(struct vm_area_struct *vma, struct page
>> *page,
>> 2114                                int writable)
>> 2115{
>> 2116        pte_t entry;
>> 2117
>> 2118        if (writable) {
>> 2119                entry =
>> 2120                    pte_mkwrite(pte_mkdirty(mk_pte(page,
>> vma->vm_page_prot)));
>> 2121        } else {
>> 2122                entry = huge_pte_wrprotect(mk_pte(page,
>> vma->vm_page_prot));
>> 2123        }
>> 2124        entry = pte_mkyoung(entry);
>> 2125        entry = pte_mkhuge(entry);
>> 2126
>> 2127        return entry;
>> 2128}
>>
>> Hence, normal linux pte must has L_PTE_HUGEPAE cleared;
>> A huge linux pte must has L_PTE_HUGEPAGE(BIT11) set
>> This could lead to L_PTE_HPAGE_2M(BIT12) or L_PTE_HPAGE_16M(BIT13) set
>> respectively, that's why the masking is needed for pte_pfn.
>
> But if you avoid setting L_PTE_HPAGE_*, than we don't need the masking
> for pte_pfn. In which case, we don't need to differentiate between a
> normal and a huge pte in pte_pfn(), so no need for L_PTE_HUGEPAGE. The
> set_huge_pte_at() function is only called with a huge pte, so it doesn't
> need to check the L_PTE_HUGEPAGE bit either.
>

I understood what you mean now, and the original design is almost like you said.
But the consequences of eliminating L_PTE_HUGEPAGE as well as L_PTE_HPAGE_*
only leave us with huge page size fixed at build time, I mean boot
time huge page
size configuration feature like X86 will NOT be feasible anymore!

looks like we have to made a choice now, what do you think? Catalin

> --
> Catalin
>

[PATCH 2/7] Add various hugetlb page table fix

[Catalin Marinas]

On Tue, Feb 07, 2012 at 02:46:50PM +0000, carson bill wrote:
> 2012/2/7, Catalin Marinas <catalin.marinas@arm.com>:
> > But if you avoid setting L_PTE_HPAGE_*, than we don't need the
> > masking for pte_pfn. In which case, we don't need to differentiate
> > between a normal and a huge pte in pte_pfn(), so no need for
> > L_PTE_HUGEPAGE. The set_huge_pte_at() function is only called with a
> > huge pte, so it doesn't need to check the L_PTE_HUGEPAGE bit either.
> 
> I understood what you mean now, and the original design is almost like
> you said.  But the consequences of eliminating L_PTE_HUGEPAGE as well
> as L_PTE_HPAGE_* only leave us with huge page size fixed at build
> time, I mean boot time huge page size configuration feature like X86
> will NOT be feasible anymore!

Yes it will :). Just store the page size in some variable that you check
at run-time. We won't support mixed huge page sizes though.

-- 
Catalin

[PATCH 2/7] Add various hugetlb page table fix

[carson bill]

2012/2/7, Catalin Marinas <catalin.marinas@arm.com>:
> On Tue, Feb 07, 2012 at 02:46:50PM +0000, carson bill wrote:
>> 2012/2/7, Catalin Marinas <catalin.marinas@arm.com>:
>> > But if you avoid setting L_PTE_HPAGE_*, than we don't need the
>> > masking for pte_pfn. In which case, we don't need to differentiate
>> > between a normal and a huge pte in pte_pfn(), so no need for
>> > L_PTE_HUGEPAGE. The set_huge_pte_at() function is only called with a
>> > huge pte, so it doesn't need to check the L_PTE_HUGEPAGE bit either.
>>
>> I understood what you mean now, and the original design is almost like
>> you said.  But the consequences of eliminating L_PTE_HUGEPAGE as well
>> as L_PTE_HPAGE_* only leave us with huge page size fixed at build
>> time, I mean boot time huge page size configuration feature like X86
>> will NOT be feasible anymore!
>
> Yes it will :). Just store the page size in some variable that you check
> at run-time. We won't support mixed huge page sizes though.
>

Got it:)
Please review the other parts of patch, let me know if any question arises.



> --
> Catalin
>