[PATCH v3] Add build-id to XENVER hypercall.

[PATCH v3] Add build-id to XENVER hypercall.

[Konrad Rzeszutek Wilk]

Since v2 (http://lists.xen.org/archives/html/xen-devel/2015-11/msg00630.html)
 - Made it work under ARM (build_id is part of the hypervisor).
 - Only made two sub-ops be 'priv' - commandline and build_id.
 - Applied all review comments.
 - Autodetect whether ld is able to use --build-id

v1 (http://lists.xen.org/archives/html/xen-devel/2015-10/msg01090.html)
 - Made it work on EFI
 - Compiles on ARM
 - Redid it per comments.

Attached are the three patches that will add XENVER_build_id and
add the proper bits in libxl/libxc.

However they also change the behavior of the existing hypercall
for XENVER_commandline - to return the string '<denied>' for non-priv
guests. This is with XSM enabled or disabled.

The new sub-op - XENVER_build_id on the other hand will return
-EPERM (XSM or not) for !priv guests.

Please take a look and provide your feedback at your leisure.

The patches are also at my git tree:

git://xenbits.xen.org/people/konradwilk/xen.git xenver.v3


 Config.mk                                    | 11 +++++++
 tools/flask/policy/policy/modules/xen/xen.te |  4 +++
 tools/libxc/xc_private.c                     |  7 ++++
 tools/libxc/xc_private.h                     | 10 ++++++
 tools/libxl/libxl.c                          | 24 ++++++++++++++
 tools/libxl/libxl.h                          |  5 +++
 tools/libxl/libxl_types.idl                  |  1 +
 tools/libxl/xl_cmdimpl.c                     |  1 +
 xen/arch/arm/Makefile                        |  6 ++--
 xen/arch/arm/xen.lds.S                       |  6 ++++
 xen/arch/x86/Makefile                        | 16 +++++++---
 xen/arch/x86/xen.lds.S                       |  6 ++++
 xen/common/kernel.c                          | 48 ++++++++++++++++++++++++++--
 xen/common/version.c                         | 47 +++++++++++++++++++++++++++
 xen/include/asm-arm/config.h                 |  2 ++
 xen/include/public/version.h                 | 16 +++++++++-
 xen/include/xen/version.h                    |  2 ++
 xen/include/xsm/dummy.h                      | 21 ++++++++++++
 xen/include/xsm/xsm.h                        |  5 +++
 xen/xsm/dummy.c                              |  1 +
 xen/xsm/flask/hooks.c                        | 24 ++++++++++++++
 xen/xsm/flask/policy/access_vectors          |  2 ++
 22 files changed, 254 insertions(+), 11 deletions(-)


Konrad Rzeszutek Wilk (3):
      xsm/xen_version: Add XSM for the xen_version hypercall (v6).
      XENVER_build_id: Provide ld-embedded build-ids (v8)
      libxl: info: Display build_id of the hypervisor.

[PATCH v3 1/3] xsm/xen_version: Add XSM for the xen_version hypercall (v6).

[Konrad Rzeszutek Wilk]

All of XENVER_* have now an XSM check.

The subop for XENVER_commandline is now a priviliged operation.
To not break guests we still return an string - but it is
just '<denied>\0'.

The rest: XENVER_[version|extraversion|capabilities|
parameters|get_features|page_size|guest_handle|changeset|
compile_info] behave as before - allowed by default for all guests.

This is with the XSM default (and non-default) policy and with
the dummy ones.

Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
v2: Do XSM check for all the XENVER_ ops.
v3: Add empty data conditions.
v4: Return <denied> for priv subops.
v5: Move extraversion from priv to normal. Drop the XSM check
    for the non-priv subops.
v6: Add +1 for strlen(xen_deny()) to include NULL. Move changeset,
    compile_info to non-priv subops.
---
 tools/flask/policy/policy/modules/xen/xen.te |  4 ++++
 xen/common/kernel.c                          | 13 +++++++++++--
 xen/common/version.c                         |  5 +++++
 xen/include/xen/version.h                    |  1 +
 xen/include/xsm/dummy.h                      | 21 +++++++++++++++++++++
 xen/include/xsm/xsm.h                        |  5 +++++
 xen/xsm/dummy.c                              |  1 +
 xen/xsm/flask/hooks.c                        | 24 ++++++++++++++++++++++++
 xen/xsm/flask/policy/access_vectors          |  2 ++
 9 files changed, 74 insertions(+), 2 deletions(-)

diff --git a/tools/flask/policy/policy/modules/xen/xen.te b/tools/flask/policy/policy/modules/xen/xen.te
index d35ae22..17f304e 100644
--- a/tools/flask/policy/policy/modules/xen/xen.te
+++ b/tools/flask/policy/policy/modules/xen/xen.te
@@ -73,6 +73,10 @@ allow dom0_t xen_t:xen2 {
     pmu_ctrl
     get_symbol
 };
+
+# Allow dom0 to use XENVER_commandline
+allow dom0_t xen_t:xen2 version_priv;
+
 allow dom0_t xen_t:mmu memorymap;
 
 # Allow dom0 to use these domctls on itself. For domctls acting on other
diff --git a/xen/common/kernel.c b/xen/common/kernel.c
index 6a3196a..2b3ccc4 100644
--- a/xen/common/kernel.c
+++ b/xen/common/kernel.c
@@ -13,6 +13,7 @@
 #include <xen/nmi.h>
 #include <xen/guest_access.h>
 #include <xen/hypercall.h>
+#include <xsm/xsm.h>
 #include <asm/current.h>
 #include <public/nmi.h>
 #include <public/version.h>
@@ -226,9 +227,10 @@ void __init do_initcalls(void)
 /*
  * Simple hypercalls.
  */
-
 DO(xen_version)(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
 {
+    bool_t deny = !!xsm_version_op(XSM_OTHER, cmd);
+
     switch ( cmd )
     {
     case XENVER_version:
@@ -354,10 +356,17 @@ DO(xen_version)(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
         return 0;
 
     case XENVER_commandline:
-        if ( copy_to_guest(arg, saved_cmdline, ARRAY_SIZE(saved_cmdline)) )
+    {
+        size_t len = ARRAY_SIZE(saved_cmdline);
+
+        if ( deny )
+            len = strlen(xen_deny()) + 1;
+
+        if ( copy_to_guest(arg, deny ? xen_deny() : saved_cmdline, len) )
             return -EFAULT;
         return 0;
     }
+    }
 
     return -ENOSYS;
 }
diff --git a/xen/common/version.c b/xen/common/version.c
index b152e27..95332a0 100644
--- a/xen/common/version.c
+++ b/xen/common/version.c
@@ -55,3 +55,8 @@ const char *xen_banner(void)
 {
     return XEN_BANNER;
 }
+
+const char *xen_deny(void)
+{
+    return "<denied>\0";
+}
diff --git a/xen/include/xen/version.h b/xen/include/xen/version.h
index 81a3c7d..2015c0b 100644
--- a/xen/include/xen/version.h
+++ b/xen/include/xen/version.h
@@ -12,5 +12,6 @@ unsigned int xen_minor_version(void);
 const char *xen_extra_version(void);
 const char *xen_changeset(void);
 const char *xen_banner(void);
+const char *xen_deny(void);
 
 #endif /* __XEN_VERSION_H__ */
diff --git a/xen/include/xsm/dummy.h b/xen/include/xsm/dummy.h
index 55b84f0..3f3614e 100644
--- a/xen/include/xsm/dummy.h
+++ b/xen/include/xsm/dummy.h
@@ -721,3 +721,24 @@ static XSM_INLINE int xsm_pmu_op (XSM_DEFAULT_ARG struct domain *d, unsigned int
 }
 
 #endif /* CONFIG_X86 */
+
+#include <public/version.h>
+static XSM_INLINE int xsm_version_op (XSM_DEFAULT_ARG uint32_t op)
+{
+    XSM_ASSERT_ACTION(XSM_OTHER);
+    switch ( op )
+    {
+    case XENVER_version:
+    case XENVER_extraversion:
+    case XENVER_compile_info:
+    case XENVER_capabilities:
+    case XENVER_changeset:
+    case XENVER_platform_parameters:
+    case XENVER_get_features:
+    case XENVER_pagesize:
+    case XENVER_guest_handle:
+        return 0; /* These MUST always be accessible to any guest. */
+    default:
+        return xsm_default_action(XSM_PRIV, current->domain, NULL);
+    }
+}
diff --git a/xen/include/xsm/xsm.h b/xen/include/xsm/xsm.h
index 2c365cd..64f1fa3 100644
--- a/xen/include/xsm/xsm.h
+++ b/xen/include/xsm/xsm.h
@@ -192,6 +192,7 @@ struct xsm_operations {
     int (*ioport_mapping) (struct domain *d, uint32_t s, uint32_t e, uint8_t allow);
     int (*pmu_op) (struct domain *d, unsigned int op);
 #endif
+    int (*version_op) (uint32_t cmd);
 };
 
 #ifdef CONFIG_XSM
@@ -730,6 +731,10 @@ static inline int xsm_pmu_op (xsm_default_t def, struct domain *d, unsigned int
 
 #endif /* CONFIG_X86 */
 
+static inline int xsm_version_op (xsm_default_t def, uint32_t op)
+{
+    return xsm_ops->version_op(op);
+}
 #endif /* XSM_NO_WRAPPERS */
 
 #ifdef CONFIG_MULTIBOOT
diff --git a/xen/xsm/dummy.c b/xen/xsm/dummy.c
index 0f32636..1469dce 100644
--- a/xen/xsm/dummy.c
+++ b/xen/xsm/dummy.c
@@ -162,4 +162,5 @@ void xsm_fixup_ops (struct xsm_operations *ops)
     set_to_dummy_if_null(ops, ioport_mapping);
     set_to_dummy_if_null(ops, pmu_op);
 #endif
+    set_to_dummy_if_null(ops, version_op);
 }
diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c
index 9b7de30..0c49804 100644
--- a/xen/xsm/flask/hooks.c
+++ b/xen/xsm/flask/hooks.c
@@ -26,6 +26,7 @@
 #include <public/xen.h>
 #include <public/physdev.h>
 #include <public/platform.h>
+#include <public/version.h>
 
 #include <public/xsm/flask_op.h>
 
@@ -1621,6 +1622,28 @@ static int flask_pmu_op (struct domain *d, unsigned int op)
 }
 #endif /* CONFIG_X86 */
 
+static int flask_version_op (uint32_t op)
+{
+    u32 dsid = domain_sid(current->domain);
+
+    switch ( op )
+    {
+    case XENVER_version:
+    case XENVER_extraversion:
+    case XENVER_compile_info:
+    case XENVER_capabilities:
+    case XENVER_changeset:
+    case XENVER_platform_parameters:
+    case XENVER_get_features:
+    case XENVER_pagesize:
+    case XENVER_guest_handle:
+        return 0; /* These MUST always be accessible to guests. */
+    default:
+        return avc_has_perm(dsid, SECINITSID_XEN, SECCLASS_XEN2,
+                            XEN2__VERSION_PRIV, NULL);
+    }
+}
+
 long do_flask_op(XEN_GUEST_HANDLE_PARAM(xsm_op_t) u_flask_op);
 int compat_flask_op(XEN_GUEST_HANDLE_PARAM(xsm_op_t) u_flask_op);
 
@@ -1759,6 +1782,7 @@ static struct xsm_operations flask_ops = {
     .ioport_mapping = flask_ioport_mapping,
     .pmu_op = flask_pmu_op,
 #endif
+    .version_op = flask_version_op,
 };
 
 static __init void flask_init(void)
diff --git a/xen/xsm/flask/policy/access_vectors b/xen/xsm/flask/policy/access_vectors
index effb59f..44a106e 100644
--- a/xen/xsm/flask/policy/access_vectors
+++ b/xen/xsm/flask/policy/access_vectors
@@ -93,6 +93,8 @@ class xen2
     pmu_ctrl
 # PMU use (domains, including unprivileged ones, will be using this operation)
     pmu_use
+# XENVER_commandline usage.
+    version_priv
 }
 
 # Classes domain and domain2 consist of operations that a domain performs on
-- 
2.1.0

[PATCH v3 2/3] XENVER_build_id: Provide ld-embedded build-ids (v8)

[Konrad Rzeszutek Wilk]

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'.

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

Note that there are no changes to the XSM files (dummy.c
and hooks.c) as the privileged subops fall in the default case.

The version of ld that first implemented --build-id is v2.18.
Hence we check for that or later version - if older version
found we do not build the hypervisor with the build-id
(and the return code is -ENODATA for that case).

Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Martin Pohlack <mpohlack@amazon.de>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
v1: Rebase it on Martin's initial patch
v2: Move it to XENVER hypercall
v3: Fix EFI building (Ross's fix)
v4: Don't use the third argument for length.
v5: Use new structure for XENVER_build_id with variable buf.
v6: Include Ross's fix.
v7: Include detection of bin-utils for build-id support, add
    probing for size, and return -EPERM for XSM denied calls.
v8: Build xen_build_id under ARM, required adding ELFSIZE in proper file.
---
 Config.mk                           | 11 ++++++++++
 tools/libxc/xc_private.c            |  7 +++++++
 tools/libxc/xc_private.h            | 10 +++++++++
 xen/arch/arm/Makefile               |  6 +++---
 xen/arch/arm/xen.lds.S              |  6 ++++++
 xen/arch/x86/Makefile               | 16 +++++++++-----
 xen/arch/x86/xen.lds.S              |  6 ++++++
 xen/common/kernel.c                 | 35 +++++++++++++++++++++++++++++++
 xen/common/version.c                | 42 +++++++++++++++++++++++++++++++++++++
 xen/include/asm-arm/config.h        |  2 ++
 xen/include/public/version.h        | 16 +++++++++++++-
 xen/include/xen/version.h           |  1 +
 xen/xsm/flask/policy/access_vectors |  4 ++--
 13 files changed, 151 insertions(+), 11 deletions(-)

diff --git a/Config.mk b/Config.mk
index 62f8209..80d6972 100644
--- 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
+
 # as-insn: Check whether assembler supports an instruction.
 # Usage: cflags-y += $(call as-insn "insn",option-yes,option-no)
 as-insn = $(if $(shell echo 'void _(void) { asm volatile ( $(2) ); }' \
diff --git a/tools/libxc/xc_private.c b/tools/libxc/xc_private.c
index 6c0c0d6..876e0df 100644
--- a/tools/libxc/xc_private.c
+++ b/tools/libxc/xc_private.c
@@ -712,6 +712,13 @@ int xc_version(xc_interface *xch, int cmd, void *arg)
     case XENVER_commandline:
         sz = sizeof(xen_commandline_t);
         break;
+    case XENVER_build_id:
+        {
+            xen_build_id_t *build_id = (xen_build_id_t *)arg;
+            sz = sizeof(*build_id) + build_id->len;
+            HYPERCALL_BOUNCE_SET_DIR(arg, XC_HYPERCALL_BUFFER_BOUNCE_BOTH);
+            break;
+        }
     default:
         ERROR("xc_version: unknown command %d\n", cmd);
         return -EINVAL;
diff --git a/tools/libxc/xc_private.h b/tools/libxc/xc_private.h
index f603c15..b1b1432 100644
--- a/tools/libxc/xc_private.h
+++ b/tools/libxc/xc_private.h
@@ -202,6 +202,16 @@ enum {
 #define DECLARE_HYPERCALL_BOUNCE(_ubuf, _sz, _dir) DECLARE_NAMED_HYPERCALL_BOUNCE(_ubuf, _ubuf, _sz, _dir)
 
 /*
+ * Change the direction.
+ *
+ * Can only be used if the bounce_pre/bounce_post commands have
+ * not been used.
+ */
+#define HYPERCALL_BOUNCE_SET_DIR(_buf, _dir) do { if ((HYPERCALL_BUFFER(_buf))->hbuf)         \
+                                                        assert(1);                            \
+                                                   (HYPERCALL_BUFFER(_buf))->dir = _dir;      \
+                                                } while (0)
+/*
  * Set the size of data to bounce. Useful when the size is not known
  * when the bounce buffer is declared.
  */
diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
index 2f050f5..fdf0800 100644
--- a/xen/arch/arm/Makefile
+++ b/xen/arch/arm/Makefile
@@ -79,17 +79,17 @@ $(BASEDIR)/common/symbols-dummy.o:
 	$(MAKE) -f $(BASEDIR)/Rules.mk -C $(BASEDIR)/common symbols-dummy.o
 
 $(TARGET)-syms: prelink.o xen.lds $(BASEDIR)/common/symbols-dummy.o
-	$(LD) $(LDFLAGS) -T xen.lds -N prelink.o \
+	$(LD) $(LDFLAGS) -T xen.lds -N prelink.o $(build_id) \
 	    $(BASEDIR)/common/symbols-dummy.o -o $(@D)/.$(@F).0
 	$(NM) -pa --format=sysv $(@D)/.$(@F).0 \
 		| $(BASEDIR)/tools/symbols --sysv --sort >$(@D)/.$(@F).0.S
 	$(MAKE) -f $(BASEDIR)/Rules.mk $(@D)/.$(@F).0.o
-	$(LD) $(LDFLAGS) -T xen.lds -N prelink.o \
+	$(LD) $(LDFLAGS) -T xen.lds -N prelink.o $(build_id) \
 	    $(@D)/.$(@F).0.o -o $(@D)/.$(@F).1
 	$(NM) -pa --format=sysv $(@D)/.$(@F).1 \
 		| $(BASEDIR)/tools/symbols --sysv --sort >$(@D)/.$(@F).1.S
 	$(MAKE) -f $(BASEDIR)/Rules.mk $(@D)/.$(@F).1.o
-	$(LD) $(LDFLAGS) -T xen.lds -N prelink.o \
+	$(LD) $(LDFLAGS) -T xen.lds -N prelink.o $(build_id) \
 	    $(@D)/.$(@F).1.o -o $@
 	rm -f $(@D)/.$(@F).[0-9]*
 
diff --git a/xen/arch/arm/xen.lds.S b/xen/arch/arm/xen.lds.S
index 0488f37..1d262c4 100644
--- a/xen/arch/arm/xen.lds.S
+++ b/xen/arch/arm/xen.lds.S
@@ -53,6 +53,12 @@ SECTIONS
         _erodata = .;          /* End of read-only data */
   } :text
 
+  .note.gnu.build-id : {
+      __note_gnu_build_id_start = .;
+      *(.note.gnu.build-id)
+      __note_gnu_build_id_end = .;
+  } :text
+
   .data : {                    /* Data */
        . = ALIGN(PAGE_SIZE);
        *(.data.page_aligned)
diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
index 8e6e901..b88a84f 100644
--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -103,20 +103,23 @@ $(BASEDIR)/common/symbols-dummy.o:
 	$(MAKE) -f $(BASEDIR)/Rules.mk -C $(BASEDIR)/common symbols-dummy.o
 
 $(TARGET)-syms: prelink.o xen.lds $(BASEDIR)/common/symbols-dummy.o
-	$(LD) $(LDFLAGS) -T xen.lds -N prelink.o \
+	$(LD) $(LDFLAGS) -T xen.lds -N prelink.o $(build_id) \
 	    $(BASEDIR)/common/symbols-dummy.o -o $(@D)/.$(@F).0
 	$(NM) -pa --format=sysv $(@D)/.$(@F).0 \
 		| $(BASEDIR)/tools/symbols --sysv --sort >$(@D)/.$(@F).0.S
 	$(MAKE) -f $(BASEDIR)/Rules.mk $(@D)/.$(@F).0.o
-	$(LD) $(LDFLAGS) -T xen.lds -N prelink.o \
+	$(LD) $(LDFLAGS) -T xen.lds -N prelink.o $(build_id) \
 	    $(@D)/.$(@F).0.o -o $(@D)/.$(@F).1
 	$(NM) -pa --format=sysv $(@D)/.$(@F).1 \
 		| $(BASEDIR)/tools/symbols --sysv --sort --warn-dup >$(@D)/.$(@F).1.S
 	$(MAKE) -f $(BASEDIR)/Rules.mk $(@D)/.$(@F).1.o
-	$(LD) $(LDFLAGS) -T xen.lds -N prelink.o \
+	$(LD) $(LDFLAGS) -T xen.lds -N prelink.o $(build_id) \
 	    $(@D)/.$(@F).1.o -o $@
 	rm -f $(@D)/.$(@F).[0-9]*
 
+build_id.o: $(TARGET)-syms
+	$(OBJCOPY) --only-section=.note.gnu.build-id $< build_id.o
+
 EFI_LDFLAGS = $(patsubst -m%,-mi386pep,$(LDFLAGS)) --subsystem=10
 EFI_LDFLAGS += --image-base=$(1) --stack=0,0 --heap=0,0 --strip-debug
 EFI_LDFLAGS += --section-alignment=0x200000 --file-alignment=0x20
@@ -129,6 +132,9 @@ $(TARGET).efi: VIRT_BASE = 0x$(shell $(NM) efi/relocs-dummy.o | sed -n 's, A VIR
 $(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),)
+$(TARGET).efi: build_id.o
+endif
 $(TARGET).efi: prelink-efi.o 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 \
@@ -144,8 +150,8 @@ $(TARGET).efi: prelink-efi.o efi.lds efi/relocs-dummy.o $(BASEDIR)/common/symbol
 	$(guard) $(NM) -pa --format=sysv $(@D)/.$(@F).$(VIRT_BASE).1 \
 		| $(guard) $(BASEDIR)/tools/symbols --sysv --sort >$(@D)/.$(@F).1s.S
 	$(guard) $(MAKE) -f $(BASEDIR)/Rules.mk $(@D)/.$(@F).1r.o $(@D)/.$(@F).1s.o
-	$(guard) $(LD) $(call EFI_LDFLAGS,$(VIRT_BASE)) -T efi.lds -N $< \
-	                $(@D)/.$(@F).1r.o $(@D)/.$(@F).1s.o -o $@
+	$(guard) $(LD) $(call EFI_LDFLAGS,$(VIRT_BASE)) -T efi.lds $(build_id) -N $< \
+	                $(@D)/.$(@F).1r.o $(@D)/.$(@F).1s.o build_id.o -o $@
 	if $(guard) false; then rm -f $@; echo 'EFI support disabled'; fi
 	rm -f $(@D)/.$(@F).[0-9]*
 
diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S
index e18e08f..329eab6 100644
--- 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
+
   . = ALIGN(SMP_CACHE_BYTES);
   .data.read_mostly : {
        /* Exception table */
diff --git a/xen/common/kernel.c b/xen/common/kernel.c
index 2b3ccc4..62a95a5 100644
--- 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:
+    {
+        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);
+        if ( rc )
+            return rc;
+
+        if ( guest_handle_is_null(arg) )
+            return sz;
+
+        if ( sz > build_id.len )
+            return -ENOBUFS;
+
+        if ( copy_to_guest_offset(arg, offsetof(xen_build_id_t, buf), p, sz) )
+            return -EFAULT;
+
+        return sz;
+    }
     }
 
     return -ENOSYS;
diff --git a/xen/common/version.c b/xen/common/version.c
index 95332a0..f7e120f 100644
--- a/xen/common/version.c
+++ b/xen/common/version.c
@@ -1,5 +1,9 @@
 #include <xen/compile.h>
 #include <xen/version.h>
+#include <xen/types.h>
+#include <xen/string.h>
+#include <xen/elf.h>
+#include <xen/errno.h>
 
 const char *xen_compile_date(void)
 {
@@ -60,3 +64,41 @@ const char *xen_deny(void)
 {
     return "<denied>\0";
 }
+
+#define NT_GNU_BUILD_ID 3
+/* Defined in linker script. */
+extern const Elf_Note __note_gnu_build_id_start[], __note_gnu_build_id_end[];
+
+int xen_build_id(char **p, unsigned int *len)
+{
+    const Elf_Note *n = __note_gnu_build_id_start;
+    static bool_t checked = 0;
+
+    if ( checked )
+    {
+        *len = n->descsz;
+        *p = ELFNOTE_DESC(n);
+        return 0;
+    }
+    /* --build-id invoked with wrong parameters. */
+    if ( __note_gnu_build_id_end <= __note_gnu_build_id_start )
+        return -ENODATA;
+
+    /* Check for full Note header. */
+    if ( &n[1] > __note_gnu_build_id_end )
+        return -ENODATA;
+
+    /* Check if we really have a build-id. */
+    if ( NT_GNU_BUILD_ID != n->type )
+        return -ENODATA;
+
+    /* Sanity check, name should be "GNU" for ld-generated build-id. */
+    if ( strncmp(ELFNOTE_NAME(n), "GNU", n->namesz) != 0 )
+        return -ENODATA;
+
+    *len = n->descsz;
+    *p = ELFNOTE_DESC(n);
+
+    checked = 1;
+    return 0;
+}
diff --git a/xen/include/asm-arm/config.h b/xen/include/asm-arm/config.h
index 1520b41..b863c80 100644
--- a/xen/include/asm-arm/config.h
+++ b/xen/include/asm-arm/config.h
@@ -15,8 +15,10 @@
 
 #if defined(CONFIG_ARM_64)
 # define LONG_BYTEORDER 3
+# define ELFSIZE 64
 #else
 # define LONG_BYTEORDER 2
+# define ELFSIZE 32
 #endif
 
 #define BYTES_PER_LONG (1 << LONG_BYTEORDER)
diff --git a/xen/include/public/version.h b/xen/include/public/version.h
index 44f26b0..adca602 100644
--- a/xen/include/public/version.h
+++ b/xen/include/public/version.h
@@ -30,7 +30,8 @@
 
 #include "xen.h"
 
-/* NB. All ops return zero on success, except XENVER_{version,pagesize} */
+/* NB. All ops return zero on success, except
+ * XENVER_{version,pagesize,build_id} */
 
 /* arg == NULL; returns major:minor (16:16). */
 #define XENVER_version      0
@@ -83,6 +84,19 @@ typedef struct xen_feature_info xen_feature_info_t;
 #define XENVER_commandline 9
 typedef char xen_commandline_t[1024];
 
+/* Return value is the number of bytes written, or XEN_Exx on error.
+ * Calling with empty parameter returns the size of build_id. */
+#define XENVER_build_id 10
+struct xen_build_id {
+        uint32_t        len; /* IN: size of buf[]. */
+#if defined(__STDC_VERSION__) && __STDC_VERSION__ >= 199901L
+        unsigned char   buf[];
+#elif defined(__GNUC__)
+        unsigned char   buf[1]; /* OUT: Variable length buffer with build_id. */
+#endif
+};
+typedef struct xen_build_id xen_build_id_t;
+
 #endif /* __XEN_PUBLIC_VERSION_H__ */
 
 /*
diff --git a/xen/include/xen/version.h b/xen/include/xen/version.h
index 2015c0b..466c977 100644
--- a/xen/include/xen/version.h
+++ b/xen/include/xen/version.h
@@ -13,5 +13,6 @@ const char *xen_extra_version(void);
 const char *xen_changeset(void);
 const char *xen_banner(void);
 const char *xen_deny(void);
+int xen_build_id(char **p, unsigned int *len);
 
 #endif /* __XEN_VERSION_H__ */
diff --git a/xen/xsm/flask/policy/access_vectors b/xen/xsm/flask/policy/access_vectors
index 44a106e..c123256 100644
--- a/xen/xsm/flask/policy/access_vectors
+++ b/xen/xsm/flask/policy/access_vectors
@@ -93,8 +93,8 @@ class xen2
     pmu_ctrl
 # PMU use (domains, including unprivileged ones, will be using this operation)
     pmu_use
-# XENVER_commandline usage.
-    version_priv
+# XENVER_[commandline|build_id] usage.
+   version_priv
 }
 
 # Classes domain and domain2 consist of operations that a domain performs on
-- 
2.1.0

[PATCH v3 3/3] libxl: info: Display build_id of the hypervisor.

[Konrad Rzeszutek Wilk]

If the hypervisor is built with we will display it.

Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
v2: Include HAVE_*, use libxl_zalloc, s/rc/ret/
---
 tools/libxl/libxl.c         | 24 ++++++++++++++++++++++++
 tools/libxl/libxl.h         |  5 +++++
 tools/libxl/libxl_types.idl |  1 +
 tools/libxl/xl_cmdimpl.c    |  1 +
 4 files changed, 31 insertions(+)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 9207621..b894c1f 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -5263,6 +5263,7 @@ libxl_numainfo *libxl_get_numainfo(libxl_ctx *ctx, int *nr)
 
 const libxl_version_info* libxl_get_version_info(libxl_ctx *ctx)
 {
+    GC_INIT(ctx);
     union {
         xen_extraversion_t xen_extra;
         xen_compile_info_t xen_cc;
@@ -5270,8 +5271,10 @@ const libxl_version_info* libxl_get_version_info(libxl_ctx *ctx)
         xen_capabilities_info_t xen_caps;
         xen_platform_parameters_t p_parms;
         xen_commandline_t xen_commandline;
+        xen_build_id_t build_id;
     } u;
     long xen_version;
+    int ret;
     libxl_version_info *info = &ctx->version_info;
 
     if (info->xen_version_extra != NULL)
@@ -5304,6 +5307,27 @@ const libxl_version_info* libxl_get_version_info(libxl_ctx *ctx)
     xc_version(ctx->xch, XENVER_commandline, &u.xen_commandline);
     info->commandline = strdup(u.xen_commandline);
 
+    u.build_id.len = sizeof(u) - sizeof(u.build_id);
+    ret = xc_version(ctx->xch, XENVER_build_id, &u.build_id);
+    switch ( ret ) {
+    case -EPERM:
+    case -ENODATA:
+    case 0:
+        info->build_id = strdup("");
+        break;
+    default:
+        if (ret > 0) {
+            unsigned int i;
+
+            info->build_id = libxl__zalloc(NOGC, (ret * 2) + 1);
+
+            for (i = 0; i < ret ; i++)
+                snprintf(&info->build_id[i * 2], 3, "%02hhx", u.build_id.buf[i]);
+        } else
+            LOGEV(ERROR, ret, "getting build_id");
+        break;
+    }
+    GC_FREE;
     return info;
 }
 
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 05606a7..b91d906 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -218,6 +218,11 @@
 #define LIBXL_HAVE_SOFT_RESET 1
 
 /*
+ * LIBXL_HAVE_BUILD_ID means that libxl_version_info has the extra
+ * field for the hypervisor build_id.
+ */
+#define LIBXL_HAVE_BUILD_ID 1
+/*
  * libxl ABI compatibility
  *
  * The only guarantee which libxl makes regarding ABI compatibility
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index 9658356..6c29885 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -355,6 +355,7 @@ libxl_version_info = Struct("version_info", [
     ("virt_start",        uint64),
     ("pagesize",          integer),
     ("commandline",       string),
+    ("build_id",          string),
     ], dir=DIR_OUT)
 
 libxl_domain_create_info = Struct("domain_create_info",[
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index f9933cb..1fbe7f3 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -5549,6 +5549,7 @@ static void output_xeninfo(void)
     printf("cc_compile_by          : %s\n", info->compile_by);
     printf("cc_compile_domain      : %s\n", info->compile_domain);
     printf("cc_compile_date        : %s\n", info->compile_date);
+    printf("build_id               : %s\n", info->build_id);
 
     return;
 }
-- 
2.1.0

Re: [PATCH v3 1/3] xsm/xen_version: Add XSM for the xen_version hypercall (v6).

[Daniel De Graaf]

On 07/01/16 21:25, Konrad Rzeszutek Wilk wrote:
> All of XENVER_* have now an XSM check.
>
> The subop for XENVER_commandline is now a priviliged operation.
> To not break guests we still return an string - but it is
> just '<denied>\0'.
>
> The rest: XENVER_[version|extraversion|capabilities|
> parameters|get_features|page_size|guest_handle|changeset|
> compile_info] behave as before - allowed by default for all guests.
>
> This is with the XSM default (and non-default) policy and with
> the dummy ones.
>
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

Acked-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>

Re: [PATCH v3 3/3] libxl: info: Display build_id of the hypervisor.

[Wei Liu]

On Thu, Jan 07, 2016 at 09:25:20PM -0500, Konrad Rzeszutek Wilk wrote:
> If the hypervisor is built with we will display it.
> 
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> ---
> v2: Include HAVE_*, use libxl_zalloc, s/rc/ret/
> ---
>  tools/libxl/libxl.c         | 24 ++++++++++++++++++++++++
>  tools/libxl/libxl.h         |  5 +++++
>  tools/libxl/libxl_types.idl |  1 +
>  tools/libxl/xl_cmdimpl.c    |  1 +
>  4 files changed, 31 insertions(+)
> 
> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> index 9207621..b894c1f 100644
> --- a/tools/libxl/libxl.c
> +++ b/tools/libxl/libxl.c
> @@ -5263,6 +5263,7 @@ libxl_numainfo *libxl_get_numainfo(libxl_ctx *ctx, int *nr)
>  
>  const libxl_version_info* libxl_get_version_info(libxl_ctx *ctx)
>  {
> +    GC_INIT(ctx);
>      union {
>          xen_extraversion_t xen_extra;
>          xen_compile_info_t xen_cc;
> @@ -5270,8 +5271,10 @@ const libxl_version_info* libxl_get_version_info(libxl_ctx *ctx)
>          xen_capabilities_info_t xen_caps;
>          xen_platform_parameters_t p_parms;
>          xen_commandline_t xen_commandline;
> +        xen_build_id_t build_id;
>      } u;
>      long xen_version;
> +    int ret;
>      libxl_version_info *info = &ctx->version_info;
>  
>      if (info->xen_version_extra != NULL)
> @@ -5304,6 +5307,27 @@ const libxl_version_info* libxl_get_version_info(libxl_ctx *ctx)
>      xc_version(ctx->xch, XENVER_commandline, &u.xen_commandline);
>      info->commandline = strdup(u.xen_commandline);
>  
> +    u.build_id.len = sizeof(u) - sizeof(u.build_id);
> +    ret = xc_version(ctx->xch, XENVER_build_id, &u.build_id);
> +    switch ( ret ) {
> +    case -EPERM:
> +    case -ENODATA:
> +    case 0:
> +        info->build_id = strdup("");

I guess you're following existing strdup examples in this function.

Since now there is a GC in scope, you can use libxl__strdup. Presumably
you can also change other instances to use libxl__strdup.

Wei.

Re: [PATCH v3 1/3] xsm/xen_version: Add XSM for the xen_version hypercall (v6).

[Jan Beulich]

>>> On 08.01.16 at 03:25, <konrad.wilk@oracle.com> wrote:
> @@ -226,9 +227,10 @@ void __init do_initcalls(void)
>  /*
>   * Simple hypercalls.
>   */
> -
>  DO(xen_version)(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
>  {
> +    bool_t deny = !!xsm_version_op(XSM_OTHER, cmd);
> +
>      switch ( cmd )
>      {
>      case XENVER_version:
> @@ -354,10 +356,17 @@ DO(xen_version)(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
>          return 0;
>  
>      case XENVER_commandline:
> -        if ( copy_to_guest(arg, saved_cmdline, ARRAY_SIZE(saved_cmdline)) )
> +    {
> +        size_t len = ARRAY_SIZE(saved_cmdline);
> +
> +        if ( deny )
> +            len = strlen(xen_deny()) + 1;
> +
> +        if ( copy_to_guest(arg, deny ? xen_deny() : saved_cmdline, len) )
>              return -EFAULT;
>          return 0;
>      }
> +    }
>  
>      return -ENOSYS;
>  }

As said before, I don't think it is appropriate for "deny" to be
ignored for all other sub-ops when there is a designated policy.

> --- a/xen/common/version.c
> +++ b/xen/common/version.c
> @@ -55,3 +55,8 @@ const char *xen_banner(void)
>  {
>      return XEN_BANNER;
>  }
> +
> +const char *xen_deny(void)
> +{
> +    return "<denied>\0";
> +}

There's still this strange extra NUL character here.

> @@ -1621,6 +1622,28 @@ static int flask_pmu_op (struct domain *d, unsigned int op)
>  }
>  #endif /* CONFIG_X86 */
>  
> +static int flask_version_op (uint32_t op)
> +{
> +    u32 dsid = domain_sid(current->domain);
> +
> +    switch ( op )
> +    {
> +    case XENVER_version:
> +    case XENVER_extraversion:
> +    case XENVER_compile_info:
> +    case XENVER_capabilities:
> +    case XENVER_changeset:
> +    case XENVER_platform_parameters:
> +    case XENVER_get_features:
> +    case XENVER_pagesize:
> +    case XENVER_guest_handle:
> +        return 0; /* These MUST always be accessible to guests. */
> +    default:
> +        return avc_has_perm(dsid, SECINITSID_XEN, SECCLASS_XEN2,
> +                            XEN2__VERSION_PRIV, NULL);
> +    }
> +}

And along with the comment above, I don't think there should be
a switch statement here, but instead "op" should be subjected to
policy restrictions.

Jan

Re: [PATCH v3 2/3] XENVER_build_id: Provide ld-embedded build-ids (v8)

[Jan Beulich]

>>> On 08.01.16 at 03:25, <konrad.wilk@oracle.com> 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?
I ask because xen-syms isn't present on the boot partition, and
may not be present anywhere on an otherwise functioning
system.

> --- 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)?

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?

> +        if ( copy_to_guest_offset(arg, offsetof(xen_build_id_t, buf), p, sz) )
> +            return -EFAULT;
> +
> +        return sz;
> +    }

Or how much got copied?

> +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

Re: [PATCH v3 2/3] XENVER_build_id: Provide ld-embedded build-ids (v8)

[Konrad Rzeszutek Wilk]

On Tue, Jan 12, 2016 at 09:25:27AM -0700, Jan Beulich wrote:
> >>> On 08.01.16 at 03:25, <konrad.wilk@oracle.com> 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
> 

Re: [PATCH v3 2/3] XENVER_build_id: Provide ld-embedded build-ids (v8)

[Jan Beulich]

>>> On 12.01.16 at 17:43, <konrad.wilk@oracle.com> wrote:
> On Tue, Jan 12, 2016 at 09:25:27AM -0700, Jan Beulich wrote:
>> >>> On 08.01.16 at 03:25, <konrad.wilk@oracle.com> 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 ?

Key handlers are strictly a debugging facility.

> Or are you suggesting that perhaps the kernel should at boot time
> print the build-id (like it does the changset)?

Perhaps, albeit to me that's a bit orthogonal to being able to find out
the build ID for a given binary.

>> > +        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.

Ah, actually I now see you have Andrew's beloved NULL handle
check up a few lines - that may suffice. Albeit I'm not generally in
favor of this model; I prefer a first attempt to succeed if possible,
and a second one only to be needed if the caller estimated size in
fact didn't suffice (and then no 3rd one being necessary in order
to obtain the needed size).

>> > +        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.

Ah, right. Perhaps I shouldn't be reviewing patches a few minutes
before heading home...

Jan

Re: [PATCH v3 2/3] XENVER_build_id: Provide ld-embedded build-ids (v8)

[Konrad Rzeszutek Wilk]

> > Or are you suggesting that perhaps the kernel should at boot time
> > print the build-id (like it does the changset)?
> 
> Perhaps, albeit to me that's a bit orthogonal to being able to find out
> the build ID for a given binary.

I can make some magic objdump + awk to extract the buildid from
the hypervisor and deposit as a text file in /usr/../xen/debug?
Similar to how xen-syms is copied there?

Would that work?

But I am a bit lost of what your use-case is?

The third patch implements 'xl info' to display it.

> 
> >> > +        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.
> 
> Ah, actually I now see you have Andrew's beloved NULL handle
> check up a few lines - that may suffice. Albeit I'm not generally in
> favor of this model; I prefer a first attempt to succeed if possible,
> and a second one only to be needed if the caller estimated size in
> fact didn't suffice (and then no 3rd one being necessary in order
> to obtain the needed size).

The code I wrote (libxl) tries with a large buffer (1020 bytes)
and if that didn't work just reports the error.

I shall change the code to follow your steps.

Re: [PATCH v3 3/3] libxl: info: Display build_id of the hypervisor.

[Konrad Rzeszutek Wilk]

> > +        info->build_id = strdup("");
> 
> I guess you're following existing strdup examples in this function.

/me nods.
> 
> Since now there is a GC in scope, you can use libxl__strdup. Presumably
> you can also change other instances to use libxl__strdup.

Would you be OK if I did it in another patch? I can make it
part of the series if you would like.

> 
> Wei.
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

Re: [PATCH v3 2/3] XENVER_build_id: Provide ld-embedded build-ids (v8)

[Jan Beulich]

>>> On 14.01.16 at 03:02, <konrad@kernel.org> wrote:
>> > Or are you suggesting that perhaps the kernel should at boot time
>> > print the build-id (like it does the changset)?
>> 
>> Perhaps, albeit to me that's a bit orthogonal to being able to find out
>> the build ID for a given binary.
> 
> I can make some magic objdump + awk to extract the buildid from
> the hypervisor and deposit as a text file in /usr/../xen/debug?
> Similar to how xen-syms is copied there?
> 
> Would that work?

No, because of ...

> But I am a bit lost of what your use-case is?

... this is about holding a standalone Xen binary in hands, and
wanting to identify its build ID. Not sure how limited the range of
permitted things is in the simplified ELF binary we generate -
perhaps adding a proper PT_NOTE segment there would do?

Jan

Re: [PATCH v3 2/3] XENVER_build_id: Provide ld-embedded build-ids (v8)

[Martin Pohlack]

On 14.01.2016 09:58, Jan Beulich wrote:
>>>> On 14.01.16 at 03:02, <konrad@kernel.org> wrote:
>>>> Or are you suggesting that perhaps the kernel should at boot time
>>>> print the build-id (like it does the changset)?
>>>
>>> Perhaps, albeit to me that's a bit orthogonal to being able to find out
>>> the build ID for a given binary.
>>
>> I can make some magic objdump + awk to extract the buildid from
>> the hypervisor and deposit as a text file in /usr/../xen/debug?
>> Similar to how xen-syms is copied there?
>>
>> Would that work?
> 
> No, because of ...
> 
>> But I am a bit lost of what your use-case is?
> 
> ... this is about holding a standalone Xen binary in hands, and
> wanting to identify its build ID. Not sure how limited the range of
> permitted things is in the simplified ELF binary we generate -
> perhaps adding a proper PT_NOTE segment there would do?

To clarify your question:

  readelf -n xen-syms

should result in finding and dumping the build ID.  The original patch
to xen.lds did achieve that.

Is that enough for you or do you want that to work as well for the
stripped xen file?

Martin
Amazon Development Center Germany GmbH
Berlin - Dresden - Aachen
main office: Krausenstr. 38, 10117 Berlin
Geschaeftsfuehrer: Dr. Ralf Herbrich, Christian Schlaeger
Ust-ID: DE289237879
Eingetragen am Amtsgericht Charlottenburg HRB 149173 B

Re: [PATCH v3 2/3] XENVER_build_id: Provide ld-embedded build-ids (v8)

[Jan Beulich]

>>> On 14.01.16 at 10:07, <mpohlack@amazon.com> wrote:
> On 14.01.2016 09:58, Jan Beulich wrote:
>>>>> On 14.01.16 at 03:02, <konrad@kernel.org> wrote:
>>>>> Or are you suggesting that perhaps the kernel should at boot time
>>>>> print the build-id (like it does the changset)?
>>>>
>>>> Perhaps, albeit to me that's a bit orthogonal to being able to find out
>>>> the build ID for a given binary.
>>>
>>> I can make some magic objdump + awk to extract the buildid from
>>> the hypervisor and deposit as a text file in /usr/../xen/debug?
>>> Similar to how xen-syms is copied there?
>>>
>>> Would that work?
>> 
>> No, because of ...
>> 
>>> But I am a bit lost of what your use-case is?
>> 
>> ... this is about holding a standalone Xen binary in hands, and
>> wanting to identify its build ID. Not sure how limited the range of
>> permitted things is in the simplified ELF binary we generate -
>> perhaps adding a proper PT_NOTE segment there would do?
> 
> To clarify your question:
> 
>   readelf -n xen-syms
> 
> should result in finding and dumping the build ID.  The original patch
> to xen.lds did achieve that.
> 
> Is that enough for you or do you want that to work as well for the
> stripped xen file?

"Stripped" isn't the right term, because the uncompressed variant
of xen.gz is farther away from xen-syms than just being stripped.
But yes, all this is about is that it should be possible to obtain
xen.gz's and xen.efi's build IDs as well.

Jan

Re: [PATCH v3 2/3] XENVER_build_id: Provide ld-embedded build-ids (v8)

[Konrad Rzeszutek Wilk]

>> --- 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)?

+ld-ver-build-id = $(shell $(1) --build-id 2>&1 | \
+                                       grep -q unrecognized && echo n
|| echo y)

-ish ?

>
> In any event the option should be added to LDFLAGS unless this
> conflicts with something else.

That had some interesting side-effect:

$ find . -name *.o | xargs readelf -n | more
File: ./arch/x86/built_in.o

Displaying notes found at file offset 0x00000040 with length 0x00000240:
  Owner                 Data size       Description
  GNU                  0x00000014       NT_GNU_BUILD_ID (unique build
ID bitstring)
    Build ID: a114d1fdec2ace38448f141013f5a659122f2390
  GNU                  0x00000014       NT_GNU_BUILD_ID (unique build
ID bitstring)
    Build ID: 4a913d3d1ece4d175fc0df0b36745b801f88bfab
  GNU                  0x00000014       NT_GNU_BUILD_ID (unique build
ID bitstring)
    Build ID: ead89ba3aa9a8257cfc863966b7a9a725ecce133
  GNU                  0x00000014       NT_GNU_BUILD_ID (unique build
ID bitstring)
    Build ID: 9b034a93573015c611d0900e949370d9f776bac1
  GNU                  0x00000014       NT_GNU_BUILD_ID (unique build
ID bitstring)
    Build ID: 0e5fab6126ce69edd5720b96e2d5414618259c78
  GNU                  0x00000014       NT_GNU_BUILD_ID (unique build
ID bitstring)
    Build ID: 49eca2138553b5e85ee45bb47c52aca394c31c31
  GNU                  0x00000014       NT_GNU_BUILD_ID (unique build
ID bitstring)
    Build ID: f1cc2c8ae09fefe1440662efc44208a0b9ff8ddd
  GNU                  0x00000014       NT_GNU_BUILD_ID (unique build
ID bitstring)
    Build ID: 29991f03f7a1aeeb59c8f83c0e7a349384a7262c
  GNU                  0x00000014       NT_GNU_BUILD_ID (unique build
ID bitstring)
    Build ID: 3e2c4df9f60fdbd970bd1a35d03c7b68fd794385
  GNU                  0x00000014       NT_GNU_BUILD_ID (unique build
ID bitstring)
    Build ID: 2c4f5cd9bcf553a022dace08b7741d85a4eb657f
  GNU                  0x00000014       NT_GNU_BUILD_ID (unique build
ID bitstring)
    Build ID: 41c502289e5b28722ab5df6ae5bd7b99fb90c09e
  GNU                  0x00000014       NT_GNU_BUILD_ID (unique build
ID bitstring)
    Build ID: f268afebdf211de6bb6d12e513520bba969130cc
  GNU                  0x00000014       NT_GNU_BUILD_ID (unique build
ID bitstring)
    Build ID: b73b7296f6165d3dcacee36691d92ab1996d9908
  GNU                  0x00000014       NT_GNU_BUILD_ID (unique build
ID bitstring)
    Build ID: ff8e080d8e01966cf8d893ce6cd258dd0d1c6124
  GNU                  0x00000014       NT_GNU_BUILD_ID (unique build
ID bitstring)
    Build ID: dcc8964716f74bb710911d80faaae016820f72d9
  GNU                  0x00000014       NT_GNU_BUILD_ID (unique build
ID bitstring)
    Build ID: 649f4bb66df2fead426edda62d0c90ab088c5fd4


And during build:
ld: warning: Cannot create .note.gnu.build-id section, --build-id ignored.
ld: warning: Cannot create .note.gnu.build-id section, --build-id ignored.
ld: warning: Cannot create .note.gnu.build-id section, --build-id ignored.

With:
[konrad@char xen]$ readelf -n xen-syms  | grep Build | wc
     38     114    2090

I think we should skip on the LDFLAGS idea.

Re: [PATCH v3 2/3] XENVER_build_id: Provide ld-embedded build-ids (v8)

[Jan Beulich]

>>> On 15.01.16 at 22:49, <konrad@kernel.org> wrote:
>> > --- 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)?
> 
> +ld-ver-build-id = $(shell $(1) --build-id 2>&1 | \
> +                                       grep -q unrecognized && echo n
> || echo y)
> 
> -ish ?

If that fulfills the purpose - why not?

>> In any event the option should be added to LDFLAGS unless this
>> conflicts with something else.
> 
> That had some interesting side-effect:
> 
> $ find . -name *.o | xargs readelf -n | more
> File: ./arch/x86/built_in.o
> 
> Displaying notes found at file offset 0x00000040 with length 0x00000240:
>   Owner                 Data size       Description
>   GNU                  0x00000014       NT_GNU_BUILD_ID (unique build
> ID bitstring)
>     Build ID: a114d1fdec2ace38448f141013f5a659122f2390
>   GNU                  0x00000014       NT_GNU_BUILD_ID (unique build
> ID bitstring)
>     Build ID: 4a913d3d1ece4d175fc0df0b36745b801f88bfab
>   GNU                  0x00000014       NT_GNU_BUILD_ID (unique build
> ID bitstring)
>     Build ID: ead89ba3aa9a8257cfc863966b7a9a725ecce133
>   GNU                  0x00000014       NT_GNU_BUILD_ID (unique build
> ID bitstring)
>     Build ID: 9b034a93573015c611d0900e949370d9f776bac1
>   GNU                  0x00000014       NT_GNU_BUILD_ID (unique build
> ID bitstring)
>     Build ID: 0e5fab6126ce69edd5720b96e2d5414618259c78
>   GNU                  0x00000014       NT_GNU_BUILD_ID (unique build
> ID bitstring)
>     Build ID: 49eca2138553b5e85ee45bb47c52aca394c31c31
>   GNU                  0x00000014       NT_GNU_BUILD_ID (unique build
> ID bitstring)
>     Build ID: f1cc2c8ae09fefe1440662efc44208a0b9ff8ddd
>   GNU                  0x00000014       NT_GNU_BUILD_ID (unique build
> ID bitstring)
>     Build ID: 29991f03f7a1aeeb59c8f83c0e7a349384a7262c
>   GNU                  0x00000014       NT_GNU_BUILD_ID (unique build
> ID bitstring)
>     Build ID: 3e2c4df9f60fdbd970bd1a35d03c7b68fd794385
>   GNU                  0x00000014       NT_GNU_BUILD_ID (unique build
> ID bitstring)
>     Build ID: 2c4f5cd9bcf553a022dace08b7741d85a4eb657f
>   GNU                  0x00000014       NT_GNU_BUILD_ID (unique build
> ID bitstring)
>     Build ID: 41c502289e5b28722ab5df6ae5bd7b99fb90c09e
>   GNU                  0x00000014       NT_GNU_BUILD_ID (unique build
> ID bitstring)
>     Build ID: f268afebdf211de6bb6d12e513520bba969130cc
>   GNU                  0x00000014       NT_GNU_BUILD_ID (unique build
> ID bitstring)
>     Build ID: b73b7296f6165d3dcacee36691d92ab1996d9908
>   GNU                  0x00000014       NT_GNU_BUILD_ID (unique build
> ID bitstring)
>     Build ID: ff8e080d8e01966cf8d893ce6cd258dd0d1c6124
>   GNU                  0x00000014       NT_GNU_BUILD_ID (unique build
> ID bitstring)
>     Build ID: dcc8964716f74bb710911d80faaae016820f72d9
>   GNU                  0x00000014       NT_GNU_BUILD_ID (unique build
> ID bitstring)
>     Build ID: 649f4bb66df2fead426edda62d0c90ab088c5fd4
> 
> 
> And during build:
> ld: warning: Cannot create .note.gnu.build-id section, --build-id ignored.
> ld: warning: Cannot create .note.gnu.build-id section, --build-id ignored.
> ld: warning: Cannot create .note.gnu.build-id section, --build-id ignored.
> 
> With:
> [konrad@char xen]$ readelf -n xen-syms  | grep Build | wc
>      38     114    2090
> 
> I think we should skip on the LDFLAGS idea.

Isn't that merely a result of LDFLAGS being used for both the
linking of the various built_in.o and the final binaries? A fully
shared set of flags for these two pretty distinct operations
doesn't seem very sensible anyway. In fact I wonder how
much of LDFLAGS is actually relevant when passing -r to ld.

Jan

Re: [PATCH v3 2/3] XENVER_build_id: Provide ld-embedded build-ids (v8)

[Konrad Rzeszutek Wilk]

>>> Wouldn't it be better to probe the linker for recognizing the --build-id
>>> command line option, along the lines of $(cc-option)?
>>
>> +ld-ver-build-id = $(shell $(1) --build-id 2>&1 | \
>> +                                       grep -q unrecognized && echo n
>> || echo y)
>>
>> -ish ?
>
> If that fulfills the purpose - why not?

Great. Will use that (after checking it on on SLES10).
>
>>> In any event the option should be added to LDFLAGS unless this
>>> conflicts with something else.
>>
>> That had some interesting side-effect:
>>
>> $ find . -name *.o | xargs readelf -n | more
>> File: ./arch/x86/built_in.o
>>
>> Displaying notes found at file offset 0x00000040 with length 0x00000240:
>>   Owner                 Data size       Description
>>   GNU                  0x00000014       NT_GNU_BUILD_ID (unique build
>> ID bitstring)
>>     Build ID: a114d1fdec2ace38448f141013f5a659122f2390
>>   GNU                  0x00000014       NT_GNU_BUILD_ID (unique build
>> ID bitstring)
.. snip..
>> And during build:
>> ld: warning: Cannot create .note.gnu.build-id section, --build-id ignored.
.. snip..
>> I think we should skip on the LDFLAGS idea.
>
> Isn't that merely a result of LDFLAGS being used for both the
> linking of the various built_in.o and the final binaries? A fully

True.
> shared set of flags for these two pretty distinct operations
> doesn't seem very sensible anyway. In fact I wonder how
> much of LDFLAGS is actually relevant when passing -r to ld.
>

That would imply that the idea of putting --build-id should be part
only of the final binaries. Which comes back to how this patch was done
originally - only have the --build-id on specific $(LD) lines.

Are you OK if I re-institute the $(build_id) back in the Makefile, perhaps
rename it to $(build_id_linker) to make it more clear?

Thanks.

Re: [PATCH v3 2/3] XENVER_build_id: Provide ld-embedded build-ids (v8)

[Jan Beulich]

>>> On 19.01.16 at 17:05, <konrad@kernel.org> wrote:
> Are you OK if I re-institute the $(build_id) back in the Makefile, perhaps
> rename it to $(build_id_linker) to make it more clear?

Well, perhaps that's the least troublesome route right now, even
if I don't really like it.

Jan

Re: [PATCH v3 2/3] XENVER_build_id: Provide ld-embedded build-ids (v8)

[Konrad Rzeszutek Wilk]

On Tue, Jan 19, 2016 at 09:36:27AM -0700, Jan Beulich wrote:
> >>> On 19.01.16 at 17:05, <konrad@kernel.org> wrote:
> > Are you OK if I re-institute the $(build_id) back in the Makefile, perhaps
> > rename it to $(build_id_linker) to make it more clear?
> 
> Well, perhaps that's the least troublesome route right now, even
> if I don't really like it.

:-)

Thank you for being OK with this.

I can poke at the LD_FLAGS and the prolifieration of various flags in parallel.
Perhaps after the cleanup of the C-and-P states and now the T-states being trapped?
> 
> Jan
> 

Re: [PATCH v3 2/3] XENVER_build_id: Provide ld-embedded build-ids (v8)

[Konrad Rzeszutek Wilk]

> 
> > Or are you suggesting that perhaps the kernel should at boot time
> > print the build-id (like it does the changset)?
> 
> Perhaps, albeit to me that's a bit orthogonal to being able to find out
> the build ID for a given binary.


I looked in the mkelf32 and it looks quite easy to make the resulting
xen.gz have the .note section.

But I am at lost for the EFI file. Could you suggest some ideas of what
type of PE/COFF section it should be put in? Or good specs to consult?

Thank you!

Re: [PATCH v3 2/3] XENVER_build_id: Provide ld-embedded build-ids (v8)

[Jan Beulich]

>>> On 25.01.16 at 16:16, <konrad.wilk@oracle.com> wrote:
>>  
>> > Or are you suggesting that perhaps the kernel should at boot time
>> > print the build-id (like it does the changset)?
>> 
>> Perhaps, albeit to me that's a bit orthogonal to being able to find out
>> the build ID for a given binary.
> 
> 
> I looked in the mkelf32 and it looks quite easy to make the resulting
> xen.gz have the .note section.
> 
> But I am at lost for the EFI file. Could you suggest some ideas of what
> type of PE/COFF section it should be put in? Or good specs to consult?

Well, for xen.efi I really don't have any good idea either. Perhaps
we simply need to declare this as going to be deal with by the
intended re-unification of both binaries which Daniel has been
planning?

Jan

Re: [PATCH v3 2/3] XENVER_build_id: Provide ld-embedded build-ids (v8)

[Ian Jackson]

Konrad Rzeszutek Wilk writes ("[PATCH v3 2/3] XENVER_build_id: Provide ld-embedded build-ids (v8)"):
> 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'.
> 
> For EFI builds we re-use the same build-id that the xen-syms
> was built with.
> 
> Note that there are no changes to the XSM files (dummy.c
> and hooks.c) as the privileged subops fall in the default case.
> 
> The version of ld that first implemented --build-id is v2.18.
> Hence we check for that or later version - if older version
> found we do not build the hypervisor with the build-id
> (and the return code is -ENODATA for that case).

I think this commit message is missing some important explanation for
poor ignorant souls like me.

Something like:

  It is possible to have ld record an arbitrary identifier in an ELF,
  which can then be read out with `readelf'.  We also want to provide
  a way for guests to retrieve the same identifier.

  [ xxx do we? Maybe hosting providers would want to hide which of
  their hosts had been updated ]

  We construct the build-id out of [ ???? ]

  There is ???? no impact on users who want reproducible builds [or]
  ???  users who want reproducible builds are expected to use the
  facilities documented in ??? comments in Config.mk [or something]

Ian.