RE: [PATCH v6 07/10] btrace, gdbserver: Add ptwrite to btrace_config_pt.

["Willgerodt, Felix" <felix.willgerodt@intel.com>]

> -----Original Message-----
> From: Metzger, Markus T <markus.t.metzger@intel.com>
> Sent: Mittwoch, 19. Oktober 2022 16:42
> To: Willgerodt, Felix <felix.willgerodt@intel.com>
> Cc: gdb-patches@sourceware.org
> Subject: RE: [PATCH v6 07/10] btrace, gdbserver: Add ptwrite to
> btrace_config_pt.
> 
> Hello Felix,
> 
> >This enables gdb and gdbserver to communicate about ptwrite support.  If
> >ptwrite support would be enabled unconditionally, GDBs with older libipt
> >versions would break.
> >---
> > gdb/btrace.c                 |  8 +++++++-
> > gdb/doc/gdb.texinfo          | 21 +++++++++++++++++++++
> > gdb/features/btrace-conf.dtd |  1 +
> > gdb/remote.c                 | 30 ++++++++++++++++++++++++++++++
> > gdbserver/linux-low.cc       |  2 ++
> > gdbserver/server.cc          | 18 ++++++++++++++++++
> > gdbsupport/btrace-common.h   |  6 ++++++
> > 7 files changed, 85 insertions(+), 1 deletion(-)
> 
> Looks good to me with one nit:
> 
> >--- a/gdb/features/btrace-conf.dtd
> >+++ b/gdb/features/btrace-conf.dtd
> >@@ -12,3 +12,4 @@
> >
> > <!ELEMENT pt	EMPTY>
> > <!ATTLIST pt	size	CDATA	#IMPLIED>
> >+<!ATTLIST pt	ptwrite	(yes | no) #IMPLIED>
> 
> Should there be a space between 'ptwrite' and '(yes|no)'?

There is a tab there. That matches with the alignment of the file.
The other lines also have tabs.
I am not quite sure what you want changed.

> >diff --git a/gdbserver/linux-low.cc b/gdbserver/linux-low.cc
> >index 2f71360d3bd..1333ec13dbd 100644
> >--- a/gdbserver/linux-low.cc
> >+++ b/gdbserver/linux-low.cc
> >@@ -6806,6 +6806,8 @@ linux_process_target::read_btrace_conf (const
> >btrace_target_info *tinfo,
> > 	case BTRACE_FORMAT_PT:
> > 	  buffer_xml_printf (buffer, "<pt");
> > 	  buffer_xml_printf (buffer, " size=\"0x%x\"", conf->pt.size);
> >+	  const char *ptw = conf->pt.ptwrite ? "yes" : "no";
> >+	  buffer_xml_printf (buffer, " ptwrite=\"%s\"", ptw);
> 
> We don't really need the PTW variable.

Right. I will inline it.

Thanks,
Felix

Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928