Re: [RFC 01/13] binutils: Support for the NT_X86_CPUID core dump note

[Lancelot SIX <lsix@lancelotsix.com>]

Hi John

I am aware this is a RFC series where you are mostly looking for
feedback on the overall direction, but I have an implemenation question
below (feel free to discard as part of the RFC process).t

On Mon, Oct 09, 2023 at 11:36:03AM -0700, John Baldwin wrote:
> This core dump note contains an array of CPUID leaf values.  Each
> entry in the array contains six 32-bit integers describing the inputs
> to the CPUID instruction (%eax and %ecx) and the outputs of the
> instruction (%eax, %ebx, %ecx, and %edx) similar to the C structure:
> 
> struct cpuid_leaf
> {
>     uint32_t leaf;
>     uint32_t subleaf;
>     uint32_t eax;
>     uint32_t ebx;
>     uint32_t ecx;
>     uint32_t edx;
> };
> 
> Current implementations of this note contain leaves associated with
> the XSAVE state area (major leaf 0xd), but future implementations may
> add other leaf values in the future.
> ---
>  bfd/elf-bfd.h        |  2 ++
>  bfd/elf.c            | 35 +++++++++++++++++++++++++++++++++++
>  binutils/readelf.c   |  2 ++
>  include/elf/common.h |  2 ++
>  4 files changed, 41 insertions(+)
> 
> diff --git a/bfd/elf-bfd.h b/bfd/elf-bfd.h
> index 335081ec629..235d0931ab4 100644
> --- a/bfd/elf-bfd.h
> +++ b/bfd/elf-bfd.h
> @@ -2871,6 +2871,8 @@ extern char *elfcore_write_prxfpreg
>    (bfd *, char *, int *, const void *, int);
>  extern char *elfcore_write_xstatereg
>    (bfd *, char *, int *, const void *, int);
> +extern char *elfcore_write_x86_cpuid
> +  (bfd *, char *, int *, const void *, int);
>  extern char *elfcore_write_x86_segbases
>    (bfd *, char *, int *, const void *, int);
>  extern char *elfcore_write_ppc_vmx
> diff --git a/bfd/elf.c b/bfd/elf.c
> index b5b0c69e097..35679821a49 100644
> --- a/bfd/elf.c
> +++ b/bfd/elf.c
> @@ -10495,6 +10495,16 @@ elfcore_grok_xstatereg (bfd *abfd, Elf_Internal_Note *note)
>    return elfcore_make_note_pseudosection (abfd, ".reg-xstate", note);
>  }
>  
> +/* Some systems dump an array of x86 cpuid leaves with a note type of
> +   NT_X86_CPUID.  Just include the whole note's contents
> +   literally.  */
> +
> +static bool
> +elfcore_grok_x86_cpuid (bfd *abfd, Elf_Internal_Note *note)
> +{
> +  return elfcore_make_note_pseudosection (abfd, ".reg-x86-cpuid", note);
> +}
> +
>  static bool
>  elfcore_grok_ppc_vmx (bfd *abfd, Elf_Internal_Note *note)
>  {
> @@ -11190,6 +11200,13 @@ elfcore_grok_note (bfd *abfd, Elf_Internal_Note *note)
>        else
>  	return true;
>  
> +    case NT_X86_CPUID:
> +      if (note->namesz == 6
> +	  && strcmp (note->namedata, "LINUX") == 0)
> +	return elfcore_grok_x86_cpuid (abfd, note);
> +      else
> +	return true;
> +
>      case NT_PPC_VMX:
>        if (note->namesz == 6
>  	  && strcmp (note->namedata, "LINUX") == 0)
> @@ -11768,6 +11785,9 @@ elfcore_grok_freebsd_note (bfd *abfd, Elf_Internal_Note *note)
>      case NT_X86_XSTATE:
>        return elfcore_grok_xstatereg (abfd, note);
>  
> +    case NT_X86_CPUID:
> +      return elfcore_grok_x86_cpuid (abfd, note);
> +
>      case NT_FREEBSD_PTLWPINFO:
>        return elfcore_make_note_pseudosection (abfd, ".note.freebsdcore.lwpinfo",
>  					      note);
> @@ -12640,6 +12660,19 @@ elfcore_write_xstatereg (bfd *abfd, char *buf, int *bufsiz,
>  			     note_name, NT_X86_XSTATE, xfpregs, size);
>  }
>  
> +char *
> +elfcore_write_x86_cpuid (bfd *abfd, char *buf, int *bufsiz,
> +			 const void *cpuid, int size)
> +{
> +  char *note_name;
> +  if (get_elf_backend_data (abfd)->elf_osabi == ELFOSABI_FREEBSD)
> +    note_name = "FreeBSD";

The code above (in elfcore_grok_note) only accept "LINUX", and the
comment in "include/elf/common.h" says "note name must be "LINUX".

Is this something you intend to adjust later when adding full FreeBSD
support?

Best,
Lancelot.

> +  else
> +    note_name = "LINUX";
> +  return elfcore_write_note (abfd, buf, bufsiz,
> +			     note_name, NT_X86_CPUID, cpuid, size);
> +}
> +
>  char *
>  elfcore_write_x86_segbases (bfd *abfd, char *buf, int *bufsiz,
>  			    const void *regs, int size)
> @@ -13233,6 +13266,8 @@ elfcore_write_register_note (bfd *abfd,
>      return elfcore_write_prxfpreg (abfd, buf, bufsiz, data, size);
>    if (strcmp (section, ".reg-xstate") == 0)
>      return elfcore_write_xstatereg (abfd, buf, bufsiz, data, size);
> +  if (strcmp (section, ".reg-x86-cpuid") == 0)
> +    return elfcore_write_x86_cpuid (abfd, buf, bufsiz, data, size);
>    if (strcmp (section, ".reg-x86-segbases") == 0)
>      return elfcore_write_x86_segbases (abfd, buf, bufsiz, data, size);
>    if (strcmp (section, ".reg-ppc-vmx") == 0)
> diff --git a/binutils/readelf.c b/binutils/readelf.c
> index c9b6210e229..cb80aa6f396 100644
> --- a/binutils/readelf.c
> +++ b/binutils/readelf.c
> @@ -20134,6 +20134,8 @@ get_note_type (Filedata * filedata, unsigned e_type)
>  	return _("NT_X86_XSTATE (x86 XSAVE extended state)");
>        case NT_X86_CET:
>  	return _("NT_X86_CET (x86 CET state)");
> +      case NT_X86_CPUID:
> +	return _("NT_X86_CPUID (x86 CPUID leaves)");
>        case NT_S390_HIGH_GPRS:
>  	return _("NT_S390_HIGH_GPRS (s390 upper register halves)");
>        case NT_S390_TIMER:
> diff --git a/include/elf/common.h b/include/elf/common.h
> index 244b13361e5..e8c6d753987 100644
> --- a/include/elf/common.h
> +++ b/include/elf/common.h
> @@ -645,6 +645,8 @@
>  					/*   note name must be "LINUX".  */
>  #define NT_X86_CET	0x203		/* x86 CET state.  */
>  					/*   note name must be "LINUX".  */
> +#define NT_X86_CPUID	0x205		/* x86 CPUID leaves.  */
> +					/*   note name must be "LINUX".  */
>  #define NT_S390_HIGH_GPRS 0x300		/* S/390 upper halves of GPRs  */
>  					/*   note name must be "LINUX".  */
>  #define NT_S390_TIMER	0x301		/* S390 timer */
> -- 
> 2.41.0
>