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

[Glenn Washburn]

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

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.

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

Glenn

> 
> > > +         commands
> > > +                 silent
> > > +                 delete $break_efi_start_bpnum
> > > +                 set $onstart_name = "onstart"
> > > +                 is_user_command $onstart_name
> > > +                 if $ret
> > > +                         onstart
> > > +                 end
> > > +                 continue
> > > +         end
> > > +         set $break_efi_start_bpnum = $bpnum
> > > +         continue
> > > + end
> > > +end
> > > +document run_on_start
> > > + On some targets, such as x86_64-efi, even if you know
> > > where the
> > > + firmware will load the grub image, you can not simply set
> > > a break
> > 
> > Nit, s/grub/GRUB/...
> > 
> > > + point before the image is loaded because loading the
> > > image
> > > + overwrites the break point in memory. So setup a hardware
> > > watch
> > > + point, which does not have that problem, and if that gets
> > > triggered,
> > > + then reset the break point. If a user-defined command
> > > named
> > > + "onstart" exists it will be run after the start is hit.
> > > + NOTE: This assumes symbols have already been correctly
> > > loaded for
> > > + the EFI application.
> > > +end
> > > +
> > >  ###
> > >
> > >  set confirm off
> > > @@ -151,6 +188,7 @@ if ! $runonce
> > >           exec-file kernel.exec
> > >   else
> > >           file kernel.exec
> > > +         run_on_start
> > >           runtime_load_module
> > >   end
> > 
> > Daniel