Re: [PATCH v4 12/15] gdb: Allow running user-defined commands at GRUB start

[Daniel Kiper]

On Thu, Dec 22, 2022 at 12:08:17AM -0600, Glenn Washburn wrote:
> On Wed, 21 Dec 2022 12:19:16 -0600
> Glenn Washburn <development@efficientek.com> wrote:
>
> > On Wed, 21 Dec 2022 16:27:40 +0100
> > Daniel Kiper <dkiper@net-space.pl> wrote:
> >
> > > On Thu, Dec 15, 2022 at 11:29:35PM -0600, Glenn Washburn wrote:
> > > > A new command, run_on_start, is created which handles some
> > > > complexities of the EFI platform when breaking on GRUB start. If
> > > > GRUB start is hooked, run "onstart" command if it is defned.
> > > >
> > > > Signed-off-by: Glenn Washburn <development@efficientek.com>
> > > > ---
> > > >  grub-core/gdb_grub.in | 38 ++++++++++++++++++++++++++++++++++++++
> > > >  1 file changed, 38 insertions(+)
> > > >
> > > > diff --git a/grub-core/gdb_grub.in b/grub-core/gdb_grub.in
> > > > index 8ae6344edf..3b3cea1a4d 100644
> > > > --- a/grub-core/gdb_grub.in
> > > > +++ b/grub-core/gdb_grub.in
> > > > @@ -36,6 +36,8 @@ end
> > > >  define dynamic_load_symbols
> > > >         dynamic_load_kernel_exec_symbols $arg0
> > > >
> > > > +       run_on_start
> > > > +
> > > >         # We may have been very late to loading the kernel.exec
> > > > symbols and # and modules may already be loaded. So load symbols
> > > > for any already # loaded.
> > > > @@ -134,6 +136,41 @@ document runtime_load_module
> > > >         Load module symbols at runtime as they are loaded.
> > > >  end
> > > >
> > > > +define run_on_start
> > > > +       # TODO: Add check to see if _start symbol is defined, if
> > > > not, then
> > > > +       # the symbols have not yet been loaded and this command
> > > > will not work.
> > > > +       watch *_start
> > > > +       set $break_efi_start_bpnum = $bpnum
> > > > +       commands
> > > > +               silent
> > > > +               delete $break_efi_start_bpnum
> > > > +               break _start
> > >
> > > s/break/hbreak/?
> >
> > A regular break works here for me. I want to avoid hbreak at all
> > costs, which is why I had a previous convoluted method using break
> > (which I thought worked, and then found it didn't quite). My
> > understanding is that the number of hardware breakpoints are limited
> > and commonly its around 4. Specifically, my understanding is that on
> > x86-64 the number is exactly 4, so I would prefer the user have
> > usable as many as possible.
> >
> > Really, I'd like to figure out why sometimes break works and why
> > sometimes not, and then figure out a way to make it work for these
> > scripts. I recently had the idea that maybe the UEFI firmware sets up
> > the pages where it loads the .text section of the GRUB UEFI binary to
> > readonly in the page table structure. But I went through the structure
> > when %eip is at _start and the R/W bit is set on the pages I checked.
> > Even if the pages were set to readonly, I suspect the qemu gdb stub
> > allows writing to that memory anyway.
> >
> > So I'm at a loss as to what could be preventing break from working.
> > I'd love to hear some ideas if anyone has some.
>
> Okay so its been a while since I was deep in this and I forgot some
> stuff I already knew (and which I should incorporate into the
> documentation patch). The reason that software break doesn't always

Great!

> work, is that when its not working its because the breakpoint is being
> set before the GRUB EFI app is loaded into memory. So happens is that
> GDB sets a breakpoint (ie a 0xcc instruction where the symbol points
> to), but it then gets over-written by the UEFI firmware as its loads
> the GRUB EFI app. So the app loading effectively will clear all
> software breakpoints. This doesn't affect the hardware breakpoints
> because they are in CPU debug registers which are not affected by the
> firmware loading the EFI app.

Yeah, this is what I expected...

> The reason that the above worked, and was written that way in the first
> place, is that when the watch commands run the memory at _start has just
> been modified. This happens when the UEFI firmware is loading the GRUB
> EFI app. Then we can use software break without worrying about it being
> cleared.

Could you add this (whole) explanation to the commit message to make it
clear why we can use "break" here?

> Revisiting this has given me some ideas for improving the patch series.

Cool!

Thanks,

Daniel