Re: [Xen-devel] [PATCH v10 17/24] build_id: Provide ld-embedded build-ids

[Jan Beulich]

>>> On 02.05.16 at 13:19,  wrote:
> On Mon, May 02, 2016 at 05:13:24AM -0600, Jan Beulich wrote:
>> >>> On 02.05.16 at 13:05,  wrote:
>> > On Mon, May 02, 2016 at 12:02:43AM -0600, Jan Beulich wrote:
>> >> >>> On 29.04.16 at 19:23,  wrote:
>> >> > On Fri, Apr 29, 2016 at 10:38:07AM -0600, Jan Beulich wrote:
>> >> >> >>> On 27.04.16 at 21:27,  wrote:
>> >> >> > @@ -304,6 +338,32 @@ int main(int argc, char **argv)
>> >> >> >  /*mem_siz = (u32)in64_phdr.p_memsz;*/
>> >> >> >  mem_siz = (u32)(final_exec_addr - in64_phdr.p_vaddr);
>> >> >> >  
>> >> >> > +note_sz = note_base = offset = 0;
>> >> >> > +if ( num_phdrs > 1 )
>> >> >> > +{
>> >> >> > +offset = in64_phdr.p_offset;
>> >> >> > +note_base = in64_phdr.p_vaddr;
>> >> >> > +
>> >> >> > +(void)lseek(infd, in64_ehdr.e_phoff+sizeof(in64_phdr), 
>> >> >> > SEEK_SET);
>> >> >> > +do_read(infd, _phdr, sizeof(in64_phdr));
>> >> >> > +endianadjust_phdr64(_phdr);
>> >> >> > +
>> >> >> > +(void)lseek(infd, offset, SEEK_SET);
>> >> >> > +
>> >> >> > +note_sz = in64_phdr.p_memsz;
>> >> >> > +note_base = in64_phdr.p_vaddr - note_base;
>> >> >> > +
>> >> >> > +if ( in64_phdr.p_offset > dat_siz || offset > 
>> >> >> > in64_phdr.p_offset )
>> >> >> > +{
>> >> >> > +fprintf(stderr, "Expected .note section within .text 
>> >> > section!\n" \
>> >> >> > +"Offset %ld not within %d!\n",
>> >> >> > +in64_phdr.p_offset, dat_siz);
>> >> >> 
>> >> >> This fails to build on a 32-bit build host (which is one of the two
>> >> >> post-commit, pre-push checks I normally do).
>> >> > 
>> >> > I hadn't realized that it was possible to build an 64-bit hypervisor on 
>> >> > 32-bit
>> >> > GCC toolchain. I've never done that - always built the hypervisor in 
>> >> > 64-bit 
>> >> > env and the 32-bit toolstack in 32-bit environment. Then booted it.
>> >> 
>> >> 32-bit toolchain? No. A 64-bit cross tool chain (similar to what I use
>> >> for ARM build testing, except that here I also actively run the
>> >> resulting hypervisor).
>> > 
>> > Then I'm a bit confused what you meant by "32-bit build host" in your
>> > previous email.
>> 
>> What's confusing you here? Running a 64-bit hypervisor and/or a
>> 64-bit kernel underneath a 32-bit distro is working quite fine.
>> 
> 
> Hmm... How did you discover that problem if you did not cross-compile
> with 32-bit toolchain? That's how I discovered the breakage.

Hmm, so maybe we're just meaning "32-bit" for different things: I
imply it to stand for the target architecture, but maybe you mean
the (build) host architecture instead. Konrad's original reply meant
to me that he somehow expected the _native_ tool chain on a
32-bit host to be capable of that (where host arch == target arch),
which in theory was possible too, but I don't think is in actual use
anywhere.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v10 17/24] build_id: Provide ld-embedded build-ids

[Wei Liu]

On Mon, May 02, 2016 at 05:13:24AM -0600, Jan Beulich wrote:
> >>> On 02.05.16 at 13:05,  wrote:
> > On Mon, May 02, 2016 at 12:02:43AM -0600, Jan Beulich wrote:
> >> >>> On 29.04.16 at 19:23,  wrote:
> >> > On Fri, Apr 29, 2016 at 10:38:07AM -0600, Jan Beulich wrote:
> >> >> >>> On 27.04.16 at 21:27,  wrote:
> >> >> > @@ -304,6 +338,32 @@ int main(int argc, char **argv)
> >> >> >  /*mem_siz = (u32)in64_phdr.p_memsz;*/
> >> >> >  mem_siz = (u32)(final_exec_addr - in64_phdr.p_vaddr);
> >> >> >  
> >> >> > +note_sz = note_base = offset = 0;
> >> >> > +if ( num_phdrs > 1 )
> >> >> > +{
> >> >> > +offset = in64_phdr.p_offset;
> >> >> > +note_base = in64_phdr.p_vaddr;
> >> >> > +
> >> >> > +(void)lseek(infd, in64_ehdr.e_phoff+sizeof(in64_phdr), 
> >> >> > SEEK_SET);
> >> >> > +do_read(infd, _phdr, sizeof(in64_phdr));
> >> >> > +endianadjust_phdr64(_phdr);
> >> >> > +
> >> >> > +(void)lseek(infd, offset, SEEK_SET);
> >> >> > +
> >> >> > +note_sz = in64_phdr.p_memsz;
> >> >> > +note_base = in64_phdr.p_vaddr - note_base;
> >> >> > +
> >> >> > +if ( in64_phdr.p_offset > dat_siz || offset > 
> >> >> > in64_phdr.p_offset )
> >> >> > +{
> >> >> > +fprintf(stderr, "Expected .note section within .text 
> >> > section!\n" \
> >> >> > +"Offset %ld not within %d!\n",
> >> >> > +in64_phdr.p_offset, dat_siz);
> >> >> 
> >> >> This fails to build on a 32-bit build host (which is one of the two
> >> >> post-commit, pre-push checks I normally do).
> >> > 
> >> > I hadn't realized that it was possible to build an 64-bit hypervisor on 
> >> > 32-bit
> >> > GCC toolchain. I've never done that - always built the hypervisor in 
> >> > 64-bit 
> >> > env and the 32-bit toolstack in 32-bit environment. Then booted it.
> >> 
> >> 32-bit toolchain? No. A 64-bit cross tool chain (similar to what I use
> >> for ARM build testing, except that here I also actively run the
> >> resulting hypervisor).
> > 
> > Then I'm a bit confused what you meant by "32-bit build host" in your
> > previous email.
> 
> What's confusing you here? Running a 64-bit hypervisor and/or a
> 64-bit kernel underneath a 32-bit distro is working quite fine.
> 

Hmm... How did you discover that problem if you did not cross-compile
with 32-bit toolchain? That's how I discovered the breakage.

Actually never mind. It's probably counter-productive to quibble over
words we use to describe build setups, especially ...

> > Anyway,  does my patch ("mkelf32: fix compilation on 32 bit build host")
> > fix the problem you saw?
> 
> Yes. That was also what I had used as a temporary workaround. I
> just didn't have the time right away to put this into proper patch
> shape.
> 

... now that you confirm the build is fixed.

Thanks for confirming BTW.

Wei.

> Jan
> 

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v10 17/24] build_id: Provide ld-embedded build-ids

[Jan Beulich]

>>> On 02.05.16 at 13:05,  wrote:
> On Mon, May 02, 2016 at 12:02:43AM -0600, Jan Beulich wrote:
>> >>> On 29.04.16 at 19:23,  wrote:
>> > On Fri, Apr 29, 2016 at 10:38:07AM -0600, Jan Beulich wrote:
>> >> >>> On 27.04.16 at 21:27,  wrote:
>> >> > @@ -304,6 +338,32 @@ int main(int argc, char **argv)
>> >> >  /*mem_siz = (u32)in64_phdr.p_memsz;*/
>> >> >  mem_siz = (u32)(final_exec_addr - in64_phdr.p_vaddr);
>> >> >  
>> >> > +note_sz = note_base = offset = 0;
>> >> > +if ( num_phdrs > 1 )
>> >> > +{
>> >> > +offset = in64_phdr.p_offset;
>> >> > +note_base = in64_phdr.p_vaddr;
>> >> > +
>> >> > +(void)lseek(infd, in64_ehdr.e_phoff+sizeof(in64_phdr), 
>> >> > SEEK_SET);
>> >> > +do_read(infd, _phdr, sizeof(in64_phdr));
>> >> > +endianadjust_phdr64(_phdr);
>> >> > +
>> >> > +(void)lseek(infd, offset, SEEK_SET);
>> >> > +
>> >> > +note_sz = in64_phdr.p_memsz;
>> >> > +note_base = in64_phdr.p_vaddr - note_base;
>> >> > +
>> >> > +if ( in64_phdr.p_offset > dat_siz || offset > 
>> >> > in64_phdr.p_offset )
>> >> > +{
>> >> > +fprintf(stderr, "Expected .note section within .text 
>> > section!\n" \
>> >> > +"Offset %ld not within %d!\n",
>> >> > +in64_phdr.p_offset, dat_siz);
>> >> 
>> >> This fails to build on a 32-bit build host (which is one of the two
>> >> post-commit, pre-push checks I normally do).
>> > 
>> > I hadn't realized that it was possible to build an 64-bit hypervisor on 
>> > 32-bit
>> > GCC toolchain. I've never done that - always built the hypervisor in 
>> > 64-bit 
>> > env and the 32-bit toolstack in 32-bit environment. Then booted it.
>> 
>> 32-bit toolchain? No. A 64-bit cross tool chain (similar to what I use
>> for ARM build testing, except that here I also actively run the
>> resulting hypervisor).
> 
> Then I'm a bit confused what you meant by "32-bit build host" in your
> previous email.

What's confusing you here? Running a 64-bit hypervisor and/or a
64-bit kernel underneath a 32-bit distro is working quite fine.

> Anyway,  does my patch ("mkelf32: fix compilation on 32 bit build host")
> fix the problem you saw?

Yes. That was also what I had used as a temporary workaround. I
just didn't have the time right away to put this into proper patch
shape.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v10 17/24] build_id: Provide ld-embedded build-ids

[Wei Liu]

On Mon, May 02, 2016 at 12:02:43AM -0600, Jan Beulich wrote:
> >>> On 29.04.16 at 19:23,  wrote:
> > On Fri, Apr 29, 2016 at 10:38:07AM -0600, Jan Beulich wrote:
> >> >>> On 27.04.16 at 21:27,  wrote:
> >> > @@ -304,6 +338,32 @@ int main(int argc, char **argv)
> >> >  /*mem_siz = (u32)in64_phdr.p_memsz;*/
> >> >  mem_siz = (u32)(final_exec_addr - in64_phdr.p_vaddr);
> >> >  
> >> > +note_sz = note_base = offset = 0;
> >> > +if ( num_phdrs > 1 )
> >> > +{
> >> > +offset = in64_phdr.p_offset;
> >> > +note_base = in64_phdr.p_vaddr;
> >> > +
> >> > +(void)lseek(infd, in64_ehdr.e_phoff+sizeof(in64_phdr), 
> >> > SEEK_SET);
> >> > +do_read(infd, _phdr, sizeof(in64_phdr));
> >> > +endianadjust_phdr64(_phdr);
> >> > +
> >> > +(void)lseek(infd, offset, SEEK_SET);
> >> > +
> >> > +note_sz = in64_phdr.p_memsz;
> >> > +note_base = in64_phdr.p_vaddr - note_base;
> >> > +
> >> > +if ( in64_phdr.p_offset > dat_siz || offset > 
> >> > in64_phdr.p_offset )
> >> > +{
> >> > +fprintf(stderr, "Expected .note section within .text 
> > section!\n" \
> >> > +"Offset %ld not within %d!\n",
> >> > +in64_phdr.p_offset, dat_siz);
> >> 
> >> This fails to build on a 32-bit build host (which is one of the two
> >> post-commit, pre-push checks I normally do).
> > 
> > I hadn't realized that it was possible to build an 64-bit hypervisor on 
> > 32-bit
> > GCC toolchain. I've never done that - always built the hypervisor in 64-bit 
> > env and the 32-bit toolstack in 32-bit environment. Then booted it.
> 
> 32-bit toolchain? No. A 64-bit cross tool chain (similar to what I use
> for ARM build testing, except that here I also actively run the
> resulting hypervisor).
> 

Then I'm a bit confused what you meant by "32-bit build host" in your
previous email.

Anyway,  does my patch ("mkelf32: fix compilation on 32 bit build host")
fix the problem you saw?

Wei.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v10 17/24] build_id: Provide ld-embedded build-ids

[Konrad Rzeszutek Wilk]

On Fri, Apr 29, 2016 at 10:38:07AM -0600, Jan Beulich wrote:
> >>> On 27.04.16 at 21:27,  wrote:
> > @@ -304,6 +338,32 @@ int main(int argc, char **argv)
> >  /*mem_siz = (u32)in64_phdr.p_memsz;*/
> >  mem_siz = (u32)(final_exec_addr - in64_phdr.p_vaddr);
> >  
> > +note_sz = note_base = offset = 0;
> > +if ( num_phdrs > 1 )
> > +{
> > +offset = in64_phdr.p_offset;
> > +note_base = in64_phdr.p_vaddr;
> > +
> > +(void)lseek(infd, in64_ehdr.e_phoff+sizeof(in64_phdr), SEEK_SET);
> > +do_read(infd, _phdr, sizeof(in64_phdr));
> > +endianadjust_phdr64(_phdr);
> > +
> > +(void)lseek(infd, offset, SEEK_SET);
> > +
> > +note_sz = in64_phdr.p_memsz;
> > +note_base = in64_phdr.p_vaddr - note_base;
> > +
> > +if ( in64_phdr.p_offset > dat_siz || offset > in64_phdr.p_offset )
> > +{
> > +fprintf(stderr, "Expected .note section within .text 
> > section!\n" \
> > +"Offset %ld not within %d!\n",
> > +in64_phdr.p_offset, dat_siz);
> 
> This fails to build on a 32-bit build host (which is one of the two
> post-commit, pre-push checks I normally do).

I hadn't realized that it was possible to build an 64-bit hypervisor on 32-bit
GCC toolchain. I've never done that - always built the hypervisor in 64-bit env
and the 32-bit toolstack in 32-bit environment. Then booted it.

Anyhow, Wei beat me on IRC with the patch and will be sending it shortly!

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v10 17/24] build_id: Provide ld-embedded build-ids

[Jan Beulich]

>>> On 27.04.16 at 21:27,  wrote:
> @@ -304,6 +338,32 @@ int main(int argc, char **argv)
>  /*mem_siz = (u32)in64_phdr.p_memsz;*/
>  mem_siz = (u32)(final_exec_addr - in64_phdr.p_vaddr);
>  
> +note_sz = note_base = offset = 0;
> +if ( num_phdrs > 1 )
> +{
> +offset = in64_phdr.p_offset;
> +note_base = in64_phdr.p_vaddr;
> +
> +(void)lseek(infd, in64_ehdr.e_phoff+sizeof(in64_phdr), SEEK_SET);
> +do_read(infd, _phdr, sizeof(in64_phdr));
> +endianadjust_phdr64(_phdr);
> +
> +(void)lseek(infd, offset, SEEK_SET);
> +
> +note_sz = in64_phdr.p_memsz;
> +note_base = in64_phdr.p_vaddr - note_base;
> +
> +if ( in64_phdr.p_offset > dat_siz || offset > in64_phdr.p_offset )
> +{
> +fprintf(stderr, "Expected .note section within .text section!\n" 
> \
> +"Offset %ld not within %d!\n",
> +in64_phdr.p_offset, dat_siz);

This fails to build on a 32-bit build host (which is one of the two
post-commit, pre-push checks I normally do).

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [PATCH v10 17/24] build_id: Provide ld-embedded build-ids

[Konrad Rzeszutek Wilk]

This patch enables the Elf to be built with the build-id
and provide in the Xen hypervisor the code to extract it.

The man-page for ld --build-id says it is:

"Request the creation of a ".note.gnu.build-id" ELF note
section or a ".build-id" COFF section.  The contents of the
note are unique bits identifying this linked file. style can be
"uuid" to use 128 random bits, "sha1" to use a 160-bit SHA1 hash
on the normative parts of the output contents, ..."

One can also retrieve the value of the build-id by doing
'readelf -n xen-syms'.

For EFI builds we re-use the same build-id that the xen-syms
was built with.

The version of ld that first implemented --build-id is v2.18.
We check for to see if the linker supports the --build-id
parameter and if so use it.

For x86 we have two binaries - the xen-syms and the xen - an
smaller version with lots of sections removed. To make it possible
for readelf -n xen we also modify mkelf32 and xen.lds.S to include
the PT_NOTE ELF section.

The EFI binary is more complicated. We only build one type of
binary and expanding the amount of sections the EFI binary has to
include an .note one is pointless - as there is no concept of
PT_NOTE. The best we can do is move this .note in the .rodata section.

Further development wise should move it to .buildid section
so that DataDirectory debug data nor CodeView can view it.
(The author has no clue what those are).

Note that in earlier patches the linker script had:

 __note_gnu_build_id_start = .;
 *(.rodata.note.gnu.build-id)
 __note_gnu_build_id_end = .;
 *(.note)
 *(.note.*)

Which meant you could have different ELF notes _outside_ the
__note_gnu_build_id_end. However for EFI builds we take the whole
.note* section and jam it in the EFI to be between
__note_gnu_build_id_start and __note_gnu_build_id_end.
To not make this happend we make on the ELF build the section
be called .note.gnu.build-id  (instead of just .note).
If there is a need for a different type of note other folks
can add it as a different section name.

Note that we do call --binary-id=sha1 on all linker invocations.
We have to do to enforce that the symbol offsets don't changes
(the side effect is that we we would have multiple binary ids -
except that the last one is the final one).

Without this working the symbol table embedded in Xen ends
up incorrect - some of the values it contains would be offset by the
size of the included build id.

This obviously causes problems when resolving symbols.

We also define the NT_GNU_BUILD_ID in the elfstructs.h as we
need to use it in various places.

Suggested-by: Andrew Cooper 
Signed-off-by: Martin Pohlack 
Signed-off-by: Konrad Rzeszutek Wilk 
Acked-by: Julien Grall 
Reviewed-by: Andrew Cooper 
Acked-by: Jan Beulich 

---
Cc: Stefano Stabellini 
Cc: Julien Grall 
Cc: Keir Fraser 
Cc: Jan Beulich 
Cc: Andrew Cooper 

v1: Rebase it on Martin's initial patch
v2: Move it to XENVER hypercall
v3: Fix EFI building (Ross's fix)
Don't use the third argument for length.
Use new structure for XENVER_build_id with variable buf.
Include Ross's fix.
Include detection of bin-utils for build-id support, add
probing for size, and return -EPERM for XSM denied calls.
Build xen_build_id under ARM, required adding ELFSIZE in proper file.
Rebase on top XSM version class.
v4:
Include the build-id .note in the xen ELF binary.
s/build_id/build_id_linker/
For EFI build, moved the --build-id values in .data section
Rebase on staging.
Split patch in two. Always do --build-id call. Include the .note in
.rodata. USe const void * and ssize_t
Use -S to make build_id.o and objcopy differently (Andrew suggested)
v5: Put back the #ifdef LOCK_PROFILE on ARM. (Bad change). Move the _erodata
around. s/ssize_t/unsigned int/
v6: Redid it per Jan's review.
v7: Move build-id note in .rodata.note for EFI builds only.
Move build-id note in .rodata for EFI builds only. Retain
it in .note. Change name of object file used by EFI builds to notes.o
Make on ELF builds the PT_NOTE section name be .note.gnu.build-id and
ingest that in ELF build.
Define NT_GNU_BUILD_ID in elfstructs.h
v8: s/num_phdrs/notes_phdrs/
   Added Andrew's Reviewed-by
v9:
   Made mkelf32 parse --notes as first argument
   Moved _erodata past .note.gnu.. section
   Added rm -f note.o on clean target.
   Make build_id_[p|len] be __read_mostly
   Add Jan's Acked-by
   Made the detection be smarter and just check for option requested being 
mirrored.
   (That takes care of checking unrecognized in different languages).
---
 Config.mk|  11 
 xen/arch/arm/Makefile|   2 +-
 xen/arch/arm/xen.lds.S   |  15 -
 xen/arch/x86/Makefile|  30