[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[PATCH 2/2] xen/version: Remove xen_build_id() and export the variable instead



The API is unergonomic to use, and leads to poor code generation because of
unnecessary forcing of data to the stack.

Rename build_id_p to xen_build_id, and build_id_len to xen_build_id_len, make
them __ro_after_init, and export the variables directly.

No functional change.

Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
---
CC: Anthony PERARD <anthony.perard@xxxxxxxxxx>
CC: Michal Orzel <michal.orzel@xxxxxxx>
CC: Jan Beulich <jbeulich@xxxxxxxx>
CC: Julien Grall <julien@xxxxxxx>
CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
CC: Stefano Stabellini <sstabellini@xxxxxxxxxx>
CC: Ross Lagerwall <ross.lagerwall@xxxxxxxxxx>

Both the code diffstat, and the binary on x86 speaks for themselves:

  add/remove: 1/2 grow/shrink: 0/6 up/down: 4/-348 (-344)
  Function                                     old     new   delta
  xen_build_id_len                               -       4      +4
  build_id_len                                   4       -      -4
  build_id_p                                     8       -      -8
  xen_build_id                                  42       8     -34
  livepatch_printall                           470     432     -38
  build_id_dep                                 152     113     -39
  livepatch_op                                7930    7866     -64
  do_xen_version                              2436    2363     -73
  livepatch_op.cold                           1420    1332     -88
---
 xen/common/kernel.c       | 14 ++++----------
 xen/common/livepatch.c    | 23 ++++++++++-------------
 xen/common/version.c      | 25 +++++++------------------
 xen/include/xen/version.h |  4 +++-
 4 files changed, 24 insertions(+), 42 deletions(-)

diff --git a/xen/common/kernel.c b/xen/common/kernel.c
index 5be668ba855a..e6979352e100 100644
--- a/xen/common/kernel.c
+++ b/xen/common/kernel.c
@@ -510,21 +510,15 @@ static long xenver_varbuf_op(int cmd, 
XEN_GUEST_HANDLE_PARAM(void) arg)
     struct xen_varbuf user_str;
     const char *str = NULL;
     size_t sz;
-    int rc;
 
     switch ( cmd )
     {
     case XENVER_build_id:
-    {
-        unsigned int local_sz;
-
-        rc = xen_build_id((const void **)&str, &local_sz);
-        if ( rc )
-            return rc;
-
-        sz = local_sz;
+        str = xen_build_id;
+        sz  = xen_build_id_len;
+        if ( !sz )
+            return -ENODATA;
         goto have_len;
-    }
 
     case XENVER_extraversion2:
         str = xen_extra_version();
diff --git a/xen/common/livepatch.c b/xen/common/livepatch.c
index 9a0df5363b59..9285f88644f4 100644
--- a/xen/common/livepatch.c
+++ b/xen/common/livepatch.c
@@ -475,8 +475,8 @@ static int parse_buildid(const struct livepatch_elf_sec 
*sec,
 
 static int check_xen_buildid(const struct livepatch_elf *elf)
 {
-    const void *id;
-    unsigned int len;
+    const void *id = xen_build_id;
+    unsigned int len = xen_build_id_len;
     struct livepatch_build_id lp_id;
     const struct livepatch_elf_sec *sec =
         livepatch_elf_sec_by_name(elf, ELF_LIVEPATCH_XEN_DEPENDS);
@@ -498,13 +498,12 @@ static int check_xen_buildid(const struct livepatch_elf 
*elf)
         return -EINVAL;
     }
 
-    rc = xen_build_id(&id, &len);
-    if ( rc )
+    if ( !len )
     {
         printk(XENLOG_ERR LIVEPATCH
                "%s: unable to get running Xen build-id: %d\n",
                elf->name, rc);
-        return rc;
+        return -ENODATA;
     }
 
     if ( lp_id.len != len || memcmp(id, lp_id.p, len) )
@@ -1984,7 +1983,6 @@ static int build_id_dep(struct payload *payload, bool 
internal)
 {
     const void *id = NULL;
     unsigned int len = 0;
-    int rc;
     const char *name = "hypervisor";
 
     ASSERT(payload->dep.len && payload->dep.p);
@@ -1992,9 +1990,10 @@ static int build_id_dep(struct payload *payload, bool 
internal)
     /* First time user is against hypervisor. */
     if ( internal )
     {
-        rc = xen_build_id(&id, &len);
-        if ( rc )
-            return rc;
+        id = xen_build_id;
+        len = xen_build_id_len;
+        if ( !len )
+            return -ENODATA;
     }
     else
     {
@@ -2249,14 +2248,12 @@ static const char *state2str(unsigned int state)
 static void cf_check livepatch_printall(unsigned char key)
 {
     struct payload *data;
-    const void *binary_id = NULL;
-    unsigned int len = 0;
     unsigned int i;
 
     printk("'%c' pressed - Dumping all livepatch patches\n", key);
 
-    if ( !xen_build_id(&binary_id, &len) )
-        printk("build-id: %*phN\n", len, binary_id);
+    if ( xen_build_id_len )
+        printk("build-id: %*phN\n", xen_build_id_len, xen_build_id);
 
     if ( !spin_trylock(&payload_lock) )
     {
diff --git a/xen/common/version.c b/xen/common/version.c
index 84bd77e74653..553b97ba9b3c 100644
--- a/xen/common/version.c
+++ b/xen/common/version.c
@@ -94,8 +94,8 @@ const char *xen_build_info(void)
     return build_info;
 }
 
-static const void *build_id_p __read_mostly;
-static unsigned int build_id_len __read_mostly;
+const void *__ro_after_init xen_build_id;
+unsigned int __ro_after_init xen_build_id_len;
 
 void print_version(void)
 {
@@ -106,19 +106,8 @@ void print_version(void)
 
     printk("Latest ChangeSet: %s\n", xen_changeset());
 
-    if ( build_id_len )
-        printk("build-id: %*phN\n", build_id_len, build_id_p);
-}
-
-int xen_build_id(const void **p, unsigned int *len)
-{
-    if ( !build_id_len )
-        return -ENODATA;
-
-    *len = build_id_len;
-    *p = build_id_p;
-
-    return 0;
+    if ( xen_build_id_len )
+        printk("build-id: %*phN\n", xen_build_id_len, xen_build_id);
 }
 
 #ifdef BUILD_ID
@@ -193,7 +182,7 @@ void __init xen_build_init(void)
 
     sz = (uintptr_t)__note_gnu_build_id_end - (uintptr_t)n;
 
-    rc = xen_build_id_check(n, sz, &build_id_p, &build_id_len);
+    rc = xen_build_id_check(n, sz, &xen_build_id, &xen_build_id_len);
 
 #ifdef CONFIG_X86
     /*
@@ -219,8 +208,8 @@ void __init xen_build_init(void)
 
             if ( info->cv_signature == CVINFO_PDB70_CVSIGNATURE )
             {
-                build_id_p = info->signature;
-                build_id_len = sizeof(info->signature);
+                xen_build_id = info->signature;
+                xen_build_id_len = sizeof(info->signature);
                 rc = 0;
             }
         }
diff --git a/xen/include/xen/version.h b/xen/include/xen/version.h
index 4856ad1b446d..6f5d9c956054 100644
--- a/xen/include/xen/version.h
+++ b/xen/include/xen/version.h
@@ -17,10 +17,12 @@ const char *xen_changeset(void);
 const char *xen_banner(void);
 const char *xen_deny(void);
 const char *xen_build_info(void);
-int xen_build_id(const void **p, unsigned int *len);
 
 extern char xen_cap_info[128];
 
+extern const void *xen_build_id;
+extern unsigned int xen_build_id_len; /* 0 -> No build id. */
+
 #ifdef BUILD_ID
 void xen_build_init(void);
 int xen_build_id_check(const Elf_Note *n, unsigned int n_sz,
-- 
2.39.5




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.