[v6,08/15] gdb: Update x86 Linux architectures to support XSAVE layouts.

Message ID 20230714155151.21723-9-jhb@FreeBSD.org
State New
Headers
Series Handle variable XSAVE layouts |

Commit Message

John Baldwin July 14, 2023, 3:51 p.m. UTC
  Refactor i386_linux_core_read_xcr0 to fetch and return a corresponding
x86_xsave_layout as well as xcr0 using the size of an existing
NT_X86_XSTATE core dump to determine the offsets via
i387_set_xsave_layout.  Use this to add an implementation of
gdbarch_core_xfer_x86_xsave_layout.

Use tdep->xsave_layout.sizeof_xsave as the size of the XSTATE register
set.
---
 gdb/amd64-linux-tdep.c | 11 +++++--
 gdb/i386-linux-tdep.c  | 67 ++++++++++++++++++++++++------------------
 gdb/i386-linux-tdep.h  | 25 +++++++++-------
 3 files changed, 61 insertions(+), 42 deletions(-)
  

Comments

Simon Marchi July 26, 2023, 8:45 p.m. UTC | #1
> diff --git a/gdb/i386-linux-tdep.c b/gdb/i386-linux-tdep.c
> index 554c1455a5f..f27efbb1d59 100644
> --- a/gdb/i386-linux-tdep.c
> +++ b/gdb/i386-linux-tdep.c
> @@ -638,45 +638,51 @@ static int i386_linux_sc_reg_offset[] =
>    0 * 4				/* %gs */
>  };
>  
> -/* Get XSAVE extended state xcr0 from core dump.  */
> +/* See i386-linux-tdep.h.  */
>  
>  uint64_t
> -i386_linux_core_read_xcr0 (bfd *abfd)
> +i386_linux_core_read_xsave_info (bfd *abfd, x86_xsave_layout &layout)
>  {
>    asection *xstate = bfd_get_section_by_name (abfd, ".reg-xstate");
> -  uint64_t xcr0;
> +  if (xstate == nullptr)
> +    return X86_XSTATE_SSE_MASK;
>  
> -  if (xstate)
> +  /* Check extended state size.  */
> +  size_t size = bfd_section_size (xstate);
> +  if (size < X86_XSTATE_AVX_SIZE)
> +    return X86_XSTATE_SSE_MASK;
> +
> +  char contents[8];
> +  if (! bfd_get_section_contents (abfd, xstate, contents,
> +				  I386_LINUX_XSAVE_XCR0_OFFSET, 8))
>      {
> -      size_t size = bfd_section_size (xstate);
> -
> -      /* Check extended state size.  */
> -      if (size < X86_XSTATE_AVX_SIZE)
> -	xcr0 = X86_XSTATE_SSE_MASK;
> -      else
> -	{
> -	  char contents[8];
> -
> -	  if (! bfd_get_section_contents (abfd, xstate, contents,
> -					  I386_LINUX_XSAVE_XCR0_OFFSET,
> -					  8))
> -	    {
> -	      warning (_("Couldn't read `xcr0' bytes from "
> -			 "`.reg-xstate' section in core file."));
> -	      return 0;
> -	    }
> -
> -	  xcr0 = bfd_get_64 (abfd, contents);
> -	}
> +      warning (_("Couldn't read `xcr0' bytes from "
> +		 "`.reg-xstate' section in core file."));
> +      return X86_XSTATE_SSE_MASK;
>      }
> -  else
> -    xcr0 = 0;
> +
> +  uint64_t xcr0 = bfd_get_64 (abfd, contents);
> +
> +  if (!i387_set_xsave_layout (xcr0, size, layout))
> +    return X86_XSTATE_SSE_MASK;
>  
>    return xcr0;
>  }

I think I spotted a change of behavior in
i386_linux_core_read_xsave_info, which has consequences down the line.

If there's no .reg-xstate section, we would previously return xcr0 == 0,
i386_linux_read_description would return nullptr, and we would choose
between X87 and SSE based on .reg-xfp.

After the patch, i386_linux_core_read_xsave_info returns SSE, meaning
we'll never reach the point that checks for .reg-xfp.

Maybe the first return (if there's not .reg-xstate section) should
return 0?

>  
>  /* See i386-linux-tdep.h.  */
>  
> +bool
> +i386_linux_core_read_x86_xsave_layout (struct gdbarch *gdbarch,
> +				       x86_xsave_layout &layout)
> +{
> +  if (i386_linux_core_read_xsave_info (core_bfd, layout) == X86_XSTATE_SSE_MASK)
> +    return false;
> +
> +  return true;
> +}

Same as earlier, could be:

    return i386_linux_core_read_xsave_info (core_bfd, layout) != X86_XSTATE_SSE_MASK;

... although if i386_linux_core_read_xsave_info can return 0, we'd need
to check for that too.

Simon
  
John Baldwin July 26, 2023, 9:16 p.m. UTC | #2
On 7/26/23 1:45 PM, Simon Marchi wrote:
>> diff --git a/gdb/i386-linux-tdep.c b/gdb/i386-linux-tdep.c
>> index 554c1455a5f..f27efbb1d59 100644
>> --- a/gdb/i386-linux-tdep.c
>> +++ b/gdb/i386-linux-tdep.c
>> @@ -638,45 +638,51 @@ static int i386_linux_sc_reg_offset[] =
>>     0 * 4				/* %gs */
>>   };
>>   
>> -/* Get XSAVE extended state xcr0 from core dump.  */
>> +/* See i386-linux-tdep.h.  */
>>   
>>   uint64_t
>> -i386_linux_core_read_xcr0 (bfd *abfd)
>> +i386_linux_core_read_xsave_info (bfd *abfd, x86_xsave_layout &layout)
>>   {
>>     asection *xstate = bfd_get_section_by_name (abfd, ".reg-xstate");
>> -  uint64_t xcr0;
>> +  if (xstate == nullptr)
>> +    return X86_XSTATE_SSE_MASK;
>>   
>> -  if (xstate)
>> +  /* Check extended state size.  */
>> +  size_t size = bfd_section_size (xstate);
>> +  if (size < X86_XSTATE_AVX_SIZE)
>> +    return X86_XSTATE_SSE_MASK;
>> +
>> +  char contents[8];
>> +  if (! bfd_get_section_contents (abfd, xstate, contents,
>> +				  I386_LINUX_XSAVE_XCR0_OFFSET, 8))
>>       {
>> -      size_t size = bfd_section_size (xstate);
>> -
>> -      /* Check extended state size.  */
>> -      if (size < X86_XSTATE_AVX_SIZE)
>> -	xcr0 = X86_XSTATE_SSE_MASK;
>> -      else
>> -	{
>> -	  char contents[8];
>> -
>> -	  if (! bfd_get_section_contents (abfd, xstate, contents,
>> -					  I386_LINUX_XSAVE_XCR0_OFFSET,
>> -					  8))
>> -	    {
>> -	      warning (_("Couldn't read `xcr0' bytes from "
>> -			 "`.reg-xstate' section in core file."));
>> -	      return 0;
>> -	    }
>> -
>> -	  xcr0 = bfd_get_64 (abfd, contents);
>> -	}
>> +      warning (_("Couldn't read `xcr0' bytes from "
>> +		 "`.reg-xstate' section in core file."));
>> +      return X86_XSTATE_SSE_MASK;
>>       }
>> -  else
>> -    xcr0 = 0;
>> +
>> +  uint64_t xcr0 = bfd_get_64 (abfd, contents);
>> +
>> +  if (!i387_set_xsave_layout (xcr0, size, layout))
>> +    return X86_XSTATE_SSE_MASK;
>>   
>>     return xcr0;
>>   }
> 
> I think I spotted a change of behavior in
> i386_linux_core_read_xsave_info, which has consequences down the line.
> 
> If there's no .reg-xstate section, we would previously return xcr0 == 0,
> i386_linux_read_description would return nullptr, and we would choose
> between X87 and SSE based on .reg-xfp.
> 
> After the patch, i386_linux_core_read_xsave_info returns SSE, meaning
> we'll never reach the point that checks for .reg-xfp.
> 
> Maybe the first return (if there's not .reg-xstate section) should
> return 0?

Ooo, that's a good catch.  This function is shared with amd64, so I think
it's best if it keeps returning an xcr0 value, but we could patch
i386_linux_core_read_description to instead do this:

static const struct target_desc *
i386_linux_core_read_description (struct gdbarch *gdbarch,
				  struct target_ops *target,
				  bfd *abfd)
{
   /* Linux/i386.  */
   x86_xsave_layout layout;
   uint64_t xcr0 = i386_linux_core_read_xsave_info (abfd, layout);
   if (xcr0 == X86_XSTATE_X87_MASK
       && bfd_get_section_by_name (abfd, ".reg-xfp") != NULL)
     xcr0 = X86_XSTATE_SSE_MASK;

   return i386_linux_read_description (xcr0);
}
  
Simon Marchi July 27, 2023, 9:48 p.m. UTC | #3
> Ooo, that's a good catch.  This function is shared with amd64, so I think
> it's best if it keeps returning an xcr0 value, but we could patch
> i386_linux_core_read_description to instead do this:
> 
> static const struct target_desc *
> i386_linux_core_read_description (struct gdbarch *gdbarch,
>                   struct target_ops *target,
>                   bfd *abfd)
> {
>   /* Linux/i386.  */
>   x86_xsave_layout layout;
>   uint64_t xcr0 = i386_linux_core_read_xsave_info (abfd, layout);
>   if (xcr0 == X86_XSTATE_X87_MASK
>       && bfd_get_section_by_name (abfd, ".reg-xfp") != NULL)
>     xcr0 = X86_XSTATE_SSE_MASK;
> 
>   return i386_linux_read_description (xcr0);
> }

And i386_linux_core_read_xsave_info would return X86_XSTATE_X87_MASK if
it does not find a .reg-xstate section?

Simon
  
John Baldwin July 28, 2023, 4:30 p.m. UTC | #4
On 7/27/23 2:48 PM, Simon Marchi wrote:
> 
>> Ooo, that's a good catch.  This function is shared with amd64, so I think
>> it's best if it keeps returning an xcr0 value, but we could patch
>> i386_linux_core_read_description to instead do this:
>>
>> static const struct target_desc *
>> i386_linux_core_read_description (struct gdbarch *gdbarch,
>>                    struct target_ops *target,
>>                    bfd *abfd)
>> {
>>    /* Linux/i386.  */
>>    x86_xsave_layout layout;
>>    uint64_t xcr0 = i386_linux_core_read_xsave_info (abfd, layout);
>>    if (xcr0 == X86_XSTATE_X87_MASK
>>        && bfd_get_section_by_name (abfd, ".reg-xfp") != NULL)
>>      xcr0 = X86_XSTATE_SSE_MASK;
>>
>>    return i386_linux_read_description (xcr0);
>> }
> 
> And i386_linux_core_read_xsave_info would return X86_XSTATE_X87_MASK if
> it does not find a .reg-xstate section?

Hmmm.  It would need to do something like that yes.  I realize I have a
bug for i386-fbsd as well where it would return SSE when it should be
returning X87.  For amd64, both Linux and FreeBSD use FXSAVE (which
includes SSE) as .reg2 (generic FP regs).  For i386, both use FSAVE (which
only includes X87) for .reg2, and Linux/i386 also has .reg-fxp that uses
FXSAVE.  This means that the "default" xcr0 value really should be SSE
for amd64, and X87 for i386.

I was considering returning a bool from the helper methods
(i386_*_core_read_xsave_info yesterday (it makes the new gdbarch method
implementations slightly easier to read IMO).  Other options are to add a
parameter to the helper that is the "default" value of xcr0 to use, or to
return a value of 0 for xcr0 when no valid XSAVE info is found.  Returning
0 is pretty close to the bool approach without requiring a dummy xcr0 arg
in the gdbarch methods, so I think I'll go with that.

In that case, i386_linux_core_read_description would go back to what it
was in this patch, but the amd64 version would change slightly:

static const struct target_desc *
amd64_linux_core_read_description (struct gdbarch *gdbarch,
				  struct target_ops *target,
				  bfd *abfd)
{
   /* Linux/x86-64.  */
   x86_xsave_layout layout;
   uint64_t xcr0 = i386_linux_core_read_xsave_info (abfd, layout);
   if (xcr0 == 0)
     xcr0 = X86_XSTATE_SSE_MASK;

   return amd64_linux_read_description (xcr0 & X86_XSTATE_ALL_MASK,
				       gdbarch_ptr_bit (gdbarch) == 32);
}

Namely to add the 'xcr == 0' case.  If that approach seems sensible I
will post a new series since it touches both this patch and the FreeBSD
one.
  
Simon Marchi July 28, 2023, 5:58 p.m. UTC | #5
On 2023-07-28 12:30, John Baldwin wrote:
> On 7/27/23 2:48 PM, Simon Marchi wrote:
>>
>>> Ooo, that's a good catch.  This function is shared with amd64, so I think
>>> it's best if it keeps returning an xcr0 value, but we could patch
>>> i386_linux_core_read_description to instead do this:
>>>
>>> static const struct target_desc *
>>> i386_linux_core_read_description (struct gdbarch *gdbarch,
>>>                    struct target_ops *target,
>>>                    bfd *abfd)
>>> {
>>>    /* Linux/i386.  */
>>>    x86_xsave_layout layout;
>>>    uint64_t xcr0 = i386_linux_core_read_xsave_info (abfd, layout);
>>>    if (xcr0 == X86_XSTATE_X87_MASK
>>>        && bfd_get_section_by_name (abfd, ".reg-xfp") != NULL)
>>>      xcr0 = X86_XSTATE_SSE_MASK;
>>>
>>>    return i386_linux_read_description (xcr0);
>>> }
>>
>> And i386_linux_core_read_xsave_info would return X86_XSTATE_X87_MASK if
>> it does not find a .reg-xstate section?
> 
> Hmmm.  It would need to do something like that yes.  I realize I have a
> bug for i386-fbsd as well where it would return SSE when it should be
> returning X87.  For amd64, both Linux and FreeBSD use FXSAVE (which
> includes SSE) as .reg2 (generic FP regs).  For i386, both use FSAVE (which
> only includes X87) for .reg2, and Linux/i386 also has .reg-fxp that uses
> FXSAVE.  This means that the "default" xcr0 value really should be SSE
> for amd64, and X87 for i386.
> 
> I was considering returning a bool from the helper methods
> (i386_*_core_read_xsave_info yesterday (it makes the new gdbarch method
> implementations slightly easier to read IMO).  Other options are to add a
> parameter to the helper that is the "default" value of xcr0 to use, or to
> return a value of 0 for xcr0 when no valid XSAVE info is found.  Returning
> 0 is pretty close to the bool approach without requiring a dummy xcr0 arg
> in the gdbarch methods, so I think I'll go with that.
> 
> In that case, i386_linux_core_read_description would go back to what it
> was in this patch, but the amd64 version would change slightly:
> 
> static const struct target_desc *
> amd64_linux_core_read_description (struct gdbarch *gdbarch,
>                   struct target_ops *target,
>                   bfd *abfd)
> {
>   /* Linux/x86-64.  */
>   x86_xsave_layout layout;
>   uint64_t xcr0 = i386_linux_core_read_xsave_info (abfd, layout);
>   if (xcr0 == 0)
>     xcr0 = X86_XSTATE_SSE_MASK;
> 
>   return amd64_linux_read_description (xcr0 & X86_XSTATE_ALL_MASK,
>                        gdbarch_ptr_bit (gdbarch) == 32);
> }
> 
> Namely to add the 'xcr == 0' case.  If that approach seems sensible I
> will post a new series since it touches both this patch and the FreeBSD
> one.

I previously considered suggesting making the *_core_read_xsave_info
functions return gdb::optional<int>, where they would have returned an
empty optional if the core does not have xsave info.  The calling code
could then fall back to some other strategy.  Returning 0 achieves the
same thing in a different way, it's fine with me (as long as it's
properly documented).  I prefer that over the "passing a default value"
approach.

If the changes only touch this patch, you can send an update just for
this one (instead of the whole series).

Simon
  
John Baldwin July 28, 2023, 9:30 p.m. UTC | #6
On 7/28/23 10:58 AM, Simon Marchi wrote:
> On 2023-07-28 12:30, John Baldwin wrote:
>> On 7/27/23 2:48 PM, Simon Marchi wrote:
>>>
>>>> Ooo, that's a good catch.  This function is shared with amd64, so I think
>>>> it's best if it keeps returning an xcr0 value, but we could patch
>>>> i386_linux_core_read_description to instead do this:
>>>>
>>>> static const struct target_desc *
>>>> i386_linux_core_read_description (struct gdbarch *gdbarch,
>>>>                     struct target_ops *target,
>>>>                     bfd *abfd)
>>>> {
>>>>     /* Linux/i386.  */
>>>>     x86_xsave_layout layout;
>>>>     uint64_t xcr0 = i386_linux_core_read_xsave_info (abfd, layout);
>>>>     if (xcr0 == X86_XSTATE_X87_MASK
>>>>         && bfd_get_section_by_name (abfd, ".reg-xfp") != NULL)
>>>>       xcr0 = X86_XSTATE_SSE_MASK;
>>>>
>>>>     return i386_linux_read_description (xcr0);
>>>> }
>>>
>>> And i386_linux_core_read_xsave_info would return X86_XSTATE_X87_MASK if
>>> it does not find a .reg-xstate section?
>>
>> Hmmm.  It would need to do something like that yes.  I realize I have a
>> bug for i386-fbsd as well where it would return SSE when it should be
>> returning X87.  For amd64, both Linux and FreeBSD use FXSAVE (which
>> includes SSE) as .reg2 (generic FP regs).  For i386, both use FSAVE (which
>> only includes X87) for .reg2, and Linux/i386 also has .reg-fxp that uses
>> FXSAVE.  This means that the "default" xcr0 value really should be SSE
>> for amd64, and X87 for i386.
>>
>> I was considering returning a bool from the helper methods
>> (i386_*_core_read_xsave_info yesterday (it makes the new gdbarch method
>> implementations slightly easier to read IMO).  Other options are to add a
>> parameter to the helper that is the "default" value of xcr0 to use, or to
>> return a value of 0 for xcr0 when no valid XSAVE info is found.  Returning
>> 0 is pretty close to the bool approach without requiring a dummy xcr0 arg
>> in the gdbarch methods, so I think I'll go with that.
>>
>> In that case, i386_linux_core_read_description would go back to what it
>> was in this patch, but the amd64 version would change slightly:
>>
>> static const struct target_desc *
>> amd64_linux_core_read_description (struct gdbarch *gdbarch,
>>                    struct target_ops *target,
>>                    bfd *abfd)
>> {
>>    /* Linux/x86-64.  */
>>    x86_xsave_layout layout;
>>    uint64_t xcr0 = i386_linux_core_read_xsave_info (abfd, layout);
>>    if (xcr0 == 0)
>>      xcr0 = X86_XSTATE_SSE_MASK;
>>
>>    return amd64_linux_read_description (xcr0 & X86_XSTATE_ALL_MASK,
>>                         gdbarch_ptr_bit (gdbarch) == 32);
>> }
>>
>> Namely to add the 'xcr == 0' case.  If that approach seems sensible I
>> will post a new series since it touches both this patch and the FreeBSD
>> one.
> 
> I previously considered suggesting making the *_core_read_xsave_info
> functions return gdb::optional<int>, where they would have returned an
> empty optional if the core does not have xsave info.  The calling code
> could then fall back to some other strategy.  Returning 0 achieves the
> same thing in a different way, it's fine with me (as long as it's
> properly documented).  I prefer that over the "passing a default value"
> approach.
> 
> If the changes only touch this patch, you can send an update just for
> this one (instead of the whole series).

It touches both this one and the similar FreeBSD one, so I've sent both.
This patch (patch 8) also now contains one extra change from Keith's
testing (testing sizeof_xsave != 0 to control use of .reg-xstate for
i386)
  

Patch

diff --git a/gdb/amd64-linux-tdep.c b/gdb/amd64-linux-tdep.c
index cbbac1a0c64..d51f4d5af53 100644
--- a/gdb/amd64-linux-tdep.c
+++ b/gdb/amd64-linux-tdep.c
@@ -1616,7 +1616,8 @@  amd64_linux_core_read_description (struct gdbarch *gdbarch,
 				  bfd *abfd)
 {
   /* Linux/x86-64.  */
-  uint64_t xcr0 = i386_linux_core_read_xcr0 (abfd);
+  x86_xsave_layout layout;
+  uint64_t xcr0 = i386_linux_core_read_xsave_info (abfd, layout);
 
   return amd64_linux_read_description (xcr0 & X86_XSTATE_ALL_MASK,
 				       gdbarch_ptr_bit (gdbarch) == 32);
@@ -1661,8 +1662,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
@@ -1804,6 +1807,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_read_x86_xsave_layout
+    (gdbarch, i386_linux_core_read_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 554c1455a5f..f27efbb1d59 100644
--- a/gdb/i386-linux-tdep.c
+++ b/gdb/i386-linux-tdep.c
@@ -638,45 +638,51 @@  static int i386_linux_sc_reg_offset[] =
   0 * 4				/* %gs */
 };
 
-/* Get XSAVE extended state xcr0 from core dump.  */
+/* See i386-linux-tdep.h.  */
 
 uint64_t
-i386_linux_core_read_xcr0 (bfd *abfd)
+i386_linux_core_read_xsave_info (bfd *abfd, x86_xsave_layout &layout)
 {
   asection *xstate = bfd_get_section_by_name (abfd, ".reg-xstate");
-  uint64_t xcr0;
+  if (xstate == nullptr)
+    return X86_XSTATE_SSE_MASK;
 
-  if (xstate)
+  /* Check extended state size.  */
+  size_t size = bfd_section_size (xstate);
+  if (size < X86_XSTATE_AVX_SIZE)
+    return X86_XSTATE_SSE_MASK;
+
+  char contents[8];
+  if (! bfd_get_section_contents (abfd, xstate, contents,
+				  I386_LINUX_XSAVE_XCR0_OFFSET, 8))
     {
-      size_t size = bfd_section_size (xstate);
-
-      /* Check extended state size.  */
-      if (size < X86_XSTATE_AVX_SIZE)
-	xcr0 = X86_XSTATE_SSE_MASK;
-      else
-	{
-	  char contents[8];
-
-	  if (! bfd_get_section_contents (abfd, xstate, contents,
-					  I386_LINUX_XSAVE_XCR0_OFFSET,
-					  8))
-	    {
-	      warning (_("Couldn't read `xcr0' bytes from "
-			 "`.reg-xstate' section in core file."));
-	      return 0;
-	    }
-
-	  xcr0 = bfd_get_64 (abfd, contents);
-	}
+      warning (_("Couldn't read `xcr0' bytes from "
+		 "`.reg-xstate' section in core file."));
+      return X86_XSTATE_SSE_MASK;
     }
-  else
-    xcr0 = 0;
+
+  uint64_t xcr0 = bfd_get_64 (abfd, contents);
+
+  if (!i387_set_xsave_layout (xcr0, size, layout))
+    return X86_XSTATE_SSE_MASK;
 
   return xcr0;
 }
 
 /* See i386-linux-tdep.h.  */
 
+bool
+i386_linux_core_read_x86_xsave_layout (struct gdbarch *gdbarch,
+				       x86_xsave_layout &layout)
+{
+  if (i386_linux_core_read_xsave_info (core_bfd, layout) == X86_XSTATE_SSE_MASK)
+    return false;
+
+  return true;
+}
+
+/* See i386-linux-tdep.h.  */
+
 const struct target_desc *
 i386_linux_read_description (uint64_t xcr0)
 {
@@ -708,7 +714,8 @@  i386_linux_core_read_description (struct gdbarch *gdbarch,
 				  bfd *abfd)
 {
   /* Linux/i386.  */
-  uint64_t xcr0 = i386_linux_core_read_xcr0 (abfd);
+  x86_xsave_layout layout;
+  uint64_t xcr0 = i386_linux_core_read_xsave_info (abfd, layout);
   const struct target_desc *tdesc = i386_linux_read_description (xcr0);
 
   if (tdesc != NULL)
@@ -768,8 +775,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 +879,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_read_x86_xsave_layout
+    (gdbarch, i386_linux_core_read_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 c0fd1f7f300..66f08aae22d 100644
--- a/gdb/i386-linux-tdep.h
+++ b/gdb/i386-linux-tdep.h
@@ -20,6 +20,8 @@ 
 #ifndef I386_LINUX_TDEP_H
 #define I386_LINUX_TDEP_H
 
+#include "gdbsupport/x86-xstate.h"
+
 /* The Linux kernel pretends there is an additional "orig_eax"
    register.  Since GDB needs access to that register to be able to
    properly restart system calls when necessary (see
@@ -34,8 +36,18 @@ 
 /* Total number of registers for GNU/Linux.  */
 #define I386_LINUX_NUM_REGS (I386_LINUX_ORIG_EAX_REGNUM + 1)
 
-/* Get XSAVE extended state xcr0 from core dump.  */
-extern uint64_t i386_linux_core_read_xcr0 (bfd *abfd);
+/* Read the XSAVE extended state xcr0 value from the ABFD core file.
+   If it appears to be valid, return it and fill LAYOUT with values
+   inferred from that value.
+
+   Otherwise, return X86_XSTATE_SSE_MASK as a fallback and leave
+   LAYOUT untouched.  */
+extern uint64_t i386_linux_core_read_xsave_info (bfd *abfd,
+						 x86_xsave_layout &layout);
+
+/* Implement the core_read_x86_xsave_layout gdbarch method.  */
+extern bool i386_linux_core_read_x86_xsave_layout (struct gdbarch *gdbarch,
+						   x86_xsave_layout &layout);
 
 /* Handle and display information related to the MPX bound violation
    to the user.  */
@@ -52,14 +64,7 @@  extern const struct target_desc *i386_linux_read_description (uint64_t xcr0);
 	  fxsave_bytes[0..463]
 	  sw_usable_bytes[464..511]
 	  xstate_hdr_bytes[512..575]
-	  avx_bytes[576..831]
-	  mpx_bytes [960..1032]
-	  avx512_k_regs[1088..1152]
-	  avx512_zmmh_regs0-7[1153..1407]
-	  avx512_zmmh_regs8-15[1408..1663]
-	  avx512_zmm_regs16-31[1664..2687]
-	  pkru[2688..2752]
-	  future_state etc
+	  extended state regions (AVX, MPX, AVX512, PKRU, etc.)
 	};
 
   Same memory layout will be used for the coredump NT_X86_XSTATE