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

[Oliver Steffen]

Quoting Oliver Steffen (2023-10-02 10:28:08)
> 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?

I thnik it should be possible to specify a dependency on the `part_gpt`
module, which would be much more elegant.
But I can't get it to work somehow.


>
> -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
> >