Re: bli module inclusion gives me just black screen on real hardware

[Oliver Steffen]

On Sun, Oct 1, 2023 at 4:08 PM Tobias Powalowski
<tobias.powalowski@googlemail.com> wrote:
>
> Am 27.09.23 um 09:43 schrieb Oliver Steffen:
> > Quoting Daniel Kiper (2023-09-26 17:57:05)
> >> On Sat, Aug 12, 2023 at 03:01:19PM +0200, Tobias Powalowski via Grub-devel 
> >> wrote:
> >>> Am 12.08.23 um 14:52 schrieb Oliver Steffen:
> >>>> Quoting Daniel Kiper (2023-08-11 18:13:57)
> >>>>> Hi,
> >>>>>
> >>>>> Sorry for late reply but I was on vacation...
> >>>>>
> >>>>> On Tue, Jul 18, 2023 at 02:34:08PM +0200, Tobias Powalowski via 
> >>>>> Grub-devel wrote:
> >>>>>> Hi,
> >>>>>>
> >>>>>> I tried to add the bli module to my standalone grub for my project. In 
> >>>>>> qemu
> >>>>>> testing all is fine, if trying to boot it on real hardware gives me 
> >>>>>> just a
> >>>>>> blank screen and game over.
> >>>> Sorry to hear that.
> >>>> I just tried it again on a Lenovo T14s (x86_64) and I can reach the menu
> >>>> and the grub shell without problems.  What hardware are you using?
> >>>>
> >>>>>> Used grub version is 2.12rc1.
> >>>>>>
> >>>>>> Any ideas?
> >>>> You could apply following patch.  It adds debug prints to the bli modue.
> >>>> Then enable debug output in your grub.cfg by setting
> >>>>
> >>>>      set debug=bli
> >>>>      set pager=1
> >>>>
> >>>> I recommend adding
> >>>>
> >>>>     sleep 10 --verbose --interruptible
> >>>>
> >>>> just after the line loading the bli module.  This is to keep the
> >>>> messages visible for a moment before the menu clears the screen (should
> >>>> it get to there).
> >>>> This might give us a hint where it goes wrong.
> >>>>
> >>>>
> >>>>   From cda95baaf0a3c0f277a5e37aff399574627b2263 Mon Sep 17 00:00:00 2001
> >>>> From: Oliver Steffen <osteffen@redhat.com>
> >>>> Date: Sat, 12 Aug 2023 14:23:31 +0200
> >>>> Subject: [PATCH] bli dprint
> >>>>
> >>>> ---
> >>>>    grub-core/commands/bli.c | 15 ++++++++++++---
> >>>>    1 file changed, 12 insertions(+), 3 deletions(-)
> >>>>
> >>>> diff --git a/grub-core/commands/bli.c b/grub-core/commands/bli.c
> >>>> index e0d8a54f7..95caa4feb 100644
> >>>> --- a/grub-core/commands/bli.c
> >>>> +++ b/grub-core/commands/bli.c
> >>>> @@ -89,23 +89,30 @@ set_loader_device_part_uuid (void)
> >>>>      grub_err_t status = GRUB_ERR_NONE;
> >>>>      char *part_uuid = NULL;
> >>>>
> >>>> +  grub_dprintf ("bli", "getting loaded image\n");
> >>>>      image = grub_efi_get_loaded_image (grub_efi_image_handle);
> >>>>      if (image == NULL)
> >>>>        return grub_error (GRUB_ERR_BAD_DEVICE, N_("unable to find boot 
> >>>> device"));
> >>>>
> >>>> +  grub_dprintf ("bli", "getting boot disk\n");
> >>>>      device_name = grub_efidisk_get_device_name (image->device_handle);
> >>>>      if (device_name == NULL)
> >>>>        return grub_error (GRUB_ERR_BAD_DEVICE, N_("unable to find boot 
> >>>> device"));
> >>>>
> >>>> +  grub_dprintf ("bli", "getting uuid\n");
> >>>>      status = get_part_uuid (device_name, &part_uuid);
> >>>>
> >>>>      if (status == GRUB_ERR_NONE)
> >>>> -    status = grub_efi_set_variable_to_string ("LoaderDevicePartUUID",
> >>>> &bli_vendor_guid, part_uuid,
> >>>> -                                         
> >>>> GRUB_EFI_VARIABLE_BOOTSERVICE_ACCESS |
> >>>> -                                         
> >>>> GRUB_EFI_VARIABLE_RUNTIME_ACCESS);
> >>>> +    {
> >>>> +      grub_dprintf ("bli", "setting var\n");
> >>>> +      status = grub_efi_set_variable_to_string
> >>>> ("LoaderDevicePartUUID", &bli_vendor_guid, part_uuid,
> >>>> +                                           
> >>>> GRUB_EFI_VARIABLE_BOOTSERVICE_ACCESS |
> >>>> +                                           
> >>>> GRUB_EFI_VARIABLE_RUNTIME_ACCESS);
> >>>> +    }
> >>>>      else
> >>>>        grub_error (status, N_("unable to determine partition UUID of
> >>>> boot device"));
> >>>>
> >>>> +  grub_dprintf ("bli", "cleanup\n");
> >>>>      grub_free (part_uuid);
> >>>>      grub_free (device_name);
> >>>>      return status;
> >>>> @@ -113,8 +120,10 @@ set_loader_device_part_uuid (void)
> >>>>
> >>>>    GRUB_MOD_INIT (bli)
> >>>>    {
> >>>> +  grub_dprintf ("bli", "init start\n");
> >>>>      grub_efi_set_variable_to_string ("LoaderInfo", &bli_vendor_guid,
> >>>> PACKAGE_STRING,
> >>>>                                 GRUB_EFI_VARIABLE_BOOTSERVICE_ACCESS |
> >>>>                                 GRUB_EFI_VARIABLE_RUNTIME_ACCESS);
> >>>>      set_loader_device_part_uuid ();
> >>>> +  grub_dprintf ("bli", "init end\n");
> >>>>    }
> >>> Hi I'll try after my vaccation, give me 2 weeks and I report back.
> >> Are you back? Could you check the patch from Oliver?
> >>
> >> Daniel
> >>
> > I think it is worth also trying the GUID patches discussed in this thread:
> > https://mail.gnu.org/archive/html/grub-devel/2023-09/msg00045.html
> >
> > Oliver
> >
> Hi,
>
> ok I think I just missconfigured the standalone --modules="" array that
> triggers the hard hang. I changed that and now the bli module loads
> without issues on my systems. I wasn't able to get any pager information.
>
> Maybe it's a race condition if all available modules are put into
> --modules="" in alphabetical order to the array.

Module load order matters, because bli does its magic in the module init
function. It requires the gpt module to be present at that time to read
the partition table and get the GUID. This is the downside of doing
stuff in the init function instead of having the module provide a separate
'bli' command that needs to get called later.

This was introduced in v6 of the patch series, see
- https://mail.gnu.org/archive/html/grub-devel/2023-04/msg00009.html
- https://mail.gnu.org/archive/html/grub-devel/2023-04/msg00068.html

I get that this constraint on the module load order can be confusing,
but it saves the need to call another command.
Daniel, any thoughts on this? Is it worth reintroducing the bli command?

-Oliver

>
> Anyway now it works.
>
> greetings
>
> tpowa
>
> --
> Tobias Powalowski
> Arch Linux Developer (tpowa)
> https://www.archlinux.org
> tpowa@archlinux.org
>
> Archboot Developer
> https://archboot.com
>