Re: [PATCH v2 1/5] gdb/aarch64: Enable FPMR for AArch64 in gdb on Linux

[Luis <luis.machado.foss@gmail.com>]

On 07/10/2025 13:31, Ezra.Sitorus@arm.com wrote:
> From: Ezra Sitorus <ezra.sitorus@arm.com>
> 
> The Floating Point Mode Register controls the behaviours of FP8
> instructions. This patch add FPMR to GDB if it is enabled on the
> target.
> ---
> Changes from v1->v2:
> * Addressed comments/whitespace/formatting issues.
> * gdb/arch/aarch64.h: operator() takes fpmr into account now.
> * Defined HWCAP2_FPMR in gdb/arch/aarch64.h
> 
> Ezra
> 
>   gdb/aarch64-linux-nat.c       | 57 +++++++++++++++++++++++++++++++++++
>   gdb/aarch64-linux-tdep.c      |  1 +
>   gdb/aarch64-tdep.c            | 18 ++++++++++-
>   gdb/aarch64-tdep.h            |  9 ++++++
>   gdb/arch/aarch64.c            |  4 +++
>   gdb/arch/aarch64.h            | 12 +++++++-
>   gdb/features/Makefile         |  1 +
>   gdb/features/aarch64-fpmr.c   | 44 +++++++++++++++++++++++++++
>   gdb/features/aarch64-fpmr.xml | 53 ++++++++++++++++++++++++++++++++
>   9 files changed, 197 insertions(+), 2 deletions(-)
>   create mode 100644 gdb/features/aarch64-fpmr.c
>   create mode 100644 gdb/features/aarch64-fpmr.xml
> 
> diff --git a/gdb/aarch64-linux-nat.c b/gdb/aarch64-linux-nat.c
> index 89ecedda57d..503a41c973d 100644
> --- a/gdb/aarch64-linux-nat.c
> +++ b/gdb/aarch64-linux-nat.c
> @@ -604,6 +604,48 @@ store_gcsregs_to_thread (regcache *regcache)
>       perror_with_name (_("Unable to store GCS registers"));
>   }
>   
> +/* Fill GDB's REGCACHE with the FPMR register set content from the
> +   thread associated with REGCACHE.  */
> +
> +static void
> +fetch_fpmr_from_thread (struct regcache *regcache)
> +{
> +  aarch64_gdbarch_tdep *tdep
> +    = gdbarch_tdep<aarch64_gdbarch_tdep> (regcache->arch ());
> +
> +    int tid = regcache->ptid ().lwp ();
> +
> +    struct iovec iov;
> +    uint64_t val;
> +    iov.iov_base = &val;
> +    iov.iov_len = sizeof (val);
> +
> +    if (ptrace (PTRACE_GETREGSET, tid, NT_ARM_FPMR, &iov) < 0)
> +      perror_with_name (_("Unable to fetch FPMR register set"));
> +    regcache->raw_supply (tdep->fpmr_regnum, &val);
> +}
> +
> +/* Store the NT_ARM_FPMR register set contents from GDB's REGCACHE to the
> +    thread associated with REGCACHE.  */
> +
> +static void
> +store_fpmr_to_thread (struct regcache *regcache)
> +{
> +  aarch64_gdbarch_tdep *tdep
> +    = gdbarch_tdep<aarch64_gdbarch_tdep> (regcache->arch ());
> +
> +  int tid = regcache->ptid ().lwp ();
> +
> +  struct iovec iov;
> +  uint64_t val;
> +  iov.iov_base = &val;
> +  iov.iov_len = sizeof (val);
> +
> +  regcache->raw_collect (tdep->fpmr_regnum, (char *) &val);
> +  if (ptrace (PTRACE_SETREGSET, tid, NT_ARM_FPMR, &iov) < 0)
> +    perror_with_name (_("Unable to store FPMR register set"));
> +}
> +
>   /* The AArch64 version of the "fetch_registers" target_ops method.  Fetch
>      REGNO from the target and place the result into REGCACHE.  */
>   
> @@ -642,6 +684,9 @@ aarch64_fetch_registers (struct regcache *regcache, int regno)
>   
>         if (tdep->has_gcs_linux ())
>   	fetch_gcsregs_from_thread (regcache);
> +
> +      if (tdep->has_fpmr ())
> +	fetch_fpmr_from_thread (regcache);
>       }
>     /* General purpose register?  */
>     else if (regno < AARCH64_V0_REGNUM)
> @@ -679,6 +724,9 @@ aarch64_fetch_registers (struct regcache *regcache, int regno)
>   	   && (regno == tdep->gcs_reg_base || regno == tdep->gcs_linux_reg_base
>   	       || regno == tdep->gcs_linux_reg_base + 1))
>       fetch_gcsregs_from_thread (regcache);
> +  /* FPMR?  */
> +  else if (tdep->has_fpmr () && (regno == tdep->fpmr_regnum))
> +    fetch_fpmr_from_thread (regcache);
>   }
>   
>   /* A version of the "fetch_registers" target_ops method used when running
> @@ -753,6 +801,9 @@ aarch64_store_registers (struct regcache *regcache, int regno)
>   
>         if (tdep->has_gcs_linux ())
>   	store_gcsregs_to_thread (regcache);
> +
> +      if (tdep->has_fpmr ())
> +	store_fpmr_to_thread (regcache);
>       }
>     /* General purpose register?  */
>     else if (regno < AARCH64_V0_REGNUM)
> @@ -784,6 +835,9 @@ aarch64_store_registers (struct regcache *regcache, int regno)
>   	   && (regno == tdep->gcs_reg_base || regno == tdep->gcs_linux_reg_base
>   	       || regno == tdep->gcs_linux_reg_base + 1))
>       store_gcsregs_to_thread (regcache);
> +  /* FPMR?  */
> +  else if (tdep->has_fpmr () && regno == tdep->fpmr_regnum)
> +    store_fpmr_to_thread (regcache);
>   
>     /* PAuth registers are read-only.  */
>   }
> @@ -969,6 +1023,9 @@ aarch64_linux_nat_target::read_description ()
>     if ((hwcap2 & HWCAP2_SME2) || (hwcap2 & HWCAP2_SME2P1))
>       features.sme2 = supports_zt_registers (tid);
>   
> +  /* Check for FPMR.  */
> +  features.fpmr = hwcap2 & HWCAP2_FPMR;
> +
>     return aarch64_read_description (features);
>   }
>   
> diff --git a/gdb/aarch64-linux-tdep.c b/gdb/aarch64-linux-tdep.c
> index 048be4f3532..10b44d978af 100644
> --- a/gdb/aarch64-linux-tdep.c
> +++ b/gdb/aarch64-linux-tdep.c
> @@ -1712,6 +1712,7 @@ aarch64_linux_core_read_description (struct gdbarch *gdbarch,
>     features.pauth = hwcap & AARCH64_HWCAP_PACA;
>     features.gcs = features.gcs_linux = hwcap & HWCAP_GCS;
>     features.mte = hwcap2 & HWCAP2_MTE;
> +  features.fpmr = hwcap2 & HWCAP2_FPMR;
>   
>     /* Handle the TLS section.  */
>     asection *tls = bfd_get_section_by_name (abfd, ".reg-aarch-tls");
> diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
> index 500ac77d75a..6c720ce8427 100644
> --- a/gdb/aarch64-tdep.c
> +++ b/gdb/aarch64-tdep.c
> @@ -4140,9 +4140,14 @@ aarch64_features_from_target_desc (const struct target_desc *tdesc)
>     features.gcs_linux = (tdesc_find_feature (tdesc, "org.gnu.gdb.aarch64.gcs.linux")
>   			!= nullptr);
>   
> +  /* Check for FPMR feature.  */
> +  features.fpmr = (tdesc_find_feature (tdesc, "org.gnu.gdb.aarch64.fpmr")
> +		   != nullptr);
> +
>     return features;
>   }
>   
> +

Spurious blank line.

>   /* Implement the "cannot_store_register" gdbarch method.  */
>   
>   static int
> @@ -4448,11 +4453,12 @@ static struct gdbarch *
>   aarch64_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
>   {
>     const struct tdesc_feature *feature_core, *feature_fpu, *feature_sve;
> -  const struct tdesc_feature *feature_pauth;
> +  const struct tdesc_feature *feature_pauth, *feature_fpmr;
>     bool valid_p = true;
>     int i, num_regs = 0, num_pseudo_regs = 0;
>     int first_pauth_regnum = -1, ra_sign_state_offset = -1;
>     int first_mte_regnum = -1, first_tls_regnum = -1;
> +  int fpmr_regnum = -1;

Nit, but you can move the above initialization closer to where it is 
used. Same for feature_fpmr above.

>     uint64_t vq = aarch64_get_tdesc_vq (info.target_desc);
>     uint64_t svq = aarch64_get_tdesc_svq (info.target_desc);
>   
> @@ -4550,6 +4556,14 @@ aarch64_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
>         num_pseudo_regs += 32;	/* add the Bn scalar register pseudos */
>       }
>   
> +  feature_fpmr = tdesc_find_feature (tdesc, "org.gnu.gdb.aarch64.fpmr");
> +  if (feature_fpmr != nullptr)
> +    {
> +      fpmr_regnum = num_regs++;
> +      valid_p &= tdesc_numbered_register (feature_fpmr, tdesc_data.get (),
> +					  fpmr_regnum, "fpmr");
> +    }
> +
>     int first_sme_regnum = -1;
>     int first_sme2_regnum = -1;
>     int first_sme_pseudo_regnum = -1;
> @@ -4761,6 +4775,8 @@ aarch64_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
>     /* Set the SME2 register set details.  */
>     tdep->sme2_zt0_regnum = first_sme2_regnum;
>   
> +  /* Set the FPMR regnum.  */
> +  tdep->fpmr_regnum = fpmr_regnum;

Nit. Make the above assignment part of the same block of assignments to 
tdep values above instead of making it part of the block that sets the 
gdbarch hooks below. It provides some visual separation and makes the 
code easier to read.

>     set_gdbarch_push_dummy_call (gdbarch, aarch64_push_dummy_call);
>     set_gdbarch_frame_align (gdbarch, aarch64_frame_align);
>   
> diff --git a/gdb/aarch64-tdep.h b/gdb/aarch64-tdep.h
> index 99e7d26ce4a..9acd29b2d88 100644
> --- a/gdb/aarch64-tdep.h
> +++ b/gdb/aarch64-tdep.h
> @@ -207,6 +207,15 @@ struct aarch64_gdbarch_tdep : gdbarch_tdep_base
>     {
>       return gcs_linux_reg_base != -1;
>     }
> +
> +  /* First FPMR register.  This is -1 if FPMR is not supported.  */
> +  int fpmr_regnum = -1;
> +
> +  bool
> +  has_fpmr () const
> +  {
> +    return fpmr_regnum != -1;
> +  }
>   };
>   
>   const target_desc *aarch64_read_description (const aarch64_features &features);
> diff --git a/gdb/arch/aarch64.c b/gdb/arch/aarch64.c
> index dff2bc16003..622138f43b5 100644
> --- a/gdb/arch/aarch64.c
> +++ b/gdb/arch/aarch64.c
> @@ -20,6 +20,7 @@
>   
>   #include "../features/aarch64-core.c"
>   #include "../features/aarch64-fpu.c"
> +#include "../features/aarch64-fpmr.c"
>   #include "../features/aarch64-sve.c"
>   #include "../features/aarch64-pauth.c"
>   #include "../features/aarch64-mte.c"
> @@ -73,6 +74,9 @@ aarch64_create_target_description (const aarch64_features &features)
>     if (features.gcs_linux)
>       regnum = create_feature_aarch64_gcs_linux (tdesc.get (), regnum);
>   
> +  if (features.fpmr)
> +    regnum = create_feature_aarch64_fpmr (tdesc.get (), regnum);
> +
>     return tdesc.release ();
>   }
>   
> diff --git a/gdb/arch/aarch64.h b/gdb/arch/aarch64.h
> index 679d845df74..8f4ba9c9e0c 100644
> --- a/gdb/arch/aarch64.h
> +++ b/gdb/arch/aarch64.h
> @@ -34,6 +34,7 @@ struct aarch64_features
>     uint64_t vq = 0;
>     bool pauth = false;
>     bool mte = false;
> +  bool fpmr = false;
>   
>     /* A positive TLS value indicates the number of TLS registers available.  */
>     uint8_t tls = 0;
> @@ -68,7 +69,8 @@ inline bool operator==(const aarch64_features &lhs, const aarch64_features &rhs)
>       && lhs.svq == rhs.svq
>       && lhs.sme2 == rhs.sme2
>       && lhs.gcs == rhs.gcs
> -    && lhs.gcs_linux == rhs.gcs_linux;
> +    && lhs.gcs_linux == rhs.gcs_linux
> +    && lhs.fpmr == rhs.fpmr;
>   }
>   
>   namespace std
> @@ -94,6 +96,9 @@ namespace std
>   
>         /* SME2 feature.  */
>         h = h << 1 | features.sme2;
> +
> +      /* FPMR feature.  */
> +      h = h << 1 | features.fpmr;
>         return h;
>       }
>     };
> @@ -238,4 +243,9 @@ enum aarch64_regnum
>   /* Size of the SME2 ZT0 register in bytes.  */
>   #define AARCH64_SME2_ZT0_SIZE 64
>   
> +/* Feature check for Floating Point Mode Register.  */
> +#ifndef HWCAP2_FPMR
> +#define HWCAP2_FPMR (1ULL << 48)
> +#endif /* HWCAP2_FPMR */
> +
>   #endif /* GDB_ARCH_AARCH64_H */
> diff --git a/gdb/features/Makefile b/gdb/features/Makefile
> index d17c349b6cf..ed1b8bf119c 100644
> --- a/gdb/features/Makefile
> +++ b/gdb/features/Makefile
> @@ -201,6 +201,7 @@ $(outdir)/%.dat: %.xml number-regs.xsl sort-regs.xsl gdbserver-regs.xsl
>   # For targets with feature based target descriptions,
>   # the set of xml files we'll generate .c files for GDB from.
>   FEATURE_XMLFILES = aarch64-core.xml \
> +	aarch64-fpmr.xml \
>   	aarch64-fpu.xml \
>   	aarch64-pauth.xml \
>   	aarch64-mte.xml \
> diff --git a/gdb/features/aarch64-fpmr.c b/gdb/features/aarch64-fpmr.c
> new file mode 100644
> index 00000000000..a372b12530b
> --- /dev/null
> +++ b/gdb/features/aarch64-fpmr.c
> @@ -0,0 +1,44 @@
> +/* THIS FILE IS GENERATED.  -*- buffer-read-only: t -*- vi:set ro:
> +  Original: aarch64-fpmr.xml */
> +
> +#include "gdbsupport/tdesc.h"
> +
> +static int
> +create_feature_aarch64_fpmr (struct target_desc *result, long regnum)
> +{
> +  struct tdesc_feature *feature;
> +
> +  feature = tdesc_create_feature (result, "org.gnu.gdb.aarch64.fpmr");
> +  tdesc_type_with_fields *type_with_fields;
> +  type_with_fields = tdesc_create_enum (feature, "fp8_fmt", 3);
> +  tdesc_add_enum_value (type_with_fields, 0, "E5M2");
> +  tdesc_add_enum_value (type_with_fields, 1, "E4M3");
> +
> +  type_with_fields = tdesc_create_enum (feature, "osc", 1);
> +  tdesc_add_enum_value (type_with_fields, 0, "Inf/NaN");
> +  tdesc_add_enum_value (type_with_fields, 1, "MaxNormal");
> +
> +  type_with_fields = tdesc_create_enum (feature, "osm", 1);
> +  tdesc_add_enum_value (type_with_fields, 0, "Inf");
> +  tdesc_add_enum_value (type_with_fields, 1, "MaxNormal");
> +
> +  type_with_fields = tdesc_create_flags (feature, "fpmr_flags", 8);
> +  tdesc_type *field_type;
> +  field_type = tdesc_named_type (feature, "fp8_fmt");
> +  tdesc_add_typed_bitfield (type_with_fields, "F8S1", 0, 2, field_type);
> +  field_type = tdesc_named_type (feature, "fp8_fmt");
> +  tdesc_add_typed_bitfield (type_with_fields, "F8S2", 3, 5, field_type);
> +  field_type = tdesc_named_type (feature, "fp8_fmt");
> +  tdesc_add_typed_bitfield (type_with_fields, "F8D", 6, 8, field_type);
> +  field_type = tdesc_named_type (feature, "osm");
> +  tdesc_add_typed_bitfield (type_with_fields, "OSM", 14, 14, field_type);
> +  field_type = tdesc_named_type (feature, "osc");
> +  tdesc_add_typed_bitfield (type_with_fields, "OSC", 15, 15, field_type);
> +  tdesc_add_bitfield (type_with_fields, "LSCALE", 16, 22);
> +  field_type = tdesc_named_type (feature, "int8");
> +  tdesc_add_typed_bitfield (type_with_fields, "NSCALE", 24, 31, field_type);
> +  tdesc_add_bitfield (type_with_fields, "LSCALE2", 32, 37);
> +
> +  tdesc_create_reg (feature, "fpmr", regnum++, 1, NULL, 64, "fpmr_flags");
> +  return regnum;
> +}
> diff --git a/gdb/features/aarch64-fpmr.xml b/gdb/features/aarch64-fpmr.xml
> new file mode 100644
> index 00000000000..8be2eec58bb
> --- /dev/null
> +++ b/gdb/features/aarch64-fpmr.xml
> @@ -0,0 +1,53 @@
> +<?xml version="1.0"?>
> +<!-- Copyright (C) 2025 Free Software Foundation, Inc.
> +
> +     Copying and distribution of this file, with or without modification,
> +     are permitted in any medium without royalty provided the copyright
> +     notice and this notice are preserved.  -->
> +
> +<!DOCTYPE feature SYSTEM "gdb-target.dtd">
> +<feature name="org.gnu.gdb.aarch64.fpmr">
> +    <!-- FP8 format for F8S1, F8S2 and F8D fields. This is either E5M2 or E4M3.  -->

Formatting. Two spaces after period. Please check the rest of the patch 
for these.

> +  <enum id="fp8_fmt" size="3">
> +    <evalue name="E5M2" value="0"/>
> +    <evalue name="E4M3" value="1"/>
> +  </enum>
> +
> +  <!-- Overflow saturation for FP8 convert instructions. Specifies the result
> +  when a floating-point Overflow exception is detected.  -->

s/Overflow/overflow?

> +  <enum id="osc" size="1">
> +    <!-- Infinity or NaN is generated. -->
> +    <evalue name="Inf/NaN" value="0"/>
> +    <!-- Maximum normal number is generated. -->
> +    <evalue name="MaxNormal" value="1"/>
> +  </enum>
> +
> +  <!-- Overflow saturation for FP8 multiplication instructions. Specifies the
> +  result when a floating-point Overflow exception is detected.  -->

s/Overflow/overflow?

> +  <enum id="osm" size="1">
> +    <!-- Infinity generated. -->
> +    <evalue name="Inf" value="0"/>
> +    <!-- Maximum normal number is generated. -->
> +    <evalue name="MaxNormal" value="1"/>
> +  </enum>
> +
> +  <flags id="fpmr_flags" size="8">
> +    <!-- SRC1 Format.  -->
> +    <field name="F8S1" start="0" end="2" type="fp8_fmt"/>
> +    <!-- SRC2 Format.  -->
> +    <field name="F8S2" start="3" end="5" type="fp8_fmt"/>
> +    <!-- F8D Format.  -->
> +    <field name="F8D" start="6" end="8" type="fp8_fmt"/>
> +    <!-- OSM. -->
> +    <field name="OSM" start="14" end="14" type="osm"/>
> +    <!-- OSC. -->
> +    <field name="OSC" start="15" end="15" type="osc"/>
> +    <!-- LSCALE.  -->
> +    <field name="LSCALE" start="16" end="22"/>
> +    <!-- NSCALE.  -->
> +    <field name="NSCALE" start="24" end="31" type="int8"/>
> +    <!-- LSCALE2.  -->
> +    <field name="LSCALE2" start="32" end="37"/>
> +  </flags>
> +  <reg name="fpmr" bitsize="64" type="fpmr_flags"/>
> +</feature>

Otherwise this looks ok with these nits fixed.

Reviewed-By: Luis Machado <luis.machado.foss@gmail.com>