| Message ID | 20190627093335.56355-3-roger.pau@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v3,1/3] x86/linker: add a reloc section to ELF linker script | expand |
>>> On 27.06.19 at 11:33, <roger.pau@citrix.com> wrote: > After building the hypervisor binary. Note that the check is performed > by searching for the magic header value at the start of the binary. > > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> Acked-by: Jan Beulich <jbeulich@suse.com>
On 27/06/2019 10:33, Roger Pau Monne wrote: > diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile > index 8a8d8f060f..94e6c9aee3 100644 > --- a/xen/arch/x86/Makefile > +++ b/xen/arch/x86/Makefile > @@ -99,9 +99,14 @@ endif > syms-warn-dup-y := --warn-dup > syms-warn-dup-$(CONFIG_SUPPRESS_DUPLICATE_SYMBOL_WARNINGS) := > > +$(TARGET): TMP = $(@D)/.$(@F) I'd suggest giving this a .elf32 suffix to make it clear which pass of the build it comes from. > $(TARGET): $(TARGET)-syms $(efi-y) boot/mkelf32 > - ./boot/mkelf32 $(notes_phdrs) $(TARGET)-syms $(TARGET) $(XEN_IMG_OFFSET) \ > + ./boot/mkelf32 $(notes_phdrs) $(TARGET)-syms $(TMP) $(XEN_IMG_OFFSET) \ > `$(NM) $(TARGET)-syms | sed -ne 's/^\([^ ]*\) . __2M_rwdata_end$$/0x\1/p'` > + # Check for multiboot{1,2} headers > + od -t x4 -N 8192 $(TMP) | grep 1badb002 > /dev/null || > + od -t x4 -N 32768 $(TMP) | grep e85250d6 > /dev/null This works, but Makefile:104: recipe for target '/local/xen.git/xen/xen' failed Isn't helpful to identify what went wrong. Sadly, we can't use $(error ...) in a shell snippet, but: andrewcoop@andrewcoop:/local/xen.git/xen$ git d diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile index 94e6c9aee3..a1d6605a8b 100644 --- a/xen/arch/x86/Makefile +++ b/xen/arch/x86/Makefile @@ -99,13 +99,14 @@ endif syms-warn-dup-y := --warn-dup syms-warn-dup-$(CONFIG_SUPPRESS_DUPLICATE_SYMBOL_WARNINGS) := -$(TARGET): TMP = $(@D)/.$(@F) +$(TARGET): TMP = $(@D)/.$(@F).elf32 $(TARGET): $(TARGET)-syms $(efi-y) boot/mkelf32 ./boot/mkelf32 $(notes_phdrs) $(TARGET)-syms $(TMP) $(XEN_IMG_OFFSET) \ `$(NM) $(TARGET)-syms | sed -ne 's/^\([^ ]*\) . __2M_rwdata_end$$/0x\1/p'` - # Check for multiboot{1,2} headers - od -t x4 -N 8192 $(TMP) | grep 1badb002 > /dev/null - od -t x4 -N 32768 $(TMP) | grep e85250d6 > /dev/null + od -t x4 -N 8192 $(TMP) | grep 1badb002 > /dev/null || \ + { echo "No Multiboot1 header found"; false; } + od -t x4 -N 32768 $(TMP) | grep e85250d6 > /dev/null || \ + { echo "No Multiboot2 header found"; false; } mv $(TMP) $(TARGET) ALL_OBJS := $(BASEDIR)/arch/x86/boot/built_in.o $(BASEDIR)/arch/x86/efi/built_in.o $(ALL_OBJS) results in: No Multiboot1 header found Makefile:104: recipe for target '/local/xen.git/xen/xen' failed make[2]: *** [/local/xen.git/xen/xen] Error 1 Makefile:136: recipe for target '/local/xen.git/xen/xen' failed make[1]: *** [/local/xen.git/xen/xen] Error 2 Makefile:45: recipe for target 'build' failed make: *** [build] Error 2 Which is far more clear. Thoughts? ~Andrew <html> <head> <meta http-equiv="Content-Type" content="text/html; charset=UTF-8"> </head> <body text="#000000" bgcolor="#FFFFFF"> <div class="moz-cite-prefix">On 27/06/2019 10:33, Roger Pau Monne wrote:<br> </div> <blockquote type="cite" cite="mid:20190627093335.56355-3-roger.pau@citrix.com"> <pre class="moz-quote-pre" wrap="">diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile index 8a8d8f060f..94e6c9aee3 100644 --- a/xen/arch/x86/Makefile +++ b/xen/arch/x86/Makefile @@ -99,9 +99,14 @@ endif syms-warn-dup-y := --warn-dup syms-warn-dup-$(CONFIG_SUPPRESS_DUPLICATE_SYMBOL_WARNINGS) := +$(TARGET): TMP = $(@D)/.$(@F)</pre> </blockquote> <br> I'd suggest giving this a .elf32 suffix to make it clear which pass of the build it comes from.<br> <br> <blockquote type="cite" cite="mid:20190627093335.56355-3-roger.pau@citrix.com"> <pre class="moz-quote-pre" wrap=""> $(TARGET): $(TARGET)-syms $(efi-y) boot/mkelf32 - ./boot/mkelf32 $(notes_phdrs) $(TARGET)-syms $(TARGET) $(XEN_IMG_OFFSET) \ + ./boot/mkelf32 $(notes_phdrs) $(TARGET)-syms $(TMP) $(XEN_IMG_OFFSET) \ `$(NM) $(TARGET)-syms | sed -ne 's/^\([^ ]*\) . __2M_rwdata_end$$/0x\1/p'` + # Check for multiboot{1,2} headers + od -t x4 -N 8192 $(TMP) | grep 1badb002 > /dev/null || + od -t x4 -N 32768 $(TMP) | grep e85250d6 > /dev/null</pre> </blockquote> <br> This works, but <br> <br> Makefile:104: recipe for target '/local/xen.git/xen/xen' failed<br> <br> Isn't helpful to identify what went wrong. Sadly, we can't use $(error ...) in a shell snippet, but:<br> <br> <pre><a class="moz-txt-link-abbreviated" href="mailto:andrewcoop@andrewcoop:/local/xen.git/xen$">andrewcoop@andrewcoop:/local/xen.git/xen$</a> git d diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile index 94e6c9aee3..a1d6605a8b 100644 --- a/xen/arch/x86/Makefile +++ b/xen/arch/x86/Makefile @@ -99,13 +99,14 @@ endif syms-warn-dup-y := --warn-dup syms-warn-dup-$(CONFIG_SUPPRESS_DUPLICATE_SYMBOL_WARNINGS) := -$(TARGET): TMP = $(@D)/.$(@F) +$(TARGET): TMP = $(@D)/.$(@F).elf32 $(TARGET): $(TARGET)-syms $(efi-y) boot/mkelf32 ./boot/mkelf32 $(notes_phdrs) $(TARGET)-syms $(TMP) $(XEN_IMG_OFFSET) \ `$(NM) $(TARGET)-syms | sed -ne 's/^\([^ ]*\) . __2M_rwdata_end$$/0x\1/p'` - # Check for multiboot{1,2} headers - od -t x4 -N 8192 $(TMP) | grep 1badb002 > /dev/null - od -t x4 -N 32768 $(TMP) | grep e85250d6 > /dev/null + od -t x4 -N 8192 $(TMP) | grep 1badb002 > /dev/null || \ + { echo "No Multiboot1 header found"; false; } + od -t x4 -N 32768 $(TMP) | grep e85250d6 > /dev/null || \ + { echo "No Multiboot2 header found"; false; } mv $(TMP) $(TARGET) ALL_OBJS := $(BASEDIR)/arch/x86/boot/built_in.o $(BASEDIR)/arch/x86/efi/built_in.o $(ALL_OBJS) </pre> results in:<br> <br> No Multiboot1 header found<br> Makefile:104: recipe for target '/local/xen.git/xen/xen' failed<br> make[2]: *** [/local/xen.git/xen/xen] Error 1<br> Makefile:136: recipe for target '/local/xen.git/xen/xen' failed<br> make[1]: *** [/local/xen.git/xen/xen] Error 2<br> Makefile:45: recipe for target 'build' failed<br> make: *** [build] Error 2<br> <br> Which is far more clear.<br> <br> Thoughts?<br> <br> ~Andrew<br> </body> </html>
>>> On 27.06.19 at 13:51, <andrew.cooper3@citrix.com> wrote: > On 27/06/2019 10:33, Roger Pau Monne wrote: >> diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile >> index 8a8d8f060f..94e6c9aee3 100644 >> --- a/xen/arch/x86/Makefile >> +++ b/xen/arch/x86/Makefile >> @@ -99,9 +99,14 @@ endif >> syms-warn-dup-y := --warn-dup >> syms-warn-dup-$(CONFIG_SUPPRESS_DUPLICATE_SYMBOL_WARNINGS) := >> >> +$(TARGET): TMP = $(@D)/.$(@F) > > I'd suggest giving this a .elf32 suffix to make it clear which pass of > the build it comes from. > >> $(TARGET): $(TARGET)-syms $(efi-y) boot/mkelf32 >> - ./boot/mkelf32 $(notes_phdrs) $(TARGET)-syms $(TARGET) $(XEN_IMG_OFFSET) \ >> + ./boot/mkelf32 $(notes_phdrs) $(TARGET)-syms $(TMP) $(XEN_IMG_OFFSET) \ >> `$(NM) $(TARGET)-syms | sed -ne 's/^\([^ ]*\) . __2M_rwdata_end$$/0x\1/p'` >> + # Check for multiboot{1,2} headers >> + od -t x4 -N 8192 $(TMP) | grep 1badb002 > /dev/null || >> + od -t x4 -N 32768 $(TMP) | grep e85250d6 > /dev/null > > This works, but > > Makefile:104: recipe for target '/local/xen.git/xen/xen' failed > > Isn't helpful to identify what went wrong. Sadly, we can't use $(error > ...) in a shell snippet, I think we could: $(if $(shell od -t x4 -N 8192 $(TMP) | grep 1badb002),,$(error ...)exit 1) But I admit I didn't check whether it is well defined that only one of the last two operands of $(if ) get actually evaluated. > but: > > andrewcoop@andrewcoop:/local/xen.git/xen$ git d > diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile > index 94e6c9aee3..a1d6605a8b 100644 > --- a/xen/arch/x86/Makefile > +++ b/xen/arch/x86/Makefile > @@ -99,13 +99,14 @@ endif > syms-warn-dup-y := --warn-dup > syms-warn-dup-$(CONFIG_SUPPRESS_DUPLICATE_SYMBOL_WARNINGS) := > > -$(TARGET): TMP = $(@D)/.$(@F) > +$(TARGET): TMP = $(@D)/.$(@F).elf32 > $(TARGET): $(TARGET)-syms $(efi-y) boot/mkelf32 > ./boot/mkelf32 $(notes_phdrs) $(TARGET)-syms $(TMP) $(XEN_IMG_OFFSET) \ > `$(NM) $(TARGET)-syms | sed -ne 's/^\([^ ]*\) . __2M_rwdata_end$$/0x\1/p'` > - # Check for multiboot{1,2} headers > - od -t x4 -N 8192 $(TMP) | grep 1badb002 > /dev/null > - od -t x4 -N 32768 $(TMP) | grep e85250d6 > /dev/null > + od -t x4 -N 8192 $(TMP) | grep 1badb002 > /dev/null || \ > + { echo "No Multiboot1 header found"; false; } > + od -t x4 -N 32768 $(TMP) | grep e85250d6 > /dev/null || \ > + { echo "No Multiboot2 header found"; false; } > mv $(TMP) $(TARGET) > > ALL_OBJS := $(BASEDIR)/arch/x86/boot/built_in.o > $(BASEDIR)/arch/x86/efi/built_in.o $(ALL_OBJS) > > results in: > > No Multiboot1 header found > Makefile:104: recipe for target '/local/xen.git/xen/xen' failed > make[2]: *** [/local/xen.git/xen/xen] Error 1 > Makefile:136: recipe for target '/local/xen.git/xen/xen' failed > make[1]: *** [/local/xen.git/xen/xen] Error 2 > Makefile:45: recipe for target 'build' failed > make: *** [build] Error 2 > > Which is far more clear. > > Thoughts? Good idea. The only further request I have is to add >&2 to the echo commands (unless we go the $(error ) route anyway). Jan
On 27/06/2019 13:10, Jan Beulich wrote: >>>> On 27.06.19 at 13:51, <andrew.cooper3@citrix.com> wrote: >> On 27/06/2019 10:33, Roger Pau Monne wrote: >>> diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile >>> index 8a8d8f060f..94e6c9aee3 100644 >>> --- a/xen/arch/x86/Makefile >>> +++ b/xen/arch/x86/Makefile >>> @@ -99,9 +99,14 @@ endif >>> syms-warn-dup-y := --warn-dup >>> syms-warn-dup-$(CONFIG_SUPPRESS_DUPLICATE_SYMBOL_WARNINGS) := >>> >>> +$(TARGET): TMP = $(@D)/.$(@F) >> I'd suggest giving this a .elf32 suffix to make it clear which pass of >> the build it comes from. >> >>> $(TARGET): $(TARGET)-syms $(efi-y) boot/mkelf32 >>> - ./boot/mkelf32 $(notes_phdrs) $(TARGET)-syms $(TARGET) $(XEN_IMG_OFFSET) \ >>> + ./boot/mkelf32 $(notes_phdrs) $(TARGET)-syms $(TMP) $(XEN_IMG_OFFSET) \ >>> `$(NM) $(TARGET)-syms | sed -ne 's/^\([^ ]*\) . __2M_rwdata_end$$/0x\1/p'` >>> + # Check for multiboot{1,2} headers >>> + od -t x4 -N 8192 $(TMP) | grep 1badb002 > /dev/null || >>> + od -t x4 -N 32768 $(TMP) | grep e85250d6 > /dev/null >> This works, but >> >> Makefile:104: recipe for target '/local/xen.git/xen/xen' failed >> >> Isn't helpful to identify what went wrong. Sadly, we can't use $(error >> ...) in a shell snippet, > I think we could: > > $(if $(shell od -t x4 -N 8192 $(TMP) | grep 1badb002),,$(error ...)exit 1) > > But I admit I didn't check whether it is well defined that only one of > the last two operands of $(if ) get actually evaluated. Of the two options, I think the || { ; false } is clearer to follow. > >> but: >> >> andrewcoop@andrewcoop:/local/xen.git/xen$ git d >> diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile >> index 94e6c9aee3..a1d6605a8b 100644 >> --- a/xen/arch/x86/Makefile >> +++ b/xen/arch/x86/Makefile >> @@ -99,13 +99,14 @@ endif >> syms-warn-dup-y := --warn-dup >> syms-warn-dup-$(CONFIG_SUPPRESS_DUPLICATE_SYMBOL_WARNINGS) := >> >> -$(TARGET): TMP = $(@D)/.$(@F) >> +$(TARGET): TMP = $(@D)/.$(@F).elf32 >> $(TARGET): $(TARGET)-syms $(efi-y) boot/mkelf32 >> ./boot/mkelf32 $(notes_phdrs) $(TARGET)-syms $(TMP) $(XEN_IMG_OFFSET) \ >> `$(NM) $(TARGET)-syms | sed -ne 's/^\([^ ]*\) . __2M_rwdata_end$$/0x\1/p'` >> - # Check for multiboot{1,2} headers >> - od -t x4 -N 8192 $(TMP) | grep 1badb002 > /dev/null >> - od -t x4 -N 32768 $(TMP) | grep e85250d6 > /dev/null >> + od -t x4 -N 8192 $(TMP) | grep 1badb002 > /dev/null || \ >> + { echo "No Multiboot1 header found"; false; } >> + od -t x4 -N 32768 $(TMP) | grep e85250d6 > /dev/null || \ >> + { echo "No Multiboot2 header found"; false; } >> mv $(TMP) $(TARGET) >> >> ALL_OBJS := $(BASEDIR)/arch/x86/boot/built_in.o >> $(BASEDIR)/arch/x86/efi/built_in.o $(ALL_OBJS) >> >> results in: >> >> No Multiboot1 header found >> Makefile:104: recipe for target '/local/xen.git/xen/xen' failed >> make[2]: *** [/local/xen.git/xen/xen] Error 1 >> Makefile:136: recipe for target '/local/xen.git/xen/xen' failed >> make[1]: *** [/local/xen.git/xen/xen] Error 2 >> Makefile:45: recipe for target 'build' failed >> make: *** [build] Error 2 >> >> Which is far more clear. >> >> Thoughts? > Good idea. The only further request I have is to add >&2 to the > echo commands. Done. ~Andrew
On Thu, Jun 27, 2019 at 12:51:05PM +0100, Andrew Cooper wrote: > On 27/06/2019 10:33, Roger Pau Monne wrote: > > diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile > > index 8a8d8f060f..94e6c9aee3 100644 > > --- a/xen/arch/x86/Makefile > > +++ b/xen/arch/x86/Makefile > > @@ -99,9 +99,14 @@ endif > > syms-warn-dup-y := --warn-dup > > syms-warn-dup-$(CONFIG_SUPPRESS_DUPLICATE_SYMBOL_WARNINGS) := > > > > +$(TARGET): TMP = $(@D)/.$(@F) > > I'd suggest giving this a .elf32 suffix to make it clear which pass of > the build it comes from. That's fine, please also adjust the ignored list and the clean target. > > $(TARGET): $(TARGET)-syms $(efi-y) boot/mkelf32 > > - ./boot/mkelf32 $(notes_phdrs) $(TARGET)-syms $(TARGET) $(XEN_IMG_OFFSET) \ > > + ./boot/mkelf32 $(notes_phdrs) $(TARGET)-syms $(TMP) $(XEN_IMG_OFFSET) \ > > `$(NM) $(TARGET)-syms | sed -ne 's/^\([^ ]*\) . __2M_rwdata_end$$/0x\1/p'` > > + # Check for multiboot{1,2} headers > > + od -t x4 -N 8192 $(TMP) | grep 1badb002 > /dev/null || > > + od -t x4 -N 32768 $(TMP) | grep e85250d6 > /dev/null > > This works, but > > Makefile:104: recipe for target '/local/xen.git/xen/xen' failed > > Isn't helpful to identify what went wrong. Sadly, we can't use $(error > ...) in a shell snippet, but: > > andrewcoop@andrewcoop:/local/xen.git/xen$ git d > diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile > index 94e6c9aee3..a1d6605a8b 100644 > --- a/xen/arch/x86/Makefile > +++ b/xen/arch/x86/Makefile > @@ -99,13 +99,14 @@ endif > syms-warn-dup-y := --warn-dup > syms-warn-dup-$(CONFIG_SUPPRESS_DUPLICATE_SYMBOL_WARNINGS) := > > -$(TARGET): TMP = $(@D)/.$(@F) > +$(TARGET): TMP = $(@D)/.$(@F).elf32 > $(TARGET): $(TARGET)-syms $(efi-y) boot/mkelf32 > ./boot/mkelf32 $(notes_phdrs) $(TARGET)-syms $(TMP) $(XEN_IMG_OFFSET) \ > `$(NM) $(TARGET)-syms | sed -ne 's/^\([^ ]*\) . __2M_rwdata_end$$/0x\1/p'` > - # Check for multiboot{1,2} headers > - od -t x4 -N 8192 $(TMP) | grep 1badb002 > /dev/null > - od -t x4 -N 32768 $(TMP) | grep e85250d6 > /dev/null > + od -t x4 -N 8192 $(TMP) | grep 1badb002 > /dev/null || \ > + { echo "No Multiboot1 header found"; false; } > + od -t x4 -N 32768 $(TMP) | grep e85250d6 > /dev/null || \ > + { echo "No Multiboot2 header found"; false; } > mv $(TMP) $(TARGET) > > ALL_OBJS := $(BASEDIR)/arch/x86/boot/built_in.o $(BASEDIR)/arch/x86/efi/built_in.o $(ALL_OBJS) > > results in: > > No Multiboot1 header found > Makefile:104: recipe for target '/local/xen.git/xen/xen' failed > make[2]: *** [/local/xen.git/xen/xen] Error 1 > Makefile:136: recipe for target '/local/xen.git/xen/xen' failed > make[1]: *** [/local/xen.git/xen/xen] Error 2 > Makefile:45: recipe for target 'build' failed > make: *** [build] Error 2 > > Which is far more clear. > > Thoughts? Thanks, is indeed better. I also agree with Jan on the redirection to stderr. Roger.
diff --git a/.gitignore b/.gitignore index a77cbff02c..56bcb64837 100644 --- a/.gitignore +++ b/.gitignore @@ -278,6 +278,7 @@ tools/xentrace/xentrace xen/.banner xen/.config xen/.config.old +xen/.xen xen/System.map xen/arch/x86/asm-macros.i xen/arch/x86/boot/mkelf32 diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile index 8a8d8f060f..94e6c9aee3 100644 --- a/xen/arch/x86/Makefile +++ b/xen/arch/x86/Makefile @@ -99,9 +99,14 @@ endif syms-warn-dup-y := --warn-dup syms-warn-dup-$(CONFIG_SUPPRESS_DUPLICATE_SYMBOL_WARNINGS) := +$(TARGET): TMP = $(@D)/.$(@F) $(TARGET): $(TARGET)-syms $(efi-y) boot/mkelf32 - ./boot/mkelf32 $(notes_phdrs) $(TARGET)-syms $(TARGET) $(XEN_IMG_OFFSET) \ + ./boot/mkelf32 $(notes_phdrs) $(TARGET)-syms $(TMP) $(XEN_IMG_OFFSET) \ `$(NM) $(TARGET)-syms | sed -ne 's/^\([^ ]*\) . __2M_rwdata_end$$/0x\1/p'` + # Check for multiboot{1,2} headers + od -t x4 -N 8192 $(TMP) | grep 1badb002 > /dev/null + od -t x4 -N 32768 $(TMP) | grep e85250d6 > /dev/null + mv $(TMP) $(TARGET) ALL_OBJS := $(BASEDIR)/arch/x86/boot/built_in.o $(BASEDIR)/arch/x86/efi/built_in.o $(ALL_OBJS) @@ -249,7 +254,7 @@ efi/mkreloc: efi/mkreloc.c clean:: rm -f asm-offsets.s *.lds boot/*.o boot/*~ boot/core boot/mkelf32 rm -f asm-macros.i $(BASEDIR)/include/asm-x86/asm-macros.* - rm -f $(BASEDIR)/.xen-syms.[0-9]* boot/.*.d + rm -f $(BASEDIR)/.xen-syms.[0-9]* boot/.*.d $(BASEDIR)/.xen rm -f $(BASEDIR)/.xen.efi.[0-9]* efi/*.efi efi/mkreloc rm -f boot/cmdline.S boot/reloc.S boot/*.lnk boot/*.bin rm -f note.o
After building the hypervisor binary. Note that the check is performed by searching for the magic header value at the start of the binary. Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> --- Cc: Jan Beulich <jbeulich@suse.com> Cc: Andrew Cooper <andrew.cooper3@citrix.com> Cc: Wei Liu <wl@xen.org> --- Changes since v2: - Use a variable to store the intermediate file name. - Remove the intermediate file in the clean target. - Add intermediate file to gitignore. Changes since v1: - Use an intermediate file to perform the header checks. --- .gitignore | 1 + xen/arch/x86/Makefile | 9 +++++++-- 2 files changed, 8 insertions(+), 2 deletions(-)