[PATCH 0/2] ARM: move FDT mapping out of linear region

[PATCH 0/2] ARM: move FDT mapping out of linear region

[Ard Biesheuvel]

For the ARM kernel, setting up the kernel's virtual address space at boot
is tricky, given the risk of collision between 1:1 mapped regions and
virtually remapped regions. There is also a concern regarding the exact
mapping attributes that are needed for each region, which differs between
ARM architecture revisions, UP vs SMP etc

For this reason, the kernel VA space is set up in two stages: at early
boot, only the kernel itself and the DT are mapped using section mappings
and later on, all existing mappings are torn down except the one covering
the first memblock covering the kernel, and remapped again using the full
fledged mapping routines that can map at page granularity and use all the
right mapping attributes.

There are cases where this may result in the DT getting unmapped at
this point, and not remapped again. For instance, if the first memblock
is not PMD aligned, we will align it up, and mark the memory below it
as MEMBLOCK_NOMAP, to avoid having to allocate page tables to create
the mapping before we have a mapped memblock to allocate from. If the
DT happens to reside in this region, it will not be mapped at all when
the permanent kernel VA mappings are in place, resulting in crashes.

As we happen to have a 4 MB hole in the kernel VA space (between the
end of the VMALLOC space and the start of the FIXMAP region), let's
use it to create a permanent, read-only mapping of the DT that is not
affected by any such issues.

Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Florian Fainelli <f.fainelli@gmail.com>
Cc: Russell King <linux@armlinux.org.uk>
Cc: Nicolas Pitre <nico@fluxnic.net>
Cc: Rob Herring <robh+dt@kernel.org>

Ard Biesheuvel (2):
  ARM: centralize phys-to-virt conversion of DT/ATAGS address
  ARM: move device tree mapping out of linear region

 arch/arm/include/asm/memory.h |  5 +++++
 arch/arm/include/asm/prom.h   |  4 ++--
 arch/arm/kernel/atags.h       |  4 ++--
 arch/arm/kernel/atags_parse.c |  6 +++---
 arch/arm/kernel/devtree.c     |  6 +++---
 arch/arm/kernel/head.S        |  5 ++---
 arch/arm/kernel/setup.c       | 19 ++++++++++++++-----
 arch/arm/mm/init.c            |  1 -
 arch/arm/mm/mmu.c             | 20 ++++++++++++++------
 arch/arm/mm/pv-fixup-asm.S    |  4 ++--
 10 files changed, 47 insertions(+), 27 deletions(-)

-- 
2.17.1


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

[PATCH 1/2] ARM: centralize phys-to-virt conversion of DT/ATAGS address

[Ard Biesheuvel]

Before moving the DT mapping out of the linear region, let's prepare
for this change by removing all the phys-to-virt translations of the
__atags_pointer variable, and perform this translation only once at
setup time.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 arch/arm/include/asm/prom.h   |  4 ++--
 arch/arm/kernel/atags.h       |  4 ++--
 arch/arm/kernel/atags_parse.c |  6 +++---
 arch/arm/kernel/devtree.c     |  6 +++---
 arch/arm/kernel/setup.c       | 14 +++++++++-----
 arch/arm/mm/mmu.c             |  4 ++--
 6 files changed, 21 insertions(+), 17 deletions(-)

diff --git a/arch/arm/include/asm/prom.h b/arch/arm/include/asm/prom.h
index 1e36c40533c1..402e3f34c7ed 100644
--- a/arch/arm/include/asm/prom.h
+++ b/arch/arm/include/asm/prom.h
@@ -9,12 +9,12 @@
 
 #ifdef CONFIG_OF
 
-extern const struct machine_desc *setup_machine_fdt(unsigned int dt_phys);
+extern const struct machine_desc *setup_machine_fdt(void *dt_virt);
 extern void __init arm_dt_init_cpu_maps(void);
 
 #else /* CONFIG_OF */
 
-static inline const struct machine_desc *setup_machine_fdt(unsigned int dt_phys)
+static inline const struct machine_desc *setup_machine_fdt(void *dt_virt)
 {
 	return NULL;
 }
diff --git a/arch/arm/kernel/atags.h b/arch/arm/kernel/atags.h
index 067e12edc341..f2819c25b602 100644
--- a/arch/arm/kernel/atags.h
+++ b/arch/arm/kernel/atags.h
@@ -2,11 +2,11 @@
 void convert_to_tag_list(struct tag *tags);
 
 #ifdef CONFIG_ATAGS
-const struct machine_desc *setup_machine_tags(phys_addr_t __atags_pointer,
+const struct machine_desc *setup_machine_tags(void *__atags_vaddr,
 	unsigned int machine_nr);
 #else
 static inline const struct machine_desc * __init __noreturn
-setup_machine_tags(phys_addr_t __atags_pointer, unsigned int machine_nr)
+setup_machine_tags(void *__atags_vaddr, unsigned int machine_nr)
 {
 	early_print("no ATAGS support: can't continue\n");
 	while (true);
diff --git a/arch/arm/kernel/atags_parse.c b/arch/arm/kernel/atags_parse.c
index 6c12d9fe694e..373b61f9a4f0 100644
--- a/arch/arm/kernel/atags_parse.c
+++ b/arch/arm/kernel/atags_parse.c
@@ -174,7 +174,7 @@ static void __init squash_mem_tags(struct tag *tag)
 }
 
 const struct machine_desc * __init
-setup_machine_tags(phys_addr_t __atags_pointer, unsigned int machine_nr)
+setup_machine_tags(void *atags_vaddr, unsigned int machine_nr)
 {
 	struct tag *tags = (struct tag *)&default_tags;
 	const struct machine_desc *mdesc = NULL, *p;
@@ -195,8 +195,8 @@ setup_machine_tags(phys_addr_t __atags_pointer, unsigned int machine_nr)
 	if (!mdesc)
 		return NULL;
 
-	if (__atags_pointer)
-		tags = phys_to_virt(__atags_pointer);
+	if (atags_vaddr)
+		tags = atags_vaddr;
 	else if (mdesc->atag_offset)
 		tags = (void *)(PAGE_OFFSET + mdesc->atag_offset);
 
diff --git a/arch/arm/kernel/devtree.c b/arch/arm/kernel/devtree.c
index 39c978698406..4e09883c276d 100644
--- a/arch/arm/kernel/devtree.c
+++ b/arch/arm/kernel/devtree.c
@@ -203,12 +203,12 @@ static const void * __init arch_get_next_mach(const char *const **match)
 
 /**
  * setup_machine_fdt - Machine setup when an dtb was passed to the kernel
- * @dt_phys: physical address of dt blob
+ * @dt_virt: virtual address of dt blob
  *
  * If a dtb was passed to the kernel in r2, then use it to choose the
  * correct machine_desc and to setup the system.
  */
-const struct machine_desc * __init setup_machine_fdt(unsigned int dt_phys)
+const struct machine_desc * __init setup_machine_fdt(void *dt_virt)
 {
 	const struct machine_desc *mdesc, *mdesc_best = NULL;
 
@@ -221,7 +221,7 @@ const struct machine_desc * __init setup_machine_fdt(unsigned int dt_phys)
 	mdesc_best = &__mach_desc_GENERIC_DT;
 #endif
 
-	if (!dt_phys || !early_init_dt_verify(phys_to_virt(dt_phys)))
+	if (!dt_virt || !early_init_dt_verify(dt_virt))
 		return NULL;
 
 	mdesc = of_flat_dt_match_machine(mdesc_best, arch_get_next_mach);
diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
index d8e18cdd96d3..0ea0172d0c6d 100644
--- a/arch/arm/kernel/setup.c
+++ b/arch/arm/kernel/setup.c
@@ -89,6 +89,7 @@ unsigned int cacheid __read_mostly;
 EXPORT_SYMBOL(cacheid);
 
 unsigned int __atags_pointer __initdata;
+void *atags_vaddr __initdata;
 
 unsigned int system_rev;
 EXPORT_SYMBOL(system_rev);
@@ -1075,19 +1076,22 @@ void __init hyp_mode_check(void)
 
 void __init setup_arch(char **cmdline_p)
 {
-	const struct machine_desc *mdesc;
+	const struct machine_desc *mdesc = NULL;
+
+	if (__atags_pointer)
+		atags_vaddr = phys_to_virt(__atags_pointer);
 
 	setup_processor();
-	mdesc = setup_machine_fdt(__atags_pointer);
+	if (atags_vaddr)
+		mdesc = setup_machine_fdt(atags_vaddr);
 	if (!mdesc)
-		mdesc = setup_machine_tags(__atags_pointer, __machine_arch_type);
+		mdesc = setup_machine_tags(atags_vaddr, __machine_arch_type);
 	if (!mdesc) {
 		early_print("\nError: invalid dtb and unrecognized/unsupported machine ID\n");
 		early_print("  r1=0x%08x, r2=0x%08x\n", __machine_arch_type,
 			    __atags_pointer);
 		if (__atags_pointer)
-			early_print("  r2[]=%*ph\n", 16,
-				    phys_to_virt(__atags_pointer));
+			early_print("  r2[]=%*ph\n", 16, atags_vaddr);
 		dump_machine_table();
 	}
 
diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
index c36f977b2ccb..0665a0dbd040 100644
--- a/arch/arm/mm/mmu.c
+++ b/arch/arm/mm/mmu.c
@@ -1503,7 +1503,7 @@ static void __init map_lowmem(void)
 }
 
 #ifdef CONFIG_ARM_PV_FIXUP
-extern unsigned long __atags_pointer;
+extern void *atags_vaddr;
 typedef void pgtables_remap(long long offset, unsigned long pgd, void *bdata);
 pgtables_remap lpae_pgtables_remap_asm;
 
@@ -1534,7 +1534,7 @@ static void __init early_paging_init(const struct machine_desc *mdesc)
 	 */
 	lpae_pgtables_remap = (pgtables_remap *)(unsigned long)__pa(lpae_pgtables_remap_asm);
 	pa_pgd = __pa(swapper_pg_dir);
-	boot_data = __va(__atags_pointer);
+	boot_data = atags_vaddr;
 	barrier();
 
 	pr_info("Switching physical address space to 0x%08llx\n",
-- 
2.17.1


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

[PATCH 2/2] ARM: move device tree mapping out of linear region

[Ard Biesheuvel]

On ARM, setting up the linear region is tricky, given the constraints
around placement and alignment of the memblocks, and how the kernel
itself as well as the DT are placed in physical memory.

Let's simplify matters a bit, by moving the device tree mapping to the
top of the address space, right below the fixmap region, and create a
read-only mapping for it that is independent of the size of the linear
region, and how it is organized.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 arch/arm/include/asm/memory.h |  5 +++++
 arch/arm/kernel/head.S        |  5 ++---
 arch/arm/kernel/setup.c       | 11 ++++++++---
 arch/arm/mm/init.c            |  1 -
 arch/arm/mm/mmu.c             | 20 ++++++++++++++------
 arch/arm/mm/pv-fixup-asm.S    |  4 ++--
 6 files changed, 31 insertions(+), 15 deletions(-)

diff --git a/arch/arm/include/asm/memory.h b/arch/arm/include/asm/memory.h
index 99035b5891ef..ad9dcf4f751d 100644
--- a/arch/arm/include/asm/memory.h
+++ b/arch/arm/include/asm/memory.h
@@ -67,6 +67,10 @@
  */
 #define XIP_VIRT_ADDR(physaddr)  (MODULES_VADDR + ((physaddr) & 0x000fffff))
 
+#define FDT_FIXED_BASE		UL(0xff800000)
+#define FDT_FIXED_SIZE		(2 * PMD_SIZE)
+#define FDT_VIRT_ADDR(physaddr)	((void *)(FDT_FIXED_BASE | (physaddr) % PMD_SIZE))
+
 #if !defined(CONFIG_SMP) && !defined(CONFIG_ARM_LPAE)
 /*
  * Allow 16MB-aligned ioremap pages
@@ -107,6 +111,7 @@ extern unsigned long vectors_base;
 #define MODULES_VADDR		PAGE_OFFSET
 
 #define XIP_VIRT_ADDR(physaddr)  (physaddr)
+#define FDT_VIRT_ADDR(physaddr)  (physaddr)
 
 #endif /* !CONFIG_MMU */
 
diff --git a/arch/arm/kernel/head.S b/arch/arm/kernel/head.S
index f8904227e7fd..9b18d8c66129 100644
--- a/arch/arm/kernel/head.S
+++ b/arch/arm/kernel/head.S
@@ -275,9 +275,8 @@ __create_page_tables:
 	 */
 	mov	r0, r2, lsr #SECTION_SHIFT
 	movs	r0, r0, lsl #SECTION_SHIFT
-	subne	r3, r0, r8
-	addne	r3, r3, #PAGE_OFFSET
-	addne	r3, r4, r3, lsr #(SECTION_SHIFT - PMD_ORDER)
+	ldrne	r3, =FDT_FIXED_BASE >> (SECTION_SHIFT - PMD_ORDER)
+	addne	r3, r3, r4
 	orrne	r6, r7, r0
 	strne	r6, [r3], #1 << PMD_ORDER
 	addne	r6, r6, #1 << SECTION_SHIFT
diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
index 0ea0172d0c6d..bdba280a569d 100644
--- a/arch/arm/kernel/setup.c
+++ b/arch/arm/kernel/setup.c
@@ -18,6 +18,7 @@
 #include <linux/of_platform.h>
 #include <linux/init.h>
 #include <linux/kexec.h>
+#include <linux/libfdt.h>
 #include <linux/of_fdt.h>
 #include <linux/cpu.h>
 #include <linux/interrupt.h>
@@ -89,7 +90,6 @@ unsigned int cacheid __read_mostly;
 EXPORT_SYMBOL(cacheid);
 
 unsigned int __atags_pointer __initdata;
-void *atags_vaddr __initdata;
 
 unsigned int system_rev;
 EXPORT_SYMBOL(system_rev);
@@ -1077,13 +1077,18 @@ void __init hyp_mode_check(void)
 void __init setup_arch(char **cmdline_p)
 {
 	const struct machine_desc *mdesc = NULL;
+	void *atags_vaddr;
 
 	if (__atags_pointer)
-		atags_vaddr = phys_to_virt(__atags_pointer);
+		atags_vaddr = FDT_VIRT_ADDR(__atags_pointer);
 
 	setup_processor();
-	if (atags_vaddr)
+	if (atags_vaddr) {
 		mdesc = setup_machine_fdt(atags_vaddr);
+		if (mdesc)
+			memblock_reserve(__atags_pointer,
+					 fdt_totalsize(atags_vaddr));
+	}
 	if (!mdesc)
 		mdesc = setup_machine_tags(atags_vaddr, __machine_arch_type);
 	if (!mdesc) {
diff --git a/arch/arm/mm/init.c b/arch/arm/mm/init.c
index 000c1b48e973..652a87bd065c 100644
--- a/arch/arm/mm/init.c
+++ b/arch/arm/mm/init.c
@@ -223,7 +223,6 @@ void __init arm_memblock_init(const struct machine_desc *mdesc)
 	if (mdesc->reserve)
 		mdesc->reserve();
 
-	early_init_fdt_reserve_self();
 	early_init_fdt_scan_reserved_mem();
 
 	/* reserve memory for DMA contiguous allocations */
diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
index 0665a0dbd040..59fe24eb23c6 100644
--- a/arch/arm/mm/mmu.c
+++ b/arch/arm/mm/mmu.c
@@ -40,6 +40,8 @@
 #include "mm.h"
 #include "tcm.h"
 
+extern unsigned long __atags_pointer;
+
 /*
  * empty_zero_page is a special page that is used for
  * zero-initialized data and COW.
@@ -947,7 +949,7 @@ static void __init create_mapping(struct map_desc *md)
 		return;
 	}
 
-	if ((md->type == MT_DEVICE || md->type == MT_ROM) &&
+	if (md->type == MT_DEVICE &&
 	    md->virtual >= PAGE_OFFSET && md->virtual < FIXADDR_START &&
 	    (md->virtual < VMALLOC_START || md->virtual >= VMALLOC_END)) {
 		pr_warn("BUG: mapping for 0x%08llx at 0x%08lx out of vmalloc space\n",
@@ -1343,6 +1345,15 @@ static void __init devicemaps_init(const struct machine_desc *mdesc)
 	for (addr = VMALLOC_START; addr < (FIXADDR_TOP & PMD_MASK); addr += PMD_SIZE)
 		pmd_clear(pmd_off_k(addr));
 
+	if (__atags_pointer) {
+		/* create a read-only mapping of the device tree or ATAGS data */
+		map.pfn = __phys_to_pfn(__atags_pointer & SECTION_MASK);
+		map.virtual = FDT_FIXED_BASE;
+		map.length = FDT_FIXED_SIZE;
+		map.type = MT_ROM;
+		create_mapping(&map);
+	}
+
 	/*
 	 * Map the kernel if it is XIP.
 	 * It is always first in the modulearea.
@@ -1503,8 +1514,7 @@ static void __init map_lowmem(void)
 }
 
 #ifdef CONFIG_ARM_PV_FIXUP
-extern void *atags_vaddr;
-typedef void pgtables_remap(long long offset, unsigned long pgd, void *bdata);
+typedef void pgtables_remap(long long offset, unsigned long pgd);
 pgtables_remap lpae_pgtables_remap_asm;
 
 /*
@@ -1517,7 +1527,6 @@ static void __init early_paging_init(const struct machine_desc *mdesc)
 	unsigned long pa_pgd;
 	unsigned int cr, ttbcr;
 	long long offset;
-	void *boot_data;
 
 	if (!mdesc->pv_fixup)
 		return;
@@ -1534,7 +1543,6 @@ static void __init early_paging_init(const struct machine_desc *mdesc)
 	 */
 	lpae_pgtables_remap = (pgtables_remap *)(unsigned long)__pa(lpae_pgtables_remap_asm);
 	pa_pgd = __pa(swapper_pg_dir);
-	boot_data = atags_vaddr;
 	barrier();
 
 	pr_info("Switching physical address space to 0x%08llx\n",
@@ -1570,7 +1578,7 @@ static void __init early_paging_init(const struct machine_desc *mdesc)
 	 * needs to be assembly.  It's fairly simple, as we're using the
 	 * temporary tables setup by the initial assembly code.
 	 */
-	lpae_pgtables_remap(offset, pa_pgd, boot_data);
+	lpae_pgtables_remap(offset, pa_pgd);
 
 	/* Re-enable the caches and cacheable TLB walks */
 	asm volatile("mcr p15, 0, %0, c2, c0, 2" : : "r" (ttbcr));
diff --git a/arch/arm/mm/pv-fixup-asm.S b/arch/arm/mm/pv-fixup-asm.S
index 8eade0416739..5c5e1952000a 100644
--- a/arch/arm/mm/pv-fixup-asm.S
+++ b/arch/arm/mm/pv-fixup-asm.S
@@ -39,8 +39,8 @@ ENTRY(lpae_pgtables_remap_asm)
 
 	/* Update level 2 entries for the boot data */
 	add	r7, r2, #0x1000
-	add	r7, r7, r3, lsr #SECTION_SHIFT - L2_ORDER
-	bic	r7, r7, #(1 << L2_ORDER) - 1
+	movw	r3, #FDT_FIXED_BASE >> (SECTION_SHIFT - L2_ORDER)
+	add	r7, r7, r3
 	ldrd	r4, r5, [r7]
 	adds	r4, r4, r0
 	adc	r5, r5, r1
-- 
2.17.1


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

Re: [PATCH 0/2] ARM: move FDT mapping out of linear region

[Linus Walleij]

On Tue, Oct 6, 2020 at 11:49 AM Ard Biesheuvel <ardb@kernel.org> wrote:

> For the ARM kernel, setting up the kernel's virtual address space at boot
> is tricky, given the risk of collision between 1:1 mapped regions and
> virtually remapped regions. There is also a concern regarding the exact
> mapping attributes that are needed for each region, which differs between
> ARM architecture revisions, UP vs SMP etc
>
> For this reason, the kernel VA space is set up in two stages: at early
> boot, only the kernel itself and the DT are mapped using section mappings
> and later on, all existing mappings are torn down except the one covering
> the first memblock covering the kernel, and remapped again using the full
> fledged mapping routines that can map at page granularity and use all the
> right mapping attributes.
>
> There are cases where this may result in the DT getting unmapped at
> this point, and not remapped again. For instance, if the first memblock
> is not PMD aligned, we will align it up, and mark the memory below it
> as MEMBLOCK_NOMAP, to avoid having to allocate page tables to create
> the mapping before we have a mapped memblock to allocate from. If the
> DT happens to reside in this region, it will not be mapped at all when
> the permanent kernel VA mappings are in place, resulting in crashes.
>
> As we happen to have a 4 MB hole in the kernel VA space (between the
> end of the VMALLOC space and the start of the FIXMAP region), let's
> use it to create a permanent, read-only mapping of the DT that is not
> affected by any such issues.
>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Florian Fainelli <f.fainelli@gmail.com>
> Cc: Russell King <linux@armlinux.org.uk>
> Cc: Nicolas Pitre <nico@fluxnic.net>
> Cc: Rob Herring <robh+dt@kernel.org>

Hats off, this works perfectly. I tossed these patches under my KASan
stack and it boots without any problems on the Qualcomm APQ8060
with its peculiar memory layout.

Reviewed-by/Tested-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

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

Re: [PATCH 2/2] ARM: move device tree mapping out of linear region

[Linus Walleij]

On Tue, Oct 6, 2020 at 11:49 AM Ard Biesheuvel <ardb@kernel.org> wrote:

> On ARM, setting up the linear region is tricky, given the constraints
> around placement and alignment of the memblocks, and how the kernel
> itself as well as the DT are placed in physical memory.
>
> Let's simplify matters a bit, by moving the device tree mapping to the
> top of the address space, right below the fixmap region, and create a
> read-only mapping for it that is independent of the size of the linear
> region, and how it is organized.
>
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>

Looks good to me as stated on patch 0/2

Should we also patch Documentation/arm/memory.rst to reflect this?

Yours,
Linus Walleij

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

Re: [PATCH 2/2] ARM: move device tree mapping out of linear region

[Nicolas Pitre]

On Tue, 6 Oct 2020, Ard Biesheuvel wrote:

> On ARM, setting up the linear region is tricky, given the constraints
> around placement and alignment of the memblocks, and how the kernel
> itself as well as the DT are placed in physical memory.
> 
> Let's simplify matters a bit, by moving the device tree mapping to the
> top of the address space, right below the fixmap region, and create a
> read-only mapping for it that is independent of the size of the linear
> region, and how it is organized.

You say this is below the fixmap region but it is also worth mentioning 
it is right above the vmalloc region, and in doing so we lose the 
vmalloc access overflow guard area.

Also you should document your static mapping usage in 
Documentation/arm/memory.rst.


Nicolas

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

Re: [PATCH 2/2] ARM: move device tree mapping out of linear region

[Ard Biesheuvel]

On Tue, 6 Oct 2020 at 17:59, Nicolas Pitre <nico@fluxnic.net> wrote:
>
> On Tue, 6 Oct 2020, Ard Biesheuvel wrote:
>
> > On ARM, setting up the linear region is tricky, given the constraints
> > around placement and alignment of the memblocks, and how the kernel
> > itself as well as the DT are placed in physical memory.
> >
> > Let's simplify matters a bit, by moving the device tree mapping to the
> > top of the address space, right below the fixmap region, and create a
> > read-only mapping for it that is independent of the size of the linear
> > region, and how it is organized.
>
> You say this is below the fixmap region but it is also worth mentioning
> it is right above the vmalloc region, and in doing so we lose the
> vmalloc access overflow guard area.
>

Not entirely,
- in the !LPAE case, we only use 2 out of 4 MB,
- the mapping is read-only, so any stray writes will still be caught.

But I take your point: given the de facto constraints imposed by the
!LPAE case, i.e., that a DT can be at most 1 MB in the general case,
or up to 2 MB but only if it is 1 MB aligned in memory, we could make
the actual size of the permanent mapping dependent on the actual size
of the DT blob, in which case we should be able to preserve 2-3 MB of
the guard region.

> Also you should document your static mapping usage in
> Documentation/arm/memory.rst.
>

Ack. Linus mentioned that too.

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

Re: [PATCH 2/2] ARM: move device tree mapping out of linear region

[Nicolas Pitre]

On Tue, 6 Oct 2020, Ard Biesheuvel wrote:

> On Tue, 6 Oct 2020 at 17:59, Nicolas Pitre <nico@fluxnic.net> wrote:
> >
> > On Tue, 6 Oct 2020, Ard Biesheuvel wrote:
> >
> > > On ARM, setting up the linear region is tricky, given the constraints
> > > around placement and alignment of the memblocks, and how the kernel
> > > itself as well as the DT are placed in physical memory.
> > >
> > > Let's simplify matters a bit, by moving the device tree mapping to the
> > > top of the address space, right below the fixmap region, and create a
> > > read-only mapping for it that is independent of the size of the linear
> > > region, and how it is organized.
> >
> > You say this is below the fixmap region but it is also worth mentioning
> > it is right above the vmalloc region, and in doing so we lose the
> > vmalloc access overflow guard area.
> >
> 
> Not entirely,
> - in the !LPAE case, we only use 2 out of 4 MB,
> - the mapping is read-only, so any stray writes will still be caught.
> 
> But I take your point: given the de facto constraints imposed by the
> !LPAE case, i.e., that a DT can be at most 1 MB in the general case,
> or up to 2 MB but only if it is 1 MB aligned in memory, we could make
> the actual size of the permanent mapping dependent on the actual size
> of the DT blob, in which case we should be able to preserve 2-3 MB of
> the guard region.

What about a DT larger than 2MB? Weren't people at some point who 
thought of e.g. stuffing a ramdisk image in there?

This is unfortunate that the DT is needed early, otherwise it would have 
been nicer to simply put it in the vmalloc area directly.


Nicolas

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

Re: [PATCH 2/2] ARM: move device tree mapping out of linear region

[Linus Walleij]

On Tue, Oct 6, 2020 at 6:46 PM Nicolas Pitre <nico@fluxnic.net> wrote:

> This is unfortunate that the DT is needed early, otherwise it would have
> been nicer to simply put it in the vmalloc area directly.

We can issue unflatten_and_copy_device_tree() like most arches
do and then remove the mapping and memblock after that, but then
we also need a remove_mapping() call that is counterpart to
create_mapping() to get rid of this early map.

But that can be a separate project (if desirable).

Yours,
Linus Walleij

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

Re: [PATCH 2/2] ARM: move device tree mapping out of linear region

[Ard Biesheuvel]

On Wed, 7 Oct 2020 at 11:56, Linus Walleij <linus.walleij@linaro.org> wrote:
>
> On Tue, Oct 6, 2020 at 6:46 PM Nicolas Pitre <nico@fluxnic.net> wrote:
>
> > This is unfortunate that the DT is needed early, otherwise it would have
> > been nicer to simply put it in the vmalloc area directly.
>
> We can issue unflatten_and_copy_device_tree() like most arches
> do and then remove the mapping and memblock after that, but then
> we also need a remove_mapping() call that is counterpart to
> create_mapping() to get rid of this early map.
>
> But that can be a separate project (if desirable).
>

I still don't get what the advantage is of doing that. With this
series applied, the DT could be anywhere in the 32-bit addressable
physical memory, and I intend to update the EFI code for ARM to take
advantage of that (currently, it has to reason about how large the
linear region might be, which is annoying).

unflatten_and_copy_device_tree() reallocates the DT into lowmem, and
then unflattens it. So if the DT was in lowmem, you release some
lowmem at one end, and allocate it at another. If the DT was not in
lowmem to begin with, you waste lowmem for something that does not
need to be in lowmem in the first place.

I know none of this is likely to make a huge difference, given that
DTBs are typically small. However, I'd like to understand the
rationale for doing the copy, beyond 'other architectures do it'
(which does not include arm64 btw)

-- 
Ard.

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