U-Boot v2022.07-rc2 regression: Bootmenu is completely broken

U-Boot v2022.07-rc2 regression: Bootmenu is completely broken

[Pali Rohár]

Hello!

U-Boot Bootmenu in git master branch is completely broken and does not
work. You can test it e.g. in qemu version of nokia n900 (see rx51.rst).

Bootmenu just prints garbage on both serial console and VGA video
output and no option works.

  *** U-Boot Boot Menu ***

      A
      I
      E
      U
      U


  Press UP/DOWN to move, ENTER to select, ESC/CTRL+C to quit

I bisected this issue and problem is in:

  a3d0aa87acbec4b983f54c24a6e7fd8e9e7b9728 is the first bad commit
  commit a3d0aa87acbec4b983f54c24a6e7fd8e9e7b9728
  Author: Masahisa Kojima <masahisa.kojima@linaro.org>
  Date:   Thu Apr 28 17:09:41 2022 +0900

      bootmenu: update bootmenu_entry structure

      This is a preparation for succeeding addition of uefi boot
      and distro boot menu entries into bootmenu.
      The bootmenu_entry title is updated to u16 string because
      uefi use u16 string. This commit also factors out the function
      to prepare the entries generated by "bootmenu_x" U-Boot environment
      variable.

      Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
      Reviewed-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>

  :040000 040000 37c91714b049be307521831c0470b3b6c8af2407 b3e865073a46108cccbf4dc0b401789ef0b05a2e M      cmd

Before that commit Bootmenu works fine and on serial console prints:

  *** U-Boot Boot Menu ***

      Attached kernel
      Internal eMMC
      External SD card
      U-Boot boot order
      U-Boot console


  Press UP/DOWN to move, ENTER to select, ESC/CTRL+C to quit


Could you please look at it and fix it?

Re: U-Boot v2022.07-rc2 regression: Bootmenu is completely broken

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1934 bytes --]

On Mon, May 23, 2022 at 09:27:37PM +0200, Pali Rohár wrote:
> Hello!
> 
> U-Boot Bootmenu in git master branch is completely broken and does not
> work. You can test it e.g. in qemu version of nokia n900 (see rx51.rst).
> 
> Bootmenu just prints garbage on both serial console and VGA video
> output and no option works.
> 
>   *** U-Boot Boot Menu ***
> 
>       A
>       I
>       E
>       U
>       U
> 
> 
>   Press UP/DOWN to move, ENTER to select, ESC/CTRL+C to quit
> 
> I bisected this issue and problem is in:
> 
>   a3d0aa87acbec4b983f54c24a6e7fd8e9e7b9728 is the first bad commit
>   commit a3d0aa87acbec4b983f54c24a6e7fd8e9e7b9728
>   Author: Masahisa Kojima <masahisa.kojima@linaro.org>
>   Date:   Thu Apr 28 17:09:41 2022 +0900
> 
>       bootmenu: update bootmenu_entry structure
> 
>       This is a preparation for succeeding addition of uefi boot
>       and distro boot menu entries into bootmenu.
>       The bootmenu_entry title is updated to u16 string because
>       uefi use u16 string. This commit also factors out the function
>       to prepare the entries generated by "bootmenu_x" U-Boot environment
>       variable.
> 
>       Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
>       Reviewed-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> 
>   :040000 040000 37c91714b049be307521831c0470b3b6c8af2407 b3e865073a46108cccbf4dc0b401789ef0b05a2e M      cmd
> 
> Before that commit Bootmenu works fine and on serial console prints:
> 
>   *** U-Boot Boot Menu ***
> 
>       Attached kernel
>       Internal eMMC
>       External SD card
>       U-Boot boot order
>       U-Boot console
> 
> 
>   Press UP/DOWN to move, ENTER to select, ESC/CTRL+C to quit
> 
> 
> Could you please look at it and fix it?

To be clear, the bootmenu stuff is or is not supposed to be tested with
the current rx51 test script?

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: U-Boot v2022.07-rc2 regression: Bootmenu is completely broken

[Pali Rohár]

On Monday 23 May 2022 15:29:04 Tom Rini wrote:
> On Mon, May 23, 2022 at 09:27:37PM +0200, Pali Rohár wrote:
> > Hello!
> > 
> > U-Boot Bootmenu in git master branch is completely broken and does not
> > work. You can test it e.g. in qemu version of nokia n900 (see rx51.rst).
> > 
> > Bootmenu just prints garbage on both serial console and VGA video
> > output and no option works.
> > 
> >   *** U-Boot Boot Menu ***
> > 
> >       A
> >       I
> >       E
> >       U
> >       U
> > 
> > 
> >   Press UP/DOWN to move, ENTER to select, ESC/CTRL+C to quit
> > 
> > I bisected this issue and problem is in:
> > 
> >   a3d0aa87acbec4b983f54c24a6e7fd8e9e7b9728 is the first bad commit
> >   commit a3d0aa87acbec4b983f54c24a6e7fd8e9e7b9728
> >   Author: Masahisa Kojima <masahisa.kojima@linaro.org>
> >   Date:   Thu Apr 28 17:09:41 2022 +0900
> > 
> >       bootmenu: update bootmenu_entry structure
> > 
> >       This is a preparation for succeeding addition of uefi boot
> >       and distro boot menu entries into bootmenu.
> >       The bootmenu_entry title is updated to u16 string because
> >       uefi use u16 string. This commit also factors out the function
> >       to prepare the entries generated by "bootmenu_x" U-Boot environment
> >       variable.
> > 
> >       Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
> >       Reviewed-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> > 
> >   :040000 040000 37c91714b049be307521831c0470b3b6c8af2407 b3e865073a46108cccbf4dc0b401789ef0b05a2e M      cmd
> > 
> > Before that commit Bootmenu works fine and on serial console prints:
> > 
> >   *** U-Boot Boot Menu ***
> > 
> >       Attached kernel
> >       Internal eMMC
> >       External SD card
> >       U-Boot boot order
> >       U-Boot console
> > 
> > 
> >   Press UP/DOWN to move, ENTER to select, ESC/CTRL+C to quit
> > 
> > 
> > Could you please look at it and fix it?
> 
> To be clear, the bootmenu stuff is or is not supposed to be tested with
> the current rx51 test script?

rx51 test script sets bootmenu timeout to 1s and just let bootmenu
timeout. Timeout cause that default boot action is taken. Script does
not interact with stdin / bootmenu.

So... not selecting anything in bootmenu is working and hence rx51 test
script did not complained.

Re: U-Boot v2022.07-rc2 regression: Bootmenu is completely broken

[Masahisa Kojima]

On Tue, 24 May 2022 at 04:41, Pali Rohár <pali@kernel.org> wrote:
>
> On Monday 23 May 2022 15:29:04 Tom Rini wrote:
> > On Mon, May 23, 2022 at 09:27:37PM +0200, Pali Rohár wrote:
> > > Hello!
> > >
> > > U-Boot Bootmenu in git master branch is completely broken and does not
> > > work. You can test it e.g. in qemu version of nokia n900 (see rx51.rst).
> > >
> > > Bootmenu just prints garbage on both serial console and VGA video
> > > output and no option works.
> > >
> > >   *** U-Boot Boot Menu ***
> > >
> > >       A
> > >       I
> > >       E
> > >       U
> > >       U
> > >
> > >
> > >   Press UP/DOWN to move, ENTER to select, ESC/CTRL+C to quit
> > >
> > > I bisected this issue and problem is in:
> > >
> > >   a3d0aa87acbec4b983f54c24a6e7fd8e9e7b9728 is the first bad commit
> > >   commit a3d0aa87acbec4b983f54c24a6e7fd8e9e7b9728
> > >   Author: Masahisa Kojima <masahisa.kojima@linaro.org>
> > >   Date:   Thu Apr 28 17:09:41 2022 +0900
> > >
> > >       bootmenu: update bootmenu_entry structure

This patch modified the menu title from char* to
u16*(UTF16 string) to support the EFI system.
But if the system does not support EFI, printf("%ls") does not
work correctly.
 # I think currently only the first character is displayed.

I will send a fix.
Sorry for bothering you.

Thanks,
Masahisa Kojima

> > >
> > >       This is a preparation for succeeding addition of uefi boot
> > >       and distro boot menu entries into bootmenu.
> > >       The bootmenu_entry title is updated to u16 string because
> > >       uefi use u16 string. This commit also factors out the function
> > >       to prepare the entries generated by "bootmenu_x" U-Boot environment
> > >       variable.
> > >
> > >       Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
> > >       Reviewed-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> > >
> > >   :040000 040000 37c91714b049be307521831c0470b3b6c8af2407 b3e865073a46108cccbf4dc0b401789ef0b05a2e M      cmd
> > >
> > > Before that commit Bootmenu works fine and on serial console prints:
> > >
> > >   *** U-Boot Boot Menu ***
> > >
> > >       Attached kernel
> > >       Internal eMMC
> > >       External SD card
> > >       U-Boot boot order
> > >       U-Boot console
> > >
> > >
> > >   Press UP/DOWN to move, ENTER to select, ESC/CTRL+C to quit
> > >
> > >
> > > Could you please look at it and fix it?
> >
> > To be clear, the bootmenu stuff is or is not supposed to be tested with
> > the current rx51 test script?
>
> rx51 test script sets bootmenu timeout to 1s and just let bootmenu
> timeout. Timeout cause that default boot action is taken. Script does
> not interact with stdin / bootmenu.
>
> So... not selecting anything in bootmenu is working and hence rx51 test
> script did not complained.

Re: U-Boot v2022.07-rc2 regression: Bootmenu is completely broken

[Pali Rohár]

On Tuesday 24 May 2022 09:26:08 Masahisa Kojima wrote:
> On Tue, 24 May 2022 at 04:41, Pali Rohár <pali@kernel.org> wrote:
> >
> > On Monday 23 May 2022 15:29:04 Tom Rini wrote:
> > > On Mon, May 23, 2022 at 09:27:37PM +0200, Pali Rohár wrote:
> > > > Hello!
> > > >
> > > > U-Boot Bootmenu in git master branch is completely broken and does not
> > > > work. You can test it e.g. in qemu version of nokia n900 (see rx51.rst).
> > > >
> > > > Bootmenu just prints garbage on both serial console and VGA video
> > > > output and no option works.
> > > >
> > > >   *** U-Boot Boot Menu ***
> > > >
> > > >       A
> > > >       I
> > > >       E
> > > >       U
> > > >       U
> > > >
> > > >
> > > >   Press UP/DOWN to move, ENTER to select, ESC/CTRL+C to quit
> > > >
> > > > I bisected this issue and problem is in:
> > > >
> > > >   a3d0aa87acbec4b983f54c24a6e7fd8e9e7b9728 is the first bad commit
> > > >   commit a3d0aa87acbec4b983f54c24a6e7fd8e9e7b9728
> > > >   Author: Masahisa Kojima <masahisa.kojima@linaro.org>
> > > >   Date:   Thu Apr 28 17:09:41 2022 +0900
> > > >
> > > >       bootmenu: update bootmenu_entry structure
> 
> This patch modified the menu title from char* to
> u16*(UTF16 string) to support the EFI system.
> But if the system does not support EFI, printf("%ls") does not
> work correctly.

Now I see it from commit description.

But WHY on the earth?? It double size of all strings which is the issue
for space limited devices. Nobody sane is using legacy encoding UTF-16
in these days and even Windows people are moving to UTF-8. So
introduction of UTF-16 in **common** U-Boot code is step backward. And
if EFI is the reason, it should have been put into EFI code, not into
common U-Boot code. I really dislike this EFI bloatware... I have
checked and this commit increased size of u-boot binaries also on
non-EFI platforms, which is really bad. Sigh. For example on Nokia N900
we really do not have free space for this EFI nonsense.

>  # I think currently only the first character is displayed.
> 
> I will send a fix.
> Sorry for bothering you.
> 
> Thanks,
> Masahisa Kojima
> 
> > > >
> > > >       This is a preparation for succeeding addition of uefi boot
> > > >       and distro boot menu entries into bootmenu.
> > > >       The bootmenu_entry title is updated to u16 string because
> > > >       uefi use u16 string. This commit also factors out the function
> > > >       to prepare the entries generated by "bootmenu_x" U-Boot environment
> > > >       variable.
> > > >
> > > >       Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
> > > >       Reviewed-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> > > >
> > > >   :040000 040000 37c91714b049be307521831c0470b3b6c8af2407 b3e865073a46108cccbf4dc0b401789ef0b05a2e M      cmd
> > > >
> > > > Before that commit Bootmenu works fine and on serial console prints:
> > > >
> > > >   *** U-Boot Boot Menu ***
> > > >
> > > >       Attached kernel
> > > >       Internal eMMC
> > > >       External SD card
> > > >       U-Boot boot order
> > > >       U-Boot console
> > > >
> > > >
> > > >   Press UP/DOWN to move, ENTER to select, ESC/CTRL+C to quit
> > > >
> > > >
> > > > Could you please look at it and fix it?
> > >
> > > To be clear, the bootmenu stuff is or is not supposed to be tested with
> > > the current rx51 test script?
> >
> > rx51 test script sets bootmenu timeout to 1s and just let bootmenu
> > timeout. Timeout cause that default boot action is taken. Script does
> > not interact with stdin / bootmenu.
> >
> > So... not selecting anything in bootmenu is working and hence rx51 test
> > script did not complained.