Hollis Blanchard
2006-Jun-12 20:12 UTC
[Xen-devel] [patch] architecture-specific ELF header checking
This patch has only been compile-tested on x86, but it should be pretty straightforward. It could break IA64 since it adds checks they weren''t doing before, but I would expect their ELF binaries are labeled properly. Also, EM_NUM was wrong, but never used. It might be worth it to pull in a fresh copy of xc_elf.h from wherever it came from. Check ELF header fields without ifdefs. Signed-off-by: Hollis Blanchard <hollisb@us.ibm.com> diff -r 34ff26fb2240 config/ia64.mk --- a/config/ia64.mk Mon Jun 12 12:01:32 2006 +0100 +++ b/config/ia64.mk Mon Jun 12 15:04:35 2006 -0500 @@ -2,3 +2,4 @@ CONFIG_IOEMU := y CONFIG_IOEMU := y LIBDIR := lib +CFLAGS += -DELFCLASS=ELFCLASS64 -DELFDATA=ELFDATA2LSB -DELFMACHINE=EM_IA_64 diff -r 34ff26fb2240 config/x86_32.mk --- a/config/x86_32.mk Mon Jun 12 12:01:32 2006 +0100 +++ b/config/x86_32.mk Mon Jun 12 15:04:35 2006 -0500 @@ -5,5 +5,6 @@ CONFIG_IOEMU := y CONFIG_IOEMU := y CONFIG_MBOOTPACK := y -CFLAGS += -m32 -march=i686 +CFLAGS += -m32 -march=i686 \ + -DELFCLASS=ELFCLASS32 -DELFDATA=ELFDATA2LSB -DELFMACHINE=EM_386 LIBDIR := lib diff -r 34ff26fb2240 config/x86_64.mk --- a/config/x86_64.mk Mon Jun 12 12:01:32 2006 +0100 +++ b/config/x86_64.mk Mon Jun 12 15:04:35 2006 -0500 @@ -5,5 +5,6 @@ CONFIG_IOEMU := y CONFIG_IOEMU := y CONFIG_MBOOTPACK := y -CFLAGS += -m64 +CFLAGS += -m64 \ + -DELFCLASS=ELFCLASS64 -DELFDATA=ELFDATA2LSB -DELFMACHINE=EM_X86_64 LIBDIR = lib64 diff -r 34ff26fb2240 tools/libxc/xc_elf.h --- a/tools/libxc/xc_elf.h Mon Jun 12 12:01:32 2006 +0100 +++ b/tools/libxc/xc_elf.h Mon Jun 12 15:04:35 2006 -0500 @@ -170,13 +170,14 @@ typedef struct { #define EM_PARISC 15 /* HPPA */ #define EM_SPARC32PLUS 18 /* Enhanced instruction set SPARC */ #define EM_PPC 20 /* PowerPC */ +#define EM_PPC64 21 /* PowerPC 64-bit */ #define EM_ARM 40 /* Advanced RISC Machines ARM */ #define EM_ALPHA 41 /* DEC ALPHA */ #define EM_SPARCV9 43 /* SPARC version 9 */ #define EM_ALPHA_EXP 0x9026 /* DEC ALPHA */ +#define EM_IA_64 50 /* Intel Merced */ #define EM_X86_64 62 /* AMD x86-64 architecture */ #define EM_VAX 75 /* DEC VAX */ -#define EM_NUM 15 /* number of machine types */ /* Version */ #define EV_NONE 0 /* Invalid */ diff -r 34ff26fb2240 tools/libxc/xc_load_elf.c --- a/tools/libxc/xc_load_elf.c Mon Jun 12 12:01:32 2006 +0100 +++ b/tools/libxc/xc_load_elf.c Mon Jun 12 15:04:35 2006 -0500 @@ -62,14 +62,9 @@ static int parseelfimage(const char *ima } if ( -#if defined(__i386__) - (ehdr->e_ident[EI_CLASS] != ELFCLASS32) || - (ehdr->e_machine != EM_386) || -#elif defined(__x86_64__) - (ehdr->e_ident[EI_CLASS] != ELFCLASS64) || - (ehdr->e_machine != EM_X86_64) || -#endif - (ehdr->e_ident[EI_DATA] != ELFDATA2LSB) || + (ehdr->e_ident[EI_CLASS] != ELFCLASS) || + (ehdr->e_machine != ELFMACHINE) || + (ehdr->e_ident[EI_DATA] != ELFDATA) || (ehdr->e_type != ET_EXEC) ) { ERROR("Kernel not a Xen-compatible Elf image."); -- Hollis Blanchard IBM Linux Technology Center _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir Fraser
2006-Jun-13 09:15 UTC
Re: [Xen-devel] [patch] architecture-specific ELF header checking
On 12 Jun 2006, at 21:12, Hollis Blanchard wrote:> This patch has only been compile-tested on x86, but it should be pretty > straightforward. It could break IA64 since it adds checks they weren''t > doing before, but I would expect their ELF binaries are labeled > properly.I am not keen on adding loads of -D CFLAGS options for very localised/specific macros. They could go in a per-arch header file, but I think in this case just having ifdef''s in xc_elf.h is clean enough. Nothing outside the ELF-parsing code should be looking at these values so keeping them private is sensible. Apart from that, the general idea is fine so I''ll modify and apply. -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Hollis Blanchard
2006-Jun-13 15:17 UTC
Re: [Xen-devel] [patch] architecture-specific ELF header checking
On Tue, 2006-06-13 at 10:15 +0100, Keir Fraser wrote:> On 12 Jun 2006, at 21:12, Hollis Blanchard wrote: > > > This patch has only been compile-tested on x86, but it should be pretty > > straightforward. It could break IA64 since it adds checks they weren''t > > doing before, but I would expect their ELF binaries are labeled > > properly. > > I am not keen on adding loads of -D CFLAGS options for very > localised/specific macros.I agree; a per-arch header file would be ideal.> They could go in a per-arch header file, but > I think in this case just having ifdef''s in xc_elf.h is clean enough.I would like to minimize the amount of code modified by new architectures. I think this is a worthy goal because it would avoid patch conflicts and minimize the chances of accidentally breaking other architectures. (I guess this is true of all modular code actually; it would be nice if one could add a new scheduler just by adding a new source file, without needing to modify other code.) In general we can use the build system to give us indirection, instead of using conditionals in the code. For example, consider xen/include/public/arch-*.h: just like the "asm" symlink, the build system could create a symlink to the appropriate architecture''s header for us. If we had a per-arch header file, I could place PPC ELF definitions in that. If we keep everything in e.g. xc_elf.h, that means I need to modify it.> Nothing outside the ELF-parsing code should be looking at these values > so keeping them private is sensible. Apart from that, the general idea > is fine so I''ll modify and apply.Thanks; I will add ifdefs to xc_elf.h when I see your commit. -- Hollis Blanchard IBM Linux Technology Center _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir Fraser
2006-Jun-13 15:42 UTC
Re: [Xen-devel] [patch] architecture-specific ELF header checking
On 13 Jun 2006, at 16:17, Hollis Blanchard wrote:> I would like to minimize the amount of code modified by new > architectures. I think this is a worthy goal because it would avoid > patch conflicts and minimize the chances of accidentally breaking other > architectures. (I guess this is true of all modular code actually; it > would be nice if one could add a new scheduler just by adding a new > source file, without needing to modify other code.) > > In general we can use the build system to give us indirection, instead > of using conditionals in the code. For example, consider > xen/include/public/arch-*.h: just like the "asm" symlink, the build > system could create a symlink to the appropriate architecture''s header > for us.I agree, but there''s also a question of scope. Macros that are used only be elf-parsing code and that concern the details of elf-header-specific field enumerations arguably do not belong in a repository-global header file. For libxc we probably need per-arch subdirectories for arch-specific source files. There''s all sorts of intermingled arch-specific code in tools/libxc right now. We could then have arch-specific implementations of xc_elf_check_header() or somesuch in tools/libxc/<arch>, like we do for Xen''s domain0 elf-parsing code. That would be cleaner and give more per-arch flexibility. -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Hollis Blanchard
2006-Jun-13 15:58 UTC
Re: [Xen-devel] [patch] architecture-specific ELF header checking
On Tue, 2006-06-13 at 16:42 +0100, Keir Fraser wrote:> On 13 Jun 2006, at 16:17, Hollis Blanchard wrote: > > > I would like to minimize the amount of code modified by new > > architectures. I think this is a worthy goal because it would avoid > > patch conflicts and minimize the chances of accidentally breaking other > > architectures. (I guess this is true of all modular code actually; it > > would be nice if one could add a new scheduler just by adding a new > > source file, without needing to modify other code.) > > > > In general we can use the build system to give us indirection, instead > > of using conditionals in the code. For example, consider > > xen/include/public/arch-*.h: just like the "asm" symlink, the build > > system could create a symlink to the appropriate architecture''s header > > for us. > > I agree, but there''s also a question of scope. Macros that are used > only be elf-parsing code and that concern the details of > elf-header-specific field enumerations arguably do not belong in a > repository-global header file."config.h" seems to be a well-accepted idea in most projects that I''ve seen, but I don''t care too much in this case.> For libxc we probably need per-arch subdirectories for arch-specific > source files. There''s all sorts of intermingled arch-specific code in > tools/libxc right now. We could then have arch-specific implementations > of xc_elf_check_header() or somesuch in tools/libxc/<arch>, like we do > for Xen''s domain0 elf-parsing code. That would be cleaner and give more > per-arch flexibility.Absolutely! I also have a (not quite complete) xc_ppc_linux_build.c that belongs in a directory like that. As PPC implements more libxc functionality, the number of files like that will increase. Anyways, I''m OK with whatever you commit. We could hack xc_elf.h for now, with per-arch directories being the stated direction for the future. -- Hollis Blanchard IBM Linux Technology Center _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Gerd Hoffmann
2006-Jun-14 07:39 UTC
Re: [Xen-devel] [patch] architecture-specific ELF header checking
> For libxc we probably need per-arch subdirectories for arch-specific > source files. There''s all sorts of intermingled arch-specific code in > tools/libxc right now. We could then have arch-specific implementations > of xc_elf_check_header() or somesuch in tools/libxc/<arch>, like we do > for Xen''s domain0 elf-parsing code. That would be cleaner and give more > per-arch flexibility.btw: for the elf parser it is probably useful to switch around using -Dsomething, so one can simply compile the elf loader twice. We''ll need something like that in case we''ll support both 32bit + 64bit guests in a 64bit hypervisor. With my domain builder rewrite it should be easy to do that ;) cheers, Gerd -- Gerd Hoffmann <kraxel@suse.de> http://www.suse.de/~kraxel/julika-dora.jpeg _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Hollis Blanchard
2006-Jun-15 14:50 UTC
Re: [Xen-devel] [patch] architecture-specific ELF header checking
On Wed, 2006-06-14 at 09:39 +0200, Gerd Hoffmann wrote:> > For libxc we probably need per-arch subdirectories for arch-specific > > source files. There''s all sorts of intermingled arch-specific code in > > tools/libxc right now. We could then have arch-specific implementations > > of xc_elf_check_header() or somesuch in tools/libxc/<arch>, like we do > > for Xen''s domain0 elf-parsing code. That would be cleaner and give more > > per-arch flexibility. > > btw: for the elf parser it is probably useful to switch around using > -Dsomething, so one can simply compile the elf loader twice. We''ll need > something like that in case we''ll support both 32bit + 64bit guests in a > 64bit hypervisor. With my domain builder rewrite it should be easy to > do that ;)I agree. As another example, ppc64 uses both 64-bit vmlinux and 32-bit zImage. -- Hollis Blanchard IBM Linux Technology Center _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel