[PATCH v2 2/2] efi/fdt: ignore dtb when acpi option is used with force

[Ard Biesheuvel]

On Mon, 25 Nov 2024 at 19:16, Levi Yun <yeoreum.yun at arm.com> wrote:
>
> Hi Ard.
>
> >
> > The DT is not stored in a variable.
> >
> > If CONFIG_EFI_ARMSTUB_DTB_LOADER is enabled, it may be provided via
> > dtb= on the command line, but I have little sympathy for a user that
> > passes both dtb= *and* acpi=force, so this is a scenario that we can
> > ignore.
> >
> > Otherwise, it is taken from a EFI config table, which is just a
> > <guid,addr> tuple describing a location in physical memory where the
> > firmware has placed a DT. If the firmware puts a corrupted DT there,
> > the firmware should be fixed instead.
> >
> > acpi=force is intended to force the use of ACPI tables on a system
> > that provides both.
> >
> > > also, although acpi= could differ from architecture, the force option's menaing
> > > seems the same over architecture (ignore DT boot with ACPI tables).
> > >
> > > So I think the check the "acpi=force" option to prevent loading DT seems
> > > good.
> > >
> >
> > The EFI stub does not care about ACPI vs DT boot, and I'd prefer to
> > keep it that way unless there is a good reason.
> >
> > Which real-world problem does this patch aim to solve?
>
> Well. I had lack of explaination. In case of Juno platform, it loads
> FDT from "Fdt" variable from the storage and install it into
> configuration table with corrupted Fdt because of FDT stored in variable
> storage was corrupted.
>
> In that siutation, If it loads corrupted fdt, it prints error message
> while sanity check in early_init_dt_scan().
> This kind of error message would be confused to user because
> user already specifies to boot with acpi table only with acpi=force
> option.
>
> anyway, what kind of way to install fdt into configuraiton table is not
> matter. but when the dt installed in configuration table isn't valid,
> it could produce the error message which seems violate user specified
> option.
>
> unless check the acpi=force to ignore DT, I think it would require to
> check the installed DT in configuration table or passed should have
> simple sanity check doen in early_init_dt_scan() so that error messsage
> which makes some confusion for this situation.
>

Thanks for explaining the issue in more detail. Juno is a development
platform with a highly unusual boot stack so I don't think we need to
accommodate its quirks in the upstream kernel.

And I still don't think this is something worth fixing in general.
Even if the machine description should be taken from ACPI tables only,
the DT /chosen node is always used as a conduit by the EFI stub, and
there are cases, e.g., for initrd info or the kaslr seed, where this
information might come from the bootloader, such as older GRUB builds.