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

[John Baldwin <jhb@FreeBSD.org>]

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