[PATCH 2/2] config: efi-x86_payload: Enable DEBUG_UART

[Heinrich Schuchardt]

Am 4. Januar 2025 20:29:43 MEZ schrieb Simon Glass <sjg at chromium.org>:
>Hi Heinrich,
>
>On Sat, 4 Jan 2025 at 05:16, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
>>
>> On 28.11.24 04:47, Kever Yang wrote:
>> > The efi_stub is useing DEBUG_UART interface by default, Enable it.
>>
>> As Simon already wrote in a code comment the implementation of the EFI
>> stub is broken as it is not hardware agnostic.
>
>If I said that I was wrong, sorry. Actually that is the stub's intent,
>to take over the machine, exit boot-services and carry on. It is the
>EFI app which is designed to use boot services.
>
>So the stub cannot be hardware-agnostic, unfortunately. It only runs
>on x86 hardware, unless, for example, EFI has something like
>serial_getinfo() or we disable the UART / keyboard input.

I would assume that there is no serial console that the user has access to on many x86 systems (like on my laptop).

In the stub itself we can use the Simple Text Output Protocol and the Simple Text Input Protocol offered by the UEFI implementation that started the stub.

The UEFI specification defines a Serial Io Protocol which may be implemented but I would not know why we should restrict the usability in this way.

Once we start U-Boot of course we will use hardware specific IO, e.g. on a RISC-V system use the debug console offered by the SBI 

Best regards

Heinrich

>
>>
>> In the EFI stub we should never directly access hardware. Please, use
>> SimpleTextOutputProtocol.OutputString() for printing.
>
>Once the stub starts, boot services are gone. See efi_main() for how
>this works. There is a definite point at which we stop using that and
>start using U-Boot's own thing.
>
>At this point I think I may be misunderstanding what you said...
>
>>
>> >
>> > Signed-off-by: Kever Yang <kever.yang at rock-chips.com>
>> > ---
>> >
>> >   configs/efi-x86_payload32_defconfig | 1 +
>> >   configs/efi-x86_payload64_defconfig | 1 +
>> >   2 files changed, 2 insertions(+)
>> >
>> > diff --git a/configs/efi-x86_payload32_defconfig b/configs/efi-x86_payload32_defconfig
>> > index 071ddb8e36d..f0b9acc358d 100644
>> > --- a/configs/efi-x86_payload32_defconfig
>> > +++ b/configs/efi-x86_payload32_defconfig
>> > @@ -5,6 +5,7 @@ CONFIG_DEFAULT_DEVICE_TREE="efi-x86_payload"
>> >   CONFIG_PRE_CON_BUF_ADDR=0x100000
>> >   CONFIG_VENDOR_EFI=y
>> >   CONFIG_TARGET_EFI_PAYLOAD=y
>> > +CONFIG_DEBUG_UART=y
>>
>> On x86 this uses the NS16550 UART driver. This is fine as long as we
>> don't use the debug UART in the EFI stub. But correcting this is for a
>> separate patch.
>>
>> Having a debug UART available after the EFI stub makes sense.
>>
>> I will update the commit message when merging.
>>
>> Reviewed-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
>>
>> >   CONFIG_EFI=y
>> >   CONFIG_EFI_STUB=y
>> >   CONFIG_FIT=y
>> > diff --git a/configs/efi-x86_payload64_defconfig b/configs/efi-x86_payload64_defconfig
>> > index 71612d7fb19..b02a861e59c 100644
>> > --- a/configs/efi-x86_payload64_defconfig
>> > +++ b/configs/efi-x86_payload64_defconfig
>> > @@ -5,6 +5,7 @@ CONFIG_DEFAULT_DEVICE_TREE="efi-x86_payload"
>> >   CONFIG_PRE_CON_BUF_ADDR=0x100000
>> >   CONFIG_VENDOR_EFI=y
>> >   CONFIG_TARGET_EFI_PAYLOAD=y
>> > +CONFIG_DEBUG_UART=y
>> >   CONFIG_EFI=y
>> >   CONFIG_EFI_STUB=y
>> >   CONFIG_EFI_STUB_64BIT=y
>>
>
>Regards,
>Simon