[PATCH] efi_loader: Ignore DT when ACPI is on

[Heinrich Schuchardt]

On 2/27/22 15:13, Alexander Graf wrote:
>
> On 27.02.22 14:07, Mark Kettenis wrote:
>>> From: Alexander Graf <agraf at csgraf.de>
>>> Date: Sun, 27 Feb 2022 13:18:56 +0100
>>>
>>> For targets that enable ACPI, we should not pass Device Trees into
>>> the payload. However, our distro boot logic always passes the builtin
>>> DT as an argument.

Hello Alex,

nice to see you back at U-Boot.

If CONFIG_GENERATE_ACPI_TABLE=n, no ACPI table is generated. In this
case U-Boot may fall back to the built in devicetree.

If CONFIG_GENERATE_ACPI_TABLE=y, copy_fdt() does not exist and a
device-tree cannot be supplied to the EFI binary. If you still try
supply a device tree, the bootefi command writes an error an refuses to
boot.

With which defconfig does this lead to a boot failure?
Which board supplies ${fdt_addr_r} and has CONFIG_GENERATE_ACPI_TABLE=y?

See include/config_distro_bootcmd.h line 127ff.

Best regards

Heinrich

>>>
>>> To make it easy to use ACPI with distro boot, let's just ignore the DT
>>> argument to bootefi when ACPI is enabled. That way, we can successfully
>>> distro boot payloads on ACPI enabled targets.
>> I've never understood why it is bad to provide both the DT and ACPI
>> tables.  Several TianoCore EDK2 based firmwares do this or at least
>> give you the option to do this.  And it works fine.  If both are
>> provided, you just leave it up to the OS to decide what it wants to
>> use.  Which makes me think the text was put there in the standard by
>> people with an agenda...
>
>
> It's much more profane :). Upstream Linux went with "DT first, ACPI
> last" while RHEL had a downstream patch that went "ACPI first, DT last".
> And then people were confused why RHEL and upstream kernels had such
> vastly different behavior on their systems.
>
> Long story short, the best way to avoid this ambiguity is to only
> provide one of the 2 to the OS. And that's what the BBRs dictate now.
> They don't dictate what firmware does internally though, so we're free
> to do whatever we like - and I think "if someone went through the effort
> to enable ACPI on this system, they probably want to use it" is a fair
> assumption with U-Boot. So we should prefer ACPI if available.
>
>
> Alex
>
>
>>
>> I think it would be good for U-Boot to give users a choice here, with
>> all possible combinations (only DT, only ACPI, both DT and ACPI).  But
>> this is a step in the right direction.
>>
>> Reviewed-by: Mark Kettenis <kettenis at openbsd.org>
>>
>>> Signed-off-by: Alexander Graf <agraf at csgraf.de>
>>> ---
>>>   cmd/bootefi.c | 4 ++--
>>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/cmd/bootefi.c b/cmd/bootefi.c
>>> index 94d18ca73f..2c9bc0dcd2 100644
>>> --- a/cmd/bootefi.c
>>> +++ b/cmd/bootefi.c
>>> @@ -265,8 +265,8 @@ efi_status_t efi_install_fdt(void *fdt)
>>>        */
>>>   #if CONFIG_IS_ENABLED(GENERATE_ACPI_TABLE)
>>>       if (fdt) {
>>> -        log_err("ERROR: can't have ACPI table and device tree.\n");
>>> -        return EFI_LOAD_ERROR;
>>> +        log_warning("WARNING: Can't have ACPI table and device tree
>>> - ignoring DT.\n");
>>> +        return EFI_SUCCESS;
>>>       }
>>>   #else
>>>       bootm_headers_t img = { 0 };
>>> --
>>> 2.32.0
>>>
>>>