Message ID | 57ADFD32020000780010580C@prv-mh.provo.novell.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
> --- a/xen/arch/x86/Makefile > +++ b/xen/arch/x86/Makefile > @@ -158,24 +158,30 @@ $(TARGET).efi: VIRT_BASE = 0x$(shell $(N > $(TARGET).efi: ALT_BASE = 0x$(shell $(NM) efi/relocs-dummy.o | sed -n 's, A ALT_START$$,,p') > # Don't use $(wildcard ...) here - at least make 3.80 expands this too early! > $(TARGET).efi: guard = $(if $(shell echo efi/dis* | grep disabled),:) > + > ifneq ($(build_id_linker),) > -$(TARGET).efi: note.o > +ifeq ($(call ld-ver-build-id,$(LD) $(EFI_LDFLAGS)),y) > +CFLAGS += -DBUILD_ID_EFI > +EFI_LDFLAGS += $(build_id_linker) > +note_file := efi/buildid.o > +else > note_file := note.o > +endif > else > note_file := > endif > > -$(TARGET).efi: prelink-efi.o efi.lds efi/relocs-dummy.o $(BASEDIR)/common/symbols-dummy.o efi/mkreloc > +$(TARGET).efi: prelink-efi.o $(note_file) efi.lds efi/relocs-dummy.o $(BASEDIR)/common/symbols-dummy.o efi/mkreloc > $(foreach base, $(VIRT_BASE) $(ALT_BASE), \ > $(guard) $(LD) $(call EFI_LDFLAGS,$(base)) -T efi.lds -N $< efi/relocs-dummy.o \ > - $(BASEDIR)/common/symbols-dummy.o -o $(@D)/.$(@F).$(base).0 &&) : > + $(BASEDIR)/common/symbols-dummy.o $(note_file) -o $(@D)/.$(@F).$(base).0 &&) : > $(guard) efi/mkreloc $(foreach base,$(VIRT_BASE) $(ALT_BASE),$(@D)/.$(@F).$(base).0) >$(@D)/.$(@F).0r.S > $(guard) $(NM) -pa --format=sysv $(@D)/.$(@F).$(VIRT_BASE).0 \ > | $(guard) $(BASEDIR)/tools/symbols $(all_symbols) --sysv --sort >$(@D)/.$(@F).0s.S > $(guard) $(MAKE) -f $(BASEDIR)/Rules.mk $(@D)/.$(@F).0r.o $(@D)/.$(@F).0s.o > $(foreach base, $(VIRT_BASE) $(ALT_BASE), \ > $(guard) $(LD) $(call EFI_LDFLAGS,$(base)) -T efi.lds -N $< \ > - $(@D)/.$(@F).0r.o $(@D)/.$(@F).0s.o -o $(@D)/.$(@F).$(base).1 &&) : > + $(@D)/.$(@F).0r.o $(@D)/.$(@F).0s.o $(note_file) -o $(@D)/.$(@F).$(base).1 &&) : > $(guard) efi/mkreloc $(foreach base,$(VIRT_BASE) $(ALT_BASE),$(@D)/.$(@F).$(base).1) >$(@D)/.$(@F).1r.S > $(guard) $(NM) -pa --format=sysv $(@D)/.$(@F).$(VIRT_BASE).1 \ > | $(guard) $(BASEDIR)/tools/symbols $(all_symbols) --sysv --sort >$(@D)/.$(@F).1s.S > @@ -185,8 +191,8 @@ $(TARGET).efi: prelink-efi.o efi.lds efi > if $(guard) false; then rm -f $@; echo 'EFI support disabled'; fi > rm -f $(@D)/.$(@F).[0-9]* > > -efi/boot.init.o efi/runtime.o efi/compat.o: $(BASEDIR)/arch/x86/efi/built_in.o > -efi/boot.init.o efi/runtime.o efi/compat.o: ; > +efi/boot.init.o efi/runtime.o efi/compat.o efi/buildid.o: $(BASEDIR)/arch/x86/efi/built_in.o > +efi/boot.init.o efi/runtime.o efi/compat.o efi/buildid.o: ; > > asm-offsets.s: $(TARGET_SUBARCH)/asm-offsets.c > $(CC) $(filter-out -flto,$(CFLAGS)) -S -o $@ $< > --- a/xen/arch/x86/efi/Makefile > +++ b/xen/arch/x86/efi/Makefile > @@ -9,6 +9,9 @@ efi := $(if $(efi),$(shell $(CC) $(filte > efi := $(if $(efi),$(shell $(LD) -mi386pep --subsystem=10 -o check.efi check.o 2>disabled && echo y)) > efi := $(if $(efi),$(shell rm disabled)y,$(shell $(call create,boot.init.o); $(call create,runtime.o))) > > -extra-$(efi) += boot.init.o relocs-dummy.o runtime.o compat.o > +extra-$(efi) += boot.init.o relocs-dummy.o runtime.o compat.o buildid.o > + > +%.o: %.ihex > + $(OBJCOPY) -I ihex -O binary $< $@ > Under Ubuntu 14.04.4 this fails compilation: make[4]: *** No rule to make target `buildid.o', needed by `stub.o'. Stop. The properties of Ubuntu 14.04.4 are: konrad@ubuntu:~/xen$ gcc --version gcc (Ubuntu 4.8.4-2ubuntu1~14.04.3) 4.8.4 konrad@ubuntu:~/xen/xen$ ld --version GNU ld (GNU Binutils for Ubuntu) 2.24 konrad@ubuntu:~/xen/xen$ ld -mi386pep ld: no input files konrad@ubuntu:~/xen/xen$ cat /etc/debian_version jessie/sid Hadn't dug in this.
>>> On 15.08.16 at 01:42, <konrad@kernel.org> wrote: >> --- a/xen/arch/x86/efi/Makefile >> +++ b/xen/arch/x86/efi/Makefile >> @@ -9,6 +9,9 @@ efi := $(if $(efi),$(shell $(CC) $(filte >> efi := $(if $(efi),$(shell $(LD) -mi386pep --subsystem=10 -o check.efi check.o 2>disabled && echo y)) >> efi := $(if $(efi),$(shell rm disabled)y,$(shell $(call create,boot.init.o); $(call create,runtime.o))) >> >> -extra-$(efi) += boot.init.o relocs-dummy.o runtime.o compat.o >> +extra-$(efi) += boot.init.o relocs-dummy.o runtime.o compat.o buildid.o >> + >> +%.o: %.ihex >> + $(OBJCOPY) -I ihex -O binary $< $@ > > > Under Ubuntu 14.04.4 this fails compilation: > > > make[4]: *** No rule to make target `buildid.o', needed by `stub.o'. That's extremely odd, namely considering that the rule is right there in the quoted text above. Could you double check xen/arch/x86/efi/buildid.ihex is actually there? > The properties of Ubuntu 14.04.4 are: > konrad@ubuntu:~/xen$ gcc --version > gcc (Ubuntu 4.8.4-2ubuntu1~14.04.3) 4.8.4 > konrad@ubuntu:~/xen/xen$ ld --version > GNU ld (GNU Binutils for Ubuntu) 2.24 > konrad@ubuntu:~/xen/xen$ ld -mi386pep > ld: no input files > > konrad@ubuntu:~/xen/xen$ cat /etc/debian_version > jessie/sid For the error above the version of make is actually the most relevant one (albeit for this trivial a rule I can't really how the make version could matter). Since you've chopped off other make output - does the error indeed occur in xen/arch/x86/efi/ (and not in xen/arch/x86/)? I agree this should be the case, since in the other directory the printed name would have to be efi/buildid.o - I'm just trying make sure impossible things really are impossible. Jan
On Mon, Aug 15, 2016 at 01:58:47AM -0600, Jan Beulich wrote: > >>> On 15.08.16 at 01:42, <konrad@kernel.org> wrote: > >> --- a/xen/arch/x86/efi/Makefile > >> +++ b/xen/arch/x86/efi/Makefile > >> @@ -9,6 +9,9 @@ efi := $(if $(efi),$(shell $(CC) $(filte > >> efi := $(if $(efi),$(shell $(LD) -mi386pep --subsystem=10 -o check.efi check.o 2>disabled && echo y)) > >> efi := $(if $(efi),$(shell rm disabled)y,$(shell $(call create,boot.init.o); $(call create,runtime.o))) > >> > >> -extra-$(efi) += boot.init.o relocs-dummy.o runtime.o compat.o > >> +extra-$(efi) += boot.init.o relocs-dummy.o runtime.o compat.o buildid.o > >> + > >> +%.o: %.ihex > >> + $(OBJCOPY) -I ihex -O binary $< $@ > > > > > > Under Ubuntu 14.04.4 this fails compilation: > > > > > > make[4]: *** No rule to make target `buildid.o', needed by `stub.o'. > > That's extremely odd, namely considering that the rule is right > there in the quoted text above. Could you double check > xen/arch/x86/efi/buildid.ihex is actually there? <sigh> No it is not. I had issues with your email (git am -s) so I did it by hand (patch -p2) and of course forgot to include the new file. And later on built it on another OS after pulling from a git tree which missed this file. Sorry about the false report!
On Mon, Aug 15, 2016 at 10:15:47AM -0400, Konrad Rzeszutek Wilk wrote: > On Mon, Aug 15, 2016 at 01:58:47AM -0600, Jan Beulich wrote: > > >>> On 15.08.16 at 01:42, <konrad@kernel.org> wrote: > > >> --- a/xen/arch/x86/efi/Makefile > > >> +++ b/xen/arch/x86/efi/Makefile > > >> @@ -9,6 +9,9 @@ efi := $(if $(efi),$(shell $(CC) $(filte > > >> efi := $(if $(efi),$(shell $(LD) -mi386pep --subsystem=10 -o check.efi check.o 2>disabled && echo y)) > > >> efi := $(if $(efi),$(shell rm disabled)y,$(shell $(call create,boot.init.o); $(call create,runtime.o))) > > >> > > >> -extra-$(efi) += boot.init.o relocs-dummy.o runtime.o compat.o > > >> +extra-$(efi) += boot.init.o relocs-dummy.o runtime.o compat.o buildid.o > > >> + > > >> +%.o: %.ihex > > >> + $(OBJCOPY) -I ihex -O binary $< $@ > > > > > > > > > Under Ubuntu 14.04.4 this fails compilation: > > > > > > > > > make[4]: *** No rule to make target `buildid.o', needed by `stub.o'. > > > > That's extremely odd, namely considering that the rule is right > > there in the quoted text above. Could you double check > > xen/arch/x86/efi/buildid.ihex is actually there? > > <sigh> > No it is not. I had issues with your email (git am -s) so I did it by > hand (patch -p2) and of course forgot to include the new file. And later > on built it on another OS after pulling from a git tree which missed this file. > > Sorry about the false report! About the review: > > x86/EFI: use less crude way of generating the build ID > > Recent enough binutils support --build-id also for COFF/PE output, and > hence we should use that in favor of the original hack when possible. Could you mention the exact version that gained this? > > This gets complicated by the linker requiring at least one COFF object > file to attach the .buildid section to. Hence the patch introduces a Is that requirement spelled out in the manpage of the linker? > buildid.ihex (in order to avoid introducing binary files into the repo) > which then gets converted to a binary minimal COFF object (no sections, > no symbols). Otherwise: Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> Thanks!
>>> On 19.08.16 at 18:14, <konrad.wilk@oracle.com> wrote: > On Mon, Aug 15, 2016 at 10:15:47AM -0400, Konrad Rzeszutek Wilk wrote: >> On Mon, Aug 15, 2016 at 01:58:47AM -0600, Jan Beulich wrote: >> > >>> On 15.08.16 at 01:42, <konrad@kernel.org> wrote: >> > >> --- a/xen/arch/x86/efi/Makefile >> > >> +++ b/xen/arch/x86/efi/Makefile >> > >> @@ -9,6 +9,9 @@ efi := $(if $(efi),$(shell $(CC) $(filte >> > >> efi := $(if $(efi),$(shell $(LD) -mi386pep --subsystem=10 -o check.efi > check.o 2>disabled && echo y)) >> > >> efi := $(if $(efi),$(shell rm disabled)y,$(shell $(call > create,boot.init.o); $(call create,runtime.o))) >> > >> >> > >> -extra-$(efi) += boot.init.o relocs-dummy.o runtime.o compat.o >> > >> +extra-$(efi) += boot.init.o relocs-dummy.o runtime.o compat.o buildid.o >> > >> + >> > >> +%.o: %.ihex >> > >> + $(OBJCOPY) -I ihex -O binary $< $@ >> > > >> > > >> > > Under Ubuntu 14.04.4 this fails compilation: >> > > >> > > >> > > make[4]: *** No rule to make target `buildid.o', needed by `stub.o'. >> > >> > That's extremely odd, namely considering that the rule is right >> > there in the quoted text above. Could you double check >> > xen/arch/x86/efi/buildid.ihex is actually there? >> >> <sigh> >> No it is not. I had issues with your email (git am -s) so I did it by >> hand (patch -p2) and of course forgot to include the new file. And later >> on built it on another OS after pulling from a git tree which missed this > file. >> >> Sorry about the false report! > > About the review: > >> >> x86/EFI: use less crude way of generating the build ID >> >> Recent enough binutils support --build-id also for COFF/PE output, and >> hence we should use that in favor of the original hack when possible. > > Could you mention the exact version that gained this? Looks like that was 2.25; added. >> This gets complicated by the linker requiring at least one COFF object >> file to attach the .buildid section to. Hence the patch introduces a > > Is that requirement spelled out in the manpage of the linker? No. Since it mysteriously failed initially, I had to dig into their sources to figure out that requirement. >> buildid.ihex (in order to avoid introducing binary files into the repo) >> which then gets converted to a binary minimal COFF object (no sections, >> no symbols). > > Otherwise: > > Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> Thanks. Jan
--- a/xen/arch/x86/Makefile +++ b/xen/arch/x86/Makefile @@ -158,24 +158,30 @@ $(TARGET).efi: VIRT_BASE = 0x$(shell $(N $(TARGET).efi: ALT_BASE = 0x$(shell $(NM) efi/relocs-dummy.o | sed -n 's, A ALT_START$$,,p') # Don't use $(wildcard ...) here - at least make 3.80 expands this too early! $(TARGET).efi: guard = $(if $(shell echo efi/dis* | grep disabled),:) + ifneq ($(build_id_linker),) -$(TARGET).efi: note.o +ifeq ($(call ld-ver-build-id,$(LD) $(EFI_LDFLAGS)),y) +CFLAGS += -DBUILD_ID_EFI +EFI_LDFLAGS += $(build_id_linker) +note_file := efi/buildid.o +else note_file := note.o +endif else note_file := endif -$(TARGET).efi: prelink-efi.o efi.lds efi/relocs-dummy.o $(BASEDIR)/common/symbols-dummy.o efi/mkreloc +$(TARGET).efi: prelink-efi.o $(note_file) efi.lds efi/relocs-dummy.o $(BASEDIR)/common/symbols-dummy.o efi/mkreloc $(foreach base, $(VIRT_BASE) $(ALT_BASE), \ $(guard) $(LD) $(call EFI_LDFLAGS,$(base)) -T efi.lds -N $< efi/relocs-dummy.o \ - $(BASEDIR)/common/symbols-dummy.o -o $(@D)/.$(@F).$(base).0 &&) : + $(BASEDIR)/common/symbols-dummy.o $(note_file) -o $(@D)/.$(@F).$(base).0 &&) : $(guard) efi/mkreloc $(foreach base,$(VIRT_BASE) $(ALT_BASE),$(@D)/.$(@F).$(base).0) >$(@D)/.$(@F).0r.S $(guard) $(NM) -pa --format=sysv $(@D)/.$(@F).$(VIRT_BASE).0 \ | $(guard) $(BASEDIR)/tools/symbols $(all_symbols) --sysv --sort >$(@D)/.$(@F).0s.S $(guard) $(MAKE) -f $(BASEDIR)/Rules.mk $(@D)/.$(@F).0r.o $(@D)/.$(@F).0s.o $(foreach base, $(VIRT_BASE) $(ALT_BASE), \ $(guard) $(LD) $(call EFI_LDFLAGS,$(base)) -T efi.lds -N $< \ - $(@D)/.$(@F).0r.o $(@D)/.$(@F).0s.o -o $(@D)/.$(@F).$(base).1 &&) : + $(@D)/.$(@F).0r.o $(@D)/.$(@F).0s.o $(note_file) -o $(@D)/.$(@F).$(base).1 &&) : $(guard) efi/mkreloc $(foreach base,$(VIRT_BASE) $(ALT_BASE),$(@D)/.$(@F).$(base).1) >$(@D)/.$(@F).1r.S $(guard) $(NM) -pa --format=sysv $(@D)/.$(@F).$(VIRT_BASE).1 \ | $(guard) $(BASEDIR)/tools/symbols $(all_symbols) --sysv --sort >$(@D)/.$(@F).1s.S @@ -185,8 +191,8 @@ $(TARGET).efi: prelink-efi.o efi.lds efi if $(guard) false; then rm -f $@; echo 'EFI support disabled'; fi rm -f $(@D)/.$(@F).[0-9]* -efi/boot.init.o efi/runtime.o efi/compat.o: $(BASEDIR)/arch/x86/efi/built_in.o -efi/boot.init.o efi/runtime.o efi/compat.o: ; +efi/boot.init.o efi/runtime.o efi/compat.o efi/buildid.o: $(BASEDIR)/arch/x86/efi/built_in.o +efi/boot.init.o efi/runtime.o efi/compat.o efi/buildid.o: ; asm-offsets.s: $(TARGET_SUBARCH)/asm-offsets.c $(CC) $(filter-out -flto,$(CFLAGS)) -S -o $@ $< --- a/xen/arch/x86/efi/Makefile +++ b/xen/arch/x86/efi/Makefile @@ -9,6 +9,9 @@ efi := $(if $(efi),$(shell $(CC) $(filte efi := $(if $(efi),$(shell $(LD) -mi386pep --subsystem=10 -o check.efi check.o 2>disabled && echo y)) efi := $(if $(efi),$(shell rm disabled)y,$(shell $(call create,boot.init.o); $(call create,runtime.o))) -extra-$(efi) += boot.init.o relocs-dummy.o runtime.o compat.o +extra-$(efi) += boot.init.o relocs-dummy.o runtime.o compat.o buildid.o + +%.o: %.ihex + $(OBJCOPY) -I ihex -O binary $< $@ stub.o: $(extra-y) --- /dev/null +++ b/xen/arch/x86/efi/buildid.ihex @@ -0,0 +1,3 @@ +:10000000648600004D8DAD57140000000000000014 +:0400100000000000EC +:00000001FF --- a/xen/arch/x86/efi/mkreloc.c +++ b/xen/arch/x86/efi/mkreloc.c @@ -342,6 +342,7 @@ int main(int argc, char *argv[]) */ if ( memcmp(sec1[i].name, ".initcal", sizeof(sec1[i].name)) == 0 || memcmp(sec1[i].name, ".init.se", sizeof(sec1[i].name)) == 0 || + memcmp(sec1[i].name, ".buildid", sizeof(sec1[i].name)) == 0 || memcmp(sec1[i].name, ".lockpro", sizeof(sec1[i].name)) == 0 ) continue; --- a/xen/arch/x86/xen.lds.S +++ b/xen/arch/x86/xen.lds.S @@ -91,7 +91,7 @@ SECTIONS *(.rodata) *(.rodata.*) -#if defined(BUILD_ID) && defined(EFI) +#if defined(BUILD_ID) && defined(EFI) && !defined(BUILD_ID_EFI) /* * No mechanism to put an PT_NOTE in the EFI file - so put * it in .rodata section. (notes.o supplies us with .note.gnu.build-id). @@ -120,19 +120,26 @@ SECTIONS #endif } :text -#if defined(BUILD_ID) && !defined(EFI) +#if defined(BUILD_ID) +#if !defined(EFI) /* * What a strange section name. The reason is that on ELF builds this section * is extracted to notes.o (which then is ingested in the EFI file). But the * compiler may want to inject other things in the .note which we don't care * about - hence this unique name. */ - . = ALIGN(4); .note.gnu.build-id : { __note_gnu_build_id_start = .; *(.note.gnu.build-id) __note_gnu_build_id_end = .; } :note :text +#elif defined(BUILD_ID_EFI) + .buildid : { + __note_gnu_build_id_start = .; + *(.buildid) + __note_gnu_build_id_end = .; + } :text +#endif #endif _erodata = .; --- a/xen/common/version.c +++ b/xen/common/version.c @@ -4,6 +4,7 @@ #include <xen/lib.h> #include <xen/string.h> #include <xen/types.h> +#include <xen/efi.h> #include <xen/elf.h> #include <xen/version.h> @@ -117,10 +118,33 @@ int xen_build_id_check(const Elf_Note *n return 0; } +struct pe_external_debug_directory +{ + uint32_t characteristics; + uint32_t time_stamp; + uint16_t major_version; + uint16_t minor_version; +#define PE_IMAGE_DEBUG_TYPE_CODEVIEW 2 + uint32_t type; + uint32_t size; + uint32_t rva_of_data; + uint32_t filepos_of_data; +}; + +struct cv_info_pdb70 +{ +#define CVINFO_PDB70_CVSIGNATURE 0x53445352 /* "RSDS" */ + uint32_t cv_signature; + unsigned char signature[16]; + uint32_t age; + char pdb_filename[]; +}; + static int __init xen_build_init(void) { const Elf_Note *n = __note_gnu_build_id_start; unsigned int sz; + int rc; /* --build-id invoked with wrong parameters. */ if ( __note_gnu_build_id_end <= &n[0] ) @@ -132,7 +156,38 @@ static int __init xen_build_init(void) sz = (void *)__note_gnu_build_id_end - (void *)n; - return xen_build_id_check(n, sz, &build_id_p, &build_id_len); + rc = xen_build_id_check(n, sz, &build_id_p, &build_id_len); + +#ifdef CONFIG_X86 + /* Alternatively we may have a CodeView record from an EFI build. */ + if ( rc && efi_enabled ) + { + const struct pe_external_debug_directory *dir = (const void *)n; + + /* + * Validate that the full-note-header check above won't prevent + * fall-through to the CodeView case here. + */ + BUILD_BUG_ON(sizeof(*n) > sizeof(*dir)); + + if ( sz > sizeof(*dir) + sizeof(struct cv_info_pdb70) && + dir->type == PE_IMAGE_DEBUG_TYPE_CODEVIEW && + dir->size > sizeof(struct cv_info_pdb70) && + XEN_VIRT_START + dir->rva_of_data == (unsigned long)(dir + 1) ) + { + const struct cv_info_pdb70 *info = (const void *)(dir + 1); + + if ( info->cv_signature == CVINFO_PDB70_CVSIGNATURE ) + { + build_id_p = info->signature; + build_id_len = sizeof(info->signature); + rc = 0; + } + } + } +#endif + + return rc; } __initcall(xen_build_init); #endif