| Message ID | 1585668191-16287-4-git-send-email-vincent.chen@sifive.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | riscv: Add KGDB and KDB support | expand |
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.chen@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. > + > +/** > * 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. > + 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. > + 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 >
On Fri, Apr 3, 2020 at 6:03 PM Daniel Thompson <daniel.thompson@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.chen@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 > >
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); + +/** * 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 + case 'S': + if (!strncmp(remcom_in_buffer, "qSupported:", 11)) + strcpy(remcom_out_buffer, arch_gdb_stub_feature); + 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
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.chen@sifive.com> --- include/linux/kgdb.h | 9 +++++++++ kernel/debug/gdbstub.c | 13 +++++++++++++ lib/Kconfig.kgdb | 5 +++++ 3 files changed, 27 insertions(+)