[PATCH v3 00/13] Handle variable XSAVE layouts

[PATCH v3 00/13] Handle variable XSAVE layouts

[John Baldwin]

Changes since V2:

- Pulled in some of the changes from Intel's branch Felix pointed me
  at, in particular gdbserver support.  However, relative to that
  branch these patches make the following changes:

  - The i387_* structs and class remain in gdbserver/i387-fp.cc
    rather than moving to gdbsupport/.

  - Rather than invoking cpuid each time an XSAVE area is parsed,
    the x86_xsave_layout structure is used to hold offsets and
    CPUID is only invoked the first time NT_X86_XSTATE is probed
    with the offsets cached for later use.

  - I did not update the ChangeLog bits of these log messages, but
    probably they can be dropped for the Intel commits as GDB
    commits generally do not include these now?

- Added Linux support both for gdbarches and the x86 native targets.
  I wasn't sure if PT_GETREGSET on Linux provides a way to query the
  size of the XSAVE register set (on FreeBSD PT_GETREGSET returns
  the register set's size in iov_len of the passed-in iovec if the
  original iovec has a NULL pointer and zero length), so I used
  cpuid leaf 0xd subleaf 0x0 to query the size of the register set
  for the native targets as well as for the Linux gdbserver support.

Note that this still depends on the size and xcr0 mask heuristic for
Linux and FreeBSD core dumps to determine the layout (and I have not
added any additional layouts as I wasn't sure if Jini was intending to
add additional AMD-specific layouts).  I'd kind of like to land this
series before doing a followup to flesh out a new core dump note.

I think for the new core dump note what I would propose is a simple
array of CPUID results for sub-leaves of the 0xd leaf (as a
NT_X86_XSTATE_CPUID or the like) where each entry in the array
contained the subleaf as well as eax, ebx, ecx, and edx results.  This
note might even eventually permit handling "compact" XSTATE in future
core dumps rather than only "standard".

I have tested this on both an AMD Ryzen 9 5900X and Intel Core
i7-8650U on FreeBSD/amd64 as well as on a Linux/x86-64 VM on the Intel
system.  I also tested FreeBSD/i386 in a VM on the AMD system.

Aleksandar Paunovic (2):
  gdbserver: Refactor the legacy region within the xsave struct
  gdbserver: Read offsets of the XSAVE extended region via CPUID

John Baldwin (11):
  x86: Add an x86_xsave_layout structure to handle variable XSAVE
    layouts.
  core: Support fetching TARGET_OBJECT_X86_XSAVE_LAYOUT from
    architectures.
  nat/x86-cpuid.h: Add x86_cpuid_count wrapper around __get_cpuid_count.
  x86 nat: Add helper functions to save the XSAVE layout for the host.
  gdb: Update x86 FreeBSD architectures to support XSAVE layouts.
  gdb: Support XSAVE layouts for the current host in the FreeBSD x86
    targets.
  gdb: Update x86 Linux architectures to support XSAVE layouts.
  gdb: Support XSAVE layouts for the current host in the Linux x86
    targets.
  gdb: Use x86_xstate_layout to parse the XSAVE extended state area.
  gdbserver: Add a function to set the XSAVE mask and size.
  x86: Remove X86_XSTATE_SIZE and related constants.

 gdb/amd64-fbsd-nat.c       |  40 +--
 gdb/amd64-fbsd-tdep.c      |   8 +-
 gdb/amd64-linux-nat.c      |   6 +-
 gdb/amd64-linux-tdep.c     |   8 +-
 gdb/configure.nat          |   8 +-
 gdb/corelow.c              |  22 ++
 gdb/gdbarch-components.py  |  13 +
 gdb/gdbarch-gen.h          |  10 +
 gdb/gdbarch.c              |  32 +++
 gdb/i386-fbsd-nat.c        |  39 +--
 gdb/i386-fbsd-tdep.c       |  37 ++-
 gdb/i386-fbsd-tdep.h       |   6 +
 gdb/i386-linux-nat.c       |   8 +-
 gdb/i386-linux-tdep.c      |  34 ++-
 gdb/i386-linux-tdep.h      |   6 +
 gdb/i386-tdep.c            |  36 ++-
 gdb/i386-tdep.h            |   3 +
 gdb/i387-tdep.c            | 493 ++++++++++++++++++++++++-------------
 gdb/i387-tdep.h            |   8 +
 gdb/nat/x86-cpuid.h        |  27 ++
 gdb/nat/x86-xstate.c       |  65 +++++
 gdb/nat/x86-xstate.h       |  35 +++
 gdb/target.h               |   2 +
 gdb/x86-fbsd-nat.c         |  51 ++++
 gdb/x86-fbsd-nat.h         |  29 ++-
 gdb/x86-linux-nat.c        |  33 +++
 gdb/x86-linux-nat.h        |  11 +
 gdbserver/configure.srv    |  12 +-
 gdbserver/i387-fp.cc       | 312 ++++++++++++++---------
 gdbserver/i387-fp.h        |   2 +-
 gdbserver/linux-x86-low.cc |  10 +-
 gdbsupport/x86-xstate.h    |  69 ++++--
 32 files changed, 1067 insertions(+), 408 deletions(-)
 create mode 100644 gdb/nat/x86-xstate.c
 create mode 100644 gdb/nat/x86-xstate.h

-- 
2.34.1

[PATCH v3 01/13] x86: Add an x86_xsave_layout structure to handle variable XSAVE layouts.

[John Baldwin]

The standard layout of the XSAVE extended state area consists of three
regions.  The first 512 bytes (legacy region) match the layout of the
FXSAVE instruction including floating point registers, MMX registers,
and SSE registers.  The next 64 bytes (XSAVE header) contains a header
with a fixed layout.  The final region (extended region) contains zero
or more optional state components.  Examples of these include the
upper 128 bits of YMM registers for AVX.

These optional state components generally have an
architecturally-fixed size, but they are not assigned architectural
offsets in the extended region.  Instead, processors provide
additional CPUID leafs describing the size and offset of each
component in the "standard" layout for a given CPU.  (There is also a
"compact" format which uses an alternate layout, but existing OS's
currently export the "standard" layout when exporting XSAVE data via
ptrace() and core dumps.)

To date, GDB has assumed the layout used on current Intel processors
for state components in the extended region and hardcoded those
offsets in the tables in i387-tdep.c and i387-fp.cc.  However, this
fails on recent AMD processors which use a different layout.
Specifically, AMD Ryzen 9 processors at least do not leave space for
the MPX register set in between the AVX and AVX512 register sets.  It
is not known if other AMD processors are effected, but seems probable.

To rectify this, add an x86_xsave_layout structure which contains the
total size of the XSAVE extended state area as well as the offset of
each known optional state component.  This structure is stored in
i386_gdbarch_tdep and is fetched from the current target in
i386_gdbarch_init as a TARGET_OBJECT_X86_XSAVE_LAYOUT object.

Subsequent commits will modify XSAVE parsing to use x86_xsave_layout.

Co-authored-by: Aleksandar Paunovic <aleksandar.paunovic@intel.com>
---
 gdb/i386-tdep.c         | 36 +++++++++++++++++++++++++-
 gdb/i386-tdep.h         |  3 +++
 gdb/target.h            |  2 ++
 gdbsupport/x86-xstate.h | 57 +++++++++++++++++++++++++++++++++++------
 4 files changed, 89 insertions(+), 9 deletions(-)

diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
index 8501e12e241..be9c4c7e41e 100644
--- a/gdb/i386-tdep.c
+++ b/gdb/i386-tdep.c
@@ -8439,6 +8439,21 @@ i386_type_align (struct gdbarch *gdbarch, struct type *type)
 }
 
 \f
+/* Fetch the XSAVE layout for the current target.  */
+
+static struct x86_xsave_layout
+i386_fetch_xsave_layout ()
+{
+  struct x86_xsave_layout layout;
+  LONGEST len = target_read (current_inferior ()->top_target (),
+			     TARGET_OBJECT_X86_XSAVE_LAYOUT, nullptr,
+			     (gdb_byte *) &layout, 0, sizeof (layout));
+  if (len != sizeof (layout))
+    return {};
+
+  return layout;
+}
+
 /* Note: This is called for both i386 and amd64.  */
 
 static struct gdbarch *
@@ -8446,13 +8461,30 @@ i386_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
 {
   struct gdbarch *gdbarch;
   const struct target_desc *tdesc;
+  struct x86_xsave_layout xsave_layout;
   int mm0_regnum;
   int ymm0_regnum;
   int bnd0_regnum;
   int num_bnd_cooked;
 
+  xsave_layout = i386_fetch_xsave_layout ();
+
   /* If there is already a candidate, use it.  */
-  arches = gdbarch_list_lookup_by_info (arches, &info);
+  for (arches = gdbarch_list_lookup_by_info (arches, &info);
+       arches != NULL;
+       arches = gdbarch_list_lookup_by_info (arches->next, &info))
+    {
+      /* Check that the XSAVE layout of ARCHES matches the layout for
+	 the current target.  */
+      i386_gdbarch_tdep *other_tdep
+	= (i386_gdbarch_tdep *) gdbarch_tdep (arches->gdbarch);
+
+      if (other_tdep->xsave_layout != xsave_layout)
+	continue;
+
+      break;
+    }
+
   if (arches != NULL)
     return arches->gdbarch;
 
@@ -8705,6 +8737,8 @@ i386_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
       gdbarch_free (gdbarch);
       return NULL;
     }
+  if (tdep->xcr0 != 0)
+    tdep->xsave_layout = xsave_layout;
 
   num_bnd_cooked = (tdep->bnd0r_regnum > 0 ? I387_NUM_BND_REGS : 0);
 
diff --git a/gdb/i386-tdep.h b/gdb/i386-tdep.h
index a8067cf6b6c..a2c66ecfed1 100644
--- a/gdb/i386-tdep.h
+++ b/gdb/i386-tdep.h
@@ -23,6 +23,7 @@
 #include "gdbarch.h"
 #include "infrun.h"
 #include "expression.h"
+#include "gdbsupport/x86-xstate.h"
 
 struct frame_info;
 struct gdbarch;
@@ -145,6 +146,8 @@ struct i386_gdbarch_tdep : gdbarch_tdep
   /* Offset of XCR0 in XSAVE extended state.  */
   int xsave_xcr0_offset = 0;
 
+  x86_xsave_layout xsave_layout;
+
   /* Register names.  */
   const char * const *register_names = nullptr;
 
diff --git a/gdb/target.h b/gdb/target.h
index f77dbf05113..a5ec3bba496 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -204,6 +204,8 @@ enum target_object
   TARGET_OBJECT_FREEBSD_VMMAP,
   /* FreeBSD process strings.  */
   TARGET_OBJECT_FREEBSD_PS_STRINGS,
+  /* x86 XSAVE area layout.  */
+  TARGET_OBJECT_X86_XSAVE_LAYOUT,
   /* Possible future objects: TARGET_OBJECT_FILE, ...  */
 };
 
diff --git a/gdbsupport/x86-xstate.h b/gdbsupport/x86-xstate.h
index d4845243b4b..9e2dd2155be 100644
--- a/gdbsupport/x86-xstate.h
+++ b/gdbsupport/x86-xstate.h
@@ -20,23 +20,64 @@
 #ifndef COMMON_X86_XSTATE_H
 #define COMMON_X86_XSTATE_H
 
+/* The extended state feature IDs in the state component bitmap.  */
+#define X86_XSTATE_X87_ID	0
+#define X86_XSTATE_SSE_ID	1
+#define X86_XSTATE_AVX_ID	2
+#define X86_XSTATE_BNDREGS_ID	3
+#define X86_XSTATE_BNDCFG_ID	4
+#define X86_XSTATE_K_ID		5
+#define X86_XSTATE_ZMM_H_ID	6
+#define X86_XSTATE_ZMM_ID	7
+#define X86_XSTATE_PT_ID	8
+#define X86_XSTATE_PKRU_ID	9
+
 /* The extended state feature bits.  */
-#define X86_XSTATE_X87		(1ULL << 0)
-#define X86_XSTATE_SSE		(1ULL << 1)
-#define X86_XSTATE_AVX		(1ULL << 2)
-#define X86_XSTATE_BNDREGS	(1ULL << 3)
-#define X86_XSTATE_BNDCFG	(1ULL << 4)
+#define X86_XSTATE_X87		(1ULL << X86_XSTATE_X87_ID)
+#define X86_XSTATE_SSE		(1ULL << X86_XSTATE_SSE_ID)
+#define X86_XSTATE_AVX		(1ULL << X86_XSTATE_AVX_ID)
+#define X86_XSTATE_BNDREGS	(1ULL << X86_XSTATE_BNDREGS_ID)
+#define X86_XSTATE_BNDCFG	(1ULL << X86_XSTATE_BNDCFG_ID)
 #define X86_XSTATE_MPX		(X86_XSTATE_BNDREGS | X86_XSTATE_BNDCFG)
 
 /* AVX 512 adds three feature bits.  All three must be enabled.  */
-#define X86_XSTATE_K		(1ULL << 5)
-#define X86_XSTATE_ZMM_H	(1ULL << 6)
-#define X86_XSTATE_ZMM		(1ULL << 7)
+#define X86_XSTATE_K		(1ULL << X86_XSTATE_K_ID)
+#define X86_XSTATE_ZMM_H	(1ULL << X86_XSTATE_ZMM_H_ID)
+#define X86_XSTATE_ZMM		(1ULL << X86_XSTATE_ZMM_ID)
 #define X86_XSTATE_AVX512	(X86_XSTATE_K | X86_XSTATE_ZMM_H \
 				 | X86_XSTATE_ZMM)
 
 #define X86_XSTATE_PKRU		(1ULL << 9)
 
+/* Size and offsets of register states in the XSAVE area extended
+   region.  Offsets are set to 0 to indicate the absence of the
+   associated registers.  */
+
+struct x86_xsave_layout
+{
+  int sizeof_xsave = 0;
+  int avx_offset = 0;
+  int bndregs_offset = 0;
+  int bndcfg_offset = 0;
+  int k_offset = 0;
+  int zmm_h_offset = 0;
+  int zmm_offset = 0;
+  int pkru_offset = 0;
+
+  bool operator!= (const x86_xsave_layout &other)
+  {
+    return sizeof_xsave != other.sizeof_xsave
+      || avx_offset != other.avx_offset
+      || bndregs_offset != other.bndregs_offset
+      || bndcfg_offset != other.bndcfg_offset
+      || k_offset != other.k_offset
+      || zmm_h_offset != other.zmm_h_offset
+      || zmm_offset != other.zmm_offset
+      || pkru_offset != other.pkru_offset;
+  }
+};
+
+
 /* Supported mask and size of the extended state.  */
 #define X86_XSTATE_X87_MASK	X86_XSTATE_X87
 #define X86_XSTATE_SSE_MASK	(X86_XSTATE_X87 | X86_XSTATE_SSE)
-- 
2.34.1

[PATCH v3 02/13] core: Support fetching TARGET_OBJECT_X86_XSAVE_LAYOUT from architectures.

[John Baldwin]

Add gdbarch_core_xfer_x86_xsave_layout to fetch the x86 XSAVE layout
structure from a core file.

Current OS's do not export the offsets of XSAVE state components in
core dumps, so provide an i387_set_xsave_layout helper function to set
offsets based on known combinations of XCR0 masks and total state
sizes.  Eventually when core dumps do contain this information this
function should only be used as a fall back for older core dumps.
---
 gdb/corelow.c             | 22 +++++++++++++++++
 gdb/gdbarch-components.py | 13 ++++++++++
 gdb/gdbarch-gen.h         | 10 ++++++++
 gdb/gdbarch.c             | 32 ++++++++++++++++++++++++
 gdb/i387-tdep.c           | 51 +++++++++++++++++++++++++++++++++++++++
 gdb/i387-tdep.h           |  7 ++++++
 6 files changed, 135 insertions(+)

diff --git a/gdb/corelow.c b/gdb/corelow.c
index 8c33fb7ebb2..bbe24e2ab2c 100644
--- a/gdb/corelow.c
+++ b/gdb/corelow.c
@@ -1026,6 +1026,28 @@ core_target::xfer_partial (enum target_object object, const char *annex,
 	}
       return TARGET_XFER_E_IO;
 
+    case TARGET_OBJECT_X86_XSAVE_LAYOUT:
+      if (readbuf)
+	{
+	  if (m_core_gdbarch != nullptr
+	      && gdbarch_core_xfer_x86_xsave_layout_p (m_core_gdbarch))
+	    {
+	      LONGEST l = gdbarch_core_xfer_x86_xsave_layout  (m_core_gdbarch,
+							       readbuf, offset,
+							       len);
+
+	      if (l >= 0)
+		{
+		  *xfered_len = l;
+		  if (l == 0)
+		    return TARGET_XFER_EOF;
+		  else
+		    return TARGET_XFER_OK;
+		}
+	    }
+	}
+      return TARGET_XFER_E_IO;
+
     default:
       return this->beneath ()->xfer_partial (object, annex, readbuf,
 					     writebuf, offset, len,
diff --git a/gdb/gdbarch-components.py b/gdb/gdbarch-components.py
index e8f20c83ff0..a90e8df36cb 100644
--- a/gdb/gdbarch-components.py
+++ b/gdb/gdbarch-components.py
@@ -1584,6 +1584,19 @@ of bytes read (zero indicates EOF, a negative value indicates failure).
     invalid=True,
 )
 
+Method(
+    comment="""
+Read offset OFFSET of TARGET_OBJECT_X86_XSAVE_LAYOUT from core file
+into buffer READBUF with length LEN.  Return the number of bytes read
+(zero indicates EOF, a negative value indicates failure).
+""",
+    type="LONGEST",
+    name="core_xfer_x86_xsave_layout",
+    params=[("gdb_byte *", "readbuf"), ("ULONGEST", "offset"), ("ULONGEST", "len")],
+    predicate=True,
+    invalid=True,
+)
+
 Value(
     comment="""
 BFD target to use when generating a core file.
diff --git a/gdb/gdbarch-gen.h b/gdb/gdbarch-gen.h
index 882b9057b1a..d02935becae 100644
--- a/gdb/gdbarch-gen.h
+++ b/gdb/gdbarch-gen.h
@@ -921,6 +921,16 @@ typedef LONGEST (gdbarch_core_xfer_siginfo_ftype) (struct gdbarch *gdbarch, gdb_
 extern LONGEST gdbarch_core_xfer_siginfo (struct gdbarch *gdbarch, gdb_byte *readbuf, ULONGEST offset, ULONGEST len);
 extern void set_gdbarch_core_xfer_siginfo (struct gdbarch *gdbarch, gdbarch_core_xfer_siginfo_ftype *core_xfer_siginfo);
 
+/* Read offset OFFSET of TARGET_OBJECT_X86_XSAVE_LAYOUT from core file
+   into buffer READBUF with length LEN.  Return the number of bytes read
+   (zero indicates EOF, a negative value indicates failure). */
+
+extern bool gdbarch_core_xfer_x86_xsave_layout_p (struct gdbarch *gdbarch);
+
+typedef LONGEST (gdbarch_core_xfer_x86_xsave_layout_ftype) (struct gdbarch *gdbarch, gdb_byte *readbuf, ULONGEST offset, ULONGEST len);
+extern LONGEST gdbarch_core_xfer_x86_xsave_layout (struct gdbarch *gdbarch, gdb_byte *readbuf, ULONGEST offset, ULONGEST len);
+extern void set_gdbarch_core_xfer_x86_xsave_layout (struct gdbarch *gdbarch, gdbarch_core_xfer_x86_xsave_layout_ftype *core_xfer_x86_xsave_layout);
+
 /* BFD target to use when generating a core file. */
 
 extern bool gdbarch_gcore_bfd_target_p (struct gdbarch *gdbarch);
diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c
index a588bdef61a..d8accffd7aa 100644
--- a/gdb/gdbarch.c
+++ b/gdb/gdbarch.c
@@ -176,6 +176,7 @@ struct gdbarch
   gdbarch_core_pid_to_str_ftype *core_pid_to_str;
   gdbarch_core_thread_name_ftype *core_thread_name;
   gdbarch_core_xfer_siginfo_ftype *core_xfer_siginfo;
+  gdbarch_core_xfer_x86_xsave_layout_ftype *core_xfer_x86_xsave_layout;
   const char * gcore_bfd_target;
   int vtable_function_descriptors;
   int vbit_in_delta;
@@ -532,6 +533,7 @@ verify_gdbarch (struct gdbarch *gdbarch)
   /* Skip verify of core_pid_to_str, has predicate.  */
   /* Skip verify of core_thread_name, has predicate.  */
   /* Skip verify of core_xfer_siginfo, has predicate.  */
+  /* Skip verify of core_xfer_x86_xsave_layout, has predicate.  */
   /* Skip verify of gcore_bfd_target, has predicate.  */
   /* Skip verify of vtable_function_descriptors, invalid_p == 0 */
   /* Skip verify of vbit_in_delta, invalid_p == 0 */
@@ -1126,6 +1128,12 @@ gdbarch_dump (struct gdbarch *gdbarch, struct ui_file *file)
   gdb_printf (file,
                       "gdbarch_dump: core_xfer_siginfo = <%s>\n",
                       host_address_to_string (gdbarch->core_xfer_siginfo));
+  gdb_printf (file,
+                      "gdbarch_dump: gdbarch_core_xfer_x86_xsave_layout_p() = %d\n",
+                      gdbarch_core_xfer_x86_xsave_layout_p (gdbarch));
+  gdb_printf (file,
+                      "gdbarch_dump: core_xfer_x86_xsave_layout = <%s>\n",
+                      host_address_to_string (gdbarch->core_xfer_x86_xsave_layout));
   gdb_printf (file,
                       "gdbarch_dump: gdbarch_gcore_bfd_target_p() = %d\n",
                       gdbarch_gcore_bfd_target_p (gdbarch));
@@ -3864,6 +3872,30 @@ set_gdbarch_core_xfer_siginfo (struct gdbarch *gdbarch,
   gdbarch->core_xfer_siginfo = core_xfer_siginfo;
 }
 
+bool
+gdbarch_core_xfer_x86_xsave_layout_p (struct gdbarch *gdbarch)
+{
+  gdb_assert (gdbarch != NULL);
+  return gdbarch->core_xfer_x86_xsave_layout != NULL;
+}
+
+LONGEST
+gdbarch_core_xfer_x86_xsave_layout (struct gdbarch *gdbarch, gdb_byte *readbuf, ULONGEST offset, ULONGEST len)
+{
+  gdb_assert (gdbarch != NULL);
+  gdb_assert (gdbarch->core_xfer_x86_xsave_layout != NULL);
+  if (gdbarch_debug >= 2)
+    gdb_printf (gdb_stdlog, "gdbarch_core_xfer_x86_xsave_layout called\n");
+  return gdbarch->core_xfer_x86_xsave_layout (gdbarch, readbuf, offset, len);
+}
+
+void
+set_gdbarch_core_xfer_x86_xsave_layout (struct gdbarch *gdbarch,
+                                        gdbarch_core_xfer_x86_xsave_layout_ftype core_xfer_x86_xsave_layout)
+{
+  gdbarch->core_xfer_x86_xsave_layout = core_xfer_x86_xsave_layout;
+}
+
 bool
 gdbarch_gcore_bfd_target_p (struct gdbarch *gdbarch)
 {
diff --git a/gdb/i387-tdep.c b/gdb/i387-tdep.c
index f056ea59347..80a130fdeff 100644
--- a/gdb/i387-tdep.c
+++ b/gdb/i387-tdep.c
@@ -898,6 +898,57 @@ static int xsave_pkeys_offset[] =
   (xsave + xsave_pkeys_offset[regnum - I387_PKRU_REGNUM (tdep)])
 
 
+/* See i387-tdep.h.  */
+
+void
+i387_set_xsave_layout (uint64_t xcr0, size_t xsave_size,
+		       x86_xsave_layout *layout)
+{
+  layout->sizeof_xsave = xsave_size;
+  if (HAS_PKRU(xcr0) && xsave_size == 2696)
+    {
+      /* Intel CPUs supporting PKRU.  */
+      layout->avx_offset = 576;
+      layout->bndregs_offset = 960;
+      layout->bndcfg_offset = 1024;
+      layout->k_offset = 1088;
+      layout->zmm_h_offset = 1152;
+      layout->zmm_offset = 1664;
+      layout->pkru_offset = 2688;
+    }
+  else if (HAS_PKRU(xcr0) && xsave_size == 2440)
+    {
+      /* AMD Ryzen 9 CPUs supporting PKRU.  */
+      layout->avx_offset = 576;
+      layout->k_offset = 832;
+      layout->zmm_h_offset = 896;
+      layout->zmm_offset = 1408;
+      layout->pkru_offset = 2432;
+    }
+  else if (HAS_AVX512(xcr0) && xsave_size == 2688)
+    {
+      /* Intel CPUs supporting AVX512.  */
+      layout->avx_offset = 576;
+      layout->bndregs_offset = 960;
+      layout->bndcfg_offset = 1024;
+      layout->k_offset = 1088;
+      layout->zmm_h_offset = 1152;
+      layout->zmm_offset = 1664;
+    }
+  else if (HAS_MPX(xcr0) && xsave_size == 1088)
+    {
+      /* Intel CPUs supporting MPX.  */
+      layout->avx_offset = 576;
+      layout->bndregs_offset = 960;
+      layout->bndcfg_offset = 1024;
+    }
+  else if (HAS_AVX(xcr0) && xsave_size == 832)
+    {
+      /* Intel and AMD CPUs supporting AVX.  */
+      layout->avx_offset = 576;
+    }
+}
+
 /* 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 698ff2ee206..4e1403aa753 100644
--- a/gdb/i387-tdep.h
+++ b/gdb/i387-tdep.h
@@ -25,6 +25,7 @@ struct frame_info;
 struct regcache;
 struct type;
 struct ui_file;
+struct x86_xsave_layout;
 
 /* Number of i387 floating point registers.  */
 #define I387_NUM_REGS	16
@@ -138,6 +139,12 @@ extern void i387_collect_fsave (const struct regcache *regcache, int regnum,
 extern void i387_supply_fxsave (struct regcache *regcache, int regnum,
 				const void *fxsave);
 
+/* Select an XSAVE layout based on the XCR0 bitmask and total XSAVE
+   extended state size.  */
+
+extern void i387_set_xsave_layout (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.34.1

[PATCH v3 03/13] nat/x86-cpuid.h: Add x86_cpuid_count wrapper around __get_cpuid_count.

[John Baldwin]

---
 gdb/nat/x86-cpuid.h | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/gdb/nat/x86-cpuid.h b/gdb/nat/x86-cpuid.h
index a42b4b2b08c..1dbb08d8222 100644
--- a/gdb/nat/x86-cpuid.h
+++ b/gdb/nat/x86-cpuid.h
@@ -48,6 +48,25 @@ x86_cpuid (unsigned int __level,
   return __get_cpuid (__level, __eax, __ebx, __ecx, __edx);
 }
 
+static __inline int
+x86_cpuid_count (unsigned int __level, unsigned int __sublevel,
+		 unsigned int *__eax, unsigned int *__ebx,
+		 unsigned int *__ecx, unsigned int *__edx)
+{
+  unsigned int __scratch;
+
+  if (!__eax)
+    __eax = &__scratch;
+  if (!__ebx)
+    __ebx = &__scratch;
+  if (!__ecx)
+    __ecx = &__scratch;
+  if (!__edx)
+    __edx = &__scratch;
+
+  return __get_cpuid_count (__level, __sublevel, __eax, __ebx, __ecx, __edx);
+}
+
 #else
 
 static __inline int
@@ -58,6 +77,14 @@ x86_cpuid (unsigned int __level,
   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 */
 
 #endif /* NAT_X86_CPUID_H */
-- 
2.34.1

[PATCH v3 04/13] x86 nat: Add helper functions to save the XSAVE layout for the host.

[John Baldwin]

x86_xsave_length returns the total length of the XSAVE state area
standard format as queried from CPUID.

x86_fetch_xsave_layout uses CPUID to query the offsets of XSAVE
extended regions from the running host.  The total length of the XSAVE
state area can either be supplied the caller if known (e.g. from
FreeBSD's PT_GETXSTATEINFO) or it can be queried from the running host
using x86_xsave_length.
---
 gdb/nat/x86-xstate.c | 65 ++++++++++++++++++++++++++++++++++++++++++++
 gdb/nat/x86-xstate.h | 35 ++++++++++++++++++++++++
 2 files changed, 100 insertions(+)
 create mode 100644 gdb/nat/x86-xstate.c
 create mode 100644 gdb/nat/x86-xstate.h

diff --git a/gdb/nat/x86-xstate.c b/gdb/nat/x86-xstate.c
new file mode 100644
index 00000000000..f9d59096a44
--- /dev/null
+++ b/gdb/nat/x86-xstate.c
@@ -0,0 +1,65 @@
+/* x86 XSAVE extended state functions.
+
+   Copyright (C) 2022 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 "gdbsupport/x86-xstate.h"
+#include "nat/x86-cpuid.h"
+#include "nat/x86-xstate.h"
+
+/* Fetch the offset of a specific XSAVE extended region.  */
+
+static int
+xsave_feature_offset (uint64_t xcr0, int feature)
+{
+  uint32_t ebx;
+
+  if ((xcr0 & (1ULL << feature)) == 0)
+    return 0;
+
+  if (!x86_cpuid_count (0xd, feature, nullptr, &ebx, nullptr, nullptr))
+    return 0;
+  return ebx;
+}
+
+/* See x86-xstate.h.  */
+
+int
+x86_xsave_length ()
+{
+  uint32_t ecx;
+
+  if (!x86_cpuid_count (0xd, 0, nullptr, nullptr, &ecx, nullptr))
+    return 0;
+  return ecx;
+}
+
+/* See x86-xstate.h.  */
+
+void
+x86_fetch_xsave_layout (uint64_t xcr0, int len, x86_xsave_layout &layout)
+{
+  layout.sizeof_xsave = len;
+  layout.avx_offset = xsave_feature_offset(xcr0, X86_XSTATE_AVX_ID);
+  layout.bndregs_offset = xsave_feature_offset(xcr0, X86_XSTATE_BNDREGS_ID);
+  layout.bndcfg_offset = xsave_feature_offset(xcr0, X86_XSTATE_BNDCFG_ID);
+  layout.k_offset = xsave_feature_offset(xcr0, X86_XSTATE_K_ID);
+  layout.zmm_h_offset = xsave_feature_offset(xcr0, X86_XSTATE_ZMM_H_ID);
+  layout.zmm_offset = xsave_feature_offset(xcr0, X86_XSTATE_ZMM_ID);
+  layout.pkru_offset = xsave_feature_offset(xcr0, X86_XSTATE_PKRU_ID);
+}
diff --git a/gdb/nat/x86-xstate.h b/gdb/nat/x86-xstate.h
new file mode 100644
index 00000000000..bcc1449eda9
--- /dev/null
+++ b/gdb/nat/x86-xstate.h
@@ -0,0 +1,35 @@
+/* x86 XSAVE extended state functions.
+
+   Copyright (C) 2022 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/>.  */
+
+#ifndef NAT_X86_XSTATE_H
+#define NAT_X86_XSTATE_H
+
+#include "gdbsupport/x86-xstate.h"
+
+/* Return the size of the XSAVE extended state fetched via CPUID.  */
+
+int x86_xsave_length ();
+
+/* Save the size and offsets of the XSAVE extended regions for the
+   running host in LAYOUT.  Offsets of each of the enabled regions in
+   XCR0 are fetched via CPUID.  */
+
+void x86_fetch_xsave_layout (uint64_t xcr0, int len, x86_xsave_layout &layout);
+
+#endif /* NAT_X86_XSTATE_H */
-- 
2.34.1

[PATCH v3 05/13] gdb: Update x86 FreeBSD architectures to support XSAVE layouts.

[John Baldwin]

Add an implementation of gdbarch_core_xfer_x86_xsave_layout which uses
the size of an existing NT_X86_XSTATE core dump to determine the
offsets via i387_set_xsave_layout.

Use tdep->xsave_layout.sizeof_xsave as the size of the XSTATE register
set and only fetch/store the register set if this size is non-zero.
---
 gdb/amd64-fbsd-tdep.c |  8 ++++++--
 gdb/i386-fbsd-tdep.c  | 37 ++++++++++++++++++++++++++++++++++---
 gdb/i386-fbsd-tdep.h  |  6 ++++++
 3 files changed, 46 insertions(+), 5 deletions(-)

diff --git a/gdb/amd64-fbsd-tdep.c b/gdb/amd64-fbsd-tdep.c
index 55764beaad2..f0f78f8ed08 100644
--- a/gdb/amd64-fbsd-tdep.c
+++ b/gdb/amd64-fbsd-tdep.c
@@ -271,8 +271,10 @@ amd64fbsd_iterate_over_regset_sections (struct gdbarch *gdbarch,
   cb (".reg-x86-segbases", AMD64_FBSD_SIZEOF_SEGBASES_REGSET,
       AMD64_FBSD_SIZEOF_SEGBASES_REGSET, &amd64_fbsd_segbases_regset,
       "segment bases", cb_data);
-  cb (".reg-xstate", X86_XSTATE_SIZE (tdep->xcr0), X86_XSTATE_SIZE (tdep->xcr0),
-      &amd64fbsd_xstateregset, "XSAVE extended state", cb_data);
+  if (tdep->xsave_layout.sizeof_xsave != 0)
+    cb (".reg-xstate", tdep->xsave_layout.sizeof_xsave,
+	tdep->xsave_layout.sizeof_xsave, &amd64fbsd_xstateregset,
+	"XSAVE extended state", cb_data);
 }
 
 /* Implement the get_thread_local_address gdbarch method.  */
@@ -313,6 +315,8 @@ amd64fbsd_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
   tramp_frame_prepend_unwinder (gdbarch, &amd64_fbsd_sigframe);
 
   tdep->xsave_xcr0_offset = I386_FBSD_XSAVE_XCR0_OFFSET;
+  set_gdbarch_core_xfer_x86_xsave_layout
+    (gdbarch, i386fbsd_core_xfer_x86_xsave_layout);
 
   /* Iterate over core file register note sections.  */
   set_gdbarch_iterate_over_regset_sections
diff --git a/gdb/i386-fbsd-tdep.c b/gdb/i386-fbsd-tdep.c
index d68498cd5e9..1dfb8d5c737 100644
--- a/gdb/i386-fbsd-tdep.c
+++ b/gdb/i386-fbsd-tdep.c
@@ -18,6 +18,7 @@
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
 #include "defs.h"
+#include "gdbcore.h"
 #include "osabi.h"
 #include "regcache.h"
 #include "regset.h"
@@ -278,6 +279,34 @@ i386fbsd_core_read_xcr0 (bfd *abfd)
   return xcr0;
 }
 
+/* Implement the core_xfer_x86_xsave_layout gdbarch method.  */
+
+LONGEST
+i386fbsd_core_xfer_x86_xsave_layout (struct gdbarch *gdbarch,
+				     gdb_byte *readbuf, ULONGEST offset,
+				     ULONGEST len)
+{
+  asection *xstate = bfd_get_section_by_name (core_bfd, ".reg-xstate");
+  if (xstate == nullptr)
+    return -1;
+
+  size_t size = bfd_section_size (xstate);
+  if (size < X86_XSTATE_AVX_SIZE)
+    return -1;
+
+  if (offset > sizeof (x86_xsave_layout))
+    return -1;
+
+  x86_xsave_layout layout;
+  i387_set_xsave_layout (i386fbsd_core_read_xcr0 (core_bfd), size, &layout);
+
+  if (offset + len > sizeof (layout))
+    len = sizeof (layout) - offset;
+
+  memcpy (readbuf, ((gdb_byte *) &layout) + offset, len);
+  return len;
+}
+
 /* Implement the core_read_description gdbarch method.  */
 
 static const struct target_desc *
@@ -335,9 +364,9 @@ i386fbsd_iterate_over_regset_sections (struct gdbarch *gdbarch,
       I386_FBSD_SIZEOF_SEGBASES_REGSET, &i386_fbsd_segbases_regset,
       "segment bases", cb_data);
 
-  if (tdep->xcr0 & X86_XSTATE_AVX)
-    cb (".reg-xstate", X86_XSTATE_SIZE (tdep->xcr0),
-	X86_XSTATE_SIZE (tdep->xcr0), &i386fbsd_xstateregset,
+  if (tdep->xsave_layout.sizeof_xsave != 0)
+    cb (".reg-xstate", tdep->xsave_layout.sizeof_xsave,
+	tdep->xsave_layout.sizeof_xsave, &i386fbsd_xstateregset,
 	"XSAVE extended state", cb_data);
 }
 
@@ -386,6 +415,8 @@ i386fbsd_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
   i386_elf_init_abi (info, gdbarch);
 
   tdep->xsave_xcr0_offset = I386_FBSD_XSAVE_XCR0_OFFSET;
+  set_gdbarch_core_xfer_x86_xsave_layout
+    (gdbarch, i386fbsd_core_xfer_x86_xsave_layout);
 
   /* Iterate over core file register note sections.  */
   set_gdbarch_iterate_over_regset_sections
diff --git a/gdb/i386-fbsd-tdep.h b/gdb/i386-fbsd-tdep.h
index 76f4c20f657..305ef8416ed 100644
--- a/gdb/i386-fbsd-tdep.h
+++ b/gdb/i386-fbsd-tdep.h
@@ -25,6 +25,12 @@
 /* Get XSAVE extended state xcr0 from core dump.  */
 extern uint64_t i386fbsd_core_read_xcr0 (bfd *abfd);
 
+/* Implement the core_xfer_x86_xsave_layout gdbarch method.  */
+extern LONGEST i386fbsd_core_xfer_x86_xsave_layout (struct gdbarch *gdbarch,
+						    gdb_byte *readbuf,
+						    ULONGEST offset,
+						    ULONGEST len);
+
 /* The format of the XSAVE extended area is determined by hardware.
    Cores store the XSAVE extended area in a NT_X86_XSTATE note that
    matches the layout on Linux.  */
-- 
2.34.1

[PATCH v3 06/13] gdb: Support XSAVE layouts for the current host in the FreeBSD x86 targets.

[John Baldwin]

Use the CPUID instruction to fetch the offsets of supported state
components.
---
 gdb/amd64-fbsd-nat.c | 40 +++++++++-------------------------
 gdb/configure.nat    |  5 +++--
 gdb/i386-fbsd-nat.c  | 39 ++++++++-------------------------
 gdb/x86-fbsd-nat.c   | 51 ++++++++++++++++++++++++++++++++++++++++++++
 gdb/x86-fbsd-nat.h   | 29 ++++++++++++++++++++++++-
 5 files changed, 101 insertions(+), 63 deletions(-)

diff --git a/gdb/amd64-fbsd-nat.c b/gdb/amd64-fbsd-nat.c
index d125d582a21..37180f415c4 100644
--- a/gdb/amd64-fbsd-nat.c
+++ b/gdb/amd64-fbsd-nat.c
@@ -31,9 +31,9 @@
 
 #include "amd64-tdep.h"
 #include "amd64-fbsd-tdep.h"
+#include "i387-tdep.h"
 #include "amd64-nat.h"
 #include "x86-nat.h"
-#include "gdbsupport/x86-xstate.h"
 #include "x86-fbsd-nat.h"
 
 class amd64_fbsd_nat_target final : public x86_fbsd_nat_target
@@ -47,10 +47,6 @@ class amd64_fbsd_nat_target final : public x86_fbsd_nat_target
 
 static amd64_fbsd_nat_target the_amd64_fbsd_nat_target;
 
-#ifdef PT_GETXSTATE_INFO
-static size_t xsave_len;
-#endif
-
 /* This is a layout of the amd64 'struct reg' but with i386
    registers.  */
 
@@ -146,9 +142,9 @@ amd64_fbsd_nat_target::fetch_registers (struct regcache *regcache, int regnum)
      fetching the FPU/XSAVE state unnecessarily.  */
 
 #ifdef PT_GETXSTATE_INFO
-  if (xsave_len != 0)
+  if (xsave_info.xsave_len != 0)
     {
-      void *xstateregs = alloca (xsave_len);
+      void *xstateregs = alloca (xsave_info.xsave_len);
 
       if (ptrace (PT_GETXSTATE, pid, (PTRACE_TYPE_ARG3) xstateregs, 0) == -1)
 	perror_with_name (_("Couldn't get extended state status"));
@@ -223,9 +219,9 @@ amd64_fbsd_nat_target::store_registers (struct regcache *regcache, int regnum)
      fetching the FPU/XSAVE state unnecessarily.  */
 
 #ifdef PT_GETXSTATE_INFO
-  if (xsave_len != 0)
+  if (xsave_info.xsave_len != 0)
     {
-      void *xstateregs = alloca (xsave_len);
+      void *xstateregs = alloca (xsave_info.xsave_len);
 
       if (ptrace (PT_GETXSTATE, pid, (PTRACE_TYPE_ARG3) xstateregs, 0) == -1)
 	perror_with_name (_("Couldn't get extended state status"));
@@ -233,7 +229,7 @@ amd64_fbsd_nat_target::store_registers (struct regcache *regcache, int regnum)
       amd64_collect_xsave (regcache, regnum, xstateregs, 0);
 
       if (ptrace (PT_SETXSTATE, pid, (PTRACE_TYPE_ARG3) xstateregs,
-		  xsave_len) == -1)
+		  xsave_info.xsave_len) == -1)
 	perror_with_name (_("Couldn't write extended state status"));
       return;
     }
@@ -303,10 +299,6 @@ amd64fbsd_supply_pcb (struct regcache *regcache, struct pcb *pcb)
 const struct target_desc *
 amd64_fbsd_nat_target::read_description ()
 {
-#ifdef PT_GETXSTATE_INFO
-  static int xsave_probed;
-  static uint64_t xcr0;
-#endif
   struct reg regs;
   int is64;
 
@@ -315,25 +307,13 @@ amd64_fbsd_nat_target::read_description ()
     perror_with_name (_("Couldn't get registers"));
   is64 = (regs.r_cs == GSEL (GUCODE_SEL, SEL_UPL));
 #ifdef PT_GETXSTATE_INFO
-  if (!xsave_probed)
-    {
-      struct ptrace_xstate_info info;
-
-      if (ptrace (PT_GETXSTATE_INFO, inferior_ptid.pid (),
-		  (PTRACE_TYPE_ARG3) &info, sizeof (info)) == 0)
-	{
-	  xsave_len = info.xsave_len;
-	  xcr0 = info.xsave_mask;
-	}
-      xsave_probed = 1;
-    }
-
-  if (xsave_len != 0)
+  probe_xsave_layout (inferior_ptid.pid ());
+  if (xsave_info.xsave_len != 0)
     {
       if (is64)
-	return amd64_target_description (xcr0, true);
+	return amd64_target_description (xsave_info.xsave_mask, true);
       else
-	return i386_target_description (xcr0, true);
+	return i386_target_description (xsave_info.xsave_mask, true);
     }
 #endif
   if (is64)
diff --git a/gdb/configure.nat b/gdb/configure.nat
index d219d6a960c..7dcbbdcc985 100644
--- a/gdb/configure.nat
+++ b/gdb/configure.nat
@@ -166,7 +166,8 @@ case ${gdb_host} in
 	    i386)
 		# Host: FreeBSD/i386
 		NATDEPFILES="${NATDEPFILES} x86-nat.o nat/x86-dregs.o \
-		x86-bsd-nat.o x86-fbsd-nat.o i386-fbsd-nat.o bsd-kvm.o"
+		nat/x86-xstate.o x86-bsd-nat.o x86-fbsd-nat.o i386-fbsd-nat.o \
+		bsd-kvm.o"
 		;;
 	    mips)
 		# Host: FreeBSD/mips
@@ -195,7 +196,7 @@ case ${gdb_host} in
 		# Host: FreeBSD/amd64
 		NATDEPFILES="${NATDEPFILES} amd64-nat.o \
 		amd64-fbsd-nat.o bsd-kvm.o x86-nat.o nat/x86-dregs.o \
-		x86-bsd-nat.o x86-fbsd-nat.o"
+		nat/x86-xstate.o x86-bsd-nat.o x86-fbsd-nat.o"
 		;;
 	esac
 	;;
diff --git a/gdb/i386-fbsd-nat.c b/gdb/i386-fbsd-nat.c
index 0a00aef5901..e9b93fb906d 100644
--- a/gdb/i386-fbsd-nat.c
+++ b/gdb/i386-fbsd-nat.c
@@ -31,7 +31,6 @@
 #include "i386-fbsd-tdep.h"
 #include "i387-tdep.h"
 #include "x86-nat.h"
-#include "gdbsupport/x86-xstate.h"
 #include "x86-fbsd-nat.h"
 
 class i386_fbsd_nat_target final : public x86_fbsd_nat_target
@@ -47,10 +46,6 @@ class i386_fbsd_nat_target final : public x86_fbsd_nat_target
 
 static i386_fbsd_nat_target the_i386_fbsd_nat_target;
 
-#ifdef PT_GETXSTATE_INFO
-static size_t xsave_len;
-#endif
-
 static int have_ptrace_xmmregs;
 
 /* Fetch register REGNUM from the inferior.  If REGNUM is -1, do this
@@ -102,9 +97,9 @@ i386_fbsd_nat_target::fetch_registers (struct regcache *regcache, int regnum)
      fetching the FPU/XSAVE state unnecessarily.  */
 
 #ifdef PT_GETXSTATE_INFO
-  if (xsave_len != 0)
+  if (xsave_info.xsave_len != 0)
     {
-      void *xstateregs = alloca (xsave_len);
+      void *xstateregs = alloca (xsave_info.xsave_len);
 
       if (ptrace (PT_GETXSTATE, pid, (PTRACE_TYPE_ARG3) xstateregs, 0) == -1)
 	perror_with_name (_("Couldn't get extended state status"));
@@ -181,17 +176,17 @@ i386_fbsd_nat_target::store_registers (struct regcache *regcache, int regnum)
      fetching the FPU/XSAVE state unnecessarily.  */
 
 #ifdef PT_GETXSTATE_INFO
-  if (xsave_len != 0)
+  if (xsave_info.xsave_len != 0)
     {
-      void *xstateregs = alloca (xsave_len);
+      void *xstateregs = alloca (xsave_info.xsave_len);
 
       if (ptrace (PT_GETXSTATE, pid, (PTRACE_TYPE_ARG3) xstateregs, 0) == -1)
 	perror_with_name (_("Couldn't get extended state status"));
 
       i387_collect_xsave (regcache, regnum, xstateregs, 0);
 
-      if (ptrace (PT_SETXSTATE, pid, (PTRACE_TYPE_ARG3) xstateregs, xsave_len)
-	  == -1)
+      if (ptrace (PT_SETXSTATE, pid, (PTRACE_TYPE_ARG3) xstateregs,
+		  xsave_info.xsave_len) == -1)
 	perror_with_name (_("Couldn't write extended state status"));
       return;
     }
@@ -309,28 +304,12 @@ i386fbsd_supply_pcb (struct regcache *regcache, struct pcb *pcb)
 const struct target_desc *
 i386_fbsd_nat_target::read_description ()
 {
-#ifdef PT_GETXSTATE_INFO
-  static int xsave_probed;
-  static uint64_t xcr0;
-#endif
   static int xmm_probed;
 
 #ifdef PT_GETXSTATE_INFO
-  if (!xsave_probed)
-    {
-      struct ptrace_xstate_info info;
-
-      if (ptrace (PT_GETXSTATE_INFO, inferior_ptid.pid (),
-		  (PTRACE_TYPE_ARG3) &info, sizeof (info)) == 0)
-	{
-	  xsave_len = info.xsave_len;
-	  xcr0 = info.xsave_mask;
-	}
-      xsave_probed = 1;
-    }
-
-  if (xsave_len != 0)
-    return i386_target_description (xcr0, true);
+  probe_xsave_layout (inferior_ptid.pid ());
+  if (xsave_info.xsave_len != 0)
+    return i386_target_description (xsave_info.xsave_mask, true);
 #endif
 
   if (!xmm_probed)
diff --git a/gdb/x86-fbsd-nat.c b/gdb/x86-fbsd-nat.c
index ad8c693b68e..1712acf16c0 100644
--- a/gdb/x86-fbsd-nat.c
+++ b/gdb/x86-fbsd-nat.c
@@ -19,6 +19,9 @@
 
 #include "defs.h"
 #include "x86-fbsd-nat.h"
+#ifdef PT_GETXSTATE_INFO
+#include "nat/x86-xstate.h"
+#endif
 
 /* Implement the virtual fbsd_nat_target::low_new_fork method.  */
 
@@ -43,3 +46,51 @@ x86_fbsd_nat_target::low_new_fork (ptid_t parent, pid_t child)
   child_state = x86_debug_reg_state (child);
   *child_state = *parent_state;
 }
+
+#ifdef PT_GETXSTATE_INFO
+/* Implement the "xfer_partial" target_ops method.  */
+
+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_XSAVE_LAYOUT:
+      if (xsave_layout.sizeof_xsave == 0)
+	return TARGET_XFER_E_IO;
+
+      if (offset > sizeof (xsave_layout))
+	return TARGET_XFER_E_IO;
+
+      if (offset + len > sizeof (xsave_layout))
+	len = sizeof (xsave_layout) - offset;
+
+      memcpy (readbuf, ((gdb_byte *) &xsave_layout) + offset, len);
+      *xfered_len = len;
+      return len == 0 ? TARGET_XFER_EOF : TARGET_XFER_OK;
+    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)
+{
+  if (xsave_probed)
+    return;
+
+  xsave_probed = true;
+
+  if (ptrace (PT_GETXSTATE_INFO, pid, (PTRACE_TYPE_ARG3) &xsave_info,
+	      sizeof (xsave_info)) != 0)
+    return;
+  if (xsave_info.xsave_len != 0)
+    x86_fetch_xsave_layout (xsave_info.xsave_mask, xsave_info.xsave_len,
+			    xsave_layout);
+}
+#endif
diff --git a/gdb/x86-fbsd-nat.h b/gdb/x86-fbsd-nat.h
index cdb8cd36a4c..10ec81acdd9 100644
--- a/gdb/x86-fbsd-nat.h
+++ b/gdb/x86-fbsd-nat.h
@@ -20,6 +20,11 @@
 #ifndef X86_FBSD_NAT_H
 #define X86_FBSD_NAT_H
 
+#include <sys/ptrace.h>
+
+#ifdef PT_GETXSTATE_INFO
+#include "gdbsupport/x86-xstate.h"
+#endif
 #include "fbsd-nat.h"
 #include "x86-bsd-nat.h"
 
@@ -27,10 +32,32 @@
 
 class x86_fbsd_nat_target : public x86bsd_nat_target<fbsd_nat_target>
 {
+  void low_new_fork (ptid_t parent, pid_t child) override;
+
+public:
   bool supports_stopped_by_hw_breakpoint () override
   { return true; }
 
-  void low_new_fork (ptid_t parent, pid_t child) override;
+#ifdef PT_GETXSTATE_INFO
+  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);
+
+  struct ptrace_xstate_info xsave_info;
+  x86_xsave_layout xsave_layout;
+
+private:
+  bool xsave_probed;
+#endif
 };
 
+#ifdef PT_GETXSTATE_INFO
+#endif
+
 #endif /* x86-bsd-nat.h */
-- 
2.34.1

[PATCH v3 07/13] gdb: Update x86 Linux architectures to support XSAVE layouts.

[John Baldwin]

Add an implementation of gdbarch_core_xfer_x86_xsave_layout which uses
the size of an existing NT_X86_XSTATE core dump to determine the
offsets via i387_set_xsave_layout.

Use tdep->xsave_layout.sizeof_xsave as the size of the XSTATE register
set.
---
 gdb/amd64-linux-tdep.c |  8 ++++++--
 gdb/i386-linux-tdep.c  | 34 ++++++++++++++++++++++++++++++++--
 gdb/i386-linux-tdep.h  |  6 ++++++
 3 files changed, 44 insertions(+), 4 deletions(-)

diff --git a/gdb/amd64-linux-tdep.c b/gdb/amd64-linux-tdep.c
index 0e5194fbeee..bb49224231d 100644
--- a/gdb/amd64-linux-tdep.c
+++ b/gdb/amd64-linux-tdep.c
@@ -1657,8 +1657,10 @@ amd64_linux_iterate_over_regset_sections (struct gdbarch *gdbarch,
 
   cb (".reg", 27 * 8, 27 * 8, &i386_gregset, NULL, cb_data);
   cb (".reg2", 512, 512, &amd64_fpregset, NULL, cb_data);
-  cb (".reg-xstate", X86_XSTATE_SIZE (tdep->xcr0), X86_XSTATE_SIZE (tdep->xcr0),
-      &amd64_linux_xstateregset, "XSAVE extended state", cb_data);
+  if (tdep->xsave_layout.sizeof_xsave != 0)
+    cb (".reg-xstate", tdep->xsave_layout.sizeof_xsave,
+	tdep->xsave_layout.sizeof_xsave, &amd64_linux_xstateregset,
+	"XSAVE extended state", cb_data);
 }
 
 /* The instruction sequences used in x86_64 machines for a
@@ -1800,6 +1802,8 @@ amd64_linux_init_abi_common(struct gdbarch_info info, struct gdbarch *gdbarch,
   tdep->sc_num_regs = ARRAY_SIZE (amd64_linux_sc_reg_offset);
 
   tdep->xsave_xcr0_offset = I386_LINUX_XSAVE_XCR0_OFFSET;
+  set_gdbarch_core_xfer_x86_xsave_layout
+    (gdbarch, i386_linux_core_xfer_x86_xsave_layout);
 
   /* Add the %orig_rax register used for syscall restarting.  */
   set_gdbarch_write_pc (gdbarch, amd64_linux_write_pc);
diff --git a/gdb/i386-linux-tdep.c b/gdb/i386-linux-tdep.c
index 5d7f54194af..52d4c7e395b 100644
--- a/gdb/i386-linux-tdep.c
+++ b/gdb/i386-linux-tdep.c
@@ -675,6 +675,34 @@ i386_linux_core_read_xcr0 (bfd *abfd)
   return xcr0;
 }
 
+/* Implement the core_xfer_x86_xsave_layout gdbarch method.  */
+
+LONGEST
+i386_linux_core_xfer_x86_xsave_layout (struct gdbarch *gdbarch,
+				       gdb_byte *readbuf, ULONGEST offset,
+				       ULONGEST len)
+{
+  asection *xstate = bfd_get_section_by_name (core_bfd, ".reg-xstate");
+  if (xstate == nullptr)
+    return -1;
+
+  size_t size = bfd_section_size (xstate);
+  if (size < X86_XSTATE_AVX_SIZE)
+    return -1;
+
+  if (offset > sizeof (x86_xsave_layout))
+    return -1;
+
+  x86_xsave_layout layout;
+  i387_set_xsave_layout (i386_linux_core_read_xcr0 (core_bfd), size, &layout);
+
+  if (offset + len > sizeof (layout))
+    len = sizeof (layout) - offset;
+
+  memcpy (readbuf, ((gdb_byte *) &layout) + offset, len);
+  return len;
+}
+
 /* See i386-linux-tdep.h.  */
 
 const struct target_desc *
@@ -768,8 +796,8 @@ i386_linux_iterate_over_regset_sections (struct gdbarch *gdbarch,
   cb (".reg", 68, 68, &i386_gregset, NULL, cb_data);
 
   if (tdep->xcr0 & X86_XSTATE_AVX)
-    cb (".reg-xstate", X86_XSTATE_SIZE (tdep->xcr0),
-	X86_XSTATE_SIZE (tdep->xcr0), &i386_linux_xstateregset,
+    cb (".reg-xstate", tdep->xsave_layout.sizeof_xsave,
+	tdep->xsave_layout.sizeof_xsave, &i386_linux_xstateregset,
 	"XSAVE extended state", cb_data);
   else if (tdep->xcr0 & X86_XSTATE_SSE)
     cb (".reg-xfp", 512, 512, &i386_fpregset, "extended floating-point",
@@ -872,6 +900,8 @@ i386_linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
   tdep->sc_num_regs = ARRAY_SIZE (i386_linux_sc_reg_offset);
 
   tdep->xsave_xcr0_offset = I386_LINUX_XSAVE_XCR0_OFFSET;
+  set_gdbarch_core_xfer_x86_xsave_layout
+    (gdbarch, i386_linux_core_xfer_x86_xsave_layout);
 
   set_gdbarch_process_record (gdbarch, i386_process_record);
   set_gdbarch_process_record_signal (gdbarch, i386_linux_record_signal);
diff --git a/gdb/i386-linux-tdep.h b/gdb/i386-linux-tdep.h
index 6b3555aa3ea..02664d2d3b9 100644
--- a/gdb/i386-linux-tdep.h
+++ b/gdb/i386-linux-tdep.h
@@ -37,6 +37,12 @@
 /* Get XSAVE extended state xcr0 from core dump.  */
 extern uint64_t i386_linux_core_read_xcr0 (bfd *abfd);
 
+/* Implement the core_xfer_x86_xsave_layout gdbarch method.  */
+extern LONGEST i386_linux_core_xfer_x86_xsave_layout (struct gdbarch *gdbarch,
+						      gdb_byte *readbuf,
+						      ULONGEST offset,
+						      ULONGEST len);
+
 /* Handle and display information related to the MPX bound violation
    to the user.  */
 extern void i386_linux_report_signal_info (struct gdbarch *gdbarch,
-- 
2.34.1

[PATCH v3 08/13] gdb: Support XSAVE layouts for the current host in the Linux x86 targets.

[John Baldwin]

Note that this uses the CPUID instruction to determine the total size
of the XSAVE register set.  If there is a way to fetch the register set
size using ptrace that would probably be better.
---
 gdb/amd64-linux-nat.c |  6 ++++--
 gdb/configure.nat     |  3 ++-
 gdb/i386-linux-nat.c  |  8 ++++++--
 gdb/x86-linux-nat.c   | 33 +++++++++++++++++++++++++++++++++
 gdb/x86-linux-nat.h   | 11 +++++++++++
 5 files changed, 56 insertions(+), 5 deletions(-)

diff --git a/gdb/amd64-linux-nat.c b/gdb/amd64-linux-nat.c
index 3d28d7e1d57..2f0e2691e28 100644
--- a/gdb/amd64-linux-nat.c
+++ b/gdb/amd64-linux-nat.c
@@ -210,6 +210,7 @@ void
 amd64_linux_nat_target::fetch_registers (struct regcache *regcache, int regnum)
 {
   struct gdbarch *gdbarch = regcache->arch ();
+  i386_gdbarch_tdep *tdep = (i386_gdbarch_tdep *) gdbarch_tdep (gdbarch);
   int tid;
 
   /* GNU/Linux LWP ID's are process ID's.  */
@@ -235,7 +236,7 @@ amd64_linux_nat_target::fetch_registers (struct regcache *regcache, int regnum)
 
       if (have_ptrace_getregset == TRIBOOL_TRUE)
 	{
-	  char xstateregs[X86_XSTATE_MAX_SIZE];
+	  char xstateregs[tdep->xsave_layout.sizeof_xsave];
 	  struct iovec iov;
 
 	  /* Pre-4.14 kernels have a bug (fixed by commit 0852b374173b
@@ -270,6 +271,7 @@ void
 amd64_linux_nat_target::store_registers (struct regcache *regcache, int regnum)
 {
   struct gdbarch *gdbarch = regcache->arch ();
+  i386_gdbarch_tdep *tdep = (i386_gdbarch_tdep *) gdbarch_tdep (gdbarch);
   int tid;
 
   /* GNU/Linux LWP ID's are process ID's.  */
@@ -299,7 +301,7 @@ amd64_linux_nat_target::store_registers (struct regcache *regcache, int regnum)
 
       if (have_ptrace_getregset == TRIBOOL_TRUE)
 	{
-	  char xstateregs[X86_XSTATE_MAX_SIZE];
+	  char xstateregs[tdep->xsave_layout.sizeof_xsave];
 	  struct iovec iov;
 
 	  iov.iov_base = xstateregs;
diff --git a/gdb/configure.nat b/gdb/configure.nat
index 7dcbbdcc985..d6ffd055eb3 100644
--- a/gdb/configure.nat
+++ b/gdb/configure.nat
@@ -254,6 +254,7 @@ case ${gdb_host} in
 	    i386)
 		# Host: Intel 386 running GNU/Linux.
 		NATDEPFILES="${NATDEPFILES} x86-nat.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"
 		;;
@@ -319,7 +320,7 @@ case ${gdb_host} in
 	    i386)
 		# Host: GNU/Linux x86-64
 		NATDEPFILES="${NATDEPFILES} x86-nat.o nat/x86-dregs.o \
-		amd64-nat.o amd64-linux-nat.o x86-linux-nat.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 \
 		nat/amd64-linux-siginfo.o"
diff --git a/gdb/i386-linux-nat.c b/gdb/i386-linux-nat.c
index e9b0f8a222f..0a3453aed30 100644
--- a/gdb/i386-linux-nat.c
+++ b/gdb/i386-linux-nat.c
@@ -330,7 +330,9 @@ store_fpregs (const struct regcache *regcache, int tid, int regno)
 static int
 fetch_xstateregs (struct regcache *regcache, int tid)
 {
-  char xstateregs[X86_XSTATE_MAX_SIZE];
+  struct gdbarch *gdbarch = regcache->arch ();
+  i386_gdbarch_tdep *tdep = (i386_gdbarch_tdep *) gdbarch_tdep (gdbarch);
+  char xstateregs[tdep->xsave_layout.sizeof_xsave];
   struct iovec iov;
 
   if (have_ptrace_getregset != TRIBOOL_TRUE)
@@ -353,7 +355,9 @@ fetch_xstateregs (struct regcache *regcache, int tid)
 static int
 store_xstateregs (const struct regcache *regcache, int tid, int regno)
 {
-  char xstateregs[X86_XSTATE_MAX_SIZE];
+  struct gdbarch *gdbarch = regcache->arch ();
+  i386_gdbarch_tdep *tdep = (i386_gdbarch_tdep *) gdbarch_tdep (gdbarch);
+  char xstateregs[tdep->xsave_layout.sizeof_xsave];
   struct iovec iov;
 
   if (have_ptrace_getregset != TRIBOOL_TRUE)
diff --git a/gdb/x86-linux-nat.c b/gdb/x86-linux-nat.c
index 80be9733310..6249e61cc09 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-xstate.h"
 #include "nat/linux-btrace.h"
 #include "nat/linux-nat.h"
 #include "nat/x86-linux.h"
@@ -176,6 +177,8 @@ x86_linux_nat_target::read_description ()
 	  /* Get XCR0 from XSAVE extended state.  */
 	  xcr0 = xstateregs[(I386_LINUX_XSAVE_XCR0_OFFSET
 			     / sizeof (uint64_t))];
+
+	  x86_fetch_xsave_layout (xcr0, x86_xsave_length (), xsave_layout);
 	}
     }
 
@@ -207,6 +210,36 @@ x86_linux_nat_target::read_description ()
 
   gdb_assert_not_reached ("failed to return tdesc");
 }
+
+/* Implement the "xfer_partial" target_ops method.  */
+
+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_XSAVE_LAYOUT:
+      if (xsave_layout.sizeof_xsave == 0)
+	return TARGET_XFER_E_IO;
+
+      if (offset > sizeof (xsave_layout))
+	return TARGET_XFER_E_IO;
+
+      if (offset + len > sizeof (xsave_layout))
+	len = sizeof (xsave_layout) - offset;
+
+      memcpy (readbuf, ((gdb_byte *) &xsave_layout) + offset, len);
+      *xfered_len = len;
+      return len == 0 ? TARGET_XFER_EOF : TARGET_XFER_OK;
+    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 1a562349691..0fc68cb87ff 100644
--- a/gdb/x86-linux-nat.h
+++ b/gdb/x86-linux-nat.h
@@ -22,6 +22,7 @@
 
 #include "gdb_proc_service.h"  /* For ps_err_e.  */
 #include "linux-nat.h"
+#include "gdbsupport/x86-xstate.h"
 #include "x86-nat.h"
 #include "nat/x86-linux.h"
 
@@ -32,6 +33,13 @@ struct x86_linux_nat_target : public x86_nat_target<linux_nat_target>
   /* Add the description reader.  */
   const struct target_desc *read_description () override;
 
+  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;
+
   struct btrace_target_info *enable_btrace (thread_info *tp,
 					    const struct btrace_config *conf) override;
   void disable_btrace (struct btrace_target_info *tinfo) override;
@@ -74,6 +82,9 @@ struct x86_linux_nat_target : public x86_nat_target<linux_nat_target>
 protected:
   /* Override the GNU/Linux inferior startup hook.  */
   void post_startup_inferior (ptid_t) override;
+
+private:
+  x86_xsave_layout xsave_layout;
 };
 
 \f
-- 
2.34.1

[PATCH v3 09/13] gdb: Use x86_xstate_layout to parse the XSAVE extended state area.

[John Baldwin]

All of the tables describing the offsets of indvidual registers for
XSAVE state components now hold relative offsets rather than absolute
offsets.  Some tables (those for MPX registers and ZMMH registers) had
to be split into separate tables as they held entries that spanned
multiple state components.
---
 gdb/i387-tdep.c | 442 +++++++++++++++++++++++++++++-------------------
 gdb/i387-tdep.h |   1 +
 2 files changed, 265 insertions(+), 178 deletions(-)

diff --git a/gdb/i387-tdep.c b/gdb/i387-tdep.c
index 80a130fdeff..86b642b5bf9 100644
--- a/gdb/i387-tdep.c
+++ b/gdb/i387-tdep.c
@@ -738,89 +738,104 @@ i387_collect_fxsave (const struct regcache *regcache, int regnum, void *fxsave)
 
 static int xsave_avxh_offset[] =
 {
-  576 + 0 * 16,		/* Upper 128bit of %ymm0 through ...  */
-  576 + 1 * 16,
-  576 + 2 * 16,
-  576 + 3 * 16,
-  576 + 4 * 16,
-  576 + 5 * 16,
-  576 + 6 * 16,
-  576 + 7 * 16,
-  576 + 8 * 16,
-  576 + 9 * 16,
-  576 + 10 * 16,
-  576 + 11 * 16,
-  576 + 12 * 16,
-  576 + 13 * 16,
-  576 + 14 * 16,
-  576 + 15 * 16		/* Upper 128bit of ... %ymm15 (128 bits each).  */
+  0 * 16,		/* Upper 128bit of %ymm0 through ...  */
+  1 * 16,
+  2 * 16,
+  3 * 16,
+  4 * 16,
+  5 * 16,
+  6 * 16,
+  7 * 16,
+  8 * 16,
+  9 * 16,
+  10 * 16,
+  11 * 16,
+  12 * 16,
+  13 * 16,
+  14 * 16,
+  15 * 16		/* Upper 128bit of ... %ymm15 (128 bits each).  */
 };
 
-#define XSAVE_AVXH_ADDR(tdep, xsave, regnum) \
-  (xsave + xsave_avxh_offset[regnum - I387_YMM0H_REGNUM (tdep)])
+#define XSAVE_AVXH_ADDR(tdep, xsave, regnum)			\
+  (xsave + (tdep)->xsave_layout.avx_offset			\
+   + xsave_avxh_offset[regnum - I387_YMM0H_REGNUM (tdep)])
 
 /* At xsave_ymm_avx512_offset[REGNUM] you'll find the offset to the location in
-   the upper 128bit of ZMM register data structure used by the "xsave"
+   the second 128bits of ZMM register data structure used by the "xsave"
    instruction where GDB register REGNUM is stored.  */
 
 static int xsave_ymm_avx512_offset[] =
 {
   /* HI16_ZMM_area + 16 bytes + regnum* 64 bytes.  */
-  1664 + 16 + 0 * 64,		/* %ymm16 through...  */
-  1664 + 16 + 1 * 64,
-  1664 + 16 + 2 * 64,
-  1664 + 16 + 3 * 64,
-  1664 + 16 + 4 * 64,
-  1664 + 16 + 5 * 64,
-  1664 + 16 + 6 * 64,
-  1664 + 16 + 7 * 64,
-  1664 + 16 + 8 * 64,
-  1664 + 16 + 9 * 64,
-  1664 + 16 + 10 * 64,
-  1664 + 16 + 11 * 64,
-  1664 + 16 + 12 * 64,
-  1664 + 16 + 13 * 64,
-  1664 + 16 + 14 * 64,
-  1664 + 16 + 15 * 64		/* ...  %ymm31 (128 bits each).  */
+  16 + 0 * 64,		/* %ymm16 through...  */
+  16 + 1 * 64,
+  16 + 2 * 64,
+  16 + 3 * 64,
+  16 + 4 * 64,
+  16 + 5 * 64,
+  16 + 6 * 64,
+  16 + 7 * 64,
+  16 + 8 * 64,
+  16 + 9 * 64,
+  16 + 10 * 64,
+  16 + 11 * 64,
+  16 + 12 * 64,
+  16 + 13 * 64,
+  16 + 14 * 64,
+  16 + 15 * 64		/* ...  %ymm31 (128 bits each).  */
 };
 
-#define XSAVE_YMM_AVX512_ADDR(tdep, xsave, regnum) \
-  (xsave + xsave_ymm_avx512_offset[regnum - I387_YMM16H_REGNUM (tdep)])
+#define XSAVE_YMM_AVX512_ADDR(tdep, xsave, regnum)			\
+  (xsave + (tdep)->xsave_layout.zmm_offset				\
+   + xsave_ymm_avx512_offset[regnum - I387_YMM16H_REGNUM (tdep)])
+
+/* At xsave_xmm_avx512_offset[REGNUM] you'll find the offset to the location in
+   the first 128bits of ZMM register data structure used by the "xsave"
+   instruction where GDB register REGNUM is stored.  */
 
 static int xsave_xmm_avx512_offset[] =
 {
-  1664 + 0 * 64,		/* %ymm16 through...  */
-  1664 + 1 * 64,
-  1664 + 2 * 64,
-  1664 + 3 * 64,
-  1664 + 4 * 64,
-  1664 + 5 * 64,
-  1664 + 6 * 64,
-  1664 + 7 * 64,
-  1664 + 8 * 64,
-  1664 + 9 * 64,
-  1664 + 10 * 64,
-  1664 + 11 * 64,
-  1664 + 12 * 64,
-  1664 + 13 * 64,
-  1664 + 14 * 64,
-  1664 + 15 * 64		/* ...  %ymm31 (128 bits each).  */
+  0 * 64,			/* %xmm16 through...  */
+  1 * 64,
+  2 * 64,
+  3 * 64,
+  4 * 64,
+  5 * 64,
+  6 * 64,
+  7 * 64,
+  8 * 64,
+  9 * 64,
+  10 * 64,
+  11 * 64,
+  12 * 64,
+  13 * 64,
+  14 * 64,
+  15 * 64			/* ...  %xmm31 (128 bits each).  */
 };
 
-#define XSAVE_XMM_AVX512_ADDR(tdep, xsave, regnum) \
-  (xsave + xsave_xmm_avx512_offset[regnum - I387_XMM16_REGNUM (tdep)])
+#define XSAVE_XMM_AVX512_ADDR(tdep, xsave, regnum)			\
+  (xsave + (tdep)->xsave_layout.zmm_offset				\
+   + xsave_xmm_avx512_offset[regnum - I387_XMM16_REGNUM (tdep)])
 
-static int xsave_mpx_offset[] = {
-  960 + 0 * 16,			/* bnd0r...bnd3r registers.  */
-  960 + 1 * 16,
-  960 + 2 * 16,
-  960 + 3 * 16,
-  1024 + 0 * 8,			/* bndcfg ... bndstatus.  */
-  1024 + 1 * 8,
+static int xsave_bndregs_offset[] = {
+  0 * 16,			/* bnd0r...bnd3r registers.  */
+  1 * 16,
+  2 * 16,
+  3 * 16
 };
 
-#define XSAVE_MPX_ADDR(tdep, xsave, regnum) \
-  (xsave + xsave_mpx_offset[regnum - I387_BND0R_REGNUM (tdep)])
+#define XSAVE_BNDREGS_ADDR(tdep, xsave, regnum)				\
+  (xsave + (tdep)->xsave_layout.bndregs_offset				\
+   + xsave_bndregs_offset[regnum - I387_BND0R_REGNUM (tdep)])
+
+static int xsave_bndcfg_offset[] = {
+  0 * 8,			/* bndcfg ... bndstatus.  */
+  1 * 8,
+};
+
+#define XSAVE_BNDCFG_ADDR(tdep, xsave, regnum)			\
+  (xsave + (tdep)->xsave_layout.bndcfg_offset			\
+   + xsave_bndcfg_offset[regnum - I387_BNDCFGU_REGNUM (tdep)])
 
   /* At xsave_avx512__h_offset[REGNUM] you find the offset to the location
    of the AVX512 opmask register data structure used by the "xsave"
@@ -828,61 +843,78 @@ static int xsave_mpx_offset[] = {
 
 static int xsave_avx512_k_offset[] =
 {
-  1088 + 0 * 8,			/* %k0 through...  */
-  1088 + 1 * 8,
-  1088 + 2 * 8,
-  1088 + 3 * 8,
-  1088 + 4 * 8,
-  1088 + 5 * 8,
-  1088 + 6 * 8,
-  1088 + 7 * 8     		/* %k7 (64 bits each).  */
+  0 * 8,			/* %k0 through...  */
+  1 * 8,
+  2 * 8,
+  3 * 8,
+  4 * 8,
+  5 * 8,
+  6 * 8,
+  7 * 8				/* %k7 (64 bits each).	*/
 };
 
-#define XSAVE_AVX512_K_ADDR(tdep, xsave, regnum) \
-  (xsave + xsave_avx512_k_offset[regnum - I387_K0_REGNUM (tdep)])
+#define XSAVE_AVX512_K_ADDR(tdep, xsave, regnum)		\
+  (xsave + (tdep)->xsave_layout.k_offset			\
+   + xsave_avx512_k_offset[regnum - I387_K0_REGNUM (tdep)])
 
-/* At xsave_avx512_zmm_h_offset[REGNUM] you find the offset to the location in
-   the upper 256bit of AVX512 ZMMH register data structure used by the "xsave"
-   instruction where GDB register REGNUM is stored.  */
 
-static int xsave_avx512_zmm_h_offset[] =
+/* At xsave_avx512_zmm0_h_offset[REGNUM] you find the offset to the
+   location in the upper 256bit of AVX512 ZMM0-15H register data
+   structure used by the "xsave" instruction where GDB register REGNUM
+   is stored.  */
+
+/* At xsave_avx512_zmm16_h_offset[REGNUM] you find the offset to the
+   location in the upper 256bit of AVX512 ZMMH register data structure
+   used by the "xsave" instruction where GDB register REGNUM is
+   stored.  */
+
+static int xsave_avx512_zmm0_h_offset[] =
+{
+  0 * 32,		/* Upper 256bit of %zmmh0 through...  */
+  1 * 32,
+  2 * 32,
+  3 * 32,
+  4 * 32,
+  5 * 32,
+  6 * 32,
+  7 * 32,
+  8 * 32,
+  9 * 32,
+  10 * 32,
+  11 * 32,
+  12 * 32,
+  13 * 32,
+  14 * 32,
+  15 * 32		/* Upper 256bit of...  %zmmh15 (256 bits each).  */
+};
+
+#define XSAVE_AVX512_ZMM0_H_ADDR(tdep, xsave, regnum)			\
+  (xsave + (tdep)->xsave_layout.zmm_h_offset				\
+   + xsave_avx512_zmm0_h_offset[regnum - I387_ZMM0H_REGNUM (tdep)])
+
+static int xsave_avx512_zmm16_h_offset[] =
 {
-  1152 + 0 * 32,
-  1152 + 1 * 32,	/* Upper 256bit of %zmmh0 through...  */
-  1152 + 2 * 32,
-  1152 + 3 * 32,
-  1152 + 4 * 32,
-  1152 + 5 * 32,
-  1152 + 6 * 32,
-  1152 + 7 * 32,
-  1152 + 8 * 32,
-  1152 + 9 * 32,
-  1152 + 10 * 32,
-  1152 + 11 * 32,
-  1152 + 12 * 32,
-  1152 + 13 * 32,
-  1152 + 14 * 32,
-  1152 + 15 * 32,	/* Upper 256bit of...  %zmmh15 (256 bits each).  */
-  1664 + 32 + 0 * 64,   /* Upper 256bit of...  %zmmh16 (256 bits each).  */
-  1664 + 32 + 1 * 64,
-  1664 + 32 + 2 * 64,
-  1664 + 32 + 3 * 64,
-  1664 + 32 + 4 * 64,
-  1664 + 32 + 5 * 64,
-  1664 + 32 + 6 * 64,
-  1664 + 32 + 7 * 64,
-  1664 + 32 + 8 * 64,
-  1664 + 32 + 9 * 64,
-  1664 + 32 + 10 * 64,
-  1664 + 32 + 11 * 64,
-  1664 + 32 + 12 * 64,
-  1664 + 32 + 13 * 64,
-  1664 + 32 + 14 * 64,
-  1664 + 32 + 15 * 64   /* Upper 256bit of... %zmmh31 (256 bits each).  */
+  32 + 0 * 64,		/* Upper 256bit of...  %zmmh16 (256 bits each).  */
+  32 + 1 * 64,
+  32 + 2 * 64,
+  32 + 3 * 64,
+  32 + 4 * 64,
+  32 + 5 * 64,
+  32 + 6 * 64,
+  32 + 7 * 64,
+  32 + 8 * 64,
+  32 + 9 * 64,
+  32 + 10 * 64,
+  32 + 11 * 64,
+  32 + 12 * 64,
+  32 + 13 * 64,
+  32 + 14 * 64,
+  32 + 15 * 64		/* Upper 256bit of... %zmmh31 (256 bits each).  */
 };
 
-#define XSAVE_AVX512_ZMM_H_ADDR(tdep, xsave, regnum) \
-  (xsave + xsave_avx512_zmm_h_offset[regnum - I387_ZMM0H_REGNUM (tdep)])
+#define XSAVE_AVX512_ZMM16_H_ADDR(tdep, xsave, regnum)			\
+  (xsave + (tdep)->xsave_layout.zmm_offset				\
+   + xsave_avx512_zmm16_h_offset[regnum - I387_ZMM16H_REGNUM (tdep)])
 
 /* At xsave_pkeys_offset[REGNUM] you find the offset to the location
    of the PKRU register data structure used by the "xsave"
@@ -890,12 +922,13 @@ static int xsave_avx512_zmm_h_offset[] =
 
 static int xsave_pkeys_offset[] =
 {
-2688 + 0 * 8		/* %pkru (64 bits in XSTATE, 32-bit actually used by
+  0 * 8			/* %pkru (64 bits in XSTATE, 32-bit actually used by
 			   instructions and applications).  */
 };
 
-#define XSAVE_PKEYS_ADDR(tdep, xsave, regnum) \
-  (xsave + xsave_pkeys_offset[regnum - I387_PKRU_REGNUM (tdep)])
+#define XSAVE_PKEYS_ADDR(tdep, xsave, regnum)				\
+  (xsave + (tdep)->xsave_layout.pkru_offset				\
+   + xsave_pkeys_offset[regnum - I387_PKRU_REGNUM (tdep)])
 
 
 /* See i387-tdep.h.  */
@@ -994,14 +1027,16 @@ i387_supply_xsave (struct regcache *regcache, int regnum,
       x87 = 0x1,
       sse = 0x2,
       avxh = 0x4,
-      mpx  = 0x8,
-      avx512_k = 0x10,
-      avx512_zmm_h = 0x20,
-      avx512_ymmh_avx512 = 0x40,
-      avx512_xmm_avx512 = 0x80,
-      pkeys = 0x100,
-      all = x87 | sse | avxh | mpx | avx512_k | avx512_zmm_h
-	    | avx512_ymmh_avx512 | avx512_xmm_avx512 | pkeys
+      bndregs = 0x8,
+      bndcfg = 0x10,
+      avx512_k = 0x20,
+      avx512_zmm0_h = 0x40,
+      avx512_zmm16_h = 0x80,
+      avx512_ymmh_avx512 = 0x100,
+      avx512_xmm_avx512 = 0x200,
+      pkeys = 0x400,
+      all = x87 | sse | avxh | bndregs | bndcfg | avx512_k | avx512_zmm0_h
+	    | avx512_zmm16_h | avx512_ymmh_avx512 | avx512_xmm_avx512 | pkeys
     } regclass;
 
   gdb_assert (regs != NULL);
@@ -1014,8 +1049,11 @@ i387_supply_xsave (struct regcache *regcache, int regnum,
 	   && regnum < I387_PKEYSEND_REGNUM (tdep))
     regclass = pkeys;
   else if (regnum >= I387_ZMM0H_REGNUM (tdep)
+	   && regnum < I387_ZMM16H_REGNUM (tdep))
+    regclass = avx512_zmm0_h;
+  else if (regnum >= I387_ZMM16H_REGNUM (tdep)
 	   && regnum < I387_ZMMENDH_REGNUM (tdep))
-    regclass = avx512_zmm_h;
+    regclass = avx512_zmm16_h;
   else if (regnum >= I387_K0_REGNUM (tdep)
 	   && regnum < I387_KEND_REGNUM (tdep))
     regclass = avx512_k;
@@ -1029,8 +1067,11 @@ i387_supply_xsave (struct regcache *regcache, int regnum,
 	   && regnum < I387_YMMENDH_REGNUM (tdep))
     regclass = avxh;
   else if (regnum >= I387_BND0R_REGNUM (tdep)
+	   && regnum < I387_BNDCFGU_REGNUM (tdep))
+    regclass = bndregs;
+  else if (regnum >= I387_BNDCFGU_REGNUM (tdep)
 	   && regnum < I387_MPXEND_REGNUM (tdep))
-    regclass = mpx;
+    regclass = bndcfg;
   else if (regnum >= I387_XMM0_REGNUM (tdep)
 	   && regnum < I387_MXCSR_REGNUM (tdep))
     regclass = sse;
@@ -1063,13 +1104,20 @@ i387_supply_xsave (struct regcache *regcache, int regnum,
 	regcache->raw_supply (regnum, XSAVE_PKEYS_ADDR (tdep, regs, regnum));
       return;
 
-    case avx512_zmm_h:
-      if ((clear_bv & (regnum < zmm_endlo_regnum ? X86_XSTATE_ZMM_H
-						 : X86_XSTATE_ZMM)))
+    case avx512_zmm0_h:
+      if ((clear_bv & X86_XSTATE_ZMM_H))
 	regcache->raw_supply (regnum, zero);
       else
 	regcache->raw_supply (regnum,
-			      XSAVE_AVX512_ZMM_H_ADDR (tdep, regs, regnum));
+			      XSAVE_AVX512_ZMM0_H_ADDR (tdep, regs, regnum));
+      return;
+
+    case avx512_zmm16_h:
+      if ((clear_bv & X86_XSTATE_ZMM))
+	regcache->raw_supply (regnum, zero);
+      else
+	regcache->raw_supply (regnum,
+			      XSAVE_AVX512_ZMM16_H_ADDR (tdep, regs, regnum));
       return;
 
     case avx512_k:
@@ -1102,11 +1150,18 @@ i387_supply_xsave (struct regcache *regcache, int regnum,
 	regcache->raw_supply (regnum, XSAVE_AVXH_ADDR (tdep, regs, regnum));
       return;
 
-    case mpx:
+    case bndcfg:
+      if ((clear_bv & X86_XSTATE_BNDCFG))
+	regcache->raw_supply (regnum, zero);
+      else
+	regcache->raw_supply (regnum, XSAVE_BNDCFG_ADDR (tdep, regs, regnum));
+      return;
+
+    case bndregs:
       if ((clear_bv & X86_XSTATE_BNDREGS))
 	regcache->raw_supply (regnum, zero);
       else
-	regcache->raw_supply (regnum, XSAVE_MPX_ADDR (tdep, regs, regnum));
+	regcache->raw_supply (regnum, XSAVE_BNDREGS_ADDR (tdep, regs, regnum));
       return;
 
     case sse:
@@ -1155,7 +1210,7 @@ i387_supply_xsave (struct regcache *regcache, int regnum,
 	    {
 	      for (i = I387_ZMM0H_REGNUM (tdep); i < zmm_endlo_regnum; i++)
 		regcache->raw_supply (i,
-				      XSAVE_AVX512_ZMM_H_ADDR (tdep, regs, i));
+				      XSAVE_AVX512_ZMM0_H_ADDR (tdep, regs, i));
 	    }
 	}
 
@@ -1183,7 +1238,8 @@ i387_supply_xsave (struct regcache *regcache, int regnum,
 	{
 	  if ((clear_bv & X86_XSTATE_ZMM))
 	    {
-	      for (i = zmm_endlo_regnum; i < I387_ZMMENDH_REGNUM (tdep); i++)
+	      for (i = I387_ZMM16H_REGNUM (tdep);
+		   i < I387_ZMMENDH_REGNUM (tdep); i++)
 		regcache->raw_supply (i, zero);
 	      for (i = I387_YMM16H_REGNUM (tdep);
 		   i < I387_YMMH_AVX512_END_REGNUM (tdep);
@@ -1196,9 +1252,10 @@ i387_supply_xsave (struct regcache *regcache, int regnum,
 	    }
 	  else
 	    {
-	      for (i = zmm_endlo_regnum; i < I387_ZMMENDH_REGNUM (tdep); i++)
+	      for (i = I387_ZMM16H_REGNUM (tdep);
+		   i < I387_ZMMENDH_REGNUM (tdep); i++)
 		regcache->raw_supply (i,
-				      XSAVE_AVX512_ZMM_H_ADDR (tdep, regs, i));
+				      XSAVE_AVX512_ZMM16_H_ADDR (tdep, regs, i));
 	      for (i = I387_YMM16H_REGNUM (tdep);
 		   i < I387_YMMH_AVX512_END_REGNUM (tdep);
 		   i++)
@@ -1241,7 +1298,7 @@ i387_supply_xsave (struct regcache *regcache, int regnum,
 	    {
 	      for (i = I387_BND0R_REGNUM (tdep);
 		   i < I387_BNDCFGU_REGNUM (tdep); i++)
-		regcache->raw_supply (i, XSAVE_MPX_ADDR (tdep, regs, i));
+		regcache->raw_supply (i, XSAVE_BNDREGS_ADDR (tdep, regs, i));
 	    }
 	}
 
@@ -1258,7 +1315,7 @@ i387_supply_xsave (struct regcache *regcache, int regnum,
 	    {
 	      for (i = I387_BNDCFGU_REGNUM (tdep);
 		   i < I387_MPXEND_REGNUM (tdep); i++)
-		regcache->raw_supply (i, XSAVE_MPX_ADDR (tdep, regs, i));
+		regcache->raw_supply (i, XSAVE_BNDCFG_ADDR (tdep, regs, i));
 	    }
 	}
 
@@ -1414,14 +1471,16 @@ i387_collect_xsave (const struct regcache *regcache, int regnum,
       x87 = 0x2,
       sse = 0x4,
       avxh = 0x8,
-      mpx  = 0x10,
-      avx512_k = 0x20,
-      avx512_zmm_h = 0x40,
-      avx512_ymmh_avx512 = 0x80,
-      avx512_xmm_avx512 = 0x100,
-      pkeys = 0x200,
-      all = x87 | sse | avxh | mpx | avx512_k | avx512_zmm_h
-	    | avx512_ymmh_avx512 | avx512_xmm_avx512 | pkeys
+      bndregs = 0x10,
+      bndcfg = 0x20,
+      avx512_k = 0x40,
+      avx512_zmm0_h = 0x80,
+      avx512_zmm16_h = 0x100,
+      avx512_ymmh_avx512 = 0x200,
+      avx512_xmm_avx512 = 0x400,
+      pkeys = 0x800,
+      all = x87 | sse | avxh | bndregs | bndcfg | avx512_k | avx512_zmm0_h
+	    | avx512_zmm16_h | avx512_ymmh_avx512 | avx512_xmm_avx512 | pkeys
     } regclass;
 
   gdb_assert (tdep->st0_regnum >= I386_ST0_REGNUM);
@@ -1433,8 +1492,11 @@ i387_collect_xsave (const struct regcache *regcache, int regnum,
 	   && regnum < I387_PKEYSEND_REGNUM (tdep))
     regclass = pkeys;
   else if (regnum >= I387_ZMM0H_REGNUM (tdep)
+	   && regnum < I387_ZMM16H_REGNUM (tdep))
+    regclass = avx512_zmm0_h;
+  else if (regnum >= I387_ZMM16H_REGNUM (tdep)
 	   && regnum < I387_ZMMENDH_REGNUM (tdep))
-    regclass = avx512_zmm_h;
+    regclass = avx512_zmm16_h;
   else if (regnum >= I387_K0_REGNUM (tdep)
 	   && regnum < I387_KEND_REGNUM (tdep))
     regclass = avx512_k;
@@ -1448,8 +1510,11 @@ i387_collect_xsave (const struct regcache *regcache, int regnum,
 	   && regnum < I387_YMMENDH_REGNUM (tdep))
     regclass = avxh;
   else if (regnum >= I387_BND0R_REGNUM (tdep)
+	   && regnum < I387_BNDCFGU_REGNUM (tdep))
+    regclass = bndregs;
+  else if (regnum >= I387_BNDCFGU_REGNUM (tdep)
 	   && regnum < I387_MPXEND_REGNUM (tdep))
-    regclass = mpx;
+    regclass = bndcfg;
   else if (regnum >= I387_XMM0_REGNUM (tdep)
 	   && regnum < I387_MXCSR_REGNUM (tdep))
     regclass = sse;
@@ -1466,7 +1531,7 @@ i387_collect_xsave (const struct regcache *regcache, int regnum,
   if (gcore)
     {
       /* Clear XSAVE extended state.  */
-      memset (regs, 0, X86_XSTATE_SIZE (tdep->xcr0));
+      memset (regs, 0, tdep->xsave_layout.sizeof_xsave);
 
       /* Update XCR0 and `xstate_bv' with XCR0 for gcore.  */
       if (tdep->xsave_xcr0_offset != -1)
@@ -1501,16 +1566,16 @@ i387_collect_xsave (const struct regcache *regcache, int regnum,
       if ((clear_bv & X86_XSTATE_BNDREGS))
 	for (i = I387_BND0R_REGNUM (tdep);
 	     i < I387_BNDCFGU_REGNUM (tdep); i++)
-	  memset (XSAVE_MPX_ADDR (tdep, regs, i), 0, 16);
+	  memset (XSAVE_BNDREGS_ADDR (tdep, regs, i), 0, 16);
 
       if ((clear_bv & X86_XSTATE_BNDCFG))
 	for (i = I387_BNDCFGU_REGNUM (tdep);
 	     i < I387_MPXEND_REGNUM (tdep); i++)
-	  memset (XSAVE_MPX_ADDR (tdep, regs, i), 0, 8);
+	  memset (XSAVE_BNDCFG_ADDR (tdep, regs, i), 0, 8);
 
       if ((clear_bv & X86_XSTATE_ZMM_H))
 	for (i = I387_ZMM0H_REGNUM (tdep); i < zmm_endlo_regnum; i++)
-	  memset (XSAVE_AVX512_ZMM_H_ADDR (tdep, regs, i), 0, 32);
+	  memset (XSAVE_AVX512_ZMM0_H_ADDR (tdep, regs, i), 0, 32);
 
       if ((clear_bv & X86_XSTATE_K))
 	for (i = I387_K0_REGNUM (tdep);
@@ -1519,8 +1584,9 @@ i387_collect_xsave (const struct regcache *regcache, int regnum,
 
       if ((clear_bv & X86_XSTATE_ZMM))
 	{
-	  for (i = zmm_endlo_regnum; i < I387_ZMMENDH_REGNUM (tdep); i++)
-	    memset (XSAVE_AVX512_ZMM_H_ADDR (tdep, regs, i), 0, 32);
+	  for (i = I387_ZMM16H_REGNUM (tdep); i < I387_ZMMENDH_REGNUM (tdep);
+	       i++)
+	    memset (XSAVE_AVX512_ZMM16_H_ADDR (tdep, regs, i), 0, 32);
 	  for (i = I387_YMM16H_REGNUM (tdep);
 	       i < I387_YMMH_AVX512_END_REGNUM (tdep); i++)
 	    memset (XSAVE_YMM_AVX512_ADDR (tdep, regs, i), 0, 16);
@@ -1583,15 +1649,27 @@ i387_collect_xsave (const struct regcache *regcache, int regnum,
 	  }
 
       /* Check if any ZMMH registers are changed.  */
-      if ((tdep->xcr0 & (X86_XSTATE_ZMM_H | X86_XSTATE_ZMM)))
-	for (i = I387_ZMM0H_REGNUM (tdep);
+      if ((tdep->xcr0 & X86_XSTATE_ZMM))
+	for (i = I387_ZMM16H_REGNUM (tdep);
 	     i < I387_ZMMENDH_REGNUM (tdep); i++)
 	  {
 	    regcache->raw_collect (i, raw);
-	    p = XSAVE_AVX512_ZMM_H_ADDR (tdep, regs, i);
+	    p = XSAVE_AVX512_ZMM16_H_ADDR (tdep, regs, i);
 	    if (memcmp (raw, p, 32) != 0)
 	      {
-		xstate_bv |= (X86_XSTATE_ZMM_H | X86_XSTATE_ZMM);
+		xstate_bv |= X86_XSTATE_ZMM;
+		memcpy (p, raw, 32);
+	      }
+	  }
+
+      if ((tdep->xcr0 & X86_XSTATE_ZMM_H))
+	for (i = I387_ZMM0H_REGNUM (tdep); i < zmm_endlo_regnum; i++)
+	  {
+	    regcache->raw_collect (i, raw);
+	    p = XSAVE_AVX512_ZMM0_H_ADDR (tdep, regs, i);
+	    if (memcmp (raw, p, 32) != 0)
+	      {
+		xstate_bv |= X86_XSTATE_ZMM_H;
 		memcpy (p, raw, 32);
 	      }
 	  }
@@ -1643,7 +1721,7 @@ i387_collect_xsave (const struct regcache *regcache, int regnum,
 	     i < I387_BNDCFGU_REGNUM (tdep); i++)
 	  {
 	    regcache->raw_collect (i, raw);
-	    p = XSAVE_MPX_ADDR (tdep, regs, i);
+	    p = XSAVE_BNDREGS_ADDR (tdep, regs, i);
 	    if (memcmp (raw, p, 16))
 	      {
 		xstate_bv |= X86_XSTATE_BNDREGS;
@@ -1657,7 +1735,7 @@ i387_collect_xsave (const struct regcache *regcache, int regnum,
 	     i < I387_MPXEND_REGNUM (tdep); i++)
 	  {
 	    regcache->raw_collect (i, raw);
-	    p = XSAVE_MPX_ADDR (tdep, regs, i);
+	    p = XSAVE_BNDCFG_ADDR (tdep, regs, i);
 	    if (memcmp (raw, p, 8))
 	      {
 		xstate_bv |= X86_XSTATE_BNDCFG;
@@ -1747,15 +1825,26 @@ i387_collect_xsave (const struct regcache *regcache, int regnum,
 	    }
 	  break;
 
-	case avx512_zmm_h:
-	  /* This is a ZMM register.  */
-	  p = XSAVE_AVX512_ZMM_H_ADDR (tdep, regs, regnum);
+	case avx512_zmm16_h:
+	  /* This is a ZMM16-31 register.  */
+	  p = XSAVE_AVX512_ZMM16_H_ADDR (tdep, regs, regnum);
 	  if (memcmp (raw, p, 32) != 0)
 	    {
-	      xstate_bv |= (X86_XSTATE_ZMM_H | X86_XSTATE_ZMM);
+	      xstate_bv |= X86_XSTATE_ZMM;
 	      memcpy (p, raw, 32);
 	    }
 	  break;
+
+	case avx512_zmm0_h:
+	  /* This is a ZMM0-15 register.  */
+	  p = XSAVE_AVX512_ZMM0_H_ADDR (tdep, regs, regnum);
+	  if (memcmp (raw, p, 32) != 0)
+	    {
+	      xstate_bv |= X86_XSTATE_ZMM_H;
+	      memcpy (p, raw, 32);
+	    }
+	  break;
+
 	case avx512_k:
 	  /* This is a AVX512 mask register.  */
 	  p = XSAVE_AVX512_K_ADDR (tdep, regs, regnum);
@@ -1796,25 +1885,22 @@ i387_collect_xsave (const struct regcache *regcache, int regnum,
 	    }
 	  break;
 
-	case mpx:
-	  if (regnum < I387_BNDCFGU_REGNUM (tdep))
+	case bndregs:
+	  regcache->raw_collect (regnum, raw);
+	  p = XSAVE_BNDREGS_ADDR (tdep, regs, regnum);
+	  if (memcmp (raw, p, 16))
 	    {
-	      regcache->raw_collect (regnum, raw);
-	      p = XSAVE_MPX_ADDR (tdep, regs, regnum);
-	      if (memcmp (raw, p, 16))
-		{
-		  xstate_bv |= X86_XSTATE_BNDREGS;
-		  memcpy (p, raw, 16);
-		}
-	    }
-	  else
-	    {
-	      p = XSAVE_MPX_ADDR (tdep, regs, regnum);
-	      xstate_bv |= X86_XSTATE_BNDCFG;
-	      memcpy (p, raw, 8);
+	      xstate_bv |= X86_XSTATE_BNDREGS;
+	      memcpy (p, raw, 16);
 	    }
 	  break;
 
+	case bndcfg:
+	  p = XSAVE_BNDCFG_ADDR (tdep, regs, regnum);
+	  xstate_bv |= X86_XSTATE_BNDCFG;
+	  memcpy (p, raw, 8);
+	  break;
+
 	case sse:
 	  /* This is an SSE register.  */
 	  p = FXSAVE_ADDR (tdep, regs, regnum);
diff --git a/gdb/i387-tdep.h b/gdb/i387-tdep.h
index 4e1403aa753..f6c2ca627eb 100644
--- a/gdb/i387-tdep.h
+++ b/gdb/i387-tdep.h
@@ -51,6 +51,7 @@ struct x86_xsave_layout;
 #define I387_K0_REGNUM(tdep) ((tdep)->k0_regnum)
 #define I387_NUM_ZMMH_REGS(tdep) ((tdep)->num_zmm_regs)
 #define I387_ZMM0H_REGNUM(tdep) ((tdep)->zmm0h_regnum)
+#define I387_ZMM16H_REGNUM(tdep) ((tdep)->zmm0h_regnum + 16)
 #define I387_NUM_YMM_AVX512_REGS(tdep) ((tdep)->num_ymm_avx512_regs)
 #define I387_YMM16H_REGNUM(tdep) ((tdep)->ymm16h_regnum)
 
-- 
2.34.1

[PATCH v3 10/13] gdbserver: Add a function to set the XSAVE mask and size.

[John Baldwin]

Make x86_xcr0 private to i387-fp.cc and use i387_set_xsave_mask to set
the value instead.  Add a static global instance of x86_xsave_layout
and initialize it in the new function as well to be used in a future
commit to parse XSAVE extended state regions.

Update the Linux port to use this function rather than setting
x86_xcr0 directly.  In the case that XML is not supported, don't
bother setting x86_xcr0 to the default value but just omit the call to
i387_set_xsave_mask as i387-fp.cc defaults to the SSE case used for
non-XML.

In addition, use x86_xsave_length to determine the size of the XSAVE
register set via CPUID.
---
 gdbserver/configure.srv    | 12 ++++++++----
 gdbserver/i387-fp.cc       | 15 +++++++++++++--
 gdbserver/i387-fp.h        |  2 +-
 gdbserver/linux-x86-low.cc | 10 ++++++----
 4 files changed, 28 insertions(+), 11 deletions(-)

diff --git a/gdbserver/configure.srv b/gdbserver/configure.srv
index d37053628fc..c19b595626a 100644
--- a/gdbserver/configure.srv
+++ b/gdbserver/configure.srv
@@ -97,7 +97,8 @@ case "${gdbserver_host}" in
   i[34567]86-*-linux*)	srv_tgtobj="${srv_tgtobj} arch/i386.o"
 			srv_tgtobj="${srv_tgtobj} $srv_linux_obj"
 			srv_tgtobj="${srv_tgtobj} linux-x86-low.o x86-low.o"
-			srv_tgtobj="${srv_tgtobj} nat/x86-dregs.o i387-fp.o"
+			srv_tgtobj="${srv_tgtobj} nat/x86-dregs.o"
+			srv_tgtobj="${srv_tgtobj} nat/x86-xstate.o i387-fp.o"
 			srv_tgtobj="${srv_tgtobj} linux-x86-tdesc.o"
 			srv_tgtobj="${srv_tgtobj} nat/linux-btrace.o"
 			srv_tgtobj="${srv_tgtobj} nat/x86-linux.o"
@@ -351,7 +352,8 @@ case "${gdbserver_host}" in
 			srv_linux_thread_db=yes
 			;;
   x86_64-*-linux*)	srv_tgtobj="$srv_linux_obj linux-x86-low.o x86-low.o"
-			srv_tgtobj="${srv_tgtobj} nat/x86-dregs.o i387-fp.o"
+			srv_tgtobj="${srv_tgtobj} nat/x86-dregs.o"
+			srv_tgtobj="${srv_tgtobj} nat/x86-xstate.o i387-fp.o"
 			srv_tgtobj="${srv_tgtobj} arch/i386.o arch/amd64.o"
 			srv_tgtobj="${srv_tgtobj} linux-x86-tdesc.o"
 			srv_tgtobj="${srv_tgtobj} nat/linux-btrace.o"
@@ -366,14 +368,16 @@ case "${gdbserver_host}" in
 			ipa_obj="${ipa_obj} arch/amd64-ipa.o"
 			;;
   x86_64-*-mingw*)	srv_regobj=""
-			srv_tgtobj="x86-low.o nat/x86-dregs.o i387-fp.o"
+			srv_tgtobj="x86-low.o nat/x86-dregs.o"
+			srv_tgtobj="${srv_tgtobj} nat/x86-xstate.o i387-fp.o"
 			srv_tgtobj="${srv_tgtobj} win32-low.o win32-i386-low.o"
 			srv_tgtobj="${srv_tgtobj} nat/windows-nat.o"
 			srv_tgtobj="${srv_tgtobj} arch/amd64.o arch/i386.o"
 			srv_mingw=yes
 			;;
   x86_64-*-cygwin*)	srv_regobj=""
-			srv_tgtobj="x86-low.o nat/x86-dregs.o i387-fp.o"
+			srv_tgtobj="x86-low.o nat/x86-dregs.o"
+			srv_tgtobj="${srv_tgtobj} nat/x86-xstate.o i387-fp.o"
 			srv_tgtobj="${srv_tgtobj} win32-low.o win32-i386-low.o"
 			srv_tgtobj="${srv_tgtobj} nat/windows-nat.o"
 			srv_tgtobj="${srv_tgtobj} arch/amd64.o arch/i386.o"
diff --git a/gdbserver/i387-fp.cc b/gdbserver/i387-fp.cc
index 674889674f1..f419e74d608 100644
--- a/gdbserver/i387-fp.cc
+++ b/gdbserver/i387-fp.cc
@@ -19,12 +19,18 @@
 #include "server.h"
 #include "i387-fp.h"
 #include "gdbsupport/x86-xstate.h"
+#include "nat/x86-xstate.h"
+
+/* Default to SSE.  */
+static unsigned long long x86_xcr0 = X86_XSTATE_SSE_MASK;
 
 static const int num_mpx_bnd_registers = 4;
 static const int num_mpx_cfg_registers = 2;
 static const int num_avx512_k_registers = 8;
 static const int num_pkeys_registers = 1;
 
+static x86_xsave_layout xsave_layout;
+
 /* Note: These functions preserve the reserved bits in control registers.
    However, gdbserver promptly throws away that information.  */
 
@@ -974,5 +980,10 @@ i387_xsave_to_cache (struct regcache *regcache, const void *buf)
     }
 }
 
-/* Default to SSE.  */
-unsigned long long x86_xcr0 = X86_XSTATE_SSE_MASK;
+/* Set the XSAVE mask and fetch the XSAVE layout via CPUID.  */
+void
+i387_set_xsave_mask (uint64_t xcr0, int len)
+{
+  x86_xcr0 = xcr0;
+  x86_fetch_xsave_layout (xcr0, len, xsave_layout);
+}
diff --git a/gdbserver/i387-fp.h b/gdbserver/i387-fp.h
index f9b4bc75849..ede16f7d901 100644
--- a/gdbserver/i387-fp.h
+++ b/gdbserver/i387-fp.h
@@ -28,6 +28,6 @@ void i387_fxsave_to_cache (struct regcache *regcache, const void *buf);
 void i387_cache_to_xsave (struct regcache *regcache, void *buf);
 void i387_xsave_to_cache (struct regcache *regcache, const void *buf);
 
-extern unsigned long long x86_xcr0;
+void i387_set_xsave_mask (uint64_t xcr0, int len);
 
 #endif /* GDBSERVER_I387_FP_H */
diff --git a/gdbserver/linux-x86-low.cc b/gdbserver/linux-x86-low.cc
index d2b55f6f0d2..f909a504798 100644
--- a/gdbserver/linux-x86-low.cc
+++ b/gdbserver/linux-x86-low.cc
@@ -25,6 +25,7 @@
 #include "i387-fp.h"
 #include "x86-low.h"
 #include "gdbsupport/x86-xstate.h"
+#include "nat/x86-xstate.h"
 #include "nat/gdb_ptrace.h"
 
 #ifdef __x86_64__
@@ -871,6 +872,7 @@ x86_linux_read_description (void)
   int xcr0_features;
   int tid;
   static uint64_t xcr0;
+  static int xsave_len;
   struct regset_info *regset;
 
   tid = lwpid_of (current_thread);
@@ -905,8 +907,6 @@ x86_linux_read_description (void)
 
   if (!use_xml)
     {
-      x86_xcr0 = X86_XSTATE_SSE_MASK;
-
       /* Don't use XML.  */
 #ifdef __x86_64__
       if (machine == EM_X86_64)
@@ -936,11 +936,13 @@ x86_linux_read_description (void)
 	  xcr0 = xstateregs[(I386_LINUX_XSAVE_XCR0_OFFSET
 			     / sizeof (uint64_t))];
 
+	  xsave_len = x86_xsave_length ();
+
 	  /* Use PTRACE_GETREGSET if it is available.  */
 	  for (regset = x86_regsets;
 	       regset->fill_function != NULL; regset++)
 	    if (regset->get_request == PTRACE_GETREGSET)
-	      regset->size = X86_XSTATE_SIZE (xcr0);
+	      regset->size = xsave_len;
 	    else if (regset->type != GENERAL_REGS)
 	      regset->size = 0;
 	}
@@ -951,7 +953,7 @@ x86_linux_read_description (void)
 		   && (xcr0 & X86_XSTATE_ALL_MASK));
 
   if (xcr0_features)
-    x86_xcr0 = xcr0;
+    i387_set_xsave_mask (xcr0, xsave_len);
 
   if (machine == EM_X86_64)
     {
-- 
2.34.1

[PATCH v3 11/13] gdbserver: Refactor the legacy region within the xsave struct

[John Baldwin]

From: Aleksandar Paunovic <aleksandar.paunovic@intel.com>

Legacy fields of the XSAVE area are already defined within fx_save
struct.  Reuse this struct to remove code duplication.

The two changed functions are called within all tests which run
gdbserver.

gdbserver/ChangeLog:
2021-03-11  Aleksandar Paunovic  <aleksandar.paunovic@intel.com>

	* i387-fp.cc (struct i387_xsave): Remove code duplication.
        (i387_cache_to_xsave): Refactoring.
        (i387_xsave_to_cache): Refactoring.

Signed-off-by: Aleksandar Paunovic <aleksandar.paunovic@intel.com>
---
 gdbserver/i387-fp.cc | 110 ++++++++++++++++++-------------------------
 1 file changed, 45 insertions(+), 65 deletions(-)

diff --git a/gdbserver/i387-fp.cc b/gdbserver/i387-fp.cc
index f419e74d608..309c3cc42db 100644
--- a/gdbserver/i387-fp.cc
+++ b/gdbserver/i387-fp.cc
@@ -82,27 +82,7 @@ struct i387_fxsave {
 };
 
 struct i387_xsave {
-  /* All these are only sixteen bits, plus padding, except for fop (which
-     is only eleven bits), and fooff / fioff (which are 32 bits each).  */
-  unsigned short fctrl;
-  unsigned short fstat;
-  unsigned short ftag;
-  unsigned short fop;
-  unsigned int fioff;
-  unsigned short fiseg;
-  unsigned short pad1;
-  unsigned int fooff;
-  unsigned short foseg;
-  unsigned short pad12;
-
-  unsigned int mxcsr;
-  unsigned int mxcsr_mask;
-
-  /* Space for eight 80-bit FP values in 128-bit spaces.  */
-  unsigned char st_space[128];
-
-  /* Space for eight 128-bit XMM values, or 16 on x86-64.  */
-  unsigned char xmm_space[256];
+  struct i387_fxsave fx;
 
   unsigned char reserved1[48];
 
@@ -286,28 +266,28 @@ i387_cache_to_xsave (struct regcache *regcache, void *buf)
       if ((clear_bv & X86_XSTATE_X87))
 	{
 	  for (i = 0; i < 8; i++)
-	    memset (((char *) &fp->st_space[0]) + i * 16, 0, 10);
+	    memset (((char *) &fp->fx.st_space[0]) + i * 16, 0, 10);
 
-	  fp->fioff = 0;
-	  fp->fooff = 0;
-	  fp->fctrl = I387_FCTRL_INIT_VAL;
-	  fp->fstat = 0;
-	  fp->ftag = 0;
-	  fp->fiseg = 0;
-	  fp->foseg = 0;
-	  fp->fop = 0;
+	  fp->fx.fioff = 0;
+	  fp->fx.fooff = 0;
+	  fp->fx.fctrl = I387_FCTRL_INIT_VAL;
+	  fp->fx.fstat = 0;
+	  fp->fx.ftag = 0;
+	  fp->fx.fiseg = 0;
+	  fp->fx.foseg = 0;
+	  fp->fx.fop = 0;
 	}
 
       if ((clear_bv & X86_XSTATE_SSE))
 	for (i = 0; i < num_xmm_registers; i++)
-	  memset (((char *) &fp->xmm_space[0]) + i * 16, 0, 16);
+	  memset (((char *) &fp->fx.xmm_space[0]) + i * 16, 0, 16);
 
       if ((clear_bv & X86_XSTATE_AVX))
 	for (i = 0; i < num_xmm_registers; i++)
 	  memset (((char *) &fp->ymmh_space[0]) + i * 16, 0, 16);
 
       if ((clear_bv & X86_XSTATE_SSE) && (clear_bv & X86_XSTATE_AVX))
-	memset (((char *) &fp->mxcsr), 0, 4);
+	memset (((char *) &fp->fx.mxcsr), 0, 4);
 
       if ((clear_bv & X86_XSTATE_BNDREGS))
 	for (i = 0; i < num_mpx_bnd_registers; i++)
@@ -348,7 +328,7 @@ i387_cache_to_xsave (struct regcache *regcache, void *buf)
       for (i = 0; i < 8; i++)
 	{
 	  collect_register (regcache, i + st0_regnum, raw);
-	  p = ((char *) &fp->st_space[0]) + i * 16;
+	  p = ((char *) &fp->fx.st_space[0]) + i * 16;
 	  if (memcmp (raw, p, 10))
 	    {
 	      xstate_bv |= X86_XSTATE_X87;
@@ -365,7 +345,7 @@ i387_cache_to_xsave (struct regcache *regcache, void *buf)
       for (i = 0; i < num_xmm_registers; i++) 
 	{
 	  collect_register (regcache, i + xmm0_regnum, raw);
-	  p = ((char *) &fp->xmm_space[0]) + i * 16;
+	  p = ((char *) &fp->fx.xmm_space[0]) + i * 16;
 	  if (memcmp (raw, p, 16))
 	    {
 	      xstate_bv |= X86_XSTATE_SSE;
@@ -536,53 +516,53 @@ i387_cache_to_xsave (struct regcache *regcache, void *buf)
   if ((x86_xcr0 & X86_XSTATE_SSE) || (x86_xcr0 & X86_XSTATE_AVX))
     {
       collect_register_by_name (regcache, "mxcsr", raw);
-      if (memcmp (raw, &fp->mxcsr, 4) != 0)
+      if (memcmp (raw, &fp->fx.mxcsr, 4) != 0)
 	{
 	  if (((fp->xstate_bv | xstate_bv)
 	       & (X86_XSTATE_SSE | X86_XSTATE_AVX)) == 0)
 	    xstate_bv |= X86_XSTATE_SSE;
-	  memcpy (&fp->mxcsr, raw, 4);
+	  memcpy (&fp->fx.mxcsr, raw, 4);
 	}
     }
 
   if (x86_xcr0 & X86_XSTATE_X87)
     {
       collect_register_by_name (regcache, "fioff", raw);
-      if (memcmp (raw, &fp->fioff, 4) != 0)
+      if (memcmp (raw, &fp->fx.fioff, 4) != 0)
 	{
 	  xstate_bv |= X86_XSTATE_X87;
-	  memcpy (&fp->fioff, raw, 4);
+	  memcpy (&fp->fx.fioff, raw, 4);
 	}
 
       collect_register_by_name (regcache, "fooff", raw);
-      if (memcmp (raw, &fp->fooff, 4) != 0)
+      if (memcmp (raw, &fp->fx.fooff, 4) != 0)
 	{
 	  xstate_bv |= X86_XSTATE_X87;
-	  memcpy (&fp->fooff, raw, 4);
+	  memcpy (&fp->fx.fooff, raw, 4);
 	}
 
       /* This one's 11 bits... */
       val2 = regcache_raw_get_unsigned_by_name (regcache, "fop");
-      val2 = (val2 & 0x7FF) | (fp->fop & 0xF800);
-      if (fp->fop != val2)
+      val2 = (val2 & 0x7FF) | (fp->fx.fop & 0xF800);
+      if (fp->fx.fop != val2)
 	{
 	  xstate_bv |= X86_XSTATE_X87;
-	  fp->fop = val2;
+	  fp->fx.fop = val2;
 	}
 
       /* Some registers are 16-bit.  */
       val = regcache_raw_get_unsigned_by_name (regcache, "fctrl");
-      if (fp->fctrl != val)
+      if (fp->fx.fctrl != val)
 	{
 	  xstate_bv |= X86_XSTATE_X87;
-	  fp->fctrl = val;
+	  fp->fx.fctrl = val;
 	}
 
       val = regcache_raw_get_unsigned_by_name (regcache, "fstat");
-      if (fp->fstat != val)
+      if (fp->fx.fstat != val)
 	{
 	  xstate_bv |= X86_XSTATE_X87;
-	  fp->fstat = val;
+	  fp->fx.fstat = val;
 	}
 
       /* Convert to the simplifed tag form stored in fxsave data.  */
@@ -595,24 +575,24 @@ i387_cache_to_xsave (struct regcache *regcache, void *buf)
 	  if (tag != 3)
 	    val2 |= (1 << i);
 	}
-      if (fp->ftag != val2)
+      if (fp->fx.ftag != val2)
 	{
 	  xstate_bv |= X86_XSTATE_X87;
-	  fp->ftag = val2;
+	  fp->fx.ftag = val2;
 	}
 
       val = regcache_raw_get_unsigned_by_name (regcache, "fiseg");
-      if (fp->fiseg != val)
+      if (fp->fx.fiseg != val)
 	{
 	  xstate_bv |= X86_XSTATE_X87;
-	  fp->fiseg = val;
+	  fp->fx.fiseg = val;
 	}
 
       val = regcache_raw_get_unsigned_by_name (regcache, "foseg");
-      if (fp->foseg != val)
+      if (fp->fx.foseg != val)
 	{
 	  xstate_bv |= X86_XSTATE_X87;
-	  fp->foseg = val;
+	  fp->fx.foseg = val;
 	}
     }
 
@@ -757,7 +737,7 @@ i387_xsave_to_cache (struct regcache *regcache, const void *buf)
 	}
       else
 	{
-	  p = (gdb_byte *) &fp->st_space[0];
+	  p = (gdb_byte *) &fp->fx.st_space[0];
 	  for (i = 0; i < 8; i++)
 	    supply_register (regcache, i + st0_regnum, p + i * 16);
 	}
@@ -774,7 +754,7 @@ i387_xsave_to_cache (struct regcache *regcache, const void *buf)
 	}
       else
 	{
-	  p = (gdb_byte *) &fp->xmm_space[0];
+	  p = (gdb_byte *) &fp->fx.xmm_space[0];
 	  for (i = 0; i < num_xmm_registers; i++)
 	    supply_register (regcache, i + xmm0_regnum, p + i * 16);
 	}
@@ -924,7 +904,7 @@ i387_xsave_to_cache (struct regcache *regcache, const void *buf)
       supply_register_by_name (regcache, "mxcsr", &default_mxcsr);
     }
   else
-    supply_register_by_name (regcache, "mxcsr", &fp->mxcsr);
+    supply_register_by_name (regcache, "mxcsr", &fp->fx.mxcsr);
 
   if ((clear_bv & X86_XSTATE_X87) != 0)
     {
@@ -945,23 +925,23 @@ i387_xsave_to_cache (struct regcache *regcache, const void *buf)
     }
   else
     {
-      supply_register_by_name (regcache, "fioff", &fp->fioff);
-      supply_register_by_name (regcache, "fooff", &fp->fooff);
+      supply_register_by_name (regcache, "fioff", &fp->fx.fioff);
+      supply_register_by_name (regcache, "fooff", &fp->fx.fooff);
 
       /* Some registers are 16-bit.  */
-      val = fp->fctrl & 0xFFFF;
+      val = fp->fx.fctrl & 0xFFFF;
       supply_register_by_name (regcache, "fctrl", &val);
 
-      val = fp->fstat & 0xFFFF;
+      val = fp->fx.fstat & 0xFFFF;
       supply_register_by_name (regcache, "fstat", &val);
 
       /* Generate the form of ftag data that GDB expects.  */
-      top = (fp->fstat >> 11) & 0x7;
+      top = (fp->fx.fstat >> 11) & 0x7;
       val = 0;
       for (i = 7; i >= 0; i--)
 	{
 	  int tag;
-	  if (fp->ftag & (1 << i))
+	  if (fp->fx.ftag & (1 << i))
 	    tag = i387_ftag (fxp, (i + 8 - top) % 8);
 	  else
 	    tag = 3;
@@ -969,13 +949,13 @@ i387_xsave_to_cache (struct regcache *regcache, const void *buf)
 	}
       supply_register_by_name (regcache, "ftag", &val);
 
-      val = fp->fiseg & 0xFFFF;
+      val = fp->fx.fiseg & 0xFFFF;
       supply_register_by_name (regcache, "fiseg", &val);
 
-      val = fp->foseg & 0xFFFF;
+      val = fp->fx.foseg & 0xFFFF;
       supply_register_by_name (regcache, "foseg", &val);
 
-      val = (fp->fop) & 0x7FF;
+      val = (fp->fx.fop) & 0x7FF;
       supply_register_by_name (regcache, "fop", &val);
     }
 }
-- 
2.34.1

[PATCH v3 12/13] gdbserver: Read offsets of the XSAVE extended region via CPUID

[John Baldwin]

From: Aleksandar Paunovic <aleksandar.paunovic@intel.com>

The legacy region and the XSAVE header region are refactored to be a
part of a class.  The extended region memory offsets are changed.
The offsets for XSAVE components AVX, MPX, BNDREGS, BNDCSR, AVX-512, PRKU
and AMX were previously hardcoded by having a fixed position within the
xsave structure.  Memory offsets for these components is now computed by
adding the relevant offsets from xsave_layout.

SDM recommends to obtain memory offsets values only by calling CPUID
because they can change depending on the CPU mode.

Two functions: i387_xsave_to_cache and i387_cache_to_xsave are
refactored but the logic on how they deal with the XSAVE area remains
the same.

The two changed functions are called within all tests which runs
gdbserver.

gdbserver/ChangeLog:
2021-06-16  Aleksandar Paunovic  <aleksandar.paunovic@intel.com>

        * i387-fp.cc (struct i387_xsave): Change into class.
	  (i387_cache_to_xsave): Refactoring.
          (i387_xsave_to_cache): Refactoring.

Signed-off-by: Aleksandar Paunovic <aleksandar.paunovic@intel.com>
Co-authored-by: John Baldwin <jhb@FreeBSD.org>
---
 gdbserver/i387-fp.cc | 275 +++++++++++++++++++++++++++----------------
 1 file changed, 175 insertions(+), 100 deletions(-)

diff --git a/gdbserver/i387-fp.cc b/gdbserver/i387-fp.cc
index 309c3cc42db..64644821e46 100644
--- a/gdbserver/i387-fp.cc
+++ b/gdbserver/i387-fp.cc
@@ -81,11 +81,11 @@ struct i387_fxsave {
   unsigned char xmm_space[256];
 };
 
-struct i387_xsave {
+struct xsave_fixed_addresses {
+  /* Size of i387_fxsave is 416 bytes.  */
   struct i387_fxsave fx;
 
   unsigned char reserved1[48];
-
   /* The extended control register 0 (the XFEATURE_ENABLED_MASK
      register).  */
   unsigned long long xcr0;
@@ -95,34 +95,110 @@ struct i387_xsave {
   /* The XSTATE_BV bit vector.  */
   unsigned long long xstate_bv;
 
-  unsigned char reserved3[56];
+  /* The XCOMP_BV bit vector.  */
+  unsigned long long xcomp_bv;
 
-  /* Space for eight upper 128-bit YMM values, or 16 on x86-64.  */
-  unsigned char ymmh_space[256];
+  unsigned char reserved3[48];
 
-  unsigned char reserved4[128];
+  /* Byte 576.  End of registers with fixed position in XSAVE.
+     The position of other XSAVE registers will be calculated
+     from the appropriate CPUID calls.  */
+};
 
-  /* Space for 4 bound registers values of 128 bits.  */
-  unsigned char mpx_bnd_space[64];
+class i387_xsave {
+  /* Pointer to the legacy region of the XSAVE area.  Legacy region
+     has fixed memory offsets.  */
+  struct xsave_fixed_addresses *xsave_fixed =  nullptr;
 
-  /* Space for 2 MPX configuration registers of 64 bits
+  /* Pointer to the XSAVE area.  The purpose of the pointer is to use it
+     as a base for calculating offsets of the extended region.  For example:
+     avx_offset = xsave + CPUID_CALL_FOR (AVX);  */
+  unsigned char *xsave = nullptr;
+
+public:
+  i387_xsave (const void *buf):
+    xsave_fixed {(struct xsave_fixed_addresses *) buf},
+    xsave {(unsigned char *) buf} {}
+
+  i387_fxsave* get_fxsave () const
+    {
+      return &xsave_fixed->fx;
+    }
+
+  /* Get a reference to xcr0 register.  */
+  unsigned long long& xcr0 () const
+    {
+      return xsave_fixed->xcr0;
+    }
+
+  /* Get a reference to xstate_bv register.  */
+  unsigned long long& xstate_bv () const
+    {
+      return xsave_fixed->xstate_bv;
+    }
+
+  /* Get a reference to xcomp_bv register.  */
+  unsigned long long& xcomp_bv () const
+    {
+      return xsave_fixed->xcomp_bv;
+    }
+
+  /* Memory address of ST values.  */
+  unsigned char* st_space () const
+    {
+      return &get_fxsave ()->st_space[0];
+    }
+
+  /* Memory address of XMM values.  */
+  unsigned char* xmm_space () const
+    {
+      return &get_fxsave ()->xmm_space[0];
+    }
+
+  /* Memory address of eight upper 128-bit YMM values, or 16 on x86-64.  */
+  unsigned char* ymmh_space () const
+    {
+      return xsave + xsave_layout.avx_offset;
+    }
+
+  /* Memory address of 4 bound registers values of 128 bits.  */
+  unsigned char* mpx_bnd_space () const
+    {
+      return xsave + xsave_layout.bndregs_offset;
+    }
+
+  /* Memory address of 2 MPX configuration registers of 64 bits
      plus reserved space.  */
-  unsigned char mpx_cfg_space[16];
+  unsigned char* mpx_cfg_space () const
+    {
+      return xsave + xsave_layout.bndcfg_offset;
+    }
 
-  unsigned char reserved5[48];
+  /* Memory address of 8 OpMask register values of 64 bits.  */
+  unsigned char* k_space () const
+    {
+      return xsave + xsave_layout.k_offset;
+    }
 
-  /* Space for 8 OpMask register values of 64 bits.  */
-  unsigned char k_space[64];
+  /* Memory address of 16 256-bit zmm0-15.  */
+  unsigned char* zmmh_low_space () const
+    {
+      return xsave + xsave_layout.zmm_h_offset;
+    }
 
-  /* Space for 16 256-bit zmm0-15.  */
-  unsigned char zmmh_low_space[512];
+  /* Memory address of 16 512-bit zmm16-31 values.  */
+  unsigned char* zmmh_high_space () const
+    {
+      return xsave + xsave_layout.zmm_offset;
+    }
 
-  /* Space for 16 512-bit zmm16-31 values.  */
-  unsigned char zmmh_high_space[1024];
-
-  /* Space for 1 32-bit PKRU register.  The HW XSTATE size for this feature is
-     actually 64 bits, but WRPKRU/RDPKRU instructions ignore upper 32 bits.  */
-  unsigned char pkru_space[8];
+  /* Memory address of 1 32-bit PKRU register.  The HW XSTATE size for this
+     feature is actually 64 bits, but WRPKRU/RDPKRU instructions ignore upper
+     32 bits.  */
+  unsigned char* pkru_space () const
+    {
+      return xsave + xsave_layout.pkru_offset;
+    }
 };
 
 void
@@ -237,15 +313,14 @@ i387_cache_to_fxsave (struct regcache *regcache, void *buf)
 void
 i387_cache_to_xsave (struct regcache *regcache, void *buf)
 {
-  struct i387_xsave *fp = (struct i387_xsave *) buf;
+  i387_xsave fp {buf};
   bool amd64 = register_size (regcache->tdesc, 0) == 8;
   int i;
   unsigned long val, val2;
   unsigned long long xstate_bv = 0;
   unsigned long long clear_bv = 0;
   char raw[64];
-  char *p;
-
+  unsigned char *p;
   /* Amd64 has 16 xmm regs; I386 has 8 xmm regs.  */
   int num_xmm_registers = amd64 ? 16 : 8;
   /* AVX512 extends the existing xmm/ymm registers to a wider mode: zmm.  */
@@ -257,7 +332,7 @@ i387_cache_to_xsave (struct regcache *regcache, void *buf)
 
   /* The supported bits in `xstat_bv' are 8 bytes.  Clear part in
      vector registers if its bit in xstat_bv is zero.  */
-  clear_bv = (~fp->xstate_bv) & x86_xcr0;
+  clear_bv = (~fp.xstate_bv ()) & x86_xcr0;
 
   /* Clear part in x87 and vector registers if its bit in xstat_bv is
      zero.  */
@@ -266,58 +341,59 @@ i387_cache_to_xsave (struct regcache *regcache, void *buf)
       if ((clear_bv & X86_XSTATE_X87))
 	{
 	  for (i = 0; i < 8; i++)
-	    memset (((char *) &fp->fx.st_space[0]) + i * 16, 0, 10);
+	    memset (fp.st_space () + i * 16, 0, 10);
 
-	  fp->fx.fioff = 0;
-	  fp->fx.fooff = 0;
-	  fp->fx.fctrl = I387_FCTRL_INIT_VAL;
-	  fp->fx.fstat = 0;
-	  fp->fx.ftag = 0;
-	  fp->fx.fiseg = 0;
-	  fp->fx.foseg = 0;
-	  fp->fx.fop = 0;
+	  fp.get_fxsave ()->fioff = 0;
+	  fp.get_fxsave ()->fooff = 0;
+	  fp.get_fxsave ()->fctrl = I387_FCTRL_INIT_VAL;
+	  fp.get_fxsave ()->fstat = 0;
+	  fp.get_fxsave ()->ftag = 0;
+	  fp.get_fxsave ()->fiseg = 0;
+	  fp.get_fxsave ()->foseg = 0;
+	  fp.get_fxsave ()->fop = 0;
 	}
 
       if ((clear_bv & X86_XSTATE_SSE))
 	for (i = 0; i < num_xmm_registers; i++)
-	  memset (((char *) &fp->fx.xmm_space[0]) + i * 16, 0, 16);
+	  memset (fp.xmm_space () + i * 16, 0, 16);
+
 
       if ((clear_bv & X86_XSTATE_AVX))
 	for (i = 0; i < num_xmm_registers; i++)
-	  memset (((char *) &fp->ymmh_space[0]) + i * 16, 0, 16);
+	  memset (fp.ymmh_space () + i * 16, 0, 16);
 
       if ((clear_bv & X86_XSTATE_SSE) && (clear_bv & X86_XSTATE_AVX))
-	memset (((char *) &fp->fx.mxcsr), 0, 4);
+	memset (((char *) &fp.get_fxsave ()->mxcsr), 0, 4);
 
       if ((clear_bv & X86_XSTATE_BNDREGS))
 	for (i = 0; i < num_mpx_bnd_registers; i++)
-	  memset (((char *) &fp->mpx_bnd_space[0]) + i * 16, 0, 16);
+	  memset (fp.mpx_bnd_space () + i * 16, 0, 16);
 
       if ((clear_bv & X86_XSTATE_BNDCFG))
 	for (i = 0; i < num_mpx_cfg_registers; i++)
-	  memset (((char *) &fp->mpx_cfg_space[0]) + i * 8, 0, 8);
+	  memset (fp.mpx_cfg_space () + i * 8, 0, 8);
 
       if ((clear_bv & X86_XSTATE_K))
 	for (i = 0; i < num_avx512_k_registers; i++)
-	  memset (((char *) &fp->k_space[0]) + i * 8, 0, 8);
+	  memset (fp.k_space () + i * 8, 0, 8);
 
       if ((clear_bv & X86_XSTATE_ZMM_H))
 	for (i = 0; i < num_avx512_zmmh_low_registers; i++)
-	  memset (((char *) &fp->zmmh_low_space[0]) + i * 32, 0, 32);
+	  memset (fp.zmmh_low_space () + i * 32, 0, 32);
 
       if ((clear_bv & X86_XSTATE_ZMM))
 	{
 	  for (i = 0; i < num_avx512_zmmh_high_registers; i++)
-	    memset (((char *) &fp->zmmh_low_space[0]) + 32 + i * 64, 0, 32);
+	    memset (fp.zmmh_low_space () + 32 + i * 64, 0, 32);
 	  for (i = 0; i < num_avx512_xmm_registers; i++)
-	    memset (((char *) &fp->zmmh_high_space[0]) + i * 64, 0, 16);
+	    memset (fp.zmmh_high_space () + i * 64, 0, 16);
 	  for (i = 0; i < num_avx512_ymmh_registers; i++)
-	    memset (((char *) &fp->zmmh_high_space[0]) + 16 + i * 64, 0, 16);
+	    memset (fp.zmmh_high_space () + 16 + i * 64, 0, 16);
 	}
 
       if ((clear_bv & X86_XSTATE_PKRU))
 	for (i = 0; i < num_pkeys_registers; i++)
-	  memset (((char *) &fp->pkru_space[0]) + i * 4, 0, 4);
+	  memset (fp.pkru_space () + i * 4, 0, 4);
     }
 
   /* Check if any x87 registers are changed.  */
@@ -328,7 +404,7 @@ i387_cache_to_xsave (struct regcache *regcache, void *buf)
       for (i = 0; i < 8; i++)
 	{
 	  collect_register (regcache, i + st0_regnum, raw);
-	  p = ((char *) &fp->fx.st_space[0]) + i * 16;
+	  p = fp.st_space () + i * 16;
 	  if (memcmp (raw, p, 10))
 	    {
 	      xstate_bv |= X86_XSTATE_X87;
@@ -345,7 +421,7 @@ i387_cache_to_xsave (struct regcache *regcache, void *buf)
       for (i = 0; i < num_xmm_registers; i++) 
 	{
 	  collect_register (regcache, i + xmm0_regnum, raw);
-	  p = ((char *) &fp->fx.xmm_space[0]) + i * 16;
+	  p = fp.xmm_space () + i * 16;
 	  if (memcmp (raw, p, 16))
 	    {
 	      xstate_bv |= X86_XSTATE_SSE;
@@ -362,7 +438,7 @@ i387_cache_to_xsave (struct regcache *regcache, void *buf)
       for (i = 0; i < num_xmm_registers; i++) 
 	{
 	  collect_register (regcache, i + ymm0h_regnum, raw);
-	  p = ((char *) &fp->ymmh_space[0]) + i * 16;
+	  p = fp.ymmh_space () + i * 16;
 	  if (memcmp (raw, p, 16))
 	    {
 	      xstate_bv |= X86_XSTATE_AVX;
@@ -379,7 +455,7 @@ i387_cache_to_xsave (struct regcache *regcache, void *buf)
       for (i = 0; i < num_mpx_bnd_registers; i++)
 	{
 	  collect_register (regcache, i + bnd0r_regnum, raw);
-	  p = ((char *) &fp->mpx_bnd_space[0]) + i * 16;
+	  p = fp.mpx_bnd_space () + i * 16;
 	  if (memcmp (raw, p, 16))
 	    {
 	      xstate_bv |= X86_XSTATE_BNDREGS;
@@ -396,7 +472,7 @@ i387_cache_to_xsave (struct regcache *regcache, void *buf)
       for (i = 0; i < num_mpx_cfg_registers; i++)
 	{
 	  collect_register (regcache, i + bndcfg_regnum, raw);
-	  p = ((char *) &fp->mpx_cfg_space[0]) + i * 8;
+	  p = fp.mpx_cfg_space () + i * 8;
 	  if (memcmp (raw, p, 8))
 	    {
 	      xstate_bv |= X86_XSTATE_BNDCFG;
@@ -413,7 +489,7 @@ i387_cache_to_xsave (struct regcache *regcache, void *buf)
       for (i = 0; i < num_avx512_k_registers; i++)
 	{
 	  collect_register (regcache, i + k0_regnum, raw);
-	  p = ((char *) &fp->k_space[0]) + i * 8;
+	  p = fp.k_space () + i * 8;
 	  if (memcmp (raw, p, 8) != 0)
 	    {
 	      xstate_bv |= X86_XSTATE_K;
@@ -430,7 +506,7 @@ i387_cache_to_xsave (struct regcache *regcache, void *buf)
       for (i = 0; i < num_avx512_zmmh_low_registers; i++)
 	{
 	  collect_register (regcache, i + zmm0h_regnum, raw);
-	  p = ((char *) &fp->zmmh_low_space[0]) + i * 32;
+	  p = fp.zmmh_low_space () + i * 32;
 	  if (memcmp (raw, p, 32) != 0)
 	    {
 	      xstate_bv |= X86_XSTATE_ZMM_H;
@@ -449,7 +525,7 @@ i387_cache_to_xsave (struct regcache *regcache, void *buf)
       for (i = 0; i < num_avx512_zmmh_high_registers; i++)
 	{
 	  collect_register (regcache, i + zmm16h_regnum, raw);
-	  p = ((char *) &fp->zmmh_high_space[0]) + 32 + i * 64;
+	  p = fp.zmmh_high_space () + 32 + i * 64;
 	  if (memcmp (raw, p, 32) != 0)
 	    {
 	      xstate_bv |= X86_XSTATE_ZMM;
@@ -468,7 +544,7 @@ i387_cache_to_xsave (struct regcache *regcache, void *buf)
       for (i = 0; i < num_avx512_xmm_registers; i++)
 	{
 	  collect_register (regcache, i + xmm_avx512_regnum, raw);
-	  p = ((char *) &fp->zmmh_high_space[0]) + i * 64;
+	  p = fp.zmmh_high_space () + i * 64;
 	  if (memcmp (raw, p, 16) != 0)
 	    {
 	      xstate_bv |= X86_XSTATE_ZMM;
@@ -487,7 +563,7 @@ i387_cache_to_xsave (struct regcache *regcache, void *buf)
       for (i = 0; i < num_avx512_ymmh_registers; i++)
 	{
 	  collect_register (regcache, i + ymmh_avx512_regnum, raw);
-	  p = ((char *) &fp->zmmh_high_space[0]) + 16 + i * 64;
+	  p = fp.zmmh_high_space () + 16 + i * 64;
 	  if (memcmp (raw, p, 16) != 0)
 	    {
 	      xstate_bv |= X86_XSTATE_ZMM;
@@ -504,7 +580,7 @@ i387_cache_to_xsave (struct regcache *regcache, void *buf)
       for (i = 0; i < num_pkeys_registers; i++)
 	{
 	  collect_register (regcache, i + pkru_regnum, raw);
-	  p = ((char *) &fp->pkru_space[0]) + i * 4;
+	  p = fp.pkru_space () + i * 4;
 	  if (memcmp (raw, p, 4) != 0)
 	    {
 	      xstate_bv |= X86_XSTATE_PKRU;
@@ -516,53 +592,53 @@ i387_cache_to_xsave (struct regcache *regcache, void *buf)
   if ((x86_xcr0 & X86_XSTATE_SSE) || (x86_xcr0 & X86_XSTATE_AVX))
     {
       collect_register_by_name (regcache, "mxcsr", raw);
-      if (memcmp (raw, &fp->fx.mxcsr, 4) != 0)
+      if (memcmp (raw, &fp.get_fxsave ()->mxcsr, 4) != 0)
 	{
-	  if (((fp->xstate_bv | xstate_bv)
+	  if (((fp.xstate_bv () | xstate_bv)
 	       & (X86_XSTATE_SSE | X86_XSTATE_AVX)) == 0)
 	    xstate_bv |= X86_XSTATE_SSE;
-	  memcpy (&fp->fx.mxcsr, raw, 4);
+	  memcpy (&fp.get_fxsave ()->mxcsr, raw, 4);
 	}
     }
 
   if (x86_xcr0 & X86_XSTATE_X87)
     {
       collect_register_by_name (regcache, "fioff", raw);
-      if (memcmp (raw, &fp->fx.fioff, 4) != 0)
+      if (memcmp (raw, &fp.get_fxsave ()->fioff, 4) != 0)
 	{
 	  xstate_bv |= X86_XSTATE_X87;
-	  memcpy (&fp->fx.fioff, raw, 4);
+	  memcpy (&fp.get_fxsave ()->fioff, raw, 4);
 	}
 
       collect_register_by_name (regcache, "fooff", raw);
-      if (memcmp (raw, &fp->fx.fooff, 4) != 0)
+      if (memcmp (raw, &fp.get_fxsave ()->fooff, 4) != 0)
 	{
 	  xstate_bv |= X86_XSTATE_X87;
-	  memcpy (&fp->fx.fooff, raw, 4);
+	  memcpy (&fp.get_fxsave ()->fooff, raw, 4);
 	}
 
       /* This one's 11 bits... */
       val2 = regcache_raw_get_unsigned_by_name (regcache, "fop");
-      val2 = (val2 & 0x7FF) | (fp->fx.fop & 0xF800);
-      if (fp->fx.fop != val2)
+      val2 = (val2 & 0x7FF) | (fp.get_fxsave ()->fop & 0xF800);
+      if (fp.get_fxsave ()->fop != val2)
 	{
 	  xstate_bv |= X86_XSTATE_X87;
-	  fp->fx.fop = val2;
+	  fp.get_fxsave ()->fop = val2;
 	}
 
       /* Some registers are 16-bit.  */
       val = regcache_raw_get_unsigned_by_name (regcache, "fctrl");
-      if (fp->fx.fctrl != val)
+      if (fp.get_fxsave ()->fctrl != val)
 	{
 	  xstate_bv |= X86_XSTATE_X87;
-	  fp->fx.fctrl = val;
+	  fp.get_fxsave ()->fctrl = val;
 	}
 
       val = regcache_raw_get_unsigned_by_name (regcache, "fstat");
-      if (fp->fx.fstat != val)
+      if (fp.get_fxsave ()->fstat != val)
 	{
 	  xstate_bv |= X86_XSTATE_X87;
-	  fp->fx.fstat = val;
+	  fp.get_fxsave ()->fstat = val;
 	}
 
       /* Convert to the simplifed tag form stored in fxsave data.  */
@@ -575,30 +651,30 @@ i387_cache_to_xsave (struct regcache *regcache, void *buf)
 	  if (tag != 3)
 	    val2 |= (1 << i);
 	}
-      if (fp->fx.ftag != val2)
+      if (fp.get_fxsave ()->ftag != val2)
 	{
 	  xstate_bv |= X86_XSTATE_X87;
-	  fp->fx.ftag = val2;
+	  fp.get_fxsave ()->ftag = val2;
 	}
 
       val = regcache_raw_get_unsigned_by_name (regcache, "fiseg");
-      if (fp->fx.fiseg != val)
+      if (fp.get_fxsave ()->fiseg != val)
 	{
 	  xstate_bv |= X86_XSTATE_X87;
-	  fp->fx.fiseg = val;
+	  fp.get_fxsave ()->fiseg = val;
 	}
 
       val = regcache_raw_get_unsigned_by_name (regcache, "foseg");
-      if (fp->fx.foseg != val)
+      if (fp.get_fxsave ()->foseg != val)
 	{
 	  xstate_bv |= X86_XSTATE_X87;
-	  fp->fx.foseg = val;
+	  fp.get_fxsave ()->foseg = val;
 	}
     }
 
   /* Update the corresponding bits in xstate_bv if any SSE/AVX
      registers are changed.  */
-  fp->xstate_bv |= xstate_bv;
+  fp.xstate_bv () |= xstate_bv;
 }
 
 static int
@@ -704,8 +780,7 @@ i387_fxsave_to_cache (struct regcache *regcache, const void *buf)
 void
 i387_xsave_to_cache (struct regcache *regcache, const void *buf)
 {
-  struct i387_xsave *fp = (struct i387_xsave *) buf;
-  struct i387_fxsave *fxp = (struct i387_fxsave *) buf;
+  i387_xsave fp {buf};
   bool amd64 = register_size (regcache->tdesc, 0) == 8;
   int i, top;
   unsigned long val;
@@ -723,7 +798,7 @@ i387_xsave_to_cache (struct regcache *regcache, const void *buf)
 
   /* The supported bits in `xstat_bv' are 8 bytes.  Clear part in
      vector registers if its bit in xstat_bv is zero.  */
-  clear_bv = (~fp->xstate_bv) & x86_xcr0;
+  clear_bv = (~fp.xstate_bv ()) & x86_xcr0;
 
   /* Check if any x87 registers are changed.  */
   if ((x86_xcr0 & X86_XSTATE_X87) != 0)
@@ -737,7 +812,7 @@ i387_xsave_to_cache (struct regcache *regcache, const void *buf)
 	}
       else
 	{
-	  p = (gdb_byte *) &fp->fx.st_space[0];
+	  p = fp.st_space ();
 	  for (i = 0; i < 8; i++)
 	    supply_register (regcache, i + st0_regnum, p + i * 16);
 	}
@@ -754,7 +829,7 @@ i387_xsave_to_cache (struct regcache *regcache, const void *buf)
 	}
       else
 	{
-	  p = (gdb_byte *) &fp->fx.xmm_space[0];
+	  p = fp.xmm_space ();
 	  for (i = 0; i < num_xmm_registers; i++)
 	    supply_register (regcache, i + xmm0_regnum, p + i * 16);
 	}
@@ -771,7 +846,7 @@ i387_xsave_to_cache (struct regcache *regcache, const void *buf)
 	}
       else
 	{
-	  p = (gdb_byte *) &fp->ymmh_space[0];
+	  p = fp.ymmh_space ();
 	  for (i = 0; i < num_xmm_registers; i++)
 	    supply_register (regcache, i + ymm0h_regnum, p + i * 16);
 	}
@@ -789,7 +864,7 @@ i387_xsave_to_cache (struct regcache *regcache, const void *buf)
 	}
       else
 	{
-	  p = (gdb_byte *) &fp->mpx_bnd_space[0];
+	  p = fp.mpx_bnd_space ();
 	  for (i = 0; i < num_mpx_bnd_registers; i++)
 	    supply_register (regcache, i + bnd0r_regnum, p + i * 16);
 	}
@@ -807,7 +882,7 @@ i387_xsave_to_cache (struct regcache *regcache, const void *buf)
 	}
       else
 	{
-	  p = (gdb_byte *) &fp->mpx_cfg_space[0];
+	  p = fp.mpx_cfg_space ();
 	  for (i = 0; i < num_mpx_cfg_registers; i++)
 	    supply_register (regcache, i + bndcfg_regnum, p + i * 8);
 	}
@@ -824,7 +899,7 @@ i387_xsave_to_cache (struct regcache *regcache, const void *buf)
 	}
       else
 	{
-	  p = (gdb_byte *) &fp->k_space[0];
+	  p = fp.k_space ();
 	  for (i = 0; i < num_avx512_k_registers; i++)
 	    supply_register (regcache, i + k0_regnum, p + i * 8);
 	}
@@ -841,7 +916,7 @@ i387_xsave_to_cache (struct regcache *regcache, const void *buf)
 	}
       else
 	{
-	  p = (gdb_byte *) &fp->zmmh_low_space[0];
+	  p = fp.zmmh_low_space ();
 	  for (i = 0; i < num_avx512_zmmh_low_registers; i++)
 	    supply_register (regcache, i + zmm0h_regnum, p + i * 32);
 	}
@@ -870,7 +945,7 @@ i387_xsave_to_cache (struct regcache *regcache, const void *buf)
 	}
       else
 	{
-	  p = (gdb_byte *) &fp->zmmh_high_space[0];
+	  p = fp.zmmh_high_space ();
 	  for (i = 0; i < num_avx512_zmmh_high_registers; i++)
 	    supply_register (regcache, i + zmm16h_regnum, p + 32 + i * 64);
 	  for (i = 0; i < num_avx512_ymmh_registers; i++)
@@ -891,7 +966,7 @@ i387_xsave_to_cache (struct regcache *regcache, const void *buf)
 	}
       else
 	{
-	  p = (gdb_byte *) &fp->pkru_space[0];
+	  p = fp.pkru_space ();
 	  for (i = 0; i < num_pkeys_registers; i++)
 	    supply_register (regcache, i + pkru_regnum, p + i * 4);
 	}
@@ -904,7 +979,7 @@ i387_xsave_to_cache (struct regcache *regcache, const void *buf)
       supply_register_by_name (regcache, "mxcsr", &default_mxcsr);
     }
   else
-    supply_register_by_name (regcache, "mxcsr", &fp->fx.mxcsr);
+    supply_register_by_name (regcache, "mxcsr", &fp.get_fxsave ()->mxcsr);
 
   if ((clear_bv & X86_XSTATE_X87) != 0)
     {
@@ -925,37 +1000,37 @@ i387_xsave_to_cache (struct regcache *regcache, const void *buf)
     }
   else
     {
-      supply_register_by_name (regcache, "fioff", &fp->fx.fioff);
-      supply_register_by_name (regcache, "fooff", &fp->fx.fooff);
+      supply_register_by_name (regcache, "fioff", &fp.get_fxsave ()->fioff);
+      supply_register_by_name (regcache, "fooff", &fp.get_fxsave ()->fooff);
 
       /* Some registers are 16-bit.  */
-      val = fp->fx.fctrl & 0xFFFF;
+      val = fp.get_fxsave ()->fctrl & 0xFFFF;
       supply_register_by_name (regcache, "fctrl", &val);
 
-      val = fp->fx.fstat & 0xFFFF;
+      val = fp.get_fxsave ()->fstat & 0xFFFF;
       supply_register_by_name (regcache, "fstat", &val);
 
       /* Generate the form of ftag data that GDB expects.  */
-      top = (fp->fx.fstat >> 11) & 0x7;
+      top = (fp.get_fxsave ()->fstat >> 11) & 0x7;
       val = 0;
       for (i = 7; i >= 0; i--)
 	{
 	  int tag;
-	  if (fp->fx.ftag & (1 << i))
-	    tag = i387_ftag (fxp, (i + 8 - top) % 8);
+	  if (fp.get_fxsave ()->ftag & (1 << i))
+	    tag = i387_ftag (fp.get_fxsave (), (i + 8 - top) % 8);
 	  else
 	    tag = 3;
 	  val |= tag << (2 * i);
 	}
       supply_register_by_name (regcache, "ftag", &val);
 
-      val = fp->fx.fiseg & 0xFFFF;
+      val = fp.get_fxsave ()->fiseg & 0xFFFF;
       supply_register_by_name (regcache, "fiseg", &val);
 
-      val = fp->fx.foseg & 0xFFFF;
+      val = fp.get_fxsave ()->foseg & 0xFFFF;
       supply_register_by_name (regcache, "foseg", &val);
 
-      val = (fp->fx.fop) & 0x7FF;
+      val = (fp.get_fxsave ()->fop) & 0x7FF;
       supply_register_by_name (regcache, "fop", &val);
     }
 }
-- 
2.34.1

[PATCH v3 13/13] x86: Remove X86_XSTATE_SIZE and related constants.

[John Baldwin]

---
 gdbsupport/x86-xstate.h | 12 ------------
 1 file changed, 12 deletions(-)

diff --git a/gdbsupport/x86-xstate.h b/gdbsupport/x86-xstate.h
index 9e2dd2155be..254912f683b 100644
--- a/gdbsupport/x86-xstate.h
+++ b/gdbsupport/x86-xstate.h
@@ -93,11 +93,6 @@ struct x86_xsave_layout
 
 #define X86_XSTATE_SSE_SIZE	576
 #define X86_XSTATE_AVX_SIZE	832
-#define X86_XSTATE_BNDREGS_SIZE	1024
-#define X86_XSTATE_BNDCFG_SIZE	1088
-#define X86_XSTATE_AVX512_SIZE	2688
-#define X86_XSTATE_PKRU_SIZE	2696
-#define X86_XSTATE_MAX_SIZE	2696
 
 
 /* In case one of the MPX XCR0 bits is set we consider we have MPX.  */
@@ -106,13 +101,6 @@ struct x86_xsave_layout
 #define HAS_AVX512(XCR0) (((XCR0) & X86_XSTATE_AVX512) != 0)
 #define HAS_PKRU(XCR0) (((XCR0) & X86_XSTATE_PKRU) != 0)
 
-/* Get I386 XSAVE extended state size.  */
-#define X86_XSTATE_SIZE(XCR0) \
-    (HAS_PKRU (XCR0) ? X86_XSTATE_PKRU_SIZE : \
-     (HAS_AVX512 (XCR0) ? X86_XSTATE_AVX512_SIZE : \
-      (HAS_MPX (XCR0) ? X86_XSTATE_BNDCFG_SIZE : \
-       (HAS_AVX (XCR0) ? X86_XSTATE_AVX_SIZE : X86_XSTATE_SSE_SIZE))))
-
 /* Initial value for fctrl register, as defined in the X86 manual, and
    confirmed in the (Linux) kernel source.  When the x87 floating point
    feature is not enabled in an inferior we use this as the value of the
-- 
2.34.1

RE: [PATCH v3 00/13] Handle variable XSAVE layouts

[George, Jini Susan]

[Public]

Thanks again for doing this, John, Felix and Aleksandar. I went through the changes and tested on a few AMD CPUs of differing xsave layouts. As of now these changes seem to cover the current AMD CPU implementations. Going forward, if additional AMD specific layouts have to be added, we would do it.

The changes look good to me (I am no approver, though) for the most part. A few minor comments:

Nit: in gdb/i386-tdep.h, it might be good to add a comment for the following line  (line # 148) for consistency with the rest of the fields.
  x86_xsave_layout xsave_layout;

Nit: You might want to modify the comments preceding the modified  xsave_*_offset[] definitions in gdb/i387-tdep.c to reflect that the offset to the locations are at (tdep)->xsave_layout.*_offset + xsave_*_offset[REGNUM].

Nit: Line 771 in gdb/i387-tdep.c. Might make sense to remove the "HI16_ZMM_area +" part.

I also agree with your proposed layout of the corefile note for the xsave layout.

Thanks,
Jini.

>>-----Original Message-----
>>From: John Baldwin <jhb@FreeBSD.org>
>>Sent: Wednesday, May 4, 2022 2:35 AM
>>To: gdb-patches@sourceware.org
>>Cc: Willgerodt, Felix <felix.willgerodt@intel.com>; George, Jini Susan
>><JiniSusan.George@amd.com>
>>Subject: [PATCH v3 00/13] Handle variable XSAVE layouts
>>
>>[CAUTION: External Email]
>>
>>Changes since V2:
>>
>>- Pulled in some of the changes from Intel's branch Felix pointed me
>>  at, in particular gdbserver support.  However, relative to that
>>  branch these patches make the following changes:
>>
>>  - The i387_* structs and class remain in gdbserver/i387-fp.cc
>>    rather than moving to gdbsupport/.
>>
>>  - Rather than invoking cpuid each time an XSAVE area is parsed,
>>    the x86_xsave_layout structure is used to hold offsets and
>>    CPUID is only invoked the first time NT_X86_XSTATE is probed
>>    with the offsets cached for later use.
>>
>>  - I did not update the ChangeLog bits of these log messages, but
>>    probably they can be dropped for the Intel commits as GDB
>>    commits generally do not include these now?
>>
>>- Added Linux support both for gdbarches and the x86 native targets.
>>  I wasn't sure if PT_GETREGSET on Linux provides a way to query the
>>  size of the XSAVE register set (on FreeBSD PT_GETREGSET returns
>>  the register set's size in iov_len of the passed-in iovec if the
>>  original iovec has a NULL pointer and zero length), so I used
>>  cpuid leaf 0xd subleaf 0x0 to query the size of the register set
>>  for the native targets as well as for the Linux gdbserver support.
>>
>>Note that this still depends on the size and xcr0 mask heuristic for Linux and
>>FreeBSD core dumps to determine the layout (and I have not added any
>>additional layouts as I wasn't sure if Jini was intending to add additional AMD-
>>specific layouts).  I'd kind of like to land this series before doing a followup to
>>flesh out a new core dump note.
>>
>>I think for the new core dump note what I would propose is a simple array of
>>CPUID results for sub-leaves of the 0xd leaf (as a NT_X86_XSTATE_CPUID or the
>>like) where each entry in the array contained the subleaf as well as eax, ebx, ecx,
>>and edx results.  This note might even eventually permit handling "compact"
>>XSTATE in future core dumps rather than only "standard".
>>
>>I have tested this on both an AMD Ryzen 9 5900X and Intel Core i7-8650U on
>>FreeBSD/amd64 as well as on a Linux/x86-64 VM on the Intel system.  I also
>>tested FreeBSD/i386 in a VM on the AMD system.
>>
>>Aleksandar Paunovic (2):
>>  gdbserver: Refactor the legacy region within the xsave struct
>>  gdbserver: Read offsets of the XSAVE extended region via CPUID
>>
>>John Baldwin (11):
>>  x86: Add an x86_xsave_layout structure to handle variable XSAVE
>>    layouts.
>>  core: Support fetching TARGET_OBJECT_X86_XSAVE_LAYOUT from
>>    architectures.
>>  nat/x86-cpuid.h: Add x86_cpuid_count wrapper around __get_cpuid_count.
>>  x86 nat: Add helper functions to save the XSAVE layout for the host.
>>  gdb: Update x86 FreeBSD architectures to support XSAVE layouts.
>>  gdb: Support XSAVE layouts for the current host in the FreeBSD x86
>>    targets.
>>  gdb: Update x86 Linux architectures to support XSAVE layouts.
>>  gdb: Support XSAVE layouts for the current host in the Linux x86
>>    targets.
>>  gdb: Use x86_xstate_layout to parse the XSAVE extended state area.
>>  gdbserver: Add a function to set the XSAVE mask and size.
>>  x86: Remove X86_XSTATE_SIZE and related constants.
>>
>> gdb/amd64-fbsd-nat.c       |  40 +--
>> gdb/amd64-fbsd-tdep.c      |   8 +-
>> gdb/amd64-linux-nat.c      |   6 +-
>> gdb/amd64-linux-tdep.c     |   8 +-
>> gdb/configure.nat          |   8 +-
>> gdb/corelow.c              |  22 ++
>> gdb/gdbarch-components.py  |  13 +
>> gdb/gdbarch-gen.h          |  10 +
>> gdb/gdbarch.c              |  32 +++
>> gdb/i386-fbsd-nat.c        |  39 +--
>> gdb/i386-fbsd-tdep.c       |  37 ++-
>> gdb/i386-fbsd-tdep.h       |   6 +
>> gdb/i386-linux-nat.c       |   8 +-
>> gdb/i386-linux-tdep.c      |  34 ++-
>> gdb/i386-linux-tdep.h      |   6 +
>> gdb/i386-tdep.c            |  36 ++-
>> gdb/i386-tdep.h            |   3 +
>> gdb/i387-tdep.c            | 493 ++++++++++++++++++++++++-------------
>> gdb/i387-tdep.h            |   8 +
>> gdb/nat/x86-cpuid.h        |  27 ++
>> gdb/nat/x86-xstate.c       |  65 +++++
>> gdb/nat/x86-xstate.h       |  35 +++
>> gdb/target.h               |   2 +
>> gdb/x86-fbsd-nat.c         |  51 ++++
>> gdb/x86-fbsd-nat.h         |  29 ++-
>> gdb/x86-linux-nat.c        |  33 +++
>> gdb/x86-linux-nat.h        |  11 +
>> gdbserver/configure.srv    |  12 +-
>> gdbserver/i387-fp.cc       | 312 ++++++++++++++---------
>> gdbserver/i387-fp.h        |   2 +-
>> gdbserver/linux-x86-low.cc |  10 +-
>> gdbsupport/x86-xstate.h    |  69 ++++--
>> 32 files changed, 1067 insertions(+), 408 deletions(-)  create mode 100644
>>gdb/nat/x86-xstate.c  create mode 100644 gdb/nat/x86-xstate.h
>>
>>--
>>2.34.1

Re: [PATCH v3 00/13] Handle variable XSAVE layouts

[John Baldwin]

On 5/10/22 3:48 AM, George, Jini Susan wrote:
> [Public]
> 
> Thanks again for doing this, John, Felix and Aleksandar. I went through the changes and tested on a few AMD CPUs of differing xsave layouts. As of now these changes seem to cover the current AMD CPU implementations. Going forward, if additional AMD specific layouts have to be added, we would do it.
> 
> The changes look good to me (I am no approver, though) for the most part. A few minor comments:
> 
> Nit: in gdb/i386-tdep.h, it might be good to add a comment for the following line  (line # 148) for consistency with the rest of the fields.
>    x86_xsave_layout xsave_layout;

Done.

> Nit: You might want to modify the comments preceding the modified  xsave_*_offset[] definitions in gdb/i387-tdep.c to reflect that the offset to the locations are at (tdep)->xsave_layout.*_offset + xsave_*_offset[REGNUM].

Ok, I have done so locally though tried to describe this a bit differently.
For example:

/* At xsave_avxh_offset[REGNUM] you'll find the relative offset within
    the AVX region of the XSAVE extended state where the upper 128bits
    of GDB register YMM0 + REGNUM is stored.  */

> Nit: Line 771 in gdb/i387-tdep.c. Might make sense to remove the "HI16_ZMM_area +" part.

Done.

Thanks!

-- 
John Baldwin

Re: [PATCH v3 00/13] Handle variable XSAVE layouts

[John Baldwin]

Felix,

Did you have any thoughts on this series?  I had tried to cc you on it,
but that didn't succeed it seems.  This does have some conflicts with
your AMX series, but those should be minor to resolve.

On 5/3/22 2:05 PM, John Baldwin wrote:
> Changes since V2:
> 
> - Pulled in some of the changes from Intel's branch Felix pointed me
>    at, in particular gdbserver support.  However, relative to that
>    branch these patches make the following changes:
> 
>    - The i387_* structs and class remain in gdbserver/i387-fp.cc
>      rather than moving to gdbsupport/.
> 
>    - Rather than invoking cpuid each time an XSAVE area is parsed,
>      the x86_xsave_layout structure is used to hold offsets and
>      CPUID is only invoked the first time NT_X86_XSTATE is probed
>      with the offsets cached for later use.
> 
>    - I did not update the ChangeLog bits of these log messages, but
>      probably they can be dropped for the Intel commits as GDB
>      commits generally do not include these now?
> 
> - Added Linux support both for gdbarches and the x86 native targets.
>    I wasn't sure if PT_GETREGSET on Linux provides a way to query the
>    size of the XSAVE register set (on FreeBSD PT_GETREGSET returns
>    the register set's size in iov_len of the passed-in iovec if the
>    original iovec has a NULL pointer and zero length), so I used
>    cpuid leaf 0xd subleaf 0x0 to query the size of the register set
>    for the native targets as well as for the Linux gdbserver support.
> 
> Note that this still depends on the size and xcr0 mask heuristic for
> Linux and FreeBSD core dumps to determine the layout (and I have not
> added any additional layouts as I wasn't sure if Jini was intending to
> add additional AMD-specific layouts).  I'd kind of like to land this
> series before doing a followup to flesh out a new core dump note.
> 
> I think for the new core dump note what I would propose is a simple
> array of CPUID results for sub-leaves of the 0xd leaf (as a
> NT_X86_XSTATE_CPUID or the like) where each entry in the array
> contained the subleaf as well as eax, ebx, ecx, and edx results.  This
> note might even eventually permit handling "compact" XSTATE in future
> core dumps rather than only "standard".
> 
> I have tested this on both an AMD Ryzen 9 5900X and Intel Core
> i7-8650U on FreeBSD/amd64 as well as on a Linux/x86-64 VM on the Intel
> system.  I also tested FreeBSD/i386 in a VM on the AMD system.
> 
> Aleksandar Paunovic (2):
>    gdbserver: Refactor the legacy region within the xsave struct
>    gdbserver: Read offsets of the XSAVE extended region via CPUID
> 
> John Baldwin (11):
>    x86: Add an x86_xsave_layout structure to handle variable XSAVE
>      layouts.
>    core: Support fetching TARGET_OBJECT_X86_XSAVE_LAYOUT from
>      architectures.
>    nat/x86-cpuid.h: Add x86_cpuid_count wrapper around __get_cpuid_count.
>    x86 nat: Add helper functions to save the XSAVE layout for the host.
>    gdb: Update x86 FreeBSD architectures to support XSAVE layouts.
>    gdb: Support XSAVE layouts for the current host in the FreeBSD x86
>      targets.
>    gdb: Update x86 Linux architectures to support XSAVE layouts.
>    gdb: Support XSAVE layouts for the current host in the Linux x86
>      targets.
>    gdb: Use x86_xstate_layout to parse the XSAVE extended state area.
>    gdbserver: Add a function to set the XSAVE mask and size.
>    x86: Remove X86_XSTATE_SIZE and related constants.
> 
>   gdb/amd64-fbsd-nat.c       |  40 +--
>   gdb/amd64-fbsd-tdep.c      |   8 +-
>   gdb/amd64-linux-nat.c      |   6 +-
>   gdb/amd64-linux-tdep.c     |   8 +-
>   gdb/configure.nat          |   8 +-
>   gdb/corelow.c              |  22 ++
>   gdb/gdbarch-components.py  |  13 +
>   gdb/gdbarch-gen.h          |  10 +
>   gdb/gdbarch.c              |  32 +++
>   gdb/i386-fbsd-nat.c        |  39 +--
>   gdb/i386-fbsd-tdep.c       |  37 ++-
>   gdb/i386-fbsd-tdep.h       |   6 +
>   gdb/i386-linux-nat.c       |   8 +-
>   gdb/i386-linux-tdep.c      |  34 ++-
>   gdb/i386-linux-tdep.h      |   6 +
>   gdb/i386-tdep.c            |  36 ++-
>   gdb/i386-tdep.h            |   3 +
>   gdb/i387-tdep.c            | 493 ++++++++++++++++++++++++-------------
>   gdb/i387-tdep.h            |   8 +
>   gdb/nat/x86-cpuid.h        |  27 ++
>   gdb/nat/x86-xstate.c       |  65 +++++
>   gdb/nat/x86-xstate.h       |  35 +++
>   gdb/target.h               |   2 +
>   gdb/x86-fbsd-nat.c         |  51 ++++
>   gdb/x86-fbsd-nat.h         |  29 ++-
>   gdb/x86-linux-nat.c        |  33 +++
>   gdb/x86-linux-nat.h        |  11 +
>   gdbserver/configure.srv    |  12 +-
>   gdbserver/i387-fp.cc       | 312 ++++++++++++++---------
>   gdbserver/i387-fp.h        |   2 +-
>   gdbserver/linux-x86-low.cc |  10 +-
>   gdbsupport/x86-xstate.h    |  69 ++++--
>   32 files changed, 1067 insertions(+), 408 deletions(-)
>   create mode 100644 gdb/nat/x86-xstate.c
>   create mode 100644 gdb/nat/x86-xstate.h
> 


-- 
John Baldwin

RE: [PATCH v3 00/13] Handle variable XSAVE layouts

[Willgerodt, Felix]

Hi John,

I saw them but I only skimmed over them a bit so far, sorry.
I didn't have any direct comments apart from what I already
stated for earlier versions, so I didn't answer.

I couldn't apply the patches unfortunately. For some reason git
tells me formatting is off. Not sure if the fault is on my end or on
yours. But I tried a bit on my side to no avail.
So I couldn't test them on some of our internal targets.

Yes you can remove the Intel changelogs.

Regards,
Felix

> -----Original Message-----
> From: John Baldwin <jhb@FreeBSD.org>
> Sent: Mittwoch, 18. Mai 2022 23:53
> To: gdb-patches@sourceware.org
> Cc: Willgerodt, Felix <felix.willgerodt@intel.com>
> Subject: Re: [PATCH v3 00/13] Handle variable XSAVE layouts
> 
> Felix,
> 
> Did you have any thoughts on this series?  I had tried to cc you on it,
> but that didn't succeed it seems.  This does have some conflicts with
> your AMX series, but those should be minor to resolve.
> 
> On 5/3/22 2:05 PM, John Baldwin wrote:
> > Changes since V2:
> >
> > - Pulled in some of the changes from Intel's branch Felix pointed me
> >    at, in particular gdbserver support.  However, relative to that
> >    branch these patches make the following changes:
> >
> >    - The i387_* structs and class remain in gdbserver/i387-fp.cc
> >      rather than moving to gdbsupport/.
> >
> >    - Rather than invoking cpuid each time an XSAVE area is parsed,
> >      the x86_xsave_layout structure is used to hold offsets and
> >      CPUID is only invoked the first time NT_X86_XSTATE is probed
> >      with the offsets cached for later use.
> >
> >    - I did not update the ChangeLog bits of these log messages, but
> >      probably they can be dropped for the Intel commits as GDB
> >      commits generally do not include these now?
> >
> > - Added Linux support both for gdbarches and the x86 native targets.
> >    I wasn't sure if PT_GETREGSET on Linux provides a way to query the
> >    size of the XSAVE register set (on FreeBSD PT_GETREGSET returns
> >    the register set's size in iov_len of the passed-in iovec if the
> >    original iovec has a NULL pointer and zero length), so I used
> >    cpuid leaf 0xd subleaf 0x0 to query the size of the register set
> >    for the native targets as well as for the Linux gdbserver support.
> >
> > Note that this still depends on the size and xcr0 mask heuristic for
> > Linux and FreeBSD core dumps to determine the layout (and I have not
> > added any additional layouts as I wasn't sure if Jini was intending to
> > add additional AMD-specific layouts).  I'd kind of like to land this
> > series before doing a followup to flesh out a new core dump note.
> >
> > I think for the new core dump note what I would propose is a simple
> > array of CPUID results for sub-leaves of the 0xd leaf (as a
> > NT_X86_XSTATE_CPUID or the like) where each entry in the array
> > contained the subleaf as well as eax, ebx, ecx, and edx results.  This
> > note might even eventually permit handling "compact" XSTATE in future
> > core dumps rather than only "standard".
> >
> > I have tested this on both an AMD Ryzen 9 5900X and Intel Core
> > i7-8650U on FreeBSD/amd64 as well as on a Linux/x86-64 VM on the Intel
> > system.  I also tested FreeBSD/i386 in a VM on the AMD system.
> >
> > Aleksandar Paunovic (2):
> >    gdbserver: Refactor the legacy region within the xsave struct
> >    gdbserver: Read offsets of the XSAVE extended region via CPUID
> >
> > John Baldwin (11):
> >    x86: Add an x86_xsave_layout structure to handle variable XSAVE
> >      layouts.
> >    core: Support fetching TARGET_OBJECT_X86_XSAVE_LAYOUT from
> >      architectures.
> >    nat/x86-cpuid.h: Add x86_cpuid_count wrapper around __get_cpuid_count.
> >    x86 nat: Add helper functions to save the XSAVE layout for the host.
> >    gdb: Update x86 FreeBSD architectures to support XSAVE layouts.
> >    gdb: Support XSAVE layouts for the current host in the FreeBSD x86
> >      targets.
> >    gdb: Update x86 Linux architectures to support XSAVE layouts.
> >    gdb: Support XSAVE layouts for the current host in the Linux x86
> >      targets.
> >    gdb: Use x86_xstate_layout to parse the XSAVE extended state area.
> >    gdbserver: Add a function to set the XSAVE mask and size.
> >    x86: Remove X86_XSTATE_SIZE and related constants.
> >
> >   gdb/amd64-fbsd-nat.c       |  40 +--
> >   gdb/amd64-fbsd-tdep.c      |   8 +-
> >   gdb/amd64-linux-nat.c      |   6 +-
> >   gdb/amd64-linux-tdep.c     |   8 +-
> >   gdb/configure.nat          |   8 +-
> >   gdb/corelow.c              |  22 ++
> >   gdb/gdbarch-components.py  |  13 +
> >   gdb/gdbarch-gen.h          |  10 +
> >   gdb/gdbarch.c              |  32 +++
> >   gdb/i386-fbsd-nat.c        |  39 +--
> >   gdb/i386-fbsd-tdep.c       |  37 ++-
> >   gdb/i386-fbsd-tdep.h       |   6 +
> >   gdb/i386-linux-nat.c       |   8 +-
> >   gdb/i386-linux-tdep.c      |  34 ++-
> >   gdb/i386-linux-tdep.h      |   6 +
> >   gdb/i386-tdep.c            |  36 ++-
> >   gdb/i386-tdep.h            |   3 +
> >   gdb/i387-tdep.c            | 493 ++++++++++++++++++++++++-------------
> >   gdb/i387-tdep.h            |   8 +
> >   gdb/nat/x86-cpuid.h        |  27 ++
> >   gdb/nat/x86-xstate.c       |  65 +++++
> >   gdb/nat/x86-xstate.h       |  35 +++
> >   gdb/target.h               |   2 +
> >   gdb/x86-fbsd-nat.c         |  51 ++++
> >   gdb/x86-fbsd-nat.h         |  29 ++-
> >   gdb/x86-linux-nat.c        |  33 +++
> >   gdb/x86-linux-nat.h        |  11 +
> >   gdbserver/configure.srv    |  12 +-
> >   gdbserver/i387-fp.cc       | 312 ++++++++++++++---------
> >   gdbserver/i387-fp.h        |   2 +-
> >   gdbserver/linux-x86-low.cc |  10 +-
> >   gdbsupport/x86-xstate.h    |  69 ++++--
> >   32 files changed, 1067 insertions(+), 408 deletions(-)
> >   create mode 100644 gdb/nat/x86-xstate.c
> >   create mode 100644 gdb/nat/x86-xstate.h
> >
> 
> 
> --
> John Baldwin
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

Re: [PATCH v3 00/13] Handle variable XSAVE layouts

[John Baldwin]

On 5/19/22 8:42 AM, Willgerodt, Felix wrote:
> Hi John,
> 
> I saw them but I only skimmed over them a bit so far, sorry.
> I didn't have any direct comments apart from what I already
> stated for earlier versions, so I didn't answer.
> 
> I couldn't apply the patches unfortunately. For some reason git
> tells me formatting is off. Not sure if the fault is on my end or on
> yours. But I tried a bit on my side to no avail.
> So I couldn't test them on some of our internal targets.

Ok, if it helps you can pull the branch directly
from https://github.com/bsdjhb/gdb/tree/xsave_layout
  
> Yes you can remove the Intel changelogs.

Ok, I will trim those.  Should I keep or drop the SOB trailers
given that at least the log messages are now a bit different?

> Regards,
> Felix
> 
>> -----Original Message-----
>> From: John Baldwin <jhb@FreeBSD.org>
>> Sent: Mittwoch, 18. Mai 2022 23:53
>> To: gdb-patches@sourceware.org
>> Cc: Willgerodt, Felix <felix.willgerodt@intel.com>
>> Subject: Re: [PATCH v3 00/13] Handle variable XSAVE layouts
>>
>> Felix,
>>
>> Did you have any thoughts on this series?  I had tried to cc you on it,
>> but that didn't succeed it seems.  This does have some conflicts with
>> your AMX series, but those should be minor to resolve.
>>
>> On 5/3/22 2:05 PM, John Baldwin wrote:
>>> Changes since V2:
>>>
>>> - Pulled in some of the changes from Intel's branch Felix pointed me
>>>     at, in particular gdbserver support.  However, relative to that
>>>     branch these patches make the following changes:
>>>
>>>     - The i387_* structs and class remain in gdbserver/i387-fp.cc
>>>       rather than moving to gdbsupport/.
>>>
>>>     - Rather than invoking cpuid each time an XSAVE area is parsed,
>>>       the x86_xsave_layout structure is used to hold offsets and
>>>       CPUID is only invoked the first time NT_X86_XSTATE is probed
>>>       with the offsets cached for later use.
>>>
>>>     - I did not update the ChangeLog bits of these log messages, but
>>>       probably they can be dropped for the Intel commits as GDB
>>>       commits generally do not include these now?
>>>
>>> - Added Linux support both for gdbarches and the x86 native targets.
>>>     I wasn't sure if PT_GETREGSET on Linux provides a way to query the
>>>     size of the XSAVE register set (on FreeBSD PT_GETREGSET returns
>>>     the register set's size in iov_len of the passed-in iovec if the
>>>     original iovec has a NULL pointer and zero length), so I used
>>>     cpuid leaf 0xd subleaf 0x0 to query the size of the register set
>>>     for the native targets as well as for the Linux gdbserver support.
>>>
>>> Note that this still depends on the size and xcr0 mask heuristic for
>>> Linux and FreeBSD core dumps to determine the layout (and I have not
>>> added any additional layouts as I wasn't sure if Jini was intending to
>>> add additional AMD-specific layouts).  I'd kind of like to land this
>>> series before doing a followup to flesh out a new core dump note.
>>>
>>> I think for the new core dump note what I would propose is a simple
>>> array of CPUID results for sub-leaves of the 0xd leaf (as a
>>> NT_X86_XSTATE_CPUID or the like) where each entry in the array
>>> contained the subleaf as well as eax, ebx, ecx, and edx results.  This
>>> note might even eventually permit handling "compact" XSTATE in future
>>> core dumps rather than only "standard".
>>>
>>> I have tested this on both an AMD Ryzen 9 5900X and Intel Core
>>> i7-8650U on FreeBSD/amd64 as well as on a Linux/x86-64 VM on the Intel
>>> system.  I also tested FreeBSD/i386 in a VM on the AMD system.
>>>
>>> Aleksandar Paunovic (2):
>>>     gdbserver: Refactor the legacy region within the xsave struct
>>>     gdbserver: Read offsets of the XSAVE extended region via CPUID
>>>
>>> John Baldwin (11):
>>>     x86: Add an x86_xsave_layout structure to handle variable XSAVE
>>>       layouts.
>>>     core: Support fetching TARGET_OBJECT_X86_XSAVE_LAYOUT from
>>>       architectures.
>>>     nat/x86-cpuid.h: Add x86_cpuid_count wrapper around __get_cpuid_count.
>>>     x86 nat: Add helper functions to save the XSAVE layout for the host.
>>>     gdb: Update x86 FreeBSD architectures to support XSAVE layouts.
>>>     gdb: Support XSAVE layouts for the current host in the FreeBSD x86
>>>       targets.
>>>     gdb: Update x86 Linux architectures to support XSAVE layouts.
>>>     gdb: Support XSAVE layouts for the current host in the Linux x86
>>>       targets.
>>>     gdb: Use x86_xstate_layout to parse the XSAVE extended state area.
>>>     gdbserver: Add a function to set the XSAVE mask and size.
>>>     x86: Remove X86_XSTATE_SIZE and related constants.
>>>
>>>    gdb/amd64-fbsd-nat.c       |  40 +--
>>>    gdb/amd64-fbsd-tdep.c      |   8 +-
>>>    gdb/amd64-linux-nat.c      |   6 +-
>>>    gdb/amd64-linux-tdep.c     |   8 +-
>>>    gdb/configure.nat          |   8 +-
>>>    gdb/corelow.c              |  22 ++
>>>    gdb/gdbarch-components.py  |  13 +
>>>    gdb/gdbarch-gen.h          |  10 +
>>>    gdb/gdbarch.c              |  32 +++
>>>    gdb/i386-fbsd-nat.c        |  39 +--
>>>    gdb/i386-fbsd-tdep.c       |  37 ++-
>>>    gdb/i386-fbsd-tdep.h       |   6 +
>>>    gdb/i386-linux-nat.c       |   8 +-
>>>    gdb/i386-linux-tdep.c      |  34 ++-
>>>    gdb/i386-linux-tdep.h      |   6 +
>>>    gdb/i386-tdep.c            |  36 ++-
>>>    gdb/i386-tdep.h            |   3 +
>>>    gdb/i387-tdep.c            | 493 ++++++++++++++++++++++++-------------
>>>    gdb/i387-tdep.h            |   8 +
>>>    gdb/nat/x86-cpuid.h        |  27 ++
>>>    gdb/nat/x86-xstate.c       |  65 +++++
>>>    gdb/nat/x86-xstate.h       |  35 +++
>>>    gdb/target.h               |   2 +
>>>    gdb/x86-fbsd-nat.c         |  51 ++++
>>>    gdb/x86-fbsd-nat.h         |  29 ++-
>>>    gdb/x86-linux-nat.c        |  33 +++
>>>    gdb/x86-linux-nat.h        |  11 +
>>>    gdbserver/configure.srv    |  12 +-
>>>    gdbserver/i387-fp.cc       | 312 ++++++++++++++---------
>>>    gdbserver/i387-fp.h        |   2 +-
>>>    gdbserver/linux-x86-low.cc |  10 +-
>>>    gdbsupport/x86-xstate.h    |  69 ++++--
>>>    32 files changed, 1067 insertions(+), 408 deletions(-)
>>>    create mode 100644 gdb/nat/x86-xstate.c
>>>    create mode 100644 gdb/nat/x86-xstate.h
>>>
>>
>>
>> --
>> John Baldwin
> Intel Deutschland GmbH
> Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
> Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
> Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva
> Chairperson of the Supervisory Board: Nicole Lau
> Registered Office: Munich
> Commercial Register: Amtsgericht Muenchen HRB 186928


-- 
John Baldwin

RE: [PATCH v3 00/13] Handle variable XSAVE layouts

[Willgerodt, Felix]

> -----Original Message-----
> From: John Baldwin <jhb@FreeBSD.org>
> Sent: Donnerstag, 19. Mai 2022 18:06
> To: Willgerodt, Felix <felix.willgerodt@intel.com>; gdb-
> patches@sourceware.org
> Subject: Re: [PATCH v3 00/13] Handle variable XSAVE layouts
> 
> On 5/19/22 8:42 AM, Willgerodt, Felix wrote:
> > Hi John,
> 
> > Yes you can remove the Intel changelogs.
> 
> Ok, I will trim those.  Should I keep or drop the SOB trailers
> given that at least the log messages are now a bit different?
>

I don't have a strong opinion on that, I think it is fine to remove
them. GDB doesn't care about them afaik.

It would be nice to keep a co-authored field if significant parts
were taken from our patches. But you seem to have done that.

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