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
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 <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
> 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.
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 <[email protected]>
Thanks Mike, Rob, do you want to merge that series through the OF tree?
--
Florian
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 <[email protected]>
>
> Thanks Mike, Rob, do you want to merge that series through the OF tree?
Sure, some arch maintainer acks would be nice.
Rob
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 <[email protected]>
> ---
> 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);
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 <[email protected]>
>> ---
>> 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
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
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
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
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 <[email protected]>
> ---
> arch/arm64/mm/init.c | 20 ++++++++------------
> 1 file changed, 8 insertions(+), 12 deletions(-)
Looks ok to me:
Acked-by: Will Deacon <[email protected]>
Will
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
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 <[email protected]>
> >
> > 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