public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] Re-map value of NT_RISCV_CSR to not collide with the value of NT_RISCV_VECTOR in Linux kernel header file 'include/uapi/linux/elf.h'
@ 2023-08-09 17:53 Greg Savin
  2023-08-10 18:39 ` John Baldwin
  0 siblings, 1 reply; 12+ messages in thread
From: Greg Savin @ 2023-08-09 17:53 UTC (permalink / raw)
  To: gdb-patches, binutils, palmer, greentime.hu; +Cc: Greg Savin

Linux kernel's file 'include/uapi/linux/elf.h' declares the value 0x900
for NT_RISCV_VECTOR, and does not have a definition for NT_RISCV_CSR, nor
does it use the value 0x901 for any note type.  This patch is intended
as a way to resolve the disagreement/collision between Linux and binutils,
over the meaning of 0x900 in the context of note types.

---
 include/elf/common.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/elf/common.h b/include/elf/common.h
index ffa6b60bd2b..0bbe245519e 100644
--- a/include/elf/common.h
+++ b/include/elf/common.h
@@ -713,7 +713,7 @@
 					/*   note name must be "LINUX".  */
 #define NT_LARCH_LBT    0xa04		/* LoongArch Binary Translation registers */
 					/*   note name must be "CORE".  */
-#define NT_RISCV_CSR    0x900		/* RISC-V Control and Status Registers */
+#define NT_RISCV_CSR    0x901		/* RISC-V Control and Status Registers */
 					/*   note name must be "LINUX".  */
 #define NT_SIGINFO	0x53494749	/* Fields of siginfo_t.  */
 #define NT_FILE		0x46494c45	/* Description of mapped files.  */
-- 
2.25.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Re-map value of NT_RISCV_CSR to not collide with the value of NT_RISCV_VECTOR in Linux kernel header file 'include/uapi/linux/elf.h'
  2023-08-09 17:53 [PATCH] Re-map value of NT_RISCV_CSR to not collide with the value of NT_RISCV_VECTOR in Linux kernel header file 'include/uapi/linux/elf.h' Greg Savin
@ 2023-08-10 18:39 ` John Baldwin
  2023-08-10 18:56   ` Greg Savin
  0 siblings, 1 reply; 12+ messages in thread
From: John Baldwin @ 2023-08-10 18:39 UTC (permalink / raw)
  To: Greg Savin, gdb-patches, binutils, palmer, greentime.hu, Andrew Burgess

On 8/9/23 10:53 AM, Greg Savin via Gdb-patches wrote:
> Linux kernel's file 'include/uapi/linux/elf.h' declares the value 0x900
> for NT_RISCV_VECTOR, and does not have a definition for NT_RISCV_CSR, nor
> does it use the value 0x901 for any note type.  This patch is intended
> as a way to resolve the disagreement/collision between Linux and binutils,
> over the meaning of 0x900 in the context of note types.
> 
> ---
>   include/elf/common.h | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/elf/common.h b/include/elf/common.h
> index ffa6b60bd2b..0bbe245519e 100644
> --- a/include/elf/common.h
> +++ b/include/elf/common.h
> @@ -713,7 +713,7 @@
>   					/*   note name must be "LINUX".  */
>   #define NT_LARCH_LBT    0xa04		/* LoongArch Binary Translation registers */
>   					/*   note name must be "CORE".  */
> -#define NT_RISCV_CSR    0x900		/* RISC-V Control and Status Registers */
> +#define NT_RISCV_CSR    0x901		/* RISC-V Control and Status Registers */
>   					/*   note name must be "LINUX".  */
>   #define NT_SIGINFO	0x53494749	/* Fields of siginfo_t.  */
>   #define NT_FILE		0x46494c45	/* Description of mapped files.  */

If there aren't any active uses of NT_RISCV_CSR "in the wild", then it might be
better to just remove it entirely?

It looks like commit e214f8db56f65531b0a5ec296c42339dcaa5af31 adding LoongArch
constants inadvertently changed the comment for this note from "CORE" (a
binutils note) to "LINUX" (a Linux kernel note).

The log message adding NT_RISCV_CSR is clearer (though the comment in common.h
is wrong and should be "GDB" it seems):

commit db6092f3aec43ea4d10efc5ff74274f04cdc0ad6
Author: Andrew Burgess <andrew.burgess@embecosm.com>
Date:   Fri Nov 27 14:04:16 2020 +0000

     bfd/binutils: add support for RISC-V CSRs in core files
     
     Adds support for including RISC-V control and status registers into
     core files.
     
     The value for the define NT_RISCV_CSR is set to 0x900, this
     corresponds to a patch I have proposed for the Linux kernel here:
     
       http://lists.infradead.org/pipermail/linux-riscv/2020-December/003910.html
     
     As I have not yet heard if the above patch will be accepted into the
     kernel or not I have set the note name string to "GDB", and the note
     type to NT_RISCV_CSR.
     
     This means that if the above patch is rejected from the kernel, and
     the note type number 0x900 is assigned to some other note type, we
     will still be able to distinguish between the GDB produced
     NT_RISCV_CSR, and the kernel produced notes, where the name would be
     set to "CORE".

Seems like the kernel patch died on the vine (no responses to the patch).
GDB does support writing this out still though via the ".reg.riscv-csr"
pseudo section, so changing this would break debugging bare metal core
dumps.  You can however, just re-use 0x900 for NT_RISCV_VECTOR without
this patch.  The different note name is sufficient to differentiate the
two.

-- 
John Baldwin


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Re-map value of NT_RISCV_CSR to not collide with the value of NT_RISCV_VECTOR in Linux kernel header file 'include/uapi/linux/elf.h'
  2023-08-10 18:39 ` John Baldwin
@ 2023-08-10 18:56   ` Greg Savin
  2023-08-10 20:00     ` John Baldwin
  0 siblings, 1 reply; 12+ messages in thread
From: Greg Savin @ 2023-08-10 18:56 UTC (permalink / raw)
  To: John Baldwin; +Cc: gdb-patches, binutils, palmer, greentime.hu, Andrew Burgess

[-- Attachment #1: Type: text/plain, Size: 3956 bytes --]

Initially I tried reusing 0x900 for NT_RISCV_VECTOR, and leaving
NT_RISCV_CSR in place with its original 0x900 value, and this was resulting
in compile errors of "duplicate case values within a switch statement."  So
I figured having duplicate NT_* values was a "no no" at least in the
standards of the GDB project.  I'm guessing that particular compile error
can be treated as a warning, but that GDB intentionally chooses compile
flags to treat the duplicate case values as a hard error (?)

--Greg

On Thu, Aug 10, 2023 at 11:39 AM John Baldwin <jhb@freebsd.org> wrote:

> On 8/9/23 10:53 AM, Greg Savin via Gdb-patches wrote:
> > Linux kernel's file 'include/uapi/linux/elf.h' declares the value 0x900
> > for NT_RISCV_VECTOR, and does not have a definition for NT_RISCV_CSR, nor
> > does it use the value 0x901 for any note type.  This patch is intended
> > as a way to resolve the disagreement/collision between Linux and
> binutils,
> > over the meaning of 0x900 in the context of note types.
> >
> > ---
> >   include/elf/common.h | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/include/elf/common.h b/include/elf/common.h
> > index ffa6b60bd2b..0bbe245519e 100644
> > --- a/include/elf/common.h
> > +++ b/include/elf/common.h
> > @@ -713,7 +713,7 @@
> >                                       /*   note name must be "LINUX".  */
> >   #define NT_LARCH_LBT    0xa04               /* LoongArch Binary
> Translation registers */
> >                                       /*   note name must be "CORE".  */
> > -#define NT_RISCV_CSR    0x900                /* RISC-V Control and
> Status Registers */
> > +#define NT_RISCV_CSR    0x901                /* RISC-V Control and
> Status Registers */
> >                                       /*   note name must be "LINUX".  */
> >   #define NT_SIGINFO  0x53494749      /* Fields of siginfo_t.  */
> >   #define NT_FILE             0x46494c45      /* Description of mapped
> files.  */
>
> If there aren't any active uses of NT_RISCV_CSR "in the wild", then it
> might be
> better to just remove it entirely?
>
> It looks like commit e214f8db56f65531b0a5ec296c42339dcaa5af31 adding
> LoongArch
> constants inadvertently changed the comment for this note from "CORE" (a
> binutils note) to "LINUX" (a Linux kernel note).
>
> The log message adding NT_RISCV_CSR is clearer (though the comment in
> common.h
> is wrong and should be "GDB" it seems):
>
> commit db6092f3aec43ea4d10efc5ff74274f04cdc0ad6
> Author: Andrew Burgess <andrew.burgess@embecosm.com>
> Date:   Fri Nov 27 14:04:16 2020 +0000
>
>      bfd/binutils: add support for RISC-V CSRs in core files
>
>      Adds support for including RISC-V control and status registers into
>      core files.
>
>      The value for the define NT_RISCV_CSR is set to 0x900, this
>      corresponds to a patch I have proposed for the Linux kernel here:
>
>
> http://lists.infradead.org/pipermail/linux-riscv/2020-December/003910.html
>
>      As I have not yet heard if the above patch will be accepted into the
>      kernel or not I have set the note name string to "GDB", and the note
>      type to NT_RISCV_CSR.
>
>      This means that if the above patch is rejected from the kernel, and
>      the note type number 0x900 is assigned to some other note type, we
>      will still be able to distinguish between the GDB produced
>      NT_RISCV_CSR, and the kernel produced notes, where the name would be
>      set to "CORE".
>
> Seems like the kernel patch died on the vine (no responses to the patch).
> GDB does support writing this out still though via the ".reg.riscv-csr"
> pseudo section, so changing this would break debugging bare metal core
> dumps.  You can however, just re-use 0x900 for NT_RISCV_VECTOR without
> this patch.  The different note name is sufficient to differentiate the
> two.
>
> --
> John Baldwin
>
>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Re-map value of NT_RISCV_CSR to not collide with the value of NT_RISCV_VECTOR in Linux kernel header file 'include/uapi/linux/elf.h'
  2023-08-10 18:56   ` Greg Savin
@ 2023-08-10 20:00     ` John Baldwin
  2023-08-10 22:11       ` Greg Savin
  0 siblings, 1 reply; 12+ messages in thread
From: John Baldwin @ 2023-08-10 20:00 UTC (permalink / raw)
  To: Greg Savin; +Cc: gdb-patches, binutils, palmer, greentime.hu, Andrew Burgess

On 8/10/23 11:56 AM, Greg Savin wrote:
> Initially I tried reusing 0x900 for NT_RISCV_VECTOR, and leaving
> NT_RISCV_CSR in place with its original 0x900 value, and this was resulting
> in compile errors of "duplicate case values within a switch statement."  So
> I figured having duplicate NT_* values was a "no no" at least in the
> standards of the GDB project.  I'm guessing that particular compile error
> can be treated as a warning, but that GDB intentionally chooses compile
> flags to treat the duplicate case values as a hard error (?)

Probably in this case the switch statement needs to take the note name into
context.  For example, libbfd's note parsing code calls different helper
routines for "FreeBSD" vs "NetBSD" vs "LINUX" core notes where each of these
helpers has its own switch statement on NT_* values.  The switch statement
checking NT_RISCV_CSR is probably not done in a context specific to "GDB".

> --Greg
> 
> On Thu, Aug 10, 2023 at 11:39 AM John Baldwin <jhb@freebsd.org> wrote:
> 
>> On 8/9/23 10:53 AM, Greg Savin via Gdb-patches wrote:
>>> Linux kernel's file 'include/uapi/linux/elf.h' declares the value 0x900
>>> for NT_RISCV_VECTOR, and does not have a definition for NT_RISCV_CSR, nor
>>> does it use the value 0x901 for any note type.  This patch is intended
>>> as a way to resolve the disagreement/collision between Linux and
>> binutils,
>>> over the meaning of 0x900 in the context of note types.
>>>
>>> ---
>>>    include/elf/common.h | 2 +-
>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/include/elf/common.h b/include/elf/common.h
>>> index ffa6b60bd2b..0bbe245519e 100644
>>> --- a/include/elf/common.h
>>> +++ b/include/elf/common.h
>>> @@ -713,7 +713,7 @@
>>>                                        /*   note name must be "LINUX".  */
>>>    #define NT_LARCH_LBT    0xa04               /* LoongArch Binary
>> Translation registers */
>>>                                        /*   note name must be "CORE".  */
>>> -#define NT_RISCV_CSR    0x900                /* RISC-V Control and
>> Status Registers */
>>> +#define NT_RISCV_CSR    0x901                /* RISC-V Control and
>> Status Registers */
>>>                                        /*   note name must be "LINUX".  */
>>>    #define NT_SIGINFO  0x53494749      /* Fields of siginfo_t.  */
>>>    #define NT_FILE             0x46494c45      /* Description of mapped
>> files.  */
>>
>> If there aren't any active uses of NT_RISCV_CSR "in the wild", then it
>> might be
>> better to just remove it entirely?
>>
>> It looks like commit e214f8db56f65531b0a5ec296c42339dcaa5af31 adding
>> LoongArch
>> constants inadvertently changed the comment for this note from "CORE" (a
>> binutils note) to "LINUX" (a Linux kernel note).
>>
>> The log message adding NT_RISCV_CSR is clearer (though the comment in
>> common.h
>> is wrong and should be "GDB" it seems):
>>
>> commit db6092f3aec43ea4d10efc5ff74274f04cdc0ad6
>> Author: Andrew Burgess <andrew.burgess@embecosm.com>
>> Date:   Fri Nov 27 14:04:16 2020 +0000
>>
>>       bfd/binutils: add support for RISC-V CSRs in core files
>>
>>       Adds support for including RISC-V control and status registers into
>>       core files.
>>
>>       The value for the define NT_RISCV_CSR is set to 0x900, this
>>       corresponds to a patch I have proposed for the Linux kernel here:
>>
>>
>> http://lists.infradead.org/pipermail/linux-riscv/2020-December/003910.html
>>
>>       As I have not yet heard if the above patch will be accepted into the
>>       kernel or not I have set the note name string to "GDB", and the note
>>       type to NT_RISCV_CSR.
>>
>>       This means that if the above patch is rejected from the kernel, and
>>       the note type number 0x900 is assigned to some other note type, we
>>       will still be able to distinguish between the GDB produced
>>       NT_RISCV_CSR, and the kernel produced notes, where the name would be
>>       set to "CORE".
>>
>> Seems like the kernel patch died on the vine (no responses to the patch).
>> GDB does support writing this out still though via the ".reg.riscv-csr"
>> pseudo section, so changing this would break debugging bare metal core
>> dumps.  You can however, just re-use 0x900 for NT_RISCV_VECTOR without
>> this patch.  The different note name is sufficient to differentiate the
>> two.
>>
>> --
>> John Baldwin
>>
>>
> 

-- 
John Baldwin


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH] Re-map value of NT_RISCV_CSR to not collide with the value of NT_RISCV_VECTOR in Linux kernel header file 'include/uapi/linux/elf.h'
  2023-08-10 20:00     ` John Baldwin
@ 2023-08-10 22:11       ` Greg Savin
  2023-08-10 22:11         ` [PATCH v2 1/2] Reset note name of NT_RISCV_CSR to "GDB" to be consistent with the intent described in commit db6092f3aec43ea4d10efc5ff74274f04cdc0ad6 Greg Savin
  2023-08-10 22:11         ` [PATCH v2 2/2] Propagate NT_RISCV_VECTOR from Linux kernel headers to binutils. The value is identical to pre-existing NT_RISCV_CSR but the note names different (NT_RISCV_CSR is "GDB" and NT_RISCV_VECTOR is "CORE") Greg Savin
  0 siblings, 2 replies; 12+ messages in thread
From: Greg Savin @ 2023-08-10 22:11 UTC (permalink / raw)
  To: gdb-patches, binutils, palmer, greentime.hu, John Baldwin

Sending a couple small patches as a v2, based on comments, hopefully this
is getting closer.



^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v2 1/2] Reset note name of NT_RISCV_CSR to "GDB" to be consistent with the intent described in commit db6092f3aec43ea4d10efc5ff74274f04cdc0ad6
  2023-08-10 22:11       ` Greg Savin
@ 2023-08-10 22:11         ` Greg Savin
  2023-08-11 12:23           ` Andrew Burgess
  2023-08-10 22:11         ` [PATCH v2 2/2] Propagate NT_RISCV_VECTOR from Linux kernel headers to binutils. The value is identical to pre-existing NT_RISCV_CSR but the note names different (NT_RISCV_CSR is "GDB" and NT_RISCV_VECTOR is "CORE") Greg Savin
  1 sibling, 1 reply; 12+ messages in thread
From: Greg Savin @ 2023-08-10 22:11 UTC (permalink / raw)
  To: gdb-patches, binutils, palmer, greentime.hu, John Baldwin; +Cc: Greg Savin

---
 include/elf/common.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/elf/common.h b/include/elf/common.h
index ffa6b60bd2b..e7eede23a07 100644
--- a/include/elf/common.h
+++ b/include/elf/common.h
@@ -714,7 +714,7 @@
 #define NT_LARCH_LBT    0xa04		/* LoongArch Binary Translation registers */
 					/*   note name must be "CORE".  */
 #define NT_RISCV_CSR    0x900		/* RISC-V Control and Status Registers */
-					/*   note name must be "LINUX".  */
+					/*   note name must be "GDB".  */
 #define NT_SIGINFO	0x53494749	/* Fields of siginfo_t.  */
 #define NT_FILE		0x46494c45	/* Description of mapped files.  */
 
-- 
2.25.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v2 2/2] Propagate NT_RISCV_VECTOR from Linux kernel headers to binutils. The value is identical to pre-existing NT_RISCV_CSR but the note names different (NT_RISCV_CSR is "GDB" and NT_RISCV_VECTOR is "CORE")
  2023-08-10 22:11       ` Greg Savin
  2023-08-10 22:11         ` [PATCH v2 1/2] Reset note name of NT_RISCV_CSR to "GDB" to be consistent with the intent described in commit db6092f3aec43ea4d10efc5ff74274f04cdc0ad6 Greg Savin
@ 2023-08-10 22:11         ` Greg Savin
  2023-08-11 12:51           ` Andrew Burgess
  1 sibling, 1 reply; 12+ messages in thread
From: Greg Savin @ 2023-08-10 22:11 UTC (permalink / raw)
  To: gdb-patches, binutils, palmer, greentime.hu, John Baldwin; +Cc: Greg Savin

---
 include/elf/common.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/elf/common.h b/include/elf/common.h
index e7eede23a07..d28f5c6ccf5 100644
--- a/include/elf/common.h
+++ b/include/elf/common.h
@@ -715,6 +715,8 @@
 					/*   note name must be "CORE".  */
 #define NT_RISCV_CSR    0x900		/* RISC-V Control and Status Registers */
 					/*   note name must be "GDB".  */
+#define NT_RISCV_VECTOR 0x900		/* RISC-V Vector Registers */
+					/*   note name must be "CORE".  */
 #define NT_SIGINFO	0x53494749	/* Fields of siginfo_t.  */
 #define NT_FILE		0x46494c45	/* Description of mapped files.  */
 
-- 
2.25.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 1/2] Reset note name of NT_RISCV_CSR to "GDB" to be consistent with the intent described in commit db6092f3aec43ea4d10efc5ff74274f04cdc0ad6
  2023-08-10 22:11         ` [PATCH v2 1/2] Reset note name of NT_RISCV_CSR to "GDB" to be consistent with the intent described in commit db6092f3aec43ea4d10efc5ff74274f04cdc0ad6 Greg Savin
@ 2023-08-11 12:23           ` Andrew Burgess
  0 siblings, 0 replies; 12+ messages in thread
From: Andrew Burgess @ 2023-08-11 12:23 UTC (permalink / raw)
  To: Greg Savin via Binutils, gdb-patches, binutils, palmer,
	greentime.hu, John Baldwin
  Cc: Greg Savin

Greg Savin via Binutils <binutils@sourceware.org> writes:

> ---
>  include/elf/common.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/elf/common.h b/include/elf/common.h
> index ffa6b60bd2b..e7eede23a07 100644
> --- a/include/elf/common.h
> +++ b/include/elf/common.h
> @@ -714,7 +714,7 @@
>  #define NT_LARCH_LBT    0xa04		/* LoongArch Binary Translation registers */
>  					/*   note name must be "CORE".  */
>  #define NT_RISCV_CSR    0x900		/* RISC-V Control and Status Registers */
> -					/*   note name must be "LINUX".  */
> +					/*   note name must be "GDB".  */

I'm not a binutils maintainer, so can't approve this, but did originally
add NT_RISCV_CSR.

FWIW, this change LGTM.

Thanks,
Andrew



>  #define NT_SIGINFO	0x53494749	/* Fields of siginfo_t.  */
>  #define NT_FILE		0x46494c45	/* Description of mapped files.  */
>  
> -- 
> 2.25.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 2/2] Propagate NT_RISCV_VECTOR from Linux kernel headers to binutils. The value is identical to pre-existing NT_RISCV_CSR but the note names different (NT_RISCV_CSR is "GDB" and NT_RISCV_VECTOR is "CORE")
  2023-08-10 22:11         ` [PATCH v2 2/2] Propagate NT_RISCV_VECTOR from Linux kernel headers to binutils. The value is identical to pre-existing NT_RISCV_CSR but the note names different (NT_RISCV_CSR is "GDB" and NT_RISCV_VECTOR is "CORE") Greg Savin
@ 2023-08-11 12:51           ` Andrew Burgess
  2023-08-11 16:43             ` Palmer Dabbelt
  0 siblings, 1 reply; 12+ messages in thread
From: Andrew Burgess @ 2023-08-11 12:51 UTC (permalink / raw)
  To: Greg Savin via Gdb-patches, gdb-patches, binutils, palmer,
	greentime.hu, John Baldwin
  Cc: Greg Savin

Greg Savin via Gdb-patches <gdb-patches@sourceware.org> writes:

> ---
>  include/elf/common.h | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/include/elf/common.h b/include/elf/common.h
> index e7eede23a07..d28f5c6ccf5 100644
> --- a/include/elf/common.h
> +++ b/include/elf/common.h
> @@ -715,6 +715,8 @@
>  					/*   note name must be "CORE".  */
>  #define NT_RISCV_CSR    0x900		/* RISC-V Control and Status Registers */
>  					/*   note name must be "GDB".  */
> +#define NT_RISCV_VECTOR 0x900		/* RISC-V Vector Registers */
> +					/*   note name must be "CORE".  */

I'm not a binutils maintainer, but do have an interest from the
RISC-V/GDB side.

Given the comments you made in an earlier mail, I guess we're going to
have to restructure some of the core file support in order to handle
NT_RISCV_CSR and NT_RISCV_VECTOR having the same values.  But that
should be doable.

No objections to this patch from me.

Thanks,
Andrew


>  #define NT_SIGINFO	0x53494749	/* Fields of siginfo_t.  */
>  #define NT_FILE		0x46494c45	/* Description of mapped files.  */
>  
> -- 
> 2.25.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 2/2] Propagate NT_RISCV_VECTOR from Linux kernel headers to binutils. The value is identical to pre-existing NT_RISCV_CSR but the note names different (NT_RISCV_CSR is "GDB" and NT_RISCV_VECTOR is "CORE")
  2023-08-11 12:51           ` Andrew Burgess
@ 2023-08-11 16:43             ` Palmer Dabbelt
  2023-08-12  0:47               ` John Baldwin
  0 siblings, 1 reply; 12+ messages in thread
From: Palmer Dabbelt @ 2023-08-11 16:43 UTC (permalink / raw)
  To: Andrew Burgess
  Cc: gdb-patches, gdb-patches, binutils, greentime.hu, jhb, greg.savin

On Fri, 11 Aug 2023 05:51:45 PDT (-0700), Andrew Burgess wrote:
> Greg Savin via Gdb-patches <gdb-patches@sourceware.org> writes:
>
>> ---
>>  include/elf/common.h | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/include/elf/common.h b/include/elf/common.h
>> index e7eede23a07..d28f5c6ccf5 100644
>> --- a/include/elf/common.h
>> +++ b/include/elf/common.h
>> @@ -715,6 +715,8 @@
>>  					/*   note name must be "CORE".  */
>>  #define NT_RISCV_CSR    0x900		/* RISC-V Control and Status Registers */
>>  					/*   note name must be "GDB".  */
>> +#define NT_RISCV_VECTOR 0x900		/* RISC-V Vector Registers */
>> +					/*   note name must be "CORE".  */
>
> I'm not a binutils maintainer, but do have an interest from the
> RISC-V/GDB side.
>
> Given the comments you made in an earlier mail, I guess we're going to
> have to restructure some of the core file support in order to handle
> NT_RISCV_CSR and NT_RISCV_VECTOR having the same values.  But that
> should be doable.
>
> No objections to this patch from me.

NT_RISCV_VECTOR isn't in a releasted kernel, so it's not a stable uABI 
yet.  So there's still time to change it.  0I've got a revert on the 
kernel lists, it'd mean we miss 6.5 but we'd just end up in 6.6 which 
isn't so bad.

> Thanks,
> Andrew
>
>
>>  #define NT_SIGINFO	0x53494749	/* Fields of siginfo_t.  */
>>  #define NT_FILE		0x46494c45	/* Description of mapped files.  */
>>
>> --
>> 2.25.1

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 2/2] Propagate NT_RISCV_VECTOR from Linux kernel headers to binutils. The value is identical to pre-existing NT_RISCV_CSR but the note names different (NT_RISCV_CSR is "GDB" and NT_RISCV_VECTOR is "CORE")
  2023-08-11 16:43             ` Palmer Dabbelt
@ 2023-08-12  0:47               ` John Baldwin
  2023-08-12  0:50                 ` Palmer Dabbelt
  0 siblings, 1 reply; 12+ messages in thread
From: John Baldwin @ 2023-08-12  0:47 UTC (permalink / raw)
  To: Palmer Dabbelt, Andrew Burgess
  Cc: gdb-patches, binutils, greentime.hu, greg.savin

On 8/11/23 9:43 AM, Palmer Dabbelt wrote:
> On Fri, 11 Aug 2023 05:51:45 PDT (-0700), Andrew Burgess wrote:
>> Greg Savin via Gdb-patches <gdb-patches@sourceware.org> writes:
>>
>>> ---
>>>   include/elf/common.h | 2 ++
>>>   1 file changed, 2 insertions(+)
>>>
>>> diff --git a/include/elf/common.h b/include/elf/common.h
>>> index e7eede23a07..d28f5c6ccf5 100644
>>> --- a/include/elf/common.h
>>> +++ b/include/elf/common.h
>>> @@ -715,6 +715,8 @@
>>>   					/*   note name must be "CORE".  */
>>>   #define NT_RISCV_CSR    0x900		/* RISC-V Control and Status Registers */
>>>   					/*   note name must be "GDB".  */
>>> +#define NT_RISCV_VECTOR 0x900		/* RISC-V Vector Registers */
>>> +					/*   note name must be "CORE".  */
>>
>> I'm not a binutils maintainer, but do have an interest from the
>> RISC-V/GDB side.
>>
>> Given the comments you made in an earlier mail, I guess we're going to
>> have to restructure some of the core file support in order to handle
>> NT_RISCV_CSR and NT_RISCV_VECTOR having the same values.  But that
>> should be doable.
>>
>> No objections to this patch from me.
> 
> NT_RISCV_VECTOR isn't in a releasted kernel, so it's not a stable uABI
> yet.  So there's still time to change it.  0I've got a revert on the
> kernel lists, it'd mean we miss 6.5 but we'd just end up in 6.6 which
> isn't so bad.

While it is possible to handle colliding NT_RISCV_* values, it will probably
be simpler all-around if you are able to make NT_RISCV_VECTOR be 0x901 since
you are going to revert and redo.

-- 
John Baldwin


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 2/2] Propagate NT_RISCV_VECTOR from Linux kernel headers to binutils. The value is identical to pre-existing NT_RISCV_CSR but the note names different (NT_RISCV_CSR is "GDB" and NT_RISCV_VECTOR is "CORE")
  2023-08-12  0:47               ` John Baldwin
@ 2023-08-12  0:50                 ` Palmer Dabbelt
  0 siblings, 0 replies; 12+ messages in thread
From: Palmer Dabbelt @ 2023-08-12  0:50 UTC (permalink / raw)
  To: jhb; +Cc: Andrew Burgess, gdb-patches, binutils, greentime.hu, greg.savin

On Fri, 11 Aug 2023 17:47:29 PDT (-0700), jhb@FreeBSD.org wrote:
> On 8/11/23 9:43 AM, Palmer Dabbelt wrote:
>> On Fri, 11 Aug 2023 05:51:45 PDT (-0700), Andrew Burgess wrote:
>>> Greg Savin via Gdb-patches <gdb-patches@sourceware.org> writes:
>>>
>>>> ---
>>>>   include/elf/common.h | 2 ++
>>>>   1 file changed, 2 insertions(+)
>>>>
>>>> diff --git a/include/elf/common.h b/include/elf/common.h
>>>> index e7eede23a07..d28f5c6ccf5 100644
>>>> --- a/include/elf/common.h
>>>> +++ b/include/elf/common.h
>>>> @@ -715,6 +715,8 @@
>>>>   					/*   note name must be "CORE".  */
>>>>   #define NT_RISCV_CSR    0x900		/* RISC-V Control and Status Registers */
>>>>   					/*   note name must be "GDB".  */
>>>> +#define NT_RISCV_VECTOR 0x900		/* RISC-V Vector Registers */
>>>> +					/*   note name must be "CORE".  */
>>>
>>> I'm not a binutils maintainer, but do have an interest from the
>>> RISC-V/GDB side.
>>>
>>> Given the comments you made in an earlier mail, I guess we're going to
>>> have to restructure some of the core file support in order to handle
>>> NT_RISCV_CSR and NT_RISCV_VECTOR having the same values.  But that
>>> should be doable.
>>>
>>> No objections to this patch from me.
>>
>> NT_RISCV_VECTOR isn't in a releasted kernel, so it's not a stable uABI
>> yet.  So there's still time to change it.  0I've got a revert on the
>> kernel lists, it'd mean we miss 6.5 but we'd just end up in 6.6 which
>> isn't so bad.
>
> While it is possible to handle colliding NT_RISCV_* values, it will probably
> be simpler all-around if you are able to make NT_RISCV_VECTOR be 0x901 since
> you are going to revert and redo.

Ya, that'd be part of the reason to backport it, there's also one or two 
other bugs we found so it's probably just a good sign this wasn't 
adequately reviewed.  Here's the Linux patch 
<https://lore.kernel.org/all/20230810214810.21905-1-palmer@rivosinc.com/>

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2023-08-12  0:50 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-08-09 17:53 [PATCH] Re-map value of NT_RISCV_CSR to not collide with the value of NT_RISCV_VECTOR in Linux kernel header file 'include/uapi/linux/elf.h' Greg Savin
2023-08-10 18:39 ` John Baldwin
2023-08-10 18:56   ` Greg Savin
2023-08-10 20:00     ` John Baldwin
2023-08-10 22:11       ` Greg Savin
2023-08-10 22:11         ` [PATCH v2 1/2] Reset note name of NT_RISCV_CSR to "GDB" to be consistent with the intent described in commit db6092f3aec43ea4d10efc5ff74274f04cdc0ad6 Greg Savin
2023-08-11 12:23           ` Andrew Burgess
2023-08-10 22:11         ` [PATCH v2 2/2] Propagate NT_RISCV_VECTOR from Linux kernel headers to binutils. The value is identical to pre-existing NT_RISCV_CSR but the note names different (NT_RISCV_CSR is "GDB" and NT_RISCV_VECTOR is "CORE") Greg Savin
2023-08-11 12:51           ` Andrew Burgess
2023-08-11 16:43             ` Palmer Dabbelt
2023-08-12  0:47               ` John Baldwin
2023-08-12  0:50                 ` Palmer Dabbelt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).