[RFC PATCH v2 0/5] Handle variable XSAVE layouts

[RFC PATCH v2 0/5] Handle variable XSAVE layouts

[John Baldwin]

Changes since V1:

- Renamed xsave_offsets to x86_xsave_layout and added a total size
  member.

- Fetch the x86_xsave_layout in i386_gdbarch_init and save it in a new
  member of i386_gdbarch_tdep.

- Patch 1 now only deals with defining the new structure and adding it
  to the tdep.

- The i387_set_xsave_layout helper function is moved to patch 2 and
  the description has been updated to be more explicit about it being
  a fallback to be supplanted in the future by a new core dump note.

- The changes to i387-tdep.c to change the XSAVE parsing has now been
  moved to the end in patch 5.  It also now uses the new member from
  tdep instead of reading from the target every time.

- Patch 3 (FreeBSD core dump support changes) uses the XSAVE size
  saved in the tdep member as the register set size and no longer uses
  REGSET_VARIABLE_SIZE.  This should properly honor the native CPU's
  layout when writing a core dump via gcore.

John Baldwin (5):
  x86: Add an x86_xsave_layout structure to handle variable XSAVE
    layouts.
  core: Support fetching TARGET_OBJECT_X86_XSAVE_LAYOUT from
    architectures.
  Update x86 FreeBSD architectures to support XSAVE layouts.
  Support XSAVE layouts for the current host in the FreeBSD/amd64
    target.
  x86: Use x86_xstate_layout to parse the XSAVE extended state area.

 gdb/amd64-fbsd-nat.c      |  90 ++++++-
 gdb/amd64-fbsd-tdep.c     |   8 +-
 gdb/corelow.c             |  22 ++
 gdb/gdbarch-components.py |  13 +
 gdb/gdbarch-gen.h         |  10 +
 gdb/gdbarch.c             |  32 +++
 gdb/i386-fbsd-tdep.c      |  37 ++-
 gdb/i386-fbsd-tdep.h      |   6 +
 gdb/i386-tdep.c           |  36 ++-
 gdb/i386-tdep.h           |  30 +++
 gdb/i387-tdep.c           | 491 ++++++++++++++++++++++++--------------
 gdb/i387-tdep.h           |   8 +
 gdb/target.h              |   2 +
 13 files changed, 594 insertions(+), 191 deletions(-)

-- 
2.34.1

[RFC PATCH v2 1/5] 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.
---
 gdb/i386-tdep.c | 36 +++++++++++++++++++++++++++++++++++-
 gdb/i386-tdep.h | 30 ++++++++++++++++++++++++++++++
 gdb/target.h    |  2 ++
 3 files changed, 67 insertions(+), 1 deletion(-)

diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
index b98f4757499..0fe7431cef1 100644
--- a/gdb/i386-tdep.c
+++ b/gdb/i386-tdep.c
@@ -8446,6 +8446,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 *
@@ -8453,13 +8468,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;
 
@@ -8712,6 +8744,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 eb58dd68e73..0034660c9f7 100644
--- a/gdb/i386-tdep.h
+++ b/gdb/i386-tdep.h
@@ -56,6 +56,34 @@ enum struct_return
   reg_struct_return		/* Return "short" structures in registers.  */
 };
 
+/* 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 avx512_k_offset = 0;
+  int avx512_zmm_h_offset = 0;
+  int avx512_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
+      || avx512_k_offset != other.avx512_k_offset
+      || avx512_zmm_h_offset != other.avx512_zmm_h_offset
+      || avx512_zmm_offset != other.avx512_zmm_offset
+      || pkru_offset != other.pkru_offset;
+  }
+};
+
 /* i386 architecture specific information.  */
 struct i386_gdbarch_tdep : gdbarch_tdep
 {
@@ -145,6 +173,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 4cc79df05b4..4298ae2ccf8 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, ...  */
 };
 
-- 
2.34.1

[RFC PATCH v2 2/5] 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 001c4f147fc..71bfdcff9dd 100644
--- a/gdb/corelow.c
+++ b/gdb/corelow.c
@@ -987,6 +987,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 c820ddae764..99eaca7f7e2 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 7a8721328ab..82292f9c954 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 9fdcf1505fe..e8681d8930b 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)
   fprintf_filtered (file,
                       "gdbarch_dump: core_xfer_siginfo = <%s>\n",
                       host_address_to_string (gdbarch->core_xfer_siginfo));
+  fprintf_filtered (file,
+                      "gdbarch_dump: gdbarch_core_xfer_x86_xsave_layout_p() = %d\n",
+                      gdbarch_core_xfer_x86_xsave_layout_p (gdbarch));
+  fprintf_filtered (file,
+                      "gdbarch_dump: core_xfer_x86_xsave_layout = <%s>\n",
+                      host_address_to_string (gdbarch->core_xfer_x86_xsave_layout));
   fprintf_filtered (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)
+    fprintf_unfiltered (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 2f0b6509457..83df8a7fb0f 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->avx512_k_offset = 1088;
+      layout->avx512_zmm_h_offset = 1152;
+      layout->avx512_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->avx512_k_offset = 832;
+      layout->avx512_zmm_h_offset = 896;
+      layout->avx512_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->avx512_k_offset = 1088;
+      layout->avx512_zmm_h_offset = 1152;
+      layout->avx512_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

RE: [RFC PATCH v2 2/5] core: Support fetching TARGET_OBJECT_X86_XSAVE_LAYOUT from architectures.

[George, Jini Susan]

[Public]

Thanks for making these changes, John. I am not a gdb maintainer, but I am inclined to be in favor of these changes till we have modifications in the kernel to store the xsave layout indicators in the coredump. AFAIK, yes, AMD CPUs never implemented MPX, and hence the gap wrt the xsave layout of Intel. (There is an additional unaccounted gap of 128 bytes too). I am not sure as to why there is no space reserved in the standard form for MPX, but it is for AVX-512 in CPUs where AVX-512 is not implemented. I am trying to find this out, but at this point, I don't have answers for this.

Are these changes catering to corefile debugging only ? If so, are you planning to make changes for live debugging ? If not, could you please let me know ? If you plan to proceed with the corresponding changes for Linux, I would be interested in looking at that patch too.

One comment that I have is for the code in i387_set_xsave_layout(). Since the hardware provides a finer granularity in setting the individual xfeature mask bits for MPX and AVX-512, I am wondering if it might be better to consider more scenarios like:

...
else if  (HAS_AVX512(xcr0) && xsave_size == 2432) /* AMD CPUs with AVX-512 */
...
else if (HAS_AVX512_ZMM_HI(xcr0) && xsave_size == 1408)
...
else if  (HAS_AVX512_ZMM_KREGS(xcr0) && xsave_size == 896)
...
etc.

(though I don't know under what circumstances, you can have only certain components of AVX-512 or MPX turned on) -- but this would be in line with the rest of the code changes. We also might be able to make the code simpler since the post MPX xsave extended feature offsets (from opmask onwards) of AMD would be at an offset which is 256 bytes lesser than the corresponding ones of Intel.

AMD processors based on the Bulldozer architecture have the Light Weight Profiling regs placed at offset 832 in the xsave area -- but I believe we can safely disregard that.

Thanks,
Jini.

>>-----Original Message-----
>>From: Gdb-patches <gdb-patches-
>>bounces+jigeorge=amd.com@sourceware.org> On Behalf Of John Baldwin
>>Sent: Friday, March 18, 2022 12:06 AM
>>To: gdb-patches@sourceware.org
>>Cc: Willgerodt@sourceware.org
>>Subject: [RFC PATCH v2 2/5] core: Support fetching
>>TARGET_OBJECT_X86_XSAVE_LAYOUT from architectures.
>>
>>[CAUTION: External Email]
>>
>>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 001c4f147fc..71bfdcff9dd
>>100644
>>--- a/gdb/corelow.c
>>+++ b/gdb/corelow.c
>>@@ -987,6 +987,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
>>c820ddae764..99eaca7f7e2 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
>>7a8721328ab..82292f9c954 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 9fdcf1505fe..e8681d8930b 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)
>>   fprintf_filtered (file,
>>                       "gdbarch_dump: core_xfer_siginfo = <%s>\n",
>>                       host_address_to_string (gdbarch->core_xfer_siginfo));
>>+  fprintf_filtered (file,
>>+                      "gdbarch_dump: gdbarch_core_xfer_x86_xsave_layout_p() =
>>%d\n",
>>+                      gdbarch_core_xfer_x86_xsave_layout_p (gdbarch));
>>+ fprintf_filtered (file,
>>+                      "gdbarch_dump: core_xfer_x86_xsave_layout = <%s>\n",
>>+                      host_address_to_string
>>+ (gdbarch->core_xfer_x86_xsave_layout));
>>   fprintf_filtered (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)
>>+    fprintf_unfiltered (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 2f0b6509457..83df8a7fb0f 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->avx512_k_offset = 1088;
>>+      layout->avx512_zmm_h_offset = 1152;
>>+      layout->avx512_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->avx512_k_offset = 832;
>>+      layout->avx512_zmm_h_offset = 896;
>>+      layout->avx512_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->avx512_k_offset = 1088;
>>+      layout->avx512_zmm_h_offset = 1152;
>>+      layout->avx512_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

[RFC PATCH v2 3/5] 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 da5c297902d..37d89c98bc9 100644
--- a/gdb/amd64-fbsd-tdep.c
+++ b/gdb/amd64-fbsd-tdep.c
@@ -253,8 +253,10 @@ amd64fbsd_iterate_over_regset_sections (struct gdbarch *gdbarch,
       &amd64_fbsd_gregset, NULL, cb_data);
   cb (".reg2", tdep->sizeof_fpregset, tdep->sizeof_fpregset, &amd64_fpregset,
       NULL, 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.  */
@@ -295,6 +297,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 16ffd576323..cbbd3dcdc5f 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"
@@ -263,6 +264,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 *
@@ -317,9 +346,9 @@ i386fbsd_iterate_over_regset_sections (struct gdbarch *gdbarch,
   cb (".reg2", tdep->sizeof_fpregset, tdep->sizeof_fpregset, &i386_fpregset,
       NULL, 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);
 }
 
@@ -372,6 +401,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

[RFC PATCH v2 4/5] Support XSAVE layouts for the current host in the FreeBSD/amd64 target.

[John Baldwin]

Use the CPUID instruction to fetch the offsets of supported state
components.
---
 gdb/amd64-fbsd-nat.c | 90 ++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 82 insertions(+), 8 deletions(-)

diff --git a/gdb/amd64-fbsd-nat.c b/gdb/amd64-fbsd-nat.c
index 98a1af03a66..9de5dee7a62 100644
--- a/gdb/amd64-fbsd-nat.c
+++ b/gdb/amd64-fbsd-nat.c
@@ -28,11 +28,15 @@
 #include <sys/sysctl.h>
 #include <sys/user.h>
 #include <machine/reg.h>
+#ifdef PT_GETXSTATE_INFO
+#include <cpuid.h>
+#endif
 
 #include "fbsd-nat.h"
 #include "amd64-tdep.h"
 #include "amd64-fbsd-tdep.h"
 #include "amd64-nat.h"
+#include "i387-tdep.h"
 #include "x86-nat.h"
 #include "gdbsupport/x86-xstate.h"
 #include "x86-bsd-nat.h"
@@ -41,6 +45,15 @@ class amd64_fbsd_nat_target final
   : public x86bsd_nat_target<fbsd_nat_target>
 {
 public:
+#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;
+#endif
+
   void fetch_registers (struct regcache *, int) override;
   void store_registers (struct regcache *, int) override;
 
@@ -54,7 +67,37 @@ class amd64_fbsd_nat_target final
 static amd64_fbsd_nat_target the_amd64_fbsd_nat_target;
 
 #ifdef PT_GETXSTATE_INFO
-static size_t xsave_len;
+static x86_xsave_layout xsave_layout;
+
+/* Implement the "xfer_partial" target_ops method.  */
+
+enum target_xfer_status
+amd64_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);
+    }
+}
 #endif
 
 /* This is a layout of the amd64 'struct reg' but with i386
@@ -152,9 +195,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_layout.sizeof_xsave != 0)
     {
-      void *xstateregs = alloca (xsave_len);
+      void *xstateregs = alloca (xsave_layout.sizeof_xsave);
 
       if (ptrace (PT_GETXSTATE, pid, (PTRACE_TYPE_ARG3) xstateregs, 0) == -1)
 	perror_with_name (_("Couldn't get extended state status"));
@@ -229,9 +272,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_layout.sizeof_xsave != 0)
     {
-      void *xstateregs = alloca (xsave_len);
+      void *xstateregs = alloca (xsave_layout.sizeof_xsave);
 
       if (ptrace (PT_GETXSTATE, pid, (PTRACE_TYPE_ARG3) xstateregs, 0) == -1)
 	perror_with_name (_("Couldn't get extended state status"));
@@ -239,7 +282,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_layout.sizeof_xsave) == -1)
 	perror_with_name (_("Couldn't write extended state status"));
       return;
     }
@@ -304,6 +347,37 @@ amd64fbsd_supply_pcb (struct regcache *regcache, struct pcb *pcb)
 }
 \f
 
+#ifdef PT_GETXSTATE_INFO
+/* Fetch the offset of a specific XSAVE extended region.  */
+static int
+xsave_leaf_offset (uint64_t xcr0, int leaf)
+{
+  uint32_t eax, ebx, ecx, edx;
+
+  if ((xcr0 & (1ULL << leaf)) == 0)
+    return 0;
+
+  __cpuid_count(0xd, leaf, eax, ebx, ecx, edx);
+  return ebx;
+}
+
+/* Fetch the offsets of the XSAVE extended regions on the running host.  */
+static void
+probe_xsave_layout (const struct ptrace_xstate_info &info)
+{
+  if (info.xsave_len == 0)
+    return;
+  xsave_layout.sizeof_xsave = info.xsave_len;
+  xsave_layout.avx_offset = xsave_leaf_offset(info.xsave_mask, 2);
+  xsave_layout.bndregs_offset = xsave_leaf_offset(info.xsave_mask, 3);
+  xsave_layout.bndcfg_offset = xsave_leaf_offset(info.xsave_mask, 4);
+  xsave_layout.avx512_k_offset = xsave_leaf_offset(info.xsave_mask, 5);
+  xsave_layout.avx512_zmm_h_offset = xsave_leaf_offset(info.xsave_mask, 6);
+  xsave_layout.avx512_zmm_offset = xsave_leaf_offset(info.xsave_mask, 7);
+  xsave_layout.pkru_offset = xsave_leaf_offset(info.xsave_mask, 9);
+}
+#endif
+
 /* Implement the read_description method.  */
 
 const struct target_desc *
@@ -328,13 +402,13 @@ amd64_fbsd_nat_target::read_description ()
       if (ptrace (PT_GETXSTATE_INFO, inferior_ptid.pid (),
 		  (PTRACE_TYPE_ARG3) &info, sizeof (info)) == 0)
 	{
-	  xsave_len = info.xsave_len;
 	  xcr0 = info.xsave_mask;
+	  probe_xsave_layout (info);
 	}
       xsave_probed = 1;
     }
 
-  if (xsave_len != 0)
+  if (xsave_layout.sizeof_xsave != 0)
     {
       if (is64)
 	return amd64_target_description (xcr0, true);
-- 
2.34.1

[RFC PATCH v2 5/5] x86: 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 | 440 +++++++++++++++++++++++++++++-------------------
 gdb/i387-tdep.h |   1 +
 2 files changed, 264 insertions(+), 177 deletions(-)

diff --git a/gdb/i387-tdep.c b/gdb/i387-tdep.c
index 83df8a7fb0f..a669a6741a5 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.avx512_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.avx512_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.avx512_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.avx512_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.avx512_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;
@@ -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