[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v3 2/3] XENVER_build_id: Provide ld-embedded build-ids (v8)
On Tue, Jan 12, 2016 at 09:25:27AM -0700, Jan Beulich wrote: > >>> On 08.01.16 at 03:25, <konrad.wilk@xxxxxxxxxx> wrote: > > The mechanism to get this is via the XENVER hypercall and > > we add a new sub-command to retrieve the binary build-id > > called XENVER_build_id. The sub-hypercall parameter > > allows an arbitrary size (the buffer and len is provided > > to the hypervisor). A NULL parameter will probe the hypervisor > > for the length of the build-id. > > > > One can also retrieve the value of the build-id by doing > > 'readelf -h xen-syms'. > > Does this or something similar also work on xen.gz and xen.efi? Sadly no. > I ask because xen-syms isn't present on the boot partition, and > may not be present anywhere on an otherwise functioning > system. Right. Some later patches (xsplice related) hook the build-id printing to a keyboard handler. Would that work ? Or are you suggesting that perhaps the kernel should at boot time print the build-id (like it does the changset)? > > > --- a/Config.mk > > +++ b/Config.mk > > @@ -126,6 +126,17 @@ endef > > check-$(gcc) = $(call cc-ver-check,CC,0x040100,"Xen requires at least > > gcc-4.1") > > $(eval $(check-y)) > > > > +ld-ver = $(shell if [ $$((`$(1) --version | head -1 | sed 's/[^0-9]/ /g' | > > awk \ > > + '{ printf "0x%02x%02x", $$1, $$2}'`)) -ge $$(($(2))) ]; \ > > + then echo y; else echo n; fi ;) > > + > > +# binutils 2.18 implement build-id. > > +ifeq ($(call ld-ver,$(LD),0x0212),n) > > +build_id := > > +else > > +build_id := --build-id=sha1 > > +endif > > Wouldn't it be better to probe the linker for recognizing the --build-id > command line option, along the lines of $(cc-option)? Let me see what happens with that. > > In any event the option should be added to LDFLAGS unless this > conflicts with something else. > > > --- a/xen/arch/x86/xen.lds.S > > +++ b/xen/arch/x86/xen.lds.S > > @@ -67,6 +67,12 @@ SECTIONS > > *(.rodata.*) > > } :text > > > > + .note.gnu.build-id : { > > + __note_gnu_build_id_start = .; > > + *(.note.gnu.build-id) > > + __note_gnu_build_id_end = .; > > + } :text > > Wouldn't this better be a generic .note section, with .note.gnu.build-id > just special cased ahead of *(.note) *(.note.*)? > > > --- a/xen/common/kernel.c > > +++ b/xen/common/kernel.c > > @@ -366,6 +366,41 @@ DO(xen_version)(int cmd, XEN_GUEST_HANDLE_PARAM(void) > > arg) > > return -EFAULT; > > return 0; > > } > > + case XENVER_build_id: > > Blank line ahead of case statements please when most/all others > have it that way in a particular switch(). > > > + { > > + xen_build_id_t build_id; > > + unsigned int sz = 0; > > + int rc = 0; > > + char *p = NULL; > > + > > + if ( deny ) > > + return -EPERM; > > + > > + /* Only return size. */ > > + if ( !guest_handle_is_null(arg) ) > > + { > > + if ( copy_from_guest(&build_id, arg, 1) ) > > + return -EFAULT; > > + > > + if ( build_id.len == 0 ) > > + return -EINVAL; > > + } > > + > > + rc = xen_build_id(&p, &sz); > > Perhaps use the helpers from xen/err.h to limit the amount of > indirection needed? > > > + if ( rc ) > > + return rc; > > + > > + if ( guest_handle_is_null(arg) ) > > + return sz; > > + > > + if ( sz > build_id.len ) > > + return -ENOBUFS; > > And how will the caller know how much is needed? Duh. I shall update the build_id.len with the appropiate value. > > > + if ( copy_to_guest_offset(arg, offsetof(xen_build_id_t, buf), p, > > sz) ) > > + return -EFAULT; > > + > > + return sz; > > + } > > Or how much got copied? That one is easy - we do return 'sz' which tells us how much got copied. Or are you suggesting that the build_id.len should also be updated? > > > +int xen_build_id(char **p, unsigned int *len) > > +{ > > + const Elf_Note *n = __note_gnu_build_id_start; > > + static bool_t checked = 0; > > Pointless initializer. > > Jan > _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |