2020-04-16 00:15:09

by Geert Uytterhoeven

[permalink] [raw]
Subject: [PATCH v5] ARM: boot: Obtain start of physical memory from DTB

Currently, the start address of physical memory is obtained by masking
the program counter with a fixed mask of 0xf8000000. This mask value
was chosen as a balance between the requirements of different platforms.
However, this does require that the start address of physical memory is
a multiple of 128 MiB, precluding booting Linux on platforms where this
requirement is not fulfilled.

Fix this limitation by obtaining the start address from the DTB instead,
if available (either explicitly passed, or appended to the kernel).
Fall back to the traditional method when needed.

This allows to boot Linux on r7s9210/rza2mevb using the 64 MiB of SDRAM
on the RZA2MEVB sub board, which is located at 0x0C000000 (CS3 space),
i.e. not at a multiple of 128 MiB.

Suggested-by: Nicolas Pitre <[email protected]>
Signed-off-by: Geert Uytterhoeven <[email protected]>
Reviewed-by: Nicolas Pitre <[email protected]>
Reviewed-by: Ard Biesheuvel <[email protected]>
Tested-by: Marek Szyprowski <[email protected]>
Tested-by: Dmitry Osipenko <[email protected]>
---
v5:
- Add Tested-by, Reviewed-by,
- Round up start of memory to satisfy 16 MiB alignment rule,

v4:
- Fix stack location after commit 184bf653a7a452c1 ("ARM:
decompressor: factor out routine to obtain the inflated image
size"),

v3:
- Add Reviewed-by,
- Fix ATAGs with appended DTB,
- Add Tested-by,

v2:
- Use "cmp r0, #-1", instead of "cmn r0, #1",
- Add missing stack setup,
- Support appended DTB.
---
arch/arm/boot/compressed/Makefile | 6 ++-
arch/arm/boot/compressed/fdt_get_mem_start.c | 57 ++++++++++++++++++++
arch/arm/boot/compressed/head.S | 54 ++++++++++++++++++-
3 files changed, 115 insertions(+), 2 deletions(-)
create mode 100644 arch/arm/boot/compressed/fdt_get_mem_start.c

diff --git a/arch/arm/boot/compressed/Makefile b/arch/arm/boot/compressed/Makefile
index 9c11e7490292f0e0..82e4cee97cb5d905 100644
--- a/arch/arm/boot/compressed/Makefile
+++ b/arch/arm/boot/compressed/Makefile
@@ -86,12 +86,15 @@ libfdt_objs := $(addsuffix .o, $(basename $(libfdt)))
$(addprefix $(obj)/,$(libfdt) $(libfdt_hdrs)): $(obj)/%: $(srctree)/scripts/dtc/libfdt/%
$(call cmd,shipped)

-$(addprefix $(obj)/,$(libfdt_objs) atags_to_fdt.o): \
+$(addprefix $(obj)/,$(libfdt_objs) atags_to_fdt.o fdt_get_mem_start.o): \
$(addprefix $(obj)/,$(libfdt_hdrs))

ifeq ($(CONFIG_ARM_ATAG_DTB_COMPAT),y)
OBJS += $(libfdt_objs) atags_to_fdt.o
endif
+ifeq ($(CONFIG_USE_OF),y)
+OBJS += $(libfdt_objs) fdt_get_mem_start.o
+endif

targets := vmlinux vmlinux.lds piggy_data piggy.o \
lib1funcs.o ashldi3.o bswapsdi2.o \
@@ -115,6 +118,7 @@ CFLAGS_fdt.o := $(nossp-flags-y)
CFLAGS_fdt_ro.o := $(nossp-flags-y)
CFLAGS_fdt_rw.o := $(nossp-flags-y)
CFLAGS_fdt_wip.o := $(nossp-flags-y)
+CFLAGS_fdt_get_mem_start.o := $(nossp-flags-y)

ccflags-y := -fpic $(call cc-option,-mno-single-pic-base,) -fno-builtin \
-I$(obj) $(DISABLE_ARM_SSP_PER_TASK_PLUGIN)
diff --git a/arch/arm/boot/compressed/fdt_get_mem_start.c b/arch/arm/boot/compressed/fdt_get_mem_start.c
new file mode 100644
index 0000000000000000..e29caa4cbfecb36a
--- /dev/null
+++ b/arch/arm/boot/compressed/fdt_get_mem_start.c
@@ -0,0 +1,57 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include <linux/kernel.h>
+#include <linux/sizes.h>
+
+#include <libfdt.h>
+
+static const void *getprop(const void *fdt, const char *node_path,
+ const char *property)
+{
+ int offset = fdt_path_offset(fdt, node_path);
+
+ if (offset == -FDT_ERR_NOTFOUND)
+ return NULL;
+
+ return fdt_getprop(fdt, offset, property, NULL);
+}
+
+static uint32_t get_addr_size(const void *fdt)
+{
+ const __be32 *addr_len = getprop(fdt, "/", "#address-cells");
+
+ if (!addr_len) {
+ /* default */
+ return 1;
+ }
+
+ return fdt32_to_cpu(*addr_len);
+}
+
+/*
+ * Get the start of physical memory
+ */
+
+unsigned long fdt_get_mem_start(const void *fdt)
+{
+ uint32_t addr_size, mem_start;
+ const __be32 *memory;
+
+ if (!fdt)
+ return -1;
+
+ if (*(__be32 *)fdt != cpu_to_fdt32(FDT_MAGIC))
+ return -1;
+
+ /* Find the first memory node */
+ memory = getprop(fdt, "/memory", "reg");
+ if (!memory)
+ return -1;
+
+ /* There may be multiple cells on LPAE platforms */
+ addr_size = get_addr_size(fdt);
+
+ mem_start = fdt32_to_cpu(memory[addr_size - 1]);
+ /* Must be a multiple of 16 MiB for phys/virt patching */
+ return round_up(mem_start, SZ_16M);
+}
diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
index cabdd8f4a2482e2b..2d2a42865b3974da 100644
--- a/arch/arm/boot/compressed/head.S
+++ b/arch/arm/boot/compressed/head.S
@@ -254,8 +254,58 @@ not_angel:
.text

#ifdef CONFIG_AUTO_ZRELADDR
+#ifdef CONFIG_USE_OF
/*
- * Find the start of physical memory. As we are executing
+ * Find the start of physical memory.
+ * Try the DTB first, if available.
+ */
+ adr r0, LC0
+ ldr r1, [r0] @ get absolute LC0
+ ldr sp, [r0, #24] @ get stack location
+ sub r1, r0, r1 @ compute relocation offset
+ add sp, sp, r1 @ apply relocation
+
+#ifdef CONFIG_ARM_APPENDED_DTB
+ /*
+ * Look for an appended DTB. If found, use it and
+ * move stack away from it.
+ */
+ ldr r6, [r0, #12] @ get &_edata
+ add r6, r6, r1 @ relocate it
+ ldmia r6, {r0, r5} @ get DTB signature and size
+#ifndef __ARMEB__
+ ldr r1, =0xedfe0dd0 @ sig is 0xd00dfeed big endian
+ /* convert DTB size to little endian */
+ eor r2, r5, r5, ror #16
+ bic r2, r2, #0x00ff0000
+ mov r5, r5, ror #8
+ eor r5, r5, r2, lsr #8
+#else
+ ldr r1, =0xd00dfeed
+#endif
+ cmp r0, r1 @ do we have a DTB there?
+ bne 1f
+
+ /* preserve 64-bit alignment */
+ add r5, r5, #7
+ bic r5, r5, #7
+ add sp, sp, r5 @ if so, move stack above DTB
+ mov r0, r6 @ and extract memory start from DTB
+ b 2f
+
+1:
+#endif /* CONFIG_ARM_APPENDED_DTB */
+
+ mov r0, r8
+2:
+ bl fdt_get_mem_start
+ mov r4, r0
+ cmp r0, #-1
+ bne 1f
+#endif /* CONFIG_USE_OF */
+
+ /*
+ * Fall back to the traditional method. As we are executing
* without the MMU on, we are in the physical address space.
* We just need to get rid of any offset by aligning the
* address.
@@ -273,6 +323,8 @@ not_angel:
*/
mov r4, pc
and r4, r4, #0xf8000000
+
+1:
/* Determine final kernel image address. */
add r4, r4, #TEXT_OFFSET
#else
--
2.17.1


2020-04-21 15:23:21

by Ard Biesheuvel

[permalink] [raw]
Subject: Re: [PATCH v5] ARM: boot: Obtain start of physical memory from DTB

On Wed, 15 Apr 2020 at 17:34, Geert Uytterhoeven
<[email protected]> wrote:
>
> Currently, the start address of physical memory is obtained by masking
> the program counter with a fixed mask of 0xf8000000. This mask value
> was chosen as a balance between the requirements of different platforms.
> However, this does require that the start address of physical memory is
> a multiple of 128 MiB, precluding booting Linux on platforms where this
> requirement is not fulfilled.
>
> Fix this limitation by obtaining the start address from the DTB instead,
> if available (either explicitly passed, or appended to the kernel).
> Fall back to the traditional method when needed.
>
> This allows to boot Linux on r7s9210/rza2mevb using the 64 MiB of SDRAM
> on the RZA2MEVB sub board, which is located at 0x0C000000 (CS3 space),
> i.e. not at a multiple of 128 MiB.
>
> Suggested-by: Nicolas Pitre <[email protected]>
> Signed-off-by: Geert Uytterhoeven <[email protected]>
> Reviewed-by: Nicolas Pitre <[email protected]>
> Reviewed-by: Ard Biesheuvel <[email protected]>
> Tested-by: Marek Szyprowski <[email protected]>
> Tested-by: Dmitry Osipenko <[email protected]>

This is ready to go into the patch system, no?

The sooner Russell picks it up, the sooner I can respin my patches
that go on top.

Thanks,

> ---
> v5:
> - Add Tested-by, Reviewed-by,
> - Round up start of memory to satisfy 16 MiB alignment rule,
>
> v4:
> - Fix stack location after commit 184bf653a7a452c1 ("ARM:
> decompressor: factor out routine to obtain the inflated image
> size"),
>
> v3:
> - Add Reviewed-by,
> - Fix ATAGs with appended DTB,
> - Add Tested-by,
>
> v2:
> - Use "cmp r0, #-1", instead of "cmn r0, #1",
> - Add missing stack setup,
> - Support appended DTB.
> ---
> arch/arm/boot/compressed/Makefile | 6 ++-
> arch/arm/boot/compressed/fdt_get_mem_start.c | 57 ++++++++++++++++++++
> arch/arm/boot/compressed/head.S | 54 ++++++++++++++++++-
> 3 files changed, 115 insertions(+), 2 deletions(-)
> create mode 100644 arch/arm/boot/compressed/fdt_get_mem_start.c
>
> diff --git a/arch/arm/boot/compressed/Makefile b/arch/arm/boot/compressed/Makefile
> index 9c11e7490292f0e0..82e4cee97cb5d905 100644
> --- a/arch/arm/boot/compressed/Makefile
> +++ b/arch/arm/boot/compressed/Makefile
> @@ -86,12 +86,15 @@ libfdt_objs := $(addsuffix .o, $(basename $(libfdt)))
> $(addprefix $(obj)/,$(libfdt) $(libfdt_hdrs)): $(obj)/%: $(srctree)/scripts/dtc/libfdt/%
> $(call cmd,shipped)
>
> -$(addprefix $(obj)/,$(libfdt_objs) atags_to_fdt.o): \
> +$(addprefix $(obj)/,$(libfdt_objs) atags_to_fdt.o fdt_get_mem_start.o): \
> $(addprefix $(obj)/,$(libfdt_hdrs))
>
> ifeq ($(CONFIG_ARM_ATAG_DTB_COMPAT),y)
> OBJS += $(libfdt_objs) atags_to_fdt.o
> endif
> +ifeq ($(CONFIG_USE_OF),y)
> +OBJS += $(libfdt_objs) fdt_get_mem_start.o
> +endif
>
> targets := vmlinux vmlinux.lds piggy_data piggy.o \
> lib1funcs.o ashldi3.o bswapsdi2.o \
> @@ -115,6 +118,7 @@ CFLAGS_fdt.o := $(nossp-flags-y)
> CFLAGS_fdt_ro.o := $(nossp-flags-y)
> CFLAGS_fdt_rw.o := $(nossp-flags-y)
> CFLAGS_fdt_wip.o := $(nossp-flags-y)
> +CFLAGS_fdt_get_mem_start.o := $(nossp-flags-y)
>
> ccflags-y := -fpic $(call cc-option,-mno-single-pic-base,) -fno-builtin \
> -I$(obj) $(DISABLE_ARM_SSP_PER_TASK_PLUGIN)
> diff --git a/arch/arm/boot/compressed/fdt_get_mem_start.c b/arch/arm/boot/compressed/fdt_get_mem_start.c
> new file mode 100644
> index 0000000000000000..e29caa4cbfecb36a
> --- /dev/null
> +++ b/arch/arm/boot/compressed/fdt_get_mem_start.c
> @@ -0,0 +1,57 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +#include <linux/kernel.h>
> +#include <linux/sizes.h>
> +
> +#include <libfdt.h>
> +
> +static const void *getprop(const void *fdt, const char *node_path,
> + const char *property)
> +{
> + int offset = fdt_path_offset(fdt, node_path);
> +
> + if (offset == -FDT_ERR_NOTFOUND)
> + return NULL;
> +
> + return fdt_getprop(fdt, offset, property, NULL);
> +}
> +
> +static uint32_t get_addr_size(const void *fdt)
> +{
> + const __be32 *addr_len = getprop(fdt, "/", "#address-cells");
> +
> + if (!addr_len) {
> + /* default */
> + return 1;
> + }
> +
> + return fdt32_to_cpu(*addr_len);
> +}
> +
> +/*
> + * Get the start of physical memory
> + */
> +
> +unsigned long fdt_get_mem_start(const void *fdt)
> +{
> + uint32_t addr_size, mem_start;
> + const __be32 *memory;
> +
> + if (!fdt)
> + return -1;
> +
> + if (*(__be32 *)fdt != cpu_to_fdt32(FDT_MAGIC))
> + return -1;
> +
> + /* Find the first memory node */
> + memory = getprop(fdt, "/memory", "reg");
> + if (!memory)
> + return -1;
> +
> + /* There may be multiple cells on LPAE platforms */
> + addr_size = get_addr_size(fdt);
> +
> + mem_start = fdt32_to_cpu(memory[addr_size - 1]);
> + /* Must be a multiple of 16 MiB for phys/virt patching */
> + return round_up(mem_start, SZ_16M);
> +}
> diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
> index cabdd8f4a2482e2b..2d2a42865b3974da 100644
> --- a/arch/arm/boot/compressed/head.S
> +++ b/arch/arm/boot/compressed/head.S
> @@ -254,8 +254,58 @@ not_angel:
> .text
>
> #ifdef CONFIG_AUTO_ZRELADDR
> +#ifdef CONFIG_USE_OF
> /*
> - * Find the start of physical memory. As we are executing
> + * Find the start of physical memory.
> + * Try the DTB first, if available.
> + */
> + adr r0, LC0
> + ldr r1, [r0] @ get absolute LC0
> + ldr sp, [r0, #24] @ get stack location
> + sub r1, r0, r1 @ compute relocation offset
> + add sp, sp, r1 @ apply relocation
> +
> +#ifdef CONFIG_ARM_APPENDED_DTB
> + /*
> + * Look for an appended DTB. If found, use it and
> + * move stack away from it.
> + */
> + ldr r6, [r0, #12] @ get &_edata
> + add r6, r6, r1 @ relocate it
> + ldmia r6, {r0, r5} @ get DTB signature and size
> +#ifndef __ARMEB__
> + ldr r1, =0xedfe0dd0 @ sig is 0xd00dfeed big endian
> + /* convert DTB size to little endian */
> + eor r2, r5, r5, ror #16
> + bic r2, r2, #0x00ff0000
> + mov r5, r5, ror #8
> + eor r5, r5, r2, lsr #8
> +#else
> + ldr r1, =0xd00dfeed
> +#endif
> + cmp r0, r1 @ do we have a DTB there?
> + bne 1f
> +
> + /* preserve 64-bit alignment */
> + add r5, r5, #7
> + bic r5, r5, #7
> + add sp, sp, r5 @ if so, move stack above DTB
> + mov r0, r6 @ and extract memory start from DTB
> + b 2f
> +
> +1:
> +#endif /* CONFIG_ARM_APPENDED_DTB */
> +
> + mov r0, r8
> +2:
> + bl fdt_get_mem_start
> + mov r4, r0
> + cmp r0, #-1
> + bne 1f
> +#endif /* CONFIG_USE_OF */
> +
> + /*
> + * Fall back to the traditional method. As we are executing
> * without the MMU on, we are in the physical address space.
> * We just need to get rid of any offset by aligning the
> * address.
> @@ -273,6 +323,8 @@ not_angel:
> */
> mov r4, pc
> and r4, r4, #0xf8000000
> +
> +1:
> /* Determine final kernel image address. */
> add r4, r4, #TEXT_OFFSET
> #else
> --
> 2.17.1
>

2020-04-21 15:33:13

by Dmitry Osipenko

[permalink] [raw]
Subject: Re: [PATCH v5] ARM: boot: Obtain start of physical memory from DTB

21.04.2020 18:19, Ard Biesheuvel пишет:
> On Wed, 15 Apr 2020 at 17:34, Geert Uytterhoeven
> <[email protected]> wrote:
>>
>> Currently, the start address of physical memory is obtained by masking
>> the program counter with a fixed mask of 0xf8000000. This mask value
>> was chosen as a balance between the requirements of different platforms.
>> However, this does require that the start address of physical memory is
>> a multiple of 128 MiB, precluding booting Linux on platforms where this
>> requirement is not fulfilled.
>>
>> Fix this limitation by obtaining the start address from the DTB instead,
>> if available (either explicitly passed, or appended to the kernel).
>> Fall back to the traditional method when needed.
>>
>> This allows to boot Linux on r7s9210/rza2mevb using the 64 MiB of SDRAM
>> on the RZA2MEVB sub board, which is located at 0x0C000000 (CS3 space),
>> i.e. not at a multiple of 128 MiB.
>>
>> Suggested-by: Nicolas Pitre <[email protected]>
>> Signed-off-by: Geert Uytterhoeven <[email protected]>
>> Reviewed-by: Nicolas Pitre <[email protected]>
>> Reviewed-by: Ard Biesheuvel <[email protected]>
>> Tested-by: Marek Szyprowski <[email protected]>
>> Tested-by: Dmitry Osipenko <[email protected]>
>
> This is ready to go into the patch system, no?
>
> The sooner Russell picks it up, the sooner I can respin my patches
> that go on top.
>
> Thanks,

The v5 works for me on NVIDIA Tegra, just in case.

2020-04-21 16:04:01

by Russell King (Oracle)

[permalink] [raw]
Subject: Re: [PATCH v5] ARM: boot: Obtain start of physical memory from DTB

On Tue, Apr 21, 2020 at 05:19:40PM +0200, Ard Biesheuvel wrote:
> On Wed, 15 Apr 2020 at 17:34, Geert Uytterhoeven
> <[email protected]> wrote:
> >
> > Currently, the start address of physical memory is obtained by masking
> > the program counter with a fixed mask of 0xf8000000. This mask value
> > was chosen as a balance between the requirements of different platforms.
> > However, this does require that the start address of physical memory is
> > a multiple of 128 MiB, precluding booting Linux on platforms where this
> > requirement is not fulfilled.
> >
> > Fix this limitation by obtaining the start address from the DTB instead,
> > if available (either explicitly passed, or appended to the kernel).
> > Fall back to the traditional method when needed.
> >
> > This allows to boot Linux on r7s9210/rza2mevb using the 64 MiB of SDRAM
> > on the RZA2MEVB sub board, which is located at 0x0C000000 (CS3 space),
> > i.e. not at a multiple of 128 MiB.
> >
> > Suggested-by: Nicolas Pitre <[email protected]>
> > Signed-off-by: Geert Uytterhoeven <[email protected]>
> > Reviewed-by: Nicolas Pitre <[email protected]>
> > Reviewed-by: Ard Biesheuvel <[email protected]>
> > Tested-by: Marek Szyprowski <[email protected]>
> > Tested-by: Dmitry Osipenko <[email protected]>
>
> This is ready to go into the patch system, no?
>
> The sooner Russell picks it up, the sooner I can respin my patches
> that go on top.

This seems to be a particularly risky change (it's already been subject
to various failures for people) so I do not intend to rush to pick it
up.

In any case, Masahiro Yamada has resubmitted a patch to sort out the
libfdt builds that he's been trying to get merged for some time now,
so I'm going to be giving that priority. Your change conflicts with
this libfdt build change.

So, I think all in all, it needs to spend a bit longer being provenly
tested before I merged it (and eventually fixed up for the libfdt
change), and I don't think merging it so it appears in linux-next
will help with that.

--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 10.2Mbps down 587kbps up

2020-04-21 16:21:03

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH v5] ARM: boot: Obtain start of physical memory from DTB

Hi Russell,

On Tue, Apr 21, 2020 at 6:01 PM Russell King - ARM Linux admin
<[email protected]> wrote:
> On Tue, Apr 21, 2020 at 05:19:40PM +0200, Ard Biesheuvel wrote:
> > On Wed, 15 Apr 2020 at 17:34, Geert Uytterhoeven
> > <[email protected]> wrote:
> > > Currently, the start address of physical memory is obtained by masking
> > > the program counter with a fixed mask of 0xf8000000. This mask value
> > > was chosen as a balance between the requirements of different platforms.
> > > However, this does require that the start address of physical memory is
> > > a multiple of 128 MiB, precluding booting Linux on platforms where this
> > > requirement is not fulfilled.
> > >
> > > Fix this limitation by obtaining the start address from the DTB instead,
> > > if available (either explicitly passed, or appended to the kernel).
> > > Fall back to the traditional method when needed.
> > >
> > > This allows to boot Linux on r7s9210/rza2mevb using the 64 MiB of SDRAM
> > > on the RZA2MEVB sub board, which is located at 0x0C000000 (CS3 space),
> > > i.e. not at a multiple of 128 MiB.
> > >
> > > Suggested-by: Nicolas Pitre <[email protected]>
> > > Signed-off-by: Geert Uytterhoeven <[email protected]>
> > > Reviewed-by: Nicolas Pitre <[email protected]>
> > > Reviewed-by: Ard Biesheuvel <[email protected]>
> > > Tested-by: Marek Szyprowski <[email protected]>
> > > Tested-by: Dmitry Osipenko <[email protected]>
> >
> > This is ready to go into the patch system, no?
> >
> > The sooner Russell picks it up, the sooner I can respin my patches
> > that go on top.
>
> This seems to be a particularly risky change (it's already been subject
> to various failures for people) so I do not intend to rush to pick it
> up.

Yeah, I'm fully aware head.S is fragile ;-)

> In any case, Masahiro Yamada has resubmitted a patch to sort out the
> libfdt builds that he's been trying to get merged for some time now,
> so I'm going to be giving that priority. Your change conflicts with
> this libfdt build change.

OK, will resubmit after his changes have landed (in your tree?).

> So, I think all in all, it needs to spend a bit longer being provenly
> tested before I merged it (and eventually fixed up for the libfdt
> change), and I don't think merging it so it appears in linux-next
> will help with that.

Please note that I also have a DTS patch that depends on this.
Hence if this patch doesn't make it into v5.8, the board support DTS
patch that depends on this will have to be postponed one more cycle,
too...

Thanks!

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds