Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Kamil Rytarowski <n54@gmx.com>
To: Christian Biesinger <cbiesinger@google.com>
Cc: Alan Hayward <Alan.Hayward@arm.com>,
	gdb-patches <gdb-patches@sourceware.org>, nd <nd@arm.com>
Subject: Re: [PATCH] Fix arm-netbsd build error
Date: Thu, 13 Feb 2020 00:01:00 -0000	[thread overview]
Message-ID: <6227c001-8ae8-cb5e-f987-fc01499e5ba7@gmx.com> (raw)
In-Reply-To: <CAPTJ0XF3bu+9S0uuRTjXgWw7kZcBjTYsNwOn++EFdy=heGyccQ@mail.gmail.com>


[-- Attachment #1.1: Type: text/plain, Size: 11366 bytes --]

On 13.02.2020 00:42, Christian Biesinger via gdb-patches wrote:
> And one more question -- since VFP 1 is deprecated (not even mentioned
> on http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0472j/chr1359124231926.html)
> and your example codes don't include VFP 2, should I assume that
> fpu_present implies VFP 3 and that VFPv3-D16 doesn't happen?
> 

In theory we support what was mentioned in
http://netbsd.org/~kamil/arm-fpu.c

The kernel VFP attach function is located here:
https://nxr.netbsd.org/xref/src/sys/arch/arm/vfp/vfp_init.c#250

Today it is safe to assume VFPv3. As far as I am aware, VFPv3-D16 is
unsupported.

> Thanks again,
> Christian
> 
> On Wed, Feb 12, 2020 at 5:28 PM Christian Biesinger
> <cbiesinger@google.com> wrote:
>>
>> Thanks, Kamil!
>>
>> Please forgive the stupid question, but is SIMD in this context the
>> same as IWMMXT?
>>

iwmmxt is unsupported.

>> Christian
>>
>> On Wed, Feb 12, 2020 at 11:16 AM Kamil Rytarowski <n54@gmx.com> wrote:
>>>
>>> On 12.02.2020 18:09, Kamil Rytarowski wrote:
>>>> On 12.02.2020 17:28, Christian Biesinger wrote:
>>>>> On Wed, Feb 12, 2020 at 7:29 AM Alan Hayward <Alan.Hayward@arm.com> wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>>> On 11 Feb 2020, at 23:34, Christian Biesinger via gdb-patches <gdb-patches@sourceware.org> wrote:
>>>>>>>
>>>>>>> On Tue, Feb 11, 2020 at 4:55 PM Christian Biesinger
>>>>>>> <cbiesinger@google.com> wrote:
>>>>>>>>
>>>>>>>> The floating point register interface has changed to this:
>>>>>>>> https://github.com/NetBSD/src/blob/trunk/sys/arch/arm/include/reg.h
>>>>>>>>
>>>>>>>> It now uses VFP instead of FPA registers. This patch updates
>>>>>>>> arm-nbsd-nat.c accordingly.
>>>>>>>>
>>>>>>>> Tested by compiling on arm-netbsd on qemu. For actually testing, there
>>>>>>>> seems to be something missing as "info registers" only shows FPA
>>>>>>>> registers and no VFP ones. I am still investigating why this is;
>>>>>>>> please let me know if you know. However, I think this is still good
>>>>>>>> to check in as-is.
>>>>>>>
>>>>>>> Hm... this is perhaps because arm_netbsd_nat_target does not implement
>>>>>>> read_description; if it returned arm_read_description
>>>>>>> (ARM_FP_TYPE_VFPV2) this may work?
>>>>>>>
>>>>>>
>>>>>> Yes, looks like netbsd isn’t using any target description functionality.
>>>>>>
>>>>>> I suspect the code is getting into arm_gdbarch_init() with a null tdesc,
>>>>>> and then using the AUTO setting. But that’s just a guess.
>>>>>>
>>>>>> Implementing read_description as you suggest should help. However,
>>>>>> read_description should probably do HWCAP checking similar to the
>>>>>> arm_linux_nat_target and arm_fbsd_nat_target versions.
>>>>>>
>>>>>> Without that, I’d worry that your patch below might start writing off the
>>>>>> end of the regcache that had been allocated for a fewer number of registers.
>>>>>
>>>>> Hm... well, I've probably spent entirely too much time on this
>>>>> already, but just in case there's an easy fix -- Kamil, does NetBSD
>>>>> provide an API similar to Linux's HWCAP API that would let me check
>>>>> which VFP version the current CPU is using? It seems AUXV does not
>>>>> contain HWCAP data on NetBSD, unlike Linux/FreeBSD, though I may be
>>>>> missing something.
>>>>>
>>>>> (Compare arm_fbsd_read_description_auxv /
>>>>> arm_linux_nat_target::read_description)
>>>>>
>>>>> Christian
>>>>>
>>>>
>>>> Thank you for your work.
>>>>
>>>> HWCAP in auxv is not supported (although there is a stub for it).
>>>>
>>>> Here is a complete algorithm to detect FPU on ARM 32-bit.
>>>>
>>>> http://netbsd.org/~kamil/arm-fpu.c
>>>>
>>>
>>> Here is another example with SIMD:
>>>
>>> http://cvsweb.netbsd.org/bsdweb.cgi/xsrc/external/mit/pixman/dist/pixman/pixman-arm.c.diff?r1=1.1&r2=1.2&f=h
>>>
>>>>>>
>>>>>>>>
>>>>>>>> gdb/ChangeLog:
>>>>>>>>
>>>>>>>> 2020-02-11  Christian Biesinger  <cbiesinger@google.com>
>>>>>>>>
>>>>>>>>        * arm-nbsd-nat.c (arm_supply_fparegset): Rename to...
>>>>>>>>        (arm_supply_vfpregset): ...this, and update to use VFP registers.
>>>>>>>>        (fetch_fp_register): Update.
>>>>>>>>        (fetch_fp_regs): Update.
>>>>>>>>        (store_fp_register): Update.
>>>>>>>>        (store_fp_regs): Update.
>>>>>>>>        (fetch_elfcore_registers): Update.
>>>>>>>> ---
>>>>>>>> gdb/arm-nbsd-nat.c | 80 ++++++++++++++++++++--------------------------
>>>>>>>> 1 file changed, 34 insertions(+), 46 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/gdb/arm-nbsd-nat.c b/gdb/arm-nbsd-nat.c
>>>>>>>> index 11afc289c3..8027f54dfe 100644
>>>>>>>> --- a/gdb/arm-nbsd-nat.c
>>>>>>>> +++ b/gdb/arm-nbsd-nat.c
>>>>>>>> @@ -65,15 +65,13 @@ arm_supply_gregset (struct regcache *regcache, struct reg *gregset)
>>>>>>>> }
>>>>>>>>
>>>>>>>> static void
>>>>>>>> -arm_supply_fparegset (struct regcache *regcache, struct fpreg *fparegset)
>>>>>>>> +arm_supply_vfpregset (struct regcache *regcache, struct fpreg *fpregset)
>>>>>>>> {
>>>>>>>> -  int regno;
>>>>>>>> -
>>>>>>>> -  for (regno = ARM_F0_REGNUM; regno <= ARM_F7_REGNUM; regno++)
>>>>>>>> -    regcache->raw_supply (regno,
>>>>>>>> -                         (char *) &fparegset->fpr[regno - ARM_F0_REGNUM]);
>>>>>>>> +  struct vfpreg &vfp = fpregset->fpr_vfp;
>>>>>>>> +  for (int regno = ARM_D0_REGNUM; regno <= ARM_D31_REGNUM; regno++)
>>>>>>>> +    regcache->raw_supply (regno, (char *) &vfp.vfp_regs[regno - ARM_D0_REGNUM]);
>>>>>>>>
>>>>>>>> -  regcache->raw_supply (ARM_FPS_REGNUM, (char *) &fparegset->fpr_fpsr);
>>>>>>>> +  regcache->raw_supply (ARM_FPSCR_REGNUM, (char *) &vfp.vfp_fpscr);
>>>>>>>> }
>>>>>>>>
>>>>>>>> static void
>>>>>>>> @@ -147,10 +145,10 @@ static void
>>>>>>>> fetch_fp_register (struct regcache *regcache, int regno)
>>>>>>>> {
>>>>>>>>   struct fpreg inferior_fp_registers;
>>>>>>>> -  int ret;
>>>>>>>> +  int ret = ptrace (PT_GETFPREGS, regcache->ptid ().pid (),
>>>>>>>> +                   (PTRACE_TYPE_ARG3) &inferior_fp_registers, 0);
>>>>>>>>
>>>>>>>> -  ret = ptrace (PT_GETFPREGS, regcache->ptid ().pid (),
>>>>>>>> -               (PTRACE_TYPE_ARG3) &inferior_fp_registers, 0);
>>>>>>>> +  struct vfpreg &vfp = inferior_fp_registers.fpr_vfp;
>>>>>>>>
>>>>>>>>   if (ret < 0)
>>>>>>>>     {
>>>>>>>> @@ -158,18 +156,15 @@ fetch_fp_register (struct regcache *regcache, int regno)
>>>>>>>>       return;
>>>>>>>>     }
>>>>>>>>
>>>>>>>> -  switch (regno)
>>>>>>>> +  if (regno == ARM_FPSCR_REGNUM)
>>>>>>>> +    regcache->raw_supply (ARM_FPSCR_REGNUM, (char *) &vfp.vfp_fpscr);
>>>>>>>> +  else if (regno >= ARM_D0_REGNUM && regno <= ARM_D31_REGNUM)
>>>>>>>>     {
>>>>>>>> -    case ARM_FPS_REGNUM:
>>>>>>>> -      regcache->raw_supply (ARM_FPS_REGNUM,
>>>>>>>> -                           (char *) &inferior_fp_registers.fpr_fpsr);
>>>>>>>> -      break;
>>>>>>>> -
>>>>>>>> -    default:
>>>>>>>> -      regcache->raw_supply
>>>>>>>> -       (regno, (char *) &inferior_fp_registers.fpr[regno - ARM_F0_REGNUM]);
>>>>>>>> -      break;
>>>>>>>> +      regcache->raw_supply (regno,
>>>>>>>> +                           (char *) &vfp.vfp_regs[regno - ARM_D0_REGNUM]);
>>>>>>>>     }
>>>>>>>> +  else
>>>>>>>> +    warning (_("Invalid register number."));
>>>>>>>> }
>>>>>>>>
>>>>>>>> static void
>>>>>>>> @@ -188,7 +183,7 @@ fetch_fp_regs (struct regcache *regcache)
>>>>>>>>       return;
>>>>>>>>     }
>>>>>>>>
>>>>>>>> -  arm_supply_fparegset (regcache, &inferior_fp_registers);
>>>>>>>> +  arm_supply_vfpregset (regcache, &inferior_fp_registers);
>>>>>>>> }
>>>>>>>>
>>>>>>>> void
>>>>>>>> @@ -327,10 +322,9 @@ static void
>>>>>>>> store_fp_register (const struct regcache *regcache, int regno)
>>>>>>>> {
>>>>>>>>   struct fpreg inferior_fp_registers;
>>>>>>>> -  int ret;
>>>>>>>> -
>>>>>>>> -  ret = ptrace (PT_GETFPREGS, regcache->ptid ().pid (),
>>>>>>>> -               (PTRACE_TYPE_ARG3) &inferior_fp_registers, 0);
>>>>>>>> +  int ret = ptrace (PT_GETFPREGS, regcache->ptid ().pid (),
>>>>>>>> +                   (PTRACE_TYPE_ARG3) &inferior_fp_registers, 0);
>>>>>>>> +  struct vfpreg &vfp = inferior_fp_registers.fpr_vfp;
>>>>>>>>
>>>>>>>>   if (ret < 0)
>>>>>>>>     {
>>>>>>>> @@ -338,18 +332,15 @@ store_fp_register (const struct regcache *regcache, int regno)
>>>>>>>>       return;
>>>>>>>>     }
>>>>>>>>
>>>>>>>> -  switch (regno)
>>>>>>>> +  if (regno == ARM_FPSCR_REGNUM)
>>>>>>>> +    regcache->raw_collect (ARM_FPSCR_REGNUM, (char *) &vfp.vfp_fpscr);
>>>>>>>> +  else if (regno >= ARM_D0_REGNUM && regno <= ARM_D31_REGNUM)
>>>>>>>>     {
>>>>>>>> -    case ARM_FPS_REGNUM:
>>>>>>>> -      regcache->raw_collect (ARM_FPS_REGNUM,
>>>>>>>> -                            (char *) &inferior_fp_registers.fpr_fpsr);
>>>>>>>> -      break;
>>>>>>>> -
>>>>>>>> -    default:
>>>>>>>> -      regcache->raw_collect
>>>>>>>> -       (regno, (char *) &inferior_fp_registers.fpr[regno - ARM_F0_REGNUM]);
>>>>>>>> -      break;
>>>>>>>> +      regcache->raw_collect (regno,
>>>>>>>> +                            (char *) &vfp.vfp_regs[regno - ARM_D0_REGNUM]);
>>>>>>>>     }
>>>>>>>> +  else
>>>>>>>> +    warning (_("Invalid register number."));
>>>>>>>>
>>>>>>>>   ret = ptrace (PT_SETFPREGS, regcache->ptid ().pid (),
>>>>>>>>                (PTRACE_TYPE_ARG3) &inferior_fp_registers, 0);
>>>>>>>> @@ -361,20 +352,17 @@ store_fp_register (const struct regcache *regcache, int regno)
>>>>>>>> static void
>>>>>>>> store_fp_regs (const struct regcache *regcache)
>>>>>>>> {
>>>>>>>> -  struct fpreg inferior_fp_registers;
>>>>>>>> -  int ret;
>>>>>>>> -  int regno;
>>>>>>>> +  struct fpreg fpregs;
>>>>>>>>
>>>>>>>> -
>>>>>>>> -  for (regno = ARM_F0_REGNUM; regno <= ARM_F7_REGNUM; regno++)
>>>>>>>> +  for (int regno = ARM_D0_REGNUM; regno <= ARM_D31_REGNUM; regno++)
>>>>>>>>     regcache->raw_collect
>>>>>>>> -      (regno, (char *) &inferior_fp_registers.fpr[regno - ARM_F0_REGNUM]);
>>>>>>>> +      (regno, (char *) &fpregs.fpr_vfp.vfp_regs[regno - ARM_D0_REGNUM]);
>>>>>>>>
>>>>>>>> -  regcache->raw_collect (ARM_FPS_REGNUM,
>>>>>>>> -                        (char *) &inferior_fp_registers.fpr_fpsr);
>>>>>>>> +  regcache->raw_collect (ARM_FPSCR_REGNUM,
>>>>>>>> +                        (char *) &fpregs.fpr_vfp.vfp_fpscr);
>>>>>>>>
>>>>>>>> -  ret = ptrace (PT_SETFPREGS, regcache->ptid ().pid (),
>>>>>>>> -               (PTRACE_TYPE_ARG3) &inferior_fp_registers, 0);
>>>>>>>> +  int ret = ptrace (PT_SETFPREGS, regcache->ptid ().pid (),
>>>>>>>> +                   (PTRACE_TYPE_ARG3) &fpregs, 0);
>>>>>>>>
>>>>>>>>   if (ret < 0)
>>>>>>>>     warning (_("unable to store floating-point registers"));
>>>>>>>> @@ -427,7 +415,7 @@ fetch_elfcore_registers (struct regcache *regcache,
>>>>>>>>          /* The memcpy may be unnecessary, but we can't really be sure
>>>>>>>>             of the alignment of the data in the core file.  */
>>>>>>>>          memcpy (&fparegset, core_reg_sect, sizeof (fparegset));
>>>>>>>> -         arm_supply_fparegset (regcache, &fparegset);
>>>>>>>> +         arm_supply_vfpregset (regcache, &fparegset);
>>>>>>>>        }
>>>>>>>>       break;
>>>>>>>>
>>>>>>>> --
>>>>>>>> 2.25.0.225.g125e21ebc7-goog
>>>>>>>>
>>>>>>
>>>>
>>>>
>>>
>>>



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

  reply	other threads:[~2020-02-13  0:01 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20200211225503.32992-1-cbiesinger@google.com>
2020-02-11 23:35 ` Christian Biesinger via gdb-patches
2020-02-12 13:30   ` Alan Hayward
2020-02-12 16:29     ` Christian Biesinger via gdb-patches
2020-02-12 17:10       ` Kamil Rytarowski
2020-02-12 17:17         ` Kamil Rytarowski
2020-02-12 23:28           ` Christian Biesinger via gdb-patches
2020-02-12 23:43             ` Christian Biesinger via gdb-patches
2020-02-13  0:01               ` Kamil Rytarowski [this message]
2020-02-28 13:18                 ` [PATCH v2] Fix arm-netbsd build error: convert from FPA to VFP Christian Biesinger via gdb-patches
2020-02-28 14:35                   ` Alan Hayward
2020-02-28 19:19                     ` [PATCH v3] " Christian Biesinger via gdb-patches
     [not found]                       ` <ECBA936B-BF56-4F49-9C57-35E0F29D560E@arm.com>
2020-03-02 17:29                         ` Christian Biesinger via gdb-patches
2020-03-02 17:32                           ` Kamil Rytarowski

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=6227c001-8ae8-cb5e-f987-fc01499e5ba7@gmx.com \
    --to=n54@gmx.com \
    --cc=Alan.Hayward@arm.com \
    --cc=cbiesinger@google.com \
    --cc=gdb-patches@sourceware.org \
    --cc=nd@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