Hi Kever, On Sun, 5 Jan 2025 at 23:47, Kever Yang <[email protected]> wrote: > > 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
Yes, I see what you are doing. The real fix is probably to not use the debug UART in the stub. There is a large comment: * EFI uses Unicode and we don't. The easiest way to get a sensible output * function is to use the U-Boot debug UART. We use EFI's console output * function where available, and assume the built-in UART after that. We rely * on EFI to set up the UART for us and just bring in the functions here. * This last bit is a bit icky, but it's only for debugging anyway. We could * build in ns16550.c with some effort, but this is a payload loader after * all. * * Note: We avoid using printf() so we don't need to bring in lib/vsprintf.c. * That would require some refactoring since we already build this for U-Boot. * Building an EFI shared library version would have to be a separate stem. * That might push us to using the SPL framework to build this stub. However * that would involve a round of EFI-specific changes in SPL. Worth * considering if we start needing more U-Boot functionality. Note that we * could then move get_codeseg32() to arch/x86/cpu/cpu.c. We don't have a way of building the same file twice (once for the stub and once for U-Boot). That would require my split-config series - Tom rejected the last 10 patches (which actually did the work) a few years ago[1]. But I do plan to get back to it. For now, I suggest adding a new Kconfig, .e.g DEBUG_UART_DUMMY (dependent on DEBUG_UART) which defines the dummy functions. Set it to y by default, but n for the EFI-stub builds. Then you don't need to enable the debug UART here. Regards, Simoon [1] https://patchwork.ozlabs.org/project/uboot/list/?series=341504&state=* > > Thanks, > - Kever > [1] https://source.denx.de/u-boot/custodians/u-boot-rockchip/-/jobs/987462 > [2] > https://patchwork.ozlabs.org/project/uboot/patch/[email protected]/ > > On 2025/1/5 07:02, Heinrich Schuchardt wrote: > > Am 4. Januar 2025 20:29:43 MEZ schrieb Simon Glass <[email protected]>: > >> Hi Heinrich, > >> > >> On Sat, 4 Jan 2025 at 05:16, Heinrich Schuchardt <[email protected]> > >> 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 <[email protected]> > >>>> --- > >>>> > >>>> 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 <[email protected]> > >>> > >>>> 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 > >
