On Fri, Apr 3, 2020 at 6:03 PM Daniel Thompson
<daniel.thomp...@linaro.org> wrote:
>
> On Tue, Mar 31, 2020 at 11:23:09PM +0800, Vincent Chen wrote:
> > The XML packet could be supported by required architecture if the
> > architecture defines CONFIG_ACRH_SUPPORTS_GDB_XML and implement its own
> > arch_handle_qxfer_pkt(). Except for the arch_handle_qxfer_pkt(), the
> > architecture also needs to record the feature supported by gdb stub into
> > the arch_gdb_stub_feature, and these features will be reported to host gdb
> > when gdb stub receives the qSupported packet.
> >
> > Signed-off-by: Vincent Chen <vincent.c...@sifive.com>
> > ---
> >  include/linux/kgdb.h   |  9 +++++++++
> >  kernel/debug/gdbstub.c | 13 +++++++++++++
> >  lib/Kconfig.kgdb       |  5 +++++
> >  3 files changed, 27 insertions(+)
> >
> > diff --git a/include/linux/kgdb.h b/include/linux/kgdb.h
> > index b072aeb1fd78..ee9109d2f056 100644
> > --- a/include/linux/kgdb.h
> > +++ b/include/linux/kgdb.h
> > @@ -177,6 +177,15 @@ kgdb_arch_handle_exception(int vector, int signo, int 
> > err_code,
> >                          struct pt_regs *regs);
> >
> >  /**
> > + *   arch_handle_qxfer_pkt - Handle architecture specific GDB XML packets.
> > + *   @remcom_in_buffer: The buffer of the packet we have read.
> > + *   @remcom_out_buffer: The buffer of %BUFMAX bytes to write a packet 
> > into.
> > + */
> > +
> > +extern void
> > +arch_handle_qxfer_pkt(char *remcom_in_buffer, char *remcom_out_buffer);
>
> This should be prefixed kgdb_ like the other arch functions.
>

Ok, I will add the prefixed kgdb_ to the arch_handle_qxfer_pkt().

>
> > +
> > +/**
> >   *   kgdb_call_nmi_hook - Call kgdb_nmicallback() on the current CPU
> >   *   @ignored: This parameter is only here to match the prototype.
> >   *
> > diff --git a/kernel/debug/gdbstub.c b/kernel/debug/gdbstub.c
> > index 4b280fc7dd67..d6b1b630a7e7 100644
> > --- a/kernel/debug/gdbstub.c
> > +++ b/kernel/debug/gdbstub.c
> > @@ -792,6 +792,19 @@ static void gdb_cmd_query(struct kgdb_state *ks)
> >               }
> >               break;
> >  #endif
> > +#ifdef CONFIG_ACRH_SUPPORTS_GDB_XML
>
> Typo (and perhaps insufficient testing ;-) ).
>
> Additional the naming of the CONFIG option looks wrong because it
> describes why you added it, not what it actually does. Something
> like CONFIG_HAVE_ARCH_KGDB_QXFER_PKT is more descriptive.
>

OK, I will modify it.

>
> > +     case 'S':
> > +             if (!strncmp(remcom_in_buffer, "qSupported:", 11))
> > +                     strcpy(remcom_out_buffer, arch_gdb_stub_feature);
>
> Has this been declared anywhere? I cannot find it.
>
> This might also benefit from a kgdb_ prefix.
>

I think the supported functions depend on the implementation of the
architectures.
Therefore, I define arch_gdb_stub_feature[] in the
arch/riscv/include/asm/gdb_xml.h.
OK, I will add the kgdb_ prefix to arch_gdb_stub_feature[].
Thanks

>
> > +             break;
> > +     case 'X':
> > +             if (!strncmp(remcom_in_buffer, "qXfer:", 6))
> > +                     arch_handle_qxfer_pkt(remcom_in_buffer,
> > +                                           remcom_out_buffer);
> > +             break;
> > +#endif
> > +     default:
> > +             break;
> >       }
> >  }
> >
> > diff --git a/lib/Kconfig.kgdb b/lib/Kconfig.kgdb
> > index 933680b59e2d..5b586a3bba90 100644
> > --- a/lib/Kconfig.kgdb
> > +++ b/lib/Kconfig.kgdb
> > @@ -3,6 +3,11 @@
> >  config HAVE_ARCH_KGDB
> >       bool
> >
> > +# set if architecture implemented the arch_handle_qxfer_pkt function
> > +# to enable gdb stub to address XML packet sent from GDB.
> > +config ARCH_SUPPORTS_GDB_XML
> > +     bool
> > +
> >  menuconfig KGDB
> >       bool "KGDB: kernel debugger"
> >       depends on HAVE_ARCH_KGDB
> > --
> > 2.7.4
> >


_______________________________________________
Kgdb-bugreport mailing list
Kgdb-bugreport@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kgdb-bugreport

Reply via email to