diff mbox series

[4/4] x86: check for multiboot{1, 2} header presence

Message ID 20190619110250.18881-5-roger.pau@citrix.com (mailing list archive)
State New, archived
Headers show
Series x86: build with llvm 8 linker | expand

Commit Message

Roger Pau Monné June 19, 2019, 11:02 a.m. UTC
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>
---
 xen/arch/x86/Makefile | 3 +++
 1 file changed, 3 insertions(+)

Comments

Andrew Cooper June 19, 2019, 11:11 a.m. UTC | #1
On 19/06/2019 12:02, Roger Pau Monne 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>
> ---
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> Cc: Wei Liu <wl@xen.org>
> ---
>  xen/arch/x86/Makefile | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
> index 8a8d8f060f..9bb3bf6e6c 100644
> --- a/xen/arch/x86/Makefile
> +++ b/xen/arch/x86/Makefile
> @@ -102,6 +102,9 @@ syms-warn-dup-$(CONFIG_SUPPRESS_DUPLICATE_SYMBOL_WARNINGS) :=
>  $(TARGET): $(TARGET)-syms $(efi-y) boot/mkelf32
>  	./boot/mkelf32 $(notes_phdrs) $(TARGET)-syms $(TARGET) $(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 $(TARGET) | grep 1badb002 > /dev/null
> +	od -t x4 -N 32768 $(TARGET) | grep e85250d6 > /dev/null

Neat solution.  Is `grep -q` portable ?

~Andrew

>  
>  ALL_OBJS := $(BASEDIR)/arch/x86/boot/built_in.o $(BASEDIR)/arch/x86/efi/built_in.o $(ALL_OBJS)
>
Roger Pau Monné June 19, 2019, 11:20 a.m. UTC | #2
On Wed, Jun 19, 2019 at 12:11:43PM +0100, Andrew Cooper wrote:
> On 19/06/2019 12:02, Roger Pau Monne 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>
> > ---
> > Cc: Jan Beulich <jbeulich@suse.com>
> > Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> > Cc: Wei Liu <wl@xen.org>
> > ---
> >  xen/arch/x86/Makefile | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
> > index 8a8d8f060f..9bb3bf6e6c 100644
> > --- a/xen/arch/x86/Makefile
> > +++ b/xen/arch/x86/Makefile
> > @@ -102,6 +102,9 @@ syms-warn-dup-$(CONFIG_SUPPRESS_DUPLICATE_SYMBOL_WARNINGS) :=
> >  $(TARGET): $(TARGET)-syms $(efi-y) boot/mkelf32
> >  	./boot/mkelf32 $(notes_phdrs) $(TARGET)-syms $(TARGET) $(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 $(TARGET) | grep 1badb002 > /dev/null
> > +	od -t x4 -N 32768 $(TARGET) | grep e85250d6 > /dev/null
> 
> Neat solution.  Is `grep -q` portable ?

It is, but grep -q closes the pipe on the first match, and then od
will return an error. Note sure whether there's a way to workaround
this issue, but I think the above is simple enough.

Thanks, Roger.
Andrew Cooper June 19, 2019, 11:23 a.m. UTC | #3
On 19/06/2019 12:20, Roger Pau Monné wrote:
> On Wed, Jun 19, 2019 at 12:11:43PM +0100, Andrew Cooper wrote:
>> On 19/06/2019 12:02, Roger Pau Monne 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>
>>> ---
>>> Cc: Jan Beulich <jbeulich@suse.com>
>>> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
>>> Cc: Wei Liu <wl@xen.org>
>>> ---
>>>  xen/arch/x86/Makefile | 3 +++
>>>  1 file changed, 3 insertions(+)
>>>
>>> diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
>>> index 8a8d8f060f..9bb3bf6e6c 100644
>>> --- a/xen/arch/x86/Makefile
>>> +++ b/xen/arch/x86/Makefile
>>> @@ -102,6 +102,9 @@ syms-warn-dup-$(CONFIG_SUPPRESS_DUPLICATE_SYMBOL_WARNINGS) :=
>>>  $(TARGET): $(TARGET)-syms $(efi-y) boot/mkelf32
>>>  	./boot/mkelf32 $(notes_phdrs) $(TARGET)-syms $(TARGET) $(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 $(TARGET) | grep 1badb002 > /dev/null
>>> +	od -t x4 -N 32768 $(TARGET) | grep e85250d6 > /dev/null
>> Neat solution.  Is `grep -q` portable ?
> It is, but grep -q closes the pipe on the first match, and then od
> will return an error. Note sure whether there's a way to workaround
> this issue, but I think the above is simple enough.

In which case, Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Jan Beulich June 19, 2019, 1:08 p.m. UTC | #4
>>> On 19.06.19 at 13:02, <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>
> ---
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> Cc: Wei Liu <wl@xen.org>
> ---
>  xen/arch/x86/Makefile | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
> index 8a8d8f060f..9bb3bf6e6c 100644
> --- a/xen/arch/x86/Makefile
> +++ b/xen/arch/x86/Makefile
> @@ -102,6 +102,9 @@ 
> syms-warn-dup-$(CONFIG_SUPPRESS_DUPLICATE_SYMBOL_WARNINGS) :=
>  $(TARGET): $(TARGET)-syms $(efi-y) boot/mkelf32
>  	./boot/mkelf32 $(notes_phdrs) $(TARGET)-syms $(TARGET) $(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 $(TARGET) | grep 1badb002 > /dev/null
> +	od -t x4 -N 32768 $(TARGET) | grep e85250d6 > /dev/null

What's the behavior when a signature is _not _ found? Will
$(TARGET) get deleted (by make)? I don't think it would (as we
don't specific .DELETE_ON_ERROR anywhere), so a subsequent
rebuild may not even execute this rule, and hence may look to be
successful despite it not actually having been.

Jan
Roger Pau Monné June 19, 2019, 2:40 p.m. UTC | #5
On Wed, Jun 19, 2019 at 07:08:52AM -0600, Jan Beulich wrote:
> >>> On 19.06.19 at 13:02, <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>
> > ---
> > Cc: Jan Beulich <jbeulich@suse.com>
> > Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> > Cc: Wei Liu <wl@xen.org>
> > ---
> >  xen/arch/x86/Makefile | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
> > index 8a8d8f060f..9bb3bf6e6c 100644
> > --- a/xen/arch/x86/Makefile
> > +++ b/xen/arch/x86/Makefile
> > @@ -102,6 +102,9 @@ 
> > syms-warn-dup-$(CONFIG_SUPPRESS_DUPLICATE_SYMBOL_WARNINGS) :=
> >  $(TARGET): $(TARGET)-syms $(efi-y) boot/mkelf32
> >  	./boot/mkelf32 $(notes_phdrs) $(TARGET)-syms $(TARGET) $(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 $(TARGET) | grep 1badb002 > /dev/null
> > +	od -t x4 -N 32768 $(TARGET) | grep e85250d6 > /dev/null
> 
> What's the behavior when a signature is _not _ found? Will
> $(TARGET) get deleted (by make)? I don't think it would (as we
> don't specific .DELETE_ON_ERROR anywhere), so a subsequent
> rebuild may not even execute this rule, and hence may look to be
> successful despite it not actually having been.

Oh, right. It should be:

od -t x4 -N 8192 $(TARGET) | grep 1badb002 > /dev/null || (rm -rf $(TARGET); exit 1)
od -t x4 -N 32768 $(TARGET) | grep e85250d6 > /dev/null || (rm -rf $(TARGET); exit 1)

Would you be OK with the above runes?

Thanks, Roger.
Jan Beulich June 21, 2019, 6:24 a.m. UTC | #6
>>> On 19.06.19 at 16:40, <roger.pau@citrix.com> wrote:
> On Wed, Jun 19, 2019 at 07:08:52AM -0600, Jan Beulich wrote:
>> >>> On 19.06.19 at 13:02, <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>
>> > ---
>> > Cc: Jan Beulich <jbeulich@suse.com>
>> > Cc: Andrew Cooper <andrew.cooper3@citrix.com>
>> > Cc: Wei Liu <wl@xen.org>
>> > ---
>> >  xen/arch/x86/Makefile | 3 +++
>> >  1 file changed, 3 insertions(+)
>> > 
>> > diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
>> > index 8a8d8f060f..9bb3bf6e6c 100644
>> > --- a/xen/arch/x86/Makefile
>> > +++ b/xen/arch/x86/Makefile
>> > @@ -102,6 +102,9 @@ 
>> > syms-warn-dup-$(CONFIG_SUPPRESS_DUPLICATE_SYMBOL_WARNINGS) :=
>> >  $(TARGET): $(TARGET)-syms $(efi-y) boot/mkelf32
>> >  	./boot/mkelf32 $(notes_phdrs) $(TARGET)-syms $(TARGET) $(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 $(TARGET) | grep 1badb002 > /dev/null
>> > +	od -t x4 -N 32768 $(TARGET) | grep e85250d6 > /dev/null
>> 
>> What's the behavior when a signature is _not _ found? Will
>> $(TARGET) get deleted (by make)? I don't think it would (as we
>> don't specific .DELETE_ON_ERROR anywhere), so a subsequent
>> rebuild may not even execute this rule, and hence may look to be
>> successful despite it not actually having been.
> 
> Oh, right. It should be:
> 
> od -t x4 -N 8192 $(TARGET) | grep 1badb002 > /dev/null || (rm -rf $(TARGET); exit 1)
> od -t x4 -N 32768 $(TARGET) | grep e85250d6 > /dev/null || (rm -rf $(TARGET); exit 1)
> 
> Would you be OK with the above runes?

Not really, I'm afraid. This still wouldn't cope with an interrupted
build. I think it needs to be made work via an intermediate target,
or some other make trickery.

Jan
diff mbox series

Patch

diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
index 8a8d8f060f..9bb3bf6e6c 100644
--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -102,6 +102,9 @@  syms-warn-dup-$(CONFIG_SUPPRESS_DUPLICATE_SYMBOL_WARNINGS) :=
 $(TARGET): $(TARGET)-syms $(efi-y) boot/mkelf32
 	./boot/mkelf32 $(notes_phdrs) $(TARGET)-syms $(TARGET) $(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 $(TARGET) | grep 1badb002 > /dev/null
+	od -t x4 -N 32768 $(TARGET) | grep e85250d6 > /dev/null
 
 ALL_OBJS := $(BASEDIR)/arch/x86/boot/built_in.o $(BASEDIR)/arch/x86/efi/built_in.o $(ALL_OBJS)