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@citrix.com>
---
CC: Anthony PERARD <anthony.perard@vates.tech>
CC: Michal Orzel <michal.orzel@amd.com>
CC: Jan Beulich <jbeulich@suse.com>
CC: Julien Grall <julien@xen.org>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Ross Lagerwall <ross.lagerwall@citrix.com>
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
On Thu, Jul 31, 2025 at 12:02 PM Andrew Cooper
<andrew.cooper3@citrix.com> wrote:
>
> 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@citrix.com>
> ---
> CC: Anthony PERARD <anthony.perard@vates.tech>
> CC: Michal Orzel <michal.orzel@amd.com>
> CC: Jan Beulich <jbeulich@suse.com>
> CC: Julien Grall <julien@xen.org>
> CC: Roger Pau Monné <roger.pau@citrix.com>
> CC: Stefano Stabellini <sstabellini@kernel.org>
> CC: Ross Lagerwall <ross.lagerwall@citrix.com>
>
> 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;
Is it worth making this const? Or you could just remove these variables
entirely and use xen_build_id / xen_build_id_len directly in the rest of
the function.
In any case,
Reviewed-by: Ross Lagerwall <Ross.lagerwall@citrix.com>
On 31.07.2025 13:02, Andrew Cooper wrote: > 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@citrix.com> Reviewed-by: Jan Beulich <jbeulich@suse.com>
© 2016 - 2025 Red Hat, Inc.