[RFC 00/13] Proposal for a new NT_X86_CPUID core dump note

[RFC 00/13] Proposal for a new NT_X86_CPUID core dump note

[John Baldwin]

One of the shortcomings of the previous XSAVE patch series is that it
depends on heuristics based on the total XSAVE register set size and
XCR0 mask to infer layouts of the various register blocks for core
dumps.  This series introduces a new x86-specific core dump note
intended to supplant these heuristics by storing the raw CPUID leaves
describing the XSAVE layout in core dumps.

This series proposes a new core dump note, NT_X86_CPUID (0x205), which
contains an array of structures.  Each structure describes an invidual
CPUID sub-leaf containing both the inputs to CPUID (%eax and %ecx) and
the outputs (%eax, %ebx, %ecx, and %edx) in a format roughly matching
the follow C structure:

struct cpuid_leaf
{
    uint32_t leaf;
    uint32_t subleaf;
    uint32_t eax;
    uint32_t ebx;
    uint32_t ecx;
    uint32_t edx;
};

This format is not XSAVE-specific and implementations could choose to
add additional CPUID leaves to this structure if needed in the future.
Consumers of this note should lookup the value of required leaves and
ignore any unneeded leaves.

An alternate approach might be to write out a more XSAVE-specific note
that is an array containing the offset and size of each XSAVE region.

Note that either approach would enable storing XSAVE notes in the
"compact" format at some point in the future.

This series adds support for reading/writing the note to binutils as
well as suport for parsing and generating the note in GDB.  It also
hooks this into both the FreeBSD and Linux x86 architectures in GDB to
read the XSAVE layout from this note when present, and to write out a
note when generating a core via `gcore'.  I've done some limited
testing on FreeBSD/amd64 and Linux/x86-64, but it could probably use
some more testing on Linux in particular.  (I know Simon has an AMD
machine with a layout not handled by the current heuristics for
example.)

For the gcore side, a new TARGET_OBJECT_X86_CPUID is used to fetch the
current note contents from a native target.  There is still one gap
even with this patch series which is that if you are connected to a
remote target (e.g. gdbserver), we currently do not have a known XSAVE
layout to use when writing out a core via `gcore'.  One option that
would close this gap would be to extend the remote protocol to permit
reading this new object from a debug server.  The remote target could
then implement fetching this object and also make use of this object
to implement the target::fetch_x86_xsave_layout method which would
close that gap.  Another possibility would be to just pick a "known"
XSAVE format that matches one of the heuristics.

The series is available from git@github.com:bsdjhb/gdb.git on the
`nt_x86_cpuid' branch.

I also have an implementation of this core dump note available for
FreeBSD's kernel, though I won't merge it until we've collectively
settled on the format: https://reviews.freebsd.org/D42136

Things I have not done and could use help with:

- Implementation for the Linux kernel

- Coordination with folks from LLDB

John Baldwin (13):
  binutils: Support for the NT_X86_CPUID core dump note
  i387-tdep: Add function to read XSAVE layout from NT_X86_CPUID
  gdb: Use NT_X86_CPUID in x86 FreeBSD architectures to read XSAVE
    layouts
  gdb: Use NT_X86_CPUID in x86 FreeBSD architectures to read XSAVE
    layouts
  nat/x86-cpuid.h: Remove non-x86 fallbacks
  nat/x86-cpuid: Add a function to build the contents of a NT_X86_CPUID
    note
  x86_elf_make_cpuid_note: Helper routine to build NT_X86_CPUID ELF note
  x86-fbsd-nat: Support fetching TARGET_OBJECT_X86_CPUID objects
  fbsd-tdep: Export fbsd_make_corefile_notes
  {amd64,i386}-fbsd-tdep: Include NT_X86_CPUID notes in core dumps from
    gcore
  x86-linux-nat: Support fetching TARGET_OBJECT_X86_CPUID objects
  linux-tdep: Export linux_make_corefile_notes
  {amd64,i386}-linux-tdep: Include NT_X86_CPUID notes in core dumps from
    gcore

 bfd/elf-bfd.h          |   2 +
 bfd/elf.c              |  35 +++++++++++
 binutils/readelf.c     |   2 +
 gdb/amd64-fbsd-tdep.c  |   1 +
 gdb/amd64-linux-tdep.c |   1 +
 gdb/configure.nat      |  13 ++--
 gdb/fbsd-tdep.c        |   5 +-
 gdb/fbsd-tdep.h        |   7 +++
 gdb/i386-fbsd-tdep.c   |  18 +++++-
 gdb/i386-fbsd-tdep.h   |   7 +++
 gdb/i386-linux-tdep.c  |  18 +++++-
 gdb/i386-linux-tdep.h  |   7 +++
 gdb/i387-tdep.c        | 132 +++++++++++++++++++++++++++++++++++++++++
 gdb/i387-tdep.h        |   8 +++
 gdb/linux-tdep.c       |   5 +-
 gdb/linux-tdep.h       |   7 +++
 gdb/nat/x86-cpuid.c    |  91 ++++++++++++++++++++++++++++
 gdb/nat/x86-cpuid.h    |  29 +++------
 gdb/target.h           |   2 +
 gdb/x86-fbsd-nat.c     |  37 ++++++++++++
 gdb/x86-fbsd-nat.h     |   9 +++
 gdb/x86-linux-nat.c    |  37 ++++++++++++
 gdb/x86-linux-nat.h    |   9 +++
 gdb/x86-tdep.c         |  22 +++++++
 gdb/x86-tdep.h         |   9 +++
 include/elf/common.h   |   2 +
 26 files changed, 480 insertions(+), 35 deletions(-)
 create mode 100644 gdb/nat/x86-cpuid.c

-- 
2.41.0

[RFC 01/13] binutils: Support for the NT_X86_CPUID core dump note

[John Baldwin]

This core dump note contains an array of CPUID leaf values.  Each
entry in the array contains six 32-bit integers describing the inputs
to the CPUID instruction (%eax and %ecx) and the outputs of the
instruction (%eax, %ebx, %ecx, and %edx) similar to the C structure:

struct cpuid_leaf
{
    uint32_t leaf;
    uint32_t subleaf;
    uint32_t eax;
    uint32_t ebx;
    uint32_t ecx;
    uint32_t edx;
};

Current implementations of this note contain leaves associated with
the XSAVE state area (major leaf 0xd), but future implementations may
add other leaf values in the future.
---
 bfd/elf-bfd.h        |  2 ++
 bfd/elf.c            | 35 +++++++++++++++++++++++++++++++++++
 binutils/readelf.c   |  2 ++
 include/elf/common.h |  2 ++
 4 files changed, 41 insertions(+)

diff --git a/bfd/elf-bfd.h b/bfd/elf-bfd.h
index 335081ec629..235d0931ab4 100644
--- a/bfd/elf-bfd.h
+++ b/bfd/elf-bfd.h
@@ -2871,6 +2871,8 @@ extern char *elfcore_write_prxfpreg
   (bfd *, char *, int *, const void *, int);
 extern char *elfcore_write_xstatereg
   (bfd *, char *, int *, const void *, int);
+extern char *elfcore_write_x86_cpuid
+  (bfd *, char *, int *, const void *, int);
 extern char *elfcore_write_x86_segbases
   (bfd *, char *, int *, const void *, int);
 extern char *elfcore_write_ppc_vmx
diff --git a/bfd/elf.c b/bfd/elf.c
index b5b0c69e097..35679821a49 100644
--- a/bfd/elf.c
+++ b/bfd/elf.c
@@ -10495,6 +10495,16 @@ elfcore_grok_xstatereg (bfd *abfd, Elf_Internal_Note *note)
   return elfcore_make_note_pseudosection (abfd, ".reg-xstate", note);
 }
 
+/* Some systems dump an array of x86 cpuid leaves with a note type of
+   NT_X86_CPUID.  Just include the whole note's contents
+   literally.  */
+
+static bool
+elfcore_grok_x86_cpuid (bfd *abfd, Elf_Internal_Note *note)
+{
+  return elfcore_make_note_pseudosection (abfd, ".reg-x86-cpuid", note);
+}
+
 static bool
 elfcore_grok_ppc_vmx (bfd *abfd, Elf_Internal_Note *note)
 {
@@ -11190,6 +11200,13 @@ elfcore_grok_note (bfd *abfd, Elf_Internal_Note *note)
       else
 	return true;
 
+    case NT_X86_CPUID:
+      if (note->namesz == 6
+	  && strcmp (note->namedata, "LINUX") == 0)
+	return elfcore_grok_x86_cpuid (abfd, note);
+      else
+	return true;
+
     case NT_PPC_VMX:
       if (note->namesz == 6
 	  && strcmp (note->namedata, "LINUX") == 0)
@@ -11768,6 +11785,9 @@ elfcore_grok_freebsd_note (bfd *abfd, Elf_Internal_Note *note)
     case NT_X86_XSTATE:
       return elfcore_grok_xstatereg (abfd, note);
 
+    case NT_X86_CPUID:
+      return elfcore_grok_x86_cpuid (abfd, note);
+
     case NT_FREEBSD_PTLWPINFO:
       return elfcore_make_note_pseudosection (abfd, ".note.freebsdcore.lwpinfo",
 					      note);
@@ -12640,6 +12660,19 @@ elfcore_write_xstatereg (bfd *abfd, char *buf, int *bufsiz,
 			     note_name, NT_X86_XSTATE, xfpregs, size);
 }
 
+char *
+elfcore_write_x86_cpuid (bfd *abfd, char *buf, int *bufsiz,
+			 const void *cpuid, int size)
+{
+  char *note_name;
+  if (get_elf_backend_data (abfd)->elf_osabi == ELFOSABI_FREEBSD)
+    note_name = "FreeBSD";
+  else
+    note_name = "LINUX";
+  return elfcore_write_note (abfd, buf, bufsiz,
+			     note_name, NT_X86_CPUID, cpuid, size);
+}
+
 char *
 elfcore_write_x86_segbases (bfd *abfd, char *buf, int *bufsiz,
 			    const void *regs, int size)
@@ -13233,6 +13266,8 @@ elfcore_write_register_note (bfd *abfd,
     return elfcore_write_prxfpreg (abfd, buf, bufsiz, data, size);
   if (strcmp (section, ".reg-xstate") == 0)
     return elfcore_write_xstatereg (abfd, buf, bufsiz, data, size);
+  if (strcmp (section, ".reg-x86-cpuid") == 0)
+    return elfcore_write_x86_cpuid (abfd, buf, bufsiz, data, size);
   if (strcmp (section, ".reg-x86-segbases") == 0)
     return elfcore_write_x86_segbases (abfd, buf, bufsiz, data, size);
   if (strcmp (section, ".reg-ppc-vmx") == 0)
diff --git a/binutils/readelf.c b/binutils/readelf.c
index c9b6210e229..cb80aa6f396 100644
--- a/binutils/readelf.c
+++ b/binutils/readelf.c
@@ -20134,6 +20134,8 @@ get_note_type (Filedata * filedata, unsigned e_type)
 	return _("NT_X86_XSTATE (x86 XSAVE extended state)");
       case NT_X86_CET:
 	return _("NT_X86_CET (x86 CET state)");
+      case NT_X86_CPUID:
+	return _("NT_X86_CPUID (x86 CPUID leaves)");
       case NT_S390_HIGH_GPRS:
 	return _("NT_S390_HIGH_GPRS (s390 upper register halves)");
       case NT_S390_TIMER:
diff --git a/include/elf/common.h b/include/elf/common.h
index 244b13361e5..e8c6d753987 100644
--- a/include/elf/common.h
+++ b/include/elf/common.h
@@ -645,6 +645,8 @@
 					/*   note name must be "LINUX".  */
 #define NT_X86_CET	0x203		/* x86 CET state.  */
 					/*   note name must be "LINUX".  */
+#define NT_X86_CPUID	0x205		/* x86 CPUID leaves.  */
+					/*   note name must be "LINUX".  */
 #define NT_S390_HIGH_GPRS 0x300		/* S/390 upper halves of GPRs  */
 					/*   note name must be "LINUX".  */
 #define NT_S390_TIMER	0x301		/* S390 timer */
-- 
2.41.0

[RFC 02/13] i387-tdep: Add function to read XSAVE layout from NT_X86_CPUID

[John Baldwin]

This can be used by x86 arches to determine the XSAVE layout instead
of guessing based on the XCR0 mask and XSAVE register note size.
---
 gdb/i387-tdep.c | 132 ++++++++++++++++++++++++++++++++++++++++++++++++
 gdb/i387-tdep.h |   8 +++
 2 files changed, 140 insertions(+)

diff --git a/gdb/i387-tdep.c b/gdb/i387-tdep.c
index 47667da21c7..1eac2b6bd2a 100644
--- a/gdb/i387-tdep.c
+++ b/gdb/i387-tdep.c
@@ -26,6 +26,8 @@
 #include "target-float.h"
 #include "value.h"
 
+#include <stdexcept>
+
 #include "i386-tdep.h"
 #include "i387-tdep.h"
 #include "gdbsupport/x86-xstate.h"
@@ -987,6 +989,136 @@ i387_guess_xsave_layout (uint64_t xcr0, size_t xsave_size,
   return true;
 }
 
+/* Parse a reg-x86-cpuid pseudo section building a hash table mapping
+   cpuid leaves to their results.  */
+
+struct cpuid_key
+{
+  cpuid_key (uint32_t _leaf, uint32_t _subleaf)
+    : leaf(_leaf), subleaf(_subleaf)
+  {}
+
+  uint32_t leaf;
+  uint32_t subleaf;
+
+  constexpr bool operator== (const cpuid_key &other) const
+  { return (leaf == other.leaf && subleaf == other.subleaf); }
+};
+
+namespace std
+{
+template<>
+struct hash<cpuid_key>
+{
+  size_t operator() (const cpuid_key &key) const
+  {
+    return key.leaf ^ (key.subleaf << 1);
+  }
+};
+}
+
+struct cpuid_values
+{
+  cpuid_values (uint32_t _eax, uint32_t _ebx, uint32_t _ecx, uint32_t _edx)
+    : eax(_eax), ebx(_ebx), ecx(_ecx), edx(_edx)
+  {}
+
+  uint32_t eax;
+  uint32_t ebx;
+  uint32_t ecx;
+  uint32_t edx;
+};
+
+typedef std::unordered_map<cpuid_key, cpuid_values> cpuid_map;
+
+static cpuid_map
+i387_parse_cpuid_from_core (bfd *bfd)
+{
+  asection *section = bfd_get_section_by_name (bfd, ".reg-x86-cpuid");
+  if (section == nullptr)
+    return {};
+
+  size_t size = bfd_section_size (section);
+  if (size == 0 || (size % (6 * 4)) != 0)
+    return {};
+
+  char contents[size];
+  if (!bfd_get_section_contents (bfd, section, contents, 0, size))
+    {
+      warning (_("Couldn't read `.reg-x86-cpuid' section in core file."));
+      return {};
+    }
+
+  cpuid_map map;
+  size_t index = 0;
+  while (index < size)
+    {
+      uint32_t leaf = bfd_get_32 (bfd, contents + index);
+      uint32_t count = bfd_get_32 (bfd, contents + index + 4);
+      uint32_t eax = bfd_get_32 (bfd, contents + index + 8);
+      uint32_t ebx = bfd_get_32 (bfd, contents + index + 12);
+      uint32_t ecx = bfd_get_32 (bfd, contents + index + 16);
+      uint32_t edx = bfd_get_32 (bfd, contents + index + 20);
+
+      if (map.count (cpuid_key (leaf, count)) != 0)
+	{
+	  warning (_("Duplicate cpuid leaf %#x,%#x"), leaf, count);
+	  return {};
+	}
+      map.emplace (cpuid_key (leaf, count),
+		   cpuid_values (eax, ebx, ecx, edx));
+
+      index += 6 * 4;
+    }
+
+  return map;
+}
+
+/* Fetch the offset of a specific XSAVE extended region.  */
+
+static int
+xsave_feature_offset (cpuid_map &map, uint64_t xcr0, int feature)
+{
+  if ((xcr0 & (1ULL << feature)) == 0)
+    return 0;
+
+  return map.at (cpuid_key (0xd, feature)).ebx;
+}
+
+/* See i387-tdep.h.  */
+
+bool
+i387_read_xsave_layout_from_core (bfd *bfd, uint64_t xcr0, size_t xsave_size,
+				  x86_xsave_layout &layout)
+{
+  cpuid_map map = i387_parse_cpuid_from_core (bfd);
+  if (map.empty ())
+    return false;
+
+  try
+    {
+      layout.sizeof_xsave = xsave_size;
+      layout.avx_offset = xsave_feature_offset (map, xcr0,
+						X86_XSTATE_AVX_ID);
+      layout.bndregs_offset = xsave_feature_offset (map, xcr0,
+						    X86_XSTATE_BNDREGS_ID);
+      layout.bndcfg_offset = xsave_feature_offset (map, xcr0,
+						   X86_XSTATE_BNDCFG_ID);
+      layout.k_offset = xsave_feature_offset (map, xcr0,
+					      X86_XSTATE_K_ID);
+      layout.zmm_h_offset = xsave_feature_offset (map, xcr0,
+						  X86_XSTATE_ZMM_H_ID);
+      layout.zmm_offset = xsave_feature_offset (map, xcr0, X86_XSTATE_ZMM_ID);
+      layout.pkru_offset = xsave_feature_offset (map, xcr0, X86_XSTATE_PKRU_ID);
+    }
+  catch (const std::out_of_range &)
+    {
+      return false;
+    }
+
+  return true;
+}
+
 /* Extract from XSAVE a bitset of the features that are available on the
    target, but which have not yet been enabled.  */
 
diff --git a/gdb/i387-tdep.h b/gdb/i387-tdep.h
index e149e30e52e..b16b9a60b67 100644
--- a/gdb/i387-tdep.h
+++ b/gdb/i387-tdep.h
@@ -147,6 +147,14 @@ extern void i387_supply_fxsave (struct regcache *regcache, int regnum,
 extern bool i387_guess_xsave_layout (uint64_t xcr0, size_t xsave_size,
 				     x86_xsave_layout &layout);
 
+/* Determine the XSAVE layout from the `reg-x86-cpuid` section in a
+   core dump.  Returns true on sucess, or false if a layout can not be
+   read.  */
+
+extern bool i387_read_xsave_layout_from_core (bfd *bfd, uint64_t xcr0,
+					      size_t xsave_size,
+					      x86_xsave_layout &layout);
+
 /* Similar to i387_supply_fxsave, but use XSAVE extended state.  */
 
 extern void i387_supply_xsave (struct regcache *regcache, int regnum,
-- 
2.41.0

[RFC 03/13] gdb: Use NT_X86_CPUID in x86 FreeBSD architectures to read XSAVE layouts

[John Baldwin]

If this core dump note is present, use it to determine the layout of
XSAVE register set notes.
---
 gdb/i386-fbsd-tdep.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/gdb/i386-fbsd-tdep.c b/gdb/i386-fbsd-tdep.c
index f8c2758c2b0..1789f3921fd 100644
--- a/gdb/i386-fbsd-tdep.c
+++ b/gdb/i386-fbsd-tdep.c
@@ -266,7 +266,8 @@ i386_fbsd_core_read_xsave_info (bfd *abfd, x86_xsave_layout &layout)
 
   uint64_t xcr0 = bfd_get_64 (abfd, contents);
 
-  if (!i387_guess_xsave_layout (xcr0, size, layout))
+  if (!i387_read_xsave_layout_from_core (abfd, xcr0, size, layout)
+      && !i387_guess_xsave_layout (xcr0, size, layout))
     return 0;
 
   return xcr0;
-- 
2.41.0

[RFC 04/13] gdb: Use NT_X86_CPUID in x86 FreeBSD architectures to read XSAVE layouts

[John Baldwin]

If this core dump note is present, use it to determine the layout of
XSAVE register set notes.
---
 gdb/i386-linux-tdep.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/gdb/i386-linux-tdep.c b/gdb/i386-linux-tdep.c
index 7ff7c155e2c..57d00a424d9 100644
--- a/gdb/i386-linux-tdep.c
+++ b/gdb/i386-linux-tdep.c
@@ -663,7 +663,8 @@ i386_linux_core_read_xsave_info (bfd *abfd, x86_xsave_layout &layout)
 
   uint64_t xcr0 = bfd_get_64 (abfd, contents);
 
-  if (!i387_guess_xsave_layout (xcr0, size, layout))
+  if (!i387_read_xsave_layout_from_core (abfd, xcr0, size, layout)
+      && !i387_guess_xsave_layout (xcr0, size, layout))
     return 0;
 
   return xcr0;
-- 
2.41.0

[RFC 05/13] nat/x86-cpuid.h: Remove non-x86 fallbacks

[John Baldwin]

This header is only suitable for use on x86 hosts and is only included
there, so these fallbacks should not be needed.
---
 gdb/nat/x86-cpuid.h | 22 ----------------------
 1 file changed, 22 deletions(-)

diff --git a/gdb/nat/x86-cpuid.h b/gdb/nat/x86-cpuid.h
index e1b0321d593..9401705c44d 100644
--- a/gdb/nat/x86-cpuid.h
+++ b/gdb/nat/x86-cpuid.h
@@ -28,8 +28,6 @@
 #define nullptr ((void *) 0)
 #endif
 
-#if defined(__i386__) || defined(__x86_64__)
-
 /* Return cpuid data for requested cpuid level, as found in returned
    eax, ebx, ecx and edx registers.  The function checks if cpuid is
    supported and returns 1 for valid cpuid information or 0 for
@@ -78,26 +76,6 @@ x86_cpuid_count (unsigned int __level, unsigned int __sublevel,
   return __get_cpuid_count (__level, __sublevel, __eax, __ebx, __ecx, __edx);
 }
 
-#else
-
-static __inline int
-x86_cpuid (unsigned int __level,
-	    unsigned int *__eax, unsigned int *__ebx,
-	    unsigned int *__ecx, unsigned int *__edx)
-{
-  return 0;
-}
-
-static __inline int
-x86_cpuid_count (unsigned int __level, unsigned int __sublevel,
-		 unsigned int *__eax, unsigned int *__ebx,
-		 unsigned int *__ecx, unsigned int *__edx)
-{
-  return 0;
-}
-
-#endif /* i386 && x86_64 */
-
 #ifndef __cplusplus
 /* Avoid leaking this local definition beyond the scope of this header
    file.  */
-- 
2.41.0

[RFC 06/13] nat/x86-cpuid: Add a function to build the contents of a NT_X86_CPUID note

[John Baldwin]

---
 gdb/nat/x86-cpuid.c | 91 +++++++++++++++++++++++++++++++++++++++++++++
 gdb/nat/x86-cpuid.h |  7 ++++
 2 files changed, 98 insertions(+)
 create mode 100644 gdb/nat/x86-cpuid.c

diff --git a/gdb/nat/x86-cpuid.c b/gdb/nat/x86-cpuid.c
new file mode 100644
index 00000000000..a0fc9c2b192
--- /dev/null
+++ b/gdb/nat/x86-cpuid.c
@@ -0,0 +1,91 @@
+/* x86 CPUID functions.
+
+   Copyright (C) 2023 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include "gdbsupport/common-defs.h"
+#include "nat/x86-cpuid.h"
+
+/* Data stored in the note for a single CPUID leaf.  */
+
+struct cpuid_leaf
+{
+  uint32_t leaf;
+  uint32_t subleaf;
+  uint32_t eax;
+  uint32_t ebx;
+  uint32_t ecx;
+  uint32_t edx;
+};
+
+/* Append a single CPUID leaf.  */
+
+static void
+append_cpuid_note (gdb::unique_xmalloc_ptr<gdb_byte> &buf, size_t &len,
+		   uint32_t leaf, uint32_t subleaf)
+{
+  struct cpuid_leaf data;
+  if (!x86_cpuid_count (leaf, subleaf, &data.eax, &data.ebx, &data.ecx,
+			&data.edx))
+    return;
+
+  data.leaf = leaf;
+  data.subleaf = subleaf;
+
+  buf.reset ((gdb_byte *) xrealloc (buf.release (), len + sizeof (data)));
+  memcpy (buf.get() + len, &data, sizeof (data));
+  len += sizeof (data);
+}
+
+/* See x86-cpuid.h.  */
+
+void
+x86_cpuid_note (gdb::unique_xmalloc_ptr<gdb_byte> &buf, size_t &len)
+{
+  buf.reset (nullptr);
+  len = 0;
+
+  /* Include 0xd sub-leaves that describe the XSAVE extended state.  */
+  uint32_t eax, edx;
+  if (x86_cpuid_count (0xd, 0, &eax, nullptr, nullptr, &edx)
+      && (eax != 0 || edx != 0))
+    {
+      /* Main leaf and sub-leaf 1. */
+      append_cpuid_note (buf, len, 0xd, 0);
+      append_cpuid_note (buf, len, 0xd, 1);
+
+      /* Sub-leaves for each enabled feature.  */
+      eax >>= 2;
+      uint32_t i = 2;
+      while (eax != 0)
+	{
+	  if ((eax & 1) == 1)
+	    append_cpuid_note (buf, len, 0xd, i);
+	  eax >>= 1;
+	  i++;
+	}
+
+      i = 0;
+      while (edx != 0)
+	{
+	  if ((edx & 1) == 1)
+	    append_cpuid_note (buf, len, 0xd, i + 32);
+	  edx >>= 1;
+	  i++;
+	}
+    }
+}
diff --git a/gdb/nat/x86-cpuid.h b/gdb/nat/x86-cpuid.h
index 9401705c44d..6b0561dfcf4 100644
--- a/gdb/nat/x86-cpuid.h
+++ b/gdb/nat/x86-cpuid.h
@@ -82,4 +82,11 @@ x86_cpuid_count (unsigned int __level, unsigned int __sublevel,
 #undef nullptr
 #endif
 
+#ifdef __cplusplus
+/* Store data for the NT_X86_CPUID core dump note in BUF for the current
+   host.  The size of the data is stored in LEN.  */
+
+void x86_cpuid_note (gdb::unique_xmalloc_ptr<gdb_byte> &buf, size_t &len);
+#endif
+
 #endif /* NAT_X86_CPUID_H */
-- 
2.41.0

[RFC 07/13] x86_elf_make_cpuid_note: Helper routine to build NT_X86_CPUID ELF note

[John Baldwin]

Fetch the CPUID data from the current target via a new
TARGET_OBJECT_X86_CPUID and write it to an output bfd for a core dump.
---
 gdb/target.h   |  2 ++
 gdb/x86-tdep.c | 22 ++++++++++++++++++++++
 gdb/x86-tdep.h |  9 +++++++++
 3 files changed, 33 insertions(+)

diff --git a/gdb/target.h b/gdb/target.h
index 936ae79219c..c43cddde57c 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -210,6 +210,8 @@ enum target_object
   TARGET_OBJECT_FREEBSD_VMMAP,
   /* FreeBSD process strings.  */
   TARGET_OBJECT_FREEBSD_PS_STRINGS,
+  /* x86 CPUID leaves stored in NT_X86_CPUID core dump note.  */
+  TARGET_OBJECT_X86_CPUID,
   /* Possible future objects: TARGET_OBJECT_FILE, ...  */
 };
 
diff --git a/gdb/x86-tdep.c b/gdb/x86-tdep.c
index 6a73f2177c9..b36a86d8397 100644
--- a/gdb/x86-tdep.c
+++ b/gdb/x86-tdep.c
@@ -18,8 +18,11 @@
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
 #include "defs.h"
+#include "elf-bfd.h"
+#include "inferior.h"
 #include "x86-tdep.h"
 #include "symtab.h"
+#include "target.h"
 
 
 /* Check whether NAME is included in NAMES[LO] (inclusive) to NAMES[HI]
@@ -75,3 +78,22 @@ x86_in_indirect_branch_thunk (CORE_ADDR pc, const char * const *register_names,
 
   return false;
 }
+
+/* See x86-tdep.h.  */
+
+void
+x86_elf_make_cpuid_note (bfd *obfd, gdb::unique_xmalloc_ptr<char> *note_data,
+			 int *note_size)
+{
+  gdb::optional<gdb::byte_vector> buf =
+    target_read_alloc (current_inferior ()->top_target (),
+		       TARGET_OBJECT_X86_CPUID, nullptr);
+  if (!buf || buf->empty ())
+    return;
+
+  note_data->reset (elfcore_write_register_note (obfd,
+						 note_data->release (),
+						 note_size,
+						 ".reg-x86-cpuid",
+						 buf->data (), buf->size ()));
+}
diff --git a/gdb/x86-tdep.h b/gdb/x86-tdep.h
index 7840ceda57d..6f210de7ad9 100644
--- a/gdb/x86-tdep.h
+++ b/gdb/x86-tdep.h
@@ -27,4 +27,13 @@ extern bool x86_in_indirect_branch_thunk (CORE_ADDR pc,
 					  const char * const *register_names,
 					  int lo, int hi);
 
+/* Add content to *NOTE_DATA (and update *NOTE_SIZE) to include a note
+   containing CPUID leaves for the current target.  The core file is
+   being written to OBFD.  If something goes wrong then *NOTE_DATA can
+   be set to nullptr.  */
+
+extern void x86_elf_make_cpuid_note (bfd *obfd,
+				     gdb::unique_xmalloc_ptr<char> *note_data,
+				     int *note_size);
+
 #endif /* x86-tdep.h */
-- 
2.41.0

[RFC 08/13] x86-fbsd-nat: Support fetching TARGET_OBJECT_X86_CPUID objects

[John Baldwin]

When probing for the cached XSAVE layout, fetch the CPUID note data
and cache it until needed for a future fetch via ::xfer_partial.
---
 gdb/configure.nat  |  6 ++++--
 gdb/x86-fbsd-nat.c | 37 +++++++++++++++++++++++++++++++++++++
 gdb/x86-fbsd-nat.h |  9 +++++++++
 3 files changed, 50 insertions(+), 2 deletions(-)

diff --git a/gdb/configure.nat b/gdb/configure.nat
index 1dc4206b69c..4ed71d8619d 100644
--- a/gdb/configure.nat
+++ b/gdb/configure.nat
@@ -165,7 +165,8 @@ case ${gdb_host} in
 		;;
 	    i386)
 		# Host: FreeBSD/i386
-		NATDEPFILES="${NATDEPFILES} x86-nat.o nat/x86-dregs.o \
+		NATDEPFILES="${NATDEPFILES} x86-nat.o nat/x86-cpuid.o \
+		nat/x86-dregs.o
 		nat/x86-xstate.o x86-bsd-nat.o x86-fbsd-nat.o i386-fbsd-nat.o \
 		bsd-kvm.o"
 		;;
@@ -195,7 +196,8 @@ case ${gdb_host} in
 	    i386)
 		# Host: FreeBSD/amd64
 		NATDEPFILES="${NATDEPFILES} amd64-nat.o \
-		amd64-fbsd-nat.o bsd-kvm.o x86-nat.o nat/x86-dregs.o \
+		amd64-fbsd-nat.o bsd-kvm.o x86-nat.o nat/x86-cpuid.o \
+		nat/x86-dregs.o \
 		nat/x86-xstate.o x86-bsd-nat.o x86-fbsd-nat.o"
 		;;
 	esac
diff --git a/gdb/x86-fbsd-nat.c b/gdb/x86-fbsd-nat.c
index 240e228976c..14fd323a7fb 100644
--- a/gdb/x86-fbsd-nat.c
+++ b/gdb/x86-fbsd-nat.c
@@ -20,6 +20,7 @@
 #include "defs.h"
 #include "x86-fbsd-nat.h"
 #ifdef PT_GETXSTATE_INFO
+#include "nat/x86-cpuid.h"
 #include "nat/x86-xstate.h"
 #endif
 
@@ -48,6 +49,40 @@ x86_fbsd_nat_target::low_new_fork (ptid_t parent, pid_t child)
 }
 
 #ifdef PT_GETXSTATE_INFO
+enum target_xfer_status
+x86_fbsd_nat_target::xfer_partial (enum target_object object,
+				   const char *annex, gdb_byte *readbuf,
+				   const gdb_byte *writebuf,
+				   ULONGEST offset, ULONGEST len,
+				   ULONGEST *xfered_len)
+{
+  switch (object)
+    {
+    case TARGET_OBJECT_X86_CPUID:
+      if (readbuf)
+	{
+	  size_t size = m_cpuid_note_len;
+	  if (offset >= size)
+	    return TARGET_XFER_EOF;
+	  size -= offset;
+	  if (size > len)
+	    size = len;
+
+	  if (size == 0)
+	    return TARGET_XFER_EOF;
+
+	  memcpy (readbuf, m_cpuid_note.get () + offset, size);
+	  *xfered_len = size;
+	  return TARGET_XFER_OK;
+	}
+      return TARGET_XFER_E_IO;
+    default:
+      return fbsd_nat_target::xfer_partial (object, annex, readbuf,
+					    writebuf, offset, len,
+					    xfered_len);
+    }
+}
+
 void
 x86_fbsd_nat_target::probe_xsave_layout (pid_t pid)
 {
@@ -56,6 +91,8 @@ x86_fbsd_nat_target::probe_xsave_layout (pid_t pid)
 
   m_xsave_probed = true;
 
+  x86_cpuid_note (m_cpuid_note, m_cpuid_note_len);
+
   if (ptrace (PT_GETXSTATE_INFO, pid, (PTRACE_TYPE_ARG3) &m_xsave_info,
 	      sizeof (m_xsave_info)) != 0)
     return;
diff --git a/gdb/x86-fbsd-nat.h b/gdb/x86-fbsd-nat.h
index 723bb6cf305..71b6ad09916 100644
--- a/gdb/x86-fbsd-nat.h
+++ b/gdb/x86-fbsd-nat.h
@@ -42,6 +42,13 @@ class x86_fbsd_nat_target : public x86bsd_nat_target<fbsd_nat_target>
   x86_xsave_layout fetch_x86_xsave_layout () override
   { return m_xsave_layout; }
 
+  enum target_xfer_status xfer_partial (enum target_object object,
+					const char *annex,
+					gdb_byte *readbuf,
+					const gdb_byte *writebuf,
+					ULONGEST offset, ULONGEST len,
+					ULONGEST *xfered_len) override;
+
 protected:
   void probe_xsave_layout (pid_t pid);
 
@@ -49,6 +56,8 @@ class x86_fbsd_nat_target : public x86bsd_nat_target<fbsd_nat_target>
   x86_xsave_layout m_xsave_layout;
 
 private:
+  gdb::unique_xmalloc_ptr<gdb_byte> m_cpuid_note;
+  size_t m_cpuid_note_len;
   bool m_xsave_probed;
 #endif
 };
-- 
2.41.0

[RFC 09/13] fbsd-tdep: Export fbsd_make_corefile_notes

[John Baldwin]

This permits FreeBSD architectures to override the gdbarch
make_corefile_notes method to add architecture-specific notes.
---
 gdb/fbsd-tdep.c | 5 ++---
 gdb/fbsd-tdep.h | 7 +++++++
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/gdb/fbsd-tdep.c b/gdb/fbsd-tdep.c
index d166d785736..ef7ea7cf8c3 100644
--- a/gdb/fbsd-tdep.c
+++ b/gdb/fbsd-tdep.c
@@ -667,10 +667,9 @@ fbsd_make_note_desc (enum target_object object, uint32_t structsize)
   return desc;
 }
 
-/* Create appropriate note sections for a corefile, returning them in
-   allocated memory.  */
+/* See fbsd-tdep.h.  */
 
-static gdb::unique_xmalloc_ptr<char>
+gdb::unique_xmalloc_ptr<char>
 fbsd_make_corefile_notes (struct gdbarch *gdbarch, bfd *obfd, int *note_size)
 {
   gdb::unique_xmalloc_ptr<char> note_data;
diff --git a/gdb/fbsd-tdep.h b/gdb/fbsd-tdep.h
index 65541a01a01..b457925c8a3 100644
--- a/gdb/fbsd-tdep.h
+++ b/gdb/fbsd-tdep.h
@@ -76,4 +76,11 @@ extern CORE_ADDR fbsd_get_thread_local_address (struct gdbarch *gdbarch,
 extern CORE_ADDR fbsd_skip_solib_resolver (struct gdbarch *gdbarch,
 					   CORE_ADDR pc);
 
+
+/* Create appropriate note sections for a corefile, returning them in
+   allocated memory.  */
+
+extern gdb::unique_xmalloc_ptr<char> fbsd_make_corefile_notes
+(struct gdbarch *gdbarch, bfd *obfd, int *note_size);
+
 #endif /* fbsd-tdep.h */
-- 
2.41.0

[RFC 10/13] {amd64,i386}-fbsd-tdep: Include NT_X86_CPUID notes in core dumps from gcore

[John Baldwin]

Override the gdbarch make_corefile_notes method for the FreeBSD x86
arches with a new function that calls fbsd_make_corefile_notes and
x86_elf_make_cpuid_note to generate the core dump notes.
---
 gdb/amd64-fbsd-tdep.c |  1 +
 gdb/i386-fbsd-tdep.c  | 15 +++++++++++++++
 gdb/i386-fbsd-tdep.h  |  7 +++++++
 3 files changed, 23 insertions(+)

diff --git a/gdb/amd64-fbsd-tdep.c b/gdb/amd64-fbsd-tdep.c
index 2b633cd479c..30cba8f6e6a 100644
--- a/gdb/amd64-fbsd-tdep.c
+++ b/gdb/amd64-fbsd-tdep.c
@@ -329,6 +329,7 @@ amd64fbsd_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
 
   set_gdbarch_core_read_description (gdbarch,
 				     amd64fbsd_core_read_description);
+  set_gdbarch_make_corefile_notes (gdbarch, i386_fbsd_make_corefile_notes);
 
   /* FreeBSD uses SVR4-style shared libraries.  */
   set_solib_svr4_fetch_link_map_offsets
diff --git a/gdb/i386-fbsd-tdep.c b/gdb/i386-fbsd-tdep.c
index 1789f3921fd..810ecc90df1 100644
--- a/gdb/i386-fbsd-tdep.c
+++ b/gdb/i386-fbsd-tdep.c
@@ -26,6 +26,7 @@
 #include "tramp-frame.h"
 #include "i386-fbsd-tdep.h"
 
+#include "x86-tdep.h"
 #include "i386-tdep.h"
 #include "i387-tdep.h"
 #include "fbsd-tdep.h"
@@ -370,6 +371,19 @@ i386fbsd_get_thread_local_address (struct gdbarch *gdbarch, ptid_t ptid,
   return fbsd_get_thread_local_address (gdbarch, dtv_addr, lm_addr, offset);
 }
 
+/* See i386-fbsd-tdep.h.  */
+
+gdb::unique_xmalloc_ptr<char>
+i386_fbsd_make_corefile_notes (struct gdbarch *gdbarch, bfd *obfd,
+			       int *note_size)
+{
+  gdb::unique_xmalloc_ptr<char> note_data =
+    fbsd_make_corefile_notes (gdbarch, obfd, note_size);
+
+  x86_elf_make_cpuid_note (obfd, &note_data, note_size);
+  return note_data;
+}
+
 static void
 i386fbsd_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
 {
@@ -403,6 +417,7 @@ i386fbsd_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
 
   set_gdbarch_core_read_description (gdbarch,
 				     i386fbsd_core_read_description);
+  set_gdbarch_make_corefile_notes (gdbarch, i386_fbsd_make_corefile_notes);
 
   /* FreeBSD uses SVR4-style shared libraries.  */
   set_solib_svr4_fetch_link_map_offsets
diff --git a/gdb/i386-fbsd-tdep.h b/gdb/i386-fbsd-tdep.h
index c49cb1eba68..fc7bb1c521d 100644
--- a/gdb/i386-fbsd-tdep.h
+++ b/gdb/i386-fbsd-tdep.h
@@ -40,6 +40,13 @@ bool i386_fbsd_core_read_x86_xsave_layout (struct gdbarch *gdbarch,
    matches the layout on Linux.  */
 #define I386_FBSD_XSAVE_XCR0_OFFSET 464
 
+/* Create appropriate note sections for a corefile, returning them in
+   allocated memory.  Extends fbsd_make_corefile_notes to add a
+   NT_X86_CPUID note.  */
+
+gdb::unique_xmalloc_ptr<char> i386_fbsd_make_corefile_notes
+(struct gdbarch *gdbarch, bfd *obfd, int *note_size);
+
 extern const struct regset i386_fbsd_gregset;
 
 #endif /* i386-fbsd-tdep.h */
-- 
2.41.0

[RFC 11/13] x86-linux-nat: Support fetching TARGET_OBJECT_X86_CPUID objects

[John Baldwin]

When probing for the cached XSAVE layout, fetch the CPUID note data
and cache it until needed for a future fetch via ::xfer_partial.
---
 gdb/configure.nat   |  7 ++++---
 gdb/x86-linux-nat.c | 37 +++++++++++++++++++++++++++++++++++++
 gdb/x86-linux-nat.h |  9 +++++++++
 3 files changed, 50 insertions(+), 3 deletions(-)

diff --git a/gdb/configure.nat b/gdb/configure.nat
index 4ed71d8619d..c540e070dbb 100644
--- a/gdb/configure.nat
+++ b/gdb/configure.nat
@@ -255,8 +255,8 @@ case ${gdb_host} in
 		;;
 	    i386)
 		# Host: Intel 386 running GNU/Linux.
-		NATDEPFILES="${NATDEPFILES} x86-nat.o nat/x86-dregs.o \
-		nat/x86-xstate.o \
+		NATDEPFILES="${NATDEPFILES} x86-nat.o nat/x86-cpuid.o \
+		nat/x86-dregs.o nat/x86-xstate.o \
 		i386-linux-nat.o x86-linux-nat.o nat/linux-btrace.o \
 		nat/x86-linux.o nat/x86-linux-dregs.o"
 		;;
@@ -321,7 +321,8 @@ case ${gdb_host} in
 	case ${gdb_host_cpu} in
 	    i386)
 		# Host: GNU/Linux x86-64
-		NATDEPFILES="${NATDEPFILES} x86-nat.o nat/x86-dregs.o \
+		NATDEPFILES="${NATDEPFILES} x86-nat.o nat/x86-cpuid.o \
+		nat/x86-dregs.o \
 		nat/x86-xstate.o amd64-nat.o amd64-linux-nat.o x86-linux-nat.o \
 		nat/linux-btrace.o \
 		nat/x86-linux.o nat/x86-linux-dregs.o \
diff --git a/gdb/x86-linux-nat.c b/gdb/x86-linux-nat.c
index cd8fd9c1dcd..517faccf934 100644
--- a/gdb/x86-linux-nat.c
+++ b/gdb/x86-linux-nat.c
@@ -36,6 +36,7 @@
 #include "amd64-linux-tdep.h"
 #endif
 #include "gdbsupport/x86-xstate.h"
+#include "nat/x86-cpuid.h"
 #include "nat/x86-xstate.h"
 #include "nat/linux-btrace.h"
 #include "nat/linux-nat.h"
@@ -182,6 +183,8 @@ x86_linux_nat_target::read_description ()
 			     / sizeof (uint64_t))];
 
 	  m_xsave_layout = x86_fetch_xsave_layout (xcr0, x86_xsave_length ());
+
+	  x86_cpuid_note (m_cpuid_note, m_cpuid_note_len);
 	}
     }
 
@@ -213,6 +216,40 @@ x86_linux_nat_target::read_description ()
 
   gdb_assert_not_reached ("failed to return tdesc");
 }
+
+enum target_xfer_status
+x86_linux_nat_target::xfer_partial (enum target_object object,
+				    const char *annex, gdb_byte *readbuf,
+				    const gdb_byte *writebuf,
+				    ULONGEST offset, ULONGEST len,
+				    ULONGEST *xfered_len)
+{
+  switch (object)
+    {
+    case TARGET_OBJECT_X86_CPUID:
+      if (readbuf)
+	{
+	  size_t size = m_cpuid_note_len;
+	  if (offset >= size)
+	    return TARGET_XFER_EOF;
+	  size -= offset;
+	  if (size > len)
+	    size = len;
+
+	  if (size == 0)
+	    return TARGET_XFER_EOF;
+
+	  memcpy (readbuf, m_cpuid_note.get () + offset, size);
+	  *xfered_len = size;
+	  return TARGET_XFER_OK;
+	}
+      return TARGET_XFER_E_IO;
+    default:
+      return linux_nat_target::xfer_partial (object, annex, readbuf,
+					     writebuf, offset, len,
+					     xfered_len);
+    }
+}
 \f
 
 /* Enable branch tracing.  */
diff --git a/gdb/x86-linux-nat.h b/gdb/x86-linux-nat.h
index a0f8ffc993e..c2a6452ce27 100644
--- a/gdb/x86-linux-nat.h
+++ b/gdb/x86-linux-nat.h
@@ -45,6 +45,13 @@ struct x86_linux_nat_target : public x86_nat_target<linux_nat_target>
   x86_xsave_layout fetch_x86_xsave_layout () override
   { return m_xsave_layout; }
 
+  enum target_xfer_status xfer_partial (enum target_object object,
+					const char *annex,
+					gdb_byte *readbuf,
+					const gdb_byte *writebuf,
+					ULONGEST offset, ULONGEST len,
+					ULONGEST *xfered_len) override;
+
   /* These two are rewired to low_ versions.  linux-nat.c queries
      stopped-by-watchpoint info as soon as an lwp stops (via the low_
      methods) and caches the result, to be returned via the normal
@@ -81,6 +88,8 @@ struct x86_linux_nat_target : public x86_nat_target<linux_nat_target>
 
 private:
   x86_xsave_layout m_xsave_layout;
+  gdb::unique_xmalloc_ptr<gdb_byte> m_cpuid_note;
+  size_t m_cpuid_note_len;
 };
 
 \f
-- 
2.41.0

[RFC 12/13] linux-tdep: Export linux_make_corefile_notes

[John Baldwin]

This permits Linux architectures to override the gdbarch
make_corefile_notes method to add architecture-specific notes.
---
 gdb/linux-tdep.c | 5 ++---
 gdb/linux-tdep.h | 7 +++++++
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c
index f03a5b95332..1c9c09dbd71 100644
--- a/gdb/linux-tdep.c
+++ b/gdb/linux-tdep.c
@@ -2033,10 +2033,9 @@ linux_fill_prpsinfo (struct elf_internal_linux_prpsinfo *p)
   return 1;
 }
 
-/* Build the note section for a corefile, and return it in a malloc
-   buffer.  */
+/* See linux-tdep.h.  */
 
-static gdb::unique_xmalloc_ptr<char>
+gdb::unique_xmalloc_ptr<char>
 linux_make_corefile_notes (struct gdbarch *gdbarch, bfd *obfd, int *note_size)
 {
   struct elf_internal_linux_prpsinfo prpsinfo;
diff --git a/gdb/linux-tdep.h b/gdb/linux-tdep.h
index e09a6ef32b1..20807031e44 100644
--- a/gdb/linux-tdep.h
+++ b/gdb/linux-tdep.h
@@ -52,6 +52,13 @@ typedef char *(*linux_collect_thread_registers_ftype) (const struct regcache *,
 						       bfd *, char *, int *,
 						       enum gdb_signal);
 
+/* Build the note section for a corefile, and return it in a malloc
+   buffer.  */
+
+gdb::unique_xmalloc_ptr<char> linux_make_corefile_notes (struct gdbarch *gdbarch,
+							 bfd *obfd,
+							 int *note_size);
+
 extern enum gdb_signal linux_gdb_signal_from_target (struct gdbarch *gdbarch,
 						     int signal);
 
-- 
2.41.0

[RFC 13/13] {amd64,i386}-linux-tdep: Include NT_X86_CPUID notes in core dumps from gcore

[John Baldwin]

Override the gdbarch make_corefile_notes method for the Linux x86
arches with a new function that calls linux_make_corefile_notes and
x86_elf_make_cpuid_note to generate the core dump notes.
---
 gdb/amd64-linux-tdep.c |  1 +
 gdb/i386-linux-tdep.c  | 15 +++++++++++++++
 gdb/i386-linux-tdep.h  |  7 +++++++
 3 files changed, 23 insertions(+)

diff --git a/gdb/amd64-linux-tdep.c b/gdb/amd64-linux-tdep.c
index e2fdf5fb6c8..11291a606b6 100644
--- a/gdb/amd64-linux-tdep.c
+++ b/gdb/amd64-linux-tdep.c
@@ -1838,6 +1838,7 @@ amd64_linux_init_abi_common(struct gdbarch_info info, struct gdbarch *gdbarch,
 
   set_gdbarch_core_read_description (gdbarch,
 				     amd64_linux_core_read_description);
+  set_gdbarch_make_corefile_notes (gdbarch, i386_linux_make_corefile_notes);
 
   /* Displaced stepping.  */
   set_gdbarch_displaced_step_copy_insn (gdbarch,
diff --git a/gdb/i386-linux-tdep.c b/gdb/i386-linux-tdep.c
index 57d00a424d9..6573fcd9ead 100644
--- a/gdb/i386-linux-tdep.c
+++ b/gdb/i386-linux-tdep.c
@@ -38,6 +38,7 @@
 #include "xml-syscall.h"
 #include "infrun.h"
 
+#include "x86-tdep.h"
 #include "i387-tdep.h"
 #include "gdbsupport/x86-xstate.h"
 
@@ -827,6 +828,19 @@ i386_linux_displaced_step_copy_insn (struct gdbarch *gdbarch,
   return closure_;
 }
 
+/* See i386-linux-tdep.h.  */
+
+gdb::unique_xmalloc_ptr<char>
+i386_linux_make_corefile_notes (struct gdbarch *gdbarch, bfd *obfd,
+			       int *note_size)
+{
+  gdb::unique_xmalloc_ptr<char> note_data =
+    linux_make_corefile_notes (gdbarch, obfd, note_size);
+
+  x86_elf_make_cpuid_note (obfd, &note_data, note_size);
+  return note_data;
+}
+
 static void
 i386_linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
 {
@@ -1068,6 +1082,7 @@ i386_linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
     (gdbarch, i386_linux_iterate_over_regset_sections);
   set_gdbarch_core_read_description (gdbarch,
 				     i386_linux_core_read_description);
+  set_gdbarch_make_corefile_notes (gdbarch, i386_linux_make_corefile_notes);
 
   /* Displaced stepping.  */
   set_gdbarch_displaced_step_copy_insn (gdbarch,
diff --git a/gdb/i386-linux-tdep.h b/gdb/i386-linux-tdep.h
index bd8e328b7fe..86ea96edd67 100644
--- a/gdb/i386-linux-tdep.h
+++ b/gdb/i386-linux-tdep.h
@@ -58,6 +58,13 @@ extern void i386_linux_report_signal_info (struct gdbarch *gdbarch,
 /* Return the target description according to XCR0.  */
 extern const struct target_desc *i386_linux_read_description (uint64_t xcr0);
 
+/* Create appropriate note sections for a corefile, returning them in
+   allocated memory.  Extends linux_make_corefile_notes to add a
+   NT_X86_CPUID note.  */
+
+extern gdb::unique_xmalloc_ptr<char> i386_linux_make_corefile_notes
+(struct gdbarch *gdbarch, bfd *obfd, int *note_size);
+
 /* Format of XSAVE extended state is:
 	struct
 	{
-- 
2.41.0

RE: [RFC 00/13] Proposal for a new NT_X86_CPUID core dump note

[George, Jini Susan]

[Public]

Thanks for doing this, John. I am yet to go through these patches, but I wanted to let you know that for the Linux kernel, we have a patch for creating the core dump .note section which is being reviewed internally at this point. The patch follows the same structure layout proposed by you here. We would cross check to confirm that the proposed Linux patch modifications are in line with your FreeBSD changes.

Regards,
Jini.

>>-----Original Message-----
>>From: John Baldwin <jhb@FreeBSD.org>
>>Sent: Tuesday, October 10, 2023 12:06 AM
>>To: gdb-patches@sourceware.org
>>Cc: Willgerodt; Felix <felix.willgerodt@intel.com>; George; George, Jini Susan
>><JiniSusan.George@amd.com>; Simon Marchi <simon.marchi@polymtl.ca>
>>Subject: [RFC 00/13] Proposal for a new NT_X86_CPUID core dump note
>>
>>Caution: This message originated from an External Source. Use proper caution
>>when opening attachments, clicking links, or responding.
>>
>>
>>One of the shortcomings of the previous XSAVE patch series is that it depends on
>>heuristics based on the total XSAVE register set size and
>>XCR0 mask to infer layouts of the various register blocks for core dumps.  This
>>series introduces a new x86-specific core dump note intended to supplant these
>>heuristics by storing the raw CPUID leaves describing the XSAVE layout in core
>>dumps.
>>
>>This series proposes a new core dump note, NT_X86_CPUID (0x205), which
>>contains an array of structures.  Each structure describes an invidual CPUID sub-
>>leaf containing both the inputs to CPUID (%eax and %ecx) and the outputs
>>(%eax, %ebx, %ecx, and %edx) in a format roughly matching the follow C
>>structure:
>>
>>struct cpuid_leaf
>>{
>>    uint32_t leaf;
>>    uint32_t subleaf;
>>    uint32_t eax;
>>    uint32_t ebx;
>>    uint32_t ecx;
>>    uint32_t edx;
>>};
>>
>>This format is not XSAVE-specific and implementations could choose to add
>>additional CPUID leaves to this structure if needed in the future.
>>Consumers of this note should lookup the value of required leaves and ignore
>>any unneeded leaves.
>>
>>An alternate approach might be to write out a more XSAVE-specific note that is
>>an array containing the offset and size of each XSAVE region.
>>
>>Note that either approach would enable storing XSAVE notes in the "compact"
>>format at some point in the future.
>>
>>This series adds support for reading/writing the note to binutils as well as suport
>>for parsing and generating the note in GDB.  It also hooks this into both the
>>FreeBSD and Linux x86 architectures in GDB to read the XSAVE layout from this
>>note when present, and to write out a note when generating a core via `gcore'.
>>I've done some limited testing on FreeBSD/amd64 and Linux/x86-64, but it could
>>probably use some more testing on Linux in particular.  (I know Simon has an
>>AMD machine with a layout not handled by the current heuristics for
>>example.)
>>
>>For the gcore side, a new TARGET_OBJECT_X86_CPUID is used to fetch the
>>current note contents from a native target.  There is still one gap even with this
>>patch series which is that if you are connected to a remote target (e.g.
>>gdbserver), we currently do not have a known XSAVE layout to use when writing
>>out a core via `gcore'.  One option that would close this gap would be to extend
>>the remote protocol to permit reading this new object from a debug server.  The
>>remote target could then implement fetching this object and also make use of
>>this object to implement the target::fetch_x86_xsave_layout method which
>>would close that gap.  Another possibility would be to just pick a "known"
>>XSAVE format that matches one of the heuristics.
>>
>>The series is available from git@github.com:bsdjhb/gdb.git on the
>>`nt_x86_cpuid' branch.
>>
>>I also have an implementation of this core dump note available for FreeBSD's
>>kernel, though I won't merge it until we've collectively settled on the format:
>>https://reviews.freebsd.org/D42136
>>
>>Things I have not done and could use help with:
>>
>>- Implementation for the Linux kernel
>>
>>- Coordination with folks from LLDB
>>
>>John Baldwin (13):
>>  binutils: Support for the NT_X86_CPUID core dump note
>>  i387-tdep: Add function to read XSAVE layout from NT_X86_CPUID
>>  gdb: Use NT_X86_CPUID in x86 FreeBSD architectures to read XSAVE
>>    layouts
>>  gdb: Use NT_X86_CPUID in x86 FreeBSD architectures to read XSAVE
>>    layouts
>>  nat/x86-cpuid.h: Remove non-x86 fallbacks
>>  nat/x86-cpuid: Add a function to build the contents of a NT_X86_CPUID
>>    note
>>  x86_elf_make_cpuid_note: Helper routine to build NT_X86_CPUID ELF note
>>  x86-fbsd-nat: Support fetching TARGET_OBJECT_X86_CPUID objects
>>  fbsd-tdep: Export fbsd_make_corefile_notes
>>  {amd64,i386}-fbsd-tdep: Include NT_X86_CPUID notes in core dumps from
>>    gcore
>>  x86-linux-nat: Support fetching TARGET_OBJECT_X86_CPUID objects
>>  linux-tdep: Export linux_make_corefile_notes
>>  {amd64,i386}-linux-tdep: Include NT_X86_CPUID notes in core dumps from
>>    gcore
>>
>> bfd/elf-bfd.h          |   2 +
>> bfd/elf.c              |  35 +++++++++++
>> binutils/readelf.c     |   2 +
>> gdb/amd64-fbsd-tdep.c  |   1 +
>> gdb/amd64-linux-tdep.c |   1 +
>> gdb/configure.nat      |  13 ++--
>> gdb/fbsd-tdep.c        |   5 +-
>> gdb/fbsd-tdep.h        |   7 +++
>> gdb/i386-fbsd-tdep.c   |  18 +++++-
>> gdb/i386-fbsd-tdep.h   |   7 +++
>> gdb/i386-linux-tdep.c  |  18 +++++-
>> gdb/i386-linux-tdep.h  |   7 +++
>> gdb/i387-tdep.c        | 132 +++++++++++++++++++++++++++++++++++++++++
>> gdb/i387-tdep.h        |   8 +++
>> gdb/linux-tdep.c       |   5 +-
>> gdb/linux-tdep.h       |   7 +++
>> gdb/nat/x86-cpuid.c    |  91 ++++++++++++++++++++++++++++
>> gdb/nat/x86-cpuid.h    |  29 +++------
>> gdb/target.h           |   2 +
>> gdb/x86-fbsd-nat.c     |  37 ++++++++++++
>> gdb/x86-fbsd-nat.h     |   9 +++
>> gdb/x86-linux-nat.c    |  37 ++++++++++++
>> gdb/x86-linux-nat.h    |   9 +++
>> gdb/x86-tdep.c         |  22 +++++++
>> gdb/x86-tdep.h         |   9 +++
>> include/elf/common.h   |   2 +
>> 26 files changed, 480 insertions(+), 35 deletions(-)  create mode 100644
>>gdb/nat/x86-cpuid.c
>>
>>--
>>2.41.0

Re: [RFC 00/13] Proposal for a new NT_X86_CPUID core dump note

[Simon Marchi]

On 2023-10-09 14:36, John Baldwin wrote:
> One of the shortcomings of the previous XSAVE patch series is that it
> depends on heuristics based on the total XSAVE register set size and
> XCR0 mask to infer layouts of the various register blocks for core
> dumps.  This series introduces a new x86-specific core dump note
> intended to supplant these heuristics by storing the raw CPUID leaves
> describing the XSAVE layout in core dumps.
> 
> This series proposes a new core dump note, NT_X86_CPUID (0x205), which
> contains an array of structures.  Each structure describes an invidual
> CPUID sub-leaf containing both the inputs to CPUID (%eax and %ecx) and
> the outputs (%eax, %ebx, %ecx, and %edx) in a format roughly matching
> the follow C structure:
> 
> struct cpuid_leaf
> {
>     uint32_t leaf;
>     uint32_t subleaf;
>     uint32_t eax;
>     uint32_t ebx;
>     uint32_t ecx;
>     uint32_t edx;
> };
> 
> This format is not XSAVE-specific and implementations could choose to
> add additional CPUID leaves to this structure if needed in the future.
> Consumers of this note should lookup the value of required leaves and
> ignore any unneeded leaves.
> 
> An alternate approach might be to write out a more XSAVE-specific note
> that is an array containing the offset and size of each XSAVE region.
> 
> Note that either approach would enable storing XSAVE notes in the
> "compact" format at some point in the future.
> 
> This series adds support for reading/writing the note to binutils as
> well as suport for parsing and generating the note in GDB.  It also
> hooks this into both the FreeBSD and Linux x86 architectures in GDB to
> read the XSAVE layout from this note when present, and to write out a
> note when generating a core via `gcore'.  I've done some limited
> testing on FreeBSD/amd64 and Linux/x86-64, but it could probably use
> some more testing on Linux in particular.  (I know Simon has an AMD
> machine with a layout not handled by the current heuristics for
> example.)

I suppose you are referring to this one, which has support for PKRU but
where PKRU is not enabled (for a reason I don't know), leading to the
total sizes being different (832 vs 896):

   XSAVE features (0xd/0):
      XCR0 valid bit field mask               = 0x0000000000000207
         x87 state                            = true
         SSE state                            = true
         AVX state                            = true
         MPX BNDREGS                          = false
         MPX BNDCSR                           = false
         AVX-512 opmask                       = false
         AVX-512 ZMM_Hi256                    = false
         AVX-512 Hi16_ZMM                     = false
         PKRU state                           = true
         XTILECFG state                       = false
         XTILEDATA state                      = false
      bytes required by fields in XCR0        = 0x00000340 (832)
      bytes required by XSAVE/XRSTOR area     = 0x00000380 (896)

That machine's layout is handled by the heuristic since commit:

  gdb/x86: use size of XSAVE area of enabled features
  https://gitlab.com/gnutools/binutils-gdb/-/commit/054f25955c2b77f6e21073bfdd70a60e9df1ffe7

But anyhow, I can use it for testing.

> For the gcore side, a new TARGET_OBJECT_X86_CPUID is used to fetch the
> current note contents from a native target.  There is still one gap
> even with this patch series which is that if you are connected to a
> remote target (e.g. gdbserver), we currently do not have a known XSAVE
> layout to use when writing out a core via `gcore'.  One option that
> would close this gap would be to extend the remote protocol to permit
> reading this new object from a debug server.  The remote target could
> then implement fetching this object and also make use of this object
> to implement the target::fetch_x86_xsave_layout method which would
> close that gap.  Another possibility would be to just pick a "known"
> XSAVE format that matches one of the heuristics.

I think it would make sense to send the CPUID info over the wire.  Can
it be extra data in the target description XML?  Otherwise, a new
qxfer packet I suppose.

If the current GDBserver doesn't support sending the CPUID info, I'm not
sure I would make the remote target write a CPUID note based on a
heuristic.  It has pros and cons, we can talk about that when we get
there.

Simon

Re: [RFC 02/13] i387-tdep: Add function to read XSAVE layout from NT_X86_CPUID

[Simon Marchi]

On 2023-10-09 14:36, John Baldwin wrote:
> This can be used by x86 arches to determine the XSAVE layout instead
> of guessing based on the XCR0 mask and XSAVE register note size.

Just some nits below:

> +typedef std::unordered_map<cpuid_key, cpuid_values> cpuid_map;

For new stuff I would suggest:

  using cpuid_map = std::unordered_map<cpuid_key, cpuid_values>;

> +
> +static cpuid_map
> +i387_parse_cpuid_from_core (bfd *bfd)
> +{
> +  asection *section = bfd_get_section_by_name (bfd, ".reg-x86-cpuid");
> +  if (section == nullptr)
> +    return {};
> +
> +  size_t size = bfd_section_size (section);
> +  if (size == 0 || (size % (6 * 4)) != 0)

That 4 could be `sizeof (uint32_t)`.  And `6 * 4` appears below again,
it could a constexpr value with a name like entry_size.

> +    return {};
> +
> +  char contents[size];
> +  if (!bfd_get_section_contents (bfd, section, contents, 0, size))
> +    {
> +      warning (_("Couldn't read `.reg-x86-cpuid' section in core file."));
> +      return {};
> +    }
> +
> +  cpuid_map map;
> +  size_t index = 0;
> +  while (index < size)
> +    {
> +      uint32_t leaf = bfd_get_32 (bfd, contents + index);
> +      uint32_t count = bfd_get_32 (bfd, contents + index + 4);
> +      uint32_t eax = bfd_get_32 (bfd, contents + index + 8);
> +      uint32_t ebx = bfd_get_32 (bfd, contents + index + 12);
> +      uint32_t ecx = bfd_get_32 (bfd, contents + index + 16);
> +      uint32_t edx = bfd_get_32 (bfd, contents + index + 20);
> +
> +      if (map.count (cpuid_key (leaf, count)) != 0)
> +	{
> +	  warning (_("Duplicate cpuid leaf %#x,%#x"), leaf, count);
> +	  return {};
> +	}
> +      map.emplace (cpuid_key (leaf, count),
> +		   cpuid_values (eax, ebx, ecx, edx));

This could be slightly more optimal using map::try_emplace (to avoid
having two map lookups).

Simon

Re: [RFC 04/13] gdb: Use NT_X86_CPUID in x86 FreeBSD architectures to read XSAVE layouts

[Simon Marchi]

On 2023-10-09 14:36, John Baldwin wrote:
> If this core dump note is present, use it to determine the layout of
> XSAVE register set notes.

s/FreeBSD/Linux in the subject of the commit.

Simon

Re: [RFC 05/13] nat/x86-cpuid.h: Remove non-x86 fallbacks

[Simon Marchi]

On 2023-10-09 14:36, John Baldwin wrote:
> This header is only suitable for use on x86 hosts and is only included
> there, so these fallbacks should not be needed.
> ---
>  gdb/nat/x86-cpuid.h | 22 ----------------------
>  1 file changed, 22 deletions(-)
> 
> diff --git a/gdb/nat/x86-cpuid.h b/gdb/nat/x86-cpuid.h
> index e1b0321d593..9401705c44d 100644
> --- a/gdb/nat/x86-cpuid.h
> +++ b/gdb/nat/x86-cpuid.h
> @@ -28,8 +28,6 @@
>  #define nullptr ((void *) 0)
>  #endif
>  
> -#if defined(__i386__) || defined(__x86_64__)
> -
>  /* Return cpuid data for requested cpuid level, as found in returned
>     eax, ebx, ecx and edx registers.  The function checks if cpuid is
>     supported and returns 1 for valid cpuid information or 0 for
> @@ -78,26 +76,6 @@ x86_cpuid_count (unsigned int __level, unsigned int __sublevel,
>    return __get_cpuid_count (__level, __sublevel, __eax, __ebx, __ecx, __edx);
>  }
>  
> -#else
> -
> -static __inline int
> -x86_cpuid (unsigned int __level,
> -	    unsigned int *__eax, unsigned int *__ebx,
> -	    unsigned int *__ecx, unsigned int *__edx)
> -{
> -  return 0;
> -}
> -
> -static __inline int
> -x86_cpuid_count (unsigned int __level, unsigned int __sublevel,
> -		 unsigned int *__eax, unsigned int *__ebx,
> -		 unsigned int *__ecx, unsigned int *__edx)
> -{
> -  return 0;
> -}
> -
> -#endif /* i386 && x86_64 */
> -
>  #ifndef __cplusplus
>  /* Avoid leaking this local definition beyond the scope of this header
>     file.  */

I think that one could go in right away.

Approved-By: Simon Marchi <simon.marchi@efficios.com>

Simon

Re: [RFC 06/13] nat/x86-cpuid: Add a function to build the contents of a NT_X86_CPUID note

[Simon Marchi]

On 2023-10-09 14:36, John Baldwin wrote:
> +/* See x86-cpuid.h.  */
> +
> +void
> +x86_cpuid_note (gdb::unique_xmalloc_ptr<gdb_byte> &buf, size_t &len)
> +{

I think it would be a bit simpler to make this return a gdb::byte_vector.

Simon

Re: [RFC 00/13] Proposal for a new NT_X86_CPUID core dump note

[Simon Marchi]

On 2023-10-09 14:36, John Baldwin wrote:
> One of the shortcomings of the previous XSAVE patch series is that it
> depends on heuristics based on the total XSAVE register set size and
> XCR0 mask to infer layouts of the various register blocks for core
> dumps.  This series introduces a new x86-specific core dump note
> intended to supplant these heuristics by storing the raw CPUID leaves
> describing the XSAVE layout in core dumps.
> 
> This series proposes a new core dump note, NT_X86_CPUID (0x205), which
> contains an array of structures.  Each structure describes an invidual
> CPUID sub-leaf containing both the inputs to CPUID (%eax and %ecx) and
> the outputs (%eax, %ebx, %ecx, and %edx) in a format roughly matching
> the follow C structure:
> 
> struct cpuid_leaf
> {
>     uint32_t leaf;
>     uint32_t subleaf;
>     uint32_t eax;
>     uint32_t ebx;
>     uint32_t ecx;
>     uint32_t edx;
> };
> 
> This format is not XSAVE-specific and implementations could choose to
> add additional CPUID leaves to this structure if needed in the future.
> Consumers of this note should lookup the value of required leaves and
> ignore any unneeded leaves.
> 
> An alternate approach might be to write out a more XSAVE-specific note
> that is an array containing the offset and size of each XSAVE region.

Something I thought about: I think there are asymmetrical x86 processors
now, with "big" and "little" cores, not sure how they call them.  For
them, is it possible for the XSAVE layout to be different per core, for
instance some cores supporting AVX512 and some cores not?  And
therefore, will we eventually need to include CPUID / XSAVE information
for more than one CPU core type in the core file notes?

Simon

Re: [RFC 00/13] Proposal for a new NT_X86_CPUID core dump note

[John Baldwin]

On 10/12/23 7:33 AM, Simon Marchi wrote:
> 
> 
> On 2023-10-09 14:36, John Baldwin wrote:
>> One of the shortcomings of the previous XSAVE patch series is that it
>> depends on heuristics based on the total XSAVE register set size and
>> XCR0 mask to infer layouts of the various register blocks for core
>> dumps.  This series introduces a new x86-specific core dump note
>> intended to supplant these heuristics by storing the raw CPUID leaves
>> describing the XSAVE layout in core dumps.
>>
>> This series proposes a new core dump note, NT_X86_CPUID (0x205), which
>> contains an array of structures.  Each structure describes an invidual
>> CPUID sub-leaf containing both the inputs to CPUID (%eax and %ecx) and
>> the outputs (%eax, %ebx, %ecx, and %edx) in a format roughly matching
>> the follow C structure:
>>
>> struct cpuid_leaf
>> {
>>      uint32_t leaf;
>>      uint32_t subleaf;
>>      uint32_t eax;
>>      uint32_t ebx;
>>      uint32_t ecx;
>>      uint32_t edx;
>> };
>>
>> This format is not XSAVE-specific and implementations could choose to
>> add additional CPUID leaves to this structure if needed in the future.
>> Consumers of this note should lookup the value of required leaves and
>> ignore any unneeded leaves.
>>
>> An alternate approach might be to write out a more XSAVE-specific note
>> that is an array containing the offset and size of each XSAVE region.
> 
> Something I thought about: I think there are asymmetrical x86 processors
> now, with "big" and "little" cores, not sure how they call them.  For
> them, is it possible for the XSAVE layout to be different per core, for
> instance some cores supporting AVX512 and some cores not?  And
> therefore, will we eventually need to include CPUID / XSAVE information
> for more than one CPU core type in the core file notes?

(The Intel names are "P" (performance) and "E" (energy-efficient) cores
btw)

I have no idea if they use the same or different XSAVE layouts.  It would
seem to be a royal pain if they did as everytime a user thread migrated
between cores the OS would have to convert the XSAVE block from one
layout to the other.  FreeBSD does not have any notion of multiple
layouts today and just assumes that the XSAVE layout is uniform across
all cores in a system.  I believe Linux's kernel does the same from my
reading.

-- 
John Baldwin

RE: [RFC 00/13] Proposal for a new NT_X86_CPUID core dump note

[George, Jini Susan]

[AMD Official Use Only - General]

I think that even if the xsave layout remains uniform across the cores of a system, since we are trying to design an extensible .note section which can possibly hold all kinds of CPUID information, we might want to consider various scenarios wherein the CPUID information might differ across cores (esp for big.LITTLE/(P/E)), like the cache information, perhaps ? It might be more prudent to include the coretype information also in such cases ?

Rgds
Jini.

>>-----Original Message-----
>>From: John Baldwin <jhb@FreeBSD.org>
>>Sent: Thursday, October 12, 2023 10:48 PM
>>To: Simon Marchi <simon.marchi@polymtl.ca>; gdb-patches@sourceware.org
>>Cc: Felix <felix.willgerodt@intel.com>; George, Jini Susan
>><JiniSusan.George@amd.com>
>>Subject: Re: [RFC 00/13] Proposal for a new NT_X86_CPUID core dump note
>>
>>Caution: This message originated from an External Source. Use proper caution
>>when opening attachments, clicking links, or responding.
>>
>>
>>On 10/12/23 7:33 AM, Simon Marchi wrote:
>>>
>>>
>>> On 2023-10-09 14:36, John Baldwin wrote:
>>>> One of the shortcomings of the previous XSAVE patch series is that it
>>>> depends on heuristics based on the total XSAVE register set size and
>>>> XCR0 mask to infer layouts of the various register blocks for core
>>>> dumps.  This series introduces a new x86-specific core dump note
>>>> intended to supplant these heuristics by storing the raw CPUID leaves
>>>> describing the XSAVE layout in core dumps.
>>>>
>>>> This series proposes a new core dump note, NT_X86_CPUID (0x205),
>>>> which contains an array of structures.  Each structure describes an
>>>> invidual CPUID sub-leaf containing both the inputs to CPUID (%eax and
>>>> %ecx) and the outputs (%eax, %ebx, %ecx, and %edx) in a format
>>>> roughly matching the follow C structure:
>>>>
>>>> struct cpuid_leaf
>>>> {
>>>>      uint32_t leaf;
>>>>      uint32_t subleaf;
>>>>      uint32_t eax;
>>>>      uint32_t ebx;
>>>>      uint32_t ecx;
>>>>      uint32_t edx;
>>>> };
>>>>
>>>> This format is not XSAVE-specific and implementations could choose to
>>>> add additional CPUID leaves to this structure if needed in the future.
>>>> Consumers of this note should lookup the value of required leaves and
>>>> ignore any unneeded leaves.
>>>>
>>>> An alternate approach might be to write out a more XSAVE-specific
>>>> note that is an array containing the offset and size of each XSAVE region.
>>>
>>> Something I thought about: I think there are asymmetrical x86
>>> processors now, with "big" and "little" cores, not sure how they call
>>> them.  For them, is it possible for the XSAVE layout to be different
>>> per core, for instance some cores supporting AVX512 and some cores
>>> not?  And therefore, will we eventually need to include CPUID / XSAVE
>>> information for more than one CPU core type in the core file notes?
>>
>>(The Intel names are "P" (performance) and "E" (energy-efficient) cores
>>btw)
>>
>>I have no idea if they use the same or different XSAVE layouts.  It would seem to
>>be a royal pain if they did as everytime a user thread migrated between cores
>>the OS would have to convert the XSAVE block from one layout to the other.
>>FreeBSD does not have any notion of multiple layouts today and just assumes
>>that the XSAVE layout is uniform across all cores in a system.  I believe Linux's
>>kernel does the same from my reading.
>>
>>--
>>John Baldwin

Re: [RFC 02/13] i387-tdep: Add function to read XSAVE layout from NT_X86_CPUID

[Lancelot SIX]

Hi,

I am not familiar with XSAVE details, but I have pure c++ style comments
below.

On Mon, Oct 09, 2023 at 11:36:04AM -0700, John Baldwin wrote:
> This can be used by x86 arches to determine the XSAVE layout instead
> of guessing based on the XCR0 mask and XSAVE register note size.
> ---
>  gdb/i387-tdep.c | 132 ++++++++++++++++++++++++++++++++++++++++++++++++
>  gdb/i387-tdep.h |   8 +++
>  2 files changed, 140 insertions(+)
> 
> diff --git a/gdb/i387-tdep.c b/gdb/i387-tdep.c
> index 47667da21c7..1eac2b6bd2a 100644
> --- a/gdb/i387-tdep.c
> +++ b/gdb/i387-tdep.c
> @@ -26,6 +26,8 @@
>  #include "target-float.h"
>  #include "value.h"
>  
> +#include <stdexcept>
> +
>  #include "i386-tdep.h"
>  #include "i387-tdep.h"
>  #include "gdbsupport/x86-xstate.h"
> @@ -987,6 +989,136 @@ i387_guess_xsave_layout (uint64_t xcr0, size_t xsave_size,
>    return true;
>  }
>  
> +/* Parse a reg-x86-cpuid pseudo section building a hash table mapping
> +   cpuid leaves to their results.  */
> +
> +struct cpuid_key
> +{
> +  cpuid_key (uint32_t _leaf, uint32_t _subleaf)
> +    : leaf(_leaf), subleaf(_subleaf)
> +  {}
> +
> +  uint32_t leaf;
> +  uint32_t subleaf;
> +
> +  constexpr bool operator== (const cpuid_key &other) const
> +  { return (leaf == other.leaf && subleaf == other.subleaf); }
> +};
> +
> +namespace std
> +{
> +template<>
> +struct hash<cpuid_key>
> +{
> +  size_t operator() (const cpuid_key &key) const
> +  {
> +    return key.leaf ^ (key.subleaf << 1);
> +  }
> +};
> +}

I think there was a discussion not long ago regarding opening std, and
it seems that the prefered approach is to use:

template<>
struct std::hash<cpuid_key>
{
  ...
};

See
https://sourceware.org/pipermail/gdb-patches/2023-September/202336.html
for the discussion.

> +
> +struct cpuid_values
> +{
> +  cpuid_values (uint32_t _eax, uint32_t _ebx, uint32_t _ecx, uint32_t _edx)
> +    : eax(_eax), ebx(_ebx), ecx(_ecx), edx(_edx)
> +  {}
> +
> +  uint32_t eax;
> +  uint32_t ebx;
> +  uint32_t ecx;
> +  uint32_t edx;
> +};
> +
> +typedef std::unordered_map<cpuid_key, cpuid_values> cpuid_map;
> +
> +static cpuid_map
> +i387_parse_cpuid_from_core (bfd *bfd)
> +{
> +  asection *section = bfd_get_section_by_name (bfd, ".reg-x86-cpuid");
> +  if (section == nullptr)
> +    return {};
> +
> +  size_t size = bfd_section_size (section);
> +  if (size == 0 || (size % (6 * 4)) != 0)
> +    return {};
> +
> +  char contents[size];

If I remember correctly, VLAs are not a C++ feature (but are supported
as a GCC extension
https://gcc.gnu.org/onlinedocs/gcc/Variable-Length.html).  I am unsure
if GDB has a policy regarding the use of extensions, so maybe this is
fine.  Otherwise, you could use a std::vector instead (it comes with a
dynamic allocation, but I am not too concerned at this is hardly on a
performance critical path)

   std::vector<char> contents (size);

> +  if (!bfd_get_section_contents (bfd, section, contents, 0, size))
> +    {
> +      warning (_("Couldn't read `.reg-x86-cpuid' section in core file."));
> +      return {};
> +    }
> +
> +  cpuid_map map;
> +  size_t index = 0;
> +  while (index < size)
> +    {
> +      uint32_t leaf = bfd_get_32 (bfd, contents + index);
> +      uint32_t count = bfd_get_32 (bfd, contents + index + 4);
> +      uint32_t eax = bfd_get_32 (bfd, contents + index + 8);
> +      uint32_t ebx = bfd_get_32 (bfd, contents + index + 12);
> +      uint32_t ecx = bfd_get_32 (bfd, contents + index + 16);
> +      uint32_t edx = bfd_get_32 (bfd, contents + index + 20);
> +
> +      if (map.count (cpuid_key (leaf, count)) != 0)
> +	{
> +	  warning (_("Duplicate cpuid leaf %#x,%#x"), leaf, count);
> +	  return {};
> +	}
> +      map.emplace (cpuid_key (leaf, count),
> +		   cpuid_values (eax, ebx, ecx, edx));

As Simon pointed out, there are two lookups here, where you can get away
with just one.  However, this is C++17 only which is not [yet] available
in GDB.  Instead, you can use the value returned by emplace to know if
an insertation has been done or not:

  auto emplace_result = map.emplace (cpuid_key (leaf, count),
                                     cpuid_values (eax, ebx, ecx, edx));
  if (!emplace_result.second)
    {
      warning (_("Duplicate cpuid leaf %#x,%#x"), leaf, count);
      return {};
    }

> +
> +      index += 6 * 4;
> +    }
> +
> +  return map;
> +}
> +
> +/* Fetch the offset of a specific XSAVE extended region.  */
> +
> +static int

I think it is worth returning uint32_t here as int is (in theory) target
dependent.

> +xsave_feature_offset (cpuid_map &map, uint64_t xcr0, int feature)

I think that the MAP parameter could be `const` here.

> +{
> +  if ((xcr0 & (1ULL << feature)) == 0)
> +    return 0;
> +
> +  return map.at (cpuid_key (0xd, feature)).ebx;
> +}
> +
> +/* See i387-tdep.h.  */
> +
> +bool
> +i387_read_xsave_layout_from_core (bfd *bfd, uint64_t xcr0, size_t xsave_size,
> +				  x86_xsave_layout &layout)
> +{
> +  cpuid_map map = i387_parse_cpuid_from_core (bfd);
> +  if (map.empty ())
> +    return false;
> +
> +  try
> +    {
> +      layout.sizeof_xsave = xsave_size;
> +      layout.avx_offset = xsave_feature_offset (map, xcr0,
> +						X86_XSTATE_AVX_ID);
> +      layout.bndregs_offset = xsave_feature_offset (map, xcr0,
> +						    X86_XSTATE_BNDREGS_ID);
> +      layout.bndcfg_offset = xsave_feature_offset (map, xcr0,
> +						   X86_XSTATE_BNDCFG_ID);
> +      layout.k_offset = xsave_feature_offset (map, xcr0,
> +					      X86_XSTATE_K_ID);
> +      layout.zmm_h_offset = xsave_feature_offset (map, xcr0,
> +						  X86_XSTATE_ZMM_H_ID);
> +      layout.zmm_offset = xsave_feature_offset (map, xcr0, X86_XSTATE_ZMM_ID);
> +      layout.pkru_offset = xsave_feature_offset (map, xcr0, X86_XSTATE_PKRU_ID);
> +    }
> +  catch (const std::out_of_range &)
> +    {
> +      return false;
> +    }
> +
> +  return true;
> +}
> +
>  /* Extract from XSAVE a bitset of the features that are available on the
>     target, but which have not yet been enabled.  */
>  
> diff --git a/gdb/i387-tdep.h b/gdb/i387-tdep.h
> index e149e30e52e..b16b9a60b67 100644
> --- a/gdb/i387-tdep.h
> +++ b/gdb/i387-tdep.h
> @@ -147,6 +147,14 @@ extern void i387_supply_fxsave (struct regcache *regcache, int regnum,
>  extern bool i387_guess_xsave_layout (uint64_t xcr0, size_t xsave_size,
>  				     x86_xsave_layout &layout);
>  
> +/* Determine the XSAVE layout from the `reg-x86-cpuid` section in a
> +   core dump.  Returns true on sucess, or false if a layout can not be

s/sucess/success/

> +   read.  */
> +
> +extern bool i387_read_xsave_layout_from_core (bfd *bfd, uint64_t xcr0,
> +					      size_t xsave_size,
> +					      x86_xsave_layout &layout);
> +
>  /* Similar to i387_supply_fxsave, but use XSAVE extended state.  */
>  
>  extern void i387_supply_xsave (struct regcache *regcache, int regnum,
> -- 
> 2.41.0
> 

Best,
Lancelot.

Re: [RFC 01/13] binutils: Support for the NT_X86_CPUID core dump note

[Lancelot SIX]

Hi John

I am aware this is a RFC series where you are mostly looking for
feedback on the overall direction, but I have an implemenation question
below (feel free to discard as part of the RFC process).t

On Mon, Oct 09, 2023 at 11:36:03AM -0700, John Baldwin wrote:
> This core dump note contains an array of CPUID leaf values.  Each
> entry in the array contains six 32-bit integers describing the inputs
> to the CPUID instruction (%eax and %ecx) and the outputs of the
> instruction (%eax, %ebx, %ecx, and %edx) similar to the C structure:
> 
> struct cpuid_leaf
> {
>     uint32_t leaf;
>     uint32_t subleaf;
>     uint32_t eax;
>     uint32_t ebx;
>     uint32_t ecx;
>     uint32_t edx;
> };
> 
> Current implementations of this note contain leaves associated with
> the XSAVE state area (major leaf 0xd), but future implementations may
> add other leaf values in the future.
> ---
>  bfd/elf-bfd.h        |  2 ++
>  bfd/elf.c            | 35 +++++++++++++++++++++++++++++++++++
>  binutils/readelf.c   |  2 ++
>  include/elf/common.h |  2 ++
>  4 files changed, 41 insertions(+)
> 
> diff --git a/bfd/elf-bfd.h b/bfd/elf-bfd.h
> index 335081ec629..235d0931ab4 100644
> --- a/bfd/elf-bfd.h
> +++ b/bfd/elf-bfd.h
> @@ -2871,6 +2871,8 @@ extern char *elfcore_write_prxfpreg
>    (bfd *, char *, int *, const void *, int);
>  extern char *elfcore_write_xstatereg
>    (bfd *, char *, int *, const void *, int);
> +extern char *elfcore_write_x86_cpuid
> +  (bfd *, char *, int *, const void *, int);
>  extern char *elfcore_write_x86_segbases
>    (bfd *, char *, int *, const void *, int);
>  extern char *elfcore_write_ppc_vmx
> diff --git a/bfd/elf.c b/bfd/elf.c
> index b5b0c69e097..35679821a49 100644
> --- a/bfd/elf.c
> +++ b/bfd/elf.c
> @@ -10495,6 +10495,16 @@ elfcore_grok_xstatereg (bfd *abfd, Elf_Internal_Note *note)
>    return elfcore_make_note_pseudosection (abfd, ".reg-xstate", note);
>  }
>  
> +/* Some systems dump an array of x86 cpuid leaves with a note type of
> +   NT_X86_CPUID.  Just include the whole note's contents
> +   literally.  */
> +
> +static bool
> +elfcore_grok_x86_cpuid (bfd *abfd, Elf_Internal_Note *note)
> +{
> +  return elfcore_make_note_pseudosection (abfd, ".reg-x86-cpuid", note);
> +}
> +
>  static bool
>  elfcore_grok_ppc_vmx (bfd *abfd, Elf_Internal_Note *note)
>  {
> @@ -11190,6 +11200,13 @@ elfcore_grok_note (bfd *abfd, Elf_Internal_Note *note)
>        else
>  	return true;
>  
> +    case NT_X86_CPUID:
> +      if (note->namesz == 6
> +	  && strcmp (note->namedata, "LINUX") == 0)
> +	return elfcore_grok_x86_cpuid (abfd, note);
> +      else
> +	return true;
> +
>      case NT_PPC_VMX:
>        if (note->namesz == 6
>  	  && strcmp (note->namedata, "LINUX") == 0)
> @@ -11768,6 +11785,9 @@ elfcore_grok_freebsd_note (bfd *abfd, Elf_Internal_Note *note)
>      case NT_X86_XSTATE:
>        return elfcore_grok_xstatereg (abfd, note);
>  
> +    case NT_X86_CPUID:
> +      return elfcore_grok_x86_cpuid (abfd, note);
> +
>      case NT_FREEBSD_PTLWPINFO:
>        return elfcore_make_note_pseudosection (abfd, ".note.freebsdcore.lwpinfo",
>  					      note);
> @@ -12640,6 +12660,19 @@ elfcore_write_xstatereg (bfd *abfd, char *buf, int *bufsiz,
>  			     note_name, NT_X86_XSTATE, xfpregs, size);
>  }
>  
> +char *
> +elfcore_write_x86_cpuid (bfd *abfd, char *buf, int *bufsiz,
> +			 const void *cpuid, int size)
> +{
> +  char *note_name;
> +  if (get_elf_backend_data (abfd)->elf_osabi == ELFOSABI_FREEBSD)
> +    note_name = "FreeBSD";

The code above (in elfcore_grok_note) only accept "LINUX", and the
comment in "include/elf/common.h" says "note name must be "LINUX".

Is this something you intend to adjust later when adding full FreeBSD
support?

Best,
Lancelot.

> +  else
> +    note_name = "LINUX";
> +  return elfcore_write_note (abfd, buf, bufsiz,
> +			     note_name, NT_X86_CPUID, cpuid, size);
> +}
> +
>  char *
>  elfcore_write_x86_segbases (bfd *abfd, char *buf, int *bufsiz,
>  			    const void *regs, int size)
> @@ -13233,6 +13266,8 @@ elfcore_write_register_note (bfd *abfd,
>      return elfcore_write_prxfpreg (abfd, buf, bufsiz, data, size);
>    if (strcmp (section, ".reg-xstate") == 0)
>      return elfcore_write_xstatereg (abfd, buf, bufsiz, data, size);
> +  if (strcmp (section, ".reg-x86-cpuid") == 0)
> +    return elfcore_write_x86_cpuid (abfd, buf, bufsiz, data, size);
>    if (strcmp (section, ".reg-x86-segbases") == 0)
>      return elfcore_write_x86_segbases (abfd, buf, bufsiz, data, size);
>    if (strcmp (section, ".reg-ppc-vmx") == 0)
> diff --git a/binutils/readelf.c b/binutils/readelf.c
> index c9b6210e229..cb80aa6f396 100644
> --- a/binutils/readelf.c
> +++ b/binutils/readelf.c
> @@ -20134,6 +20134,8 @@ get_note_type (Filedata * filedata, unsigned e_type)
>  	return _("NT_X86_XSTATE (x86 XSAVE extended state)");
>        case NT_X86_CET:
>  	return _("NT_X86_CET (x86 CET state)");
> +      case NT_X86_CPUID:
> +	return _("NT_X86_CPUID (x86 CPUID leaves)");
>        case NT_S390_HIGH_GPRS:
>  	return _("NT_S390_HIGH_GPRS (s390 upper register halves)");
>        case NT_S390_TIMER:
> diff --git a/include/elf/common.h b/include/elf/common.h
> index 244b13361e5..e8c6d753987 100644
> --- a/include/elf/common.h
> +++ b/include/elf/common.h
> @@ -645,6 +645,8 @@
>  					/*   note name must be "LINUX".  */
>  #define NT_X86_CET	0x203		/* x86 CET state.  */
>  					/*   note name must be "LINUX".  */
> +#define NT_X86_CPUID	0x205		/* x86 CPUID leaves.  */
> +					/*   note name must be "LINUX".  */
>  #define NT_S390_HIGH_GPRS 0x300		/* S/390 upper halves of GPRs  */
>  					/*   note name must be "LINUX".  */
>  #define NT_S390_TIMER	0x301		/* S390 timer */
> -- 
> 2.41.0
> 

Re: [RFC 10/13] {amd64, i386}-fbsd-tdep: Include NT_X86_CPUID notes in core dumps from gcore

[Lancelot SIX]

Hi,

Just a forating nit below (again, feel free to discard as this is a RFC
series)

On Mon, Oct 09, 2023 at 11:36:12AM -0700, John Baldwin wrote:
> Override the gdbarch make_corefile_notes method for the FreeBSD x86
> arches with a new function that calls fbsd_make_corefile_notes and
> x86_elf_make_cpuid_note to generate the core dump notes.
> ---
>  gdb/amd64-fbsd-tdep.c |  1 +
>  gdb/i386-fbsd-tdep.c  | 15 +++++++++++++++
>  gdb/i386-fbsd-tdep.h  |  7 +++++++
>  3 files changed, 23 insertions(+)
> 
> diff --git a/gdb/i386-fbsd-tdep.c b/gdb/i386-fbsd-tdep.c
> index 1789f3921fd..810ecc90df1 100644
> --- a/gdb/i386-fbsd-tdep.c
> +++ b/gdb/i386-fbsd-tdep.c
> @@ -26,6 +26,7 @@
>  #include "tramp-frame.h"
>  #include "i386-fbsd-tdep.h"
>  
> +#include "x86-tdep.h"
>  #include "i386-tdep.h"
>  #include "i387-tdep.h"
>  #include "fbsd-tdep.h"
> @@ -370,6 +371,19 @@ i386fbsd_get_thread_local_address (struct gdbarch *gdbarch, ptid_t ptid,
>    return fbsd_get_thread_local_address (gdbarch, dtv_addr, lm_addr, offset);
>  }
>  
> +/* See i386-fbsd-tdep.h.  */
> +
> +gdb::unique_xmalloc_ptr<char>
> +i386_fbsd_make_corefile_notes (struct gdbarch *gdbarch, bfd *obfd,
> +			       int *note_size)
> +{
> +  gdb::unique_xmalloc_ptr<char> note_data =
> +    fbsd_make_corefile_notes (gdbarch, obfd, note_size);

In GDB's coding style, the linebreak should be before the "=".

Best,
Lancelot.

> +
> +  x86_elf_make_cpuid_note (obfd, &note_data, note_size);
> +  return note_data;
> +}
> +
>  static void
>  i386fbsd_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
>  {
> @@ -403,6 +417,7 @@ i386fbsd_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
>  
>    set_gdbarch_core_read_description (gdbarch,
>  				     i386fbsd_core_read_description);
> +  set_gdbarch_make_corefile_notes (gdbarch, i386_fbsd_make_corefile_notes);
>  
>    /* FreeBSD uses SVR4-style shared libraries.  */
>    set_solib_svr4_fetch_link_map_offsets
> diff --git a/gdb/i386-fbsd-tdep.h b/gdb/i386-fbsd-tdep.h
> index c49cb1eba68..fc7bb1c521d 100644
> --- a/gdb/i386-fbsd-tdep.h
> +++ b/gdb/i386-fbsd-tdep.h
> @@ -40,6 +40,13 @@ bool i386_fbsd_core_read_x86_xsave_layout (struct gdbarch *gdbarch,
>     matches the layout on Linux.  */
>  #define I386_FBSD_XSAVE_XCR0_OFFSET 464
>  
> +/* Create appropriate note sections for a corefile, returning them in
> +   allocated memory.  Extends fbsd_make_corefile_notes to add a
> +   NT_X86_CPUID note.  */
> +
> +gdb::unique_xmalloc_ptr<char> i386_fbsd_make_corefile_notes
> +(struct gdbarch *gdbarch, bfd *obfd, int *note_size);
> +
>  extern const struct regset i386_fbsd_gregset;
>  
>  #endif /* i386-fbsd-tdep.h */
> -- 
> 2.41.0
> 

Re: [RFC 01/13] binutils: Support for the NT_X86_CPUID core dump note

[John Baldwin]

On 10/16/23 2:23 AM, Lancelot SIX wrote:
> Hi John
> 
> I am aware this is a RFC series where you are mostly looking for
> feedback on the overall direction, but I have an implemenation question
> below (feel free to discard as part of the RFC process).t
> 
> On Mon, Oct 09, 2023 at 11:36:03AM -0700, John Baldwin wrote:
>> This core dump note contains an array of CPUID leaf values.  Each
>> entry in the array contains six 32-bit integers describing the inputs
>> to the CPUID instruction (%eax and %ecx) and the outputs of the
>> instruction (%eax, %ebx, %ecx, and %edx) similar to the C structure:
>>
>> struct cpuid_leaf
>> {
>>      uint32_t leaf;
>>      uint32_t subleaf;
>>      uint32_t eax;
>>      uint32_t ebx;
>>      uint32_t ecx;
>>      uint32_t edx;
>> };
>>
>> Current implementations of this note contain leaves associated with
>> the XSAVE state area (major leaf 0xd), but future implementations may
>> add other leaf values in the future.
>> ---
>>   bfd/elf-bfd.h        |  2 ++
>>   bfd/elf.c            | 35 +++++++++++++++++++++++++++++++++++
>>   binutils/readelf.c   |  2 ++
>>   include/elf/common.h |  2 ++
>>   4 files changed, 41 insertions(+)
>>
>> @@ -12640,6 +12660,19 @@ elfcore_write_xstatereg (bfd *abfd, char *buf, int *bufsiz,
>>   			     note_name, NT_X86_XSTATE, xfpregs, size);
>>   }
>>   
>> +char *
>> +elfcore_write_x86_cpuid (bfd *abfd, char *buf, int *bufsiz,
>> +			 const void *cpuid, int size)
>> +{
>> +  char *note_name;
>> +  if (get_elf_backend_data (abfd)->elf_osabi == ELFOSABI_FREEBSD)
>> +    note_name = "FreeBSD";
> 
> The code above (in elfcore_grok_note) only accept "LINUX", and the
> comment in "include/elf/common.h" says "note name must be "LINUX".
> 
> Is this something you intend to adjust later when adding full FreeBSD
> support?

I really just kept that to match what is there for NT_X86_XSTATE, but
this patch already includes FreeBSD support (FreeBSD notes use
elfcore_grok_freebsd_note rather than elfcore_grok_note).  I almost
left that comment about the note name out, and probably I should just
do that.  (Arguably I should also remove that comment from NT_X86_XSTATE
and possibly some others that both FreeBSD and Linux generate.)

On a related note, abusing elf_osabi to determine the note name is a
bit of a hack.  It would be nice if elfcore_write_register_note
accepted the note name as an argument, but I'm not sure how much
that would break (e.g. if that's a public binutils API used by
other things than gdb).

-- 
John Baldwin

Re: [RFC 02/13] i387-tdep: Add function to read XSAVE layout from NT_X86_CPUID

[John Baldwin]

On 10/11/23 9:27 PM, Simon Marchi wrote:
> 
> 
> On 2023-10-09 14:36, John Baldwin wrote:
>> This can be used by x86 arches to determine the XSAVE layout instead
>> of guessing based on the XCR0 mask and XSAVE register note size.
> 
> Just some nits below:
> 
>> +typedef std::unordered_map<cpuid_key, cpuid_values> cpuid_map;
> 
> For new stuff I would suggest:
> 
>    using cpuid_map = std::unordered_map<cpuid_key, cpuid_values>;

Ok.

>> +
>> +static cpuid_map
>> +i387_parse_cpuid_from_core (bfd *bfd)
>> +{
>> +  asection *section = bfd_get_section_by_name (bfd, ".reg-x86-cpuid");
>> +  if (section == nullptr)
>> +    return {};
>> +
>> +  size_t size = bfd_section_size (section);
>> +  if (size == 0 || (size % (6 * 4)) != 0)
> 
> That 4 could be `sizeof (uint32_t)`.  And `6 * 4` appears below again,
> it could a constexpr value with a name like entry_size.

Ok.

>> +    return {};
>> +
>> +  char contents[size];
>> +  if (!bfd_get_section_contents (bfd, section, contents, 0, size))
>> +    {
>> +      warning (_("Couldn't read `.reg-x86-cpuid' section in core file."));
>> +      return {};
>> +    }
>> +
>> +  cpuid_map map;
>> +  size_t index = 0;
>> +  while (index < size)
>> +    {
>> +      uint32_t leaf = bfd_get_32 (bfd, contents + index);
>> +      uint32_t count = bfd_get_32 (bfd, contents + index + 4);
>> +      uint32_t eax = bfd_get_32 (bfd, contents + index + 8);
>> +      uint32_t ebx = bfd_get_32 (bfd, contents + index + 12);
>> +      uint32_t ecx = bfd_get_32 (bfd, contents + index + 16);
>> +      uint32_t edx = bfd_get_32 (bfd, contents + index + 20);
>> +
>> +      if (map.count (cpuid_key (leaf, count)) != 0)
>> +	{
>> +	  warning (_("Duplicate cpuid leaf %#x,%#x"), leaf, count);
>> +	  return {};
>> +	}
>> +      map.emplace (cpuid_key (leaf, count),
>> +		   cpuid_values (eax, ebx, ecx, edx));
> 
> This could be slightly more optimal using map::try_emplace (to avoid
> having two map lookups).

Ok.

Presumably C++17 will be required before this series lands, and I
don't plan to backport it to GDB 14.  (If I did it could also turn
back into a loop + emplace though as part of the backport.)

-- 
John Baldwin

Re: [RFC 02/13] i387-tdep: Add function to read XSAVE layout from NT_X86_CPUID

[John Baldwin]

On 10/16/23 2:17 AM, Lancelot SIX wrote:
> Hi,
> 
> I am not familiar with XSAVE details, but I have pure c++ style comments
> below.

Thanks, I've generally accepted the changes aside from a few modifications below.
  
> On Mon, Oct 09, 2023 at 11:36:04AM -0700, John Baldwin wrote:
>> This can be used by x86 arches to determine the XSAVE layout instead
>> of guessing based on the XCR0 mask and XSAVE register note size.
>> ---
>>   gdb/i387-tdep.c | 132 ++++++++++++++++++++++++++++++++++++++++++++++++
>>   gdb/i387-tdep.h |   8 +++
>>   2 files changed, 140 insertions(+)
>>
>> diff --git a/gdb/i387-tdep.c b/gdb/i387-tdep.c
>> index 47667da21c7..1eac2b6bd2a 100644
>> --- a/gdb/i387-tdep.c
>> +++ b/gdb/i387-tdep.c
>> +  size_t size = bfd_section_size (section);
>> +  if (size == 0 || (size % (6 * 4)) != 0)
>> +    return {};
>> +
>> +  char contents[size];
> 
> If I remember correctly, VLAs are not a C++ feature (but are supported
> as a GCC extension
> https://gcc.gnu.org/onlinedocs/gcc/Variable-Length.html).  I am unsure
> if GDB has a policy regarding the use of extensions, so maybe this is
> fine.  Otherwise, you could use a std::vector instead (it comes with a
> dynamic allocation, but I am not too concerned at this is hardly on a
> performance critical path)
> 
>     std::vector<char> contents (size);

I've used a gdb::byte_vector instead of a plain std::vector<>.

>> +      if (map.count (cpuid_key (leaf, count)) != 0)
>> +	{
>> +	  warning (_("Duplicate cpuid leaf %#x,%#x"), leaf, count);
>> +	  return {};
>> +	}
>> +      map.emplace (cpuid_key (leaf, count),
>> +		   cpuid_values (eax, ebx, ecx, edx));
> 
> As Simon pointed out, there are two lookups here, where you can get away
> with just one.  However, this is C++17 only which is not [yet] available
> in GDB.  Instead, you can use the value returned by emplace to know if
> an insertation has been done or not:
> 
>    auto emplace_result = map.emplace (cpuid_key (leaf, count),
>                                       cpuid_values (eax, ebx, ecx, edx));
>    if (!emplace_result.second)
>      {
>        warning (_("Duplicate cpuid leaf %#x,%#x"), leaf, count);
>        return {};
>      }

I'm going to assume that C++17 will land first in GDB before this and just
go with try_emplace for now.  If I end up backporting this to GDB 14 (which
I don't currently anticipate), then this wil be nicer for that.

-- 
John Baldwin

Re: [RFC 04/13] gdb: Use NT_X86_CPUID in x86 FreeBSD architectures to read XSAVE layouts

[John Baldwin]

On 10/11/23 9:28 PM, Simon Marchi wrote:
> 
> 
> On 2023-10-09 14:36, John Baldwin wrote:
>> If this core dump note is present, use it to determine the layout of
>> XSAVE register set notes.
> 
> s/FreeBSD/Linux in the subject of the commit.

Oops, fixed.

-- 
John Baldwin

Re: [RFC 06/13] nat/x86-cpuid: Add a function to build the contents of a NT_X86_CPUID note

[John Baldwin]

On 10/11/23 9:41 PM, Simon Marchi wrote:
> 
> 
> On 2023-10-09 14:36, John Baldwin wrote:
>> +/* See x86-cpuid.h.  */
>> +
>> +void
>> +x86_cpuid_note (gdb::unique_xmalloc_ptr<gdb_byte> &buf, size_t &len)
>> +{
> 
> I think it would be a bit simpler to make this return a gdb::byte_vector.

Yes, thanks.  I couldn't remember the name of this class.

-- 
John Baldwin

Re: [RFC 10/13] {amd64, i386}-fbsd-tdep: Include NT_X86_CPUID notes in core dumps from gcore

[John Baldwin]

On 10/16/23 2:31 AM, Lancelot SIX wrote:
> Hi,
> 
> Just a forating nit below (again, feel free to discard as this is a RFC
> series)
> 
> On Mon, Oct 09, 2023 at 11:36:12AM -0700, John Baldwin wrote:
>> Override the gdbarch make_corefile_notes method for the FreeBSD x86
>> arches with a new function that calls fbsd_make_corefile_notes and
>> x86_elf_make_cpuid_note to generate the core dump notes.
>> ---
>>   gdb/amd64-fbsd-tdep.c |  1 +
>>   gdb/i386-fbsd-tdep.c  | 15 +++++++++++++++
>>   gdb/i386-fbsd-tdep.h  |  7 +++++++
>>   3 files changed, 23 insertions(+)
>>
>> diff --git a/gdb/i386-fbsd-tdep.c b/gdb/i386-fbsd-tdep.c
>> index 1789f3921fd..810ecc90df1 100644
>> --- a/gdb/i386-fbsd-tdep.c
>> +++ b/gdb/i386-fbsd-tdep.c
>> @@ -26,6 +26,7 @@
>>   #include "tramp-frame.h"
>>   #include "i386-fbsd-tdep.h"
>>   
>> +#include "x86-tdep.h"
>>   #include "i386-tdep.h"
>>   #include "i387-tdep.h"
>>   #include "fbsd-tdep.h"
>> @@ -370,6 +371,19 @@ i386fbsd_get_thread_local_address (struct gdbarch *gdbarch, ptid_t ptid,
>>     return fbsd_get_thread_local_address (gdbarch, dtv_addr, lm_addr, offset);
>>   }
>>   
>> +/* See i386-fbsd-tdep.h.  */
>> +
>> +gdb::unique_xmalloc_ptr<char>
>> +i386_fbsd_make_corefile_notes (struct gdbarch *gdbarch, bfd *obfd,
>> +			       int *note_size)
>> +{
>> +  gdb::unique_xmalloc_ptr<char> note_data =
>> +    fbsd_make_corefile_notes (gdbarch, obfd, note_size);
> 
> In GDB's coding style, the linebreak should be before the "=".

Thanks, fixed here and in the final patch.

-- 
John Baldwin

Re: [RFC 00/13] Proposal for a new NT_X86_CPUID core dump note

[John Baldwin]

On 10/13/23 2:38 AM, George, Jini Susan wrote:
> [AMD Official Use Only - General]
> 
> I think that even if the xsave layout remains uniform across the cores of a system, since we are trying to design an extensible .note section which can possibly hold all kinds of CPUID information, we might want to consider various scenarios wherein the CPUID information might differ across cores (esp for big.LITTLE/(P/E)), like the cache information, perhaps ? It might be more prudent to include the coretype information also in such cases ?

It's certainly occurred to me that it might be prudent to include the full
complement of CPUID leaves even in the initial version of this note should
that information prove useful in the future.

However, I'm not quite sure how we should "name" the different CPUID leaf
sets in this case (e.g. the set for E cores vs the set for P cores).  In
particular, it's not quite clear to me how many sets future systems might
have?  One route is to use additional NT_ values for different sets,
(e.g. a NT_X86_CPUID_E_CORE or some such if the "default" set is for the
P core).  We could also embed an identifier at the start of a note, e.g.
a 32-bit integer, and dump one note per set, so you have always have a note
for set 0, but in a system with E cores you might have a set 1 where the
convention would be P cores are set 0, and E cores are set 1.  Future
systems with multiple core types would have to decide what type of mapping
to use between core types and set IDs.  One issue with the second approach
is if you want these notes accessible via ptrace() and not just in cores,
it wouldn't be clear how to request the leaves for set N if they all share
the NT_* value.

> Rgds
> Jini.
> 
>>> -----Original Message-----
>>> From: John Baldwin <jhb@FreeBSD.org>
>>> Sent: Thursday, October 12, 2023 10:48 PM
>>> To: Simon Marchi <simon.marchi@polymtl.ca>; gdb-patches@sourceware.org
>>> Cc: Felix <felix.willgerodt@intel.com>; George, Jini Susan
>>> <JiniSusan.George@amd.com>
>>> Subject: Re: [RFC 00/13] Proposal for a new NT_X86_CPUID core dump note
>>>
>>> Caution: This message originated from an External Source. Use proper caution
>>> when opening attachments, clicking links, or responding.
>>>
>>>
>>> On 10/12/23 7:33 AM, Simon Marchi wrote:
>>>>
>>>>
>>>> On 2023-10-09 14:36, John Baldwin wrote:
>>>>> One of the shortcomings of the previous XSAVE patch series is that it
>>>>> depends on heuristics based on the total XSAVE register set size and
>>>>> XCR0 mask to infer layouts of the various register blocks for core
>>>>> dumps.  This series introduces a new x86-specific core dump note
>>>>> intended to supplant these heuristics by storing the raw CPUID leaves
>>>>> describing the XSAVE layout in core dumps.
>>>>>
>>>>> This series proposes a new core dump note, NT_X86_CPUID (0x205),
>>>>> which contains an array of structures.  Each structure describes an
>>>>> invidual CPUID sub-leaf containing both the inputs to CPUID (%eax and
>>>>> %ecx) and the outputs (%eax, %ebx, %ecx, and %edx) in a format
>>>>> roughly matching the follow C structure:
>>>>>
>>>>> struct cpuid_leaf
>>>>> {
>>>>>       uint32_t leaf;
>>>>>       uint32_t subleaf;
>>>>>       uint32_t eax;
>>>>>       uint32_t ebx;
>>>>>       uint32_t ecx;
>>>>>       uint32_t edx;
>>>>> };
>>>>>
>>>>> This format is not XSAVE-specific and implementations could choose to
>>>>> add additional CPUID leaves to this structure if needed in the future.
>>>>> Consumers of this note should lookup the value of required leaves and
>>>>> ignore any unneeded leaves.
>>>>>
>>>>> An alternate approach might be to write out a more XSAVE-specific
>>>>> note that is an array containing the offset and size of each XSAVE region.
>>>>
>>>> Something I thought about: I think there are asymmetrical x86
>>>> processors now, with "big" and "little" cores, not sure how they call
>>>> them.  For them, is it possible for the XSAVE layout to be different
>>>> per core, for instance some cores supporting AVX512 and some cores
>>>> not?  And therefore, will we eventually need to include CPUID / XSAVE
>>>> information for more than one CPU core type in the core file notes?
>>>
>>> (The Intel names are "P" (performance) and "E" (energy-efficient) cores
>>> btw)
>>>
>>> I have no idea if they use the same or different XSAVE layouts.  It would seem to
>>> be a royal pain if they did as everytime a user thread migrated between cores
>>> the OS would have to convert the XSAVE block from one layout to the other.
>>> FreeBSD does not have any notion of multiple layouts today and just assumes
>>> that the XSAVE layout is uniform across all cores in a system.  I believe Linux's
>>> kernel does the same from my reading.
>>>
>>> --
>>> John Baldwin
> 

-- 
John Baldwin

RE: [RFC 00/13] Proposal for a new NT_X86_CPUID core dump note

[George, Jini Susan]

[AMD Official Use Only - General]

I think it might be better to go with different NT_ values for the P and the E cores. We will also need to put in additional mapping information as to which core/ cpu id belongs to which core set (P or E).

Thanks
Jini.

>>-----Original Message-----
>>From: John Baldwin <jhb@FreeBSD.org>
>>Sent: Tuesday, October 17, 2023 6:06 AM
>>To: George, Jini Susan <JiniSusan.George@amd.com>; Simon Marchi
>><simon.marchi@polymtl.ca>; gdb-patches@sourceware.org
>>Cc: Felix <felix.willgerodt@intel.com>; Balasubrmanian, Vignesh
>><Vignesh.Balasubrmanian@amd.com>
>>Subject: Re: [RFC 00/13] Proposal for a new NT_X86_CPUID core dump note
>>
>>Caution: This message originated from an External Source. Use proper caution
>>when opening attachments, clicking links, or responding.
>>
>>
>>On 10/13/23 2:38 AM, George, Jini Susan wrote:
>>> [AMD Official Use Only - General]
>>>
>>> I think that even if the xsave layout remains uniform across the cores of a
>>system, since we are trying to design an extensible .note section which can
>>possibly hold all kinds of CPUID information, we might want to consider various
>>scenarios wherein the CPUID information might differ across cores (esp for
>>big.LITTLE/(P/E)), like the cache information, perhaps ? It might be more prudent
>>to include the coretype information also in such cases ?
>>
>>It's certainly occurred to me that it might be prudent to include the full
>>complement of CPUID leaves even in the initial version of this note should that
>>information prove useful in the future.
>>
>>However, I'm not quite sure how we should "name" the different CPUID leaf sets
>>in this case (e.g. the set for E cores vs the set for P cores).  In particular, it's not
>>quite clear to me how many sets future systems might have?  One route is to use
>>additional NT_ values for different sets, (e.g. a NT_X86_CPUID_E_CORE or some
>>such if the "default" set is for the P core).  We could also embed an identifier at
>>the start of a note, e.g.
>>a 32-bit integer, and dump one note per set, so you have always have a note for
>>set 0, but in a system with E cores you might have a set 1 where the convention
>>would be P cores are set 0, and E cores are set 1.  Future systems with multiple
>>core types would have to decide what type of mapping to use between core
>>types and set IDs.  One issue with the second approach is if you want these
>>notes accessible via ptrace() and not just in cores, it wouldn't be clear how to
>>request the leaves for set N if they all share the NT_* value.
>>
>>> Rgds
>>> Jini.
>>>
>>>>> -----Original Message-----
>>>>> From: John Baldwin <jhb@FreeBSD.org>
>>>>> Sent: Thursday, October 12, 2023 10:48 PM
>>>>> To: Simon Marchi <simon.marchi@polymtl.ca>;
>>>>> gdb-patches@sourceware.org
>>>>> Cc: Felix <felix.willgerodt@intel.com>; George, Jini Susan
>>>>> <JiniSusan.George@amd.com>
>>>>> Subject: Re: [RFC 00/13] Proposal for a new NT_X86_CPUID core dump
>>>>> note
>>>>>
>>>>> Caution: This message originated from an External Source. Use proper
>>>>> caution when opening attachments, clicking links, or responding.
>>>>>
>>>>>
>>>>> On 10/12/23 7:33 AM, Simon Marchi wrote:
>>>>>>
>>>>>>
>>>>>> On 2023-10-09 14:36, John Baldwin wrote:
>>>>>>> One of the shortcomings of the previous XSAVE patch series is that
>>>>>>> it depends on heuristics based on the total XSAVE register set
>>>>>>> size and
>>>>>>> XCR0 mask to infer layouts of the various register blocks for core
>>>>>>> dumps.  This series introduces a new x86-specific core dump note
>>>>>>> intended to supplant these heuristics by storing the raw CPUID
>>>>>>> leaves describing the XSAVE layout in core dumps.
>>>>>>>
>>>>>>> This series proposes a new core dump note, NT_X86_CPUID (0x205),
>>>>>>> which contains an array of structures.  Each structure describes
>>>>>>> an invidual CPUID sub-leaf containing both the inputs to CPUID
>>>>>>> (%eax and
>>>>>>> %ecx) and the outputs (%eax, %ebx, %ecx, and %edx) in a format
>>>>>>> roughly matching the follow C structure:
>>>>>>>
>>>>>>> struct cpuid_leaf
>>>>>>> {
>>>>>>>       uint32_t leaf;
>>>>>>>       uint32_t subleaf;
>>>>>>>       uint32_t eax;
>>>>>>>       uint32_t ebx;
>>>>>>>       uint32_t ecx;
>>>>>>>       uint32_t edx;
>>>>>>> };
>>>>>>>
>>>>>>> This format is not XSAVE-specific and implementations could choose
>>>>>>> to add additional CPUID leaves to this structure if needed in the future.
>>>>>>> Consumers of this note should lookup the value of required leaves
>>>>>>> and ignore any unneeded leaves.
>>>>>>>
>>>>>>> An alternate approach might be to write out a more XSAVE-specific
>>>>>>> note that is an array containing the offset and size of each XSAVE region.
>>>>>>
>>>>>> Something I thought about: I think there are asymmetrical x86
>>>>>> processors now, with "big" and "little" cores, not sure how they
>>>>>> call them.  For them, is it possible for the XSAVE layout to be
>>>>>> different per core, for instance some cores supporting AVX512 and
>>>>>> some cores not?  And therefore, will we eventually need to include
>>>>>> CPUID / XSAVE information for more than one CPU core type in the core
>>file notes?
>>>>>
>>>>> (The Intel names are "P" (performance) and "E" (energy-efficient)
>>>>> cores
>>>>> btw)
>>>>>
>>>>> I have no idea if they use the same or different XSAVE layouts.  It
>>>>> would seem to be a royal pain if they did as everytime a user thread
>>>>> migrated between cores the OS would have to convert the XSAVE block
>>from one layout to the other.
>>>>> FreeBSD does not have any notion of multiple layouts today and just
>>>>> assumes that the XSAVE layout is uniform across all cores in a
>>>>> system.  I believe Linux's kernel does the same from my reading.
>>>>>
>>>>> --
>>>>> John Baldwin
>>>
>>
>>--
>>John Baldwin

Re: [RFC 00/13] Proposal for a new NT_X86_CPUID core dump note

[John Baldwin]

On 10/26/23 9:18 AM, George, Jini Susan wrote:
> [AMD Official Use Only - General]
> 
> I think it might be better to go with different NT_ values for the P and the E cores. We will also need to put in additional mapping information as to which core/ cpu id belongs to which core set (P or E).

Hmm, do you mean mapping thread (LWP) IDs to specific core sets?
Right now core dumps don't really include the mapping of threads
to individual CPU cores, in part because threads can migrate
between cores (subject to administrative and application constraints).
I'm not really sure there is a use case for storing that information.

One way you could store that would be to have a per-thread note
storing the logical CPU mask (or cpuset, not sure which term Linux
uses, FreeBSD uses cpuset) for each thread, but then you'd need
perhaps some other note mapping logical CPU IDs to other CPU data.
(For example, if you did have a separate NT_X86_CPUID_E_CORE, you
could have some other note that was a short table mapping CPU IDs
to CPUID notes (e.g. cpu 0 is NT_X86_CPUID, cpu 1 is NT_X86_CPUID,
cpu 2 is NT_X86_CPUID_E_CORE).

However, I'm not sure it makes sense to store all of that now without
a use case.  I do think though that this does show there are ways to
extend the information in the future should a use case arrive.

> Thanks
> Jini.
> 
>>> -----Original Message-----
>>> From: John Baldwin <jhb@FreeBSD.org>
>>> Sent: Tuesday, October 17, 2023 6:06 AM
>>> To: George, Jini Susan <JiniSusan.George@amd.com>; Simon Marchi
>>> <simon.marchi@polymtl.ca>; gdb-patches@sourceware.org
>>> Cc: Felix <felix.willgerodt@intel.com>; Balasubrmanian, Vignesh
>>> <Vignesh.Balasubrmanian@amd.com>
>>> Subject: Re: [RFC 00/13] Proposal for a new NT_X86_CPUID core dump note
>>>
>>> Caution: This message originated from an External Source. Use proper caution
>>> when opening attachments, clicking links, or responding.
>>>
>>>
>>> On 10/13/23 2:38 AM, George, Jini Susan wrote:
>>>> [AMD Official Use Only - General]
>>>>
>>>> I think that even if the xsave layout remains uniform across the cores of a
>>> system, since we are trying to design an extensible .note section which can
>>> possibly hold all kinds of CPUID information, we might want to consider various
>>> scenarios wherein the CPUID information might differ across cores (esp for
>>> big.LITTLE/(P/E)), like the cache information, perhaps ? It might be more prudent
>>> to include the coretype information also in such cases ?
>>>
>>> It's certainly occurred to me that it might be prudent to include the full
>>> complement of CPUID leaves even in the initial version of this note should that
>>> information prove useful in the future.
>>>
>>> However, I'm not quite sure how we should "name" the different CPUID leaf sets
>>> in this case (e.g. the set for E cores vs the set for P cores).  In particular, it's not
>>> quite clear to me how many sets future systems might have?  One route is to use
>>> additional NT_ values for different sets, (e.g. a NT_X86_CPUID_E_CORE or some
>>> such if the "default" set is for the P core).  We could also embed an identifier at
>>> the start of a note, e.g.
>>> a 32-bit integer, and dump one note per set, so you have always have a note for
>>> set 0, but in a system with E cores you might have a set 1 where the convention
>>> would be P cores are set 0, and E cores are set 1.  Future systems with multiple
>>> core types would have to decide what type of mapping to use between core
>>> types and set IDs.  One issue with the second approach is if you want these
>>> notes accessible via ptrace() and not just in cores, it wouldn't be clear how to
>>> request the leaves for set N if they all share the NT_* value.
>>>
>>>> Rgds
>>>> Jini.
>>>>
>>>>>> -----Original Message-----
>>>>>> From: John Baldwin <jhb@FreeBSD.org>
>>>>>> Sent: Thursday, October 12, 2023 10:48 PM
>>>>>> To: Simon Marchi <simon.marchi@polymtl.ca>;
>>>>>> gdb-patches@sourceware.org
>>>>>> Cc: Felix <felix.willgerodt@intel.com>; George, Jini Susan
>>>>>> <JiniSusan.George@amd.com>
>>>>>> Subject: Re: [RFC 00/13] Proposal for a new NT_X86_CPUID core dump
>>>>>> note
>>>>>>
>>>>>> Caution: This message originated from an External Source. Use proper
>>>>>> caution when opening attachments, clicking links, or responding.
>>>>>>
>>>>>>
>>>>>> On 10/12/23 7:33 AM, Simon Marchi wrote:
>>>>>>>
>>>>>>>
>>>>>>> On 2023-10-09 14:36, John Baldwin wrote:
>>>>>>>> One of the shortcomings of the previous XSAVE patch series is that
>>>>>>>> it depends on heuristics based on the total XSAVE register set
>>>>>>>> size and
>>>>>>>> XCR0 mask to infer layouts of the various register blocks for core
>>>>>>>> dumps.  This series introduces a new x86-specific core dump note
>>>>>>>> intended to supplant these heuristics by storing the raw CPUID
>>>>>>>> leaves describing the XSAVE layout in core dumps.
>>>>>>>>
>>>>>>>> This series proposes a new core dump note, NT_X86_CPUID (0x205),
>>>>>>>> which contains an array of structures.  Each structure describes
>>>>>>>> an invidual CPUID sub-leaf containing both the inputs to CPUID
>>>>>>>> (%eax and
>>>>>>>> %ecx) and the outputs (%eax, %ebx, %ecx, and %edx) in a format
>>>>>>>> roughly matching the follow C structure:
>>>>>>>>
>>>>>>>> struct cpuid_leaf
>>>>>>>> {
>>>>>>>>        uint32_t leaf;
>>>>>>>>        uint32_t subleaf;
>>>>>>>>        uint32_t eax;
>>>>>>>>        uint32_t ebx;
>>>>>>>>        uint32_t ecx;
>>>>>>>>        uint32_t edx;
>>>>>>>> };
>>>>>>>>
>>>>>>>> This format is not XSAVE-specific and implementations could choose
>>>>>>>> to add additional CPUID leaves to this structure if needed in the future.
>>>>>>>> Consumers of this note should lookup the value of required leaves
>>>>>>>> and ignore any unneeded leaves.
>>>>>>>>
>>>>>>>> An alternate approach might be to write out a more XSAVE-specific
>>>>>>>> note that is an array containing the offset and size of each XSAVE region.
>>>>>>>
>>>>>>> Something I thought about: I think there are asymmetrical x86
>>>>>>> processors now, with "big" and "little" cores, not sure how they
>>>>>>> call them.  For them, is it possible for the XSAVE layout to be
>>>>>>> different per core, for instance some cores supporting AVX512 and
>>>>>>> some cores not?  And therefore, will we eventually need to include
>>>>>>> CPUID / XSAVE information for more than one CPU core type in the core
>>> file notes?
>>>>>>
>>>>>> (The Intel names are "P" (performance) and "E" (energy-efficient)
>>>>>> cores
>>>>>> btw)
>>>>>>
>>>>>> I have no idea if they use the same or different XSAVE layouts.  It
>>>>>> would seem to be a royal pain if they did as everytime a user thread
>>>>>> migrated between cores the OS would have to convert the XSAVE block
>> >from one layout to the other.
>>>>>> FreeBSD does not have any notion of multiple layouts today and just
>>>>>> assumes that the XSAVE layout is uniform across all cores in a
>>>>>> system.  I believe Linux's kernel does the same from my reading.
>>>>>>
>>>>>> --
>>>>>> John Baldwin
>>>>
>>>
>>> --
>>> John Baldwin
> 

-- 
John Baldwin

RE: [RFC 00/13] Proposal for a new NT_X86_CPUID core dump note

[George, Jini Susan]

[Public]

If tomorrow, we have to deal with varying xsave layouts/ cache or any other information for different coresets, then we would need the mapping from LWPIDs to coresets.

>One way you could store that would be to have a per-thread note storing the
>logical CPU mask (or cpuset, not sure which term Linux uses, FreeBSD uses
>cpuset) for each thread, but then you'd need perhaps some other note mapping
>logical CPU IDs to other CPU data.
>(For example, if you did have a separate NT_X86_CPUID_E_CORE, you could
>have some other note that was a short table mapping CPU IDs to CPUID notes
>(e.g. cpu 0 is NT_X86_CPUID, cpu 1 is NT_X86_CPUID, cpu 2 is
>NT_X86_CPUID_E_CORE).

I was thinking of having the per-thread note storing the last executed cpu for the thread -- not sure if the cpuset will be needed, but I could be missing something. But yes, we will need 3 extra note types for P/E scenarios. (the per process E note, the per process cpu ID to coreset mapping note and the per thread last executed cpu/ cpuset note).

So, yes, since we can add other required information as need be as extra notes in the future, I feel none of this should hinder the current patch with going through.

Thanks,
Jini.

>>-----Original Message-----
>>From: John Baldwin <jhb@FreeBSD.org>
>>Sent: Friday, October 27, 2023 8:23 AM
>>To: George, Jini Susan <JiniSusan.George@amd.com>; Simon Marchi
>><simon.marchi@polymtl.ca>; gdb-patches@sourceware.org
>>Cc: Felix <felix.willgerodt@intel.com>; Balasubrmanian, Vignesh
>><Vignesh.Balasubrmanian@amd.com>
>>Subject: Re: [RFC 00/13] Proposal for a new NT_X86_CPUID core dump note
>>
>>Caution: This message originated from an External Source. Use proper caution
>>when opening attachments, clicking links, or responding.
>>
>>
>>On 10/26/23 9:18 AM, George, Jini Susan wrote:
>>> [AMD Official Use Only - General]
>>>
>>> I think it might be better to go with different NT_ values for the P and the E
>>cores. We will also need to put in additional mapping information as to which
>>core/ cpu id belongs to which core set (P or E).
>>
>>Hmm, do you mean mapping thread (LWP) IDs to specific core sets?
>>Right now core dumps don't really include the mapping of threads to individual
>>CPU cores, in part because threads can migrate between cores (subject to
>>administrative and application constraints).
>>I'm not really sure there is a use case for storing that information.
>>
>>One way you could store that would be to have a per-thread note storing the
>>logical CPU mask (or cpuset, not sure which term Linux uses, FreeBSD uses
>>cpuset) for each thread, but then you'd need perhaps some other note mapping
>>logical CPU IDs to other CPU data.
>>(For example, if you did have a separate NT_X86_CPUID_E_CORE, you could
>>have some other note that was a short table mapping CPU IDs to CPUID notes
>>(e.g. cpu 0 is NT_X86_CPUID, cpu 1 is NT_X86_CPUID, cpu 2 is
>>NT_X86_CPUID_E_CORE).
>>
>>However, I'm not sure it makes sense to store all of that now without a use
>>case.  I do think though that this does show there are ways to extend the
>>information in the future should a use case arrive.
>>
>>> Thanks
>>> Jini.
>>>
>>>>> -----Original Message-----
>>>>> From: John Baldwin <jhb@FreeBSD.org>
>>>>> Sent: Tuesday, October 17, 2023 6:06 AM
>>>>> To: George, Jini Susan <JiniSusan.George@amd.com>; Simon Marchi
>>>>> <simon.marchi@polymtl.ca>; gdb-patches@sourceware.org
>>>>> Cc: Felix <felix.willgerodt@intel.com>; Balasubrmanian, Vignesh
>>>>> <Vignesh.Balasubrmanian@amd.com>
>>>>> Subject: Re: [RFC 00/13] Proposal for a new NT_X86_CPUID core dump
>>>>> note
>>>>>
>>>>> Caution: This message originated from an External Source. Use proper
>>>>> caution when opening attachments, clicking links, or responding.
>>>>>
>>>>>
>>>>> On 10/13/23 2:38 AM, George, Jini Susan wrote:
>>>>>> [AMD Official Use Only - General]
>>>>>>
>>>>>> I think that even if the xsave layout remains uniform across the
>>>>>> cores of a
>>>>> system, since we are trying to design an extensible .note section
>>>>> which can possibly hold all kinds of CPUID information, we might
>>>>> want to consider various scenarios wherein the CPUID information
>>>>> might differ across cores (esp for big.LITTLE/(P/E)), like the cache
>>>>> information, perhaps ? It might be more prudent to include the coretype
>>information also in such cases ?
>>>>>
>>>>> It's certainly occurred to me that it might be prudent to include
>>>>> the full complement of CPUID leaves even in the initial version of
>>>>> this note should that information prove useful in the future.
>>>>>
>>>>> However, I'm not quite sure how we should "name" the different CPUID
>>>>> leaf sets in this case (e.g. the set for E cores vs the set for P
>>>>> cores).  In particular, it's not quite clear to me how many sets
>>>>> future systems might have?  One route is to use additional NT_
>>>>> values for different sets, (e.g. a NT_X86_CPUID_E_CORE or some such
>>>>> if the "default" set is for the P core).  We could also embed an identifier at
>>the start of a note, e.g.
>>>>> a 32-bit integer, and dump one note per set, so you have always have
>>>>> a note for set 0, but in a system with E cores you might have a set
>>>>> 1 where the convention would be P cores are set 0, and E cores are
>>>>> set 1.  Future systems with multiple core types would have to decide
>>>>> what type of mapping to use between core types and set IDs.  One
>>>>> issue with the second approach is if you want these notes accessible
>>>>> via ptrace() and not just in cores, it wouldn't be clear how to request the
>>leaves for set N if they all share the NT_* value.
>>>>>
>>>>>> Rgds
>>>>>> Jini.
>>>>>>
>>>>>>>> -----Original Message-----
>>>>>>>> From: John Baldwin <jhb@FreeBSD.org>
>>>>>>>> Sent: Thursday, October 12, 2023 10:48 PM
>>>>>>>> To: Simon Marchi <simon.marchi@polymtl.ca>;
>>>>>>>> gdb-patches@sourceware.org
>>>>>>>> Cc: Felix <felix.willgerodt@intel.com>; George, Jini Susan
>>>>>>>> <JiniSusan.George@amd.com>
>>>>>>>> Subject: Re: [RFC 00/13] Proposal for a new NT_X86_CPUID core
>>>>>>>> dump note
>>>>>>>>
>>>>>>>> Caution: This message originated from an External Source. Use
>>>>>>>> proper caution when opening attachments, clicking links, or responding.
>>>>>>>>
>>>>>>>>
>>>>>>>> On 10/12/23 7:33 AM, Simon Marchi wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On 2023-10-09 14:36, John Baldwin wrote:
>>>>>>>>>> One of the shortcomings of the previous XSAVE patch series is
>>>>>>>>>> that it depends on heuristics based on the total XSAVE register
>>>>>>>>>> set size and
>>>>>>>>>> XCR0 mask to infer layouts of the various register blocks for
>>>>>>>>>> core dumps.  This series introduces a new x86-specific core
>>>>>>>>>> dump note intended to supplant these heuristics by storing the
>>>>>>>>>> raw CPUID leaves describing the XSAVE layout in core dumps.
>>>>>>>>>>
>>>>>>>>>> This series proposes a new core dump note, NT_X86_CPUID
>>>>>>>>>> (0x205), which contains an array of structures.  Each structure
>>>>>>>>>> describes an invidual CPUID sub-leaf containing both the inputs
>>>>>>>>>> to CPUID (%eax and
>>>>>>>>>> %ecx) and the outputs (%eax, %ebx, %ecx, and %edx) in a format
>>>>>>>>>> roughly matching the follow C structure:
>>>>>>>>>>
>>>>>>>>>> struct cpuid_leaf
>>>>>>>>>> {
>>>>>>>>>>        uint32_t leaf;
>>>>>>>>>>        uint32_t subleaf;
>>>>>>>>>>        uint32_t eax;
>>>>>>>>>>        uint32_t ebx;
>>>>>>>>>>        uint32_t ecx;
>>>>>>>>>>        uint32_t edx;
>>>>>>>>>> };
>>>>>>>>>>
>>>>>>>>>> This format is not XSAVE-specific and implementations could
>>>>>>>>>> choose to add additional CPUID leaves to this structure if needed in
>>the future.
>>>>>>>>>> Consumers of this note should lookup the value of required
>>>>>>>>>> leaves and ignore any unneeded leaves.
>>>>>>>>>>
>>>>>>>>>> An alternate approach might be to write out a more
>>>>>>>>>> XSAVE-specific note that is an array containing the offset and size of
>>each XSAVE region.
>>>>>>>>>
>>>>>>>>> Something I thought about: I think there are asymmetrical x86
>>>>>>>>> processors now, with "big" and "little" cores, not sure how they
>>>>>>>>> call them.  For them, is it possible for the XSAVE layout to be
>>>>>>>>> different per core, for instance some cores supporting AVX512
>>>>>>>>> and some cores not?  And therefore, will we eventually need to
>>>>>>>>> include CPUID / XSAVE information for more than one CPU core
>>>>>>>>> type in the core
>>>>> file notes?
>>>>>>>>
>>>>>>>> (The Intel names are "P" (performance) and "E" (energy-efficient)
>>>>>>>> cores
>>>>>>>> btw)
>>>>>>>>
>>>>>>>> I have no idea if they use the same or different XSAVE layouts.
>>>>>>>> It would seem to be a royal pain if they did as everytime a user
>>>>>>>> thread migrated between cores the OS would have to convert the
>>>>>>>> XSAVE block
>>>> >from one layout to the other.
>>>>>>>> FreeBSD does not have any notion of multiple layouts today and
>>>>>>>> just assumes that the XSAVE layout is uniform across all cores in
>>>>>>>> a system.  I believe Linux's kernel does the same from my reading.
>>>>>>>>
>>>>>>>> --
>>>>>>>> John Baldwin
>>>>>>
>>>>>
>>>>> --
>>>>> John Baldwin
>>>
>>
>>--
>>John Baldwin

Re: [RFC 00/13] Proposal for a new NT_X86_CPUID core dump note

[John Baldwin]

On 10/27/23 4:11 AM, George, Jini Susan wrote:
> [Public]
> 
> If tomorrow, we have to deal with varying xsave layouts/ cache or any other information for different coresets, then we would need the mapping from LWPIDs to coresets.
> 
>> One way you could store that would be to have a per-thread note storing the
>> logical CPU mask (or cpuset, not sure which term Linux uses, FreeBSD uses
>> cpuset) for each thread, but then you'd need perhaps some other note mapping
>> logical CPU IDs to other CPU data.
>> (For example, if you did have a separate NT_X86_CPUID_E_CORE, you could
>> have some other note that was a short table mapping CPU IDs to CPUID notes
>> (e.g. cpu 0 is NT_X86_CPUID, cpu 1 is NT_X86_CPUID, cpu 2 is
>> NT_X86_CPUID_E_CORE).
> 
> I was thinking of having the per-thread note storing the last executed cpu for the thread -- not sure if the cpuset will be needed, but I could be missing something. But yes, we will need 3 extra note types for P/E scenarios. (the per process E note, the per process cpu ID to coreset mapping note and the per thread last executed cpu/ cpuset note).
> 
> So, yes, since we can add other required information as need be as extra notes in the future, I feel none of this should hinder the current patch with going through.

Ok, sounds good.  Please cc me once you post the Linux kernel patches.

I opened a thread over on the LLVM discourse with the LLDB folks on this topic and
the only feedback so far has been positive:

https://discourse.llvm.org/t/proposal-for-a-new-nt-x86-cpuid-core-dump-note/74458

-- 
John Baldwin