Re: [PATCH v3 04/23] elf: Add relocation types to elfstructs.h

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

On Mon, Feb 15, 2016 at 01:34:42AM -0700, Jan Beulich wrote:
> >>> On 12.02.16 at 19:05, <konrad.wilk@oracle.com> wrote:
> > --- a/xen/include/xen/elfstructs.h
> > +++ b/xen/include/xen/elfstructs.h
> > @@ -348,6 +348,14 @@ typedef struct {
> >  #define	ELF64_R_TYPE(info)	((info) & 0xFFFFFFFF)
> >  #define ELF64_R_INFO(s,t) 	(((s) << 32) + (u_int32_t)(t))
> >  
> > +/* x86-64 relocation types. We list only the ones we implement. */
> 
> "we implement" is too vague for my taste: This comment should
> have some kind of reference to xSplice.


/* x86-64 relocation types. We list only the ones xSplice implements. */

?

> 
> > +#define R_X86_64_NONE		0	/* No reloc */
> > +#define R_X86_64_64		1	/* Direct 64 bit  */
> > +#define R_X86_64_PC32		2	/* PC relative 32 bit signed */
> > +#define R_X86_64_PLT32		4	/* 32 bit PLT address */
> > +#define R_X86_64_32		10	/* Direct 32 bit zero extended */
> > +#define R_X86_64_32S		11	/* Direct 32 bit sign extended */
> 
> Is there really a use case for the last two in the hypervisor
> (which doesn't live in the top 2G of address space)? (If the

No. But they are there to catch tools (and developers) by accident
building the payloads with wacky linker options (like I did).

> use case are constants, I suppose R_X86_64_{8,16} ought
> to also be permitted.) Also, is there a reason why at least
> R_X86_64_PC64 shouldn't also be supported?

It hasn't been implemented. Nor has the situation come up
when this was used.

In the previous round of reviews the feedback was that we should only
list the ones the code base was referencing.

Let me add R_X86_64_PC64 on the TODO list.
> 
> Jan
>