Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Luis Machado via Gdb-patches <gdb-patches@sourceware.org>
To: John Baldwin <jhb@FreeBSD.org>, gdb-patches@sourceware.org
Subject: Re: [PATCH 08/12] Add an aarch64-tls feature which includes the tpidr register.
Date: Tue, 3 May 2022 22:34:07 +0100	[thread overview]
Message-ID: <49f9bf8e-f7e0-2f67-a8cc-12b7393584de@arm.com> (raw)
In-Reply-To: <2d8c9fa6-4bf4-58dc-3b40-a31b15060253@FreeBSD.org>

On 5/3/22 22:30, John Baldwin wrote:
> On 5/3/22 2:14 PM, Luis Machado wrote:
>> On 3/23/22 21:00, John Baldwin wrote:
>>> ---
>>>    gdb/aarch64-linux-nat.c          |  3 ++-
>>>    gdb/aarch64-linux-tdep.c         |  2 +-
>>>    gdb/aarch64-tdep.c               | 42 
>>> ++++++++++++++++++++++++++------
>>>    gdb/aarch64-tdep.h               | 10 +++++++-
>>>    gdb/arch/aarch64.c               |  7 +++++-
>>>    gdb/arch/aarch64.h               |  9 +++++--
>>>    gdb/features/Makefile            |  1 +
>>>    gdb/features/aarch64-tls.c       | 14 +++++++++++
>>>    gdb/features/aarch64-tls.xml     | 12 +++++++++
>>>    gdbserver/linux-aarch64-tdesc.cc |  2 +-
>>>    gdbserver/netbsd-aarch64-low.cc  |  2 +-
>>>    11 files changed, 88 insertions(+), 16 deletions(-)
>>>    create mode 100644 gdb/features/aarch64-tls.c
>>>    create mode 100644 gdb/features/aarch64-tls.xml
>>>
>>> diff --git a/gdb/aarch64-linux-nat.c b/gdb/aarch64-linux-nat.c
>>> index 7bb82d17cc8..4da274c285a 100644
>>> --- a/gdb/aarch64-linux-nat.c
>>> +++ b/gdb/aarch64-linux-nat.c
>>> @@ -646,7 +646,8 @@ aarch64_linux_nat_target::read_description ()
>>>      bool pauth_p = hwcap & AARCH64_HWCAP_PACA;
>>>      bool mte_p = hwcap2 & HWCAP2_MTE;
>>> -  return aarch64_read_description (aarch64_sve_get_vq (tid), 
>>> pauth_p, mte_p);
>>> +  return aarch64_read_description (aarch64_sve_get_vq (tid), 
>>> pauth_p, mte_p,
>>> +                   false);
>>>    }
>>>    /* Convert a native/host siginfo object, into/from the siginfo in the
>>> diff --git a/gdb/aarch64-linux-tdep.c b/gdb/aarch64-linux-tdep.c
>>> index cb132d5a540..f5aac7bc0b4 100644
>>> --- a/gdb/aarch64-linux-tdep.c
>>> +++ b/gdb/aarch64-linux-tdep.c
>>> @@ -763,7 +763,7 @@ aarch64_linux_core_read_description (struct 
>>> gdbarch *gdbarch,
>>>      bool pauth_p = hwcap & AARCH64_HWCAP_PACA;
>>>      bool mte_p = hwcap2 & HWCAP2_MTE;
>>>      return aarch64_read_description (aarch64_linux_core_read_vq 
>>> (gdbarch, abfd),
>>> -                   pauth_p, mte_p);
>>> +                   pauth_p, mte_p, false);
>>>    }
>>>    /* Implementation of `gdbarch_stap_is_single_operand', as defined in
>>> diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
>>> index b714f6194b6..38c5c9e0e00 100644
>>> --- a/gdb/aarch64-tdep.c
>>> +++ b/gdb/aarch64-tdep.c
>>> @@ -58,7 +58,7 @@
>>>    #define HA_MAX_NUM_FLDS        4
>>>    /* All possible aarch64 target descriptors.  */
>>> -static target_desc *tdesc_aarch64_list[AARCH64_MAX_SVE_VQ + 
>>> 1][2/*pauth*/][2 /* mte */];
>>> +static target_desc *tdesc_aarch64_list[AARCH64_MAX_SVE_VQ + 
>>> 1][2/*pauth*/][2 /* mte */][2 /* tls */];
>>>    /* The standard register names, and all the valid aliases for 
>>> them.  */
>>>    static const struct
>>> @@ -178,6 +178,12 @@ static const char *const 
>>> aarch64_mte_register_names[] =
>>>      "tag_ctl"
>>>    };
>>> +static const char *const aarch64_tls_register_names[] =
>>> +{
>>> +  /* Thread/Process ID Register.  */
>>> +  "tpidr"
>>> +};
>>> +
>>>    /* AArch64 prologue cache structure.  */
>>>    struct aarch64_prologue_cache
>>>    {
>>> @@ -3327,21 +3333,23 @@ aarch64_displaced_step_hw_singlestep (struct 
>>> gdbarch *gdbarch)
>>>       If VQ is zero then it is assumed SVE is not supported.
>>>       (It is not possible to set VQ to zero on an SVE system).
>>> -   MTE_P indicates the presence of the Memory Tagging Extension 
>>> feature. */
>>> +   MTE_P indicates the presence of the Memory Tagging Extension 
>>> feature.
>>> +
>>> +   TLS_P indicates the presence of the Thread Local Storage 
>>> feature.  */
>>>    const target_desc *
>>> -aarch64_read_description (uint64_t vq, bool pauth_p, bool mte_p)
>>> +aarch64_read_description (uint64_t vq, bool pauth_p, bool mte_p, 
>>> bool tls_p)
>>>    {
>>>      if (vq > AARCH64_MAX_SVE_VQ)
>>>        error (_("VQ is %" PRIu64 ", maximum supported value is %d"), vq,
>>>           AARCH64_MAX_SVE_VQ);
>>> -  struct target_desc *tdesc = tdesc_aarch64_list[vq][pauth_p][mte_p];
>>> +  struct target_desc *tdesc = 
>>> tdesc_aarch64_list[vq][pauth_p][mte_p][tls_p];
>>>      if (tdesc == NULL)
>>>        {
>>> -      tdesc = aarch64_create_target_description (vq, pauth_p, mte_p);
>>> -      tdesc_aarch64_list[vq][pauth_p][mte_p] = tdesc;
>>> +      tdesc = aarch64_create_target_description (vq, pauth_p, mte_p, 
>>> tls_p);
>>> +      tdesc_aarch64_list[vq][pauth_p][mte_p][tls_p] = tdesc;
>>>        }
>>>      return tdesc;
>>> @@ -3430,7 +3438,7 @@ aarch64_gdbarch_init (struct gdbarch_info info, 
>>> struct gdbarch_list *arches)
>>>      bool valid_p = true;
>>>      int i, num_regs = 0, num_pseudo_regs = 0;
>>>      int first_pauth_regnum = -1, pauth_ra_state_offset = -1;
>>> -  int first_mte_regnum = -1;
>>> +  int first_mte_regnum = -1, first_tls_regnum = -1;
>>>      /* Use the vector length passed via the target info.  Here -1 is 
>>> used for no
>>>         SVE, and 0 is unset.  If unset then use the vector length 
>>> from the existing
>>> @@ -3462,7 +3470,7 @@ aarch64_gdbarch_init (struct gdbarch_info info, 
>>> struct gdbarch_list *arches)
>>>         value.  */
>>>      const struct target_desc *tdesc = info.target_desc;
>>>      if (!tdesc_has_registers (tdesc) || vq != aarch64_get_tdesc_vq 
>>> (tdesc))
>>> -    tdesc = aarch64_read_description (vq, false, false);
>>> +    tdesc = aarch64_read_description (vq, false, false, false);
>>>      gdb_assert (tdesc);
>>>      feature_core = tdesc_find_feature 
>>> (tdesc,"org.gnu.gdb.aarch64.core");
>>> @@ -3471,6 +3479,8 @@ aarch64_gdbarch_init (struct gdbarch_info info, 
>>> struct gdbarch_list *arches)
>>>      feature_pauth = tdesc_find_feature (tdesc, 
>>> "org.gnu.gdb.aarch64.pauth");
>>>      const struct tdesc_feature *feature_mte
>>>        = tdesc_find_feature (tdesc, "org.gnu.gdb.aarch64.mte");
>>> +  const struct tdesc_feature *feature_tls
>>> +    = tdesc_find_feature (tdesc, "org.gnu.gdb.aarch64.tls");
>>>      if (feature_core == nullptr)
>>>        return nullptr;
>>> @@ -3555,6 +3565,21 @@ aarch64_gdbarch_init (struct gdbarch_info 
>>> info, struct gdbarch_list *arches)
>>>          num_regs += i;
>>>        }
>>> +  /* Add the TLS register.  */
>>> +  if (feature_tls != nullptr)
>>> +    {
>>> +      first_tls_regnum = num_regs;
>>> +
>>> +      /* Validate the descriptor provides the mandatory MTE 
>>> registers and
>>> +     allocate their numbers.  */
>>> +      for (i = 0; i < ARRAY_SIZE (aarch64_tls_register_names); i++)
>>> +    valid_p &= tdesc_numbered_register (feature_tls, tdesc_data.get (),
>>> +                        first_tls_regnum + i,
>>> +                        aarch64_tls_register_names[i]);
>>> +
>>> +      num_regs += i;
>>> +    }
>>> +
>>>      if (!valid_p)
>>>        return nullptr;
>>> @@ -3573,6 +3598,7 @@ aarch64_gdbarch_init (struct gdbarch_info info, 
>>> struct gdbarch_list *arches)
>>>      tdep->pauth_ra_state_regnum = (feature_pauth == NULL) ? -1
>>>                    : pauth_ra_state_offset + num_regs;
>>>      tdep->mte_reg_base = first_mte_regnum;
>>> +  tdep->tls_reg_base = first_tls_regnum;
>>>      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 60a9d5a29c2..092586d4ee3 100644
>>> --- a/gdb/aarch64-tdep.h
>>> +++ b/gdb/aarch64-tdep.h
>>> @@ -111,10 +111,18 @@ struct aarch64_gdbarch_tdep : gdbarch_tdep
>>>      {
>>>        return mte_reg_base != -1;
>>>      }
>>> +
>>> +  /* TLS register.  This is -1 if the TLS register is not 
>>> available.  */
>>> +  int tls_reg_base = 0;
>>> +
>>> +  bool has_tls() const
>>> +  {
>>> +    return tls_reg_base != -1;
>>> +  }
>>>    };
>>>    const target_desc *aarch64_read_description (uint64_t vq, bool 
>>> pauth_p,
>>> -                         bool mte_p);
>>> +                         bool mte_p, bool tls_p);
>>>    extern int aarch64_process_record (struct gdbarch *gdbarch,
>>>                       struct regcache *regcache, CORE_ADDR addr);
>>> diff --git a/gdb/arch/aarch64.c b/gdb/arch/aarch64.c
>>> index 485d667ccde..733a3fd6d2a 100644
>>> --- a/gdb/arch/aarch64.c
>>> +++ b/gdb/arch/aarch64.c
>>> @@ -24,11 +24,13 @@
>>>    #include "../features/aarch64-sve.c"
>>>    #include "../features/aarch64-pauth.c"
>>>    #include "../features/aarch64-mte.c"
>>> +#include "../features/aarch64-tls.c"
>>>    /* See arch/aarch64.h.  */
>>>    target_desc *
>>> -aarch64_create_target_description (uint64_t vq, bool pauth_p, bool 
>>> mte_p)
>>> +aarch64_create_target_description (uint64_t vq, bool pauth_p, bool 
>>> mte_p,
>>> +                   bool tls_p)
>>>    {
>>>      target_desc_up tdesc = allocate_target_description ();
>>> @@ -52,5 +54,8 @@ aarch64_create_target_description (uint64_t vq, 
>>> bool pauth_p, bool mte_p)
>>>      if (mte_p)
>>>        regnum = create_feature_aarch64_mte (tdesc.get (), regnum);
>>> +  if (tls_p)
>>> +    regnum = create_feature_aarch64_tls (tdesc.get (), regnum);
>>> +
>>>      return tdesc.release ();
>>>    }
>>> diff --git a/gdb/arch/aarch64.h b/gdb/arch/aarch64.h
>>> index e416e346e9a..79821666ce6 100644
>>> --- a/gdb/arch/aarch64.h
>>> +++ b/gdb/arch/aarch64.h
>>> @@ -29,6 +29,7 @@ struct aarch64_features
>>>      bool sve = false;
>>>      bool pauth = false;
>>>      bool mte = false;
>>> +  bool tls = false;
>>>    };
>>>    /* Create the aarch64 target description.  A non zero VQ value 
>>> indicates both
>>> @@ -36,10 +37,12 @@ struct aarch64_features
>>>       an SVE Z register.  HAS_PAUTH_P indicates the presence of the 
>>> PAUTH
>>>       feature.
>>> -   MTE_P indicates the presence of the Memory Tagging Extension 
>>> feature.  */
>>> +   MTE_P indicates the presence of the Memory Tagging Extension 
>>> feature.
>>> +
>>> +   TLS_P indicates the presence of the Thread Local Storage 
>>> feature.  */
>>>    target_desc *aarch64_create_target_description (uint64_t vq, bool 
>>> has_pauth_p,
>>> -                        bool mte_p);
>>> +                        bool mte_p, bool tls_p);
>>>    /* Register numbers of various important registers.
>>>       Note that on SVE, the Z registers reuse the V register numbers 
>>> and the V
>>> @@ -84,6 +87,8 @@ enum aarch64_regnum
>>>    #define AARCH64_PAUTH_CMASK_REGNUM(pauth_reg_base) (pauth_reg_base 
>>> + 1)
>>>    #define AARCH64_PAUTH_REGS_SIZE (16)
>>> +#define    AARCH64_TPIDR_REGNUM(tls_reg_base) (tls_reg_base)
>>> +
>>>    #define AARCH64_X_REGS_NUM 31
>>>    #define AARCH64_V_REGS_NUM 32
>>>    #define AARCH64_SVE_Z_REGS_NUM AARCH64_V_REGS_NUM
>>> diff --git a/gdb/features/Makefile b/gdb/features/Makefile
>>> index 4b09819389a..946ec983df5 100644
>>> --- a/gdb/features/Makefile
>>> +++ b/gdb/features/Makefile
>>> @@ -198,6 +198,7 @@ FEATURE_XMLFILES = aarch64-core.xml \
>>>        aarch64-fpu.xml \
>>>        aarch64-pauth.xml \
>>>        aarch64-mte.xml \
>>> +    aarch64-tls.xml \
>>>        arc/v1-core.xml \
>>>        arc/v1-aux.xml \
>>>        arc/v2-core.xml \
>>> diff --git a/gdb/features/aarch64-tls.c b/gdb/features/aarch64-tls.c
>>> new file mode 100644
>>> index 00000000000..30d730dffba
>>> --- /dev/null
>>> +++ b/gdb/features/aarch64-tls.c
>>> @@ -0,0 +1,14 @@
>>> +/* THIS FILE IS GENERATED.  -*- buffer-read-only: t -*- vi:set ro:
>>> +  Original: aarch64-tls.xml */
>>> +
>>> +#include "gdbsupport/tdesc.h"
>>> +
>>> +static int
>>> +create_feature_aarch64_tls (struct target_desc *result, long regnum)
>>> +{
>>> +  struct tdesc_feature *feature;
>>> +
>>> +  feature = tdesc_create_feature (result, "org.gnu.gdb.aarch64.tls");
>>> +  tdesc_create_reg (feature, "tpidr", regnum++, 1, NULL, 64, 
>>> "data_ptr");
>>> +  return regnum;
>>> +}
>>> diff --git a/gdb/features/aarch64-tls.xml b/gdb/features/aarch64-tls.xml
>>> new file mode 100644
>>> index 00000000000..48e0e9a6030
>>> --- /dev/null
>>> +++ b/gdb/features/aarch64-tls.xml
>>> @@ -0,0 +1,12 @@
>>> +<?xml version="1.0"?>
>>> +<!-- Copyright (C) 2009-2022 Free Software Foundation, Inc.
>>> +     Contributed by ARM Ltd.
>>> +
>>> +     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.tls">
>>> +  <reg name="tpidr" bitsize="64" type="data_ptr"/>
>>> +</feature>
>>> diff --git a/gdbserver/linux-aarch64-tdesc.cc 
>>> b/gdbserver/linux-aarch64-tdesc.cc
>>> index e982ab85067..14d6a4f80eb 100644
>>> --- a/gdbserver/linux-aarch64-tdesc.cc
>>> +++ b/gdbserver/linux-aarch64-tdesc.cc
>>> @@ -42,7 +42,7 @@ aarch64_linux_read_description (uint64_t vq, bool 
>>> pauth_p, bool mte_p)
>>>      if (tdesc == NULL)
>>>        {
>>> -      tdesc = aarch64_create_target_description (vq, pauth_p, mte_p);
>>> +      tdesc = aarch64_create_target_description (vq, pauth_p, mte_p, 
>>> false);
>>>          static const char *expedite_regs_aarch64[] = { "x29", "sp", 
>>> "pc", NULL };
>>>          static const char *expedite_regs_aarch64_sve[] = { "x29", 
>>> "sp", "pc",
>>> diff --git a/gdbserver/netbsd-aarch64-low.cc 
>>> b/gdbserver/netbsd-aarch64-low.cc
>>> index 202bf1cdac6..b371e599232 100644
>>> --- a/gdbserver/netbsd-aarch64-low.cc
>>> +++ b/gdbserver/netbsd-aarch64-low.cc
>>> @@ -96,7 +96,7 @@ void
>>>    netbsd_aarch64_target::low_arch_setup ()
>>>    {
>>>      target_desc *tdesc
>>> -    = aarch64_create_target_description (0, false);
>>> +    = aarch64_create_target_description (0, false, false, false);
>>>      static const char *expedite_regs_aarch64[] = { "x29", "sp", 
>>> "pc", NULL };
>>>      init_target_desc (tdesc, expedite_regs_aarch64);
>>
>> LGTM from aarch64's side.
> 
> Hmm, did you mean to send this in response to the V3 series posted last 
> week?
> 

Indeed. I went through v3 before, but when I went to reply to it, my 
filters had sent it to a separate folder. So yeah, this was for v3.

  reply	other threads:[~2022-05-03 21:34 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-23 21:00 [PATCH 00/12] * Support for Thread Local Storage (TLS) variables on FreeBSD arm and aarch64 architectures John Baldwin
2022-03-23 21:00 ` [PATCH 01/12] Handle another edge case for TLS variable lookups John Baldwin
2022-03-23 23:55   ` John Baldwin
2022-03-24  0:26     ` John Baldwin
2022-03-23 21:00 ` [PATCH 02/12] fbsd-nat: Add helper routines for register sets using PT_[G]SETREGSET John Baldwin
2022-03-24  8:51   ` Luis Machado via Gdb-patches
2022-03-24 17:45     ` John Baldwin
2022-03-23 21:00 ` [PATCH 03/12] Create pseudo sections for NT_ARM_TLS notes on FreeBSD John Baldwin
2022-03-23 21:00 ` [PATCH 04/12] Add an arm-tls feature which includes the tpidruro register from CP15 John Baldwin
2022-04-04  8:01   ` Luis Machado via Gdb-patches
2022-04-12 23:36     ` John Baldwin
2022-04-14 10:23       ` Luis Machado via Gdb-patches
2022-04-19 16:18         ` John Baldwin
2022-04-20  6:59           ` Luis Machado via Gdb-patches
2022-03-23 21:00 ` [PATCH 05/12] Read the tpidruro register from NT_ARM_TLS core dump notes on FreeBSD/arm John Baldwin
2022-03-23 21:00 ` [PATCH 06/12] Support TLS variables " John Baldwin
2022-03-23 21:00 ` [PATCH 07/12] Fetch the NT_ARM_TLS register set for native FreeBSD/arm processes John Baldwin
2022-03-23 21:00 ` [PATCH 08/12] Add an aarch64-tls feature which includes the tpidr register John Baldwin
2022-03-28 10:16   ` Luis Machado via Gdb-patches
2022-04-01 23:30     ` John Baldwin
2022-04-04  8:06       ` Luis Machado via Gdb-patches
2022-04-04 12:18         ` Luis Machado via Gdb-patches
2022-05-03 21:14   ` Luis Machado via Gdb-patches
2022-05-03 21:30     ` John Baldwin
2022-05-03 21:34       ` Luis Machado via Gdb-patches [this message]
2022-03-23 21:00 ` [PATCH 09/12] Read the tpidr register from NT_ARM_TLS core dump notes on FreeBSD/Aarch64 John Baldwin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=49f9bf8e-f7e0-2f67-a8cc-12b7393584de@arm.com \
    --to=gdb-patches@sourceware.org \
    --cc=jhb@FreeBSD.org \
    --cc=luis.machado@arm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox