* [PATCH v2 0/2] ARM: move FDT mapping out of linear region
@ 2020-10-07 8:39 Ard Biesheuvel
2020-10-07 8:39 ` [PATCH v2 1/2] ARM: centralize phys-to-virt conversion of DT/ATAGS address Ard Biesheuvel
` (2 more replies)
0 siblings, 3 replies; 37+ messages in thread
From: Ard Biesheuvel @ 2020-10-07 8:39 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Florian Fainelli, Nicolas Pitre, Linus Walleij, Russell King,
Rob Herring, 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.
Changes since v1:
- bump the start of the fixmap region by 512 KB so we still have a full
guard region even on LPAE builds, which will use the entire 4 MB window
for the read-only mapping of the DT
- update Documentation/arm/memory.rst accordingly
- add Linus's ack
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
Documentation/arm/memory.rst | 7 ++++++-
arch/arm/include/asm/fixmap.h | 2 +-
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 ++--
12 files changed, 54 insertions(+), 29 deletions(-)
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH v2 1/2] ARM: centralize phys-to-virt conversion of DT/ATAGS address
2020-10-07 8:39 [PATCH v2 0/2] ARM: move FDT mapping out of linear region Ard Biesheuvel
@ 2020-10-07 8:39 ` Ard Biesheuvel
2020-10-07 15:28 ` Nicolas Pitre
2020-10-07 8:39 ` [PATCH v2 2/2] ARM: move device tree mapping out of linear region Ard Biesheuvel
2020-10-11 16:39 ` [PATCH v2 0/2] ARM: move FDT " Ard Biesheuvel
2 siblings, 1 reply; 37+ messages in thread
From: Ard Biesheuvel @ 2020-10-07 8:39 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Florian Fainelli, Nicolas Pitre, Linus Walleij, Russell King,
Rob Herring, 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.
Tested-by: Linus Walleij <linus.walleij@linaro.org>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
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 5b1dcf1ad042..58e56e23a19a 100644
--- a/arch/arm/kernel/setup.c
+++ b/arch/arm/kernel/setup.c
@@ -90,6 +90,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);
@@ -1076,19 +1077,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
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH v2 2/2] ARM: move device tree mapping out of linear region
2020-10-07 8:39 [PATCH v2 0/2] ARM: move FDT mapping out of linear region Ard Biesheuvel
2020-10-07 8:39 ` [PATCH v2 1/2] ARM: centralize phys-to-virt conversion of DT/ATAGS address Ard Biesheuvel
@ 2020-10-07 8:39 ` Ard Biesheuvel
2020-10-07 15:28 ` Nicolas Pitre
[not found] ` <CGME20201028091912eucas1p13fb9cd947faa6bfd79199ea79648b6af@eucas1p1.samsung.com>
2020-10-11 16:39 ` [PATCH v2 0/2] ARM: move FDT " Ard Biesheuvel
2 siblings, 2 replies; 37+ messages in thread
From: Ard Biesheuvel @ 2020-10-07 8:39 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Florian Fainelli, Nicolas Pitre, Linus Walleij, Russell King,
Rob Herring, 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 between the end of the vmalloc region
and the start of the 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.
Since this region was formerly used as a guard region, which will now be
populated fully on LPAE builds by this read-only mapping (which will
still be able to function as a guard region for stray writes), bump the
start of the [underutilized] fixmap region by 512 KB as well, to ensure
that there is always a proper guard region here. Doing so still leaves
ample room for the fixmap space, even with NR_CPUS set to its maximum
value of 32.
Tested-by: Linus Walleij <linus.walleij@linaro.org>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
Documentation/arm/memory.rst | 7 ++++++-
arch/arm/include/asm/fixmap.h | 2 +-
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 ++--
8 files changed, 38 insertions(+), 17 deletions(-)
diff --git a/Documentation/arm/memory.rst b/Documentation/arm/memory.rst
index 0521b4ce5c96..34bb23c44a71 100644
--- a/Documentation/arm/memory.rst
+++ b/Documentation/arm/memory.rst
@@ -45,9 +45,14 @@ fffe8000 fffeffff DTCM mapping area for platforms with
fffe0000 fffe7fff ITCM mapping area for platforms with
ITCM mounted inside the CPU.
-ffc00000 ffefffff Fixmap mapping region. Addresses provided
+ffc80000 ffefffff Fixmap mapping region. Addresses provided
by fix_to_virt() will be located here.
+ffc00000 ffc7ffff Guard region
+
+ff800000 ffbfffff Permanent, fixed read-only mapping of the
+ firmware provided DT blob
+
fee00000 feffffff Mapping of PCI I/O space. This is a static
mapping within the vmalloc space.
diff --git a/arch/arm/include/asm/fixmap.h b/arch/arm/include/asm/fixmap.h
index fc56fc3e1931..9575b404019c 100644
--- a/arch/arm/include/asm/fixmap.h
+++ b/arch/arm/include/asm/fixmap.h
@@ -2,7 +2,7 @@
#ifndef _ASM_FIXMAP_H
#define _ASM_FIXMAP_H
-#define FIXADDR_START 0xffc00000UL
+#define FIXADDR_START 0xffc80000UL
#define FIXADDR_END 0xfff00000UL
#define FIXADDR_TOP (FIXADDR_END - PAGE_SIZE)
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 58e56e23a19a..c08574cad748 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>
@@ -90,7 +91,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);
@@ -1078,13 +1078,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..a7231d151c63 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 */
+ 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
^ permalink raw reply related [flat|nested] 37+ messages in thread
* Re: [PATCH v2 2/2] ARM: move device tree mapping out of linear region
2020-10-07 8:39 ` [PATCH v2 2/2] ARM: move device tree mapping out of linear region Ard Biesheuvel
@ 2020-10-07 15:28 ` Nicolas Pitre
2020-10-07 15:32 ` Ard Biesheuvel
2020-10-07 21:32 ` Russell King - ARM Linux admin
[not found] ` <CGME20201028091912eucas1p13fb9cd947faa6bfd79199ea79648b6af@eucas1p1.samsung.com>
1 sibling, 2 replies; 37+ messages in thread
From: Nicolas Pitre @ 2020-10-07 15:28 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Rob Herring, Linus Walleij, Florian Fainelli, Russell King,
linux-arm-kernel
On Wed, 7 Oct 2020, Ard Biesheuvel wrote:
> @@ -1078,13 +1078,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 __atags_pointer is zero, you'll end up with atags_vaddr being
undefined here. I'm surprised the compiler didn't warn about that.
> @@ -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",
Hmmm... I wonder why MT_ROM was there in the first place.
Digging into the history of this particular test (which has seen many
slight variations over the years and could probably be simplified
further nowdays) I finally found where that MT_ROM condition was
introduced:
https://git.kernel.org/pub/scm/linux/kernel/git/history/history.git/commit/?id=457450ab3b94
Oh well...
Nicolas
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v2 1/2] ARM: centralize phys-to-virt conversion of DT/ATAGS address
2020-10-07 8:39 ` [PATCH v2 1/2] ARM: centralize phys-to-virt conversion of DT/ATAGS address Ard Biesheuvel
@ 2020-10-07 15:28 ` Nicolas Pitre
0 siblings, 0 replies; 37+ messages in thread
From: Nicolas Pitre @ 2020-10-07 15:28 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Rob Herring, Linus Walleij, Florian Fainelli, Russell King,
linux-arm-kernel
On Wed, 7 Oct 2020, Ard Biesheuvel wrote:
> 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.
>
> Tested-by: Linus Walleij <linus.walleij@linaro.org>
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
Acked-by: Nicolas Pitre <nico@fluxnic.net>
> ---
> 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 5b1dcf1ad042..58e56e23a19a 100644
> --- a/arch/arm/kernel/setup.c
> +++ b/arch/arm/kernel/setup.c
> @@ -90,6 +90,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);
> @@ -1076,19 +1077,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
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v2 2/2] ARM: move device tree mapping out of linear region
2020-10-07 15:28 ` Nicolas Pitre
@ 2020-10-07 15:32 ` Ard Biesheuvel
2020-10-07 16:38 ` Nicolas Pitre
2020-10-07 16:40 ` Nicolas Pitre
2020-10-07 21:32 ` Russell King - ARM Linux admin
1 sibling, 2 replies; 37+ messages in thread
From: Ard Biesheuvel @ 2020-10-07 15:32 UTC (permalink / raw)
To: Nicolas Pitre
Cc: Rob Herring, Linus Walleij, Florian Fainelli, Russell King, Linux ARM
On Wed, 7 Oct 2020 at 17:28, Nicolas Pitre <nico@fluxnic.net> wrote:
>
> On Wed, 7 Oct 2020, Ard Biesheuvel wrote:
>
> > @@ -1078,13 +1078,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 __atags_pointer is zero, you'll end up with atags_vaddr being
> undefined here. I'm surprised the compiler didn't warn about that.
>
Ah yes, well spotted. I'll add a NULL initializer.
> > @@ -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",
>
> Hmmm... I wonder why MT_ROM was there in the first place.
>
> Digging into the history of this particular test (which has seen many
> slight variations over the years and could probably be simplified
> further nowdays) I finally found where that MT_ROM condition was
> introduced:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/history/history.git/commit/?id=457450ab3b94
>
> Oh well...
>
I didn't dig quite that far, but I did notice that the test predates
get Git era.
My reasoning to remove it was that MT_ROM is only used locally in the
same source file, whereas MT_DEVICE is used all over the place by
machine files, and so MT_DEVICE obviously requires this kind of
policing to ensure that the static device mappings do not overlap. The
XIP kernel mapping and now the DTB mapping were carefully placed in
the memory map, and so MT_ROM regions can be disregarded here.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v2 2/2] ARM: move device tree mapping out of linear region
2020-10-07 15:32 ` Ard Biesheuvel
@ 2020-10-07 16:38 ` Nicolas Pitre
2020-10-07 16:40 ` Nicolas Pitre
1 sibling, 0 replies; 37+ messages in thread
From: Nicolas Pitre @ 2020-10-07 16:38 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Rob Herring, Linus Walleij, Florian Fainelli, Russell King, Linux ARM
On Wed, 7 Oct 2020, Ard Biesheuvel wrote:
> On Wed, 7 Oct 2020 at 17:28, Nicolas Pitre <nico@fluxnic.net> wrote:
> >
> > On Wed, 7 Oct 2020, Ard Biesheuvel wrote:
> >
> > > @@ -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",
> >
> > Hmmm... I wonder why MT_ROM was there in the first place.
> >
> > Digging into the history of this particular test (which has seen many
> > slight variations over the years and could probably be simplified
> > further nowdays) I finally found where that MT_ROM condition was
> > introduced:
> >
> > https://git.kernel.org/pub/scm/linux/kernel/git/history/history.git/commit/?id=457450ab3b94
> >
> > Oh well...
> >
>
> I didn't dig quite that far, but I did notice that the test predates
> get Git era.
I was just wondering who added that useless condition given that the
only MT_ROM usage before your patch was for the XIP mapping of the
kernel which is always located in the module area i.e. below
PAGE_OFFSET.
Nicolas
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v2 2/2] ARM: move device tree mapping out of linear region
2020-10-07 15:32 ` Ard Biesheuvel
2020-10-07 16:38 ` Nicolas Pitre
@ 2020-10-07 16:40 ` Nicolas Pitre
1 sibling, 0 replies; 37+ messages in thread
From: Nicolas Pitre @ 2020-10-07 16:40 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Rob Herring, Linus Walleij, Florian Fainelli, Russell King, Linux ARM
On Wed, 7 Oct 2020, Ard Biesheuvel wrote:
> On Wed, 7 Oct 2020 at 17:28, Nicolas Pitre <nico@fluxnic.net> wrote:
> >
> > On Wed, 7 Oct 2020, Ard Biesheuvel wrote:
> >
> > > @@ -1078,13 +1078,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 __atags_pointer is zero, you'll end up with atags_vaddr being
> > undefined here. I'm surprised the compiler didn't warn about that.
> >
>
> Ah yes, well spotted. I'll add a NULL initializer.
OK. Then you may add
Reviewed-by: Nicolas Pitre <nico@fluxnic.net>
Nicolas
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v2 2/2] ARM: move device tree mapping out of linear region
2020-10-07 15:28 ` Nicolas Pitre
2020-10-07 15:32 ` Ard Biesheuvel
@ 2020-10-07 21:32 ` Russell King - ARM Linux admin
2020-10-07 22:05 ` Nicolas Pitre
1 sibling, 1 reply; 37+ messages in thread
From: Russell King - ARM Linux admin @ 2020-10-07 21:32 UTC (permalink / raw)
To: Nicolas Pitre
Cc: Rob Herring, Linus Walleij, Florian Fainelli, Ard Biesheuvel,
linux-arm-kernel
On Wed, Oct 07, 2020 at 11:28:03AM -0400, Nicolas Pitre wrote:
> On Wed, 7 Oct 2020, Ard Biesheuvel wrote:
>
> > @@ -1078,13 +1078,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 __atags_pointer is zero, you'll end up with atags_vaddr being
> undefined here. I'm surprised the compiler didn't warn about that.
The warning has been disabled. See commit
78a5255ffb6a1af189a83e493d916ba1c54d8c75.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v2 2/2] ARM: move device tree mapping out of linear region
2020-10-07 21:32 ` Russell King - ARM Linux admin
@ 2020-10-07 22:05 ` Nicolas Pitre
0 siblings, 0 replies; 37+ messages in thread
From: Nicolas Pitre @ 2020-10-07 22:05 UTC (permalink / raw)
To: Russell King - ARM Linux admin
Cc: Rob Herring, Linus Walleij, Florian Fainelli, Ard Biesheuvel,
linux-arm-kernel
On Wed, 7 Oct 2020, Russell King - ARM Linux admin wrote:
> On Wed, Oct 07, 2020 at 11:28:03AM -0400, Nicolas Pitre wrote:
> > On Wed, 7 Oct 2020, Ard Biesheuvel wrote:
> >
> > > @@ -1078,13 +1078,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 __atags_pointer is zero, you'll end up with atags_vaddr being
> > undefined here. I'm surprised the compiler didn't warn about that.
>
> The warning has been disabled. See commit
> 78a5255ffb6a1af189a83e493d916ba1c54d8c75.
Hmmm... that's nasty. I guess we'll have to get into the habit of
test-compiling our patches with W=2 then, at least for catching the
obvious ones like this case. Someone should automate that into
checkpatch or the like.
Nicolas
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v2 0/2] ARM: move FDT mapping out of linear region
2020-10-07 8:39 [PATCH v2 0/2] ARM: move FDT mapping out of linear region Ard Biesheuvel
2020-10-07 8:39 ` [PATCH v2 1/2] ARM: centralize phys-to-virt conversion of DT/ATAGS address Ard Biesheuvel
2020-10-07 8:39 ` [PATCH v2 2/2] ARM: move device tree mapping out of linear region Ard Biesheuvel
@ 2020-10-11 16:39 ` Ard Biesheuvel
2 siblings, 0 replies; 37+ messages in thread
From: Ard Biesheuvel @ 2020-10-11 16:39 UTC (permalink / raw)
To: Linux ARM
Cc: Linus Walleij, Florian Fainelli, Russell King, Rob Herring,
Nicolas Pitre
On Wed, 7 Oct 2020 at 10:39, 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.
>
> Changes since v1:
> - bump the start of the fixmap region by 512 KB so we still have a full
> guard region even on LPAE builds, which will use the entire 4 MB window
> for the read-only mapping of the DT
> - update Documentation/arm/memory.rst accordingly
> - add Linus's ack
>
> 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
>
I've dropped these into Russell's patch tracker.
Thanks all,
> Documentation/arm/memory.rst | 7 ++++++-
> arch/arm/include/asm/fixmap.h | 2 +-
> 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 ++--
> 12 files changed, 54 insertions(+), 29 deletions(-)
>
> --
> 2.17.1
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v2 2/2] ARM: move device tree mapping out of linear region
[not found] ` <CGME20201028091912eucas1p13fb9cd947faa6bfd79199ea79648b6af@eucas1p1.samsung.com>
@ 2020-10-28 9:19 ` Marek Szyprowski
0 siblings, 0 replies; 37+ messages in thread
From: Marek Szyprowski @ 2020-10-28 9:19 UTC (permalink / raw)
To: Ard Biesheuvel, linux-arm-kernel, 'Linux Samsung SOC',
Krzysztof Kozlowski
Cc: Florian Fainelli, Nicolas Pitre, Linus Walleij, Russell King,
Rob Herring
Hi,
On 07.10.2020 10:39, 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 between the end of the vmalloc region
> and the start of the 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.
>
> Since this region was formerly used as a guard region, which will now be
> populated fully on LPAE builds by this read-only mapping (which will
> still be able to function as a guard region for stray writes), bump the
> start of the [underutilized] fixmap region by 512 KB as well, to ensure
> that there is always a proper guard region here. Doing so still leaves
> ample room for the fixmap space, even with NR_CPUS set to its maximum
> value of 32.
>
> Tested-by: Linus Walleij <linus.walleij@linaro.org>
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
This patch landed in linux-next 20201028 as commit 7a1be318f579 ("ARM:
9012/1: move device tree mapping out of linear region"). Sadly it broke
booting almost all Samsung Exynos-based boards. The only one which
booted, used an appended device tree. I can provide more information if
needed, just let me know what to check. "Starting kernel ..." is the
last message I see here. No output from earlycon.
> ---
> Documentation/arm/memory.rst | 7 ++++++-
> arch/arm/include/asm/fixmap.h | 2 +-
> 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 ++--
> 8 files changed, 38 insertions(+), 17 deletions(-)
>
> diff --git a/Documentation/arm/memory.rst b/Documentation/arm/memory.rst
> index 0521b4ce5c96..34bb23c44a71 100644
> --- a/Documentation/arm/memory.rst
> +++ b/Documentation/arm/memory.rst
> @@ -45,9 +45,14 @@ fffe8000 fffeffff DTCM mapping area for platforms with
> fffe0000 fffe7fff ITCM mapping area for platforms with
> ITCM mounted inside the CPU.
>
> -ffc00000 ffefffff Fixmap mapping region. Addresses provided
> +ffc80000 ffefffff Fixmap mapping region. Addresses provided
> by fix_to_virt() will be located here.
>
> +ffc00000 ffc7ffff Guard region
> +
> +ff800000 ffbfffff Permanent, fixed read-only mapping of the
> + firmware provided DT blob
> +
> fee00000 feffffff Mapping of PCI I/O space. This is a static
> mapping within the vmalloc space.
>
> diff --git a/arch/arm/include/asm/fixmap.h b/arch/arm/include/asm/fixmap.h
> index fc56fc3e1931..9575b404019c 100644
> --- a/arch/arm/include/asm/fixmap.h
> +++ b/arch/arm/include/asm/fixmap.h
> @@ -2,7 +2,7 @@
> #ifndef _ASM_FIXMAP_H
> #define _ASM_FIXMAP_H
>
> -#define FIXADDR_START 0xffc00000UL
> +#define FIXADDR_START 0xffc80000UL
> #define FIXADDR_END 0xfff00000UL
> #define FIXADDR_TOP (FIXADDR_END - PAGE_SIZE)
>
> 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 58e56e23a19a..c08574cad748 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>
> @@ -90,7 +91,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);
> @@ -1078,13 +1078,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..a7231d151c63 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 */
> + 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
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v2 2/2] ARM: move device tree mapping out of linear region
@ 2020-10-28 9:19 ` Marek Szyprowski
0 siblings, 0 replies; 37+ messages in thread
From: Marek Szyprowski @ 2020-10-28 9:19 UTC (permalink / raw)
To: Ard Biesheuvel, linux-arm-kernel, 'Linux Samsung SOC',
Krzysztof Kozlowski
Cc: Linus Walleij, Florian Fainelli, Russell King, Rob Herring,
Nicolas Pitre
Hi,
On 07.10.2020 10:39, 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 between the end of the vmalloc region
> and the start of the 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.
>
> Since this region was formerly used as a guard region, which will now be
> populated fully on LPAE builds by this read-only mapping (which will
> still be able to function as a guard region for stray writes), bump the
> start of the [underutilized] fixmap region by 512 KB as well, to ensure
> that there is always a proper guard region here. Doing so still leaves
> ample room for the fixmap space, even with NR_CPUS set to its maximum
> value of 32.
>
> Tested-by: Linus Walleij <linus.walleij@linaro.org>
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
This patch landed in linux-next 20201028 as commit 7a1be318f579 ("ARM:
9012/1: move device tree mapping out of linear region"). Sadly it broke
booting almost all Samsung Exynos-based boards. The only one which
booted, used an appended device tree. I can provide more information if
needed, just let me know what to check. "Starting kernel ..." is the
last message I see here. No output from earlycon.
> ---
> Documentation/arm/memory.rst | 7 ++++++-
> arch/arm/include/asm/fixmap.h | 2 +-
> 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 ++--
> 8 files changed, 38 insertions(+), 17 deletions(-)
>
> diff --git a/Documentation/arm/memory.rst b/Documentation/arm/memory.rst
> index 0521b4ce5c96..34bb23c44a71 100644
> --- a/Documentation/arm/memory.rst
> +++ b/Documentation/arm/memory.rst
> @@ -45,9 +45,14 @@ fffe8000 fffeffff DTCM mapping area for platforms with
> fffe0000 fffe7fff ITCM mapping area for platforms with
> ITCM mounted inside the CPU.
>
> -ffc00000 ffefffff Fixmap mapping region. Addresses provided
> +ffc80000 ffefffff Fixmap mapping region. Addresses provided
> by fix_to_virt() will be located here.
>
> +ffc00000 ffc7ffff Guard region
> +
> +ff800000 ffbfffff Permanent, fixed read-only mapping of the
> + firmware provided DT blob
> +
> fee00000 feffffff Mapping of PCI I/O space. This is a static
> mapping within the vmalloc space.
>
> diff --git a/arch/arm/include/asm/fixmap.h b/arch/arm/include/asm/fixmap.h
> index fc56fc3e1931..9575b404019c 100644
> --- a/arch/arm/include/asm/fixmap.h
> +++ b/arch/arm/include/asm/fixmap.h
> @@ -2,7 +2,7 @@
> #ifndef _ASM_FIXMAP_H
> #define _ASM_FIXMAP_H
>
> -#define FIXADDR_START 0xffc00000UL
> +#define FIXADDR_START 0xffc80000UL
> #define FIXADDR_END 0xfff00000UL
> #define FIXADDR_TOP (FIXADDR_END - PAGE_SIZE)
>
> 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 58e56e23a19a..c08574cad748 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>
> @@ -90,7 +91,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);
> @@ -1078,13 +1078,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..a7231d151c63 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 */
> + 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
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v2 2/2] ARM: move device tree mapping out of linear region
2020-10-28 9:19 ` Marek Szyprowski
@ 2020-10-28 9:22 ` Ard Biesheuvel
-1 siblings, 0 replies; 37+ messages in thread
From: Ard Biesheuvel @ 2020-10-28 9:22 UTC (permalink / raw)
To: Marek Szyprowski
Cc: Linux ARM, Linux Samsung SOC, Krzysztof Kozlowski,
Florian Fainelli, Nicolas Pitre, Linus Walleij, Russell King,
Rob Herring
On Wed, 28 Oct 2020 at 10:19, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
>
> Hi,
>
> On 07.10.2020 10:39, 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 between the end of the vmalloc region
> > and the start of the 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.
> >
> > Since this region was formerly used as a guard region, which will now be
> > populated fully on LPAE builds by this read-only mapping (which will
> > still be able to function as a guard region for stray writes), bump the
> > start of the [underutilized] fixmap region by 512 KB as well, to ensure
> > that there is always a proper guard region here. Doing so still leaves
> > ample room for the fixmap space, even with NR_CPUS set to its maximum
> > value of 32.
> >
> > Tested-by: Linus Walleij <linus.walleij@linaro.org>
> > Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
>
> This patch landed in linux-next 20201028 as commit 7a1be318f579 ("ARM:
> 9012/1: move device tree mapping out of linear region"). Sadly it broke
> booting almost all Samsung Exynos-based boards. The only one which
> booted, used an appended device tree. I can provide more information if
> needed, just let me know what to check. "Starting kernel ..." is the
> last message I see here. No output from earlycon.
>
Thanks for the report. I will have a look later today.
Do these platforms happen to have any static device mappings that may
collide with this mapping of the FDT? Also, could this be related to
device drivers making changes in memory to the FDT image? Because the
permanent mapping of the FDT is read-only now.
> > ---
> > Documentation/arm/memory.rst | 7 ++++++-
> > arch/arm/include/asm/fixmap.h | 2 +-
> > 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 ++--
> > 8 files changed, 38 insertions(+), 17 deletions(-)
> >
> > diff --git a/Documentation/arm/memory.rst b/Documentation/arm/memory.rst
> > index 0521b4ce5c96..34bb23c44a71 100644
> > --- a/Documentation/arm/memory.rst
> > +++ b/Documentation/arm/memory.rst
> > @@ -45,9 +45,14 @@ fffe8000 fffeffff DTCM mapping area for platforms with
> > fffe0000 fffe7fff ITCM mapping area for platforms with
> > ITCM mounted inside the CPU.
> >
> > -ffc00000 ffefffff Fixmap mapping region. Addresses provided
> > +ffc80000 ffefffff Fixmap mapping region. Addresses provided
> > by fix_to_virt() will be located here.
> >
> > +ffc00000 ffc7ffff Guard region
> > +
> > +ff800000 ffbfffff Permanent, fixed read-only mapping of the
> > + firmware provided DT blob
> > +
> > fee00000 feffffff Mapping of PCI I/O space. This is a static
> > mapping within the vmalloc space.
> >
> > diff --git a/arch/arm/include/asm/fixmap.h b/arch/arm/include/asm/fixmap.h
> > index fc56fc3e1931..9575b404019c 100644
> > --- a/arch/arm/include/asm/fixmap.h
> > +++ b/arch/arm/include/asm/fixmap.h
> > @@ -2,7 +2,7 @@
> > #ifndef _ASM_FIXMAP_H
> > #define _ASM_FIXMAP_H
> >
> > -#define FIXADDR_START 0xffc00000UL
> > +#define FIXADDR_START 0xffc80000UL
> > #define FIXADDR_END 0xfff00000UL
> > #define FIXADDR_TOP (FIXADDR_END - PAGE_SIZE)
> >
> > 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 58e56e23a19a..c08574cad748 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>
> > @@ -90,7 +91,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);
> > @@ -1078,13 +1078,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..a7231d151c63 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 */
> > + 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
>
> Best regards
> --
> Marek Szyprowski, PhD
> Samsung R&D Institute Poland
>
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v2 2/2] ARM: move device tree mapping out of linear region
@ 2020-10-28 9:22 ` Ard Biesheuvel
0 siblings, 0 replies; 37+ messages in thread
From: Ard Biesheuvel @ 2020-10-28 9:22 UTC (permalink / raw)
To: Marek Szyprowski
Cc: Florian Fainelli, Nicolas Pitre, Linus Walleij, Russell King,
Krzysztof Kozlowski, Rob Herring, Linux Samsung SOC, Linux ARM
On Wed, 28 Oct 2020 at 10:19, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
>
> Hi,
>
> On 07.10.2020 10:39, 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 between the end of the vmalloc region
> > and the start of the 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.
> >
> > Since this region was formerly used as a guard region, which will now be
> > populated fully on LPAE builds by this read-only mapping (which will
> > still be able to function as a guard region for stray writes), bump the
> > start of the [underutilized] fixmap region by 512 KB as well, to ensure
> > that there is always a proper guard region here. Doing so still leaves
> > ample room for the fixmap space, even with NR_CPUS set to its maximum
> > value of 32.
> >
> > Tested-by: Linus Walleij <linus.walleij@linaro.org>
> > Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
>
> This patch landed in linux-next 20201028 as commit 7a1be318f579 ("ARM:
> 9012/1: move device tree mapping out of linear region"). Sadly it broke
> booting almost all Samsung Exynos-based boards. The only one which
> booted, used an appended device tree. I can provide more information if
> needed, just let me know what to check. "Starting kernel ..." is the
> last message I see here. No output from earlycon.
>
Thanks for the report. I will have a look later today.
Do these platforms happen to have any static device mappings that may
collide with this mapping of the FDT? Also, could this be related to
device drivers making changes in memory to the FDT image? Because the
permanent mapping of the FDT is read-only now.
> > ---
> > Documentation/arm/memory.rst | 7 ++++++-
> > arch/arm/include/asm/fixmap.h | 2 +-
> > 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 ++--
> > 8 files changed, 38 insertions(+), 17 deletions(-)
> >
> > diff --git a/Documentation/arm/memory.rst b/Documentation/arm/memory.rst
> > index 0521b4ce5c96..34bb23c44a71 100644
> > --- a/Documentation/arm/memory.rst
> > +++ b/Documentation/arm/memory.rst
> > @@ -45,9 +45,14 @@ fffe8000 fffeffff DTCM mapping area for platforms with
> > fffe0000 fffe7fff ITCM mapping area for platforms with
> > ITCM mounted inside the CPU.
> >
> > -ffc00000 ffefffff Fixmap mapping region. Addresses provided
> > +ffc80000 ffefffff Fixmap mapping region. Addresses provided
> > by fix_to_virt() will be located here.
> >
> > +ffc00000 ffc7ffff Guard region
> > +
> > +ff800000 ffbfffff Permanent, fixed read-only mapping of the
> > + firmware provided DT blob
> > +
> > fee00000 feffffff Mapping of PCI I/O space. This is a static
> > mapping within the vmalloc space.
> >
> > diff --git a/arch/arm/include/asm/fixmap.h b/arch/arm/include/asm/fixmap.h
> > index fc56fc3e1931..9575b404019c 100644
> > --- a/arch/arm/include/asm/fixmap.h
> > +++ b/arch/arm/include/asm/fixmap.h
> > @@ -2,7 +2,7 @@
> > #ifndef _ASM_FIXMAP_H
> > #define _ASM_FIXMAP_H
> >
> > -#define FIXADDR_START 0xffc00000UL
> > +#define FIXADDR_START 0xffc80000UL
> > #define FIXADDR_END 0xfff00000UL
> > #define FIXADDR_TOP (FIXADDR_END - PAGE_SIZE)
> >
> > 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 58e56e23a19a..c08574cad748 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>
> > @@ -90,7 +91,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);
> > @@ -1078,13 +1078,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..a7231d151c63 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 */
> > + 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
>
> Best regards
> --
> Marek Szyprowski, PhD
> Samsung R&D Institute Poland
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v2 2/2] ARM: move device tree mapping out of linear region
2020-10-28 9:22 ` Ard Biesheuvel
@ 2020-10-28 9:24 ` Ard Biesheuvel
-1 siblings, 0 replies; 37+ messages in thread
From: Ard Biesheuvel @ 2020-10-28 9:24 UTC (permalink / raw)
To: Marek Szyprowski
Cc: Linux ARM, Linux Samsung SOC, Krzysztof Kozlowski,
Florian Fainelli, Nicolas Pitre, Linus Walleij, Russell King,
Rob Herring
On Wed, 28 Oct 2020 at 10:22, Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Wed, 28 Oct 2020 at 10:19, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
> >
> > Hi,
> >
> > On 07.10.2020 10:39, 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 between the end of the vmalloc region
> > > and the start of the 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.
> > >
> > > Since this region was formerly used as a guard region, which will now be
> > > populated fully on LPAE builds by this read-only mapping (which will
> > > still be able to function as a guard region for stray writes), bump the
> > > start of the [underutilized] fixmap region by 512 KB as well, to ensure
> > > that there is always a proper guard region here. Doing so still leaves
> > > ample room for the fixmap space, even with NR_CPUS set to its maximum
> > > value of 32.
> > >
> > > Tested-by: Linus Walleij <linus.walleij@linaro.org>
> > > Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> > > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> >
> > This patch landed in linux-next 20201028 as commit 7a1be318f579 ("ARM:
> > 9012/1: move device tree mapping out of linear region"). Sadly it broke
> > booting almost all Samsung Exynos-based boards. The only one which
> > booted, used an appended device tree. I can provide more information if
> > needed, just let me know what to check. "Starting kernel ..." is the
> > last message I see here. No output from earlycon.
> >
>
> Thanks for the report. I will have a look later today.
>
> Do these platforms happen to have any static device mappings that may
> collide with this mapping of the FDT? Also, could this be related to
> device drivers making changes in memory to the FDT image? Because the
> permanent mapping of the FDT is read-only now.
>
IOW, does using MT_MEMORY_RW instead of MT_ROM for the mapping make a
difference?
>
>
> > > ---
> > > Documentation/arm/memory.rst | 7 ++++++-
> > > arch/arm/include/asm/fixmap.h | 2 +-
> > > 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 ++--
> > > 8 files changed, 38 insertions(+), 17 deletions(-)
> > >
> > > diff --git a/Documentation/arm/memory.rst b/Documentation/arm/memory.rst
> > > index 0521b4ce5c96..34bb23c44a71 100644
> > > --- a/Documentation/arm/memory.rst
> > > +++ b/Documentation/arm/memory.rst
> > > @@ -45,9 +45,14 @@ fffe8000 fffeffff DTCM mapping area for platforms with
> > > fffe0000 fffe7fff ITCM mapping area for platforms with
> > > ITCM mounted inside the CPU.
> > >
> > > -ffc00000 ffefffff Fixmap mapping region. Addresses provided
> > > +ffc80000 ffefffff Fixmap mapping region. Addresses provided
> > > by fix_to_virt() will be located here.
> > >
> > > +ffc00000 ffc7ffff Guard region
> > > +
> > > +ff800000 ffbfffff Permanent, fixed read-only mapping of the
> > > + firmware provided DT blob
> > > +
> > > fee00000 feffffff Mapping of PCI I/O space. This is a static
> > > mapping within the vmalloc space.
> > >
> > > diff --git a/arch/arm/include/asm/fixmap.h b/arch/arm/include/asm/fixmap.h
> > > index fc56fc3e1931..9575b404019c 100644
> > > --- a/arch/arm/include/asm/fixmap.h
> > > +++ b/arch/arm/include/asm/fixmap.h
> > > @@ -2,7 +2,7 @@
> > > #ifndef _ASM_FIXMAP_H
> > > #define _ASM_FIXMAP_H
> > >
> > > -#define FIXADDR_START 0xffc00000UL
> > > +#define FIXADDR_START 0xffc80000UL
> > > #define FIXADDR_END 0xfff00000UL
> > > #define FIXADDR_TOP (FIXADDR_END - PAGE_SIZE)
> > >
> > > 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 58e56e23a19a..c08574cad748 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>
> > > @@ -90,7 +91,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);
> > > @@ -1078,13 +1078,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..a7231d151c63 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 */
> > > + 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
> >
> > Best regards
> > --
> > Marek Szyprowski, PhD
> > Samsung R&D Institute Poland
> >
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v2 2/2] ARM: move device tree mapping out of linear region
@ 2020-10-28 9:24 ` Ard Biesheuvel
0 siblings, 0 replies; 37+ messages in thread
From: Ard Biesheuvel @ 2020-10-28 9:24 UTC (permalink / raw)
To: Marek Szyprowski
Cc: Florian Fainelli, Nicolas Pitre, Linus Walleij, Russell King,
Krzysztof Kozlowski, Rob Herring, Linux Samsung SOC, Linux ARM
On Wed, 28 Oct 2020 at 10:22, Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Wed, 28 Oct 2020 at 10:19, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
> >
> > Hi,
> >
> > On 07.10.2020 10:39, 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 between the end of the vmalloc region
> > > and the start of the 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.
> > >
> > > Since this region was formerly used as a guard region, which will now be
> > > populated fully on LPAE builds by this read-only mapping (which will
> > > still be able to function as a guard region for stray writes), bump the
> > > start of the [underutilized] fixmap region by 512 KB as well, to ensure
> > > that there is always a proper guard region here. Doing so still leaves
> > > ample room for the fixmap space, even with NR_CPUS set to its maximum
> > > value of 32.
> > >
> > > Tested-by: Linus Walleij <linus.walleij@linaro.org>
> > > Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> > > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> >
> > This patch landed in linux-next 20201028 as commit 7a1be318f579 ("ARM:
> > 9012/1: move device tree mapping out of linear region"). Sadly it broke
> > booting almost all Samsung Exynos-based boards. The only one which
> > booted, used an appended device tree. I can provide more information if
> > needed, just let me know what to check. "Starting kernel ..." is the
> > last message I see here. No output from earlycon.
> >
>
> Thanks for the report. I will have a look later today.
>
> Do these platforms happen to have any static device mappings that may
> collide with this mapping of the FDT? Also, could this be related to
> device drivers making changes in memory to the FDT image? Because the
> permanent mapping of the FDT is read-only now.
>
IOW, does using MT_MEMORY_RW instead of MT_ROM for the mapping make a
difference?
>
>
> > > ---
> > > Documentation/arm/memory.rst | 7 ++++++-
> > > arch/arm/include/asm/fixmap.h | 2 +-
> > > 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 ++--
> > > 8 files changed, 38 insertions(+), 17 deletions(-)
> > >
> > > diff --git a/Documentation/arm/memory.rst b/Documentation/arm/memory.rst
> > > index 0521b4ce5c96..34bb23c44a71 100644
> > > --- a/Documentation/arm/memory.rst
> > > +++ b/Documentation/arm/memory.rst
> > > @@ -45,9 +45,14 @@ fffe8000 fffeffff DTCM mapping area for platforms with
> > > fffe0000 fffe7fff ITCM mapping area for platforms with
> > > ITCM mounted inside the CPU.
> > >
> > > -ffc00000 ffefffff Fixmap mapping region. Addresses provided
> > > +ffc80000 ffefffff Fixmap mapping region. Addresses provided
> > > by fix_to_virt() will be located here.
> > >
> > > +ffc00000 ffc7ffff Guard region
> > > +
> > > +ff800000 ffbfffff Permanent, fixed read-only mapping of the
> > > + firmware provided DT blob
> > > +
> > > fee00000 feffffff Mapping of PCI I/O space. This is a static
> > > mapping within the vmalloc space.
> > >
> > > diff --git a/arch/arm/include/asm/fixmap.h b/arch/arm/include/asm/fixmap.h
> > > index fc56fc3e1931..9575b404019c 100644
> > > --- a/arch/arm/include/asm/fixmap.h
> > > +++ b/arch/arm/include/asm/fixmap.h
> > > @@ -2,7 +2,7 @@
> > > #ifndef _ASM_FIXMAP_H
> > > #define _ASM_FIXMAP_H
> > >
> > > -#define FIXADDR_START 0xffc00000UL
> > > +#define FIXADDR_START 0xffc80000UL
> > > #define FIXADDR_END 0xfff00000UL
> > > #define FIXADDR_TOP (FIXADDR_END - PAGE_SIZE)
> > >
> > > 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 58e56e23a19a..c08574cad748 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>
> > > @@ -90,7 +91,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);
> > > @@ -1078,13 +1078,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..a7231d151c63 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 */
> > > + 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
> >
> > Best regards
> > --
> > Marek Szyprowski, PhD
> > Samsung R&D Institute Poland
> >
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v2 2/2] ARM: move device tree mapping out of linear region
2020-10-28 9:24 ` Ard Biesheuvel
@ 2020-10-28 9:43 ` Marek Szyprowski
-1 siblings, 0 replies; 37+ messages in thread
From: Marek Szyprowski @ 2020-10-28 9:43 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Linux ARM, Linux Samsung SOC, Krzysztof Kozlowski,
Florian Fainelli, Nicolas Pitre, Linus Walleij, Russell King,
Rob Herring
Hi Ard,
On 28.10.2020 10:24, Ard Biesheuvel wrote:
> On Wed, 28 Oct 2020 at 10:22, Ard Biesheuvel <ardb@kernel.org> wrote:
>> On Wed, 28 Oct 2020 at 10:19, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
>>> On 07.10.2020 10:39, 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 between the end of the vmalloc region
>>>> and the start of the 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.
>>>>
>>>> Since this region was formerly used as a guard region, which will now be
>>>> populated fully on LPAE builds by this read-only mapping (which will
>>>> still be able to function as a guard region for stray writes), bump the
>>>> start of the [underutilized] fixmap region by 512 KB as well, to ensure
>>>> that there is always a proper guard region here. Doing so still leaves
>>>> ample room for the fixmap space, even with NR_CPUS set to its maximum
>>>> value of 32.
>>>>
>>>> Tested-by: Linus Walleij <linus.walleij@linaro.org>
>>>> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
>>>> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
>>> This patch landed in linux-next 20201028 as commit 7a1be318f579 ("ARM:
>>> 9012/1: move device tree mapping out of linear region"). Sadly it broke
>>> booting almost all Samsung Exynos-based boards. The only one which
>>> booted, used an appended device tree. I can provide more information if
>>> needed, just let me know what to check. "Starting kernel ..." is the
>>> last message I see here. No output from earlycon.
>>>
>> Thanks for the report. I will have a look later today.
>>
>> Do these platforms happen to have any static device mappings that may
>> collide with this mapping of the FDT? Also, could this be related to
>> device drivers making changes in memory to the FDT image? Because the
>> permanent mapping of the FDT is read-only now.
>>
> IOW, does using MT_MEMORY_RW instead of MT_ROM for the mapping make a
> difference?
Nope, chaning it to MT_MEMORY_RW doesn't fix anything.
The only static mapping I'm aware is S5P_VA_CHIPID at 0xF6000000 +
0x02000000.
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v2 2/2] ARM: move device tree mapping out of linear region
@ 2020-10-28 9:43 ` Marek Szyprowski
0 siblings, 0 replies; 37+ messages in thread
From: Marek Szyprowski @ 2020-10-28 9:43 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Florian Fainelli, Nicolas Pitre, Linus Walleij, Russell King,
Krzysztof Kozlowski, Rob Herring, Linux Samsung SOC, Linux ARM
Hi Ard,
On 28.10.2020 10:24, Ard Biesheuvel wrote:
> On Wed, 28 Oct 2020 at 10:22, Ard Biesheuvel <ardb@kernel.org> wrote:
>> On Wed, 28 Oct 2020 at 10:19, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
>>> On 07.10.2020 10:39, 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 between the end of the vmalloc region
>>>> and the start of the 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.
>>>>
>>>> Since this region was formerly used as a guard region, which will now be
>>>> populated fully on LPAE builds by this read-only mapping (which will
>>>> still be able to function as a guard region for stray writes), bump the
>>>> start of the [underutilized] fixmap region by 512 KB as well, to ensure
>>>> that there is always a proper guard region here. Doing so still leaves
>>>> ample room for the fixmap space, even with NR_CPUS set to its maximum
>>>> value of 32.
>>>>
>>>> Tested-by: Linus Walleij <linus.walleij@linaro.org>
>>>> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
>>>> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
>>> This patch landed in linux-next 20201028 as commit 7a1be318f579 ("ARM:
>>> 9012/1: move device tree mapping out of linear region"). Sadly it broke
>>> booting almost all Samsung Exynos-based boards. The only one which
>>> booted, used an appended device tree. I can provide more information if
>>> needed, just let me know what to check. "Starting kernel ..." is the
>>> last message I see here. No output from earlycon.
>>>
>> Thanks for the report. I will have a look later today.
>>
>> Do these platforms happen to have any static device mappings that may
>> collide with this mapping of the FDT? Also, could this be related to
>> device drivers making changes in memory to the FDT image? Because the
>> permanent mapping of the FDT is read-only now.
>>
> IOW, does using MT_MEMORY_RW instead of MT_ROM for the mapping make a
> difference?
Nope, chaning it to MT_MEMORY_RW doesn't fix anything.
The only static mapping I'm aware is S5P_VA_CHIPID at 0xF6000000 +
0x02000000.
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v2 2/2] ARM: move device tree mapping out of linear region
2020-10-28 9:19 ` Marek Szyprowski
@ 2020-10-28 12:05 ` Joel Stanley
-1 siblings, 0 replies; 37+ messages in thread
From: Joel Stanley @ 2020-10-28 12:05 UTC (permalink / raw)
To: Marek Szyprowski
Cc: Ard Biesheuvel, Linux ARM, Linux Samsung SOC,
Krzysztof Kozlowski, Linus Walleij, Florian Fainelli,
Russell King, Rob Herring, Nicolas Pitre, Cédric Le Goater
On Wed, 28 Oct 2020 at 09:19, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
>
> Hi,
>
> On 07.10.2020 10:39, 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 between the end of the vmalloc region
> > and the start of the 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.
> >
> > Since this region was formerly used as a guard region, which will now be
> > populated fully on LPAE builds by this read-only mapping (which will
> > still be able to function as a guard region for stray writes), bump the
> > start of the [underutilized] fixmap region by 512 KB as well, to ensure
> > that there is always a proper guard region here. Doing so still leaves
> > ample room for the fixmap space, even with NR_CPUS set to its maximum
> > value of 32.
> >
> > Tested-by: Linus Walleij <linus.walleij@linaro.org>
> > Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
>
> This patch landed in linux-next 20201028 as commit 7a1be318f579 ("ARM:
> 9012/1: move device tree mapping out of linear region"). Sadly it broke
> booting almost all Samsung Exynos-based boards. The only one which
> booted, used an appended device tree. I can provide more information if
> needed, just let me know what to check. "Starting kernel ..." is the
> last message I see here. No output from earlycon.
A bisection lead me to this patch after the next-20201028 failed to
boot on the aspeed systems in testing (aspeed_g5_defconfig).
You can reproduce this with today's next and qemu 5.1:
qemu-system-arm -M romulus-bmc -nographic \
-kernel arch/arm/boot/zImage \
-dtb arch/arm/boot/dts/aspeed-bmc-opp-romulus.dtb \
-initrd any-old-file
It requires the initrd option to reproduce, but the initrd doesn't
need to be valid as we don't make it that far.
There is no output but attaching gdb shows the kernel is stuck in
setup_machine_tags. (If we enable CONFIG_ATAGS it is instead stuck in
calibrate_delay).
(gdb) bt
#0 setup_machine_tags (machine_nr=<optimized out>,
__atags_vaddr=<optimized out>) at ../arch/arm/kernel/atags.h:12
#1 setup_arch (cmdline_p=0x80c01fc4) at ../arch/arm/kernel/setup.c:1100
#2 0x80b00d2c in start_kernel () at ../init/main.c:862
#3 0x00000000 in ?? ()
Reverting 7a1be318f579 on top of next allowed the system to boot again.
Cheers,
Joel
>
> > ---
> > Documentation/arm/memory.rst | 7 ++++++-
> > arch/arm/include/asm/fixmap.h | 2 +-
> > 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 ++--
> > 8 files changed, 38 insertions(+), 17 deletions(-)
> >
> > diff --git a/Documentation/arm/memory.rst b/Documentation/arm/memory.rst
> > index 0521b4ce5c96..34bb23c44a71 100644
> > --- a/Documentation/arm/memory.rst
> > +++ b/Documentation/arm/memory.rst
> > @@ -45,9 +45,14 @@ fffe8000 fffeffff DTCM mapping area for platforms with
> > fffe0000 fffe7fff ITCM mapping area for platforms with
> > ITCM mounted inside the CPU.
> >
> > -ffc00000 ffefffff Fixmap mapping region. Addresses provided
> > +ffc80000 ffefffff Fixmap mapping region. Addresses provided
> > by fix_to_virt() will be located here.
> >
> > +ffc00000 ffc7ffff Guard region
> > +
> > +ff800000 ffbfffff Permanent, fixed read-only mapping of the
> > + firmware provided DT blob
> > +
> > fee00000 feffffff Mapping of PCI I/O space. This is a static
> > mapping within the vmalloc space.
> >
> > diff --git a/arch/arm/include/asm/fixmap.h b/arch/arm/include/asm/fixmap.h
> > index fc56fc3e1931..9575b404019c 100644
> > --- a/arch/arm/include/asm/fixmap.h
> > +++ b/arch/arm/include/asm/fixmap.h
> > @@ -2,7 +2,7 @@
> > #ifndef _ASM_FIXMAP_H
> > #define _ASM_FIXMAP_H
> >
> > -#define FIXADDR_START 0xffc00000UL
> > +#define FIXADDR_START 0xffc80000UL
> > #define FIXADDR_END 0xfff00000UL
> > #define FIXADDR_TOP (FIXADDR_END - PAGE_SIZE)
> >
> > 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 58e56e23a19a..c08574cad748 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>
> > @@ -90,7 +91,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);
> > @@ -1078,13 +1078,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..a7231d151c63 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 */
> > + 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
>
> Best regards
> --
> Marek Szyprowski, PhD
> Samsung R&D Institute Poland
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v2 2/2] ARM: move device tree mapping out of linear region
@ 2020-10-28 12:05 ` Joel Stanley
0 siblings, 0 replies; 37+ messages in thread
From: Joel Stanley @ 2020-10-28 12:05 UTC (permalink / raw)
To: Marek Szyprowski
Cc: Florian Fainelli, Nicolas Pitre, Linus Walleij, Russell King,
Krzysztof Kozlowski, Rob Herring, Linux Samsung SOC,
Cédric Le Goater, Ard Biesheuvel, Linux ARM
On Wed, 28 Oct 2020 at 09:19, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
>
> Hi,
>
> On 07.10.2020 10:39, 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 between the end of the vmalloc region
> > and the start of the 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.
> >
> > Since this region was formerly used as a guard region, which will now be
> > populated fully on LPAE builds by this read-only mapping (which will
> > still be able to function as a guard region for stray writes), bump the
> > start of the [underutilized] fixmap region by 512 KB as well, to ensure
> > that there is always a proper guard region here. Doing so still leaves
> > ample room for the fixmap space, even with NR_CPUS set to its maximum
> > value of 32.
> >
> > Tested-by: Linus Walleij <linus.walleij@linaro.org>
> > Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
>
> This patch landed in linux-next 20201028 as commit 7a1be318f579 ("ARM:
> 9012/1: move device tree mapping out of linear region"). Sadly it broke
> booting almost all Samsung Exynos-based boards. The only one which
> booted, used an appended device tree. I can provide more information if
> needed, just let me know what to check. "Starting kernel ..." is the
> last message I see here. No output from earlycon.
A bisection lead me to this patch after the next-20201028 failed to
boot on the aspeed systems in testing (aspeed_g5_defconfig).
You can reproduce this with today's next and qemu 5.1:
qemu-system-arm -M romulus-bmc -nographic \
-kernel arch/arm/boot/zImage \
-dtb arch/arm/boot/dts/aspeed-bmc-opp-romulus.dtb \
-initrd any-old-file
It requires the initrd option to reproduce, but the initrd doesn't
need to be valid as we don't make it that far.
There is no output but attaching gdb shows the kernel is stuck in
setup_machine_tags. (If we enable CONFIG_ATAGS it is instead stuck in
calibrate_delay).
(gdb) bt
#0 setup_machine_tags (machine_nr=<optimized out>,
__atags_vaddr=<optimized out>) at ../arch/arm/kernel/atags.h:12
#1 setup_arch (cmdline_p=0x80c01fc4) at ../arch/arm/kernel/setup.c:1100
#2 0x80b00d2c in start_kernel () at ../init/main.c:862
#3 0x00000000 in ?? ()
Reverting 7a1be318f579 on top of next allowed the system to boot again.
Cheers,
Joel
>
> > ---
> > Documentation/arm/memory.rst | 7 ++++++-
> > arch/arm/include/asm/fixmap.h | 2 +-
> > 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 ++--
> > 8 files changed, 38 insertions(+), 17 deletions(-)
> >
> > diff --git a/Documentation/arm/memory.rst b/Documentation/arm/memory.rst
> > index 0521b4ce5c96..34bb23c44a71 100644
> > --- a/Documentation/arm/memory.rst
> > +++ b/Documentation/arm/memory.rst
> > @@ -45,9 +45,14 @@ fffe8000 fffeffff DTCM mapping area for platforms with
> > fffe0000 fffe7fff ITCM mapping area for platforms with
> > ITCM mounted inside the CPU.
> >
> > -ffc00000 ffefffff Fixmap mapping region. Addresses provided
> > +ffc80000 ffefffff Fixmap mapping region. Addresses provided
> > by fix_to_virt() will be located here.
> >
> > +ffc00000 ffc7ffff Guard region
> > +
> > +ff800000 ffbfffff Permanent, fixed read-only mapping of the
> > + firmware provided DT blob
> > +
> > fee00000 feffffff Mapping of PCI I/O space. This is a static
> > mapping within the vmalloc space.
> >
> > diff --git a/arch/arm/include/asm/fixmap.h b/arch/arm/include/asm/fixmap.h
> > index fc56fc3e1931..9575b404019c 100644
> > --- a/arch/arm/include/asm/fixmap.h
> > +++ b/arch/arm/include/asm/fixmap.h
> > @@ -2,7 +2,7 @@
> > #ifndef _ASM_FIXMAP_H
> > #define _ASM_FIXMAP_H
> >
> > -#define FIXADDR_START 0xffc00000UL
> > +#define FIXADDR_START 0xffc80000UL
> > #define FIXADDR_END 0xfff00000UL
> > #define FIXADDR_TOP (FIXADDR_END - PAGE_SIZE)
> >
> > 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 58e56e23a19a..c08574cad748 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>
> > @@ -90,7 +91,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);
> > @@ -1078,13 +1078,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..a7231d151c63 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 */
> > + 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
>
> Best regards
> --
> Marek Szyprowski, PhD
> Samsung R&D Institute Poland
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v2 2/2] ARM: move device tree mapping out of linear region
2020-10-28 12:05 ` Joel Stanley
@ 2020-10-28 12:52 ` Ard Biesheuvel
-1 siblings, 0 replies; 37+ messages in thread
From: Ard Biesheuvel @ 2020-10-28 12:52 UTC (permalink / raw)
To: Joel Stanley
Cc: Marek Szyprowski, Linux ARM, Linux Samsung SOC,
Krzysztof Kozlowski, Linus Walleij, Florian Fainelli,
Russell King, Rob Herring, Nicolas Pitre, Cédric Le Goater
On Wed, 28 Oct 2020 at 13:05, Joel Stanley <joel@jms.id.au> wrote:
>
> On Wed, 28 Oct 2020 at 09:19, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
> >
> > Hi,
> >
> > On 07.10.2020 10:39, 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 between the end of the vmalloc region
> > > and the start of the 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.
> > >
> > > Since this region was formerly used as a guard region, which will now be
> > > populated fully on LPAE builds by this read-only mapping (which will
> > > still be able to function as a guard region for stray writes), bump the
> > > start of the [underutilized] fixmap region by 512 KB as well, to ensure
> > > that there is always a proper guard region here. Doing so still leaves
> > > ample room for the fixmap space, even with NR_CPUS set to its maximum
> > > value of 32.
> > >
> > > Tested-by: Linus Walleij <linus.walleij@linaro.org>
> > > Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> > > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> >
> > This patch landed in linux-next 20201028 as commit 7a1be318f579 ("ARM:
> > 9012/1: move device tree mapping out of linear region"). Sadly it broke
> > booting almost all Samsung Exynos-based boards. The only one which
> > booted, used an appended device tree. I can provide more information if
> > needed, just let me know what to check. "Starting kernel ..." is the
> > last message I see here. No output from earlycon.
>
> A bisection lead me to this patch after the next-20201028 failed to
> boot on the aspeed systems in testing (aspeed_g5_defconfig).
>
> You can reproduce this with today's next and qemu 5.1:
>
> qemu-system-arm -M romulus-bmc -nographic \
> -kernel arch/arm/boot/zImage \
> -dtb arch/arm/boot/dts/aspeed-bmc-opp-romulus.dtb \
> -initrd any-old-file
>
> It requires the initrd option to reproduce, but the initrd doesn't
> need to be valid as we don't make it that far.
>
> There is no output but attaching gdb shows the kernel is stuck in
> setup_machine_tags. (If we enable CONFIG_ATAGS it is instead stuck in
> calibrate_delay).
>
> (gdb) bt
> #0 setup_machine_tags (machine_nr=<optimized out>,
> __atags_vaddr=<optimized out>) at ../arch/arm/kernel/atags.h:12
> #1 setup_arch (cmdline_p=0x80c01fc4) at ../arch/arm/kernel/setup.c:1100
> #2 0x80b00d2c in start_kernel () at ../init/main.c:862
> #3 0x00000000 in ?? ()
>
> Reverting 7a1be318f579 on top of next allowed the system to boot again.
>
Does this help?
diff --git a/arch/arm/include/asm/memory.h b/arch/arm/include/asm/memory.h
index bb79e52aeb90..4f355bda872a 100644
--- a/arch/arm/include/asm/memory.h
+++ b/arch/arm/include/asm/memory.h
@@ -68,8 +68,8 @@
#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))
+#define FDT_FIXED_SIZE (2 * SECTION_SIZE)
+#define FDT_VIRT_ADDR(physaddr) ((void *)(FDT_FIXED_BASE |
(physaddr) % SECTION_SIZE))
#if !defined(CONFIG_SMP) && !defined(CONFIG_ARM_LPAE)
/*
^ permalink raw reply related [flat|nested] 37+ messages in thread
* Re: [PATCH v2 2/2] ARM: move device tree mapping out of linear region
@ 2020-10-28 12:52 ` Ard Biesheuvel
0 siblings, 0 replies; 37+ messages in thread
From: Ard Biesheuvel @ 2020-10-28 12:52 UTC (permalink / raw)
To: Joel Stanley
Cc: Florian Fainelli, Nicolas Pitre, Linus Walleij, Russell King,
Krzysztof Kozlowski, Rob Herring, Linux Samsung SOC,
Cédric Le Goater, Linux ARM, Marek Szyprowski
On Wed, 28 Oct 2020 at 13:05, Joel Stanley <joel@jms.id.au> wrote:
>
> On Wed, 28 Oct 2020 at 09:19, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
> >
> > Hi,
> >
> > On 07.10.2020 10:39, 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 between the end of the vmalloc region
> > > and the start of the 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.
> > >
> > > Since this region was formerly used as a guard region, which will now be
> > > populated fully on LPAE builds by this read-only mapping (which will
> > > still be able to function as a guard region for stray writes), bump the
> > > start of the [underutilized] fixmap region by 512 KB as well, to ensure
> > > that there is always a proper guard region here. Doing so still leaves
> > > ample room for the fixmap space, even with NR_CPUS set to its maximum
> > > value of 32.
> > >
> > > Tested-by: Linus Walleij <linus.walleij@linaro.org>
> > > Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> > > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> >
> > This patch landed in linux-next 20201028 as commit 7a1be318f579 ("ARM:
> > 9012/1: move device tree mapping out of linear region"). Sadly it broke
> > booting almost all Samsung Exynos-based boards. The only one which
> > booted, used an appended device tree. I can provide more information if
> > needed, just let me know what to check. "Starting kernel ..." is the
> > last message I see here. No output from earlycon.
>
> A bisection lead me to this patch after the next-20201028 failed to
> boot on the aspeed systems in testing (aspeed_g5_defconfig).
>
> You can reproduce this with today's next and qemu 5.1:
>
> qemu-system-arm -M romulus-bmc -nographic \
> -kernel arch/arm/boot/zImage \
> -dtb arch/arm/boot/dts/aspeed-bmc-opp-romulus.dtb \
> -initrd any-old-file
>
> It requires the initrd option to reproduce, but the initrd doesn't
> need to be valid as we don't make it that far.
>
> There is no output but attaching gdb shows the kernel is stuck in
> setup_machine_tags. (If we enable CONFIG_ATAGS it is instead stuck in
> calibrate_delay).
>
> (gdb) bt
> #0 setup_machine_tags (machine_nr=<optimized out>,
> __atags_vaddr=<optimized out>) at ../arch/arm/kernel/atags.h:12
> #1 setup_arch (cmdline_p=0x80c01fc4) at ../arch/arm/kernel/setup.c:1100
> #2 0x80b00d2c in start_kernel () at ../init/main.c:862
> #3 0x00000000 in ?? ()
>
> Reverting 7a1be318f579 on top of next allowed the system to boot again.
>
Does this help?
diff --git a/arch/arm/include/asm/memory.h b/arch/arm/include/asm/memory.h
index bb79e52aeb90..4f355bda872a 100644
--- a/arch/arm/include/asm/memory.h
+++ b/arch/arm/include/asm/memory.h
@@ -68,8 +68,8 @@
#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))
+#define FDT_FIXED_SIZE (2 * SECTION_SIZE)
+#define FDT_VIRT_ADDR(physaddr) ((void *)(FDT_FIXED_BASE |
(physaddr) % SECTION_SIZE))
#if !defined(CONFIG_SMP) && !defined(CONFIG_ARM_LPAE)
/*
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 37+ messages in thread
* Re: [PATCH v2 2/2] ARM: move device tree mapping out of linear region
2020-10-28 12:52 ` Ard Biesheuvel
@ 2020-10-28 12:59 ` Joel Stanley
-1 siblings, 0 replies; 37+ messages in thread
From: Joel Stanley @ 2020-10-28 12:59 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Marek Szyprowski, Linux ARM, Linux Samsung SOC,
Krzysztof Kozlowski, Linus Walleij, Florian Fainelli,
Russell King, Rob Herring, Nicolas Pitre, Cédric Le Goater
On Wed, 28 Oct 2020 at 12:53, Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Wed, 28 Oct 2020 at 13:05, Joel Stanley <joel@jms.id.au> wrote:
> >
> > On Wed, 28 Oct 2020 at 09:19, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
> > > This patch landed in linux-next 20201028 as commit 7a1be318f579 ("ARM:
> > > 9012/1: move device tree mapping out of linear region"). Sadly it broke
> > > booting almost all Samsung Exynos-based boards. The only one which
> > > booted, used an appended device tree. I can provide more information if
> > > needed, just let me know what to check. "Starting kernel ..." is the
> > > last message I see here. No output from earlycon.
> >
> > A bisection lead me to this patch after the next-20201028 failed to
> > boot on the aspeed systems in testing (aspeed_g5_defconfig).
> >
> > You can reproduce this with today's next and qemu 5.1:
> >
> > qemu-system-arm -M romulus-bmc -nographic \
> > -kernel arch/arm/boot/zImage \
> > -dtb arch/arm/boot/dts/aspeed-bmc-opp-romulus.dtb \
> > -initrd any-old-file
> >
> > It requires the initrd option to reproduce, but the initrd doesn't
> > need to be valid as we don't make it that far.
> >
> > There is no output but attaching gdb shows the kernel is stuck in
> > setup_machine_tags. (If we enable CONFIG_ATAGS it is instead stuck in
> > calibrate_delay).
> >
> > (gdb) bt
> > #0 setup_machine_tags (machine_nr=<optimized out>,
> > __atags_vaddr=<optimized out>) at ../arch/arm/kernel/atags.h:12
> > #1 setup_arch (cmdline_p=0x80c01fc4) at ../arch/arm/kernel/setup.c:1100
> > #2 0x80b00d2c in start_kernel () at ../init/main.c:862
> > #3 0x00000000 in ?? ()
> >
> > Reverting 7a1be318f579 on top of next allowed the system to boot again.
> >
>
> Does this help?
Yes, that boots to userspace.
>
> diff --git a/arch/arm/include/asm/memory.h b/arch/arm/include/asm/memory.h
> index bb79e52aeb90..4f355bda872a 100644
> --- a/arch/arm/include/asm/memory.h
> +++ b/arch/arm/include/asm/memory.h
> @@ -68,8 +68,8 @@
> #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))
> +#define FDT_FIXED_SIZE (2 * SECTION_SIZE)
> +#define FDT_VIRT_ADDR(physaddr) ((void *)(FDT_FIXED_BASE |
> (physaddr) % SECTION_SIZE))
>
> #if !defined(CONFIG_SMP) && !defined(CONFIG_ARM_LPAE)
> /*
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v2 2/2] ARM: move device tree mapping out of linear region
@ 2020-10-28 12:59 ` Joel Stanley
0 siblings, 0 replies; 37+ messages in thread
From: Joel Stanley @ 2020-10-28 12:59 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Florian Fainelli, Nicolas Pitre, Linus Walleij, Russell King,
Krzysztof Kozlowski, Rob Herring, Linux Samsung SOC,
Cédric Le Goater, Linux ARM, Marek Szyprowski
On Wed, 28 Oct 2020 at 12:53, Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Wed, 28 Oct 2020 at 13:05, Joel Stanley <joel@jms.id.au> wrote:
> >
> > On Wed, 28 Oct 2020 at 09:19, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
> > > This patch landed in linux-next 20201028 as commit 7a1be318f579 ("ARM:
> > > 9012/1: move device tree mapping out of linear region"). Sadly it broke
> > > booting almost all Samsung Exynos-based boards. The only one which
> > > booted, used an appended device tree. I can provide more information if
> > > needed, just let me know what to check. "Starting kernel ..." is the
> > > last message I see here. No output from earlycon.
> >
> > A bisection lead me to this patch after the next-20201028 failed to
> > boot on the aspeed systems in testing (aspeed_g5_defconfig).
> >
> > You can reproduce this with today's next and qemu 5.1:
> >
> > qemu-system-arm -M romulus-bmc -nographic \
> > -kernel arch/arm/boot/zImage \
> > -dtb arch/arm/boot/dts/aspeed-bmc-opp-romulus.dtb \
> > -initrd any-old-file
> >
> > It requires the initrd option to reproduce, but the initrd doesn't
> > need to be valid as we don't make it that far.
> >
> > There is no output but attaching gdb shows the kernel is stuck in
> > setup_machine_tags. (If we enable CONFIG_ATAGS it is instead stuck in
> > calibrate_delay).
> >
> > (gdb) bt
> > #0 setup_machine_tags (machine_nr=<optimized out>,
> > __atags_vaddr=<optimized out>) at ../arch/arm/kernel/atags.h:12
> > #1 setup_arch (cmdline_p=0x80c01fc4) at ../arch/arm/kernel/setup.c:1100
> > #2 0x80b00d2c in start_kernel () at ../init/main.c:862
> > #3 0x00000000 in ?? ()
> >
> > Reverting 7a1be318f579 on top of next allowed the system to boot again.
> >
>
> Does this help?
Yes, that boots to userspace.
>
> diff --git a/arch/arm/include/asm/memory.h b/arch/arm/include/asm/memory.h
> index bb79e52aeb90..4f355bda872a 100644
> --- a/arch/arm/include/asm/memory.h
> +++ b/arch/arm/include/asm/memory.h
> @@ -68,8 +68,8 @@
> #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))
> +#define FDT_FIXED_SIZE (2 * SECTION_SIZE)
> +#define FDT_VIRT_ADDR(physaddr) ((void *)(FDT_FIXED_BASE |
> (physaddr) % SECTION_SIZE))
>
> #if !defined(CONFIG_SMP) && !defined(CONFIG_ARM_LPAE)
> /*
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v2 2/2] ARM: move device tree mapping out of linear region
2020-10-28 12:59 ` Joel Stanley
@ 2020-10-28 13:00 ` Ard Biesheuvel
-1 siblings, 0 replies; 37+ messages in thread
From: Ard Biesheuvel @ 2020-10-28 13:00 UTC (permalink / raw)
To: Joel Stanley
Cc: Marek Szyprowski, Linux ARM, Linux Samsung SOC,
Krzysztof Kozlowski, Linus Walleij, Florian Fainelli,
Russell King, Rob Herring, Nicolas Pitre, Cédric Le Goater
On Wed, 28 Oct 2020 at 13:59, Joel Stanley <joel@jms.id.au> wrote:
>
> On Wed, 28 Oct 2020 at 12:53, Ard Biesheuvel <ardb@kernel.org> wrote:
> >
> > On Wed, 28 Oct 2020 at 13:05, Joel Stanley <joel@jms.id.au> wrote:
> > >
> > > On Wed, 28 Oct 2020 at 09:19, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
> > > > This patch landed in linux-next 20201028 as commit 7a1be318f579 ("ARM:
> > > > 9012/1: move device tree mapping out of linear region"). Sadly it broke
> > > > booting almost all Samsung Exynos-based boards. The only one which
> > > > booted, used an appended device tree. I can provide more information if
> > > > needed, just let me know what to check. "Starting kernel ..." is the
> > > > last message I see here. No output from earlycon.
> > >
> > > A bisection lead me to this patch after the next-20201028 failed to
> > > boot on the aspeed systems in testing (aspeed_g5_defconfig).
> > >
> > > You can reproduce this with today's next and qemu 5.1:
> > >
> > > qemu-system-arm -M romulus-bmc -nographic \
> > > -kernel arch/arm/boot/zImage \
> > > -dtb arch/arm/boot/dts/aspeed-bmc-opp-romulus.dtb \
> > > -initrd any-old-file
> > >
> > > It requires the initrd option to reproduce, but the initrd doesn't
> > > need to be valid as we don't make it that far.
> > >
> > > There is no output but attaching gdb shows the kernel is stuck in
> > > setup_machine_tags. (If we enable CONFIG_ATAGS it is instead stuck in
> > > calibrate_delay).
> > >
> > > (gdb) bt
> > > #0 setup_machine_tags (machine_nr=<optimized out>,
> > > __atags_vaddr=<optimized out>) at ../arch/arm/kernel/atags.h:12
> > > #1 setup_arch (cmdline_p=0x80c01fc4) at ../arch/arm/kernel/setup.c:1100
> > > #2 0x80b00d2c in start_kernel () at ../init/main.c:862
> > > #3 0x00000000 in ?? ()
> > >
> > > Reverting 7a1be318f579 on top of next allowed the system to boot again.
> > >
> >
> > Does this help?
>
> Yes, that boots to userspace.
>
Thanks. I'll take that as a tested-by
> >
> > diff --git a/arch/arm/include/asm/memory.h b/arch/arm/include/asm/memory.h
> > index bb79e52aeb90..4f355bda872a 100644
> > --- a/arch/arm/include/asm/memory.h
> > +++ b/arch/arm/include/asm/memory.h
> > @@ -68,8 +68,8 @@
> > #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))
> > +#define FDT_FIXED_SIZE (2 * SECTION_SIZE)
> > +#define FDT_VIRT_ADDR(physaddr) ((void *)(FDT_FIXED_BASE |
> > (physaddr) % SECTION_SIZE))
> >
> > #if !defined(CONFIG_SMP) && !defined(CONFIG_ARM_LPAE)
> > /*
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v2 2/2] ARM: move device tree mapping out of linear region
@ 2020-10-28 13:00 ` Ard Biesheuvel
0 siblings, 0 replies; 37+ messages in thread
From: Ard Biesheuvel @ 2020-10-28 13:00 UTC (permalink / raw)
To: Joel Stanley
Cc: Florian Fainelli, Nicolas Pitre, Linus Walleij, Russell King,
Krzysztof Kozlowski, Rob Herring, Linux Samsung SOC,
Cédric Le Goater, Linux ARM, Marek Szyprowski
On Wed, 28 Oct 2020 at 13:59, Joel Stanley <joel@jms.id.au> wrote:
>
> On Wed, 28 Oct 2020 at 12:53, Ard Biesheuvel <ardb@kernel.org> wrote:
> >
> > On Wed, 28 Oct 2020 at 13:05, Joel Stanley <joel@jms.id.au> wrote:
> > >
> > > On Wed, 28 Oct 2020 at 09:19, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
> > > > This patch landed in linux-next 20201028 as commit 7a1be318f579 ("ARM:
> > > > 9012/1: move device tree mapping out of linear region"). Sadly it broke
> > > > booting almost all Samsung Exynos-based boards. The only one which
> > > > booted, used an appended device tree. I can provide more information if
> > > > needed, just let me know what to check. "Starting kernel ..." is the
> > > > last message I see here. No output from earlycon.
> > >
> > > A bisection lead me to this patch after the next-20201028 failed to
> > > boot on the aspeed systems in testing (aspeed_g5_defconfig).
> > >
> > > You can reproduce this with today's next and qemu 5.1:
> > >
> > > qemu-system-arm -M romulus-bmc -nographic \
> > > -kernel arch/arm/boot/zImage \
> > > -dtb arch/arm/boot/dts/aspeed-bmc-opp-romulus.dtb \
> > > -initrd any-old-file
> > >
> > > It requires the initrd option to reproduce, but the initrd doesn't
> > > need to be valid as we don't make it that far.
> > >
> > > There is no output but attaching gdb shows the kernel is stuck in
> > > setup_machine_tags. (If we enable CONFIG_ATAGS it is instead stuck in
> > > calibrate_delay).
> > >
> > > (gdb) bt
> > > #0 setup_machine_tags (machine_nr=<optimized out>,
> > > __atags_vaddr=<optimized out>) at ../arch/arm/kernel/atags.h:12
> > > #1 setup_arch (cmdline_p=0x80c01fc4) at ../arch/arm/kernel/setup.c:1100
> > > #2 0x80b00d2c in start_kernel () at ../init/main.c:862
> > > #3 0x00000000 in ?? ()
> > >
> > > Reverting 7a1be318f579 on top of next allowed the system to boot again.
> > >
> >
> > Does this help?
>
> Yes, that boots to userspace.
>
Thanks. I'll take that as a tested-by
> >
> > diff --git a/arch/arm/include/asm/memory.h b/arch/arm/include/asm/memory.h
> > index bb79e52aeb90..4f355bda872a 100644
> > --- a/arch/arm/include/asm/memory.h
> > +++ b/arch/arm/include/asm/memory.h
> > @@ -68,8 +68,8 @@
> > #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))
> > +#define FDT_FIXED_SIZE (2 * SECTION_SIZE)
> > +#define FDT_VIRT_ADDR(physaddr) ((void *)(FDT_FIXED_BASE |
> > (physaddr) % SECTION_SIZE))
> >
> > #if !defined(CONFIG_SMP) && !defined(CONFIG_ARM_LPAE)
> > /*
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v2 2/2] ARM: move device tree mapping out of linear region
2020-10-28 12:52 ` Ard Biesheuvel
@ 2020-10-28 13:00 ` Russell King - ARM Linux admin
-1 siblings, 0 replies; 37+ messages in thread
From: Russell King - ARM Linux admin @ 2020-10-28 13:00 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Joel Stanley, Marek Szyprowski, Linux ARM, Linux Samsung SOC,
Krzysztof Kozlowski, Linus Walleij, Florian Fainelli,
Rob Herring, Nicolas Pitre, Cédric Le Goater
On Wed, Oct 28, 2020 at 01:52:48PM +0100, Ard Biesheuvel wrote:
> Does this help?
>
> diff --git a/arch/arm/include/asm/memory.h b/arch/arm/include/asm/memory.h
> index bb79e52aeb90..4f355bda872a 100644
> --- a/arch/arm/include/asm/memory.h
> +++ b/arch/arm/include/asm/memory.h
> @@ -68,8 +68,8 @@
> #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))
> +#define FDT_FIXED_SIZE (2 * SECTION_SIZE)
> +#define FDT_VIRT_ADDR(physaddr) ((void *)(FDT_FIXED_BASE |
> (physaddr) % SECTION_SIZE))
Is this correct? If the FDT fixed size is 2x, why does FDT_VIRT_ADDR()
only work for half of it?
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v2 2/2] ARM: move device tree mapping out of linear region
@ 2020-10-28 13:00 ` Russell King - ARM Linux admin
0 siblings, 0 replies; 37+ messages in thread
From: Russell King - ARM Linux admin @ 2020-10-28 13:00 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Linux Samsung SOC, Nicolas Pitre, Linus Walleij,
Krzysztof Kozlowski, Rob Herring, Cédric Le Goater,
Florian Fainelli, Joel Stanley, Linux ARM, Marek Szyprowski
On Wed, Oct 28, 2020 at 01:52:48PM +0100, Ard Biesheuvel wrote:
> Does this help?
>
> diff --git a/arch/arm/include/asm/memory.h b/arch/arm/include/asm/memory.h
> index bb79e52aeb90..4f355bda872a 100644
> --- a/arch/arm/include/asm/memory.h
> +++ b/arch/arm/include/asm/memory.h
> @@ -68,8 +68,8 @@
> #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))
> +#define FDT_FIXED_SIZE (2 * SECTION_SIZE)
> +#define FDT_VIRT_ADDR(physaddr) ((void *)(FDT_FIXED_BASE |
> (physaddr) % SECTION_SIZE))
Is this correct? If the FDT fixed size is 2x, why does FDT_VIRT_ADDR()
only work for half of it?
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v2 2/2] ARM: move device tree mapping out of linear region
2020-10-28 13:00 ` Russell King - ARM Linux admin
@ 2020-10-28 13:04 ` Ard Biesheuvel
-1 siblings, 0 replies; 37+ messages in thread
From: Ard Biesheuvel @ 2020-10-28 13:04 UTC (permalink / raw)
To: Russell King - ARM Linux admin
Cc: Joel Stanley, Marek Szyprowski, Linux ARM, Linux Samsung SOC,
Krzysztof Kozlowski, Linus Walleij, Florian Fainelli,
Rob Herring, Nicolas Pitre, Cédric Le Goater
On Wed, 28 Oct 2020 at 14:01, Russell King - ARM Linux admin
<linux@armlinux.org.uk> wrote:
>
> On Wed, Oct 28, 2020 at 01:52:48PM +0100, Ard Biesheuvel wrote:
> > Does this help?
> >
> > diff --git a/arch/arm/include/asm/memory.h b/arch/arm/include/asm/memory.h
> > index bb79e52aeb90..4f355bda872a 100644
> > --- a/arch/arm/include/asm/memory.h
> > +++ b/arch/arm/include/asm/memory.h
> > @@ -68,8 +68,8 @@
> > #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))
> > +#define FDT_FIXED_SIZE (2 * SECTION_SIZE)
> > +#define FDT_VIRT_ADDR(physaddr) ((void *)(FDT_FIXED_BASE |
> > (physaddr) % SECTION_SIZE))
>
> Is this correct? If the FDT fixed size is 2x, why does FDT_VIRT_ADDR()
> only work for half of it?
>
Perhaps the naming is confusing. This is only used to obtain the start
of the virtually mapped DT, which amounts to the virtual address of
the section plus the physical address modulo the section size.
I will rename physaddr to physbase when I respin this as a patch.
Shall I rename FDT_VIRT_ADDR to FDT_VIRT_BASE as well?
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v2 2/2] ARM: move device tree mapping out of linear region
@ 2020-10-28 13:04 ` Ard Biesheuvel
0 siblings, 0 replies; 37+ messages in thread
From: Ard Biesheuvel @ 2020-10-28 13:04 UTC (permalink / raw)
To: Russell King - ARM Linux admin
Cc: Linux Samsung SOC, Nicolas Pitre, Linus Walleij,
Krzysztof Kozlowski, Rob Herring, Cédric Le Goater,
Florian Fainelli, Joel Stanley, Linux ARM, Marek Szyprowski
On Wed, 28 Oct 2020 at 14:01, Russell King - ARM Linux admin
<linux@armlinux.org.uk> wrote:
>
> On Wed, Oct 28, 2020 at 01:52:48PM +0100, Ard Biesheuvel wrote:
> > Does this help?
> >
> > diff --git a/arch/arm/include/asm/memory.h b/arch/arm/include/asm/memory.h
> > index bb79e52aeb90..4f355bda872a 100644
> > --- a/arch/arm/include/asm/memory.h
> > +++ b/arch/arm/include/asm/memory.h
> > @@ -68,8 +68,8 @@
> > #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))
> > +#define FDT_FIXED_SIZE (2 * SECTION_SIZE)
> > +#define FDT_VIRT_ADDR(physaddr) ((void *)(FDT_FIXED_BASE |
> > (physaddr) % SECTION_SIZE))
>
> Is this correct? If the FDT fixed size is 2x, why does FDT_VIRT_ADDR()
> only work for half of it?
>
Perhaps the naming is confusing. This is only used to obtain the start
of the virtually mapped DT, which amounts to the virtual address of
the section plus the physical address modulo the section size.
I will rename physaddr to physbase when I respin this as a patch.
Shall I rename FDT_VIRT_ADDR to FDT_VIRT_BASE as well?
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v2 2/2] ARM: move device tree mapping out of linear region
2020-10-28 13:00 ` Ard Biesheuvel
@ 2020-10-28 13:06 ` Joel Stanley
-1 siblings, 0 replies; 37+ messages in thread
From: Joel Stanley @ 2020-10-28 13:06 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Marek Szyprowski, Linux ARM, Linux Samsung SOC,
Krzysztof Kozlowski, Linus Walleij, Florian Fainelli,
Russell King, Rob Herring, Nicolas Pitre, Cédric Le Goater
On Wed, 28 Oct 2020 at 13:00, Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Wed, 28 Oct 2020 at 13:59, Joel Stanley <joel@jms.id.au> wrote:
> >
> > On Wed, 28 Oct 2020 at 12:53, Ard Biesheuvel <ardb@kernel.org> wrote:
> > >
> > > On Wed, 28 Oct 2020 at 13:05, Joel Stanley <joel@jms.id.au> wrote:
> > > >
> > > > On Wed, 28 Oct 2020 at 09:19, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
> > > > > This patch landed in linux-next 20201028 as commit 7a1be318f579 ("ARM:
> > > > > 9012/1: move device tree mapping out of linear region"). Sadly it broke
> > > > > booting almost all Samsung Exynos-based boards. The only one which
> > > > > booted, used an appended device tree. I can provide more information if
> > > > > needed, just let me know what to check. "Starting kernel ..." is the
> > > > > last message I see here. No output from earlycon.
> > > >
> > > > A bisection lead me to this patch after the next-20201028 failed to
> > > > boot on the aspeed systems in testing (aspeed_g5_defconfig).
> > > >
> > > > You can reproduce this with today's next and qemu 5.1:
> > > >
> > > > qemu-system-arm -M romulus-bmc -nographic \
> > > > -kernel arch/arm/boot/zImage \
> > > > -dtb arch/arm/boot/dts/aspeed-bmc-opp-romulus.dtb \
> > > > -initrd any-old-file
> > > >
> > > > It requires the initrd option to reproduce, but the initrd doesn't
> > > > need to be valid as we don't make it that far.
> > > >
> > > > There is no output but attaching gdb shows the kernel is stuck in
> > > > setup_machine_tags. (If we enable CONFIG_ATAGS it is instead stuck in
> > > > calibrate_delay).
> > > >
> > > > (gdb) bt
> > > > #0 setup_machine_tags (machine_nr=<optimized out>,
> > > > __atags_vaddr=<optimized out>) at ../arch/arm/kernel/atags.h:12
> > > > #1 setup_arch (cmdline_p=0x80c01fc4) at ../arch/arm/kernel/setup.c:1100
> > > > #2 0x80b00d2c in start_kernel () at ../init/main.c:862
> > > > #3 0x00000000 in ?? ()
> > > >
> > > > Reverting 7a1be318f579 on top of next allowed the system to boot again.
> > > >
> > >
> > > Does this help?
> >
> > Yes, that boots to userspace.
> >
>
> Thanks. I'll take that as a tested-by
Please do. Thanks for the quick fix.
Cheers,
Joel
>
>
> > >
> > > diff --git a/arch/arm/include/asm/memory.h b/arch/arm/include/asm/memory.h
> > > index bb79e52aeb90..4f355bda872a 100644
> > > --- a/arch/arm/include/asm/memory.h
> > > +++ b/arch/arm/include/asm/memory.h
> > > @@ -68,8 +68,8 @@
> > > #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))
> > > +#define FDT_FIXED_SIZE (2 * SECTION_SIZE)
> > > +#define FDT_VIRT_ADDR(physaddr) ((void *)(FDT_FIXED_BASE |
> > > (physaddr) % SECTION_SIZE))
> > >
> > > #if !defined(CONFIG_SMP) && !defined(CONFIG_ARM_LPAE)
> > > /*
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v2 2/2] ARM: move device tree mapping out of linear region
@ 2020-10-28 13:06 ` Joel Stanley
0 siblings, 0 replies; 37+ messages in thread
From: Joel Stanley @ 2020-10-28 13:06 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Florian Fainelli, Nicolas Pitre, Linus Walleij, Russell King,
Krzysztof Kozlowski, Rob Herring, Linux Samsung SOC,
Cédric Le Goater, Linux ARM, Marek Szyprowski
On Wed, 28 Oct 2020 at 13:00, Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Wed, 28 Oct 2020 at 13:59, Joel Stanley <joel@jms.id.au> wrote:
> >
> > On Wed, 28 Oct 2020 at 12:53, Ard Biesheuvel <ardb@kernel.org> wrote:
> > >
> > > On Wed, 28 Oct 2020 at 13:05, Joel Stanley <joel@jms.id.au> wrote:
> > > >
> > > > On Wed, 28 Oct 2020 at 09:19, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
> > > > > This patch landed in linux-next 20201028 as commit 7a1be318f579 ("ARM:
> > > > > 9012/1: move device tree mapping out of linear region"). Sadly it broke
> > > > > booting almost all Samsung Exynos-based boards. The only one which
> > > > > booted, used an appended device tree. I can provide more information if
> > > > > needed, just let me know what to check. "Starting kernel ..." is the
> > > > > last message I see here. No output from earlycon.
> > > >
> > > > A bisection lead me to this patch after the next-20201028 failed to
> > > > boot on the aspeed systems in testing (aspeed_g5_defconfig).
> > > >
> > > > You can reproduce this with today's next and qemu 5.1:
> > > >
> > > > qemu-system-arm -M romulus-bmc -nographic \
> > > > -kernel arch/arm/boot/zImage \
> > > > -dtb arch/arm/boot/dts/aspeed-bmc-opp-romulus.dtb \
> > > > -initrd any-old-file
> > > >
> > > > It requires the initrd option to reproduce, but the initrd doesn't
> > > > need to be valid as we don't make it that far.
> > > >
> > > > There is no output but attaching gdb shows the kernel is stuck in
> > > > setup_machine_tags. (If we enable CONFIG_ATAGS it is instead stuck in
> > > > calibrate_delay).
> > > >
> > > > (gdb) bt
> > > > #0 setup_machine_tags (machine_nr=<optimized out>,
> > > > __atags_vaddr=<optimized out>) at ../arch/arm/kernel/atags.h:12
> > > > #1 setup_arch (cmdline_p=0x80c01fc4) at ../arch/arm/kernel/setup.c:1100
> > > > #2 0x80b00d2c in start_kernel () at ../init/main.c:862
> > > > #3 0x00000000 in ?? ()
> > > >
> > > > Reverting 7a1be318f579 on top of next allowed the system to boot again.
> > > >
> > >
> > > Does this help?
> >
> > Yes, that boots to userspace.
> >
>
> Thanks. I'll take that as a tested-by
Please do. Thanks for the quick fix.
Cheers,
Joel
>
>
> > >
> > > diff --git a/arch/arm/include/asm/memory.h b/arch/arm/include/asm/memory.h
> > > index bb79e52aeb90..4f355bda872a 100644
> > > --- a/arch/arm/include/asm/memory.h
> > > +++ b/arch/arm/include/asm/memory.h
> > > @@ -68,8 +68,8 @@
> > > #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))
> > > +#define FDT_FIXED_SIZE (2 * SECTION_SIZE)
> > > +#define FDT_VIRT_ADDR(physaddr) ((void *)(FDT_FIXED_BASE |
> > > (physaddr) % SECTION_SIZE))
> > >
> > > #if !defined(CONFIG_SMP) && !defined(CONFIG_ARM_LPAE)
> > > /*
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v2 2/2] ARM: move device tree mapping out of linear region
2020-10-28 13:04 ` Ard Biesheuvel
@ 2020-10-28 13:09 ` Russell King - ARM Linux admin
-1 siblings, 0 replies; 37+ messages in thread
From: Russell King - ARM Linux admin @ 2020-10-28 13:09 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Joel Stanley, Marek Szyprowski, Linux ARM, Linux Samsung SOC,
Krzysztof Kozlowski, Linus Walleij, Florian Fainelli,
Rob Herring, Nicolas Pitre, Cédric Le Goater
On Wed, Oct 28, 2020 at 02:04:41PM +0100, Ard Biesheuvel wrote:
> On Wed, 28 Oct 2020 at 14:01, Russell King - ARM Linux admin
> <linux@armlinux.org.uk> wrote:
> >
> > On Wed, Oct 28, 2020 at 01:52:48PM +0100, Ard Biesheuvel wrote:
> > > Does this help?
> > >
> > > diff --git a/arch/arm/include/asm/memory.h b/arch/arm/include/asm/memory.h
> > > index bb79e52aeb90..4f355bda872a 100644
> > > --- a/arch/arm/include/asm/memory.h
> > > +++ b/arch/arm/include/asm/memory.h
> > > @@ -68,8 +68,8 @@
> > > #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))
> > > +#define FDT_FIXED_SIZE (2 * SECTION_SIZE)
> > > +#define FDT_VIRT_ADDR(physaddr) ((void *)(FDT_FIXED_BASE |
> > > (physaddr) % SECTION_SIZE))
> >
> > Is this correct? If the FDT fixed size is 2x, why does FDT_VIRT_ADDR()
> > only work for half of it?
> >
>
> Perhaps the naming is confusing. This is only used to obtain the start
> of the virtually mapped DT, which amounts to the virtual address of
> the section plus the physical address modulo the section size.
>
> I will rename physaddr to physbase when I respin this as a patch.
> Shall I rename FDT_VIRT_ADDR to FDT_VIRT_BASE as well?
Yes please, I think that makes it clearer what is going on here.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v2 2/2] ARM: move device tree mapping out of linear region
@ 2020-10-28 13:09 ` Russell King - ARM Linux admin
0 siblings, 0 replies; 37+ messages in thread
From: Russell King - ARM Linux admin @ 2020-10-28 13:09 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Linux Samsung SOC, Nicolas Pitre, Linus Walleij,
Krzysztof Kozlowski, Rob Herring, Cédric Le Goater,
Florian Fainelli, Joel Stanley, Linux ARM, Marek Szyprowski
On Wed, Oct 28, 2020 at 02:04:41PM +0100, Ard Biesheuvel wrote:
> On Wed, 28 Oct 2020 at 14:01, Russell King - ARM Linux admin
> <linux@armlinux.org.uk> wrote:
> >
> > On Wed, Oct 28, 2020 at 01:52:48PM +0100, Ard Biesheuvel wrote:
> > > Does this help?
> > >
> > > diff --git a/arch/arm/include/asm/memory.h b/arch/arm/include/asm/memory.h
> > > index bb79e52aeb90..4f355bda872a 100644
> > > --- a/arch/arm/include/asm/memory.h
> > > +++ b/arch/arm/include/asm/memory.h
> > > @@ -68,8 +68,8 @@
> > > #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))
> > > +#define FDT_FIXED_SIZE (2 * SECTION_SIZE)
> > > +#define FDT_VIRT_ADDR(physaddr) ((void *)(FDT_FIXED_BASE |
> > > (physaddr) % SECTION_SIZE))
> >
> > Is this correct? If the FDT fixed size is 2x, why does FDT_VIRT_ADDR()
> > only work for half of it?
> >
>
> Perhaps the naming is confusing. This is only used to obtain the start
> of the virtually mapped DT, which amounts to the virtual address of
> the section plus the physical address modulo the section size.
>
> I will rename physaddr to physbase when I respin this as a patch.
> Shall I rename FDT_VIRT_ADDR to FDT_VIRT_BASE as well?
Yes please, I think that makes it clearer what is going on here.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v2 2/2] ARM: move device tree mapping out of linear region
2020-10-28 12:52 ` Ard Biesheuvel
@ 2020-10-28 13:16 ` Marek Szyprowski
-1 siblings, 0 replies; 37+ messages in thread
From: Marek Szyprowski @ 2020-10-28 13:16 UTC (permalink / raw)
To: Ard Biesheuvel, Joel Stanley
Cc: Linux ARM, Linux Samsung SOC, Krzysztof Kozlowski, Linus Walleij,
Florian Fainelli, Russell King, Rob Herring, Nicolas Pitre,
Cédric Le Goater
Hi Ard,
On 28.10.2020 13:52, Ard Biesheuvel wrote:
> On Wed, 28 Oct 2020 at 13:05, Joel Stanley <joel@jms.id.au> wrote:
>> On Wed, 28 Oct 2020 at 09:19, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
>>> On 07.10.2020 10:39, 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 between the end of the vmalloc region
>>>> and the start of the 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.
>>>>
>>>> Since this region was formerly used as a guard region, which will now be
>>>> populated fully on LPAE builds by this read-only mapping (which will
>>>> still be able to function as a guard region for stray writes), bump the
>>>> start of the [underutilized] fixmap region by 512 KB as well, to ensure
>>>> that there is always a proper guard region here. Doing so still leaves
>>>> ample room for the fixmap space, even with NR_CPUS set to its maximum
>>>> value of 32.
>>>>
>>>> Tested-by: Linus Walleij <linus.walleij@linaro.org>
>>>> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
>>>> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
>>> This patch landed in linux-next 20201028 as commit 7a1be318f579 ("ARM:
>>> 9012/1: move device tree mapping out of linear region"). Sadly it broke
>>> booting almost all Samsung Exynos-based boards. The only one which
>>> booted, used an appended device tree. I can provide more information if
>>> needed, just let me know what to check. "Starting kernel ..." is the
>>> last message I see here. No output from earlycon.
>> A bisection lead me to this patch after the next-20201028 failed to
>> boot on the aspeed systems in testing (aspeed_g5_defconfig).
>>
>> You can reproduce this with today's next and qemu 5.1:
>>
>> qemu-system-arm -M romulus-bmc -nographic \
>> -kernel arch/arm/boot/zImage \
>> -dtb arch/arm/boot/dts/aspeed-bmc-opp-romulus.dtb \
>> -initrd any-old-file
>>
>> It requires the initrd option to reproduce, but the initrd doesn't
>> need to be valid as we don't make it that far.
>>
>> There is no output but attaching gdb shows the kernel is stuck in
>> setup_machine_tags. (If we enable CONFIG_ATAGS it is instead stuck in
>> calibrate_delay).
>>
>> (gdb) bt
>> #0 setup_machine_tags (machine_nr=<optimized out>,
>> __atags_vaddr=<optimized out>) at ../arch/arm/kernel/atags.h:12
>> #1 setup_arch (cmdline_p=0x80c01fc4) at ../arch/arm/kernel/setup.c:1100
>> #2 0x80b00d2c in start_kernel () at ../init/main.c:862
>> #3 0x00000000 in ?? ()
>>
>> Reverting 7a1be318f579 on top of next allowed the system to boot again.
>>
> Does this help?
>
> diff --git a/arch/arm/include/asm/memory.h b/arch/arm/include/asm/memory.h
> index bb79e52aeb90..4f355bda872a 100644
> --- a/arch/arm/include/asm/memory.h
> +++ b/arch/arm/include/asm/memory.h
> @@ -68,8 +68,8 @@
> #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))
> +#define FDT_FIXED_SIZE (2 * SECTION_SIZE)
> +#define FDT_VIRT_ADDR(physaddr) ((void *)(FDT_FIXED_BASE |
> (physaddr) % SECTION_SIZE))
>
> #if !defined(CONFIG_SMP) && !defined(CONFIG_ARM_LPAE)
> /*
Yes, this fixes booting of my Samsung Exynos-based test boards :)
Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v2 2/2] ARM: move device tree mapping out of linear region
@ 2020-10-28 13:16 ` Marek Szyprowski
0 siblings, 0 replies; 37+ messages in thread
From: Marek Szyprowski @ 2020-10-28 13:16 UTC (permalink / raw)
To: Ard Biesheuvel, Joel Stanley
Cc: Florian Fainelli, Nicolas Pitre, Linus Walleij, Russell King,
Krzysztof Kozlowski, Rob Herring, Linux Samsung SOC,
Cédric Le Goater, Linux ARM
Hi Ard,
On 28.10.2020 13:52, Ard Biesheuvel wrote:
> On Wed, 28 Oct 2020 at 13:05, Joel Stanley <joel@jms.id.au> wrote:
>> On Wed, 28 Oct 2020 at 09:19, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
>>> On 07.10.2020 10:39, 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 between the end of the vmalloc region
>>>> and the start of the 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.
>>>>
>>>> Since this region was formerly used as a guard region, which will now be
>>>> populated fully on LPAE builds by this read-only mapping (which will
>>>> still be able to function as a guard region for stray writes), bump the
>>>> start of the [underutilized] fixmap region by 512 KB as well, to ensure
>>>> that there is always a proper guard region here. Doing so still leaves
>>>> ample room for the fixmap space, even with NR_CPUS set to its maximum
>>>> value of 32.
>>>>
>>>> Tested-by: Linus Walleij <linus.walleij@linaro.org>
>>>> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
>>>> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
>>> This patch landed in linux-next 20201028 as commit 7a1be318f579 ("ARM:
>>> 9012/1: move device tree mapping out of linear region"). Sadly it broke
>>> booting almost all Samsung Exynos-based boards. The only one which
>>> booted, used an appended device tree. I can provide more information if
>>> needed, just let me know what to check. "Starting kernel ..." is the
>>> last message I see here. No output from earlycon.
>> A bisection lead me to this patch after the next-20201028 failed to
>> boot on the aspeed systems in testing (aspeed_g5_defconfig).
>>
>> You can reproduce this with today's next and qemu 5.1:
>>
>> qemu-system-arm -M romulus-bmc -nographic \
>> -kernel arch/arm/boot/zImage \
>> -dtb arch/arm/boot/dts/aspeed-bmc-opp-romulus.dtb \
>> -initrd any-old-file
>>
>> It requires the initrd option to reproduce, but the initrd doesn't
>> need to be valid as we don't make it that far.
>>
>> There is no output but attaching gdb shows the kernel is stuck in
>> setup_machine_tags. (If we enable CONFIG_ATAGS it is instead stuck in
>> calibrate_delay).
>>
>> (gdb) bt
>> #0 setup_machine_tags (machine_nr=<optimized out>,
>> __atags_vaddr=<optimized out>) at ../arch/arm/kernel/atags.h:12
>> #1 setup_arch (cmdline_p=0x80c01fc4) at ../arch/arm/kernel/setup.c:1100
>> #2 0x80b00d2c in start_kernel () at ../init/main.c:862
>> #3 0x00000000 in ?? ()
>>
>> Reverting 7a1be318f579 on top of next allowed the system to boot again.
>>
> Does this help?
>
> diff --git a/arch/arm/include/asm/memory.h b/arch/arm/include/asm/memory.h
> index bb79e52aeb90..4f355bda872a 100644
> --- a/arch/arm/include/asm/memory.h
> +++ b/arch/arm/include/asm/memory.h
> @@ -68,8 +68,8 @@
> #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))
> +#define FDT_FIXED_SIZE (2 * SECTION_SIZE)
> +#define FDT_VIRT_ADDR(physaddr) ((void *)(FDT_FIXED_BASE |
> (physaddr) % SECTION_SIZE))
>
> #if !defined(CONFIG_SMP) && !defined(CONFIG_ARM_LPAE)
> /*
Yes, this fixes booting of my Samsung Exynos-based test boards :)
Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 37+ messages in thread
end of thread, other threads:[~2020-10-29 1:01 UTC | newest]
Thread overview: 37+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-07 8:39 [PATCH v2 0/2] ARM: move FDT mapping out of linear region Ard Biesheuvel
2020-10-07 8:39 ` [PATCH v2 1/2] ARM: centralize phys-to-virt conversion of DT/ATAGS address Ard Biesheuvel
2020-10-07 15:28 ` Nicolas Pitre
2020-10-07 8:39 ` [PATCH v2 2/2] ARM: move device tree mapping out of linear region Ard Biesheuvel
2020-10-07 15:28 ` Nicolas Pitre
2020-10-07 15:32 ` Ard Biesheuvel
2020-10-07 16:38 ` Nicolas Pitre
2020-10-07 16:40 ` Nicolas Pitre
2020-10-07 21:32 ` Russell King - ARM Linux admin
2020-10-07 22:05 ` Nicolas Pitre
[not found] ` <CGME20201028091912eucas1p13fb9cd947faa6bfd79199ea79648b6af@eucas1p1.samsung.com>
2020-10-28 9:19 ` Marek Szyprowski
2020-10-28 9:19 ` Marek Szyprowski
2020-10-28 9:22 ` Ard Biesheuvel
2020-10-28 9:22 ` Ard Biesheuvel
2020-10-28 9:24 ` Ard Biesheuvel
2020-10-28 9:24 ` Ard Biesheuvel
2020-10-28 9:43 ` Marek Szyprowski
2020-10-28 9:43 ` Marek Szyprowski
2020-10-28 12:05 ` Joel Stanley
2020-10-28 12:05 ` Joel Stanley
2020-10-28 12:52 ` Ard Biesheuvel
2020-10-28 12:52 ` Ard Biesheuvel
2020-10-28 12:59 ` Joel Stanley
2020-10-28 12:59 ` Joel Stanley
2020-10-28 13:00 ` Ard Biesheuvel
2020-10-28 13:00 ` Ard Biesheuvel
2020-10-28 13:06 ` Joel Stanley
2020-10-28 13:06 ` Joel Stanley
2020-10-28 13:00 ` Russell King - ARM Linux admin
2020-10-28 13:00 ` Russell King - ARM Linux admin
2020-10-28 13:04 ` Ard Biesheuvel
2020-10-28 13:04 ` Ard Biesheuvel
2020-10-28 13:09 ` Russell King - ARM Linux admin
2020-10-28 13:09 ` Russell King - ARM Linux admin
2020-10-28 13:16 ` Marek Szyprowski
2020-10-28 13:16 ` Marek Szyprowski
2020-10-11 16:39 ` [PATCH v2 0/2] ARM: move FDT " Ard Biesheuvel
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.