Received-SPF: pass (google.com: domain of linux-kernel+bounces-195228-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Date: Thu, 30 May 2024 12:06:01 +0200
From: Thomas =?utf-8?Q?Wei=C3=9Fschuh?= <thomas@t-8ch.de>
To: Jiaxun Yang <jiaxun.yang@flygoat.com>
Cc: Huacai Chen <chenhuacai@kernel.org>, 
	Binbin Zhou <zhoubinbin@loongson.cn>, loongarch@lists.linux.dev, linux-kernel@vger.kernel.org, 
	stable@vger.kernel.org
Subject: Re: [PATCH v3 1/4] LoongArch: Fix built-in DTB detection
Message-ID: <e1321725-ee7e-4716-ad45-634803fca5ff@t-8ch.de>
References: <20240522-loongarch-booting-fixes-v3-0-25e77a8fc86e@flygoat.com>
 <20240522-loongarch-booting-fixes-v3-1-25e77a8fc86e@flygoat.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240522-loongarch-booting-fixes-v3-1-25e77a8fc86e@flygoat.com>

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: stable@vger.kernel.org
> Fixes: 7b937cc243e5 ("of: Create of_root if no dtb provided by firmware")
> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> ---
> 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
>