[PATCHv3 0/5] use __create_pgd_mapping() to implement idmap and unify codes

[PATCHv3 0/5] use __create_pgd_mapping() to implement idmap and unify codes

[Pingfan Liu]

v2 -> v3:
  -1. leave out the part of redefinition the CONFIG_PGTABLE_LEVEL,
concentrate on sharing __create_pgd_mapping() in head.S as the first
step.
  -2. make IDMAP_PGTABLE_LEVELS use the max value ([3/5])

rfc -> v2:
  more debug and test

*** Goal of this series ***

__create_pgd_mapping() sets up the pgtable for mapping __va(paddr) ->
paddr under the MMU-on situation.  Since pgtable upper level holds the
paddr of the lower level, with a slight adaptation,
__create_pgd_mapping() can also set up the mapping under the MMU-off
situation. ([4/5])

After that, both idmap_pg_dir and init_pg_dir can be created by
__create_pgd_mapping(). And the counterpart asm code can be simplified.

This series can be applied against commit 4284bdf9405a ("Linux 5.13-rc2").


*** Plan for the next ***

The omitted part in V2, which resorts to redefinition of
CONFIG_PGTABLE_LEVEL to provides two sets of routines. One set is proper
to set up a idmap which can adress total system RAM.  That can simplify
the asm code in head.S furtherly, also provide an create_idmap() API.


*** Test ***

This series can success booting with the following configuration on either Cavium
ThunderX2 99xx or Qualcomm Falkor:
            PAGE_SIZE  VA  PA  PGTABLE_LEVEL
Qualcomm    4K         48  48  4
            4K         39  48  3
            16K        48  48  4
            16K        47  48  3
Cavium      64K        52  52  3
            64K        48  52  3
            64K        42  52  2



*** History ***

RFC:
https://lore.kernel.org/linux-arm-kernel/20210410095654.24102-1-kernelfans@gmail.com/
V2:
https://lore.kernel.org/linux-arm-kernel/20210425141304.32721-1-kernelfans@gmail.com/


Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: Ard Biesheuvel <ardb@kernel.org>
Cc: Marc Zyngier <maz@kernel.org>
Cc: Kristina Martsenko <kristina.martsenko@arm.com>
Cc: James Morse <james.morse@arm.com>
Cc: Steven Price <steven.price@arm.com>
Cc: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Cc: Pavel Tatashin <pasha.tatashin@soleen.com>
Cc: Anshuman Khandual <anshuman.khandual@arm.com>
Cc: Atish Patra <atish.patra@wdc.com>
Cc: Mike Rapoport <rppt@kernel.org>
Cc: Logan Gunthorpe <logang@deltatee.com>
Cc: Mark Brown <broonie@kernel.org>
To: linux-arm-kernel@lists.infradead.org

Pingfan Liu (5):
  arm64/mm: introduce pgtable allocator for idmap_pg_dir and init_pg_dir
  arm64/mm: disable WARN_ON() and BUG_ON() in __create_pgd_mapping() if
    too early
  arm64/mm: unconditionally set IDMAP_PGTABLE_LEVELS to max pgtable
    level
  arm64/mm: make __create_pgd_mapping() capable to handle pgtable's
    paddr
  arm64/mm: use __create_pgd_mapping() to create pgtable for
    idmap_pg_dir and init_pg_dir

 arch/arm64/include/asm/kernel-pgtable.h |  33 +++--
 arch/arm64/include/asm/pgalloc.h        |   9 ++
 arch/arm64/kernel/head.S                | 164 +++++++-----------------
 arch/arm64/mm/mmu.c                     | 108 ++++++++++++----
 4 files changed, 153 insertions(+), 161 deletions(-)

-- 
2.29.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCHv3 1/5] arm64/mm: introduce pgtable allocator for idmap_pg_dir and init_pg_dir

[Pingfan Liu]

When building a pgtable, __create_pgd_mapping() resorts to pgtable
allocator to alloc mem.

By introducing an allocator, __create_pgd_mapping() can consider both
idmap_pg_dir and init_pg_dir as a memory pool, and get memory from them
during the building of pgtable.

Signed-off-by: Pingfan Liu <kernelfans@gmail.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: Ard Biesheuvel <ardb@kernel.org>
Cc: Marc Zyngier <maz@kernel.org>
Cc: Kristina Martsenko <kristina.martsenko@arm.com>
Cc: James Morse <james.morse@arm.com>
Cc: Steven Price <steven.price@arm.com>
Cc: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Cc: Pavel Tatashin <pasha.tatashin@soleen.com>
Cc: Anshuman Khandual <anshuman.khandual@arm.com>
Cc: Atish Patra <atish.patra@wdc.com>
Cc: Mike Rapoport <rppt@kernel.org>
Cc: Logan Gunthorpe <logang@deltatee.com>
Cc: Mark Brown <broonie@kernel.org>
To: linux-arm-kernel@lists.infradead.org
---
 arch/arm64/mm/mmu.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
index 6dd9369e3ea0..f4fc905718ca 100644
--- a/arch/arm64/mm/mmu.c
+++ b/arch/arm64/mm/mmu.c
@@ -90,6 +90,28 @@ pgprot_t phys_mem_access_prot(struct file *file, unsigned long pfn,
 }
 EXPORT_SYMBOL(phys_mem_access_prot);
 
+struct headpool {
+	phys_addr_t start;
+	unsigned long size;
+	unsigned long next_idx;
+};
+
+static struct headpool cur_pool __initdata;
+
+void __init set_cur_headpool(phys_addr_t start, unsigned long size)
+{
+	cur_pool.start = start;
+	cur_pool.size = size;
+	cur_pool.next_idx = 0;
+}
+
+phys_addr_t __init head_pgtable_alloc(unsigned long unused_a)
+{
+	unsigned long idx = cur_pool.next_idx++;
+
+	return cur_pool.start + (idx << PAGE_SHIFT);
+}
+
 static phys_addr_t __init early_pgtable_alloc(int shift)
 {
 	phys_addr_t phys;
-- 
2.29.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCHv3 2/5] arm64/mm: disable WARN_ON() and BUG_ON() in __create_pgd_mapping() if too early

[Pingfan Liu]

This patch is still one of the preparation for calling
__create_pgd_mapping() from head.S

When calling from head.S, printk() is not ready to work. Hence define
SAFE_BUG_ON()/SAFE_WARN_ON(), wrapping around BUG_ON()/WARN_ON() to
protect against early calling.

Signed-off-by: Pingfan Liu <kernelfans@gmail.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: Ard Biesheuvel <ardb@kernel.org>
Cc: Marc Zyngier <maz@kernel.org>
Cc: Kristina Martsenko <kristina.martsenko@arm.com>
Cc: James Morse <james.morse@arm.com>
Cc: Steven Price <steven.price@arm.com>
Cc: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Cc: Pavel Tatashin <pasha.tatashin@soleen.com>
Cc: Anshuman Khandual <anshuman.khandual@arm.com>
Cc: Atish Patra <atish.patra@wdc.com>
Cc: Mike Rapoport <rppt@kernel.org>
Cc: Logan Gunthorpe <logang@deltatee.com>
Cc: Mark Brown <broonie@kernel.org>
To: linux-arm-kernel@lists.infradead.org
---
 arch/arm64/mm/mmu.c | 42 ++++++++++++++++++++++++++----------------
 1 file changed, 26 insertions(+), 16 deletions(-)

diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
index f4fc905718ca..fe16f235d1fa 100644
--- a/arch/arm64/mm/mmu.c
+++ b/arch/arm64/mm/mmu.c
@@ -40,6 +40,7 @@
 #define NO_BLOCK_MAPPINGS	BIT(0)
 #define NO_CONT_MAPPINGS	BIT(1)
 #define NO_EXEC_MAPPINGS	BIT(2)	/* assumes FEAT_HPDS is not used */
+#define BOOT_HEAD		BIT(3)
 
 u64 idmap_t0sz = TCR_T0SZ(VA_BITS_MIN);
 u64 idmap_ptrs_per_pgd = PTRS_PER_PGD;
@@ -139,6 +140,15 @@ static phys_addr_t __init early_pgtable_alloc(int shift)
 	return phys;
 }
 
+/*
+ * printk is not ready in the very early stage. And this pair macro should be used
+ * instead
+ */
+#define SAFE_BUG_ON(flags, cond)	\
+	do { if (likely(!(flags & BOOT_HEAD))) BUG_ON(cond); } while (0)
+#define SAFE_WARN_ON(flags, cond)	\
+	({ (flags & BOOT_HEAD) ? false : WARN_ON(cond); })
+
 static bool pgattr_change_is_safe(u64 old, u64 new)
 {
 	/*
@@ -174,7 +184,7 @@ static bool pgattr_change_is_safe(u64 old, u64 new)
 }
 
 static void init_pte(pmd_t *pmdp, unsigned long addr, unsigned long end,
-		     phys_addr_t phys, pgprot_t prot)
+		     phys_addr_t phys, pgprot_t prot, int flags)
 {
 	pte_t *ptep;
 
@@ -188,7 +198,7 @@ static void init_pte(pmd_t *pmdp, unsigned long addr, unsigned long end,
 		 * After the PTE entry has been populated once, we
 		 * only allow updates to the permission attributes.
 		 */
-		BUG_ON(!pgattr_change_is_safe(pte_val(old_pte),
+		SAFE_BUG_ON(flags, !pgattr_change_is_safe(pte_val(old_pte),
 					      READ_ONCE(pte_val(*ptep))));
 
 		phys += PAGE_SIZE;
@@ -206,19 +216,19 @@ static void alloc_init_cont_pte(pmd_t *pmdp, unsigned long addr,
 	unsigned long next;
 	pmd_t pmd = READ_ONCE(*pmdp);
 
-	BUG_ON(pmd_sect(pmd));
+	SAFE_BUG_ON(flags, pmd_sect(pmd));
 	if (pmd_none(pmd)) {
 		pmdval_t pmdval = PMD_TYPE_TABLE | PMD_TABLE_UXN;
 		phys_addr_t pte_phys;
 
 		if (flags & NO_EXEC_MAPPINGS)
 			pmdval |= PMD_TABLE_PXN;
-		BUG_ON(!pgtable_alloc);
+		SAFE_BUG_ON(flags, !pgtable_alloc);
 		pte_phys = pgtable_alloc(PAGE_SHIFT);
 		__pmd_populate(pmdp, pte_phys, pmdval);
 		pmd = READ_ONCE(*pmdp);
 	}
-	BUG_ON(pmd_bad(pmd));
+	SAFE_BUG_ON(flags, pmd_bad(pmd));
 
 	do {
 		pgprot_t __prot = prot;
@@ -230,7 +240,7 @@ static void alloc_init_cont_pte(pmd_t *pmdp, unsigned long addr,
 		    (flags & NO_CONT_MAPPINGS) == 0)
 			__prot = __pgprot(pgprot_val(prot) | PTE_CONT);
 
-		init_pte(pmdp, addr, next, phys, __prot);
+		init_pte(pmdp, addr, next, phys, __prot, flags);
 
 		phys += next - addr;
 	} while (addr = next, addr != end);
@@ -258,13 +268,13 @@ static void init_pmd(pud_t *pudp, unsigned long addr, unsigned long end,
 			 * After the PMD entry has been populated once, we
 			 * only allow updates to the permission attributes.
 			 */
-			BUG_ON(!pgattr_change_is_safe(pmd_val(old_pmd),
+			SAFE_BUG_ON(flags, !pgattr_change_is_safe(pmd_val(old_pmd),
 						      READ_ONCE(pmd_val(*pmdp))));
 		} else {
 			alloc_init_cont_pte(pmdp, addr, next, phys, prot,
 					    pgtable_alloc, flags);
 
-			BUG_ON(pmd_val(old_pmd) != 0 &&
+			SAFE_BUG_ON(flags, pmd_val(old_pmd) != 0 &&
 			       pmd_val(old_pmd) != READ_ONCE(pmd_val(*pmdp)));
 		}
 		phys += next - addr;
@@ -284,19 +294,19 @@ static void alloc_init_cont_pmd(pud_t *pudp, unsigned long addr,
 	/*
 	 * Check for initial section mappings in the pgd/pud.
 	 */
-	BUG_ON(pud_sect(pud));
+	SAFE_BUG_ON(flags, pud_sect(pud));
 	if (pud_none(pud)) {
 		pudval_t pudval = PUD_TYPE_TABLE | PUD_TABLE_UXN;
 		phys_addr_t pmd_phys;
 
 		if (flags & NO_EXEC_MAPPINGS)
 			pudval |= PUD_TABLE_PXN;
-		BUG_ON(!pgtable_alloc);
+		SAFE_BUG_ON(flags, !pgtable_alloc);
 		pmd_phys = pgtable_alloc(PMD_SHIFT);
 		__pud_populate(pudp, pmd_phys, pudval);
 		pud = READ_ONCE(*pudp);
 	}
-	BUG_ON(pud_bad(pud));
+	SAFE_BUG_ON(flags, pud_bad(pud));
 
 	do {
 		pgprot_t __prot = prot;
@@ -342,12 +352,12 @@ static void alloc_init_pud(pgd_t *pgdp, unsigned long addr, unsigned long end,
 
 		if (flags & NO_EXEC_MAPPINGS)
 			p4dval |= P4D_TABLE_PXN;
-		BUG_ON(!pgtable_alloc);
+		SAFE_BUG_ON(flags, !pgtable_alloc);
 		pud_phys = pgtable_alloc(PUD_SHIFT);
 		__p4d_populate(p4dp, pud_phys, p4dval);
 		p4d = READ_ONCE(*p4dp);
 	}
-	BUG_ON(p4d_bad(p4d));
+	SAFE_BUG_ON(flags, p4d_bad(p4d));
 
 	pudp = pud_set_fixmap_offset(p4dp, addr);
 	do {
@@ -366,13 +376,13 @@ static void alloc_init_pud(pgd_t *pgdp, unsigned long addr, unsigned long end,
 			 * After the PUD entry has been populated once, we
 			 * only allow updates to the permission attributes.
 			 */
-			BUG_ON(!pgattr_change_is_safe(pud_val(old_pud),
+			SAFE_BUG_ON(flags, !pgattr_change_is_safe(pud_val(old_pud),
 						      READ_ONCE(pud_val(*pudp))));
 		} else {
 			alloc_init_cont_pmd(pudp, addr, next, phys, prot,
 					    pgtable_alloc, flags);
 
-			BUG_ON(pud_val(old_pud) != 0 &&
+			SAFE_BUG_ON(flags, pud_val(old_pud) != 0 &&
 			       pud_val(old_pud) != READ_ONCE(pud_val(*pudp)));
 		}
 		phys += next - addr;
@@ -394,7 +404,7 @@ static void __create_pgd_mapping(pgd_t *pgdir, phys_addr_t phys,
 	 * If the virtual and physical address don't have the same offset
 	 * within a page, we cannot map the region as the caller expects.
 	 */
-	if (WARN_ON((phys ^ virt) & ~PAGE_MASK))
+	if (SAFE_WARN_ON(flags, (phys ^ virt) & ~PAGE_MASK))
 		return;
 
 	phys &= PAGE_MASK;
-- 
2.29.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCHv3 3/5] arm64/mm: unconditionally set IDMAP_PGTABLE_LEVELS to max pgtable level

[Pingfan Liu]

After commit 5dfe9d7d23c2 ("arm64: reduce ID map to a single page"),
idmap only costs a page. While with 4K page, the current code enforces
section mapping (2MB) on it due to the prot SWAPPER_MM_MMUFLAGS.

But if switching to __create_pgd_mapping() to create pgtable, the code
will decide the proper mapping level by itself, as the code snippet in
init_pmd():
    if (((addr | next | phys) & ~SECTION_MASK) == 0 &&
    		    (flags & NO_BLOCK_MAPPINGS) == 0)
    	pmd_set_huge(pmdp, phys, prot);

As the case of .idmap.text, it requires pgtable up to pte level. Hence
IDMAP_PGTABLE_LEVELS should be large enough to assure the capacity of
IDMAP_DIR_SIZE.

Signed-off-by: Pingfan Liu <kernelfans@gmail.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: Ard Biesheuvel <ardb@kernel.org>
Cc: Marc Zyngier <maz@kernel.org>
Cc: Kristina Martsenko <kristina.martsenko@arm.com>
Cc: James Morse <james.morse@arm.com>
Cc: Steven Price <steven.price@arm.com>
Cc: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Cc: Pavel Tatashin <pasha.tatashin@soleen.com>
Cc: Anshuman Khandual <anshuman.khandual@arm.com>
Cc: Atish Patra <atish.patra@wdc.com>
Cc: Mike Rapoport <rppt@kernel.org>
Cc: Logan Gunthorpe <logang@deltatee.com>
Cc: Mark Brown <broonie@kernel.org>
To: linux-arm-kernel@lists.infradead.org
---
 arch/arm64/include/asm/kernel-pgtable.h | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/arch/arm64/include/asm/kernel-pgtable.h b/arch/arm64/include/asm/kernel-pgtable.h
index d44df9d62fc9..249ab132fced 100644
--- a/arch/arm64/include/asm/kernel-pgtable.h
+++ b/arch/arm64/include/asm/kernel-pgtable.h
@@ -24,23 +24,26 @@
 #endif
 
 /*
- * The idmap and swapper page tables need some space reserved in the kernel
- * image. Both require pgd, pud (4 levels only) and pmd tables to (section)
- * map the kernel. With the 64K page configuration, swapper and idmap need to
+ * The swapper page table needs some space reserved in the kernel
+ * image. It require pgd, pud (4 levels only) and pmd tables to (section)
+ * map the kernel. With the 64K page configuration, swapper needs to
  * map to pte level. The swapper also maps the FDT (see __create_page_tables
- * for more information). Note that the number of ID map translation levels
- * could be increased on the fly if system RAM is out of reach for the default
- * VA range, so pages required to map highest possible PA are reserved in all
- * cases.
+ * for more information).
  */
 #if ARM64_SWAPPER_USES_SECTION_MAPS
 #define SWAPPER_PGTABLE_LEVELS	(CONFIG_PGTABLE_LEVELS - 1)
-#define IDMAP_PGTABLE_LEVELS	(ARM64_HW_PGTABLE_LEVELS(PHYS_MASK_SHIFT) - 1)
 #else
 #define SWAPPER_PGTABLE_LEVELS	(CONFIG_PGTABLE_LEVELS)
-#define IDMAP_PGTABLE_LEVELS	(ARM64_HW_PGTABLE_LEVELS(PHYS_MASK_SHIFT))
 #endif
 
+/*
+ * idmap also needs some space reserved in the kernel image. Since ".idmap.text"
+ * only occupies less than a page, idmap needs to map to pte level.
+ * Note that the number of ID map translation levels could be increased on the fly
+ * if system RAM is out of reach for the default VA range, so pages required to
+ * map highest possible PA are reserved in all cases.
+ */
+#define IDMAP_PGTABLE_LEVELS	(ARM64_HW_PGTABLE_LEVELS(PHYS_MASK_SHIFT))
 
 /*
  * If KASLR is enabled, then an offset K is added to the kernel address
-- 
2.29.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCHv3 4/5] arm64/mm: make __create_pgd_mapping() capable to handle pgtable's paddr

[Pingfan Liu]

This patch is the last preparation for calling __create_pgd_mapping()
from head.S.

Under mmu-offset situation, __create_pgd_mapping() handles paddr of each
pgtable level. All of pud_t */pmd_t */pte_t * points to paddr, and they
should be handled carefully to avoid the involvement of __va().

Signed-off-by: Pingfan Liu <kernelfans@gmail.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: Ard Biesheuvel <ardb@kernel.org>
Cc: Marc Zyngier <maz@kernel.org>
Cc: Kristina Martsenko <kristina.martsenko@arm.com>
Cc: James Morse <james.morse@arm.com>
Cc: Steven Price <steven.price@arm.com>
Cc: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Cc: Pavel Tatashin <pasha.tatashin@soleen.com>
Cc: Anshuman Khandual <anshuman.khandual@arm.com>
Cc: Atish Patra <atish.patra@wdc.com>
Cc: Mike Rapoport <rppt@kernel.org>
Cc: Logan Gunthorpe <logang@deltatee.com>
Cc: Mark Brown <broonie@kernel.org>
To: linux-arm-kernel@lists.infradead.org
---
 arch/arm64/mm/mmu.c | 39 ++++++++++++++++++++++++++++++++++-----
 1 file changed, 34 insertions(+), 5 deletions(-)

diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
index fe16f235d1fa..5f717552b524 100644
--- a/arch/arm64/mm/mmu.c
+++ b/arch/arm64/mm/mmu.c
@@ -189,6 +189,12 @@ static void init_pte(pmd_t *pmdp, unsigned long addr, unsigned long end,
 	pte_t *ptep;
 
 	ptep = pte_set_fixmap_offset(pmdp, addr);
+	if (likely(!(flags & BOOT_HEAD)))
+		ptep = pte_set_fixmap_offset(pmdp, addr);
+	else
+		/* for head.S, there is no __va() */
+		ptep = (pte_t *)__pmd_to_phys(*pmdp) + pte_index(addr);
+
 	do {
 		pte_t old_pte = READ_ONCE(*ptep);
 
@@ -204,7 +210,8 @@ static void init_pte(pmd_t *pmdp, unsigned long addr, unsigned long end,
 		phys += PAGE_SIZE;
 	} while (ptep++, addr += PAGE_SIZE, addr != end);
 
-	pte_clear_fixmap();
+	if (likely(!(flags & BOOT_HEAD)))
+		pte_clear_fixmap();
 }
 
 static void alloc_init_cont_pte(pmd_t *pmdp, unsigned long addr,
@@ -253,7 +260,17 @@ static void init_pmd(pud_t *pudp, unsigned long addr, unsigned long end,
 	unsigned long next;
 	pmd_t *pmdp;
 
-	pmdp = pmd_set_fixmap_offset(pudp, addr);
+	if (likely(!(flags & BOOT_HEAD))) {
+		pmdp = pmd_set_fixmap_offset(pudp, addr);
+	} else {
+#if CONFIG_PGTABLE_LEVELS > 2
+		/* for head.S, there is no __va() */
+		pmdp = (pmd_t *)__pud_to_phys(*pudp) + pmd_index(addr);
+#else
+		pmdp = (pmd_t *)pudp;
+#endif
+	}
+
 	do {
 		pmd_t old_pmd = READ_ONCE(*pmdp);
 
@@ -280,7 +297,8 @@ static void init_pmd(pud_t *pudp, unsigned long addr, unsigned long end,
 		phys += next - addr;
 	} while (pmdp++, addr = next, addr != end);
 
-	pmd_clear_fixmap();
+	if (likely(!(flags & BOOT_HEAD)))
+		pmd_clear_fixmap();
 }
 
 static void alloc_init_cont_pmd(pud_t *pudp, unsigned long addr,
@@ -359,7 +377,17 @@ static void alloc_init_pud(pgd_t *pgdp, unsigned long addr, unsigned long end,
 	}
 	SAFE_BUG_ON(flags, p4d_bad(p4d));
 
-	pudp = pud_set_fixmap_offset(p4dp, addr);
+	if (likely(!(flags & BOOT_HEAD))) {
+		pudp = pud_set_fixmap_offset(p4dp, addr);
+	} else {
+#if CONFIG_PGTABLE_LEVELS > 3
+		/* for head.S, there is no __va() */
+		pudp = (pud_t *)__p4d_to_phys(*p4dp) + pud_index(addr);
+#else
+		pudp = (pud_t *)p4dp;
+#endif
+	}
+
 	do {
 		pud_t old_pud = READ_ONCE(*pudp);
 
@@ -388,7 +416,8 @@ static void alloc_init_pud(pgd_t *pgdp, unsigned long addr, unsigned long end,
 		phys += next - addr;
 	} while (pudp++, addr = next, addr != end);
 
-	pud_clear_fixmap();
+	if (likely(!(flags & BOOT_HEAD)))
+		pud_clear_fixmap();
 }
 
 static void __create_pgd_mapping(pgd_t *pgdir, phys_addr_t phys,
-- 
2.29.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCHv3 5/5] arm64/mm: use __create_pgd_mapping() to create pgtable for idmap_pg_dir and init_pg_dir

[Pingfan Liu]

Now, everything is ready for calling __create_pgd_mapping() from head.S.
Switching to these C routine and remove the asm counterpart.

This patch has been successfully tested with the following config value:
    PAGE_SIZE  VA  PA  PGTABLE_LEVEL
    4K         48  48  4
    4K         39  48  3
    16K        48  48  4
    16K        47  48  3
    64K        52  52  3
    64K        48  52  3
    64K        42  52  2

Signed-off-by: Pingfan Liu <kernelfans@gmail.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: Ard Biesheuvel <ardb@kernel.org>
Cc: Marc Zyngier <maz@kernel.org>
Cc: Kristina Martsenko <kristina.martsenko@arm.com>
Cc: James Morse <james.morse@arm.com>
Cc: Steven Price <steven.price@arm.com>
Cc: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Cc: Pavel Tatashin <pasha.tatashin@soleen.com>
Cc: Anshuman Khandual <anshuman.khandual@arm.com>
Cc: Atish Patra <atish.patra@wdc.com>
Cc: Mike Rapoport <rppt@kernel.org>
Cc: Logan Gunthorpe <logang@deltatee.com>
Cc: Mark Brown <broonie@kernel.org>
To: linux-arm-kernel@lists.infradead.org
---
 arch/arm64/include/asm/kernel-pgtable.h |  12 +-
 arch/arm64/include/asm/pgalloc.h        |   9 ++
 arch/arm64/kernel/head.S                | 164 +++++++-----------------
 arch/arm64/mm/mmu.c                     |   7 +-
 4 files changed, 60 insertions(+), 132 deletions(-)

diff --git a/arch/arm64/include/asm/kernel-pgtable.h b/arch/arm64/include/asm/kernel-pgtable.h
index 249ab132fced..121856008a0e 100644
--- a/arch/arm64/include/asm/kernel-pgtable.h
+++ b/arch/arm64/include/asm/kernel-pgtable.h
@@ -108,15 +108,11 @@
 
 /*
  * Initial memory map attributes.
+ * When using ARM64_SWAPPER_USES_SECTION_MAPS, init_pmd()->pmd_set_huge()
+ * sets up section mapping.
  */
-#define SWAPPER_PTE_FLAGS	(PTE_TYPE_PAGE | PTE_AF | PTE_SHARED)
-#define SWAPPER_PMD_FLAGS	(PMD_TYPE_SECT | PMD_SECT_AF | PMD_SECT_S)
-
-#if ARM64_SWAPPER_USES_SECTION_MAPS
-#define SWAPPER_MM_MMUFLAGS	(PMD_ATTRINDX(MT_NORMAL) | SWAPPER_PMD_FLAGS)
-#else
-#define SWAPPER_MM_MMUFLAGS	(PTE_ATTRINDX(MT_NORMAL) | SWAPPER_PTE_FLAGS)
-#endif
+#define SWAPPER_PAGE_FLAGS	((PTE_TYPE_PAGE | PTE_AF | PTE_SHARED) | \
+	PTE_ATTRINDX(MT_NORMAL))
 
 /*
  * To make optimal use of block mappings when laying out the linear
diff --git a/arch/arm64/include/asm/pgalloc.h b/arch/arm64/include/asm/pgalloc.h
index 31fbab3d6f99..9a6fb96ff291 100644
--- a/arch/arm64/include/asm/pgalloc.h
+++ b/arch/arm64/include/asm/pgalloc.h
@@ -8,6 +8,8 @@
 #ifndef __ASM_PGALLOC_H
 #define __ASM_PGALLOC_H
 
+#ifndef __ASSEMBLY__
+
 #include <asm/pgtable-hwdef.h>
 #include <asm/processor.h>
 #include <asm/cacheflush.h>
@@ -89,3 +91,10 @@ pmd_populate(struct mm_struct *mm, pmd_t *pmdp, pgtable_t ptep)
 #define pmd_pgtable(pmd) pmd_page(pmd)
 
 #endif
+
+#define NO_BLOCK_MAPPINGS	BIT(0)
+#define NO_CONT_MAPPINGS	BIT(1)
+#define NO_EXEC_MAPPINGS	BIT(2)	/* assumes FEAT_HPDS is not used */
+#define BOOT_HEAD		BIT(3)
+
+#endif
diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
index 96873dfa67fd..7158987f52b1 100644
--- a/arch/arm64/kernel/head.S
+++ b/arch/arm64/kernel/head.S
@@ -28,6 +28,7 @@
 #include <asm/memory.h>
 #include <asm/pgtable-hwdef.h>
 #include <asm/page.h>
+#include <asm/pgalloc.h>
 #include <asm/scs.h>
 #include <asm/smp.h>
 #include <asm/sysreg.h>
@@ -93,6 +94,8 @@ SYM_CODE_START(primary_entry)
 	adrp	x23, __PHYS_OFFSET
 	and	x23, x23, MIN_KIMG_ALIGN - 1	// KASLR offset, defaults to 0
 	bl	set_cpu_boot_mode_flag
+	adrp	x4, init_thread_union
+	add	sp, x4, #THREAD_SIZE
 	bl	__create_page_tables
 	/*
 	 * The following calls CPU setup code, see arch/arm64/mm/proc.S for
@@ -144,112 +147,6 @@ SYM_CODE_END(preserve_boot_args)
 	add	\tbl, \tbl, #PAGE_SIZE		// next level table page
 	.endm
 
-/*
- * Macro to populate page table entries, these entries can be pointers to the next level
- * or last level entries pointing to physical memory.
- *
- *	tbl:	page table address
- *	rtbl:	pointer to page table or physical memory
- *	index:	start index to write
- *	eindex:	end index to write - [index, eindex] written to
- *	flags:	flags for pagetable entry to or in
- *	inc:	increment to rtbl between each entry
- *	tmp1:	temporary variable
- *
- * Preserves:	tbl, eindex, flags, inc
- * Corrupts:	index, tmp1
- * Returns:	rtbl
- */
-	.macro populate_entries, tbl, rtbl, index, eindex, flags, inc, tmp1
-.Lpe\@:	phys_to_pte \tmp1, \rtbl
-	orr	\tmp1, \tmp1, \flags	// tmp1 = table entry
-	str	\tmp1, [\tbl, \index, lsl #3]
-	add	\rtbl, \rtbl, \inc	// rtbl = pa next level
-	add	\index, \index, #1
-	cmp	\index, \eindex
-	b.ls	.Lpe\@
-	.endm
-
-/*
- * Compute indices of table entries from virtual address range. If multiple entries
- * were needed in the previous page table level then the next page table level is assumed
- * to be composed of multiple pages. (This effectively scales the end index).
- *
- *	vstart:	virtual address of start of range
- *	vend:	virtual address of end of range
- *	shift:	shift used to transform virtual address into index
- *	ptrs:	number of entries in page table
- *	istart:	index in table corresponding to vstart
- *	iend:	index in table corresponding to vend
- *	count:	On entry: how many extra entries were required in previous level, scales
- *			  our end index.
- *		On exit: returns how many extra entries required for next page table level
- *
- * Preserves:	vstart, vend, shift, ptrs
- * Returns:	istart, iend, count
- */
-	.macro compute_indices, vstart, vend, shift, ptrs, istart, iend, count
-	lsr	\iend, \vend, \shift
-	mov	\istart, \ptrs
-	sub	\istart, \istart, #1
-	and	\iend, \iend, \istart	// iend = (vend >> shift) & (ptrs - 1)
-	mov	\istart, \ptrs
-	mul	\istart, \istart, \count
-	add	\iend, \iend, \istart	// iend += (count - 1) * ptrs
-					// our entries span multiple tables
-
-	lsr	\istart, \vstart, \shift
-	mov	\count, \ptrs
-	sub	\count, \count, #1
-	and	\istart, \istart, \count
-
-	sub	\count, \iend, \istart
-	.endm
-
-/*
- * Map memory for specified virtual address range. Each level of page table needed supports
- * multiple entries. If a level requires n entries the next page table level is assumed to be
- * formed from n pages.
- *
- *	tbl:	location of page table
- *	rtbl:	address to be used for first level page table entry (typically tbl + PAGE_SIZE)
- *	vstart:	start address to map
- *	vend:	end address to map - we map [vstart, vend]
- *	flags:	flags to use to map last level entries
- *	phys:	physical address corresponding to vstart - physical memory is contiguous
- *	pgds:	the number of pgd entries
- *
- * Temporaries:	istart, iend, tmp, count, sv - these need to be different registers
- * Preserves:	vstart, vend, flags
- * Corrupts:	tbl, rtbl, istart, iend, tmp, count, sv
- */
-	.macro map_memory, tbl, rtbl, vstart, vend, flags, phys, pgds, istart, iend, tmp, count, sv
-	add \rtbl, \tbl, #PAGE_SIZE
-	mov \sv, \rtbl
-	mov \count, #0
-	compute_indices \vstart, \vend, #PGDIR_SHIFT, \pgds, \istart, \iend, \count
-	populate_entries \tbl, \rtbl, \istart, \iend, #PMD_TYPE_TABLE, #PAGE_SIZE, \tmp
-	mov \tbl, \sv
-	mov \sv, \rtbl
-
-#if SWAPPER_PGTABLE_LEVELS > 3
-	compute_indices \vstart, \vend, #PUD_SHIFT, #PTRS_PER_PUD, \istart, \iend, \count
-	populate_entries \tbl, \rtbl, \istart, \iend, #PMD_TYPE_TABLE, #PAGE_SIZE, \tmp
-	mov \tbl, \sv
-	mov \sv, \rtbl
-#endif
-
-#if SWAPPER_PGTABLE_LEVELS > 2
-	compute_indices \vstart, \vend, #SWAPPER_TABLE_SHIFT, #PTRS_PER_PMD, \istart, \iend, \count
-	populate_entries \tbl, \rtbl, \istart, \iend, #PMD_TYPE_TABLE, #PAGE_SIZE, \tmp
-	mov \tbl, \sv
-#endif
-
-	compute_indices \vstart, \vend, #SWAPPER_BLOCK_SHIFT, #PTRS_PER_PTE, \istart, \iend, \count
-	bic \count, \phys, #SWAPPER_BLOCK_SIZE - 1
-	populate_entries \tbl, \count, \istart, \iend, \flags, #SWAPPER_BLOCK_SIZE, \tmp
-	.endm
-
 /*
  * Setup the initial page tables. We only setup the barest amount which is
  * required to get the kernel running. The following sections are required:
@@ -284,8 +181,6 @@ SYM_FUNC_START_LOCAL(__create_page_tables)
 	subs	x1, x1, #64
 	b.ne	1b
 
-	mov	x7, SWAPPER_MM_MMUFLAGS
-
 	/*
 	 * Create the identity mapping.
 	 */
@@ -357,21 +252,54 @@ SYM_FUNC_START_LOCAL(__create_page_tables)
 	mov	x5, x3				// __pa(__idmap_text_start)
 	adr_l	x6, __idmap_text_end		// __pa(__idmap_text_end)
 
-	map_memory x0, x1, x3, x6, x7, x3, x4, x10, x11, x12, x13, x14
+	/*
+	 * x0 points to either idmap_pg_dir or idmap_pg_dir + PAGE_SIZE
+	 */
+	stp     x0, x1, [sp, #-64]!
+	stp     x2, x3, [sp, #48]
+	stp     x4, x5, [sp, #32]
+	stp     x6, x7, [sp, #16]
+
+	adrp    x1, idmap_pg_end
+	sub     x1, x1, x0
+	bl      set_cur_headpool
+	mov	x0, #0
+	bl	head_pgtable_alloc	// return x0, containing the appropriate pgtable level
+
+	adrp    x1, __idmap_text_start
+	adrp    x2, __idmap_text_start	// va = pa for idmap
+	adr_l   x3, __idmap_text_end
+	sub     x3, x3, x1
+	ldr     x4, =SWAPPER_PAGE_FLAGS
+	adr_l   x5, head_pgtable_alloc
+	mov     x6, #BOOT_HEAD
+	bl	__create_pgd_mapping
 
 	/*
 	 * Map the kernel image (starting with PHYS_OFFSET).
 	 */
 	adrp	x0, init_pg_dir
-	mov_q	x5, KIMAGE_VADDR		// compile time __va(_text)
-	add	x5, x5, x23			// add KASLR displacement
-	mov	x4, PTRS_PER_PGD
-	adrp	x6, _end			// runtime __pa(_end)
-	adrp	x3, _text			// runtime __pa(_text)
-	sub	x6, x6, x3			// _end - _text
-	add	x6, x6, x5			// runtime __va(_end)
-
-	map_memory x0, x1, x5, x6, x7, x3, x4, x10, x11, x12, x13, x14
+	adrp	x1, init_pg_end
+	sub	x1, x1, x0
+	bl	set_cur_headpool
+	mov	x0, #0
+	bl	head_pgtable_alloc		// return x0, containing init_pg_dir
+
+	adrp	x1, _text			// runtime __pa(_text)
+	mov_q	x2, KIMAGE_VADDR		// compile time __va(_text)
+	add	x2, x2, x23			// add KASLR displacement
+	adrp	x3, _end			// runtime __pa(_end)
+	sub	x3, x3, x1			// _end - _text
+
+	ldr	x4, =SWAPPER_PAGE_FLAGS
+	adr_l	x5, head_pgtable_alloc
+	mov	x6, #BOOT_HEAD
+	bl	__create_pgd_mapping
+
+	ldp	x6, x7, [sp, #16]
+	ldp	x4, x5, [sp, #32]
+	ldp	x2, x3, [sp, #48]
+	ldp	x0, x1, [sp], #64
 
 	/*
 	 * Since the page tables have been populated with non-cacheable
diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
index 5f717552b524..b3295523607e 100644
--- a/arch/arm64/mm/mmu.c
+++ b/arch/arm64/mm/mmu.c
@@ -37,11 +37,6 @@
 #include <asm/tlbflush.h>
 #include <asm/pgalloc.h>
 
-#define NO_BLOCK_MAPPINGS	BIT(0)
-#define NO_CONT_MAPPINGS	BIT(1)
-#define NO_EXEC_MAPPINGS	BIT(2)	/* assumes FEAT_HPDS is not used */
-#define BOOT_HEAD		BIT(3)
-
 u64 idmap_t0sz = TCR_T0SZ(VA_BITS_MIN);
 u64 idmap_ptrs_per_pgd = PTRS_PER_PGD;
 
@@ -420,7 +415,7 @@ static void alloc_init_pud(pgd_t *pgdp, unsigned long addr, unsigned long end,
 		pud_clear_fixmap();
 }
 
-static void __create_pgd_mapping(pgd_t *pgdir, phys_addr_t phys,
+void __create_pgd_mapping(pgd_t *pgdir, phys_addr_t phys,
 				 unsigned long virt, phys_addr_t size,
 				 pgprot_t prot,
 				 phys_addr_t (*pgtable_alloc)(int),
-- 
2.29.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCHv3 0/5] use __create_pgd_mapping() to implement idmap and unify codes

[Ard Biesheuvel]

On Mon, 31 May 2021 at 10:46, Pingfan Liu <kernelfans@gmail.com> wrote:
>
> v2 -> v3:
>   -1. leave out the part of redefinition the CONFIG_PGTABLE_LEVEL,
> concentrate on sharing __create_pgd_mapping() in head.S as the first
> step.
>   -2. make IDMAP_PGTABLE_LEVELS use the max value ([3/5])
>
> rfc -> v2:
>   more debug and test
>
> *** Goal of this series ***
>
> __create_pgd_mapping() sets up the pgtable for mapping __va(paddr) ->
> paddr under the MMU-on situation.  Since pgtable upper level holds the
> paddr of the lower level, with a slight adaptation,
> __create_pgd_mapping() can also set up the mapping under the MMU-off
> situation. ([4/5])
>
> After that, both idmap_pg_dir and init_pg_dir can be created by
> __create_pgd_mapping(). And the counterpart asm code can be simplified.
>

I understand the desire to simplify the page table construction code
in head.S, but up until now, we have been very careful to avoid
calling into C code with the MMU off. There are simply too many
assumptions in the compiler about the context the generated code will
execute in: one example is unaligned access, which must be disabled
for source files that may be called with the MMU off, as otherwise,
the compiler is permitted to emit loads and stores that are not
allowed on device memory (which is the default memory type used for
all memory with the MMU off)

Do you have a killer use case for this feature? Or is it just a nice cleanup?

> This series can be applied against commit 4284bdf9405a ("Linux 5.13-rc2").
>
>
> *** Plan for the next ***
>
> The omitted part in V2, which resorts to redefinition of
> CONFIG_PGTABLE_LEVEL to provides two sets of routines. One set is proper
> to set up a idmap which can adress total system RAM.  That can simplify
> the asm code in head.S furtherly, also provide an create_idmap() API.
>
>
> *** Test ***
>
> This series can success booting with the following configuration on either Cavium
> ThunderX2 99xx or Qualcomm Falkor:
>             PAGE_SIZE  VA  PA  PGTABLE_LEVEL
> Qualcomm    4K         48  48  4
>             4K         39  48  3
>             16K        48  48  4
>             16K        47  48  3
> Cavium      64K        52  52  3
>             64K        48  52  3
>             64K        42  52  2
>
>
>
> *** History ***
>
> RFC:
> https://lore.kernel.org/linux-arm-kernel/20210410095654.24102-1-kernelfans@gmail.com/
> V2:
> https://lore.kernel.org/linux-arm-kernel/20210425141304.32721-1-kernelfans@gmail.com/
>
>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: Ard Biesheuvel <ardb@kernel.org>
> Cc: Marc Zyngier <maz@kernel.org>
> Cc: Kristina Martsenko <kristina.martsenko@arm.com>
> Cc: James Morse <james.morse@arm.com>
> Cc: Steven Price <steven.price@arm.com>
> Cc: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Cc: Pavel Tatashin <pasha.tatashin@soleen.com>
> Cc: Anshuman Khandual <anshuman.khandual@arm.com>
> Cc: Atish Patra <atish.patra@wdc.com>
> Cc: Mike Rapoport <rppt@kernel.org>
> Cc: Logan Gunthorpe <logang@deltatee.com>
> Cc: Mark Brown <broonie@kernel.org>
> To: linux-arm-kernel@lists.infradead.org
>
> Pingfan Liu (5):
>   arm64/mm: introduce pgtable allocator for idmap_pg_dir and init_pg_dir
>   arm64/mm: disable WARN_ON() and BUG_ON() in __create_pgd_mapping() if
>     too early
>   arm64/mm: unconditionally set IDMAP_PGTABLE_LEVELS to max pgtable
>     level
>   arm64/mm: make __create_pgd_mapping() capable to handle pgtable's
>     paddr
>   arm64/mm: use __create_pgd_mapping() to create pgtable for
>     idmap_pg_dir and init_pg_dir
>
>  arch/arm64/include/asm/kernel-pgtable.h |  33 +++--
>  arch/arm64/include/asm/pgalloc.h        |   9 ++
>  arch/arm64/kernel/head.S                | 164 +++++++-----------------
>  arch/arm64/mm/mmu.c                     | 108 ++++++++++++----
>  4 files changed, 153 insertions(+), 161 deletions(-)
>
> --
> 2.29.2
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCHv3 0/5] use __create_pgd_mapping() to implement idmap and unify codes

[Pingfan Liu]

Hi Ard,

Thanks for your reviewing.

On Tue, Jun 1, 2021 at 3:50 AM Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Mon, 31 May 2021 at 10:46, Pingfan Liu <kernelfans@gmail.com> wrote:
> >
> > v2 -> v3:
> >   -1. leave out the part of redefinition the CONFIG_PGTABLE_LEVEL,
> > concentrate on sharing __create_pgd_mapping() in head.S as the first
> > step.
> >   -2. make IDMAP_PGTABLE_LEVELS use the max value ([3/5])
> >
> > rfc -> v2:
> >   more debug and test
> >
> > *** Goal of this series ***
> >
> > __create_pgd_mapping() sets up the pgtable for mapping __va(paddr) ->
> > paddr under the MMU-on situation.  Since pgtable upper level holds the
> > paddr of the lower level, with a slight adaptation,
> > __create_pgd_mapping() can also set up the mapping under the MMU-off
> > situation. ([4/5])
> >
> > After that, both idmap_pg_dir and init_pg_dir can be created by
> > __create_pgd_mapping(). And the counterpart asm code can be simplified.
> >
>
> I understand the desire to simplify the page table construction code
> in head.S, but up until now, we have been very careful to avoid
> calling into C code with the MMU off. There are simply too many
> assumptions in the compiler about the context the generated code will
> execute in: one example is unaligned access, which must be disabled
> for source files that may be called with the MMU off, as otherwise,
> the compiler is permitted to emit loads and stores that are not
> allowed on device memory (which is the default memory type used for
> all memory with the MMU off)
>
You are right. These C routines happen to use "unsigned long", which
can exclude this unaligned case.
To make an guarantee, is "-mno-unaligned-access" good enough?

Besides unaligned-access, any further risk originating from compiler
assumption? (I think that the common optimization: reordering,
merging, reloading on this "device" memory has no bad effect)

> Do you have a killer use case for this feature? Or is it just a nice cleanup?
>
Yes, in the omitted part in v2, I had planned to provide an unified
page table manipulation routine, and provide an create_idmap() API. So
there can be an handy interface to create a whole RAM addressable
idmapX where needed.

And three place can share the API create_idmap(): head.S,
trans_pgd_idmap_page(), kexec boot with mmu-on. As a result, most of
mm/trans_pgd.c can be cleaned after hibernation records paddr.

Thanks,

Pingfan
> > This series can be applied against commit 4284bdf9405a ("Linux 5.13-rc2").
> >
> >
> > *** Plan for the next ***
> >
> > The omitted part in V2, which resorts to redefinition of
> > CONFIG_PGTABLE_LEVEL to provides two sets of routines. One set is proper
> > to set up a idmap which can adress total system RAM.  That can simplify
> > the asm code in head.S furtherly, also provide an create_idmap() API.
> >
> >
> > *** Test ***
> >
> > This series can success booting with the following configuration on either Cavium
> > ThunderX2 99xx or Qualcomm Falkor:
> >             PAGE_SIZE  VA  PA  PGTABLE_LEVEL
> > Qualcomm    4K         48  48  4
> >             4K         39  48  3
> >             16K        48  48  4
> >             16K        47  48  3
> > Cavium      64K        52  52  3
> >             64K        48  52  3
> >             64K        42  52  2
> >
> >
> >
> > *** History ***
> >
> > RFC:
> > https://lore.kernel.org/linux-arm-kernel/20210410095654.24102-1-kernelfans@gmail.com/
> > V2:
> > https://lore.kernel.org/linux-arm-kernel/20210425141304.32721-1-kernelfans@gmail.com/
> >
> >
> > Cc: Catalin Marinas <catalin.marinas@arm.com>
> > Cc: Will Deacon <will@kernel.org>
> > Cc: Ard Biesheuvel <ardb@kernel.org>
> > Cc: Marc Zyngier <maz@kernel.org>
> > Cc: Kristina Martsenko <kristina.martsenko@arm.com>
> > Cc: James Morse <james.morse@arm.com>
> > Cc: Steven Price <steven.price@arm.com>
> > Cc: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > Cc: Pavel Tatashin <pasha.tatashin@soleen.com>
> > Cc: Anshuman Khandual <anshuman.khandual@arm.com>
> > Cc: Atish Patra <atish.patra@wdc.com>
> > Cc: Mike Rapoport <rppt@kernel.org>
> > Cc: Logan Gunthorpe <logang@deltatee.com>
> > Cc: Mark Brown <broonie@kernel.org>
> > To: linux-arm-kernel@lists.infradead.org
> >
> > Pingfan Liu (5):
> >   arm64/mm: introduce pgtable allocator for idmap_pg_dir and init_pg_dir
> >   arm64/mm: disable WARN_ON() and BUG_ON() in __create_pgd_mapping() if
> >     too early
> >   arm64/mm: unconditionally set IDMAP_PGTABLE_LEVELS to max pgtable
> >     level
> >   arm64/mm: make __create_pgd_mapping() capable to handle pgtable's
> >     paddr
> >   arm64/mm: use __create_pgd_mapping() to create pgtable for
> >     idmap_pg_dir and init_pg_dir
> >
> >  arch/arm64/include/asm/kernel-pgtable.h |  33 +++--
> >  arch/arm64/include/asm/pgalloc.h        |   9 ++
> >  arch/arm64/kernel/head.S                | 164 +++++++-----------------
> >  arch/arm64/mm/mmu.c                     | 108 ++++++++++++----
> >  4 files changed, 153 insertions(+), 161 deletions(-)
> >
> > --
> > 2.29.2
> >

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCHv3 0/5] use __create_pgd_mapping() to implement idmap and unify codes

[Catalin Marinas]

On Tue, Jun 01, 2021 at 05:25:49PM +0800, Pingfan Liu wrote:
> On Tue, Jun 1, 2021 at 3:50 AM Ard Biesheuvel <ardb@kernel.org> wrote:
> > On Mon, 31 May 2021 at 10:46, Pingfan Liu <kernelfans@gmail.com> wrote:
> > > v2 -> v3:
> > >   -1. leave out the part of redefinition the CONFIG_PGTABLE_LEVEL,
> > > concentrate on sharing __create_pgd_mapping() in head.S as the first
> > > step.
> > >   -2. make IDMAP_PGTABLE_LEVELS use the max value ([3/5])
> > >
> > > rfc -> v2:
> > >   more debug and test
> > >
> > > *** Goal of this series ***
> > >
> > > __create_pgd_mapping() sets up the pgtable for mapping __va(paddr) ->
> > > paddr under the MMU-on situation.  Since pgtable upper level holds the
> > > paddr of the lower level, with a slight adaptation,
> > > __create_pgd_mapping() can also set up the mapping under the MMU-off
> > > situation. ([4/5])
> > >
> > > After that, both idmap_pg_dir and init_pg_dir can be created by
> > > __create_pgd_mapping(). And the counterpart asm code can be simplified.
> >
> > I understand the desire to simplify the page table construction code
> > in head.S, but up until now, we have been very careful to avoid
> > calling into C code with the MMU off. There are simply too many
> > assumptions in the compiler about the context the generated code will
> > execute in: one example is unaligned access, which must be disabled
> > for source files that may be called with the MMU off, as otherwise,
> > the compiler is permitted to emit loads and stores that are not
> > allowed on device memory (which is the default memory type used for
> > all memory with the MMU off)
>
> You are right. These C routines happen to use "unsigned long", which
> can exclude this unaligned case.
> To make an guarantee, is "-mno-unaligned-access" good enough?
> 
> Besides unaligned-access, any further risk originating from compiler
> assumption? (I think that the common optimization: reordering,
> merging, reloading on this "device" memory has no bad effect)

There's also instrumentation that needs disabling (kasan, ubsan, kcov,
gcov). You can look at arch/arm64/kvm/hyp/nvhe/Makefile for various
flags added or filtered out, though the KVM hyp code runs with the MMU
on. I'm not sure what other flags are needed to guarantee the generated
code can run with the MMU off but we can always ask the toolchain folk.

However, I'm still not convinced about sharing __create_pgd_mapping()
with the early head.S code. A better option would be a separate,
stand-alone file where we have more control on what gets called or
accessed (of course, if there's any value in going this route).

> > Do you have a killer use case for this feature? Or is it just a nice cleanup?
>
> Yes, in the omitted part in v2, I had planned to provide an unified
> page table manipulation routine, and provide an create_idmap() API. So
> there can be an handy interface to create a whole RAM addressable
> idmapX where needed.

Where would this be needed?

-- 
Catalin

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCHv3 0/5] use __create_pgd_mapping() to implement idmap and unify codes

[Pingfan Liu]

On Tue, Jun 08, 2021 at 06:38:07PM +0100, Catalin Marinas wrote:
> On Tue, Jun 01, 2021 at 05:25:49PM +0800, Pingfan Liu wrote:
> > On Tue, Jun 1, 2021 at 3:50 AM Ard Biesheuvel <ardb@kernel.org> wrote:
> > > On Mon, 31 May 2021 at 10:46, Pingfan Liu <kernelfans@gmail.com> wrote:
> > > > v2 -> v3:
> > > >   -1. leave out the part of redefinition the CONFIG_PGTABLE_LEVEL,
> > > > concentrate on sharing __create_pgd_mapping() in head.S as the first
> > > > step.
> > > >   -2. make IDMAP_PGTABLE_LEVELS use the max value ([3/5])
> > > >
> > > > rfc -> v2:
> > > >   more debug and test
> > > >
> > > > *** Goal of this series ***
> > > >
> > > > __create_pgd_mapping() sets up the pgtable for mapping __va(paddr) ->
> > > > paddr under the MMU-on situation.  Since pgtable upper level holds the
> > > > paddr of the lower level, with a slight adaptation,
> > > > __create_pgd_mapping() can also set up the mapping under the MMU-off
> > > > situation. ([4/5])
> > > >
> > > > After that, both idmap_pg_dir and init_pg_dir can be created by
> > > > __create_pgd_mapping(). And the counterpart asm code can be simplified.
> > >
> > > I understand the desire to simplify the page table construction code
> > > in head.S, but up until now, we have been very careful to avoid
> > > calling into C code with the MMU off. There are simply too many
> > > assumptions in the compiler about the context the generated code will
> > > execute in: one example is unaligned access, which must be disabled
> > > for source files that may be called with the MMU off, as otherwise,
> > > the compiler is permitted to emit loads and stores that are not
> > > allowed on device memory (which is the default memory type used for
> > > all memory with the MMU off)
> >
> > You are right. These C routines happen to use "unsigned long", which
> > can exclude this unaligned case.
> > To make an guarantee, is "-mno-unaligned-access" good enough?
> > 

Unaligned-access is the main challedge, and the unaligned-access should be caused from the programer.
Compiler has enforced different default alignment, including global and local(stack) variable, function alignment.
Otherwise, the atomity can not be assumped on some base data type.

But some code may violate the alignment, which compiler has no method to prevent.
As Documentation/core-api/unaligned-memory-access.rst
  1. Casting variables to types of different lengths
  2. Pointer arithmetic followed by access to at least 2 bytes of data

So in order to prevent the unaligned-access, the involved codes should be checked carefully.

> > Besides unaligned-access, any further risk originating from compiler
> > assumption? (I think that the common optimization: reordering,
> > merging, reloading on this "device" memory has no bad effect)
> 

These should be not issue. Since MMU-off present a more strict memory model than MMU-on.
So if a code can run on a more relaxed one, it can also run on the strict one.

> There's also instrumentation that needs disabling (kasan, ubsan, kcov,
> gcov). You can look at arch/arm64/kvm/hyp/nvhe/Makefile for various
> flags added or filtered out, though the KVM hyp code runs with the MMU
> on. I'm not sure what other flags are needed to guarantee the generated
> code can run with the MMU off but we can always ask the toolchain folk.
> 

Thanks for pointing out these issues. I will check.

> However, I'm still not convinced about sharing __create_pgd_mapping()
> with the early head.S code. A better option would be a separate,
> stand-alone file where we have more control on what gets called or
> accessed (of course, if there's any value in going this route).
> 
> > > Do you have a killer use case for this feature? Or is it just a nice cleanup?
> >
> > Yes, in the omitted part in v2, I had planned to provide an unified
> > page table manipulation routine, and provide an create_idmap() API. So
> > there can be an handy interface to create a whole RAM addressable
> > idmapX where needed.
> 
> Where would this be needed?
> 

It was planned for Pavel's series: [PATCH v13 00/18] arm64: MMU enabled kexec relocation,
where Pavel uses linear mapping to copy the data. But that way should involve
copy of "EL2 vectors" to tackle the transition to EL2.  On the other hand, if
using a global addressable idmap, the logic of transition to EL2 keeps
untouched, and just enable MMU in SYM_CODE_START(arm64_relocate_new_kernel)

The other call site is in trans_pgd_idmap_page() 


Thanks,

Pingfan


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel