[PATCH 0/6] arm64: Get rid of __early_init_dt_declare_initrd()

[PATCH 0/6] arm64: Get rid of __early_init_dt_declare_initrd()

[Florian Fainelli]

Hi all,

The numbers no longer make any sense since I either did not correctly
understand the feedback being given, or dramatically changed the
approach.

This version introduces an architecture symbol: ARCH_HAS_PHYS_INITRD
which indicates whether the architecture cares/supports parsing the
physical address of the initrd. Currently ARM (32-bit), Unicore32 and
now ARM64 support that.

When that symbol is defined, we also have the generic FDT code populate
the initrd physical address and size, and we can later make use of that
within architecture specific code to populate the memblock regions and
do the righ physical to virtual address conversion.

Rob, hopefully this is what you had in mind.

Previous discussions/submissions list here:

v3:
https://www.spinics.net/lists/arm-kernel/msg683566.html
v2:
https://lkml.org/lkml/2018/10/25/4


Florian Fainelli (6):
  nds32: Remove phys_initrd_start and phys_initrd_size
  arch: Make phys_initrd_start and phys_initrd_size global variables
  arch: Define ARCH_HAS_PHYS_INITRD for ARM and Unicore32
  of/fdt: Populate phys_initrd_start/phys_initrd_size from FDT
  arm64: Utilize ARCH_HAS_PHYS_INITRD
  of/fdt: Remove definition check for __early_init_dt_declare_initrd()

 arch/Kconfig                    |  7 +++++++
 arch/arm/Kconfig                |  1 +
 arch/arm/mm/init.c              |  6 +++---
 arch/arm64/Kconfig              |  1 +
 arch/arm64/include/asm/memory.h |  8 --------
 arch/arm64/mm/init.c            | 23 +++++++++++------------
 arch/nds32/mm/init.c            |  2 --
 arch/unicore32/Kconfig          |  1 +
 arch/unicore32/mm/init.c        |  4 ++--
 drivers/of/fdt.c                |  6 ++++--
 include/linux/initrd.h          |  3 +++
 11 files changed, 33 insertions(+), 29 deletions(-)

-- 
2.17.1

[PATCH 1/6] nds32: Remove phys_initrd_start and phys_initrd_size

[Florian Fainelli]

This will conflict with a subsequent change making phys_initrd_start and
phys_initrd_size global variables. nds32 does not make use of those nor
provides a suitable declarations so just get rid of them.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 arch/nds32/mm/init.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/arch/nds32/mm/init.c b/arch/nds32/mm/init.c
index c713d2ad55dc..32f55a24ccbb 100644
--- a/arch/nds32/mm/init.c
+++ b/arch/nds32/mm/init.c
@@ -22,8 +22,6 @@
 DEFINE_PER_CPU(struct mmu_gather, mmu_gathers);
 DEFINE_SPINLOCK(anon_alias_lock);
 extern pgd_t swapper_pg_dir[PTRS_PER_PGD];
-extern unsigned long phys_initrd_start;
-extern unsigned long phys_initrd_size;
 
 /*
  * empty_zero_page is a special page that is used for
-- 
2.17.1

[PATCH 2/6] arch: Make phys_initrd_start and phys_initrd_size global variables

[Florian Fainelli]

Make phys_initrd_start and phys_initrd_size global variables that will
later be referenced by generic code under drivers/of/fdt.c.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 arch/arm/mm/init.c       | 4 ++--
 arch/unicore32/mm/init.c | 4 ++--
 include/linux/initrd.h   | 3 +++
 3 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/arch/arm/mm/init.c b/arch/arm/mm/init.c
index 0cc8e04295a4..8f364aa24172 100644
--- a/arch/arm/mm/init.c
+++ b/arch/arm/mm/init.c
@@ -51,8 +51,8 @@ unsigned long __init __clear_cr(unsigned long mask)
 }
 #endif
 
-static phys_addr_t phys_initrd_start __initdata = 0;
-static unsigned long phys_initrd_size __initdata = 0;
+phys_addr_t phys_initrd_start __initdata = 0;
+unsigned long phys_initrd_size __initdata = 0;
 
 static int __init early_initrd(char *p)
 {
diff --git a/arch/unicore32/mm/init.c b/arch/unicore32/mm/init.c
index 8f8699e62bd5..4dd26d6f02e5 100644
--- a/arch/unicore32/mm/init.c
+++ b/arch/unicore32/mm/init.c
@@ -31,8 +31,8 @@
 
 #include "mm.h"
 
-static unsigned long phys_initrd_start __initdata = 0x01000000;
-static unsigned long phys_initrd_size __initdata = SZ_8M;
+phys_addr_t phys_initrd_start __initdata = 0x01000000;
+unsigned long phys_initrd_size __initdata = SZ_8M;
 
 static int __init early_initrd(char *p)
 {
diff --git a/include/linux/initrd.h b/include/linux/initrd.h
index 84b423044088..14beaff9b445 100644
--- a/include/linux/initrd.h
+++ b/include/linux/initrd.h
@@ -21,4 +21,7 @@ extern int initrd_below_start_ok;
 extern unsigned long initrd_start, initrd_end;
 extern void free_initrd_mem(unsigned long, unsigned long);
 
+extern phys_addr_t phys_initrd_start;
+extern unsigned long phys_initrd_size;
+
 extern unsigned int real_root_dev;
-- 
2.17.1

Re: [PATCH 2/6] arch: Make phys_initrd_start and phys_initrd_size global variables

[Rob Herring]

On Mon, Oct 29, 2018 at 6:52 PM Florian Fainelli <f.fainelli@gmail.com> wrote:
>
> Make phys_initrd_start and phys_initrd_size global variables that will
> later be referenced by generic code under drivers/of/fdt.c.
>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
>  arch/arm/mm/init.c       | 4 ++--
>  arch/unicore32/mm/init.c | 4 ++--
>  include/linux/initrd.h   | 3 +++
>  3 files changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/mm/init.c b/arch/arm/mm/init.c
> index 0cc8e04295a4..8f364aa24172 100644
> --- a/arch/arm/mm/init.c
> +++ b/arch/arm/mm/init.c
> @@ -51,8 +51,8 @@ unsigned long __init __clear_cr(unsigned long mask)
>  }
>  #endif
>
> -static phys_addr_t phys_initrd_start __initdata = 0;
> -static unsigned long phys_initrd_size __initdata = 0;
> +phys_addr_t phys_initrd_start __initdata = 0;
> +unsigned long phys_initrd_size __initdata = 0;

I would declare these in common initrd code instead. Then you don't
need a kconfig symbol.

>  static int __init early_initrd(char *p)
>  {
> diff --git a/arch/unicore32/mm/init.c b/arch/unicore32/mm/init.c
> index 8f8699e62bd5..4dd26d6f02e5 100644
> --- a/arch/unicore32/mm/init.c
> +++ b/arch/unicore32/mm/init.c
> @@ -31,8 +31,8 @@
>
>  #include "mm.h"
>
> -static unsigned long phys_initrd_start __initdata = 0x01000000;
> -static unsigned long phys_initrd_size __initdata = SZ_8M;
> +phys_addr_t phys_initrd_start __initdata = 0x01000000;
> +unsigned long phys_initrd_size __initdata = SZ_8M;

You'll have to set these at runtime though. However, I seem to
remember that an exact size was needed for decompressing an initrd
which would make these defaults pointless. Maybe that was only certain
compression formats.

Rob

[PATCH 3/6] arch: Define ARCH_HAS_PHYS_INITRD for ARM and Unicore32

[Florian Fainelli]

Make ARM and Unicore32 select ARCH_HAS_PHYS_INITRD meaning that they do
define phys_initrd_start/phys_initrd_size and make use of it.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 arch/Kconfig           | 7 +++++++
 arch/arm/Kconfig       | 1 +
 arch/unicore32/Kconfig | 1 +
 3 files changed, 9 insertions(+)

diff --git a/arch/Kconfig b/arch/Kconfig
index 9d329608913e..0926f8291782 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -865,6 +865,13 @@ config HAVE_ARCH_PREL32_RELOCATIONS
 	  architectures, and don't require runtime relocation on relocatable
 	  kernels.
 
+config ARCH_HAS_PHYS_INITRD
+	bool
+	help
+	  An architecture selects this when it needs to act on the physical
+	  address of the initial ramdisk and allow generic code such as
+	  FDT to populate that address.
+
 source "kernel/gcov/Kconfig"
 
 source "scripts/gcc-plugins/Kconfig"
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index e8cd55a5b04c..b87c40701b0e 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -11,6 +11,7 @@ config ARM
 	select ARCH_HAS_KCOV
 	select ARCH_HAS_MEMBARRIER_SYNC_CORE
 	select ARCH_HAS_PTE_SPECIAL if ARM_LPAE
+	select ARCH_HAS_PHYS_INITRD
 	select ARCH_HAS_PHYS_TO_DMA
 	select ARCH_HAS_SET_MEMORY
 	select ARCH_HAS_STRICT_KERNEL_RWX if MMU && !XIP_KERNEL
diff --git a/arch/unicore32/Kconfig b/arch/unicore32/Kconfig
index 0c5111b206bd..28a66ae61dcf 100644
--- a/arch/unicore32/Kconfig
+++ b/arch/unicore32/Kconfig
@@ -2,6 +2,7 @@
 config UNICORE32
 	def_bool y
 	select ARCH_HAS_DEVMEM_IS_ALLOWED
+	select ARCH_HAS_PHYS_INITRD
 	select ARCH_MIGHT_HAVE_PC_PARPORT
 	select ARCH_MIGHT_HAVE_PC_SERIO
 	select DMA_DIRECT_OPS
-- 
2.17.1

[PATCH 4/6] of/fdt: Populate phys_initrd_start/phys_initrd_size from FDT

[Florian Fainelli]

If the architecture implements ARCH_HAS_PHYS_INITRD, make the FDT
scanning code populate the physical address of the start of the FDT and
its size.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 arch/arm/mm/init.c | 2 +-
 drivers/of/fdt.c   | 4 ++++
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mm/init.c b/arch/arm/mm/init.c
index 8f364aa24172..517e95cfb5d2 100644
--- a/arch/arm/mm/init.c
+++ b/arch/arm/mm/init.c
@@ -237,7 +237,7 @@ static void __init arm_initrd_init(void)
 	phys_addr_t start;
 	unsigned long size;
 
-	/* FDT scan will populate initrd_start */
+	/* FDT scan will populate initrd_start and phys_initrd_start */
 	if (initrd_start && !phys_initrd_size) {
 		phys_initrd_start = __virt_to_phys(initrd_start);
 		phys_initrd_size = initrd_end - initrd_start;
diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index 76c83c1ffeda..313cd4f24258 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -899,6 +899,10 @@ static void __early_init_dt_declare_initrd(unsigned long start,
 	initrd_start = (unsigned long)__va(start);
 	initrd_end = (unsigned long)__va(end);
 	initrd_below_start_ok = 1;
+#ifdef CONFIG_ARCH_HAS_PHYS_INITRD
+	phys_initrd_start = start;
+	phys_initrd_size = end - start;
+#endif
 }
 #endif
 
-- 
2.17.1

Re: [PATCH 4/6] of/fdt: Populate phys_initrd_start/phys_initrd_size from FDT

[Russell King - ARM Linux]

On Mon, Oct 29, 2018 at 04:52:04PM -0700, Florian Fainelli wrote:
> If the architecture implements ARCH_HAS_PHYS_INITRD, make the FDT
> scanning code populate the physical address of the start of the FDT and
> its size.
> 
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
>  arch/arm/mm/init.c | 2 +-
>  drivers/of/fdt.c   | 4 ++++
>  2 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/mm/init.c b/arch/arm/mm/init.c
> index 8f364aa24172..517e95cfb5d2 100644
> --- a/arch/arm/mm/init.c
> +++ b/arch/arm/mm/init.c
> @@ -237,7 +237,7 @@ static void __init arm_initrd_init(void)
>  	phys_addr_t start;
>  	unsigned long size;
>  
> -	/* FDT scan will populate initrd_start */
> +	/* FDT scan will populate initrd_start and phys_initrd_start */
>  	if (initrd_start && !phys_initrd_size) {
>  		phys_initrd_start = __virt_to_phys(initrd_start);
>  		phys_initrd_size = initrd_end - initrd_start;

We should be able to delete the whole if () { } block and comment as
a result of this series - it was added by Rob in 65939301acdb to
unify the DT initrd code.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

Re: [PATCH 4/6] of/fdt: Populate phys_initrd_start/phys_initrd_size from FDT

[Rob Herring]

On Mon, Oct 29, 2018 at 6:52 PM Florian Fainelli <f.fainelli@gmail.com> wrote:
>
> If the architecture implements ARCH_HAS_PHYS_INITRD, make the FDT
> scanning code populate the physical address of the start of the FDT and
> its size.
>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
>  arch/arm/mm/init.c | 2 +-
>  drivers/of/fdt.c   | 4 ++++
>  2 files changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/mm/init.c b/arch/arm/mm/init.c
> index 8f364aa24172..517e95cfb5d2 100644
> --- a/arch/arm/mm/init.c
> +++ b/arch/arm/mm/init.c
> @@ -237,7 +237,7 @@ static void __init arm_initrd_init(void)
>         phys_addr_t start;
>         unsigned long size;
>
> -       /* FDT scan will populate initrd_start */
> +       /* FDT scan will populate initrd_start and phys_initrd_start */
>         if (initrd_start && !phys_initrd_size) {
>                 phys_initrd_start = __virt_to_phys(initrd_start);
>                 phys_initrd_size = initrd_end - initrd_start;
> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> index 76c83c1ffeda..313cd4f24258 100644
> --- a/drivers/of/fdt.c
> +++ b/drivers/of/fdt.c
> @@ -899,6 +899,10 @@ static void __early_init_dt_declare_initrd(unsigned long start,
>         initrd_start = (unsigned long)__va(start);
>         initrd_end = (unsigned long)__va(end);

As Ard pointed out, these __va() calls will BUG on arm64 if VM
debugging is enabled. Unless the arm64 folks want to remove that check
(probably not), I'm fine with a 'if (!IS_ENABLED(CONFIG_ARM64)) {'
conditional here.

>         initrd_below_start_ok = 1;
> +#ifdef CONFIG_ARCH_HAS_PHYS_INITRD
> +       phys_initrd_start = start;
> +       phys_initrd_size = end - start;
> +#endif
>  }
>  #endif
>
> --
> 2.17.1
>

[PATCH 5/6] arm64: Utilize ARCH_HAS_PHYS_INITRD

[Florian Fainelli]

ARM64 is the only architecture that re-defines
__early_init_dt_declare_initrd() in order for that function to populate
initrd_start/initrd_end with physical addresses instead of virtual
addresses. Instead of having an override, just get rid of that
implementation and select ARCH_HAS_PHYS_INITRD which would do that for
us.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 arch/arm64/Kconfig              |  1 +
 arch/arm64/include/asm/memory.h |  8 --------
 arch/arm64/mm/init.c            | 23 +++++++++++------------
 3 files changed, 12 insertions(+), 20 deletions(-)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 964f682a2b7b..302fb721d412 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -21,6 +21,7 @@ config ARM64
 	select ARCH_HAS_GIGANTIC_PAGE if (MEMORY_ISOLATION && COMPACTION) || CMA
 	select ARCH_HAS_KCOV
 	select ARCH_HAS_MEMBARRIER_SYNC_CORE
+	select ARCH_HAS_PHYS_INITRD
 	select ARCH_HAS_PTE_SPECIAL
 	select ARCH_HAS_SET_MEMORY
 	select ARCH_HAS_SG_CHAIN
diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h
index b96442960aea..dc3ca21ba240 100644
--- a/arch/arm64/include/asm/memory.h
+++ b/arch/arm64/include/asm/memory.h
@@ -168,14 +168,6 @@
 #define IOREMAP_MAX_ORDER	(PMD_SHIFT)
 #endif
 
-#ifdef CONFIG_BLK_DEV_INITRD
-#define __early_init_dt_declare_initrd(__start, __end)			\
-	do {								\
-		initrd_start = (__start);				\
-		initrd_end = (__end);					\
-	} while (0)
-#endif
-
 #ifndef __ASSEMBLY__
 
 #include <linux/bitops.h>
diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
index 3cf87341859f..fef9eb7fdb50 100644
--- a/arch/arm64/mm/init.c
+++ b/arch/arm64/mm/init.c
@@ -61,6 +61,8 @@
  */
 s64 memstart_addr __ro_after_init = -1;
 phys_addr_t arm64_dma_phys_limit __ro_after_init;
+phys_addr_t phys_initrd_start __initdata;
+unsigned long phys_initrd_size __initdata;
 
 #ifdef CONFIG_BLK_DEV_INITRD
 static int __init early_initrd(char *p)
@@ -72,8 +74,8 @@ static int __init early_initrd(char *p)
 	if (*endp == ',') {
 		size = memparse(endp + 1, NULL);
 
-		initrd_start = start;
-		initrd_end = start + size;
+		phys_initrd_start = start;
+		phys_initrd_size = size;
 	}
 	return 0;
 }
@@ -408,14 +410,14 @@ void __init arm64_memblock_init(void)
 		memblock_add(__pa_symbol(_text), (u64)(_end - _text));
 	}
 
-	if (IS_ENABLED(CONFIG_BLK_DEV_INITRD) && initrd_start) {
+	if (IS_ENABLED(CONFIG_BLK_DEV_INITRD) && phys_initrd_size) {
 		/*
 		 * Add back the memory we just removed if it results in the
 		 * initrd to become inaccessible via the linear mapping.
 		 * Otherwise, this is a no-op
 		 */
-		u64 base = initrd_start & PAGE_MASK;
-		u64 size = PAGE_ALIGN(initrd_end) - base;
+		u64 base = phys_initrd_start & PAGE_MASK;
+		u64 size = PAGE_ALIGN(phys_initrd_size);
 
 		/*
 		 * We can only add back the initrd memory if we don't end up
@@ -460,13 +462,10 @@ void __init arm64_memblock_init(void)
 	 */
 	memblock_reserve(__pa_symbol(_text), _end - _text);
 #ifdef CONFIG_BLK_DEV_INITRD
-	if (initrd_start) {
-		memblock_reserve(initrd_start, initrd_end - initrd_start);
-
-		/* the generic initrd code expects virtual addresses */
-		initrd_start = __phys_to_virt(initrd_start);
-		initrd_end = __phys_to_virt(initrd_end);
-	}
+	/* the generic initrd code expects virtual addresses */
+	initrd_start = __phys_to_virt(phys_initrd_start);
+	initrd_end = initrd_start + phys_initrd_size;
+	initrd_below_start_ok = 0;
 #endif
 
 	early_init_fdt_scan_reserved_mem();
-- 
2.17.1

[PATCH 6/6] of/fdt: Remove definition check for __early_init_dt_declare_initrd()

[Florian Fainelli]

With the one and only architecture (ARM64) no longer defining a custom
__early_init_dt_declare_initrd() function, just get rid of the check for
that function being already defined.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/of/fdt.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index 313cd4f24258..3d84fe79eeb4 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -892,7 +892,6 @@ const void * __init of_flat_dt_match_machine(const void *default_match,
 }
 
 #ifdef CONFIG_BLK_DEV_INITRD
-#ifndef __early_init_dt_declare_initrd
 static void __early_init_dt_declare_initrd(unsigned long start,
 					   unsigned long end)
 {
@@ -904,7 +903,6 @@ static void __early_init_dt_declare_initrd(unsigned long start,
 	phys_initrd_size = end - start;
 #endif
 }
-#endif
 
 /**
  * early_init_dt_check_for_initrd - Decode initrd location from flat tree
-- 
2.17.1