Received-SPF: pass (google.com: domain of linux-kernel+bounces-195549-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Precedence: bulk
MIME-Version: 1.0
References: <20240522-loongarch-booting-fixes-v3-0-25e77a8fc86e@flygoat.com>
 <20240522-loongarch-booting-fixes-v3-1-25e77a8fc86e@flygoat.com> <e1321725-ee7e-4716-ad45-634803fca5ff@t-8ch.de>
In-Reply-To: <e1321725-ee7e-4716-ad45-634803fca5ff@t-8ch.de>
From: Huacai Chen <chenhuacai@kernel.org>
Date: Thu, 30 May 2024 22:49:18 +0800
Message-ID: <CAAhV-H78dNJVgL5mrSwRJF8jT0bV9znO=hx_EgsvKePWtac24g@mail.gmail.com>
Subject: Re: [PATCH v3 1/4] LoongArch: Fix built-in DTB detection
To: =?UTF-8?Q?Thomas_Wei=C3=9Fschuh?= <thomas@t-8ch.de>
Cc: Jiaxun Yang <jiaxun.yang@flygoat.com>, Binbin Zhou <zhoubinbin@loongson.cn>, 
	loongarch@lists.linux.dev, linux-kernel@vger.kernel.org, 
	stable@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Hi, Thomas,

On Thu, May 30, 2024 at 6:06=E2=80=AFPM Thomas Wei=C3=9Fschuh <thomas@t-8ch=
de> 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: 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/setu=
p.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 =3D 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 =3D __dtb_start;
> > -     else
> > +#endif
> > +     if (!fdt_pointer)
> >               fdt_pointer =3D efi_fdt_pointer(); /* Fallback to firmwar=
e dtb */
>
> Prefer to use non-ifdef logic:
>
>         if (IS_ENABLED(CONFIG_BUILTIN_DTB) && !fdt_check_header(__dtb_sta=
rt))
>                 fdt_pointer =3D __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
> >