[lldb] [Process/NetBSD] Use XStateRegSet for all FPU registers

[mgorny]

Summary

Unify the x86 regset API to use XStateRegSet for all FPU registers,
therefore eliminating the legacy API based on FPRegSet. This makes
the code a little bit simpler but most notably, it provides future
compatibility for register caching.

Since the NetBSD kernel takes care of providing compatibility with
pre-XSAVE processors, PT_{G,S}ETXSTATE can be used on systems supporting
only FXSAVE or even plain FSAVE (and unlike PT_{G,S}ETXMMREGS, it
clearly indicates that XMM registers are not supported).

[labath]

I don't know much about netbd, but it definitely seems cleaner...

lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp

146

This is in an anonymous namespace, so static is superfluous. However, this anonymous namespace is against the coding standards so you might as well finish adding statics, and remove it. :)

989–991

Is this code actually reachable? Ideally, I'd expect this to be checked immediately after program startup, so that we can avoid even reporting the existence of [xy]mm registers..

[mgorny]

Removed anonymous namespace and obsolete #define.

lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp

989–991

Yes, it is. I've just run it inside qemu with an emulated non-AVX CPU, and the code definitely gets run. The user-visible result is apparently that the registers are reported as unavailable.

I suppose we could use sysctl (or cpuid) to check for supported instruction sets in constructor but I don't really know how to make LLDB consider them non-existing.

In any case, I think that's material for a separate patch.

[labath]

lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp

989–991

I think you can make them non-existent by returning a smaller number via GetUserRegisterCount. Linux code has something like that, though I'm not sure that it is entirely correct.

Yeah, this is for a separate patch, though it has some implications for the caching code. The current linux code works by assuming that each "register set" is either entirely present or entirely absent (a reasonable assumption I'd say). That might make things tricky of one wants to have a single register set for reading/caching all of xstate data.

I'm wondering if register sets (as presented to the user) and register sets for the purposes of caching (maybe these should have a different name) shouldn't be distinct concepts...

[mgorny]

lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp

989–991

I've been thinking about having two levels of 'register sets' but I've came to the conclusion that the benefit is not worth the added complexity.

[labath]

I'm not sure I agree with that. Given the ever-growing number of registers (which are likely going to all end up in the "xstate" block), I think it is important to be able to add some structure to the list of registers. Also, having the ability to group registers which can come or go together will probably make it easier to implement the "disappearance" of unsupported registers. Given how xstate works, I think the individual register sets are a better match (more or less) for the individual "features" of that state rather than for the block as a whole.

I don't think it should be that hard to come up with a solution that allows caching at xstate level, but still keeps finer-grained groups. One way, which does not require the explicit concept of a "second order group", is to allow the cache reading/writing function to mark multiple sets as read/written. In this world it wouldn't be the generic code which manages the state of each set, but the individual writing function instead. This way the reading/writing function could mark any number of registers sets as read/written. The generic code could just assert that the set that it cares about has been processed correctly.

[mgorny]

I wonder if 'register sets' shouldn't become shared LLVM concept rather than per-plugin.

[labath]

In D90034#2353873, @mgorny wrote:

I wonder if 'register sets' shouldn't become shared LLVM concept rather than per-plugin.

I'm totally down with that. In fact there's been some movement on that front recently, with the introduction of RegisterInfoAndSetInterface class (currently only used for arm).

lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.h

 #include <machine/reg.h>
 // clang-format on
 
 #include "Plugins/Process/NetBSD/NativeRegisterContextNetBSD.h"
 #include "Plugins/Process/Utility/RegisterContext_x86.h"
 #include "Plugins/Process/Utility/NativeRegisterContextWatchpoint_x86.h"
 #include "Plugins/Process/Utility/lldb-x86-register-enums.h"
 
-#if defined(PT_GETXSTATE) && defined(PT_SETXSTATE)
-#define HAVE_XSTATE
-#endif
-
 namespace lldb_private {
 namespace process_netbsd {
 
 class NativeProcessNetBSD;
 
 class NativeRegisterContextNetBSD_x86_64
     : public NativeRegisterContextNetBSD,
       public NativeRegisterContextWatchpoint_x86 {
public:
 
   Status WriteAllRegisterValues(const lldb::DataBufferSP &data_sp) override;
 
   Status
   CopyHardwareWatchpointsFrom(NativeRegisterContextNetBSD &source) override;
 
 private:
   // Private member types.
-  enum { GPRegSet, FPRegSet, XStateRegSet, DBRegSet };
+  enum { GPRegSet, XStateRegSet, DBRegSet };
 
   // Private member variables.
   struct reg m_gpr;
-#if defined(__x86_64__)
-  struct fpreg m_fpr;
-#else
-  struct xmmregs m_fpr;
-#endif
-  struct dbreg m_dbr;
-#ifdef HAVE_XSTATE
   struct xstate m_xstate;
-#endif
+  struct dbreg m_dbr;
 
   int GetSetForNativeRegNum(int reg_num) const;
   int GetDR(int num) const;
 
   Status ReadRegisterSet(uint32_t set);
   Status WriteRegisterSet(uint32_t set);
 };
 
 } // namespace process_netbsd
 } // namespace lldb_private
 
 #endif // #ifndef lldb_NativeRegisterContextNetBSD_x86_64_h
 
 #endif // defined(__x86_64__)

lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp

 #include <err.h>
 #include <stdint.h>
 #include <stdlib.h>
 // clang-format on
 
 using namespace lldb_private;
 using namespace lldb_private::process_netbsd;
 
-// Private namespace.
-
-namespace {
-// x86 64-bit general purpose registers.
 static const uint32_t g_gpr_regnums_x86_64[] = {
     lldb_rax_x86_64,    lldb_rbx_x86_64,    lldb_rcx_x86_64, lldb_rdx_x86_64,
     lldb_rdi_x86_64,    lldb_rsi_x86_64,    lldb_rbp_x86_64, lldb_rsp_x86_64,
     lldb_r8_x86_64,     lldb_r9_x86_64,     lldb_r10_x86_64, lldb_r11_x86_64,
     lldb_r12_x86_64,    lldb_r13_x86_64,    lldb_r14_x86_64, lldb_r15_x86_64,
     lldb_rip_x86_64,    lldb_rflags_x86_64, lldb_cs_x86_64,  lldb_fs_x86_64,
     lldb_gs_x86_64,     lldb_ss_x86_64,     lldb_ds_x86_64,  lldb_es_x86_64,
     lldb_eax_x86_64,    lldb_ebx_x86_64,    lldb_ecx_x86_64, lldb_edx_x86_64,
static const uint32_t g_gpr_regnums_x86_64[] = {
     lldb_r15l_x86_64,   // Low 8 bits or r15
     LLDB_INVALID_REGNUM // register sets need to end with this flag
 };
 static_assert((sizeof(g_gpr_regnums_x86_64) / sizeof(g_gpr_regnums_x86_64[0])) -
                       1 ==
                   k_num_gpr_registers_x86_64,
               "g_gpr_regnums_x86_64 has wrong number of register infos");
 
-// x86 64-bit floating point registers.
-static const uint32_t g_fpu_regnums_x86_64[] = {
+// x86 64-bit registers available via XState.
+static const uint32_t g_xstate_regnums_x86_64[] = {
     lldb_fctrl_x86_64,     lldb_fstat_x86_64, lldb_ftag_x86_64,
     lldb_fop_x86_64,       lldb_fiseg_x86_64, lldb_fioff_x86_64,
     lldb_foseg_x86_64,     lldb_fooff_x86_64, lldb_mxcsr_x86_64,
     lldb_mxcsrmask_x86_64, lldb_st0_x86_64,   lldb_st1_x86_64,
     lldb_st2_x86_64,       lldb_st3_x86_64,   lldb_st4_x86_64,
     lldb_st5_x86_64,       lldb_st6_x86_64,   lldb_st7_x86_64,
     lldb_mm0_x86_64,       lldb_mm1_x86_64,   lldb_mm2_x86_64,
     lldb_mm3_x86_64,       lldb_mm4_x86_64,   lldb_mm5_x86_64,
     lldb_mm6_x86_64,       lldb_mm7_x86_64,   lldb_xmm0_x86_64,
     lldb_xmm1_x86_64,      lldb_xmm2_x86_64,  lldb_xmm3_x86_64,
     lldb_xmm4_x86_64,      lldb_xmm5_x86_64,  lldb_xmm6_x86_64,
     lldb_xmm7_x86_64,      lldb_xmm8_x86_64,  lldb_xmm9_x86_64,
     lldb_xmm10_x86_64,     lldb_xmm11_x86_64, lldb_xmm12_x86_64,
     lldb_xmm13_x86_64,     lldb_xmm14_x86_64, lldb_xmm15_x86_64,
-    LLDB_INVALID_REGNUM // register sets need to end with this flag
-};
-static_assert((sizeof(g_fpu_regnums_x86_64) / sizeof(g_fpu_regnums_x86_64[0])) -
-                      1 ==
-                  k_num_fpr_registers_x86_64,
-              "g_fpu_regnums_x86_64 has wrong number of register infos");
-
-// x86 64-bit registers available via XState.
-static const uint32_t g_xstate_regnums_x86_64[] = {
     lldb_ymm0_x86_64,   lldb_ymm1_x86_64,  lldb_ymm2_x86_64,  lldb_ymm3_x86_64,
     lldb_ymm4_x86_64,   lldb_ymm5_x86_64,  lldb_ymm6_x86_64,  lldb_ymm7_x86_64,
     lldb_ymm8_x86_64,   lldb_ymm9_x86_64,  lldb_ymm10_x86_64, lldb_ymm11_x86_64,
     lldb_ymm12_x86_64,  lldb_ymm13_x86_64, lldb_ymm14_x86_64, lldb_ymm15_x86_64,
     // Note: we currently do not provide them but this is needed to avoid
     // unnamed groups in SBFrame::GetRegisterContext().
     lldb_bnd0_x86_64,    lldb_bnd1_x86_64,    lldb_bnd2_x86_64,
     lldb_bnd3_x86_64,    lldb_bndcfgu_x86_64, lldb_bndstatus_x86_64,
     LLDB_INVALID_REGNUM // register sets need to end with this flag
 };
-static_assert((sizeof(g_xstate_regnums_x86_64) / sizeof(g_xstate_regnums_x86_64[0])) -
+static_assert((sizeof(g_xstate_regnums_x86_64) /
+               sizeof(g_xstate_regnums_x86_64[0])) -
                       1 ==
-                  k_num_avx_registers_x86_64 + k_num_mpx_registers_x86_64,
+                  k_num_fpr_registers_x86_64 + k_num_avx_registers_x86_64 +
+                      k_num_mpx_registers_x86_64,
               "g_xstate_regnums_x86_64 has wrong number of register infos");
 
 // x86 debug registers.
 static const uint32_t g_dbr_regnums_x86_64[] = {
     lldb_dr0_x86_64,   lldb_dr1_x86_64,  lldb_dr2_x86_64,  lldb_dr3_x86_64,
     lldb_dr4_x86_64,   lldb_dr5_x86_64,  lldb_dr6_x86_64,  lldb_dr7_x86_64,
     LLDB_INVALID_REGNUM // register sets need to end with this flag
 };
 static_assert((sizeof(g_dbr_regnums_x86_64) / sizeof(g_dbr_regnums_x86_64[0])) -
                       1 ==
                   k_num_dbr_registers_x86_64,
               "g_dbr_regnums_x86_64 has wrong number of register infos");
 
 // x86 32-bit general purpose registers.
-const uint32_t g_gpr_regnums_i386[] = {
+static const uint32_t g_gpr_regnums_i386[] = {
     lldb_eax_i386,      lldb_ebx_i386,    lldb_ecx_i386, lldb_edx_i386,
     lldb_edi_i386,      lldb_esi_i386,    lldb_ebp_i386, lldb_esp_i386,
     lldb_eip_i386,      lldb_eflags_i386, lldb_cs_i386,  lldb_fs_i386,
     lldb_gs_i386,       lldb_ss_i386,     lldb_ds_i386,  lldb_es_i386,
     lldb_ax_i386,       lldb_bx_i386,     lldb_cx_i386,  lldb_dx_i386,
     lldb_di_i386,       lldb_si_i386,     lldb_bp_i386,  lldb_sp_i386,
     lldb_ah_i386,       lldb_bh_i386,     lldb_ch_i386,  lldb_dh_i386,
     lldb_al_i386,       lldb_bl_i386,     lldb_cl_i386,  lldb_dl_i386,
     LLDB_INVALID_REGNUM // register sets need to end with this flag
 };
 static_assert((sizeof(g_gpr_regnums_i386) / sizeof(g_gpr_regnums_i386[0])) -
                       1 ==
                   k_num_gpr_registers_i386,
               "g_gpr_regnums_i386 has wrong number of register infos");
 
-// x86 32-bit floating point registers.
-const uint32_t g_fpu_regnums_i386[] = {
+// x86 32-bit registers available via XState.
+static const uint32_t g_xstate_regnums_i386[] = {
     lldb_fctrl_i386,    lldb_fstat_i386,     lldb_ftag_i386,  lldb_fop_i386,
     lldb_fiseg_i386,    lldb_fioff_i386,     lldb_foseg_i386, lldb_fooff_i386,
     lldb_mxcsr_i386,    lldb_mxcsrmask_i386, lldb_st0_i386,   lldb_st1_i386,
     lldb_st2_i386,      lldb_st3_i386,       lldb_st4_i386,   lldb_st5_i386,
     lldb_st6_i386,      lldb_st7_i386,       lldb_mm0_i386,   lldb_mm1_i386,
     lldb_mm2_i386,      lldb_mm3_i386,       lldb_mm4_i386,   lldb_mm5_i386,
     lldb_mm6_i386,      lldb_mm7_i386,       lldb_xmm0_i386,  lldb_xmm1_i386,
     lldb_xmm2_i386,     lldb_xmm3_i386,      lldb_xmm4_i386,  lldb_xmm5_i386,
     lldb_xmm6_i386,     lldb_xmm7_i386,
-    LLDB_INVALID_REGNUM // register sets need to end with this flag
-};
-static_assert((sizeof(g_fpu_regnums_i386) / sizeof(g_fpu_regnums_i386[0])) -
-                      1 ==
-                  k_num_fpr_registers_i386,
-              "g_fpu_regnums_i386 has wrong number of register infos");
-
-// x86 64-bit registers available via XState.
-static const uint32_t g_xstate_regnums_i386[] = {
     lldb_ymm0_i386,     lldb_ymm1_i386,  lldb_ymm2_i386,  lldb_ymm3_i386,
     lldb_ymm4_i386,     lldb_ymm5_i386,  lldb_ymm6_i386,  lldb_ymm7_i386,
     // Note: we currently do not provide them but this is needed to avoid
     // unnamed groups in SBFrame::GetRegisterContext().
     lldb_bnd0_i386,      lldb_bnd1_i386,    lldb_bnd2_i386,
     lldb_bnd3_i386,      lldb_bndcfgu_i386, lldb_bndstatus_i386,
     LLDB_INVALID_REGNUM // register sets need to end with this flag
 };
 static_assert((sizeof(g_xstate_regnums_i386) / sizeof(g_xstate_regnums_i386[0])) -
                       1 ==
-                  k_num_avx_registers_i386 + k_num_mpx_registers_i386,
+                  k_num_fpr_registers_i386 + k_num_avx_registers_i386 + k_num_mpx_registers_i386,
               "g_xstate_regnums_i386 has wrong number of register infos");
 
 // x86 debug registers.
 static const uint32_t g_dbr_regnums_i386[] = {
     lldb_dr0_i386,   lldb_dr1_i386,  lldb_dr2_i386,  lldb_dr3_i386,
     lldb_dr4_i386,   lldb_dr5_i386,  lldb_dr6_i386,  lldb_dr7_i386,
     LLDB_INVALID_REGNUM // register sets need to end with this flag
 };
 static_assert((sizeof(g_dbr_regnums_i386) / sizeof(g_dbr_regnums_i386[0])) -
                       1 ==
                   k_num_dbr_registers_i386,
               "g_dbr_regnums_i386 has wrong number of register infos");
 
 
 // Number of register sets provided by this context.
 enum { k_num_register_sets = 4 };
 
 // Register sets for x86 32-bit.
 static const RegisterSet g_reg_sets_i386[k_num_register_sets] = {
     {"General Purpose Registers", "gpr", k_num_gpr_registers_i386,
      g_gpr_regnums_i386},
-    {"Floating Point Registers", "fpu", k_num_fpr_registers_i386,
-     g_fpu_regnums_i386},
     {"Extended State Registers", "xstate",
      k_num_avx_registers_i386 + k_num_mpx_registers_i386,
      g_xstate_regnums_i386},
     {"Debug Registers", "dbr", k_num_dbr_registers_i386,
      g_dbr_regnums_i386},
 };
 
 // Register sets for x86 64-bit.
 static const RegisterSet g_reg_sets_x86_64[k_num_register_sets] = {
     {"General Purpose Registers", "gpr", k_num_gpr_registers_x86_64,
      g_gpr_regnums_x86_64},
-    {"Floating Point Registers", "fpu", k_num_fpr_registers_x86_64,
-     g_fpu_regnums_x86_64},
     {"Extended State Registers", "xstate",
      k_num_avx_registers_x86_64 + k_num_mpx_registers_x86_64,
      g_xstate_regnums_x86_64},
     {"Debug Registers", "dbr", k_num_dbr_registers_x86_64,
      g_dbr_regnums_x86_64},
 };
 
 #define REG_CONTEXT_SIZE (GetRegisterInfoInterface().GetGPRSize())
-} // namespace
 
 NativeRegisterContextNetBSD *
 NativeRegisterContextNetBSD::CreateHostNativeRegisterContextNetBSD(
     const ArchSpec &target_arch, NativeThreadProtocol &native_thread) {
   return new NativeRegisterContextNetBSD_x86_64(target_arch, native_thread);
 }
 
 // NativeRegisterContextNetBSD_x86_64 members.
if (HostInfo::GetArchitecture().GetAddressByteSize() == 4) {
     return new RegisterContextNetBSD_x86_64(target_arch);
   }
 }
 
 NativeRegisterContextNetBSD_x86_64::NativeRegisterContextNetBSD_x86_64(
     const ArchSpec &target_arch, NativeThreadProtocol &native_thread)
     : NativeRegisterContextRegisterInfo(
           native_thread, CreateRegisterInfoInterface(target_arch)),
-      m_gpr(), m_fpr(), m_dbr() {}
+      m_gpr(), m_xstate(), m_dbr() {}
 
 // CONSIDER after local and llgs debugging are merged, register set support can
 // be moved into a base x86-64 class with IsRegisterSetAvailable made virtual.
 uint32_t NativeRegisterContextNetBSD_x86_64::GetRegisterSetCount() const {
   uint32_t sets = 0;
   for (uint32_t set_index = 0; set_index < k_num_register_sets; ++set_index) {
     if (GetSetForNativeRegNum(set_index) != -1)
       ++sets;

 
 int NativeRegisterContextNetBSD_x86_64::GetSetForNativeRegNum(
     int reg_num) const {
   switch (GetRegisterInfoInterface().GetTargetArchitecture().GetMachine()) {
   case llvm::Triple::x86:
     if (reg_num >= k_first_gpr_i386 && reg_num <= k_last_gpr_i386)
       return GPRegSet;
     if (reg_num >= k_first_fpr_i386 && reg_num <= k_last_fpr_i386)
-      return FPRegSet;
+      return XStateRegSet;
     if (reg_num >= k_first_avx_i386 && reg_num <= k_last_avx_i386)
       return XStateRegSet; // AVX
     if (reg_num >= k_first_mpxr_i386 && reg_num <= k_last_mpxr_i386)
       return -1; // MPXR
     if (reg_num >= k_first_mpxc_i386 && reg_num <= k_last_mpxc_i386)
       return -1; // MPXC
     if (reg_num >= k_first_dbr_i386 && reg_num <= k_last_dbr_i386)
       return DBRegSet; // DBR
     break;
   case llvm::Triple::x86_64:
     if (reg_num >= k_first_gpr_x86_64 && reg_num <= k_last_gpr_x86_64)
       return GPRegSet;
     if (reg_num >= k_first_fpr_x86_64 && reg_num <= k_last_fpr_x86_64)
-      return FPRegSet;
+      return XStateRegSet;
     if (reg_num >= k_first_avx_x86_64 && reg_num <= k_last_avx_x86_64)
       return XStateRegSet; // AVX
     if (reg_num >= k_first_mpxr_x86_64 && reg_num <= k_last_mpxr_x86_64)
       return -1; // MPXR
     if (reg_num >= k_first_mpxc_x86_64 && reg_num <= k_last_mpxc_x86_64)
       return -1; // MPXC
     if (reg_num >= k_first_dbr_x86_64 && reg_num <= k_last_dbr_x86_64)
       return DBRegSet; // DBR
     break;
   default:
     llvm_unreachable("Unhandled target architecture.");
   }
 
   llvm_unreachable("Register does not belong to any register set");
 }
 
 Status NativeRegisterContextNetBSD_x86_64::ReadRegisterSet(uint32_t set) {
   switch (set) {
   case GPRegSet:
     return DoRegisterSet(PT_GETREGS, &m_gpr);
-  case FPRegSet:
-#if defined(__x86_64__)
-    return DoRegisterSet(PT_GETFPREGS, &m_fpr);
-#else
-    return DoRegisterSet(PT_GETXMMREGS, &m_fpr);
-#endif
   case DBRegSet:
     return DoRegisterSet(PT_GETDBREGS, &m_dbr);
-  case XStateRegSet:
-#ifdef HAVE_XSTATE
-    {
-      struct iovec iov = {&m_xstate, sizeof(m_xstate)};
-      return DoRegisterSet(PT_GETXSTATE, &iov);
-    }
-#else
-    return Status("XState is not supported by the kernel");
-#endif
+  case XStateRegSet: {
   }
   llvm_unreachable("NativeRegisterContextNetBSD_x86_64::ReadRegisterSet");
 }
 
 Status NativeRegisterContextNetBSD_x86_64::WriteRegisterSet(uint32_t set) {
   switch (set) {
   case GPRegSet:
     return DoRegisterSet(PT_SETREGS, &m_gpr);
-  case FPRegSet:
-#if defined(__x86_64__)
-    return DoRegisterSet(PT_SETFPREGS, &m_fpr);
-#else
-    return DoRegisterSet(PT_SETXMMREGS, &m_fpr);
-#endif
   case DBRegSet:
     return DoRegisterSet(PT_SETDBREGS, &m_dbr);
-  case XStateRegSet:
-#ifdef HAVE_XSTATE
-    {
-      struct iovec iov = {&m_xstate, sizeof(m_xstate)};
-      return DoRegisterSet(PT_SETXSTATE, &iov);
-    }
-#else
-    return Status("XState is not supported by the kernel");
-#endif
+  case XStateRegSet: {
   }
   llvm_unreachable("NativeRegisterContextNetBSD_x86_64::WriteRegisterSet");
 }
 
 Status
 NativeRegisterContextNetBSD_x86_64::ReadRegister(const RegisterInfo *reg_info,
                                                  RegisterValue &reg_value) {
   Status error;
#else
   case lldb_ds_x86_64:
     reg_value = (uint32_t)m_gpr.r_ds;
     break;
   case lldb_es_x86_64:
     reg_value = (uint32_t)m_gpr.r_es;
     break;
 #endif
   case lldb_fctrl_x86_64:
-    reg_value = (uint16_t)m_fpr.fxstate.fx_cw;
+    reg_value = (uint16_t)m_xstate.xs_fxsave.fx_cw;
     break;
   case lldb_fstat_x86_64:
-    reg_value = (uint16_t)m_fpr.fxstate.fx_sw;
+    reg_value = (uint16_t)m_xstate.xs_fxsave.fx_sw;
     break;
   case lldb_ftag_x86_64:
-    reg_value = (uint16_t)m_fpr.fxstate.fx_tw;
+    reg_value = (uint16_t)m_xstate.xs_fxsave.fx_tw;
     break;
   case lldb_fop_x86_64:
-    reg_value = (uint64_t)m_fpr.fxstate.fx_opcode;
+    reg_value = (uint64_t)m_xstate.xs_fxsave.fx_opcode;
     break;
   case lldb_fiseg_x86_64:
-    reg_value = (uint32_t)m_fpr.fxstate.fx_ip.fa_32.fa_seg;
+    reg_value = (uint32_t)m_xstate.xs_fxsave.fx_ip.fa_32.fa_seg;
     break;
   case lldb_fioff_x86_64:
-    reg_value = (uint32_t)m_fpr.fxstate.fx_ip.fa_32.fa_off;
+    reg_value = (uint32_t)m_xstate.xs_fxsave.fx_ip.fa_32.fa_off;
     break;
   case lldb_foseg_x86_64:
-    reg_value = (uint32_t)m_fpr.fxstate.fx_dp.fa_32.fa_seg;
+    reg_value = (uint32_t)m_xstate.xs_fxsave.fx_dp.fa_32.fa_seg;
     break;
   case lldb_fooff_x86_64:
-    reg_value = (uint32_t)m_fpr.fxstate.fx_dp.fa_32.fa_off;
+    reg_value = (uint32_t)m_xstate.xs_fxsave.fx_dp.fa_32.fa_off;
     break;
   case lldb_mxcsr_x86_64:
-    reg_value = (uint32_t)m_fpr.fxstate.fx_mxcsr;
+    reg_value = (uint32_t)m_xstate.xs_fxsave.fx_mxcsr;
     break;
   case lldb_mxcsrmask_x86_64:
-    reg_value = (uint32_t)m_fpr.fxstate.fx_mxcsr_mask;
+    reg_value = (uint32_t)m_xstate.xs_fxsave.fx_mxcsr_mask;
     break;
   case lldb_st0_x86_64:
   case lldb_st1_x86_64:
   case lldb_st2_x86_64:
   case lldb_st3_x86_64:
   case lldb_st4_x86_64:
   case lldb_st5_x86_64:
   case lldb_st6_x86_64:
   case lldb_st7_x86_64:
-    reg_value.SetBytes(&m_fpr.fxstate.fx_87_ac[reg - lldb_st0_x86_64],
+    reg_value.SetBytes(&m_xstate.xs_fxsave.fx_87_ac[reg - lldb_st0_x86_64],
                        reg_info->byte_size, endian::InlHostByteOrder());
     break;
   case lldb_mm0_x86_64:
   case lldb_mm1_x86_64:
   case lldb_mm2_x86_64:
   case lldb_mm3_x86_64:
   case lldb_mm4_x86_64:
   case lldb_mm5_x86_64:
   case lldb_mm6_x86_64:
   case lldb_mm7_x86_64:
-    reg_value.SetBytes(&m_fpr.fxstate.fx_87_ac[reg - lldb_mm0_x86_64],
+    reg_value.SetBytes(&m_xstate.xs_fxsave.fx_87_ac[reg - lldb_mm0_x86_64],
                        reg_info->byte_size, endian::InlHostByteOrder());
     break;
   case lldb_xmm0_x86_64:
   case lldb_xmm1_x86_64:
   case lldb_xmm2_x86_64:
   case lldb_xmm3_x86_64:
   case lldb_xmm4_x86_64:
   case lldb_xmm5_x86_64:
   case lldb_xmm6_x86_64:
   case lldb_xmm7_x86_64:
   case lldb_xmm8_x86_64:
   case lldb_xmm9_x86_64:
   case lldb_xmm10_x86_64:
   case lldb_xmm11_x86_64:
   case lldb_xmm12_x86_64:
   case lldb_xmm13_x86_64:
   case lldb_xmm14_x86_64:
   case lldb_xmm15_x86_64:
-    reg_value.SetBytes(&m_fpr.fxstate.fx_xmm[reg - lldb_xmm0_x86_64],
+    if (!(m_xstate.xs_rfbm & XCR0_SSE)) {
+      error.SetErrorStringWithFormat(
+          "register \"%s\" not supported by CPU/kernel", reg_info->name);
+    } else {
+      reg_value.SetBytes(&m_xstate.xs_fxsave.fx_xmm[reg - lldb_xmm0_x86_64],
                          reg_info->byte_size, endian::InlHostByteOrder());
+    }
     break;
   case lldb_ymm0_x86_64:
   case lldb_ymm1_x86_64:
   case lldb_ymm2_x86_64:
   case lldb_ymm3_x86_64:
   case lldb_ymm4_x86_64:
   case lldb_ymm5_x86_64:
   case lldb_ymm6_x86_64:
   case lldb_ymm7_x86_64:
   case lldb_ymm8_x86_64:
   case lldb_ymm9_x86_64:
   case lldb_ymm10_x86_64:
   case lldb_ymm11_x86_64:
   case lldb_ymm12_x86_64:
   case lldb_ymm13_x86_64:
   case lldb_ymm14_x86_64:
   case lldb_ymm15_x86_64:
-#ifdef HAVE_XSTATE
     if (!(m_xstate.xs_rfbm & XCR0_SSE) ||
         !(m_xstate.xs_rfbm & XCR0_YMM_Hi128)) {
-      error.SetErrorStringWithFormat("register \"%s\" not supported by CPU/kernel",
-                                     reg_info->name);
+      error.SetErrorStringWithFormat(
+          "register \"%s\" not supported by CPU/kernel", reg_info->name);
     } else {
       uint32_t reg_index = reg - lldb_ymm0_x86_64;
-      YMMReg ymm = XStateToYMM(
-          m_xstate.xs_fxsave.fx_xmm[reg_index].xmm_bytes,
+      YMMReg ymm =
+          XStateToYMM(m_xstate.xs_fxsave.fx_xmm[reg_index].xmm_bytes,
                       m_xstate.xs_ymm_hi128.xs_ymm[reg_index].ymm_bytes);
       reg_value.SetBytes(ymm.bytes, reg_info->byte_size,
                          endian::InlHostByteOrder());
     }
-#else
-    error.SetErrorString("XState queries not supported by the kernel");
-#endif
     break;
   case lldb_dr0_x86_64:
   case lldb_dr1_x86_64:
   case lldb_dr2_x86_64:
   case lldb_dr3_x86_64:
   case lldb_dr4_x86_64:
   case lldb_dr5_x86_64:
   case lldb_dr6_x86_64:
#else
   case lldb_ds_x86_64:
     m_gpr.r_ds = reg_value.GetAsUInt32();
     break;
   case lldb_es_x86_64:
     m_gpr.r_es = reg_value.GetAsUInt32();
     break;
 #endif
   case lldb_fctrl_x86_64:
-    m_fpr.fxstate.fx_cw = reg_value.GetAsUInt16();
+    m_xstate.xs_fxsave.fx_cw = reg_value.GetAsUInt16();
     break;
   case lldb_fstat_x86_64:
-    m_fpr.fxstate.fx_sw = reg_value.GetAsUInt16();
+    m_xstate.xs_fxsave.fx_sw = reg_value.GetAsUInt16();
     break;
   case lldb_ftag_x86_64:
-    m_fpr.fxstate.fx_tw = reg_value.GetAsUInt16();
+    m_xstate.xs_fxsave.fx_tw = reg_value.GetAsUInt16();
     break;
   case lldb_fop_x86_64:
-    m_fpr.fxstate.fx_opcode = reg_value.GetAsUInt16();
+    m_xstate.xs_fxsave.fx_opcode = reg_value.GetAsUInt16();
     break;
   case lldb_fiseg_x86_64:
-    m_fpr.fxstate.fx_ip.fa_32.fa_seg = reg_value.GetAsUInt32();
+    m_xstate.xs_fxsave.fx_ip.fa_32.fa_seg = reg_value.GetAsUInt32();
     break;
   case lldb_fioff_x86_64:
-    m_fpr.fxstate.fx_ip.fa_32.fa_off = reg_value.GetAsUInt32();
+    m_xstate.xs_fxsave.fx_ip.fa_32.fa_off = reg_value.GetAsUInt32();
     break;
   case lldb_foseg_x86_64:
-    m_fpr.fxstate.fx_dp.fa_32.fa_seg = reg_value.GetAsUInt32();
+    m_xstate.xs_fxsave.fx_dp.fa_32.fa_seg = reg_value.GetAsUInt32();
     break;
   case lldb_fooff_x86_64:
-    m_fpr.fxstate.fx_dp.fa_32.fa_off = reg_value.GetAsUInt32();
+    m_xstate.xs_fxsave.fx_dp.fa_32.fa_off = reg_value.GetAsUInt32();
     break;
   case lldb_mxcsr_x86_64:
-    m_fpr.fxstate.fx_mxcsr = reg_value.GetAsUInt32();
+    m_xstate.xs_fxsave.fx_mxcsr = reg_value.GetAsUInt32();
     break;
   case lldb_mxcsrmask_x86_64:
-    m_fpr.fxstate.fx_mxcsr_mask = reg_value.GetAsUInt32();
+    m_xstate.xs_fxsave.fx_mxcsr_mask = reg_value.GetAsUInt32();
     break;
   case lldb_st0_x86_64:
   case lldb_st1_x86_64:
   case lldb_st2_x86_64:
   case lldb_st3_x86_64:
   case lldb_st4_x86_64:
   case lldb_st5_x86_64:
   case lldb_st6_x86_64:
   case lldb_st7_x86_64:
-    ::memcpy(&m_fpr.fxstate.fx_87_ac[reg - lldb_st0_x86_64],
+    ::memcpy(&m_xstate.xs_fxsave.fx_87_ac[reg - lldb_st0_x86_64],
              reg_value.GetBytes(), reg_value.GetByteSize());
     break;
   case lldb_mm0_x86_64:
   case lldb_mm1_x86_64:
   case lldb_mm2_x86_64:
   case lldb_mm3_x86_64:
   case lldb_mm4_x86_64:
   case lldb_mm5_x86_64:
   case lldb_mm6_x86_64:
   case lldb_mm7_x86_64:
-    ::memcpy(&m_fpr.fxstate.fx_87_ac[reg - lldb_mm0_x86_64],
+    ::memcpy(&m_xstate.xs_fxsave.fx_87_ac[reg - lldb_mm0_x86_64],
              reg_value.GetBytes(), reg_value.GetByteSize());
     break;
   case lldb_xmm0_x86_64:
   case lldb_xmm1_x86_64:
   case lldb_xmm2_x86_64:
   case lldb_xmm3_x86_64:
   case lldb_xmm4_x86_64:
   case lldb_xmm5_x86_64:
   case lldb_xmm6_x86_64:
   case lldb_xmm7_x86_64:
   case lldb_xmm8_x86_64:
   case lldb_xmm9_x86_64:
   case lldb_xmm10_x86_64:
   case lldb_xmm11_x86_64:
   case lldb_xmm12_x86_64:
   case lldb_xmm13_x86_64:
   case lldb_xmm14_x86_64:
   case lldb_xmm15_x86_64:
-    ::memcpy(&m_fpr.fxstate.fx_xmm[reg - lldb_xmm0_x86_64],
+    if (!(m_xstate.xs_rfbm & XCR0_SSE)) {
+      error.SetErrorStringWithFormat(
+          "register \"%s\" not supported by CPU/kernel", reg_info->name);
+    } else {
+      ::memcpy(&m_xstate.xs_fxsave.fx_xmm[reg - lldb_xmm0_x86_64],
                reg_value.GetBytes(), reg_value.GetByteSize());
+    }
     break;
   case lldb_ymm0_x86_64:
   case lldb_ymm1_x86_64:
   case lldb_ymm2_x86_64:
   case lldb_ymm3_x86_64:
   case lldb_ymm4_x86_64:
   case lldb_ymm5_x86_64:
   case lldb_ymm6_x86_64:
   case lldb_ymm7_x86_64:
   case lldb_ymm8_x86_64:
   case lldb_ymm9_x86_64:
   case lldb_ymm10_x86_64:
   case lldb_ymm11_x86_64:
   case lldb_ymm12_x86_64:
   case lldb_ymm13_x86_64:
   case lldb_ymm14_x86_64:
   case lldb_ymm15_x86_64:
-#ifdef HAVE_XSTATE
     if (!(m_xstate.xs_rfbm & XCR0_SSE) ||
         !(m_xstate.xs_rfbm & XCR0_YMM_Hi128)) {
-      error.SetErrorStringWithFormat("register \"%s\" not supported by CPU/kernel",
-                                     reg_info->name);
+      error.SetErrorStringWithFormat(
+          "register \"%s\" not supported by CPU/kernel", reg_info->name);
     } else {
       uint32_t reg_index = reg - lldb_ymm0_x86_64;
       YMMReg ymm;
       ::memcpy(ymm.bytes, reg_value.GetBytes(), reg_value.GetByteSize());
-      YMMToXState(ymm,
-          m_xstate.xs_fxsave.fx_xmm[reg_index].xmm_bytes,
+      YMMToXState(ymm, m_xstate.xs_fxsave.fx_xmm[reg_index].xmm_bytes,
                   m_xstate.xs_ymm_hi128.xs_ymm[reg_index].ymm_bytes);
     }
-#else
-    error.SetErrorString("XState not supported by the kernel");
-    return error;
-#endif
     break;
   case lldb_dr0_x86_64:
   case lldb_dr1_x86_64:
   case lldb_dr2_x86_64:
   case lldb_dr3_x86_64:
   case lldb_dr4_x86_64:
   case lldb_dr5_x86_64:
   case lldb_dr6_x86_64: