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

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

[Florian Fainelli]

Hi all,

Changes in v4:

- dropped initrd_below_start_ok assignment in ARM64, not necessary at
  all (Ard)
- replace #ifdef CONFIG_BLK_DEV_INITRD with if
  (IS_ENABLED(CONFIG_BLK_DEV_INITRD) for consistency with other parts
  of arm64_memblock_init() (Rob)

Changes in v3:

- use C conditionals in drivers/of/fdt.c
- added check on phys_initrd_size in arch/arm64/mm/init.c to determine
  whether initrd_start must be populated
- fixed a build warning with ARC that was just missing an (unsigned
  long) cast

Changes in v2:

- get rid of ARCH_HAS_PHYS_INITRD and instead define
  phys_initrd_start/phys_initrd_size in init/do_mounts_initrd.c

- make __early_init_dt_declare_initrd() account for ARM64 specific
  behavior with __va() when having CONFIG_DEBUG_VM enabled

- consolidate early_initrd() command line parsing into
  init/do_mounts_initrd.c

Because phys_initrd_start/phys_initrd_size are now compiled in
ini/do_mounts_initrd.c which is only built with CONFIG_BLK_DEV_INITRD=y,
we need to be a bit careful about the uses throughout architecture
specific code.

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
  of/fdt: Populate phys_initrd_start/phys_initrd_size from FDT
  arm64: Utilize phys_initrd_start/phys_initrd_size
  of/fdt: Remove custom __early_init_dt_declare_initrd() implementation
  arch: Move initrd= parsing into do_mounts_initrd.c

 arch/arc/mm/init.c              | 25 +++++-------------------
 arch/arm/mm/init.c              | 28 ++-------------------------
 arch/arm64/include/asm/memory.h |  8 --------
 arch/arm64/mm/init.c            | 34 ++++++---------------------------
 arch/nds32/mm/init.c            |  2 --
 arch/unicore32/mm/init.c        | 24 +++++------------------
 drivers/of/fdt.c                | 17 ++++++++++++-----
 include/linux/initrd.h          |  3 +++
 init/do_mounts_initrd.c         | 20 +++++++++++++++++++
 9 files changed, 53 insertions(+), 108 deletions(-)

-- 
2.17.1

[PATCH v4 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 131104bd2538..253f79fc7196 100644
--- a/arch/nds32/mm/init.c
+++ b/arch/nds32/mm/init.c
@@ -21,8 +21,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 v4 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 declared in
init/do_mounts_initrd.c such that we can later have generic code in
drivers/of/fdt.c populate those variables for us.

This requires both the ARM and unicore32 implementations to be properly
guarded against CONFIG_BLK_DEV_INITRD, and also initialize the variables
to the expected default values (unicore32).

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

diff --git a/arch/arm/mm/init.c b/arch/arm/mm/init.c
index 32e4845af2b6..438625764ccd 100644
--- a/arch/arm/mm/init.c
+++ b/arch/arm/mm/init.c
@@ -50,9 +50,7 @@ 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;
-
+#ifdef CONFIG_BLK_DEV_INITRD
 static int __init early_initrd(char *p)
 {
 	phys_addr_t start;
@@ -89,6 +87,7 @@ static int __init parse_tag_initrd2(const struct tag *tag)
 }
 
 __tagtable(ATAG_INITRD2, parse_tag_initrd2);
+#endif
 
 static void __init find_limits(unsigned long *min, unsigned long *max_low,
 			       unsigned long *max_high)
diff --git a/arch/unicore32/mm/init.c b/arch/unicore32/mm/init.c
index cf4eb9481fd6..02aa2c0b295e 100644
--- a/arch/unicore32/mm/init.c
+++ b/arch/unicore32/mm/init.c
@@ -30,9 +30,7 @@
 
 #include "mm.h"
 
-static unsigned long phys_initrd_start __initdata = 0x01000000;
-static unsigned long phys_initrd_size __initdata = SZ_8M;
-
+#ifdef CONFIG_BLK_DEV_INITRD
 static int __init early_initrd(char *p)
 {
 	unsigned long start, size;
@@ -48,6 +46,7 @@ static int __init early_initrd(char *p)
 	return 0;
 }
 early_param("initrd", early_initrd);
+#endif
 
 /*
  * This keeps memory configuration data used by a couple memory
@@ -156,6 +155,11 @@ void __init uc32_memblock_init(struct meminfo *mi)
 	memblock_reserve(__pa(_text), _end - _text);
 
 #ifdef CONFIG_BLK_DEV_INITRD
+	if (!phys_initrd_size) {
+		phys_initrd_start = 0x01000000;
+		phys_initrd_size = SZ_8M;
+	}
+
 	if (phys_initrd_size) {
 		memblock_reserve(phys_initrd_start, phys_initrd_size);
 
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;
diff --git a/init/do_mounts_initrd.c b/init/do_mounts_initrd.c
index d1a5d885ce13..45865b72f4ea 100644
--- a/init/do_mounts_initrd.c
+++ b/init/do_mounts_initrd.c
@@ -16,6 +16,9 @@ int initrd_below_start_ok;
 unsigned int real_root_dev;	/* do_proc_dointvec cannot handle kdev_t */
 static int __initdata mount_initrd = 1;
 
+phys_addr_t phys_initrd_start __initdata;
+unsigned long phys_initrd_size __initdata;
+
 static int __init no_initrd(char *str)
 {
 	mount_initrd = 0;
-- 
2.17.1

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

[Florian Fainelli]

Now that we have central and global variables holding the physical
address and size of the initrd, we can have
early_init_dt_check_for_initrd() populate
phys_initrd_start/phys_initrd_size for us.

This allows us to remove a chunk of code from arch/arm/mm/init.c
introduced with commit 65939301acdb ("arm: set initrd_start/initrd_end
for fdt scan").

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

diff --git a/arch/arm/mm/init.c b/arch/arm/mm/init.c
index 438625764ccd..a3b6f1f1cbaf 100644
--- a/arch/arm/mm/init.c
+++ b/arch/arm/mm/init.c
@@ -235,12 +235,6 @@ static void __init arm_initrd_init(void)
 	phys_addr_t start;
 	unsigned long size;
 
-	/* FDT scan will populate initrd_start */
-	if (initrd_start && !phys_initrd_size) {
-		phys_initrd_start = __virt_to_phys(initrd_start);
-		phys_initrd_size = initrd_end - initrd_start;
-	}
-
 	initrd_start = initrd_end = 0;
 
 	if (!phys_initrd_size)
diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index bb532aae0d92..88760a0983a7 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -924,6 +924,8 @@ static void __init early_init_dt_check_for_initrd(unsigned long node)
 	end = of_read_number(prop, len/4);
 
 	__early_init_dt_declare_initrd(start, end);
+	phys_initrd_start = start;
+	phys_initrd_size = end - start;
 
 	pr_debug("initrd_start=0x%llx  initrd_end=0x%llx\n",
 		 (unsigned long long)start, (unsigned long long)end);
-- 
2.17.1

[PATCH v4 4/6] arm64: Utilize phys_initrd_start/phys_initrd_size

[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 we can leverage
drivers/of/fdt.c populating phys_initrd_start/phys_initrd_size to
populate those variables for us.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 arch/arm64/mm/init.c | 20 ++++++++------------
 1 file changed, 8 insertions(+), 12 deletions(-)

diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
index 9d9582cac6c4..a66ffcde5f13 100644
--- a/arch/arm64/mm/init.c
+++ b/arch/arm64/mm/init.c
@@ -71,8 +71,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;
 }
@@ -407,14 +407,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
@@ -458,15 +458,11 @@ void __init arm64_memblock_init(void)
 	 * pagetables with memblock.
 	 */
 	memblock_reserve(__pa_symbol(_text), _end - _text);
-#ifdef CONFIG_BLK_DEV_INITRD
-	if (initrd_start) {
-		memblock_reserve(initrd_start, initrd_end - initrd_start);
-
+	if (IS_ENABLED(CONFIG_BLK_DEV_INITRD) && phys_initrd_size) {
 		/* the generic initrd code expects virtual addresses */
-		initrd_start = __phys_to_virt(initrd_start);
-		initrd_end = __phys_to_virt(initrd_end);
+		initrd_start = __phys_to_virt(phys_initrd_start);
+		initrd_end = initrd_start + phys_initrd_size;
 	}
-#endif
 
 	early_init_fdt_scan_reserved_mem();
 
-- 
2.17.1

[PATCH v4 5/6] of/fdt: Remove custom __early_init_dt_declare_initrd() implementation

[Florian Fainelli]

Now that ARM64 uses phys_initrd_start/phys_initrd_size, we can get rid
of its custom __early_init_dt_declare_initrd() which causes a fair
amount of objects rebuild when changing CONFIG_BLK_DEV_INITRD. In order
to make sure ARM64 does not produce a BUG() when VM debugging is turned
on though, we must avoid early calls to __va() which is what
__early_init_dt_declare_initrd() does and wrap this around to avoid
running that code on ARM64.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 arch/arm64/include/asm/memory.h |  8 --------
 drivers/of/fdt.c                | 15 ++++++++++-----
 2 files changed, 10 insertions(+), 13 deletions(-)

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/drivers/of/fdt.c b/drivers/of/fdt.c
index 88760a0983a7..cd72a41fcab2 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -891,15 +891,20 @@ 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)
 {
-	initrd_start = (unsigned long)__va(start);
-	initrd_end = (unsigned long)__va(end);
-	initrd_below_start_ok = 1;
+	/* ARM64 would cause a BUG to occur here when CONFIG_DEBUG_VM is
+	 * enabled since __va() is called too early. ARM64 does make use
+	 * of phys_initrd_start/phys_initrd_size so we can skip this
+	 * conversion.
+	 */
+	if (!IS_ENABLED(CONFIG_ARM64)) {
+		initrd_start = (unsigned long)__va(start);
+		initrd_end = (unsigned long)__va(end);
+		initrd_below_start_ok = 1;
+	}
 }
-#endif
 
 /**
  * early_init_dt_check_for_initrd - Decode initrd location from flat tree
-- 
2.17.1

[PATCH v4 6/6] arch: Move initrd= parsing into do_mounts_initrd.c

[Florian Fainelli]

ARC, ARM, ARM64 and Unicore32 are all capable of parsing the "initrd="
command line parameter to allow specifying the physical address and size
of an initrd. Move that parsing into init/do_mounts_initrd.c such that
we no longer duplicate that logic.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 arch/arc/mm/init.c       | 25 +++++--------------------
 arch/arm/mm/init.c       | 17 -----------------
 arch/arm64/mm/init.c     | 18 ------------------
 arch/unicore32/mm/init.c | 18 ------------------
 init/do_mounts_initrd.c  | 17 +++++++++++++++++
 5 files changed, 22 insertions(+), 73 deletions(-)

diff --git a/arch/arc/mm/init.c b/arch/arc/mm/init.c
index f8fe5668b30f..43bf4c3a1290 100644
--- a/arch/arc/mm/init.c
+++ b/arch/arc/mm/init.c
@@ -78,24 +78,6 @@ void __init early_init_dt_add_memory_arch(u64 base, u64 size)
 		base, TO_MB(size), !in_use ? "Not used":"");
 }
 
-#ifdef CONFIG_BLK_DEV_INITRD
-static int __init early_initrd(char *p)
-{
-	unsigned long start, size;
-	char *endp;
-
-	start = memparse(p, &endp);
-	if (*endp == ',') {
-		size = memparse(endp + 1, NULL);
-
-		initrd_start = (unsigned long)__va(start);
-		initrd_end = (unsigned long)__va(start + size);
-	}
-	return 0;
-}
-early_param("initrd", early_initrd);
-#endif
-
 /*
  * First memory setup routine called from setup_arch()
  * 1. setup swapper's mm @init_mm
@@ -140,8 +122,11 @@ void __init setup_arch_memory(void)
 	memblock_reserve(low_mem_start, __pa(_end) - low_mem_start);
 
 #ifdef CONFIG_BLK_DEV_INITRD
-	if (initrd_start)
-		memblock_reserve(__pa(initrd_start), initrd_end - initrd_start);
+	if (phys_initrd_size) {
+		memblock_reserve(phys_initrd_start, phys_initrd_size);
+		initrd_start = (unsigned long)__va(phys_initrd_start);
+		initrd_end = initrd_start + phys_initrd_size;
+	}
 #endif
 
 	early_init_fdt_reserve_self();
diff --git a/arch/arm/mm/init.c b/arch/arm/mm/init.c
index a3b6f1f1cbaf..478ea8b7db87 100644
--- a/arch/arm/mm/init.c
+++ b/arch/arm/mm/init.c
@@ -51,23 +51,6 @@ unsigned long __init __clear_cr(unsigned long mask)
 #endif
 
 #ifdef CONFIG_BLK_DEV_INITRD
-static int __init early_initrd(char *p)
-{
-	phys_addr_t start;
-	unsigned long size;
-	char *endp;
-
-	start = memparse(p, &endp);
-	if (*endp == ',') {
-		size = memparse(endp + 1, NULL);
-
-		phys_initrd_start = start;
-		phys_initrd_size = size;
-	}
-	return 0;
-}
-early_param("initrd", early_initrd);
-
 static int __init parse_tag_initrd(const struct tag *tag)
 {
 	pr_warn("ATAG_INITRD is deprecated; "
diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
index a66ffcde5f13..7474093363bc 100644
--- a/arch/arm64/mm/init.c
+++ b/arch/arm64/mm/init.c
@@ -61,24 +61,6 @@
 s64 memstart_addr __ro_after_init = -1;
 phys_addr_t arm64_dma_phys_limit __ro_after_init;
 
-#ifdef CONFIG_BLK_DEV_INITRD
-static int __init early_initrd(char *p)
-{
-	unsigned long start, size;
-	char *endp;
-
-	start = memparse(p, &endp);
-	if (*endp == ',') {
-		size = memparse(endp + 1, NULL);
-
-		phys_initrd_start = start;
-		phys_initrd_size = size;
-	}
-	return 0;
-}
-early_param("initrd", early_initrd);
-#endif
-
 #ifdef CONFIG_KEXEC_CORE
 /*
  * reserve_crashkernel() - reserves memory for crash kernel
diff --git a/arch/unicore32/mm/init.c b/arch/unicore32/mm/init.c
index 02aa2c0b295e..85ef2c624090 100644
--- a/arch/unicore32/mm/init.c
+++ b/arch/unicore32/mm/init.c
@@ -30,24 +30,6 @@
 
 #include "mm.h"
 
-#ifdef CONFIG_BLK_DEV_INITRD
-static int __init early_initrd(char *p)
-{
-	unsigned long start, size;
-	char *endp;
-
-	start = memparse(p, &endp);
-	if (*endp == ',') {
-		size = memparse(endp + 1, NULL);
-
-		phys_initrd_start = start;
-		phys_initrd_size = size;
-	}
-	return 0;
-}
-early_param("initrd", early_initrd);
-#endif
-
 /*
  * This keeps memory configuration data used by a couple memory
  * initialization functions, as well as show_mem() for the skipping
diff --git a/init/do_mounts_initrd.c b/init/do_mounts_initrd.c
index 45865b72f4ea..732d21f4a637 100644
--- a/init/do_mounts_initrd.c
+++ b/init/do_mounts_initrd.c
@@ -27,6 +27,23 @@ static int __init no_initrd(char *str)
 
 __setup("noinitrd", no_initrd);
 
+static int __init early_initrd(char *p)
+{
+	phys_addr_t start;
+	unsigned long size;
+	char *endp;
+
+	start = memparse(p, &endp);
+	if (*endp == ',') {
+		size = memparse(endp + 1, NULL);
+
+		phys_initrd_start = start;
+		phys_initrd_size = size;
+	}
+	return 0;
+}
+early_param("initrd", early_initrd);
+
 static int init_linuxrc(struct subprocess_info *info, struct cred *new)
 {
 	ksys_unshare(CLONE_FS | CLONE_FILES);
-- 
2.17.1

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

[Mike Rapoport]

On Mon, Nov 05, 2018 at 02:54:25PM -0800, Florian Fainelli wrote:
> Hi all,
> 
> Changes in v4:
> 
> - dropped initrd_below_start_ok assignment in ARM64, not necessary at
>   all (Ard)
> - replace #ifdef CONFIG_BLK_DEV_INITRD with if
>   (IS_ENABLED(CONFIG_BLK_DEV_INITRD) for consistency with other parts
>   of arm64_memblock_init() (Rob)
> 
> Changes in v3:
> 
> - use C conditionals in drivers/of/fdt.c
> - added check on phys_initrd_size in arch/arm64/mm/init.c to determine
>   whether initrd_start must be populated
> - fixed a build warning with ARC that was just missing an (unsigned
>   long) cast
> 
> Changes in v2:
> 
> - get rid of ARCH_HAS_PHYS_INITRD and instead define
>   phys_initrd_start/phys_initrd_size in init/do_mounts_initrd.c
> 
> - make __early_init_dt_declare_initrd() account for ARM64 specific
>   behavior with __va() when having CONFIG_DEBUG_VM enabled
> 
> - consolidate early_initrd() command line parsing into
>   init/do_mounts_initrd.c
> 
> Because phys_initrd_start/phys_initrd_size are now compiled in
> ini/do_mounts_initrd.c which is only built with CONFIG_BLK_DEV_INITRD=y,
> we need to be a bit careful about the uses throughout architecture
> specific code.
> 
> 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
>   of/fdt: Populate phys_initrd_start/phys_initrd_size from FDT
>   arm64: Utilize phys_initrd_start/phys_initrd_size
>   of/fdt: Remove custom __early_init_dt_declare_initrd() implementation
>   arch: Move initrd= parsing into do_mounts_initrd.c

For the series:

Reviewed-by: Mike Rapoport <rppt@linux.ibm.com>
 
>  arch/arc/mm/init.c              | 25 +++++-------------------
>  arch/arm/mm/init.c              | 28 ++-------------------------
>  arch/arm64/include/asm/memory.h |  8 --------
>  arch/arm64/mm/init.c            | 34 ++++++---------------------------
>  arch/nds32/mm/init.c            |  2 --
>  arch/unicore32/mm/init.c        | 24 +++++------------------
>  drivers/of/fdt.c                | 17 ++++++++++++-----
>  include/linux/initrd.h          |  3 +++
>  init/do_mounts_initrd.c         | 20 +++++++++++++++++++
>  9 files changed, 53 insertions(+), 108 deletions(-)
> 
> -- 
> 2.17.1
> 

-- 
Sincerely yours,
Mike.

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

[Florian Fainelli]

On 11/6/18 6:06 AM, Mike Rapoport wrote:
> On Mon, Nov 05, 2018 at 02:54:25PM -0800, Florian Fainelli wrote:
>> Hi all,
>>
>> Changes in v4:
>>
>> - dropped initrd_below_start_ok assignment in ARM64, not necessary at
>>   all (Ard)
>> - replace #ifdef CONFIG_BLK_DEV_INITRD with if
>>   (IS_ENABLED(CONFIG_BLK_DEV_INITRD) for consistency with other parts
>>   of arm64_memblock_init() (Rob)
>>
>> Changes in v3:
>>
>> - use C conditionals in drivers/of/fdt.c
>> - added check on phys_initrd_size in arch/arm64/mm/init.c to determine
>>   whether initrd_start must be populated
>> - fixed a build warning with ARC that was just missing an (unsigned
>>   long) cast
>>
>> Changes in v2:
>>
>> - get rid of ARCH_HAS_PHYS_INITRD and instead define
>>   phys_initrd_start/phys_initrd_size in init/do_mounts_initrd.c
>>
>> - make __early_init_dt_declare_initrd() account for ARM64 specific
>>   behavior with __va() when having CONFIG_DEBUG_VM enabled
>>
>> - consolidate early_initrd() command line parsing into
>>   init/do_mounts_initrd.c
>>
>> Because phys_initrd_start/phys_initrd_size are now compiled in
>> ini/do_mounts_initrd.c which is only built with CONFIG_BLK_DEV_INITRD=y,
>> we need to be a bit careful about the uses throughout architecture
>> specific code.
>>
>> 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
>>   of/fdt: Populate phys_initrd_start/phys_initrd_size from FDT
>>   arm64: Utilize phys_initrd_start/phys_initrd_size
>>   of/fdt: Remove custom __early_init_dt_declare_initrd() implementation
>>   arch: Move initrd= parsing into do_mounts_initrd.c
> 
> For the series:
> 
> Reviewed-by: Mike Rapoport <rppt@linux.ibm.com>

Thanks Mike, Rob, do you want to merge that series through the OF tree?
-- 
Florian

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

[Rob Herring]

On Mon, Nov 12, 2018 at 12:32:50PM -0800, Florian Fainelli wrote:
> On 11/6/18 6:06 AM, Mike Rapoport wrote:
> > On Mon, Nov 05, 2018 at 02:54:25PM -0800, Florian Fainelli wrote:
> >> Hi all,
> >>
> >> Changes in v4:
> >>
> >> - dropped initrd_below_start_ok assignment in ARM64, not necessary at
> >>   all (Ard)
> >> - replace #ifdef CONFIG_BLK_DEV_INITRD with if
> >>   (IS_ENABLED(CONFIG_BLK_DEV_INITRD) for consistency with other parts
> >>   of arm64_memblock_init() (Rob)
> >>
> >> Changes in v3:
> >>
> >> - use C conditionals in drivers/of/fdt.c
> >> - added check on phys_initrd_size in arch/arm64/mm/init.c to determine
> >>   whether initrd_start must be populated
> >> - fixed a build warning with ARC that was just missing an (unsigned
> >>   long) cast
> >>
> >> Changes in v2:
> >>
> >> - get rid of ARCH_HAS_PHYS_INITRD and instead define
> >>   phys_initrd_start/phys_initrd_size in init/do_mounts_initrd.c
> >>
> >> - make __early_init_dt_declare_initrd() account for ARM64 specific
> >>   behavior with __va() when having CONFIG_DEBUG_VM enabled
> >>
> >> - consolidate early_initrd() command line parsing into
> >>   init/do_mounts_initrd.c
> >>
> >> Because phys_initrd_start/phys_initrd_size are now compiled in
> >> ini/do_mounts_initrd.c which is only built with CONFIG_BLK_DEV_INITRD=y,
> >> we need to be a bit careful about the uses throughout architecture
> >> specific code.
> >>
> >> 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
> >>   of/fdt: Populate phys_initrd_start/phys_initrd_size from FDT
> >>   arm64: Utilize phys_initrd_start/phys_initrd_size
> >>   of/fdt: Remove custom __early_init_dt_declare_initrd() implementation
> >>   arch: Move initrd= parsing into do_mounts_initrd.c
> > 
> > For the series:
> > 
> > Reviewed-by: Mike Rapoport <rppt@linux.ibm.com>
> 
> Thanks Mike, Rob, do you want to merge that series through the OF tree?

Sure, some arch maintainer acks would be nice.

Rob

[PATCH v4 6/6] arch: Move initrd= parsing into do_mounts_initrd.c

[Vineet Gupta]

On 11/5/18 2:58 PM, Florian Fainelli wrote:
> ARC, ARM, ARM64 and Unicore32 are all capable of parsing the "initrd="
> command line parameter to allow specifying the physical address and size
> of an initrd. Move that parsing into init/do_mounts_initrd.c such that
> we no longer duplicate that logic.
>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
>  arch/arc/mm/init.c       | 25 +++++--------------------
>  arch/arm/mm/init.c       | 17 -----------------
>  arch/arm64/mm/init.c     | 18 ------------------
>  arch/unicore32/mm/init.c | 18 ------------------
>  init/do_mounts_initrd.c  | 17 +++++++++++++++++
>  5 files changed, 22 insertions(+), 73 deletions(-)
>
> diff --git a/arch/arc/mm/init.c b/arch/arc/mm/init.c
> index f8fe5668b30f..43bf4c3a1290 100644
> --- a/arch/arc/mm/init.c
> +++ b/arch/arc/mm/init.c
> @@ -78,24 +78,6 @@ void __init early_init_dt_add_memory_arch(u64 base, u64 size)
>  		base, TO_MB(size), !in_use ? "Not used":"");
>  }
>  
> -#ifdef CONFIG_BLK_DEV_INITRD
> -static int __init early_initrd(char *p)
> -{
> -	unsigned long start, size;
> -	char *endp;
> -
> -	start = memparse(p, &endp);
> -	if (*endp == ',') {
> -		size = memparse(endp + 1, NULL);
> -
> -		initrd_start = (unsigned long)__va(start);
> -		initrd_end = (unsigned long)__va(start + size);
> -	}
> -	return 0;
> -}
> -early_param("initrd", early_initrd);
> -#endif
> -
>  /*
>   * First memory setup routine called from setup_arch()
>   * 1. setup swapper's mm @init_mm
> @@ -140,8 +122,11 @@ void __init setup_arch_memory(void)
>  	memblock_reserve(low_mem_start, __pa(_end) - low_mem_start);
>  
>  #ifdef CONFIG_BLK_DEV_INITRD
> -	if (initrd_start)
> -		memblock_reserve(__pa(initrd_start), initrd_end - initrd_start);
> +	if (phys_initrd_size) {
> +		memblock_reserve(phys_initrd_start, phys_initrd_size);
> +		initrd_start = (unsigned long)__va(phys_initrd_start);
> +		initrd_end = initrd_start + phys_initrd_size;
> +	}
>  #endif

The common code now uses phys_initrd*, and you also use the same in ARC code, do
we still need the initrd_* setting here ?
ARC semantics was using them as PA anyways.

[snip]...

>  /*
>   * This keeps memory configuration data used by a couple memory
>   * initialization functions, as well as show_mem() for the skipping
> diff --git a/init/do_mounts_initrd.c b/init/do_mounts_initrd.c
> index 45865b72f4ea..732d21f4a637 100644
> --- a/init/do_mounts_initrd.c
> +++ b/init/do_mounts_initrd.c
> @@ -27,6 +27,23 @@ static int __init no_initrd(char *str)
>  
>  __setup("noinitrd", no_initrd);
>  
> +static int __init early_initrd(char *p)
> +{
> +	phys_addr_t start;
> +	unsigned long size;
> +	char *endp;
> +
> +	start = memparse(p, &endp);
> +	if (*endp == ',') {
> +		size = memparse(endp + 1, NULL);
> +
> +		phys_initrd_start = start;
> +		phys_initrd_size = size;
> +	}
> +	return 0;
> +}
> +early_param("initrd", early_initrd);
> +
>  static int init_linuxrc(struct subprocess_info *info, struct cred *new)
>  {
>  	ksys_unshare(CLONE_FS | CLONE_FILES);

[PATCH v4 6/6] arch: Move initrd= parsing into do_mounts_initrd.c

[Florian Fainelli]

On 11/12/18 4:34 PM, Vineet Gupta wrote:
> On 11/5/18 2:58 PM, Florian Fainelli wrote:
>> ARC, ARM, ARM64 and Unicore32 are all capable of parsing the "initrd="
>> command line parameter to allow specifying the physical address and size
>> of an initrd. Move that parsing into init/do_mounts_initrd.c such that
>> we no longer duplicate that logic.
>>
>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
>> ---
>>  arch/arc/mm/init.c       | 25 +++++--------------------
>>  arch/arm/mm/init.c       | 17 -----------------
>>  arch/arm64/mm/init.c     | 18 ------------------
>>  arch/unicore32/mm/init.c | 18 ------------------
>>  init/do_mounts_initrd.c  | 17 +++++++++++++++++
>>  5 files changed, 22 insertions(+), 73 deletions(-)
>>
>> diff --git a/arch/arc/mm/init.c b/arch/arc/mm/init.c
>> index f8fe5668b30f..43bf4c3a1290 100644
>> --- a/arch/arc/mm/init.c
>> +++ b/arch/arc/mm/init.c
>> @@ -78,24 +78,6 @@ void __init early_init_dt_add_memory_arch(u64 base, u64 size)
>>  		base, TO_MB(size), !in_use ? "Not used":"");
>>  }
>>  
>> -#ifdef CONFIG_BLK_DEV_INITRD
>> -static int __init early_initrd(char *p)
>> -{
>> -	unsigned long start, size;
>> -	char *endp;
>> -
>> -	start = memparse(p, &endp);
>> -	if (*endp == ',') {
>> -		size = memparse(endp + 1, NULL);
>> -
>> -		initrd_start = (unsigned long)__va(start);
>> -		initrd_end = (unsigned long)__va(start + size);
>> -	}
>> -	return 0;
>> -}
>> -early_param("initrd", early_initrd);
>> -#endif
>> -
>>  /*
>>   * First memory setup routine called from setup_arch()
>>   * 1. setup swapper's mm @init_mm
>> @@ -140,8 +122,11 @@ void __init setup_arch_memory(void)
>>  	memblock_reserve(low_mem_start, __pa(_end) - low_mem_start);
>>  
>>  #ifdef CONFIG_BLK_DEV_INITRD
>> -	if (initrd_start)
>> -		memblock_reserve(__pa(initrd_start), initrd_end - initrd_start);
>> +	if (phys_initrd_size) {
>> +		memblock_reserve(phys_initrd_start, phys_initrd_size);
>> +		initrd_start = (unsigned long)__va(phys_initrd_start);
>> +		initrd_end = initrd_start + phys_initrd_size;
>> +	}
>>  #endif
> 
> The common code now uses phys_initrd*, and you also use the same in ARC code, do
> we still need the initrd_* setting here ?
> ARC semantics was using them as PA anyways.

Yes, the generic initrd code expects initrd_start/end to be virtual
addresses, which we now directly derive from phys_initrd_start, that
should really be equivalent.
-- 
Florian

[PATCH v4 6/6] arch: Move initrd= parsing into do_mounts_initrd.c

[Vineet Gupta]

On 11/12/18 4:38 PM, Florian Fainelli wrote:
>>>  #ifdef CONFIG_BLK_DEV_INITRD
>>> -	if (initrd_start)
>>> -		memblock_reserve(__pa(initrd_start), initrd_end - initrd_start);
>>> +	if (phys_initrd_size) {
>>> +		memblock_reserve(phys_initrd_start, phys_initrd_size);
>>> +		initrd_start = (unsigned long)__va(phys_initrd_start);
>>> +		initrd_end = initrd_start + phys_initrd_size;
>>> +	}
>>>  #endif
>> The common code now uses phys_initrd*, and you also use the same in ARC code, do
>> we still need the initrd_* setting here ?
>> ARC semantics was using them as PA anyways.
> Yes, the generic initrd code expects initrd_start/end to be virtual
> addresses, which we now directly derive from phys_initrd_start, that
> should really be equivalent.

So we can skip this explicit setting above - ARC arch code doesn't access the virt
initrd_start

[PATCH v4 6/6] arch: Move initrd= parsing into do_mounts_initrd.c

[Florian Fainelli]

On 11/12/18 4:40 PM, Vineet Gupta wrote:
> On 11/12/18 4:38 PM, Florian Fainelli wrote:
>>>>  #ifdef CONFIG_BLK_DEV_INITRD
>>>> -	if (initrd_start)
>>>> -		memblock_reserve(__pa(initrd_start), initrd_end - initrd_start);
>>>> +	if (phys_initrd_size) {
>>>> +		memblock_reserve(phys_initrd_start, phys_initrd_size);
>>>> +		initrd_start = (unsigned long)__va(phys_initrd_start);
>>>> +		initrd_end = initrd_start + phys_initrd_size;
>>>> +	}
>>>>  #endif
>>> The common code now uses phys_initrd*, and you also use the same in ARC code, do
>>> we still need the initrd_* setting here ?
>>> ARC semantics was using them as PA anyways.
>> Yes, the generic initrd code expects initrd_start/end to be virtual
>> addresses, which we now directly derive from phys_initrd_start, that
>> should really be equivalent.
> 
> So we can skip this explicit setting above - ARC arch code doesn't access the virt
> initrd_start

OK, you are saying we could just have the generic initrd code do this
assignment instead of having each architecture do it, is that a correct
understanding? If so, I suppose it could be done, whether as of this
patch series or as a follow-up, either way is fine with me.

One possible caveat is if __va() and __phys_to_virt() behave differently
(e.g: because of CONFIG_DEBUG_VIRTUAL or other things).
-- 
Florian

[PATCH v4 6/6] arch: Move initrd= parsing into do_mounts_initrd.c

[Vineet Gupta]

On 11/12/18 4:52 PM, Florian Fainelli wrote:
> On 11/12/18 4:40 PM, Vineet Gupta wrote:
>> On 11/12/18 4:38 PM, Florian Fainelli wrote:
>>>>>  #ifdef CONFIG_BLK_DEV_INITRD
>>>>> -	if (initrd_start)
>>>>> -		memblock_reserve(__pa(initrd_start), initrd_end - initrd_start);
>>>>> +	if (phys_initrd_size) {
>>>>> +		memblock_reserve(phys_initrd_start, phys_initrd_size);
>>>>> +		initrd_start = (unsigned long)__va(phys_initrd_start);
>>>>> +		initrd_end = initrd_start + phys_initrd_size;
>>>>> +	}
>>>>>  #endif
>>>> The common code now uses phys_initrd*, and you also use the same in ARC code, do
>>>> we still need the initrd_* setting here ?
>>>> ARC semantics was using them as PA anyways.
>>> Yes, the generic initrd code expects initrd_start/end to be virtual
>>> addresses, which we now directly derive from phys_initrd_start, that
>>> should really be equivalent.
>> So we can skip this explicit setting above - ARC arch code doesn't access the virt
>> initrd_start
> OK, you are saying we could just have the generic initrd code do this
> assignment instead of having each architecture do it, is that a correct
> understanding? 

Correct !

> If so, I suppose it could be done, whether as of this
> patch series or as a follow-up, either way is fine with me.

If it is not too much trouble, I'd prefer this now. I should have chimed earlier.

> One possible caveat is if __va() and __phys_to_virt() behave differently
> (e.g: because of CONFIG_DEBUG_VIRTUAL or other things).


Thing is, after your patches, we don't use the vanilla initrd_xxx in arch code any
longer. So this becomes just an implementation detail, which core code may or
maynot need and if it does, this needs to work already w/o having to set anything
in arch code. Agree ?

Thx,
-Vineet

[PATCH v4 4/6] arm64: Utilize phys_initrd_start/phys_initrd_size

[Will Deacon]

On Mon, Nov 05, 2018 at 02:54:29PM -0800, Florian Fainelli wrote:
> 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 we can leverage
> drivers/of/fdt.c populating phys_initrd_start/phys_initrd_size to
> populate those variables for us.
> 
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
>  arch/arm64/mm/init.c | 20 ++++++++------------
>  1 file changed, 8 insertions(+), 12 deletions(-)

Looks ok to me:

Acked-by: Will Deacon <will.deacon@arm.com>

Will

[PATCH v4 6/6] arch: Move initrd= parsing into do_mounts_initrd.c

[Florian Fainelli]

On 11/12/18 4:57 PM, Vineet Gupta wrote:
> On 11/12/18 4:52 PM, Florian Fainelli wrote:
>> On 11/12/18 4:40 PM, Vineet Gupta wrote:
>>> On 11/12/18 4:38 PM, Florian Fainelli wrote:
>>>>>>  #ifdef CONFIG_BLK_DEV_INITRD
>>>>>> -	if (initrd_start)
>>>>>> -		memblock_reserve(__pa(initrd_start), initrd_end - initrd_start);
>>>>>> +	if (phys_initrd_size) {
>>>>>> +		memblock_reserve(phys_initrd_start, phys_initrd_size);
>>>>>> +		initrd_start = (unsigned long)__va(phys_initrd_start);
>>>>>> +		initrd_end = initrd_start + phys_initrd_size;
>>>>>> +	}
>>>>>>  #endif
>>>>> The common code now uses phys_initrd*, and you also use the same in ARC code, do
>>>>> we still need the initrd_* setting here ?
>>>>> ARC semantics was using them as PA anyways.
>>>> Yes, the generic initrd code expects initrd_start/end to be virtual
>>>> addresses, which we now directly derive from phys_initrd_start, that
>>>> should really be equivalent.
>>> So we can skip this explicit setting above - ARC arch code doesn't access the virt
>>> initrd_start
>> OK, you are saying we could just have the generic initrd code do this
>> assignment instead of having each architecture do it, is that a correct
>> understanding? 
> 
> Correct !
> 
>> If so, I suppose it could be done, whether as of this
>> patch series or as a follow-up, either way is fine with me.
> 
> If it is not too much trouble, I'd prefer this now. I should have chimed earlier.
> 
>> One possible caveat is if __va() and __phys_to_virt() behave differently
>> (e.g: because of CONFIG_DEBUG_VIRTUAL or other things).
> 
> 
> Thing is, after your patches, we don't use the vanilla initrd_xxx in arch code any
> longer. So this becomes just an implementation detail, which core code may or
> maynot need and if it does, this needs to work already w/o having to set anything
> in arch code. Agree ?

If you do not mind, I would prefer this series to go in, as-is, and
clean up the initrd_start/initrd_end assignment as a follow up patch
series. The reason is mostly that I am not yet clear on the timing of
these operations between the architecture resolving the virtual address
and the initrd code starting to use it.

Would that sound reasonable to you?
-- 
Florian

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

[Rob Herring]

On Mon, Nov 12, 2018 at 06:22:16PM -0600, Rob Herring wrote:
> On Mon, Nov 12, 2018 at 12:32:50PM -0800, Florian Fainelli wrote:
> > On 11/6/18 6:06 AM, Mike Rapoport wrote:
> > > On Mon, Nov 05, 2018 at 02:54:25PM -0800, Florian Fainelli wrote:
> > >> Hi all,
> > >>
> > >> Changes in v4:
> > >>
> > >> - dropped initrd_below_start_ok assignment in ARM64, not necessary at
> > >>   all (Ard)
> > >> - replace #ifdef CONFIG_BLK_DEV_INITRD with if
> > >>   (IS_ENABLED(CONFIG_BLK_DEV_INITRD) for consistency with other parts
> > >>   of arm64_memblock_init() (Rob)
> > >>
> > >> Changes in v3:
> > >>
> > >> - use C conditionals in drivers/of/fdt.c
> > >> - added check on phys_initrd_size in arch/arm64/mm/init.c to determine
> > >>   whether initrd_start must be populated
> > >> - fixed a build warning with ARC that was just missing an (unsigned
> > >>   long) cast
> > >>
> > >> Changes in v2:
> > >>
> > >> - get rid of ARCH_HAS_PHYS_INITRD and instead define
> > >>   phys_initrd_start/phys_initrd_size in init/do_mounts_initrd.c
> > >>
> > >> - make __early_init_dt_declare_initrd() account for ARM64 specific
> > >>   behavior with __va() when having CONFIG_DEBUG_VM enabled
> > >>
> > >> - consolidate early_initrd() command line parsing into
> > >>   init/do_mounts_initrd.c
> > >>
> > >> Because phys_initrd_start/phys_initrd_size are now compiled in
> > >> ini/do_mounts_initrd.c which is only built with CONFIG_BLK_DEV_INITRD=y,
> > >> we need to be a bit careful about the uses throughout architecture
> > >> specific code.
> > >>
> > >> 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
> > >>   of/fdt: Populate phys_initrd_start/phys_initrd_size from FDT
> > >>   arm64: Utilize phys_initrd_start/phys_initrd_size
> > >>   of/fdt: Remove custom __early_init_dt_declare_initrd() implementation
> > >>   arch: Move initrd= parsing into do_mounts_initrd.c
> > > 
> > > For the series:
> > > 
> > > Reviewed-by: Mike Rapoport <rppt@linux.ibm.com>
> > 
> > Thanks Mike, Rob, do you want to merge that series through the OF tree?
> 
> Sure, some arch maintainer acks would be nice.

I've now applied the series.

Thanks,

Rob