2018-10-30 23:11:21

by Florian Fainelli

[permalink] [raw]
Subject: [PATCH v2 0/6] arm64: Get rid of __early_init_dt_declare_initrd()

Hi all,

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 | 35 +++++++--------------------------
arch/nds32/mm/init.c | 2 --
arch/unicore32/mm/init.c | 24 +++++-----------------
drivers/of/fdt.c | 11 +++++++++--
include/linux/initrd.h | 3 +++
init/do_mounts_initrd.c | 20 +++++++++++++++++++
9 files changed, 51 insertions(+), 105 deletions(-)

--
2.17.1



2018-10-30 23:08:53

by Florian Fainelli

[permalink] [raw]
Subject: [PATCH v2 6/6] arch: Move initrd= parsing into do_mounts_initrd.c

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 ba145065c579..369d3d699929 100644
--- a/arch/arc/mm/init.c
+++ b/arch/arc/mm/init.c
@@ -79,24 +79,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
@@ -141,8 +123,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 = __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 4bfa08e27319..58ec68709606 100644
--- a/arch/arm/mm/init.c
+++ b/arch/arm/mm/init.c
@@ -52,23 +52,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 e95cee656a55..9045afacd10b 100644
--- a/arch/arm64/mm/init.c
+++ b/arch/arm64/mm/init.c
@@ -62,24 +62,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 f2f815d46846..99acdb829a7e 100644
--- a/arch/unicore32/mm/init.c
+++ b/arch/unicore32/mm/init.c
@@ -31,24 +31,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


2018-10-30 23:08:57

by Florian Fainelli

[permalink] [raw]
Subject: [PATCH v2 5/6] of/fdt: Remove custom __early_init_dt_declare_initrd() implementation

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 | 9 +++++++--
2 files changed, 7 insertions(+), 10 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 e34cb49231b5..f2b5becae96a 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -892,15 +892,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)
{
+ /* 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


2018-10-30 23:09:00

by Florian Fainelli

[permalink] [raw]
Subject: [PATCH v2 4/6] arm64: Utilize phys_initrd_start/phys_initrd_size

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 | 21 +++++++++------------
1 file changed, 9 insertions(+), 12 deletions(-)

diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
index 3cf87341859f..e95cee656a55 100644
--- a/arch/arm64/mm/init.c
+++ b/arch/arm64/mm/init.c
@@ -72,8 +72,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 +408,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 +460,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


2018-10-30 23:09:12

by Florian Fainelli

[permalink] [raw]
Subject: [PATCH v2 3/6] of/fdt: Populate phys_initrd_start/phys_initrd_size from FDT

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 87d59a53861d..4bfa08e27319 100644
--- a/arch/arm/mm/init.c
+++ b/arch/arm/mm/init.c
@@ -236,12 +236,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 76c83c1ffeda..e34cb49231b5 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -925,6 +925,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


2018-10-30 23:10:50

by Florian Fainelli

[permalink] [raw]
Subject: [PATCH v2 2/6] arch: Make phys_initrd_start and phys_initrd_size global variables

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 0cc8e04295a4..87d59a53861d 100644
--- a/arch/arm/mm/init.c
+++ b/arch/arm/mm/init.c
@@ -51,9 +51,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;
@@ -90,6 +88,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 8f8699e62bd5..f2f815d46846 100644
--- a/arch/unicore32/mm/init.c
+++ b/arch/unicore32/mm/init.c
@@ -31,9 +31,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;
@@ -49,6 +47,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
@@ -157,6 +156,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


2018-10-30 23:11:13

by Florian Fainelli

[permalink] [raw]
Subject: [PATCH v2 1/6] nds32: Remove phys_initrd_start and phys_initrd_size

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 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


2018-10-31 07:06:06

by Mike Rapoport

[permalink] [raw]
Subject: Re: [PATCH v2 4/6] arm64: Utilize phys_initrd_start/phys_initrd_size

On Tue, Oct 30, 2018 at 04:07:19PM -0700, 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 | 21 +++++++++------------
> 1 file changed, 9 insertions(+), 12 deletions(-)
>
> diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
> index 3cf87341859f..e95cee656a55 100644
> --- a/arch/arm64/mm/init.c
> +++ b/arch/arm64/mm/init.c
> @@ -72,8 +72,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 +408,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 +460,10 @@ void __init arm64_memblock_init(void)
> */
> memblock_reserve(__pa_symbol(_text), _end - _text);
> #ifdef CONFIG_BLK_DEV_INITRD
> - if (initrd_start) {

There may be no initrd at all, so the condition here would rather become

if (phys_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

I also wonder what is the reason to keep memstart_addr randomization and
initrd setup interleaved?

What we have now is roughly:

1) set memstart_addr
2) enforce memory_limit
3) reserve initrd
4) randomize memstart_addr
5) reserve text + data
6) reserve initrd again and set virtual addresses of initrd_{start,end}

Maybe it's possible to merge (3) into (6) ?

> early_init_fdt_scan_reserved_mem();
> --
> 2.17.1
>

--
Sincerely yours,
Mike.


2018-10-31 12:31:42

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH v2 5/6] of/fdt: Remove custom __early_init_dt_declare_initrd() implementation

On Tue, Oct 30, 2018 at 6:07 PM Florian Fainelli <[email protected]> wrote:
>
> 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 | 9 +++++++--
> 2 files changed, 7 insertions(+), 10 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 e34cb49231b5..f2b5becae96a 100644
> --- a/drivers/of/fdt.c
> +++ b/drivers/of/fdt.c
> @@ -892,15 +892,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)
> {
> + /* 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))

Use 'if' not '#if'. Use C code rather than preprocessor whenever possible.

> 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
>

2018-10-31 17:34:01

by Florian Fainelli

[permalink] [raw]
Subject: Re: [PATCH v2 4/6] arm64: Utilize phys_initrd_start/phys_initrd_size

On 10/31/18 12:03 AM, Mike Rapoport wrote:
> On Tue, Oct 30, 2018 at 04:07:19PM -0700, 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 | 21 +++++++++------------
>> 1 file changed, 9 insertions(+), 12 deletions(-)
>>
>> diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
>> index 3cf87341859f..e95cee656a55 100644
>> --- a/arch/arm64/mm/init.c
>> +++ b/arch/arm64/mm/init.c
>> @@ -72,8 +72,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 +408,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 +460,10 @@ void __init arm64_memblock_init(void)
>> */
>> memblock_reserve(__pa_symbol(_text), _end - _text);
>> #ifdef CONFIG_BLK_DEV_INITRD
>> - if (initrd_start) {
>
> There may be no initrd at all, so the condition here would rather become
>
> if (phys_initrd_start)

Or use phys_initrd_size, which would be consistent with how other
architectures typically test for this.

>
>> - 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
>
> I also wonder what is the reason to keep memstart_addr randomization and
> initrd setup interleaved?
>
> What we have now is roughly:
>
> 1) set memstart_addr
> 2) enforce memory_limit
> 3) reserve initrd
> 4) randomize memstart_addr
> 5) reserve text + data
> 6) reserve initrd again and set virtual addresses of initrd_{start,end}
>
> Maybe it's possible to merge (3) into (6) ?

That's kind of orthogonal to this patch series, but it's a valid
question, not sure I would want to tackle that just now though :)
--
Florian