[v2,2/2] Hexagon: implement machine flag check

  This fixes the "invalid machine flag" error from eu-elflint when passing
hexagon binaries.

Signed-off-by: Matheus Tavares Bernardino <quic_mathbern@quicinc.com>
---
 backends/hexagon_init.c   | 1 +
 backends/hexagon_symbol.c | 8 ++++++++
 2 files changed, 9 insertions(+)
  

> -----Original Message-----
> From: Matheus Bernardino (QUIC) <quic_mathbern@quicinc.com>
> Sent: Tuesday, April 2, 2024 2:56 PM
> To: elfutils-devel@sourceware.org
> Cc: Brian Cain <bcain@quicinc.com>; Sid Manning <sidneym@quicinc.com>;
> mark@klomp.org; Andrew Pinski (QUIC) <quic_apinski@quicinc.com>;
> Matheus Bernardino (QUIC) <quic_mathbern@quicinc.com>
> Subject: [PATCH v2 2/2] Hexagon: implement machine flag check
> 
> This fixes the "invalid machine flag" error from eu-elflint when passing
> hexagon binaries.
> 
> Signed-off-by: Matheus Tavares Bernardino <quic_mathbern@quicinc.com>
> ---
>  backends/hexagon_init.c   | 1 +
>  backends/hexagon_symbol.c | 8 ++++++++
>  2 files changed, 9 insertions(+)
> 
> diff --git a/backends/hexagon_init.c b/backends/hexagon_init.c
> index 9c8c6d8d..1cd27513 100644
> --- a/backends/hexagon_init.c
> +++ b/backends/hexagon_init.c
> @@ -45,6 +45,7 @@ hexagon_init (Elf *elf __attribute__ ((unused)),
>  {
>    hexagon_init_reloc (eh);
>    HOOK (eh, reloc_simple_type);
> +  HOOK (eh, machine_flag_check);
> 
>    return eh;
>  }
> diff --git a/backends/hexagon_symbol.c b/backends/hexagon_symbol.c
> index b341243e..1e681e9f 100644
> --- a/backends/hexagon_symbol.c
> +++ b/backends/hexagon_symbol.c
> @@ -56,3 +56,11 @@ hexagon_reloc_simple_type (Ebl *ebl __attribute__
> ((unused)), int type,
>        return ELF_T_NUM;
>      }
>  }
> +
> +bool
> +hexagon_machine_flag_check (GElf_Word flags)
> +{
> +  GElf_Word arch_variant = flags &~ EF_HEXAGON_MACH;
> +  /* 0x8000 covers the "tiny core" arch variants. */
> +  return arch_variant == 0 || arch_variant == 0x8000;
> +}
> --
> 2.37.2

What about this instead?

bool hexagon_machine_flag_check(GElf_Word flags) {
	GElf_Word reserved_flags = ~(EF_HEXAGON_TINY | EF_HEXAGON_MACH);

	return (flags & reserved_flags) == 0;
}

... implies a new EF_HEXAGON_TINY 0x8000 definition BTW.
  

Hi,

On Tue, 2024-04-02 at 21:38 +0000, Brian Cain wrote:
> > diff --git a/backends/hexagon_symbol.c b/backends/hexagon_symbol.c
> > index b341243e..1e681e9f 100644
> > --- a/backends/hexagon_symbol.c
> > +++ b/backends/hexagon_symbol.c
> > @@ -56,3 +56,11 @@ hexagon_reloc_simple_type (Ebl *ebl __attribute__
> > ((unused)), int type,
> >        return ELF_T_NUM;
> >      }
> >  }
> > +
> > +bool
> > +hexagon_machine_flag_check (GElf_Word flags)
> > +{
> > +  GElf_Word arch_variant = flags &~ EF_HEXAGON_MACH;
> > +  /* 0x8000 covers the "tiny core" arch variants. */
> > +  return arch_variant == 0 || arch_variant == 0x8000;
> > +}
> > --
> > 2.37.2
> 
> What about this instead?
> 
> bool hexagon_machine_flag_check(GElf_Word flags) {
> 	GElf_Word reserved_flags = ~(EF_HEXAGON_TINY | EF_HEXAGON_MACH);
> 
> 	return (flags & reserved_flags) == 0;
> }
> 
> ... implies a new EF_HEXAGON_TINY 0x8000 definition BTW.

You obviously know this architecture better than me. But is the TINY
flag (bit 15) appropriate for all machines? It looks like it can only
be set on V67 and V71. If so maybe just check those two explicitly?

Cheers,

Mark
  

> -----Original Message-----
> From: Mark Wielaard <mark@klomp.org>
> Sent: Thursday, April 4, 2024 11:29 AM
> To: Brian Cain <bcain@quicinc.com>; Matheus Bernardino (QUIC)
> <quic_mathbern@quicinc.com>; elfutils-devel@sourceware.org
> Cc: Sid Manning <sidneym@quicinc.com>; Andrew Pinski (QUIC)
> <quic_apinski@quicinc.com>
> Subject: Re: [PATCH v2 2/2] Hexagon: implement machine flag check
> 
> WARNING: This email originated from outside of Qualcomm. Please be wary
> of any links or attachments, and do not enable macros.
> 
> Hi,
> 
> On Tue, 2024-04-02 at 21:38 +0000, Brian Cain wrote:
> > > diff --git a/backends/hexagon_symbol.c b/backends/hexagon_symbol.c
> > > index b341243e..1e681e9f 100644
> > > --- a/backends/hexagon_symbol.c
> > > +++ b/backends/hexagon_symbol.c
> > > @@ -56,3 +56,11 @@ hexagon_reloc_simple_type (Ebl *ebl __attribute__
> > > ((unused)), int type,
> > >        return ELF_T_NUM;
> > >      }
> > >  }
> > > +
> > > +bool
> > > +hexagon_machine_flag_check (GElf_Word flags)
> > > +{
> > > +  GElf_Word arch_variant = flags &~ EF_HEXAGON_MACH;
> > > +  /* 0x8000 covers the "tiny core" arch variants. */
> > > +  return arch_variant == 0 || arch_variant == 0x8000;
> > > +}
> > > --
> > > 2.37.2
> >
> > What about this instead?
> >
> > bool hexagon_machine_flag_check(GElf_Word flags) {
> >       GElf_Word reserved_flags = ~(EF_HEXAGON_TINY |
> EF_HEXAGON_MACH);
> >
> >       return (flags & reserved_flags) == 0;
> > }
> >
> > ... implies a new EF_HEXAGON_TINY 0x8000 definition BTW.
> 
> You obviously know this architecture better than me. But is the TINY
> flag (bit 15) appropriate for all machines? It looks like it can only
> be set on V67 and V71. If so maybe just check those two explicitly?

So far, V67T and V71T are the only ones with this variation.  It could also be specified on yet-to-be-created future devices (some theoretical V99T might exist).  What should the behavior be in those cases?

-Brian
  

Hi Brian,

On Thu, 2024-04-04 at 16:31 +0000, Brian Cain wrote:
> > > ... implies a new EF_HEXAGON_TINY 0x8000 definition BTW.
> > 
> > You obviously know this architecture better than me. But is the TINY
> > flag (bit 15) appropriate for all machines? It looks like it can only
> > be set on V67 and V71. If so maybe just check those two explicitly?
> 
> So far, V67T and V71T are the only ones with this variation.
> It could also be specified on yet-to-be-created future devices
> (some theoretical V99T might exist).
> What should the behavior be in those cases?

OK, if there could be other TINY variants then either the check
suggested by Matheus or you would be more correct. I do like the idea
to have a self documenting EF_HEXAGON_TINY constant for this.

Thanks,

Mark