Re: [PATCHv2 6/9] bfd/binutils: add support for RISC-V CSRs in core files

[Luis Machado <luis.machado@linaro.org>]

On 2/1/21 9:00 AM, Andrew Burgess wrote:
> Here is an update version of this patch.
> 
> In this patch I have placed the CSRs into a note named "GDB".  This
> should protect us if, in the future, the note type number I have used
> is reused by some other core file producer (Linux / FreeBSD) and given
> a different meaning.
> 
> Any feedback?
> 
> Thanks,
> Andrew
> 
> 
> ---
> 
> commit 15bcb460981a000e74b2d3dc4f59f0f06bc0cd52
> 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".
>      
>      bfd/ChangeLog:
>      
>              * elf-bfd.h (elfcore_write_riscv_csr): Declare.
>              * elf.c (elfcore_grok_riscv_csr): New function.
>              (elfcore_grok_note): Handle NT_RISCV_CSR.
>              (elfcore_write_riscv_csr): New function.
>              (elfcore_write_register_note): Handle '.reg-riscv-csr'.
>      
>      binutils/ChangeLog:
>      
>              * readelf.c (get_note_type): Handle NT_RISCV_CSR.
>      
>      include/ChangeLog:
>      
>              * elf/common.h (NT_RISCV_CSR): Define.
> 
> diff --git a/bfd/elf-bfd.h b/bfd/elf-bfd.h
> index 779acc745bc..5b7eb8897c8 100644
> --- a/bfd/elf-bfd.h
> +++ b/bfd/elf-bfd.h
> @@ -2797,6 +2797,8 @@ extern char *elfcore_write_aarch_pauth
>     (bfd *, char *, int *, const void *, int);
>   extern char *elfcore_write_arc_v2
>     (bfd *, char *, int *, const void *, int);
> +extern char *elfcore_write_riscv_csr
> +  (bfd *, char *, int *, const void *, int);
>   extern char *elfcore_write_gdb_tdesc
>     (bfd *, char *, int *, const void *, int);
>   extern char *elfcore_write_lwpstatus
> diff --git a/bfd/elf.c b/bfd/elf.c
> index c6cf7fe2d6e..d1fd29f380a 100644
> --- a/bfd/elf.c
> +++ b/bfd/elf.c
> @@ -9912,6 +9912,12 @@ elfcore_grok_arc_v2 (bfd *abfd, Elf_Internal_Note *note)
>     return elfcore_make_note_pseudosection (abfd, ".reg-arc-v2", note);
>   }
>   
> +static bfd_boolean
> +elfcore_grok_riscv_csr (bfd *abfd, Elf_Internal_Note *note)
> +{
> +  return elfcore_make_note_pseudosection (abfd, ".reg-riscv-csr", note);
> +}
> +
>   static bfd_boolean
>   elfcore_grok_gdb_tdesc (bfd *abfd, Elf_Internal_Note *note)
>   {
> @@ -10583,6 +10589,13 @@ elfcore_grok_note (bfd *abfd, Elf_Internal_Note *note)
>         else
>           return TRUE;
>   
> +    case NT_RISCV_CSR:
> +      if (note->namesz == 4
> +          && strcmp (note->namedata, "GDB") == 0)
> +        return elfcore_grok_riscv_csr (abfd, note);
> +      else
> +	return TRUE;
> +
>       case NT_PRPSINFO:
>       case NT_PSINFO:
>         if (bed->elf_backend_grok_psinfo)
> @@ -11964,6 +11977,18 @@ elfcore_write_arc_v2 (bfd *abfd,
>   			     note_name, NT_ARC_V2, arc_v2, size);
>   }
>   
> +char *
> +elfcore_write_riscv_csr (bfd *abfd,
> +                         char *buf,
> +                         int *bufsiz,
> +                         const void *csrs,
> +                         int size)
> +{
> +  const char *note_name = "GDB";
> +  return elfcore_write_note (abfd, buf, bufsiz,
> +			     note_name, NT_RISCV_CSR, csrs, size);
> +}
> +
>   char *
>   elfcore_write_gdb_tdesc (bfd *abfd,
>   			 char *buf,
> @@ -12062,6 +12087,8 @@ elfcore_write_register_note (bfd *abfd,
>       return elfcore_write_arc_v2 (abfd, buf, bufsiz, data, size);
>     if (strcmp (section, ".gdb-tdesc") == 0)
>       return elfcore_write_gdb_tdesc (abfd, buf, bufsiz, data, size);
> +  if (strcmp (section, ".reg-riscv-csr") == 0)
> +    return elfcore_write_riscv_csr (abfd, buf, bufsiz, data, size);
>     return NULL;
>   }
>   
> diff --git a/binutils/readelf.c b/binutils/readelf.c
> index feb458877c8..807eccfb026 100644
> --- a/binutils/readelf.c
> +++ b/binutils/readelf.c
> @@ -18374,6 +18374,8 @@ get_note_type (Filedata * filedata, unsigned e_type)
>   	return _("NT_ARM_HW_WATCH (AArch hardware watchpoint registers)");
>         case NT_ARC_V2:
>   	return _("NT_ARC_V2 (ARC HS accumulator/extra registers)");
> +      case NT_RISCV_CSR:
> +	return _("NT_RISCV_CSR (RISC-V control and status registers)");
>         case NT_PSTATUS:
>   	return _("NT_PSTATUS (pstatus structure)");
>         case NT_FPREGS:
> diff --git a/include/elf/common.h b/include/elf/common.h
> index e6e9c278faa..4cb3748e4fd 100644
> --- a/include/elf/common.h
> +++ b/include/elf/common.h
> @@ -674,6 +674,8 @@
>   					/*   note name must be "LINUX".  */
>   #define NT_ARC_V2	0x600		/* ARC HS accumulator/extra registers.  */
>   					/*   note name must be "LINUX".  */
> +#define NT_RISCV_CSR    0x900		/* RISC-V Control and Status Registers */
> +					/*   note name must be "CORE".  */
>   #define NT_SIGINFO	0x53494749	/* Fields of siginfo_t.  */
>   #define NT_FILE		0x46494c45	/* Description of mapped files.  */
>   
> 

LGTM.