Hi all,
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 validating the masked address against the memory
information in the passed DTB, to support booting 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.
Compared to v9[*], the major change is a step back, and only supporting
this on modern systems that pass the DTB explicitly, with improved
validation to avoid regressions.
This has been regression-tested on a variety of boards using a passed or
appended DTB.
Thanks for your comments!
[*] "[PATCH v9] ARM: boot: Validate start of physical memory against DTB"
(https://lore.kernel.org/r/[email protected])
Geert Uytterhoeven (3):
ARM: uncompress: Add be32tocpu macro
ARM: uncompress: Add OF_DT_MAGIC macro
ARM: uncompress: Validate start of physical memory against passed DTB
arch/arm/Kconfig | 7 +-
arch/arm/boot/compressed/Makefile | 5 +-
.../arm/boot/compressed/fdt_check_mem_start.c | 131 ++++++++++++++++++
arch/arm/boot/compressed/head.S | 70 +++++++---
4 files changed, 187 insertions(+), 26 deletions(-)
create mode 100644 arch/arm/boot/compressed/fdt_check_mem_start.c
--
2.25.1
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
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 validating the masked address against the memory
information in the passed DTB. Only use the start address
from DTB when masking would yield an out-of-range address, prefer the
traditional method in all other cases. Note that this applies only to the
explicitly passed DTB on modern systems, and not to a DTB appended to
the kernel, or to ATAGS. The appended DTB may need to be augmented by
information from ATAGS, which may need to rely on knowledge of the start
address of physical memory itself.
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]>
Suggested-by: Ard Biesheuvel <[email protected]>
Signed-off-by: Geert Uytterhoeven <[email protected]>
---
v10:
- Update for commit 9443076e4330a14a ("ARM: p2v: reduce p2v alignment
requirement to 2 MiB"),
- Use OF_DT_MAGIC macro,
- Rename fdt_get_mem_start() to fdt_check_mem_start(),
- Skip validation if there is an appended DTB,
- Pass mem_start to fdt_check_mem_start() to streamline code,
- Optimize register allocation,
- Update CONFIG_AUTO_ZRELADDR help text,
- Check all memory nodes and ranges (not just the first one), and
"linux,usable-memory", similar to early_init_dt_scan_memory(),
- Drop Reviewed-by, Tested-by tags,
v9:
- Add minlen parameter to getprop(), for better validation of
memory properties,
- Only use start of memory from the DTB if masking would yield an
out-of-range address, to fix kdump, as suggested by Ard.
v8:
- Rebase on top of commit 893ab00439a45513 ("kbuild: remove cc-option
test of -fno-stack-protector"),
v7:
- Rebase on top of commit 161e04a5bae58a65 ("ARM: decompressor: split
off _edata and stack base into separate object"),
v6:
- Rebase on top of commit 7ae4a78daacf240a ("ARM: 8969/1:
decompressor: simplify libfdt builds"),
- Include <linux/libfdt.h> instead of <libfdt.h>,
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/Kconfig | 7 +-
arch/arm/boot/compressed/Makefile | 5 +-
.../arm/boot/compressed/fdt_check_mem_start.c | 131 ++++++++++++++++++
arch/arm/boot/compressed/head.S | 32 ++++-
4 files changed, 168 insertions(+), 7 deletions(-)
create mode 100644 arch/arm/boot/compressed/fdt_check_mem_start.c
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index b2bf019dcefa6379..c341aa6fa862455c 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1908,9 +1908,10 @@ config AUTO_ZRELADDR
help
ZRELADDR is the physical address where the decompressed kernel
image will be placed. If AUTO_ZRELADDR is selected, the address
- will be determined at run-time by masking the current IP with
- 0xf8000000. This assumes the zImage being placed in the first 128MB
- from start of memory.
+ will be determined at run-time, either by masking the current IP
+ with 0xf8000000, or, if invalid, from the DTB passed in r2.
+ This assumes the zImage being placed in the first 128MB from
+ start of memory.
config EFI_STUB
bool
diff --git a/arch/arm/boot/compressed/Makefile b/arch/arm/boot/compressed/Makefile
index a815b1ae990d2d48..7361d45dc2ad603e 100644
--- a/arch/arm/boot/compressed/Makefile
+++ b/arch/arm/boot/compressed/Makefile
@@ -87,10 +87,13 @@ libfdt_objs := fdt_rw.o fdt_ro.o fdt_wip.o fdt.o
ifeq ($(CONFIG_ARM_ATAG_DTB_COMPAT),y)
OBJS += $(libfdt_objs) atags_to_fdt.o
endif
+ifeq ($(CONFIG_USE_OF),y)
+OBJS += $(libfdt_objs) fdt_check_mem_start.o
+endif
# -fstack-protector-strong triggers protection checks in this code,
# but it is being used too early to link to meaningful stack_chk logic.
-$(foreach o, $(libfdt_objs) atags_to_fdt.o, \
+$(foreach o, $(libfdt_objs) atags_to_fdt.o fdt_check_mem_start.o, \
$(eval CFLAGS_$(o) := -I $(srctree)/scripts/dtc/libfdt -fno-stack-protector))
# These were previously generated C files. When you are building the kernel
diff --git a/arch/arm/boot/compressed/fdt_check_mem_start.c b/arch/arm/boot/compressed/fdt_check_mem_start.c
new file mode 100644
index 0000000000000000..0bd39319d8a7f973
--- /dev/null
+++ b/arch/arm/boot/compressed/fdt_check_mem_start.c
@@ -0,0 +1,131 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include <linux/kernel.h>
+#include <linux/libfdt.h>
+#include <linux/sizes.h>
+
+static const void *get_prop(const void *fdt, const char *node_path,
+ const char *property, int minlen)
+{
+ const void *prop;
+ int offset, len;
+
+ offset = fdt_path_offset(fdt, node_path);
+ if (offset < 0)
+ return NULL;
+
+ prop = fdt_getprop(fdt, offset, property, &len);
+ if (!prop || len < minlen)
+ return NULL;
+
+ return prop;
+}
+
+static uint32_t get_cells(const void *fdt, const char *name)
+{
+ const fdt32_t *prop = get_prop(fdt, "/", name, sizeof(fdt32_t));
+
+ if (!prop) {
+ /* default */
+ return 1;
+ }
+
+ return fdt32_ld(prop);
+}
+
+static uint64_t get_val(const fdt32_t *cells, uint32_t ncells)
+{
+ uint64_t r = 0;
+
+ r = fdt32_ld(cells);
+ if (ncells > 1)
+ r = (r << 32) | fdt32_ld(cells + 1);
+
+ return r;
+}
+
+/*
+ * Check the start of physical memory
+ *
+ * Traditionally, the start address of physical memory is obtained by masking
+ * the program counter. However, this does require that this address is a
+ * multiple of 128 MiB, precluding booting Linux on platforms where this
+ * requirement is not fulfilled.
+ * Hence validate the calculated address against the memory information in the
+ * DTB, and, if out-of-range, replace it by the real start address.
+ * To preserve backwards compatibility (systems reserving a block of memory
+ * at the start of physical memory, kdump, ...), the traditional method is
+ * always used if it yields a valid address.
+ *
+ * Return value: start address of physical memory to use
+ */
+uint32_t fdt_check_mem_start(uint32_t mem_start, const void *fdt)
+{
+ uint32_t addr_cells, size_cells, base;
+ uint32_t fdt_mem_start = 0xffffffff;
+ const fdt32_t *reg, *endp;
+ uint64_t size, end;
+ const char *type;
+ int offset, len;
+
+ if (!fdt)
+ return mem_start;
+
+ if (fdt_magic(fdt) != FDT_MAGIC)
+ return mem_start;
+
+ /* There may be multiple cells on LPAE platforms */
+ addr_cells = get_cells(fdt, "#address-cells");
+ size_cells = get_cells(fdt, "#size-cells");
+ if (addr_cells > 2 || size_cells > 2)
+ return mem_start;
+
+ /* Walk all memory nodes and regions */
+ for (offset = fdt_next_node(fdt, -1, NULL); offset >= 0;
+ offset = fdt_next_node(fdt, offset, NULL)) {
+ type = fdt_getprop(fdt, offset, "device_type", NULL);
+ if (!type || strcmp(type, "memory"))
+ continue;
+
+ reg = fdt_getprop(fdt, offset, "linux,usable-memory", &len);
+ if (!reg)
+ reg = fdt_getprop(fdt, offset, "reg", &len);
+ if (!reg)
+ continue;
+
+ for (endp = reg + (len / sizeof(fdt32_t));
+ endp - reg >= addr_cells + size_cells;
+ reg += addr_cells + size_cells) {
+ size = get_val(reg + addr_cells, size_cells);
+ if (!size)
+ continue;
+
+ if (addr_cells > 1 && fdt32_ld(reg)) {
+ /* Outside 32-bit address space, skipping */
+ continue;
+ }
+
+ base = fdt32_ld(reg + addr_cells - 1);
+ end = base + size;
+ if (mem_start >= base && mem_start < end) {
+ /* Calculated address is valid, use it */
+ return mem_start;
+ }
+
+ if (base < fdt_mem_start)
+ fdt_mem_start = base;
+ }
+ }
+
+ if (fdt_mem_start == 0xffffffff) {
+ /* No usable memory found, falling back to default */
+ return mem_start;
+ }
+
+ /*
+ * The calculated address is not usable.
+ * Use the lowest usable physical memory address from the DTB instead,
+ * and make sure this is a multiple of 2 MiB for phys/virt patching.
+ */
+ return round_up(fdt_mem_start, SZ_2M);
+}
diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
index d9cce7238a365081..1b6425df87e84e71 100644
--- a/arch/arm/boot/compressed/head.S
+++ b/arch/arm/boot/compressed/head.S
@@ -282,10 +282,36 @@ not_angel:
* are already placing their zImage in (eg) the top 64MB
* of this range.
*/
- mov r4, pc
- and r4, r4, #0xf8000000
+ mov r0, pc
+ and r0, r0, #0xf8000000
+#ifdef CONFIG_USE_OF
+ adr r1, LC1
+#ifdef CONFIG_ARM_APPENDED_DTB
+ /*
+ * Look for an appended DTB. If found, we cannot use it to
+ * validate the calculated start of physical memory, as its
+ * memory nodes may need to be augmented by ATAGS stored at
+ * an offset from the same start of physical memory.
+ */
+ ldr r2, [r1, #4] @ get &_edata
+ add r2, r2, r1 @ relocate it
+ ldr r2, [r2] @ get DTB signature
+ ldr r3, =OF_DT_MAGIC
+ cmp r2, r3 @ do we have a DTB there?
+ beq 1f @ if yes, skip validation
+#endif /* CONFIG_ARM_APPENDED_DTB */
+
+ /* Make sure we have some stack */
+ ldr sp, [r1] @ get stack location
+ add sp, sp, r1 @ apply relocation
+
+ /* Validate calculated start against passed DTB */
+ mov r1, r8
+ bl fdt_check_mem_start
+1:
+#endif /* CONFIG_USE_OF */
/* Determine final kernel image address. */
- add r4, r4, #TEXT_OFFSET
+ add r4, r0, #TEXT_OFFSET
#else
ldr r4, =zreladdr
#endif
--
2.25.1
DTB stores all values as 32-bit big-endian integers.
Add a macro to convert such values to native CPU endianness, to reduce
duplication.
Signed-off-by: Geert Uytterhoeven <[email protected]>
---
v10:
- New.
---
arch/arm/boot/compressed/head.S | 26 ++++++++++++--------------
1 file changed, 12 insertions(+), 14 deletions(-)
diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
index 835ce64f1674c9a2..aabdc544c03aafdc 100644
--- a/arch/arm/boot/compressed/head.S
+++ b/arch/arm/boot/compressed/head.S
@@ -165,6 +165,16 @@
orr \res, \res, \tmp1, lsl #24
.endm
+ .macro be32tocpu, val, tmp
+#ifndef __ARMEB__
+ /* convert to little endian */
+ eor \tmp, \val, \val, ror #16
+ bic \tmp, \tmp, #0x00ff0000
+ mov \val, \val, ror #8
+ eor \val, \val, \tmp, lsr #8
+#endif
+ .endm
+
.section ".start", "ax"
/*
* sort out different calling conventions
@@ -345,13 +355,7 @@ restart: adr r0, LC1
/* Get the initial DTB size */
ldr r5, [r6, #4]
-#ifndef __ARMEB__
- /* convert to little endian */
- eor r1, r5, r5, ror #16
- bic r1, r1, #0x00ff0000
- mov r5, r5, ror #8
- eor r5, r5, r1, lsr #8
-#endif
+ be32tocpu r5, r1
dbgadtb r6, r5
/* 50% DTB growth should be good enough */
add r5, r5, r5, lsr #1
@@ -403,13 +407,7 @@ restart: adr r0, LC1
/* Get the current DTB size */
ldr r5, [r6, #4]
-#ifndef __ARMEB__
- /* convert r5 (dtb size) to little endian */
- eor r1, r5, r5, ror #16
- bic r1, r1, #0x00ff0000
- mov r5, r5, ror #8
- eor r5, r5, r1, lsr #8
-#endif
+ be32tocpu r5, r1
/* preserve 64-bit alignment */
add r5, r5, #7
--
2.25.1
The DTB magic marker is stored as a 32-bit big-endian value, and thus
depends on the CPU's endianness. Add a macro to define this value in
native endianness, to reduce #ifdef clutter and (future) duplication.
Signed-off-by: Geert Uytterhoeven <[email protected]>
---
v10:
- New.
---
arch/arm/boot/compressed/head.S | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
index aabdc544c03aafdc..d9cce7238a365081 100644
--- a/arch/arm/boot/compressed/head.S
+++ b/arch/arm/boot/compressed/head.S
@@ -11,6 +11,12 @@
#include "efi-header.S"
+#ifdef __ARMEB__
+#define OF_DT_MAGIC 0xd00dfeed
+#else
+#define OF_DT_MAGIC 0xedfe0dd0
+#endif
+
AR_CLASS( .arch armv7-a )
M_CLASS( .arch armv7-m )
@@ -335,11 +341,7 @@ restart: adr r0, LC1
*/
ldr lr, [r6, #0]
-#ifndef __ARMEB__
- ldr r1, =0xedfe0dd0 @ sig is 0xd00dfeed big endian
-#else
- ldr r1, =0xd00dfeed
-#endif
+ ldr r1, =OF_DT_MAGIC
cmp lr, r1
bne dtb_check_done @ not found
--
2.25.1
Hallo Geert,
On Thu, 3 Dec 2020 at 13:19, Geert Uytterhoeven <[email protected]> wrote:
>
> DTB stores all values as 32-bit big-endian integers.
> Add a macro to convert such values to native CPU endianness, to reduce
> duplication.
>
> Signed-off-by: Geert Uytterhoeven <[email protected]>
> ---
> v10:
> - New.
> ---
> arch/arm/boot/compressed/head.S | 26 ++++++++++++--------------
> 1 file changed, 12 insertions(+), 14 deletions(-)
>
> diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
> index 835ce64f1674c9a2..aabdc544c03aafdc 100644
> --- a/arch/arm/boot/compressed/head.S
> +++ b/arch/arm/boot/compressed/head.S
> @@ -165,6 +165,16 @@
> orr \res, \res, \tmp1, lsl #24
> .endm
>
> + .macro be32tocpu, val, tmp
> +#ifndef __ARMEB__
> + /* convert to little endian */
> + eor \tmp, \val, \val, ror #16
> + bic \tmp, \tmp, #0x00ff0000
> + mov \val, \val, ror #8
> + eor \val, \val, \tmp, lsr #8
> +#endif
> + .endm
> +
Thanks for cleaning this up.
This patch is fine as is.
Reviewed-by: Ard Biesheuvel <[email protected]>
However, I would like to note that, since we are touching this code,
we might do it properly. (Given the v10, feel free to ignore or defer
to a later time)
- put this into asm/assembler.h so other .S files can use it
- use rev instructions when supported
- factor out the difference between xx_to cpu and bswap
I.e.,
.macro bswap32, val, tmp
.if __LINUX_ARM_ARCH__ >= 6
rev \val, \val
.else
eor \tmp, \val, \val, ror #16
bic \tmp, \tmp, #0x00ff0000
mov \val, \val, ror #8
eor \val, \val, \tmp, lsr #8
.endif
.endm
.macro be32tocpu, val, tmp
#ifndef __ARMEB__
bswap32 \val, \tmp
#endif
.endm
.macro le32tocpu, val, tmp
#ifdef __ARMEB__
bswap32 \val, \tmp
#endif
.endm
This would allow us to reuse this macro in
arch/arm/crypto/aes-cipher-core.S, and replace the __rev macro it
defines.
> .section ".start", "ax"
> /*
> * sort out different calling conventions
> @@ -345,13 +355,7 @@ restart: adr r0, LC1
>
> /* Get the initial DTB size */
> ldr r5, [r6, #4]
> -#ifndef __ARMEB__
> - /* convert to little endian */
> - eor r1, r5, r5, ror #16
> - bic r1, r1, #0x00ff0000
> - mov r5, r5, ror #8
> - eor r5, r5, r1, lsr #8
> -#endif
> + be32tocpu r5, r1
> dbgadtb r6, r5
> /* 50% DTB growth should be good enough */
> add r5, r5, r5, lsr #1
> @@ -403,13 +407,7 @@ restart: adr r0, LC1
>
> /* Get the current DTB size */
> ldr r5, [r6, #4]
> -#ifndef __ARMEB__
> - /* convert r5 (dtb size) to little endian */
> - eor r1, r5, r5, ror #16
> - bic r1, r1, #0x00ff0000
> - mov r5, r5, ror #8
> - eor r5, r5, r1, lsr #8
> -#endif
> + be32tocpu r5, r1
>
> /* preserve 64-bit alignment */
> add r5, r5, #7
> --
> 2.25.1
>
On Thu, 3 Dec 2020 at 13:19, Geert Uytterhoeven <[email protected]> wrote:
>
> The DTB magic marker is stored as a 32-bit big-endian value, and thus
> depends on the CPU's endianness. Add a macro to define this value in
> native endianness, to reduce #ifdef clutter and (future) duplication.
>
> Signed-off-by: Geert Uytterhoeven <[email protected]>
Reviewed-by: Ard Biesheuvel <[email protected]>
> ---
> v10:
> - New.
> ---
> arch/arm/boot/compressed/head.S | 12 +++++++-----
> 1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
> index aabdc544c03aafdc..d9cce7238a365081 100644
> --- a/arch/arm/boot/compressed/head.S
> +++ b/arch/arm/boot/compressed/head.S
> @@ -11,6 +11,12 @@
>
> #include "efi-header.S"
>
> +#ifdef __ARMEB__
> +#define OF_DT_MAGIC 0xd00dfeed
> +#else
> +#define OF_DT_MAGIC 0xedfe0dd0
> +#endif
> +
> AR_CLASS( .arch armv7-a )
> M_CLASS( .arch armv7-m )
>
> @@ -335,11 +341,7 @@ restart: adr r0, LC1
> */
>
> ldr lr, [r6, #0]
> -#ifndef __ARMEB__
> - ldr r1, =0xedfe0dd0 @ sig is 0xd00dfeed big endian
> -#else
> - ldr r1, =0xd00dfeed
> -#endif
> + ldr r1, =OF_DT_MAGIC
> cmp lr, r1
> bne dtb_check_done @ not found
>
> --
> 2.25.1
>
On Thu, 3 Dec 2020 at 13:19, 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 validating the masked address against the memory
> information in the passed DTB. Only use the start address
> from DTB when masking would yield an out-of-range address, prefer the
> traditional method in all other cases. Note that this applies only to the
> explicitly passed DTB on modern systems, and not to a DTB appended to
> the kernel, or to ATAGS. The appended DTB may need to be augmented by
> information from ATAGS, which may need to rely on knowledge of the start
> address of physical memory itself.
>
> 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]>
> Suggested-by: Ard Biesheuvel <[email protected]>
> Signed-off-by: Geert Uytterhoeven <[email protected]>
Reviewed-by: Ard Biesheuvel <[email protected]>
Unfortunately, this may still blindly overwrite memory that is marked
as reserved in the device tree, but the current state isn't any
better, so we can fix that another time. (This would allow us to get
rid of the games we play with TEXT_OFFSET.)
> ---
> v10:
> - Update for commit 9443076e4330a14a ("ARM: p2v: reduce p2v alignment
> requirement to 2 MiB"),
> - Use OF_DT_MAGIC macro,
> - Rename fdt_get_mem_start() to fdt_check_mem_start(),
> - Skip validation if there is an appended DTB,
> - Pass mem_start to fdt_check_mem_start() to streamline code,
> - Optimize register allocation,
> - Update CONFIG_AUTO_ZRELADDR help text,
> - Check all memory nodes and ranges (not just the first one), and
> "linux,usable-memory", similar to early_init_dt_scan_memory(),
> - Drop Reviewed-by, Tested-by tags,
>
> v9:
> - Add minlen parameter to getprop(), for better validation of
> memory properties,
> - Only use start of memory from the DTB if masking would yield an
> out-of-range address, to fix kdump, as suggested by Ard.
>
> v8:
> - Rebase on top of commit 893ab00439a45513 ("kbuild: remove cc-option
> test of -fno-stack-protector"),
>
> v7:
> - Rebase on top of commit 161e04a5bae58a65 ("ARM: decompressor: split
> off _edata and stack base into separate object"),
>
> v6:
> - Rebase on top of commit 7ae4a78daacf240a ("ARM: 8969/1:
> decompressor: simplify libfdt builds"),
> - Include <linux/libfdt.h> instead of <libfdt.h>,
>
> 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/Kconfig | 7 +-
> arch/arm/boot/compressed/Makefile | 5 +-
> .../arm/boot/compressed/fdt_check_mem_start.c | 131 ++++++++++++++++++
> arch/arm/boot/compressed/head.S | 32 ++++-
> 4 files changed, 168 insertions(+), 7 deletions(-)
> create mode 100644 arch/arm/boot/compressed/fdt_check_mem_start.c
>
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index b2bf019dcefa6379..c341aa6fa862455c 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -1908,9 +1908,10 @@ config AUTO_ZRELADDR
> help
> ZRELADDR is the physical address where the decompressed kernel
> image will be placed. If AUTO_ZRELADDR is selected, the address
> - will be determined at run-time by masking the current IP with
> - 0xf8000000. This assumes the zImage being placed in the first 128MB
> - from start of memory.
> + will be determined at run-time, either by masking the current IP
> + with 0xf8000000, or, if invalid, from the DTB passed in r2.
> + This assumes the zImage being placed in the first 128MB from
> + start of memory.
>
> config EFI_STUB
> bool
> diff --git a/arch/arm/boot/compressed/Makefile b/arch/arm/boot/compressed/Makefile
> index a815b1ae990d2d48..7361d45dc2ad603e 100644
> --- a/arch/arm/boot/compressed/Makefile
> +++ b/arch/arm/boot/compressed/Makefile
> @@ -87,10 +87,13 @@ libfdt_objs := fdt_rw.o fdt_ro.o fdt_wip.o fdt.o
> ifeq ($(CONFIG_ARM_ATAG_DTB_COMPAT),y)
> OBJS += $(libfdt_objs) atags_to_fdt.o
> endif
> +ifeq ($(CONFIG_USE_OF),y)
> +OBJS += $(libfdt_objs) fdt_check_mem_start.o
> +endif
>
> # -fstack-protector-strong triggers protection checks in this code,
> # but it is being used too early to link to meaningful stack_chk logic.
> -$(foreach o, $(libfdt_objs) atags_to_fdt.o, \
> +$(foreach o, $(libfdt_objs) atags_to_fdt.o fdt_check_mem_start.o, \
> $(eval CFLAGS_$(o) := -I $(srctree)/scripts/dtc/libfdt -fno-stack-protector))
>
> # These were previously generated C files. When you are building the kernel
> diff --git a/arch/arm/boot/compressed/fdt_check_mem_start.c b/arch/arm/boot/compressed/fdt_check_mem_start.c
> new file mode 100644
> index 0000000000000000..0bd39319d8a7f973
> --- /dev/null
> +++ b/arch/arm/boot/compressed/fdt_check_mem_start.c
> @@ -0,0 +1,131 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +#include <linux/kernel.h>
> +#include <linux/libfdt.h>
> +#include <linux/sizes.h>
> +
> +static const void *get_prop(const void *fdt, const char *node_path,
> + const char *property, int minlen)
> +{
> + const void *prop;
> + int offset, len;
> +
> + offset = fdt_path_offset(fdt, node_path);
> + if (offset < 0)
> + return NULL;
> +
> + prop = fdt_getprop(fdt, offset, property, &len);
> + if (!prop || len < minlen)
> + return NULL;
> +
> + return prop;
> +}
> +
> +static uint32_t get_cells(const void *fdt, const char *name)
> +{
> + const fdt32_t *prop = get_prop(fdt, "/", name, sizeof(fdt32_t));
> +
> + if (!prop) {
> + /* default */
> + return 1;
> + }
> +
> + return fdt32_ld(prop);
> +}
> +
> +static uint64_t get_val(const fdt32_t *cells, uint32_t ncells)
> +{
> + uint64_t r = 0;
> +
> + r = fdt32_ld(cells);
> + if (ncells > 1)
> + r = (r << 32) | fdt32_ld(cells + 1);
> +
> + return r;
> +}
> +
> +/*
> + * Check the start of physical memory
> + *
> + * Traditionally, the start address of physical memory is obtained by masking
> + * the program counter. However, this does require that this address is a
> + * multiple of 128 MiB, precluding booting Linux on platforms where this
> + * requirement is not fulfilled.
> + * Hence validate the calculated address against the memory information in the
> + * DTB, and, if out-of-range, replace it by the real start address.
> + * To preserve backwards compatibility (systems reserving a block of memory
> + * at the start of physical memory, kdump, ...), the traditional method is
> + * always used if it yields a valid address.
> + *
> + * Return value: start address of physical memory to use
> + */
> +uint32_t fdt_check_mem_start(uint32_t mem_start, const void *fdt)
> +{
> + uint32_t addr_cells, size_cells, base;
> + uint32_t fdt_mem_start = 0xffffffff;
> + const fdt32_t *reg, *endp;
> + uint64_t size, end;
> + const char *type;
> + int offset, len;
> +
> + if (!fdt)
> + return mem_start;
> +
> + if (fdt_magic(fdt) != FDT_MAGIC)
> + return mem_start;
> +
> + /* There may be multiple cells on LPAE platforms */
> + addr_cells = get_cells(fdt, "#address-cells");
> + size_cells = get_cells(fdt, "#size-cells");
> + if (addr_cells > 2 || size_cells > 2)
> + return mem_start;
> +
> + /* Walk all memory nodes and regions */
> + for (offset = fdt_next_node(fdt, -1, NULL); offset >= 0;
> + offset = fdt_next_node(fdt, offset, NULL)) {
> + type = fdt_getprop(fdt, offset, "device_type", NULL);
> + if (!type || strcmp(type, "memory"))
> + continue;
> +
> + reg = fdt_getprop(fdt, offset, "linux,usable-memory", &len);
> + if (!reg)
> + reg = fdt_getprop(fdt, offset, "reg", &len);
> + if (!reg)
> + continue;
> +
> + for (endp = reg + (len / sizeof(fdt32_t));
> + endp - reg >= addr_cells + size_cells;
> + reg += addr_cells + size_cells) {
> + size = get_val(reg + addr_cells, size_cells);
> + if (!size)
> + continue;
> +
> + if (addr_cells > 1 && fdt32_ld(reg)) {
> + /* Outside 32-bit address space, skipping */
> + continue;
> + }
> +
> + base = fdt32_ld(reg + addr_cells - 1);
> + end = base + size;
> + if (mem_start >= base && mem_start < end) {
> + /* Calculated address is valid, use it */
> + return mem_start;
> + }
> +
> + if (base < fdt_mem_start)
> + fdt_mem_start = base;
> + }
> + }
> +
> + if (fdt_mem_start == 0xffffffff) {
> + /* No usable memory found, falling back to default */
> + return mem_start;
> + }
> +
> + /*
> + * The calculated address is not usable.
> + * Use the lowest usable physical memory address from the DTB instead,
> + * and make sure this is a multiple of 2 MiB for phys/virt patching.
> + */
> + return round_up(fdt_mem_start, SZ_2M);
> +}
> diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
> index d9cce7238a365081..1b6425df87e84e71 100644
> --- a/arch/arm/boot/compressed/head.S
> +++ b/arch/arm/boot/compressed/head.S
> @@ -282,10 +282,36 @@ not_angel:
> * are already placing their zImage in (eg) the top 64MB
> * of this range.
> */
> - mov r4, pc
> - and r4, r4, #0xf8000000
> + mov r0, pc
> + and r0, r0, #0xf8000000
> +#ifdef CONFIG_USE_OF
> + adr r1, LC1
> +#ifdef CONFIG_ARM_APPENDED_DTB
> + /*
> + * Look for an appended DTB. If found, we cannot use it to
> + * validate the calculated start of physical memory, as its
> + * memory nodes may need to be augmented by ATAGS stored at
> + * an offset from the same start of physical memory.
> + */
> + ldr r2, [r1, #4] @ get &_edata
> + add r2, r2, r1 @ relocate it
> + ldr r2, [r2] @ get DTB signature
> + ldr r3, =OF_DT_MAGIC
> + cmp r2, r3 @ do we have a DTB there?
> + beq 1f @ if yes, skip validation
> +#endif /* CONFIG_ARM_APPENDED_DTB */
> +
> + /* Make sure we have some stack */
> + ldr sp, [r1] @ get stack location
> + add sp, sp, r1 @ apply relocation
> +
> + /* Validate calculated start against passed DTB */
> + mov r1, r8
> + bl fdt_check_mem_start
> +1:
> +#endif /* CONFIG_USE_OF */
> /* Determine final kernel image address. */
> - add r4, r4, #TEXT_OFFSET
> + add r4, r0, #TEXT_OFFSET
> #else
> ldr r4, =zreladdr
> #endif
> --
> 2.25.1
>
On Thu, Dec 03, 2020 at 01:19:16PM +0100, Geert Uytterhoeven wrote:
> diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
> index d9cce7238a365081..1b6425df87e84e71 100644
> --- a/arch/arm/boot/compressed/head.S
> +++ b/arch/arm/boot/compressed/head.S
> @@ -282,10 +282,36 @@ not_angel:
> * are already placing their zImage in (eg) the top 64MB
> * of this range.
> */
> - mov r4, pc
> - and r4, r4, #0xf8000000
> + mov r0, pc
> + and r0, r0, #0xf8000000
> +#ifdef CONFIG_USE_OF
> + adr r1, LC1
> +#ifdef CONFIG_ARM_APPENDED_DTB
> + /*
> + * Look for an appended DTB. If found, we cannot use it to
> + * validate the calculated start of physical memory, as its
> + * memory nodes may need to be augmented by ATAGS stored at
> + * an offset from the same start of physical memory.
> + */
> + ldr r2, [r1, #4] @ get &_edata
> + add r2, r2, r1 @ relocate it
> + ldr r2, [r2] @ get DTB signature
> + ldr r3, =OF_DT_MAGIC
> + cmp r2, r3 @ do we have a DTB there?
> + beq 1f @ if yes, skip validation
> +#endif /* CONFIG_ARM_APPENDED_DTB */
> +
> + /* Make sure we have some stack */
> + ldr sp, [r1] @ get stack location
> + add sp, sp, r1 @ apply relocation
> +
> + /* Validate calculated start against passed DTB */
> + mov r1, r8
> + bl fdt_check_mem_start
I don't think this is going to work. You can only run C code when
it has been linked for a specific address and is loaded at the
correct address as it contains absolute addresses. We work around
that in the decompressor by (ab)using the GOT table, and the code
that fixes up the GOT table is run later, after you've called out
to some C code here.
Quite how this works for you without the GOT fixups having been
done, I'm not sure.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Hi Russell,
On Thu, Dec 3, 2020 at 3:05 PM Russell King - ARM Linux admin
<[email protected]> wrote:
> On Thu, Dec 03, 2020 at 01:19:16PM +0100, Geert Uytterhoeven wrote:
> > diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
> > index d9cce7238a365081..1b6425df87e84e71 100644
> > --- a/arch/arm/boot/compressed/head.S
> > +++ b/arch/arm/boot/compressed/head.S
> > @@ -282,10 +282,36 @@ not_angel:
> > * are already placing their zImage in (eg) the top 64MB
> > * of this range.
> > */
> > - mov r4, pc
> > - and r4, r4, #0xf8000000
> > + mov r0, pc
> > + and r0, r0, #0xf8000000
> > +#ifdef CONFIG_USE_OF
> > + adr r1, LC1
> > +#ifdef CONFIG_ARM_APPENDED_DTB
> > + /*
> > + * Look for an appended DTB. If found, we cannot use it to
> > + * validate the calculated start of physical memory, as its
> > + * memory nodes may need to be augmented by ATAGS stored at
> > + * an offset from the same start of physical memory.
> > + */
> > + ldr r2, [r1, #4] @ get &_edata
> > + add r2, r2, r1 @ relocate it
> > + ldr r2, [r2] @ get DTB signature
> > + ldr r3, =OF_DT_MAGIC
> > + cmp r2, r3 @ do we have a DTB there?
> > + beq 1f @ if yes, skip validation
> > +#endif /* CONFIG_ARM_APPENDED_DTB */
> > +
> > + /* Make sure we have some stack */
> > + ldr sp, [r1] @ get stack location
> > + add sp, sp, r1 @ apply relocation
> > +
> > + /* Validate calculated start against passed DTB */
> > + mov r1, r8
> > + bl fdt_check_mem_start
>
> I don't think this is going to work. You can only run C code when
> it has been linked for a specific address and is loaded at the
> correct address as it contains absolute addresses. We work around
> that in the decompressor by (ab)using the GOT table, and the code
> that fixes up the GOT table is run later, after you've called out
> to some C code here.
>
> Quite how this works for you without the GOT fixups having been
> done, I'm not sure.
It works in the same way as the existing atags_to_fdt() works:
* [...] No GOT fixup has occurred
* yet, but none of the code we're about to call uses any
* global variable.
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
On Thu, 3 Dec 2020, Geert Uytterhoeven wrote:
> DTB stores all values as 32-bit big-endian integers.
> Add a macro to convert such values to native CPU endianness, to reduce
> duplication.
>
> Signed-off-by: Geert Uytterhoeven <[email protected]>
I agree with Ard's suggestions. In any case:
Reviewed-by: Nicolas Pitre <[email protected]>
> ---
> v10:
> - New.
> ---
> arch/arm/boot/compressed/head.S | 26 ++++++++++++--------------
> 1 file changed, 12 insertions(+), 14 deletions(-)
>
> diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
> index 835ce64f1674c9a2..aabdc544c03aafdc 100644
> --- a/arch/arm/boot/compressed/head.S
> +++ b/arch/arm/boot/compressed/head.S
> @@ -165,6 +165,16 @@
> orr \res, \res, \tmp1, lsl #24
> .endm
>
> + .macro be32tocpu, val, tmp
> +#ifndef __ARMEB__
> + /* convert to little endian */
> + eor \tmp, \val, \val, ror #16
> + bic \tmp, \tmp, #0x00ff0000
> + mov \val, \val, ror #8
> + eor \val, \val, \tmp, lsr #8
> +#endif
> + .endm
> +
> .section ".start", "ax"
> /*
> * sort out different calling conventions
> @@ -345,13 +355,7 @@ restart: adr r0, LC1
>
> /* Get the initial DTB size */
> ldr r5, [r6, #4]
> -#ifndef __ARMEB__
> - /* convert to little endian */
> - eor r1, r5, r5, ror #16
> - bic r1, r1, #0x00ff0000
> - mov r5, r5, ror #8
> - eor r5, r5, r1, lsr #8
> -#endif
> + be32tocpu r5, r1
> dbgadtb r6, r5
> /* 50% DTB growth should be good enough */
> add r5, r5, r5, lsr #1
> @@ -403,13 +407,7 @@ restart: adr r0, LC1
>
> /* Get the current DTB size */
> ldr r5, [r6, #4]
> -#ifndef __ARMEB__
> - /* convert r5 (dtb size) to little endian */
> - eor r1, r5, r5, ror #16
> - bic r1, r1, #0x00ff0000
> - mov r5, r5, ror #8
> - eor r5, r5, r1, lsr #8
> -#endif
> + be32tocpu r5, r1
>
> /* preserve 64-bit alignment */
> add r5, r5, #7
> --
> 2.25.1
>
>
On Thu, 3 Dec 2020, Geert Uytterhoeven wrote:
> The DTB magic marker is stored as a 32-bit big-endian value, and thus
> depends on the CPU's endianness. Add a macro to define this value in
> native endianness, to reduce #ifdef clutter and (future) duplication.
>
> Signed-off-by: Geert Uytterhoeven <[email protected]>
Reviewed-by: Nicolas Pitre <[email protected]>
> ---
> v10:
> - New.
> ---
> arch/arm/boot/compressed/head.S | 12 +++++++-----
> 1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
> index aabdc544c03aafdc..d9cce7238a365081 100644
> --- a/arch/arm/boot/compressed/head.S
> +++ b/arch/arm/boot/compressed/head.S
> @@ -11,6 +11,12 @@
>
> #include "efi-header.S"
>
> +#ifdef __ARMEB__
> +#define OF_DT_MAGIC 0xd00dfeed
> +#else
> +#define OF_DT_MAGIC 0xedfe0dd0
> +#endif
> +
> AR_CLASS( .arch armv7-a )
> M_CLASS( .arch armv7-m )
>
> @@ -335,11 +341,7 @@ restart: adr r0, LC1
> */
>
> ldr lr, [r6, #0]
> -#ifndef __ARMEB__
> - ldr r1, =0xedfe0dd0 @ sig is 0xd00dfeed big endian
> -#else
> - ldr r1, =0xd00dfeed
> -#endif
> + ldr r1, =OF_DT_MAGIC
> cmp lr, r1
> bne dtb_check_done @ not found
>
> --
> 2.25.1
>
>
On Thu, 3 Dec 2020, Geert Uytterhoeven 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 validating the masked address against the memory
> information in the passed DTB. Only use the start address
> from DTB when masking would yield an out-of-range address, prefer the
> traditional method in all other cases. Note that this applies only to the
> explicitly passed DTB on modern systems, and not to a DTB appended to
> the kernel, or to ATAGS. The appended DTB may need to be augmented by
> information from ATAGS, which may need to rely on knowledge of the start
> address of physical memory itself.
>
> 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]>
> Suggested-by: Ard Biesheuvel <[email protected]>
> Signed-off-by: Geert Uytterhoeven <[email protected]>
I'm not that intimate with dt contents so:
Acked-by: Nicolas Pitre <[email protected]>
And it would be a good idea to repeat the GOT fixup caviat comment
before calling fdt_check_mem_start.
> ---
> v10:
> - Update for commit 9443076e4330a14a ("ARM: p2v: reduce p2v alignment
> requirement to 2 MiB"),
> - Use OF_DT_MAGIC macro,
> - Rename fdt_get_mem_start() to fdt_check_mem_start(),
> - Skip validation if there is an appended DTB,
> - Pass mem_start to fdt_check_mem_start() to streamline code,
> - Optimize register allocation,
> - Update CONFIG_AUTO_ZRELADDR help text,
> - Check all memory nodes and ranges (not just the first one), and
> "linux,usable-memory", similar to early_init_dt_scan_memory(),
> - Drop Reviewed-by, Tested-by tags,
>
> v9:
> - Add minlen parameter to getprop(), for better validation of
> memory properties,
> - Only use start of memory from the DTB if masking would yield an
> out-of-range address, to fix kdump, as suggested by Ard.
>
> v8:
> - Rebase on top of commit 893ab00439a45513 ("kbuild: remove cc-option
> test of -fno-stack-protector"),
>
> v7:
> - Rebase on top of commit 161e04a5bae58a65 ("ARM: decompressor: split
> off _edata and stack base into separate object"),
>
> v6:
> - Rebase on top of commit 7ae4a78daacf240a ("ARM: 8969/1:
> decompressor: simplify libfdt builds"),
> - Include <linux/libfdt.h> instead of <libfdt.h>,
>
> 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/Kconfig | 7 +-
> arch/arm/boot/compressed/Makefile | 5 +-
> .../arm/boot/compressed/fdt_check_mem_start.c | 131 ++++++++++++++++++
> arch/arm/boot/compressed/head.S | 32 ++++-
> 4 files changed, 168 insertions(+), 7 deletions(-)
> create mode 100644 arch/arm/boot/compressed/fdt_check_mem_start.c
>
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index b2bf019dcefa6379..c341aa6fa862455c 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -1908,9 +1908,10 @@ config AUTO_ZRELADDR
> help
> ZRELADDR is the physical address where the decompressed kernel
> image will be placed. If AUTO_ZRELADDR is selected, the address
> - will be determined at run-time by masking the current IP with
> - 0xf8000000. This assumes the zImage being placed in the first 128MB
> - from start of memory.
> + will be determined at run-time, either by masking the current IP
> + with 0xf8000000, or, if invalid, from the DTB passed in r2.
> + This assumes the zImage being placed in the first 128MB from
> + start of memory.
>
> config EFI_STUB
> bool
> diff --git a/arch/arm/boot/compressed/Makefile b/arch/arm/boot/compressed/Makefile
> index a815b1ae990d2d48..7361d45dc2ad603e 100644
> --- a/arch/arm/boot/compressed/Makefile
> +++ b/arch/arm/boot/compressed/Makefile
> @@ -87,10 +87,13 @@ libfdt_objs := fdt_rw.o fdt_ro.o fdt_wip.o fdt.o
> ifeq ($(CONFIG_ARM_ATAG_DTB_COMPAT),y)
> OBJS += $(libfdt_objs) atags_to_fdt.o
> endif
> +ifeq ($(CONFIG_USE_OF),y)
> +OBJS += $(libfdt_objs) fdt_check_mem_start.o
> +endif
>
> # -fstack-protector-strong triggers protection checks in this code,
> # but it is being used too early to link to meaningful stack_chk logic.
> -$(foreach o, $(libfdt_objs) atags_to_fdt.o, \
> +$(foreach o, $(libfdt_objs) atags_to_fdt.o fdt_check_mem_start.o, \
> $(eval CFLAGS_$(o) := -I $(srctree)/scripts/dtc/libfdt -fno-stack-protector))
>
> # These were previously generated C files. When you are building the kernel
> diff --git a/arch/arm/boot/compressed/fdt_check_mem_start.c b/arch/arm/boot/compressed/fdt_check_mem_start.c
> new file mode 100644
> index 0000000000000000..0bd39319d8a7f973
> --- /dev/null
> +++ b/arch/arm/boot/compressed/fdt_check_mem_start.c
> @@ -0,0 +1,131 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +#include <linux/kernel.h>
> +#include <linux/libfdt.h>
> +#include <linux/sizes.h>
> +
> +static const void *get_prop(const void *fdt, const char *node_path,
> + const char *property, int minlen)
> +{
> + const void *prop;
> + int offset, len;
> +
> + offset = fdt_path_offset(fdt, node_path);
> + if (offset < 0)
> + return NULL;
> +
> + prop = fdt_getprop(fdt, offset, property, &len);
> + if (!prop || len < minlen)
> + return NULL;
> +
> + return prop;
> +}
> +
> +static uint32_t get_cells(const void *fdt, const char *name)
> +{
> + const fdt32_t *prop = get_prop(fdt, "/", name, sizeof(fdt32_t));
> +
> + if (!prop) {
> + /* default */
> + return 1;
> + }
> +
> + return fdt32_ld(prop);
> +}
> +
> +static uint64_t get_val(const fdt32_t *cells, uint32_t ncells)
> +{
> + uint64_t r = 0;
> +
> + r = fdt32_ld(cells);
> + if (ncells > 1)
> + r = (r << 32) | fdt32_ld(cells + 1);
> +
> + return r;
> +}
> +
> +/*
> + * Check the start of physical memory
> + *
> + * Traditionally, the start address of physical memory is obtained by masking
> + * the program counter. However, this does require that this address is a
> + * multiple of 128 MiB, precluding booting Linux on platforms where this
> + * requirement is not fulfilled.
> + * Hence validate the calculated address against the memory information in the
> + * DTB, and, if out-of-range, replace it by the real start address.
> + * To preserve backwards compatibility (systems reserving a block of memory
> + * at the start of physical memory, kdump, ...), the traditional method is
> + * always used if it yields a valid address.
> + *
> + * Return value: start address of physical memory to use
> + */
> +uint32_t fdt_check_mem_start(uint32_t mem_start, const void *fdt)
> +{
> + uint32_t addr_cells, size_cells, base;
> + uint32_t fdt_mem_start = 0xffffffff;
> + const fdt32_t *reg, *endp;
> + uint64_t size, end;
> + const char *type;
> + int offset, len;
> +
> + if (!fdt)
> + return mem_start;
> +
> + if (fdt_magic(fdt) != FDT_MAGIC)
> + return mem_start;
> +
> + /* There may be multiple cells on LPAE platforms */
> + addr_cells = get_cells(fdt, "#address-cells");
> + size_cells = get_cells(fdt, "#size-cells");
> + if (addr_cells > 2 || size_cells > 2)
> + return mem_start;
> +
> + /* Walk all memory nodes and regions */
> + for (offset = fdt_next_node(fdt, -1, NULL); offset >= 0;
> + offset = fdt_next_node(fdt, offset, NULL)) {
> + type = fdt_getprop(fdt, offset, "device_type", NULL);
> + if (!type || strcmp(type, "memory"))
> + continue;
> +
> + reg = fdt_getprop(fdt, offset, "linux,usable-memory", &len);
> + if (!reg)
> + reg = fdt_getprop(fdt, offset, "reg", &len);
> + if (!reg)
> + continue;
> +
> + for (endp = reg + (len / sizeof(fdt32_t));
> + endp - reg >= addr_cells + size_cells;
> + reg += addr_cells + size_cells) {
> + size = get_val(reg + addr_cells, size_cells);
> + if (!size)
> + continue;
> +
> + if (addr_cells > 1 && fdt32_ld(reg)) {
> + /* Outside 32-bit address space, skipping */
> + continue;
> + }
> +
> + base = fdt32_ld(reg + addr_cells - 1);
> + end = base + size;
> + if (mem_start >= base && mem_start < end) {
> + /* Calculated address is valid, use it */
> + return mem_start;
> + }
> +
> + if (base < fdt_mem_start)
> + fdt_mem_start = base;
> + }
> + }
> +
> + if (fdt_mem_start == 0xffffffff) {
> + /* No usable memory found, falling back to default */
> + return mem_start;
> + }
> +
> + /*
> + * The calculated address is not usable.
> + * Use the lowest usable physical memory address from the DTB instead,
> + * and make sure this is a multiple of 2 MiB for phys/virt patching.
> + */
> + return round_up(fdt_mem_start, SZ_2M);
> +}
> diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
> index d9cce7238a365081..1b6425df87e84e71 100644
> --- a/arch/arm/boot/compressed/head.S
> +++ b/arch/arm/boot/compressed/head.S
> @@ -282,10 +282,36 @@ not_angel:
> * are already placing their zImage in (eg) the top 64MB
> * of this range.
> */
> - mov r4, pc
> - and r4, r4, #0xf8000000
> + mov r0, pc
> + and r0, r0, #0xf8000000
> +#ifdef CONFIG_USE_OF
> + adr r1, LC1
> +#ifdef CONFIG_ARM_APPENDED_DTB
> + /*
> + * Look for an appended DTB. If found, we cannot use it to
> + * validate the calculated start of physical memory, as its
> + * memory nodes may need to be augmented by ATAGS stored at
> + * an offset from the same start of physical memory.
> + */
> + ldr r2, [r1, #4] @ get &_edata
> + add r2, r2, r1 @ relocate it
> + ldr r2, [r2] @ get DTB signature
> + ldr r3, =OF_DT_MAGIC
> + cmp r2, r3 @ do we have a DTB there?
> + beq 1f @ if yes, skip validation
> +#endif /* CONFIG_ARM_APPENDED_DTB */
> +
> + /* Make sure we have some stack */
> + ldr sp, [r1] @ get stack location
> + add sp, sp, r1 @ apply relocation
> +
> + /* Validate calculated start against passed DTB */
> + mov r1, r8
> + bl fdt_check_mem_start
> +1:
> +#endif /* CONFIG_USE_OF */
> /* Determine final kernel image address. */
> - add r4, r4, #TEXT_OFFSET
> + add r4, r0, #TEXT_OFFSET
> #else
> ldr r4, =zreladdr
> #endif
> --
> 2.25.1
>
>
On Thu, Dec 3, 2020 at 5:27 PM Nicolas Pitre <[email protected]> wrote:
> On Thu, 3 Dec 2020, Geert Uytterhoeven wrote:
> > The DTB magic marker is stored as a 32-bit big-endian value, and thus
> > depends on the CPU's endianness. Add a macro to define this value in
> > native endianness, to reduce #ifdef clutter and (future) duplication.
> >
> > Signed-off-by: Geert Uytterhoeven <[email protected]>
>
> Reviewed-by: Nicolas Pitre <[email protected]>
Thanks, submitted to rmk's patch tracker.
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
Hi Ard,
On Thu, Dec 3, 2020 at 1:50 PM Ard Biesheuvel <[email protected]> wrote:
> On Thu, 3 Dec 2020 at 13:19, Geert Uytterhoeven <[email protected]> wrote:
> > DTB stores all values as 32-bit big-endian integers.
> > Add a macro to convert such values to native CPU endianness, to reduce
> > duplication.
> >
> > Signed-off-by: Geert Uytterhoeven <[email protected]>
> > ---
> > v10:
> > - New.
> > ---
> > arch/arm/boot/compressed/head.S | 26 ++++++++++++--------------
> > 1 file changed, 12 insertions(+), 14 deletions(-)
> >
> > diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
> > index 835ce64f1674c9a2..aabdc544c03aafdc 100644
> > --- a/arch/arm/boot/compressed/head.S
> > +++ b/arch/arm/boot/compressed/head.S
> > @@ -165,6 +165,16 @@
> > orr \res, \res, \tmp1, lsl #24
> > .endm
> >
> > + .macro be32tocpu, val, tmp
> > +#ifndef __ARMEB__
> > + /* convert to little endian */
> > + eor \tmp, \val, \val, ror #16
> > + bic \tmp, \tmp, #0x00ff0000
> > + mov \val, \val, ror #8
> > + eor \val, \val, \tmp, lsr #8
> > +#endif
> > + .endm
> > +
>
> Thanks for cleaning this up.
>
> This patch is fine as is.
>
> Reviewed-by: Ard Biesheuvel <[email protected]>
Thanks, submitted to rmk's patch tracker.
> However, I would like to note that, since we are touching this code,
> we might do it properly. (Given the v10, feel free to ignore or defer
> to a later time)
[...]
Thanks for the suggestion, adding to my list.
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