Re: [RFC PATCH] gdb: Add more support for debugging on EFI platforms

[Robbie Harwood <rharwood@redhat.com>]

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

Glenn Washburn <development@efficientek.com> writes:

> On 3/9/23 23:00, Robbie Harwood wrote:
>> Glenn Washburn <development@efficientek.com> writes:
>> 
>>> If the configure option --enable-efi-debug is given, then enable the
>>> printing early in EFI startup of the command needed to load symbols for
>>> the GRUB EFI kernel. This is needed because EFI firmware determines where
>>> to load the GRUB EFI at runtime, and so the relevant addresses are not
>>> known ahead of time. This is not printed when secure boot is enabled.
>>>
>>> The command is a custom command defined in the gdb_grub GDB script. So
>>> GDB should be started with the script as an argument to the -x option or
>>> sourced into an active GDB session before running the outputted command.
>>>
>>> Also a command named "gdbinfo" is enabled which allows the user to print
>>> the gdb command string on-demand, which can be valuable as the printing
>>> early in EFI startup is quickly replaced by other text. So if using a
>>> physical screen it may appear too briefly to be registered.
>>>
>>> Co-developed-by: Peter Jones <pjones@redhat.com>
>>> Signed-off-by: Glenn Washburn <development@efficientek.com>
>>> ---
>>> This is patch 9 from the v6 "GDB script fixes and improvements" series, with
>>> one modification. Now the gdbinfo command will print the gdb load command
>>> even when the configure option is not enabled (though still not when lockdown
>>> is enabled).
>>>
>>> Robbie had 2 concerns with the last patch.
>>>
>>>   1. Does this need to be configurable?
>>>     * I responded that this was requested by Daniel because of concerns about
>>>       it breaking silent boot and it seemed reasonable to me, but that I don't
>>>       have a strong opinion. I've left it configurable until Dnaiel weighs in.
>> 
>> Yeah, I think these concerns are valid.  The version in the rhboot
>> tree gates printing on an env var.  Right now, it seems to me that:
>> 
>> - we want it to be default-off because silent boot
>
> I understand you to be talking about a default-off at runtime, not
> built time. Right now there is a configure option which defaults to
> off, is this acceptable?

Indeed, I'm talking about runtime configurability.  Build-time
configurability means it's either always on (bad) or we have to rebuild
in order to debug (annoying, interacts poorly with scureboot).

>> - we want to have the ability to reenable without rebuilding because
>>    secureboot, convenience, etc.
>
> This would be great, but how do you propose that this would work? This 
> patch will print very early in EFI init. We can't use GRUB variables. We 
> probably could use EFI variables, but this needs to be well defined (and 
> not by me, since I don't have this requirement). I'm not sure if the 
> GRUB env block is available at this point, but that might be an option.
>
> Can you point me to RH's patch you've referred to? Does it meet this 
> requirement, and if so how?

I thought you were basing your work off it, given the "Co-developed-by"
and your review on v1:
https://lists.gnu.org/archive/html/grub-devel/2021-10/msg00076.html

(v2 was
https://lists.gnu.org/archive/html/grub-devel/2021-11/msg00006.html and
the versions we're shipping are
https://github.com/rhboot/grub2/commit/93803e83135e074ed5a3e7f67af22538896dbefe
and
https://github.com/rhboot/grub2/commit/c314270222b4adffa16840e3ca764d793e93a0e8
)

As you say, the grub env block isn't available (module loading happens
before filesystems).  We have an additional patch that allows the use of
an EFI variable for setting grubenv:
https://github.com/rhboot/grub2/commit/22a11bfa59ff8f239cfe8698274281bc919673dc
which lets one do e.g.

    grub2-editenv ./grubenv set debug=gdb,modules,dl
    { printf '\x07\x00\x00\x00' ; cat ./grubenv ; } > GRUB_ENV-91376aff-cba6-42be-949d-06fde81128e8
    cp GRUB_ENV-91376aff-cba6-42be-949d-06fde81128e8 /sys/firmware/efi/efivars/

and then have configuration for the next boot.  Javier informally
proposed this to the list a few years ago for ZFS-related things:
https://lists.gnu.org/archive/html/grub-devel/2020-02/msg00045.html but
I don't think it was ever followed up on, possibly because ZFS is not
one of our use cases :)

I'm happy to formally propose it, or feel free to include/adapt it if
it's helpful to you, etc..

Be well,
--Robbie

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