| Message ID | 1455300361-13092-5-git-send-email-konrad.wilk@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 12/02/16 18:05, Konrad Rzeszutek Wilk wrote: > From: Ross Lagerwall <ross.lagerwall@citrix.com> > > Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com> > Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> (this patch looks like it can be fast-tracked in the series?) > --- > v2: Slim the list as we do not use all of them. > --- > xen/include/xen/elfstructs.h | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/xen/include/xen/elfstructs.h b/xen/include/xen/elfstructs.h > index 12ffb82..4ff3258 100644 > --- 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. */ > +#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 */ > + > /* Program Header */ > typedef struct { > Elf32_Word p_type; /* segment type */
>>> 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. > +#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 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? Jan
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 >
>>> On 19.02.16 at 22:05, <konrad.wilk@oracle.com> wrote: > 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). Since you will need to refuse any unknown ones anyway, I don't see a reason to name some unsupported one but not others. Jan
On 02/19/2016 09:05 PM, Konrad Rzeszutek Wilk wrote: > 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. >> From my testing, GCC generates R_X86_64_64, R_X86_64_PC32, and R_X86_64_PLT32 relocations so those are the ones we need initially. More can be added later as needed, I suppose.
diff --git a/xen/include/xen/elfstructs.h b/xen/include/xen/elfstructs.h index 12ffb82..4ff3258 100644 --- 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. */ +#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 */ + /* Program Header */ typedef struct { Elf32_Word p_type; /* segment type */