Hi all,
This series fixed some issues on bootloader - kernel
interface.
The first two fixed booting with devicetree, the last two
enhanced kernel's tolerance on different bootloader implementation.
Please review.
Thanks
Signed-off-by: Jiaxun Yang <[email protected]>
---
Changes in v3:
- Polish all individual patches with comments received offline.
- Link to v2: https://lore.kernel.org/r/[email protected]
Changes in v2:
- Enhance PATCH 3-4 based on off list discussions with Huacai & co.
- Link to v1: https://lore.kernel.org/r/[email protected]
---
Jiaxun Yang (4):
LoongArch: Fix built-in DTB detection
LoongArch: smp: Add all CPUs enabled by fdt to NUMA node 0
LoongArch: Fix entry point in image header
LoongArch: Override higher address bits in JUMP_VIRT_ADDR
arch/loongarch/include/asm/stackframe.h | 2 +-
arch/loongarch/kernel/head.S | 2 +-
arch/loongarch/kernel/setup.c | 6 ++++--
arch/loongarch/kernel/smp.c | 5 ++++-
arch/loongarch/kernel/vmlinux.lds.S | 2 ++
drivers/firmware/efi/libstub/loongarch.c | 2 +-
6 files changed, 13 insertions(+), 6 deletions(-)
---
base-commit: 124cfbcd6d185d4f50be02d5f5afe61578916773
change-id: 20240521-loongarch-booting-fixes-366e13e7ca55
Best regards,
--
Jiaxun Yang <[email protected]>
fdt_check_header(__dtb_start) will always success because kernel
provided a dummy dtb, and by coincidence __dtb_start clashed with
entry of this dummy dtb. The consequence is fdt passed from
firmware will never be taken.
Fix by trying to utilise __dtb_start only when CONFIG_BUILTIN_DTB
is enabled.
Cc: [email protected]
Fixes: 7b937cc243e5 ("of: Create of_root if no dtb provided by firmware")
Signed-off-by: Jiaxun Yang <[email protected]>
---
v3: Better reasoning in commit message, thanks Binbin and Huacai!
---
arch/loongarch/kernel/setup.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/arch/loongarch/kernel/setup.c b/arch/loongarch/kernel/setup.c
index 60e0fe97f61a..ea6d5db6c878 100644
--- a/arch/loongarch/kernel/setup.c
+++ b/arch/loongarch/kernel/setup.c
@@ -275,16 +275,18 @@ static void __init arch_reserve_crashkernel(void)
static void __init fdt_setup(void)
{
#ifdef CONFIG_OF_EARLY_FLATTREE
- void *fdt_pointer;
+ void *fdt_pointer = NULL;
/* ACPI-based systems do not require parsing fdt */
if (acpi_os_get_root_pointer())
return;
+#ifdef CONFIG_BUILTIN_DTB
/* Prefer to use built-in dtb, checking its legality first. */
if (!fdt_check_header(__dtb_start))
fdt_pointer = __dtb_start;
- else
+#endif
+ if (!fdt_pointer)
fdt_pointer = efi_fdt_pointer(); /* Fallback to firmware dtb */
if (!fdt_pointer || fdt_check_header(fdt_pointer))
--
2.43.0
In JUMP_VIRT_ADDR we are performing an or calculation on
address value directly from pcaddi.
This will only work if we are currently running from direct
1:1 mapping addresses or firmware's DMW is configured exactly
same as kernel. Still, we should not rely on such assumption.
Fix by overriding higher bits in address comes from pcaddi,
so we can get rid of or operator.
Cc: [email protected]
Signed-off-by: Jiaxun Yang <[email protected]>
---
v2: Overriding address with bstrins
---
arch/loongarch/include/asm/stackframe.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/loongarch/include/asm/stackframe.h b/arch/loongarch/include/asm/stackframe.h
index 45b507a7b06f..51dec8b17d16 100644
--- a/arch/loongarch/include/asm/stackframe.h
+++ b/arch/loongarch/include/asm/stackframe.h
@@ -42,7 +42,7 @@
.macro JUMP_VIRT_ADDR temp1 temp2
li.d \temp1, CACHE_BASE
pcaddi \temp2, 0
- or \temp1, \temp1, \temp2
+ bstrins.d \temp1, \temp2, (DMW_PABITS - 1), 0
jirl zero, \temp1, 0xc
.endm
--
2.43.0
Currently kernel entry in head.S is in DMW address range,
firmware is instructed to jump to this address after loading
the image.
However kernel should not make any assumption on firmware's
DMW setting, thus the entry point should be a physical address
falls into direct translation region.
Fix by converting entry address to physical and amend entry
calculation logic in libstub accordingly.
Cc: [email protected]
Signed-off-by: Jiaxun Yang <[email protected]>
---
v2: Fix efistub
v3: Move calculation to linker script
---
arch/loongarch/kernel/head.S | 2 +-
arch/loongarch/kernel/vmlinux.lds.S | 2 ++
drivers/firmware/efi/libstub/loongarch.c | 2 +-
3 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/arch/loongarch/kernel/head.S b/arch/loongarch/kernel/head.S
index c4f7de2e2805..2cdc1ea808d9 100644
--- a/arch/loongarch/kernel/head.S
+++ b/arch/loongarch/kernel/head.S
@@ -22,7 +22,7 @@
_head:
.word MZ_MAGIC /* "MZ", MS-DOS header */
.org 0x8
- .dword kernel_entry /* Kernel entry point */
+ .dword _kernel_entry_phys /* Kernel entry point (physical address) */
.dword _kernel_asize /* Kernel image effective size */
.quad PHYS_LINK_KADDR /* Kernel image load offset from start of RAM */
.org 0x38 /* 0x20 ~ 0x37 reserved */
diff --git a/arch/loongarch/kernel/vmlinux.lds.S b/arch/loongarch/kernel/vmlinux.lds.S
index e8e97dbf9ca4..c6f89e51257a 100644
--- a/arch/loongarch/kernel/vmlinux.lds.S
+++ b/arch/loongarch/kernel/vmlinux.lds.S
@@ -6,6 +6,7 @@
#define PAGE_SIZE _PAGE_SIZE
#define RO_EXCEPTION_TABLE_ALIGN 4
+#define TO_PHYS_MASK 0x000fffffffffffff /* 48-bit */
/*
* Put .bss..swapper_pg_dir as the first thing in .bss. This will
@@ -142,6 +143,7 @@ SECTIONS
#ifdef CONFIG_EFI_STUB
/* header symbols */
+ _kernel_entry_phys = kernel_entry & TO_PHYS_MASK;
_kernel_asize = _end - _text;
_kernel_fsize = _edata - _text;
_kernel_vsize = _end - __initdata_begin;
diff --git a/drivers/firmware/efi/libstub/loongarch.c b/drivers/firmware/efi/libstub/loongarch.c
index 684c9354637c..60c145121393 100644
--- a/drivers/firmware/efi/libstub/loongarch.c
+++ b/drivers/firmware/efi/libstub/loongarch.c
@@ -41,7 +41,7 @@ static efi_status_t exit_boot_func(struct efi_boot_memmap *map, void *priv)
unsigned long __weak kernel_entry_address(unsigned long kernel_addr,
efi_loaded_image_t *image)
{
- return *(unsigned long *)(kernel_addr + 8) - VMLINUX_LOAD_ADDRESS + kernel_addr;
+ return *(unsigned long *)(kernel_addr + 8) - TO_PHYS(VMLINUX_LOAD_ADDRESS) + kernel_addr;
}
efi_status_t efi_boot_kernel(void *handle, efi_loaded_image_t *image,
--
2.43.0
NUMA enabled kernel on FDT based machine fails to boot
because CPUs are all in NUMA_NO_NODE and mm subsystem
won't accept that.
Fix by adding them to default NUMA node at FDT parsing
phase and move numa_add_cpu(0) to a later point.
Cc: [email protected]
Fixes: 88d4d957edc7 ("LoongArch: Add FDT booting support from efi system table")
Signed-off-by: Jiaxun Yang <[email protected]>
---
v3: Move numa_add_cpu(0) to a later point to ensure per_cpu
is ready.
---
arch/loongarch/kernel/smp.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/arch/loongarch/kernel/smp.c b/arch/loongarch/kernel/smp.c
index 0dfe2388ef41..1436d2465939 100644
--- a/arch/loongarch/kernel/smp.c
+++ b/arch/loongarch/kernel/smp.c
@@ -273,7 +273,6 @@ static void __init fdt_smp_setup(void)
if (cpuid == loongson_sysconf.boot_cpu_id) {
cpu = 0;
- numa_add_cpu(cpu);
} else {
cpu = cpumask_next_zero(-1, cpu_present_mask);
}
@@ -283,6 +282,9 @@ static void __init fdt_smp_setup(void)
set_cpu_present(cpu, true);
__cpu_number_map[cpuid] = cpu;
__cpu_logical_map[cpu] = cpuid;
+
+ early_numa_add_cpu(cpu, 0);
+ set_cpuid_to_node(cpuid, 0);
}
loongson_sysconf.nr_cpus = num_processors;
@@ -468,6 +470,7 @@ void smp_prepare_boot_cpu(void)
set_cpu_possible(0, true);
set_cpu_online(0, true);
set_my_cpu_offset(per_cpu_offset(0));
+ numa_add_cpu(0);
rr_node = first_node(node_online_map);
for_each_possible_cpu(cpu) {
--
2.43.0
Hi Jiaxun,
On Wed, May 22, 2024 at 11:02:19PM +0100, Jiaxun Yang wrote:
> Currently kernel entry in head.S is in DMW address range,
> firmware is instructed to jump to this address after loading
> the image.
>
> However kernel should not make any assumption on firmware's
> DMW setting, thus the entry point should be a physical address
> falls into direct translation region.
>
> Fix by converting entry address to physical and amend entry
> calculation logic in libstub accordingly.
>
> Cc: [email protected]
> Signed-off-by: Jiaxun Yang <[email protected]>
> ---
> v2: Fix efistub
> v3: Move calculation to linker script
> ---
> arch/loongarch/kernel/head.S | 2 +-
> arch/loongarch/kernel/vmlinux.lds.S | 2 ++
> drivers/firmware/efi/libstub/loongarch.c | 2 +-
> 3 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/arch/loongarch/kernel/head.S b/arch/loongarch/kernel/head.S
> index c4f7de2e2805..2cdc1ea808d9 100644
> --- a/arch/loongarch/kernel/head.S
> +++ b/arch/loongarch/kernel/head.S
> @@ -22,7 +22,7 @@
> _head:
> .word MZ_MAGIC /* "MZ", MS-DOS header */
> .org 0x8
> - .dword kernel_entry /* Kernel entry point */
> + .dword _kernel_entry_phys /* Kernel entry point (physical address) */
> .dword _kernel_asize /* Kernel image effective size */
> .quad PHYS_LINK_KADDR /* Kernel image load offset from start of RAM */
> .org 0x38 /* 0x20 ~ 0x37 reserved */
> diff --git a/arch/loongarch/kernel/vmlinux.lds.S b/arch/loongarch/kernel/vmlinux.lds.S
> index e8e97dbf9ca4..c6f89e51257a 100644
> --- a/arch/loongarch/kernel/vmlinux.lds.S
> +++ b/arch/loongarch/kernel/vmlinux.lds.S
> @@ -6,6 +6,7 @@
>
> #define PAGE_SIZE _PAGE_SIZE
> #define RO_EXCEPTION_TABLE_ALIGN 4
> +#define TO_PHYS_MASK 0x000fffffffffffff /* 48-bit */
>
> /*
> * Put .bss..swapper_pg_dir as the first thing in .bss. This will
> @@ -142,6 +143,7 @@ SECTIONS
>
> #ifdef CONFIG_EFI_STUB
> /* header symbols */
> + _kernel_entry_phys = kernel_entry & TO_PHYS_MASK;
> _kernel_asize = _end - _text;
> _kernel_fsize = _edata - _text;
> _kernel_vsize = _end - __initdata_begin;
> diff --git a/drivers/firmware/efi/libstub/loongarch.c b/drivers/firmware/efi/libstub/loongarch.c
> index 684c9354637c..60c145121393 100644
> --- a/drivers/firmware/efi/libstub/loongarch.c
> +++ b/drivers/firmware/efi/libstub/loongarch.c
> @@ -41,7 +41,7 @@ static efi_status_t exit_boot_func(struct efi_boot_memmap *map, void *priv)
> unsigned long __weak kernel_entry_address(unsigned long kernel_addr,
> efi_loaded_image_t *image)
> {
> - return *(unsigned long *)(kernel_addr + 8) - VMLINUX_LOAD_ADDRESS + kernel_addr;
> + return *(unsigned long *)(kernel_addr + 8) - TO_PHYS(VMLINUX_LOAD_ADDRESS) + kernel_addr;
> }
>
> efi_status_t efi_boot_kernel(void *handle, efi_loaded_image_t *image,
>
> --
> 2.43.0
>
This patch is now in -next as commit 75461304ee4e ("LoongArch: Fix entry
point in kernel image header"). I just bisected a build failure that I
see when building with LLVM (either 18 or 19) to this change.
$ make -skj"$(nproc)" ARCH=loongarch LLVM=1 defconfig vmlinux
..
kallsyms failure: relative symbol value 0x9000000000200000 out of range in relative mode
make[4]: *** [scripts/Makefile.vmlinux:34: vmlinux] Error 1
..
Does kallsyms need some adjustment for this?
Cheers,
Nathan
# bad: [9d99040b1bc8dbf385a8aa535e9efcdf94466e19] Add linux-next specific files for 20240529
# good: [e0cce98fe279b64f4a7d81b7f5c3a23d80b92fbc] Merge tag 'tpmdd-next-6.10-rc2' of git://git.kernel.org/pub/scm/linux/kernel/git/jarkko/linux-tpmdd
git bisect start '9d99040b1bc8dbf385a8aa535e9efcdf94466e19' 'e0cce98fe279b64f4a7d81b7f5c3a23d80b92fbc'
# bad: [270c6bb9d5e8448b74950f23ff2a192faaf10428] Merge branch 'for-next' of git://git.kernel.org/pub/scm/linux/kernel/git/wireless/wireless-next.git
git bisect bad 270c6bb9d5e8448b74950f23ff2a192faaf10428
# good: [c38b067bf2ab58d93590a50cbc06c992fe00447e] Merge branch 'ti-next' of git://git.kernel.org/pub/scm/linux/kernel/git/ti/linux.git
git bisect good c38b067bf2ab58d93590a50cbc06c992fe00447e
# bad: [6bcfb2dcf8b00c0b4cef68ac026c71dae3c25070] Merge branch 'for-next' of git://git.kernel.org/pub/scm/linux/kernel/git/printk/linux.git
git bisect bad 6bcfb2dcf8b00c0b4cef68ac026c71dae3c25070
# good: [2ca0cd490407a728a9aa57b9538f3ca8b287a089] Merge branch 'for-next' of git://git.kernel.org/pub/scm/linux/kernel/git/gfs2/linux-gfs2.git
git bisect good 2ca0cd490407a728a9aa57b9538f3ca8b287a089
# good: [f3760c80d06a838495185c0fe341c043e6495142] Merge branch 'rework/write-atomic' into for-next
git bisect good f3760c80d06a838495185c0fe341c043e6495142
# good: [b9fc9904efcaea8470f0d4cd0691f1295add9381] Merge branch 'vfs.module.description' into vfs.all
git bisect good b9fc9904efcaea8470f0d4cd0691f1295add9381
# good: [88dbb5f3c068ba8944e97235ccfdc5fbd6c7d577] Merge branch '9p-next' of git://github.com/martinetd/linux
git bisect good 88dbb5f3c068ba8944e97235ccfdc5fbd6c7d577
# bad: [b4660cd50cb1e5821532e34dbc7f47cb155ba57b] Merge branch 'next' of git://git.monstr.eu/linux-2.6-microblaze.git
git bisect bad b4660cd50cb1e5821532e34dbc7f47cb155ba57b
# bad: [c768fc96978cd0f74dd297d58720cb984a7f6341] LoongArch: Override higher address bits in JUMP_VIRT_ADDR
git bisect bad c768fc96978cd0f74dd297d58720cb984a7f6341
# good: [2624e739c2e9abe5f6cc9acc37f9752f0055fb5f] LoongArch: Add all CPUs enabled by fdt to NUMA node 0
git bisect good 2624e739c2e9abe5f6cc9acc37f9752f0055fb5f
# bad: [75461304ee4e7e2cb282265a6a89c35b81282d19] LoongArch: Fix entry point in kernel image header
git bisect bad 75461304ee4e7e2cb282265a6a89c35b81282d19
# first bad commit: [75461304ee4e7e2cb282265a6a89c35b81282d19] LoongArch: Fix entry point in kernel image header
On Thu, May 23, 2024 at 6:03 AM Jiaxun Yang <[email protected]> wrote:
>
> Currently kernel entry in head.S is in DMW address range,
> firmware is instructed to jump to this address after loading
> the image.
>
> However kernel should not make any assumption on firmware's
> DMW setting, thus the entry point should be a physical address
> falls into direct translation region.
>
> Fix by converting entry address to physical and amend entry
> calculation logic in libstub accordingly.
>
> Cc: [email protected]
> Signed-off-by: Jiaxun Yang <[email protected]>
> ---
> v2: Fix efistub
> v3: Move calculation to linker script
> ---
> arch/loongarch/kernel/head.S | 2 +-
> arch/loongarch/kernel/vmlinux.lds.S | 2 ++
> drivers/firmware/efi/libstub/loongarch.c | 2 +-
> 3 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/arch/loongarch/kernel/head.S b/arch/loongarch/kernel/head.S
> index c4f7de2e2805..2cdc1ea808d9 100644
> --- a/arch/loongarch/kernel/head.S
> +++ b/arch/loongarch/kernel/head.S
> @@ -22,7 +22,7 @@
> _head:
> .word MZ_MAGIC /* "MZ", MS-DOS header */
> .org 0x8
> - .dword kernel_entry /* Kernel entry point */
> + .dword _kernel_entry_phys /* Kernel entry point (physical address) */
> .dword _kernel_asize /* Kernel image effective size */
> .quad PHYS_LINK_KADDR /* Kernel image load offset from start of RAM */
> .org 0x38 /* 0x20 ~ 0x37 reserved */
> diff --git a/arch/loongarch/kernel/vmlinux.lds.S b/arch/loongarch/kernel/vmlinux.lds.S
> index e8e97dbf9ca4..c6f89e51257a 100644
> --- a/arch/loongarch/kernel/vmlinux.lds.S
> +++ b/arch/loongarch/kernel/vmlinux.lds.S
> @@ -6,6 +6,7 @@
>
> #define PAGE_SIZE _PAGE_SIZE
> #define RO_EXCEPTION_TABLE_ALIGN 4
> +#define TO_PHYS_MASK 0x000fffffffffffff /* 48-bit */
>
> /*
> * Put .bss..swapper_pg_dir as the first thing in .bss. This will
> @@ -142,6 +143,7 @@ SECTIONS
>
> #ifdef CONFIG_EFI_STUB
> /* header symbols */
> + _kernel_entry_phys = kernel_entry & TO_PHYS_MASK;
- _kernel_entry_phys = kernel_entry & TO_PHYS_MASK;
+ _kernel_entry_phys = ABSOLUTE(kernel_entry & TO_PHYS_MASK);
> _kernel_asize = _end - _text;
> _kernel_fsize = _edata - _text;
> _kernel_vsize = _end - __initdata_begin;
> diff --git a/drivers/firmware/efi/libstub/loongarch.c b/drivers/firmware/efi/libstub/loongarch.c
> index 684c9354637c..60c145121393 100644
> --- a/drivers/firmware/efi/libstub/loongarch.c
> +++ b/drivers/firmware/efi/libstub/loongarch.c
> @@ -41,7 +41,7 @@ static efi_status_t exit_boot_func(struct efi_boot_memmap *map, void *priv)
> unsigned long __weak kernel_entry_address(unsigned long kernel_addr,
> efi_loaded_image_t *image)
> {
> - return *(unsigned long *)(kernel_addr + 8) - VMLINUX_LOAD_ADDRESS + kernel_addr;
> + return *(unsigned long *)(kernel_addr + 8) - TO_PHYS(VMLINUX_LOAD_ADDRESS) + kernel_addr;
> }
>
> efi_status_t efi_boot_kernel(void *handle, efi_loaded_image_t *image,
>
> --
> 2.43.0
>
>
- Rui
在2024年5月30日五月 上午6:01，WANG Rui写道：
[...]
>> /*
>> * Put .bss..swapper_pg_dir as the first thing in .bss. This will
>> @@ -142,6 +143,7 @@ SECTIONS
>>
>> #ifdef CONFIG_EFI_STUB
>> /* header symbols */
>> + _kernel_entry_phys = kernel_entry & TO_PHYS_MASK;
>
> - _kernel_entry_phys = kernel_entry & TO_PHYS_MASK;
> + _kernel_entry_phys = ABSOLUTE(kernel_entry & TO_PHYS_MASK);
>
Thanks Rui!
Huacai, do you mind committing this fix?
Thanks
[...]
>>
>>
>
> - Rui
--
- Jiaxun
On 2024-05-22 23:02:17+0000, Jiaxun Yang wrote:
> fdt_check_header(__dtb_start) will always success because kernel
> provided a dummy dtb, and by coincidence __dtb_start clashed with
> entry of this dummy dtb. The consequence is fdt passed from
> firmware will never be taken.
>
> Fix by trying to utilise __dtb_start only when CONFIG_BUILTIN_DTB
> is enabled.
>
> Cc: [email protected]
> Fixes: 7b937cc243e5 ("of: Create of_root if no dtb provided by firmware")
> Signed-off-by: Jiaxun Yang <[email protected]>
> ---
> v3: Better reasoning in commit message, thanks Binbin and Huacai!
> ---
> arch/loongarch/kernel/setup.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/arch/loongarch/kernel/setup.c b/arch/loongarch/kernel/setup.c
> index 60e0fe97f61a..ea6d5db6c878 100644
> --- a/arch/loongarch/kernel/setup.c
> +++ b/arch/loongarch/kernel/setup.c
> @@ -275,16 +275,18 @@ static void __init arch_reserve_crashkernel(void)
> static void __init fdt_setup(void)
> {
> #ifdef CONFIG_OF_EARLY_FLATTREE
> - void *fdt_pointer;
> + void *fdt_pointer = NULL;
>
> /* ACPI-based systems do not require parsing fdt */
> if (acpi_os_get_root_pointer())
> return;
>
> +#ifdef CONFIG_BUILTIN_DTB
> /* Prefer to use built-in dtb, checking its legality first. */
> if (!fdt_check_header(__dtb_start))
> fdt_pointer = __dtb_start;
> - else
> +#endif
> + if (!fdt_pointer)
> fdt_pointer = efi_fdt_pointer(); /* Fallback to firmware dtb */
Prefer to use non-ifdef logic:
if (IS_ENABLED(CONFIG_BUILTIN_DTB) && !fdt_check_header(__dtb_start))
fdt_pointer = __dtb_start;
This is shorter, easier to read and will prevent bitrot.
The code will be typechecked but then optimized away, so no
runtime overhead exists.
>
> if (!fdt_pointer || fdt_check_header(fdt_pointer))
>
> --
> 2.43.0
>
On Thu, May 30, 2024 at 5:50 PM Jiaxun Yang <[email protected]> wrote:
>
>
>
> 在2024年5月30日五月 上午6:01，WANG Rui写道：
> [...]
> >> /*
> >> * Put .bss..swapper_pg_dir as the first thing in .bss. This will
> >> @@ -142,6 +143,7 @@ SECTIONS
> >>
> >> #ifdef CONFIG_EFI_STUB
> >> /* header symbols */
> >> + _kernel_entry_phys = kernel_entry & TO_PHYS_MASK;
> >
> > - _kernel_entry_phys = kernel_entry & TO_PHYS_MASK;
> > + _kernel_entry_phys = ABSOLUTE(kernel_entry & TO_PHYS_MASK);
> >
>
> Thanks Rui!
>
> Huacai, do you mind committing this fix?
I will update this patch in my tree, you needn't do anything.
Huacai
>
> Thanks
> [...]
> >>
> >>
> >
> > - Rui
>
> --
> - Jiaxun
Hi, Thomas,
On Thu, May 30, 2024 at 6:06 PM Thomas Weißschuh <thomas@t-8chde> wrote:
>
> On 2024-05-22 23:02:17+0000, Jiaxun Yang wrote:
> > fdt_check_header(__dtb_start) will always success because kernel
> > provided a dummy dtb, and by coincidence __dtb_start clashed with
> > entry of this dummy dtb. The consequence is fdt passed from
> > firmware will never be taken.
> >
> > Fix by trying to utilise __dtb_start only when CONFIG_BUILTIN_DTB
> > is enabled.
> >
> > Cc: [email protected]
> > Fixes: 7b937cc243e5 ("of: Create of_root if no dtb provided by firmware")
> > Signed-off-by: Jiaxun Yang <[email protected]>
> > ---
> > v3: Better reasoning in commit message, thanks Binbin and Huacai!
> > ---
> > arch/loongarch/kernel/setup.c | 6 ++++--
> > 1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/loongarch/kernel/setup.c b/arch/loongarch/kernel/setup.c
> > index 60e0fe97f61a..ea6d5db6c878 100644
> > --- a/arch/loongarch/kernel/setup.c
> > +++ b/arch/loongarch/kernel/setup.c
> > @@ -275,16 +275,18 @@ static void __init arch_reserve_crashkernel(void)
> > static void __init fdt_setup(void)
> > {
> > #ifdef CONFIG_OF_EARLY_FLATTREE
> > - void *fdt_pointer;
> > + void *fdt_pointer = NULL;
> >
> > /* ACPI-based systems do not require parsing fdt */
> > if (acpi_os_get_root_pointer())
> > return;
> >
> > +#ifdef CONFIG_BUILTIN_DTB
> > /* Prefer to use built-in dtb, checking its legality first. */
> > if (!fdt_check_header(__dtb_start))
> > fdt_pointer = __dtb_start;
> > - else
> > +#endif
> > + if (!fdt_pointer)
> > fdt_pointer = efi_fdt_pointer(); /* Fallback to firmware dtb */
>
> Prefer to use non-ifdef logic:
>
> if (IS_ENABLED(CONFIG_BUILTIN_DTB) && !fdt_check_header(__dtb_start))
> fdt_pointer = __dtb_start;
>
> This is shorter, easier to read and will prevent bitrot.
> The code will be typechecked but then optimized away, so no
> runtime overhead exists.
Good suggestion, I will take it.
Huacai
>
> >
> > if (!fdt_pointer || fdt_check_header(fdt_pointer))
> >
> > --
> > 2.43.0
> >