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

[Kever Yang]

Hi Heinrich and Simon,

     I send these two change because I want to fix the build error 
below(from[1]) which is for disable the DEBUG_UART[2].
Do you have clear suggestion on how to do this in efi_stub, since I have 
no idea about this module, so I just follow how
it works in other serial driver.
+/binman/rom/intel-mrc (mrc.bin):
+Image 'rom' has faked external blobs and is non-functional: 
descriptor.bin me.bin refcode.bin vga.bin mrc.bin
+In file included from lib/efi/efi_stub.c:12:
+include/debug_uart.h:205:9: error: stray '#' in program
+  205 |         #warning "DEBUG_UART not defined!"
+      |         ^
+lib/efi/efi_stub.c:91:1: note: in expansion of macro 'DEBUG_UART_FUNCS'
+   91 | DEBUG_UART_FUNCS
+      | ^~~~~~~~~~~~~~~~
+include/debug_uart.h:205:18: error: expected '=', ',', ';', 'asm' or 
'__attribute__' before string constant
+      |                  ^~~~~~~~~~~~~~~~~~~~~~~~~
+lib/efi/efi_stub.c:86:13: error: '_debug_uart_putc' defined but not 
used [-Werror=unused-function]
+   86 | static void _debug_uart_putc(int ch)
+      |             ^~~~~~~~~~~~~~~~
+make[3]: *** [scripts/Makefile.build:257: lib/efi/efi_stub.o] Error 1
+make[2]: *** [scripts/Makefile.build:398: lib/efi] Error 2

Thanks,
- Kever
[1] https://source.denx.de/u-boot/custodians/u-boot-rockchip/-/jobs/987462
[2] 
https://patchwork.ozlabs.org/project/uboot/patch/20240918130155.299353-2-lukasz.czechowski@thaumatec.com/ 

On 2025/1/5 07:02, Heinrich Schuchardt wrote:
> 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
>