* [PATCH] xen/common: Drop function calls for Xen compile/version information
@ 2017-01-16 13:04 Andrew Cooper
2017-01-16 13:18 ` Jan Beulich
` (3 more replies)
0 siblings, 4 replies; 17+ messages in thread
From: Andrew Cooper @ 2017-01-16 13:04 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Julien Grall, Stefano Stabellini, Jan Beulich
The chageset/version/compile information is currently exported as a set of
function calls into a separate translation unit, which is inefficient for all
callers.
Replace the function calls with externs pointing appropriately into .rodata,
which allows all users to generate code referencing the data directly.
No functional change, but causes smaller and more efficient compiled code.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Julien Grall <julien.grall@arm.com>
Restricted to $ARCH maintainers as this is a tree-wide change with no
behavioural change.
---
xen/arch/arm/traps.c | 2 +-
xen/arch/x86/hvm/viridian.c | 2 +-
xen/arch/x86/traps.c | 2 +-
xen/arch/x86/x86_64/traps.c | 2 +-
xen/common/hvm/save.c | 2 +-
xen/common/kernel.c | 18 ++++++------
xen/common/kexec.c | 14 ++++-----
xen/common/version.c | 72 ++++++++-------------------------------------
xen/drivers/char/console.c | 10 +++----
xen/include/xen/version.h | 25 ++++++++--------
10 files changed, 52 insertions(+), 97 deletions(-)
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 35d8e8b..5730452 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -144,7 +144,7 @@ static void print_xen_info(void)
char taint_str[TAINT_STRING_MAX_LEN];
printk("----[ Xen-%d.%d%s %s debug=%c " gcov_string " %s ]----\n",
- xen_major_version(), xen_minor_version(), xen_extra_version(),
+ xen_major_version, xen_minor_version, xen_extra_version,
#ifdef CONFIG_ARM_32
"arm32",
#else
diff --git a/xen/arch/x86/hvm/viridian.c b/xen/arch/x86/hvm/viridian.c
index 37c8f68..aebf6c1 100644
--- a/xen/arch/x86/hvm/viridian.c
+++ b/xen/arch/x86/hvm/viridian.c
@@ -95,7 +95,7 @@ void cpuid_viridian_leaves(const struct vcpu *v, uint32_t leaf,
if ( d->arch.hvm_domain.viridian.guest_os_id.raw == 0 )
break;
res->a = 1; /* Build number */
- res->b = (xen_major_version() << 16) | xen_minor_version();
+ res->b = (xen_major_version << 16) | xen_minor_version;
res->c = 0; /* SP */
res->d = 0; /* Service branch and number */
break;
diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index 4f29c3a..5dfd31a 100644
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -931,7 +931,7 @@ void cpuid_hypervisor_leaves(const struct vcpu *v, uint32_t leaf,
break;
case 1:
- res->a = (xen_major_version() << 16) | xen_minor_version();
+ res->a = (xen_major_version << 16) | xen_minor_version;
break;
case 2:
diff --git a/xen/arch/x86/x86_64/traps.c b/xen/arch/x86/x86_64/traps.c
index fc8cde6..b82872f 100644
--- a/xen/arch/x86/x86_64/traps.c
+++ b/xen/arch/x86/x86_64/traps.c
@@ -32,7 +32,7 @@ static void print_xen_info(void)
char taint_str[TAINT_STRING_MAX_LEN];
printk("----[ Xen-%d.%d%s x86_64 debug=%c " gcov_string " %s ]----\n",
- xen_major_version(), xen_minor_version(), xen_extra_version(),
+ xen_major_version, xen_minor_version, xen_extra_version,
debug_build() ? 'y' : 'n', print_tainted(taint_str));
}
diff --git a/xen/common/hvm/save.c b/xen/common/hvm/save.c
index dd2c547..245e852 100644
--- a/xen/common/hvm/save.c
+++ b/xen/common/hvm/save.c
@@ -152,7 +152,7 @@ int hvm_save(struct domain *d, hvm_domain_context_t *h)
hdr.version = HVM_FILE_VERSION;
/* Save xen changeset */
- c = strrchr(xen_changeset(), ':');
+ c = strrchr(xen_changeset, ':');
if ( c )
hdr.changeset = simple_strtoll(c, NULL, 16);
else
diff --git a/xen/common/kernel.c b/xen/common/kernel.c
index d0edb13..8008e40 100644
--- a/xen/common/kernel.c
+++ b/xen/common/kernel.c
@@ -233,14 +233,14 @@ DO(xen_version)(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
switch ( cmd )
{
case XENVER_version:
- return (xen_major_version() << 16) | xen_minor_version();
+ return (xen_major_version << 16) | xen_minor_version;
case XENVER_extraversion:
{
xen_extraversion_t extraversion;
memset(extraversion, 0, sizeof(extraversion));
- safe_strcpy(extraversion, deny ? xen_deny() : xen_extra_version());
+ safe_strcpy(extraversion, deny ? xen_deny : xen_extra_version);
if ( copy_to_guest(arg, extraversion, ARRAY_SIZE(extraversion)) )
return -EFAULT;
return 0;
@@ -251,10 +251,10 @@ DO(xen_version)(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
xen_compile_info_t info;
memset(&info, 0, sizeof(info));
- safe_strcpy(info.compiler, deny ? xen_deny() : xen_compiler());
- safe_strcpy(info.compile_by, deny ? xen_deny() : xen_compile_by());
- safe_strcpy(info.compile_domain, deny ? xen_deny() : xen_compile_domain());
- safe_strcpy(info.compile_date, deny ? xen_deny() : xen_compile_date());
+ safe_strcpy(info.compiler, deny ? xen_deny : xen_compiler);
+ safe_strcpy(info.compile_by, deny ? xen_deny : xen_compile_by);
+ safe_strcpy(info.compile_domain, deny ? xen_deny : xen_compile_domain);
+ safe_strcpy(info.compile_date, deny ? xen_deny : xen_compile_date);
if ( copy_to_guest(arg, &info, 1) )
return -EFAULT;
return 0;
@@ -290,7 +290,7 @@ DO(xen_version)(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
xen_changeset_info_t chgset;
memset(chgset, 0, sizeof(chgset));
- safe_strcpy(chgset, deny ? xen_deny() : xen_changeset());
+ safe_strcpy(chgset, deny ? xen_deny : xen_changeset);
if ( copy_to_guest(arg, chgset, ARRAY_SIZE(chgset)) )
return -EFAULT;
return 0;
@@ -371,9 +371,9 @@ DO(xen_version)(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
size_t len = ARRAY_SIZE(saved_cmdline);
if ( deny )
- len = strlen(xen_deny()) + 1;
+ len = strlen(xen_deny) + 1;
- if ( copy_to_guest(arg, deny ? xen_deny() : saved_cmdline, len) )
+ if ( copy_to_guest(arg, deny ? xen_deny : saved_cmdline, len) )
return -EFAULT;
return 0;
}
diff --git a/xen/common/kexec.c b/xen/common/kexec.c
index c83d48f..8d9ef79 100644
--- a/xen/common/kexec.c
+++ b/xen/common/kexec.c
@@ -311,13 +311,13 @@ crash_xen_info_t *kexec_crash_save_info(void)
BUG_ON(!cpumask_test_and_set_cpu(cpu, &crash_saved_cpus));
memset(&info, 0, sizeof(info));
- info.xen_major_version = xen_major_version();
- info.xen_minor_version = xen_minor_version();
- info.xen_extra_version = __pa(xen_extra_version());
- info.xen_changeset = __pa(xen_changeset());
- info.xen_compiler = __pa(xen_compiler());
- info.xen_compile_date = __pa(xen_compile_date());
- info.xen_compile_time = __pa(xen_compile_time());
+ info.xen_major_version = xen_major_version;
+ info.xen_minor_version = xen_minor_version;
+ info.xen_extra_version = __pa(xen_extra_version);
+ info.xen_changeset = __pa(xen_changeset);
+ info.xen_compiler = __pa(xen_compiler);
+ info.xen_compile_date = __pa(xen_compile_date);
+ info.xen_compile_time = __pa(xen_compile_time);
info.tainted = tainted;
/* Copy from guaranteed-aligned local copy to possibly-unaligned dest. */
diff --git a/xen/common/version.c b/xen/common/version.c
index 223cb52..65b74c1 100644
--- a/xen/common/version.c
+++ b/xen/common/version.c
@@ -10,65 +10,19 @@
#include <asm/cache.h>
-const char *xen_compile_date(void)
-{
- return XEN_COMPILE_DATE;
-}
-
-const char *xen_compile_time(void)
-{
- return XEN_COMPILE_TIME;
-}
-
-const char *xen_compile_by(void)
-{
- return XEN_COMPILE_BY;
-}
-
-const char *xen_compile_domain(void)
-{
- return XEN_COMPILE_DOMAIN;
-}
-
-const char *xen_compile_host(void)
-{
- return XEN_COMPILE_HOST;
-}
-
-const char *xen_compiler(void)
-{
- return XEN_COMPILER;
-}
-
-unsigned int xen_major_version(void)
-{
- return XEN_VERSION;
-}
-
-unsigned int xen_minor_version(void)
-{
- return XEN_SUBVERSION;
-}
-
-const char *xen_extra_version(void)
-{
- return XEN_EXTRAVERSION;
-}
-
-const char *xen_changeset(void)
-{
- return XEN_CHANGESET;
-}
-
-const char *xen_banner(void)
-{
- return XEN_BANNER;
-}
-
-const char *xen_deny(void)
-{
- return "<denied>";
-}
+const unsigned int xen_major_version = XEN_VERSION;
+const unsigned int xen_minor_version = XEN_SUBVERSION;
+
+const char xen_compile_date[] = XEN_COMPILE_DATE;
+const char xen_compile_time[] = XEN_COMPILE_TIME;
+const char xen_compile_by[] = XEN_COMPILE_BY;
+const char xen_compile_domain[] = XEN_COMPILE_DOMAIN;
+const char xen_compile_host[] = XEN_COMPILE_HOST;
+const char xen_compiler[] = XEN_COMPILER;
+const char xen_extra_version[] = XEN_EXTRAVERSION;
+const char xen_changeset[] = XEN_CHANGESET;
+const char xen_banner[] = XEN_BANNER;
+const char xen_deny[] = "<denied>";
static const void *build_id_p __read_mostly;
static unsigned int build_id_len __read_mostly;
diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index eb21e7c..7324595 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -777,13 +777,13 @@ void __init console_init_preirq(void)
/* HELLO WORLD --- start-of-day banner text. */
spin_lock(&console_lock);
- __putstr(xen_banner());
+ __putstr(xen_banner);
spin_unlock(&console_lock);
printk("Xen version %d.%d%s (%s@%s) (%s) debug=%c " gcov_string " %s\n",
- xen_major_version(), xen_minor_version(), xen_extra_version(),
- xen_compile_by(), xen_compile_domain(),
- xen_compiler(), debug_build() ? 'y' : 'n', xen_compile_date());
- printk("Latest ChangeSet: %s\n", xen_changeset());
+ xen_major_version, xen_minor_version, xen_extra_version,
+ xen_compile_by, xen_compile_domain,
+ xen_compiler, debug_build() ? 'y' : 'n', xen_compile_date);
+ printk("Latest ChangeSet: %s\n", xen_changeset);
if ( opt_sync_console )
{
diff --git a/xen/include/xen/version.h b/xen/include/xen/version.h
index 97c247a..dba3803 100644
--- a/xen/include/xen/version.h
+++ b/xen/include/xen/version.h
@@ -4,18 +4,19 @@
#include <xen/types.h>
#include <xen/elfstructs.h>
-const char *xen_compile_date(void);
-const char *xen_compile_time(void);
-const char *xen_compile_by(void);
-const char *xen_compile_domain(void);
-const char *xen_compile_host(void);
-const char *xen_compiler(void);
-unsigned int xen_major_version(void);
-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);
+extern const unsigned int xen_major_version, xen_minor_version;
+
+extern const char xen_compile_date[];
+extern const char xen_compile_time[];
+extern const char xen_compile_by[];
+extern const char xen_compile_domain[];
+extern const char xen_compile_host[];
+extern const char xen_compiler[];
+extern const char xen_extra_version[];
+extern const char xen_changeset[];
+extern const char xen_banner[];
+extern const char xen_deny[];
+
int xen_build_id(const void **p, unsigned int *len);
#ifdef BUILD_ID
--
2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH] xen/common: Drop function calls for Xen compile/version information
2017-01-16 13:04 [PATCH] xen/common: Drop function calls for Xen compile/version information Andrew Cooper
@ 2017-01-16 13:18 ` Jan Beulich
2017-01-16 13:26 ` Andrew Cooper
2017-01-16 18:23 ` Stefano Stabellini
` (2 subsequent siblings)
3 siblings, 1 reply; 17+ messages in thread
From: Jan Beulich @ 2017-01-16 13:18 UTC (permalink / raw)
To: Andrew Cooper; +Cc: JulienGrall, Stefano Stabellini, Xen-devel
>>> On 16.01.17 at 14:04, <andrew.cooper3@citrix.com> wrote:
> The chageset/version/compile information is currently exported as a set of
> function calls into a separate translation unit, which is inefficient for all
> callers.
>
> Replace the function calls with externs pointing appropriately into .rodata,
> which allows all users to generate code referencing the data directly.
>
> No functional change, but causes smaller and more efficient compiled code.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Provided especially the "smaller" part above really hold at least
on average over the entire change (which I'm not convinced it
does),
Reviewed-by: Jan Beulich <jbeulich@suse.com>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] xen/common: Drop function calls for Xen compile/version information
2017-01-16 13:18 ` Jan Beulich
@ 2017-01-16 13:26 ` Andrew Cooper
0 siblings, 0 replies; 17+ messages in thread
From: Andrew Cooper @ 2017-01-16 13:26 UTC (permalink / raw)
To: Jan Beulich; +Cc: JulienGrall, Stefano Stabellini, Xen-devel
On 16/01/17 13:18, Jan Beulich wrote:
>>>> On 16.01.17 at 14:04, <andrew.cooper3@citrix.com> wrote:
>> The chageset/version/compile information is currently exported as a set of
>> function calls into a separate translation unit, which is inefficient for all
>> callers.
>>
>> Replace the function calls with externs pointing appropriately into .rodata,
>> which allows all users to generate code referencing the data directly.
>>
>> No functional change, but causes smaller and more efficient compiled code.
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Provided especially the "smaller" part above really hold at least
> on average over the entire change (which I'm not convinced it
> does),
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
>
andrewcoop@andrewcoop:/local/xen.git/xen$ ../scripts/bloat-o-meter
xen-syms-before xen-syms-after
add/remove: 0/0 grow/shrink: 5/15 up/down: 485/-256 (229)
function old new delta
xen_banner 13 439 +426
xen_changeset 13 37 +24
xen_compile_date 13 29 +16
xen_compiler 13 28 +15
xen_compile_domain 13 17 +4
hvm_save 409 408 -1
xen_compile_host 13 11 -2
xen_compile_by 13 11 -2
xen_extra_version 13 10 -3
xen_deny 13 9 -4
xen_compile_time 13 9 -4
xen_minor_version 11 4 -7
xen_major_version 11 4 -7
cpuid_viridian_leaves 439 421 -18
print_xen_info 89 64 -25
do_xen_version 1915 1889 -26
compat_xen_version 1849 1823 -26
console_init_preirq 487 455 -32
cpuid_hypervisor_leaves 656 612 -44
kexec_crash_save_info 898 843 -55
The data has switched from being anonymous strings to named objects
replacing the functions, so the actual delta is +0/-321
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] xen/common: Drop function calls for Xen compile/version information
2017-01-16 13:04 [PATCH] xen/common: Drop function calls for Xen compile/version information Andrew Cooper
2017-01-16 13:18 ` Jan Beulich
@ 2017-01-16 18:23 ` Stefano Stabellini
2017-01-16 19:44 ` Doug Goldstein
2017-01-17 18:05 ` Konrad Rzeszutek Wilk
3 siblings, 0 replies; 17+ messages in thread
From: Stefano Stabellini @ 2017-01-16 18:23 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Julien Grall, Stefano Stabellini, Jan Beulich, Xen-devel
On Mon, 16 Jan 2017, Andrew Cooper wrote:
> The chageset/version/compile information is currently exported as a set of
> function calls into a separate translation unit, which is inefficient for all
> callers.
>
> Replace the function calls with externs pointing appropriately into .rodata,
> which allows all users to generate code referencing the data directly.
>
> No functional change, but causes smaller and more efficient compiled code.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Stefano Stabellini <sstabellini@kernel.org>
> CC: Julien Grall <julien.grall@arm.com>
>
> Restricted to $ARCH maintainers as this is a tree-wide change with no
> behavioural change.
> ---
> xen/arch/arm/traps.c | 2 +-
> xen/arch/x86/hvm/viridian.c | 2 +-
> xen/arch/x86/traps.c | 2 +-
> xen/arch/x86/x86_64/traps.c | 2 +-
> xen/common/hvm/save.c | 2 +-
> xen/common/kernel.c | 18 ++++++------
> xen/common/kexec.c | 14 ++++-----
> xen/common/version.c | 72 ++++++++-------------------------------------
> xen/drivers/char/console.c | 10 +++----
> xen/include/xen/version.h | 25 ++++++++--------
> 10 files changed, 52 insertions(+), 97 deletions(-)
>
> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> index 35d8e8b..5730452 100644
> --- a/xen/arch/arm/traps.c
> +++ b/xen/arch/arm/traps.c
> @@ -144,7 +144,7 @@ static void print_xen_info(void)
> char taint_str[TAINT_STRING_MAX_LEN];
>
> printk("----[ Xen-%d.%d%s %s debug=%c " gcov_string " %s ]----\n",
> - xen_major_version(), xen_minor_version(), xen_extra_version(),
> + xen_major_version, xen_minor_version, xen_extra_version,
> #ifdef CONFIG_ARM_32
> "arm32",
> #else
> diff --git a/xen/arch/x86/hvm/viridian.c b/xen/arch/x86/hvm/viridian.c
> index 37c8f68..aebf6c1 100644
> --- a/xen/arch/x86/hvm/viridian.c
> +++ b/xen/arch/x86/hvm/viridian.c
> @@ -95,7 +95,7 @@ void cpuid_viridian_leaves(const struct vcpu *v, uint32_t leaf,
> if ( d->arch.hvm_domain.viridian.guest_os_id.raw == 0 )
> break;
> res->a = 1; /* Build number */
> - res->b = (xen_major_version() << 16) | xen_minor_version();
> + res->b = (xen_major_version << 16) | xen_minor_version;
> res->c = 0; /* SP */
> res->d = 0; /* Service branch and number */
> break;
> diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
> index 4f29c3a..5dfd31a 100644
> --- a/xen/arch/x86/traps.c
> +++ b/xen/arch/x86/traps.c
> @@ -931,7 +931,7 @@ void cpuid_hypervisor_leaves(const struct vcpu *v, uint32_t leaf,
> break;
>
> case 1:
> - res->a = (xen_major_version() << 16) | xen_minor_version();
> + res->a = (xen_major_version << 16) | xen_minor_version;
> break;
>
> case 2:
> diff --git a/xen/arch/x86/x86_64/traps.c b/xen/arch/x86/x86_64/traps.c
> index fc8cde6..b82872f 100644
> --- a/xen/arch/x86/x86_64/traps.c
> +++ b/xen/arch/x86/x86_64/traps.c
> @@ -32,7 +32,7 @@ static void print_xen_info(void)
> char taint_str[TAINT_STRING_MAX_LEN];
>
> printk("----[ Xen-%d.%d%s x86_64 debug=%c " gcov_string " %s ]----\n",
> - xen_major_version(), xen_minor_version(), xen_extra_version(),
> + xen_major_version, xen_minor_version, xen_extra_version,
> debug_build() ? 'y' : 'n', print_tainted(taint_str));
> }
>
> diff --git a/xen/common/hvm/save.c b/xen/common/hvm/save.c
> index dd2c547..245e852 100644
> --- a/xen/common/hvm/save.c
> +++ b/xen/common/hvm/save.c
> @@ -152,7 +152,7 @@ int hvm_save(struct domain *d, hvm_domain_context_t *h)
> hdr.version = HVM_FILE_VERSION;
>
> /* Save xen changeset */
> - c = strrchr(xen_changeset(), ':');
> + c = strrchr(xen_changeset, ':');
> if ( c )
> hdr.changeset = simple_strtoll(c, NULL, 16);
> else
> diff --git a/xen/common/kernel.c b/xen/common/kernel.c
> index d0edb13..8008e40 100644
> --- a/xen/common/kernel.c
> +++ b/xen/common/kernel.c
> @@ -233,14 +233,14 @@ DO(xen_version)(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
> switch ( cmd )
> {
> case XENVER_version:
> - return (xen_major_version() << 16) | xen_minor_version();
> + return (xen_major_version << 16) | xen_minor_version;
>
> case XENVER_extraversion:
> {
> xen_extraversion_t extraversion;
>
> memset(extraversion, 0, sizeof(extraversion));
> - safe_strcpy(extraversion, deny ? xen_deny() : xen_extra_version());
> + safe_strcpy(extraversion, deny ? xen_deny : xen_extra_version);
> if ( copy_to_guest(arg, extraversion, ARRAY_SIZE(extraversion)) )
> return -EFAULT;
> return 0;
> @@ -251,10 +251,10 @@ DO(xen_version)(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
> xen_compile_info_t info;
>
> memset(&info, 0, sizeof(info));
> - safe_strcpy(info.compiler, deny ? xen_deny() : xen_compiler());
> - safe_strcpy(info.compile_by, deny ? xen_deny() : xen_compile_by());
> - safe_strcpy(info.compile_domain, deny ? xen_deny() : xen_compile_domain());
> - safe_strcpy(info.compile_date, deny ? xen_deny() : xen_compile_date());
> + safe_strcpy(info.compiler, deny ? xen_deny : xen_compiler);
> + safe_strcpy(info.compile_by, deny ? xen_deny : xen_compile_by);
> + safe_strcpy(info.compile_domain, deny ? xen_deny : xen_compile_domain);
> + safe_strcpy(info.compile_date, deny ? xen_deny : xen_compile_date);
> if ( copy_to_guest(arg, &info, 1) )
> return -EFAULT;
> return 0;
> @@ -290,7 +290,7 @@ DO(xen_version)(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
> xen_changeset_info_t chgset;
>
> memset(chgset, 0, sizeof(chgset));
> - safe_strcpy(chgset, deny ? xen_deny() : xen_changeset());
> + safe_strcpy(chgset, deny ? xen_deny : xen_changeset);
> if ( copy_to_guest(arg, chgset, ARRAY_SIZE(chgset)) )
> return -EFAULT;
> return 0;
> @@ -371,9 +371,9 @@ DO(xen_version)(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
> size_t len = ARRAY_SIZE(saved_cmdline);
>
> if ( deny )
> - len = strlen(xen_deny()) + 1;
> + len = strlen(xen_deny) + 1;
>
> - if ( copy_to_guest(arg, deny ? xen_deny() : saved_cmdline, len) )
> + if ( copy_to_guest(arg, deny ? xen_deny : saved_cmdline, len) )
> return -EFAULT;
> return 0;
> }
> diff --git a/xen/common/kexec.c b/xen/common/kexec.c
> index c83d48f..8d9ef79 100644
> --- a/xen/common/kexec.c
> +++ b/xen/common/kexec.c
> @@ -311,13 +311,13 @@ crash_xen_info_t *kexec_crash_save_info(void)
> BUG_ON(!cpumask_test_and_set_cpu(cpu, &crash_saved_cpus));
>
> memset(&info, 0, sizeof(info));
> - info.xen_major_version = xen_major_version();
> - info.xen_minor_version = xen_minor_version();
> - info.xen_extra_version = __pa(xen_extra_version());
> - info.xen_changeset = __pa(xen_changeset());
> - info.xen_compiler = __pa(xen_compiler());
> - info.xen_compile_date = __pa(xen_compile_date());
> - info.xen_compile_time = __pa(xen_compile_time());
> + info.xen_major_version = xen_major_version;
> + info.xen_minor_version = xen_minor_version;
> + info.xen_extra_version = __pa(xen_extra_version);
> + info.xen_changeset = __pa(xen_changeset);
> + info.xen_compiler = __pa(xen_compiler);
> + info.xen_compile_date = __pa(xen_compile_date);
> + info.xen_compile_time = __pa(xen_compile_time);
> info.tainted = tainted;
>
> /* Copy from guaranteed-aligned local copy to possibly-unaligned dest. */
> diff --git a/xen/common/version.c b/xen/common/version.c
> index 223cb52..65b74c1 100644
> --- a/xen/common/version.c
> +++ b/xen/common/version.c
> @@ -10,65 +10,19 @@
>
> #include <asm/cache.h>
>
> -const char *xen_compile_date(void)
> -{
> - return XEN_COMPILE_DATE;
> -}
> -
> -const char *xen_compile_time(void)
> -{
> - return XEN_COMPILE_TIME;
> -}
> -
> -const char *xen_compile_by(void)
> -{
> - return XEN_COMPILE_BY;
> -}
> -
> -const char *xen_compile_domain(void)
> -{
> - return XEN_COMPILE_DOMAIN;
> -}
> -
> -const char *xen_compile_host(void)
> -{
> - return XEN_COMPILE_HOST;
> -}
> -
> -const char *xen_compiler(void)
> -{
> - return XEN_COMPILER;
> -}
> -
> -unsigned int xen_major_version(void)
> -{
> - return XEN_VERSION;
> -}
> -
> -unsigned int xen_minor_version(void)
> -{
> - return XEN_SUBVERSION;
> -}
> -
> -const char *xen_extra_version(void)
> -{
> - return XEN_EXTRAVERSION;
> -}
> -
> -const char *xen_changeset(void)
> -{
> - return XEN_CHANGESET;
> -}
> -
> -const char *xen_banner(void)
> -{
> - return XEN_BANNER;
> -}
> -
> -const char *xen_deny(void)
> -{
> - return "<denied>";
> -}
> +const unsigned int xen_major_version = XEN_VERSION;
> +const unsigned int xen_minor_version = XEN_SUBVERSION;
> +
> +const char xen_compile_date[] = XEN_COMPILE_DATE;
> +const char xen_compile_time[] = XEN_COMPILE_TIME;
> +const char xen_compile_by[] = XEN_COMPILE_BY;
> +const char xen_compile_domain[] = XEN_COMPILE_DOMAIN;
> +const char xen_compile_host[] = XEN_COMPILE_HOST;
> +const char xen_compiler[] = XEN_COMPILER;
> +const char xen_extra_version[] = XEN_EXTRAVERSION;
> +const char xen_changeset[] = XEN_CHANGESET;
> +const char xen_banner[] = XEN_BANNER;
> +const char xen_deny[] = "<denied>";
>
> static const void *build_id_p __read_mostly;
> static unsigned int build_id_len __read_mostly;
> diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
> index eb21e7c..7324595 100644
> --- a/xen/drivers/char/console.c
> +++ b/xen/drivers/char/console.c
> @@ -777,13 +777,13 @@ void __init console_init_preirq(void)
>
> /* HELLO WORLD --- start-of-day banner text. */
> spin_lock(&console_lock);
> - __putstr(xen_banner());
> + __putstr(xen_banner);
> spin_unlock(&console_lock);
> printk("Xen version %d.%d%s (%s@%s) (%s) debug=%c " gcov_string " %s\n",
> - xen_major_version(), xen_minor_version(), xen_extra_version(),
> - xen_compile_by(), xen_compile_domain(),
> - xen_compiler(), debug_build() ? 'y' : 'n', xen_compile_date());
> - printk("Latest ChangeSet: %s\n", xen_changeset());
> + xen_major_version, xen_minor_version, xen_extra_version,
> + xen_compile_by, xen_compile_domain,
> + xen_compiler, debug_build() ? 'y' : 'n', xen_compile_date);
> + printk("Latest ChangeSet: %s\n", xen_changeset);
>
> if ( opt_sync_console )
> {
> diff --git a/xen/include/xen/version.h b/xen/include/xen/version.h
> index 97c247a..dba3803 100644
> --- a/xen/include/xen/version.h
> +++ b/xen/include/xen/version.h
> @@ -4,18 +4,19 @@
> #include <xen/types.h>
> #include <xen/elfstructs.h>
>
> -const char *xen_compile_date(void);
> -const char *xen_compile_time(void);
> -const char *xen_compile_by(void);
> -const char *xen_compile_domain(void);
> -const char *xen_compile_host(void);
> -const char *xen_compiler(void);
> -unsigned int xen_major_version(void);
> -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);
> +extern const unsigned int xen_major_version, xen_minor_version;
> +
> +extern const char xen_compile_date[];
> +extern const char xen_compile_time[];
> +extern const char xen_compile_by[];
> +extern const char xen_compile_domain[];
> +extern const char xen_compile_host[];
> +extern const char xen_compiler[];
> +extern const char xen_extra_version[];
> +extern const char xen_changeset[];
> +extern const char xen_banner[];
> +extern const char xen_deny[];
> +
> int xen_build_id(const void **p, unsigned int *len);
>
> #ifdef BUILD_ID
> --
> 2.1.4
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] xen/common: Drop function calls for Xen compile/version information
2017-01-16 13:04 [PATCH] xen/common: Drop function calls for Xen compile/version information Andrew Cooper
2017-01-16 13:18 ` Jan Beulich
2017-01-16 18:23 ` Stefano Stabellini
@ 2017-01-16 19:44 ` Doug Goldstein
2017-01-17 18:05 ` Konrad Rzeszutek Wilk
3 siblings, 0 replies; 17+ messages in thread
From: Doug Goldstein @ 2017-01-16 19:44 UTC (permalink / raw)
To: Andrew Cooper, Xen-devel; +Cc: Julien Grall, Stefano Stabellini, Jan Beulich
[-- Attachment #1.1.1: Type: text/plain, Size: 593 bytes --]
On 1/16/17 8:04 AM, Andrew Cooper wrote:
> The chageset/version/compile information is currently exported as a set of
> function calls into a separate translation unit, which is inefficient for all
> callers.
>
> Replace the function calls with externs pointing appropriately into .rodata,
> which allows all users to generate code referencing the data directly.
>
> No functional change, but causes smaller and more efficient compiled code.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Doug Goldstein <cardoe@cardoe.com>
--
Doug Goldstein
[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 959 bytes --]
[-- Attachment #2: Type: text/plain, Size: 127 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] xen/common: Drop function calls for Xen compile/version information
2017-01-16 13:04 [PATCH] xen/common: Drop function calls for Xen compile/version information Andrew Cooper
` (2 preceding siblings ...)
2017-01-16 19:44 ` Doug Goldstein
@ 2017-01-17 18:05 ` Konrad Rzeszutek Wilk
2017-01-17 18:16 ` Andrew Cooper
3 siblings, 1 reply; 17+ messages in thread
From: Konrad Rzeszutek Wilk @ 2017-01-17 18:05 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Julien Grall, Stefano Stabellini, Jan Beulich, Xen-devel
On Mon, Jan 16, 2017 at 01:04:09PM +0000, Andrew Cooper wrote:
> The chageset/version/compile information is currently exported as a set of
> function calls into a separate translation unit, which is inefficient for all
> callers.
>
> Replace the function calls with externs pointing appropriately into .rodata,
> which allows all users to generate code referencing the data directly.
>
> No functional change, but causes smaller and more efficient compiled code.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Ah crud. That breaks the livepatch test-cases (they patch the xen_extra_version
function).
Are there some other code that can be modified that is reported
by 'xl info' on which the test-cases can run (and reported easily?).
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] xen/common: Drop function calls for Xen compile/version information
2017-01-17 18:05 ` Konrad Rzeszutek Wilk
@ 2017-01-17 18:16 ` Andrew Cooper
2017-01-17 18:42 ` Konrad Rzeszutek Wilk
0 siblings, 1 reply; 17+ messages in thread
From: Andrew Cooper @ 2017-01-17 18:16 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk
Cc: Julien Grall, Stefano Stabellini, Jan Beulich, Xen-devel
On 17/01/17 18:05, Konrad Rzeszutek Wilk wrote:
> On Mon, Jan 16, 2017 at 01:04:09PM +0000, Andrew Cooper wrote:
>> The chageset/version/compile information is currently exported as a set of
>> function calls into a separate translation unit, which is inefficient for all
>> callers.
>>
>> Replace the function calls with externs pointing appropriately into .rodata,
>> which allows all users to generate code referencing the data directly.
>>
>> No functional change, but causes smaller and more efficient compiled code.
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Ah crud. That breaks the livepatch test-cases (they patch the xen_extra_version
> function).
Lucky I haven't pushed it then, (although the livepatch build seems to
still work fine for me, despite this change.)
> Are there some other code that can be modified that is reported
> by 'xl info' on which the test-cases can run (and reported easily?).
Patch do_version() itself to return the same difference of information?
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] xen/common: Drop function calls for Xen compile/version information
2017-01-17 18:16 ` Andrew Cooper
@ 2017-01-17 18:42 ` Konrad Rzeszutek Wilk
2017-01-17 19:00 ` Konrad Rzeszutek Wilk
0 siblings, 1 reply; 17+ messages in thread
From: Konrad Rzeszutek Wilk @ 2017-01-17 18:42 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Julien Grall, Stefano Stabellini, Jan Beulich, Xen-devel
On Tue, Jan 17, 2017 at 06:16:36PM +0000, Andrew Cooper wrote:
> On 17/01/17 18:05, Konrad Rzeszutek Wilk wrote:
> > On Mon, Jan 16, 2017 at 01:04:09PM +0000, Andrew Cooper wrote:
> >> The chageset/version/compile information is currently exported as a set of
> >> function calls into a separate translation unit, which is inefficient for all
> >> callers.
> >>
> >> Replace the function calls with externs pointing appropriately into .rodata,
> >> which allows all users to generate code referencing the data directly.
> >>
> >> No functional change, but causes smaller and more efficient compiled code.
> >>
> >> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> > Ah crud. That breaks the livepatch test-cases (they patch the xen_extra_version
> > function).
>
> Lucky I haven't pushed it then, (although the livepatch build seems to
> still work fine for me, despite this change.)
make tests should fail.
>
> > Are there some other code that can be modified that is reported
> > by 'xl info' on which the test-cases can run (and reported easily?).
>
> Patch do_version() itself to return the same difference of information?
Ugh. That is going to make the building of test-cases quite complex.
I guess it can just do it and .. return only one value :-)
>
> ~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] xen/common: Drop function calls for Xen compile/version information
2017-01-17 18:42 ` Konrad Rzeszutek Wilk
@ 2017-01-17 19:00 ` Konrad Rzeszutek Wilk
2017-01-17 20:39 ` Andrew Cooper
0 siblings, 1 reply; 17+ messages in thread
From: Konrad Rzeszutek Wilk @ 2017-01-17 19:00 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Julien Grall, Stefano Stabellini, Jan Beulich, Xen-devel
On Tue, Jan 17, 2017 at 01:42:54PM -0500, Konrad Rzeszutek Wilk wrote:
> On Tue, Jan 17, 2017 at 06:16:36PM +0000, Andrew Cooper wrote:
> > On 17/01/17 18:05, Konrad Rzeszutek Wilk wrote:
> > > On Mon, Jan 16, 2017 at 01:04:09PM +0000, Andrew Cooper wrote:
> > >> The chageset/version/compile information is currently exported as a set of
> > >> function calls into a separate translation unit, which is inefficient for all
> > >> callers.
> > >>
> > >> Replace the function calls with externs pointing appropriately into .rodata,
> > >> which allows all users to generate code referencing the data directly.
> > >>
> > >> No functional change, but causes smaller and more efficient compiled code.
> > >>
> > >> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> > > Ah crud. That breaks the livepatch test-cases (they patch the xen_extra_version
> > > function).
> >
> > Lucky I haven't pushed it then, (although the livepatch build seems to
> > still work fine for me, despite this change.)
>
> make tests should fail.
(which is not by built by default as requested by Jan - but the
OSSTest test cases I am working would do this).
>
> >
> > > Are there some other code that can be modified that is reported
> > > by 'xl info' on which the test-cases can run (and reported easily?).
> >
> > Patch do_version() itself to return the same difference of information?
>
> Ugh. That is going to make the building of test-cases quite complex.
>
> I guess it can just do it and .. return only one value :-)
As in something like this (not compile tested):
diff --git a/xen/arch/x86/test/xen_hello_world_func.c b/xen/arch/x86/test/xen_hello_world_func.c
index 2e4af9c..3572600 100644
--- a/xen/arch/x86/test/xen_hello_world_func.c
+++ b/xen/arch/x86/test/xen_hello_world_func.c
@@ -10,7 +10,7 @@
static unsigned long *non_canonical_addr = (unsigned long *)0xdead000000000000ULL;
/* Our replacement function for xen_extra_version. */
-const char *xen_hello_world(void)
+const char *xen_version_hello_world(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
{
unsigned long tmp;
int rc;
@@ -21,7 +21,19 @@ const char *xen_hello_world(void)
rc = __get_user(tmp, non_canonical_addr);
BUG_ON(rc != -EFAULT);
- return "Hello World";
+ if ( cmd == XENVER_extraversion )
+ {
+ xen_extraversion_t extraversion = "Hello World";
+
+ if ( copy_to_guest(arg, extraversion, ARRAY_SIZE(extraversion)) )
+ return -EFAULT;
+ return 0;
+ }
+ /*
+ * Can't return -EPERM as certain subversions can't deal with negative
+ * values.
+ */
+ return 0;
}
That will make three of the test-patches work but not for the last
one - the NOP one (which needs to patch the _whole_ function).
Argh.
Is this patch of yours that neccessary? Could at least some of the
functions still exist?
Like
xen_minor_version() and xen_extra_verison() ?
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH] xen/common: Drop function calls for Xen compile/version information
2017-01-17 19:00 ` Konrad Rzeszutek Wilk
@ 2017-01-17 20:39 ` Andrew Cooper
2017-01-17 20:46 ` Konrad Rzeszutek Wilk
0 siblings, 1 reply; 17+ messages in thread
From: Andrew Cooper @ 2017-01-17 20:39 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk
Cc: Julien Grall, Stefano Stabellini, Jan Beulich, Xen-devel
On 17/01/17 19:00, Konrad Rzeszutek Wilk wrote:
> On Tue, Jan 17, 2017 at 01:42:54PM -0500, Konrad Rzeszutek Wilk wrote:
>> On Tue, Jan 17, 2017 at 06:16:36PM +0000, Andrew Cooper wrote:
>>> On 17/01/17 18:05, Konrad Rzeszutek Wilk wrote:
>>>> On Mon, Jan 16, 2017 at 01:04:09PM +0000, Andrew Cooper wrote:
>>>>> The chageset/version/compile information is currently exported as a set of
>>>>> function calls into a separate translation unit, which is inefficient for all
>>>>> callers.
>>>>>
>>>>> Replace the function calls with externs pointing appropriately into .rodata,
>>>>> which allows all users to generate code referencing the data directly.
>>>>>
>>>>> No functional change, but causes smaller and more efficient compiled code.
>>>>>
>>>>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>>>> Ah crud. That breaks the livepatch test-cases (they patch the xen_extra_version
>>>> function).
>>> Lucky I haven't pushed it then, (although the livepatch build seems to
>>> still work fine for me, despite this change.)
>> make tests should fail.
> (which is not by built by default as requested by Jan - but the
> OSSTest test cases I am working would do this).
>>>> Are there some other code that can be modified that is reported
>>>> by 'xl info' on which the test-cases can run (and reported easily?).
>>> Patch do_version() itself to return the same difference of information?
>> Ugh. That is going to make the building of test-cases quite complex.
>>
>> I guess it can just do it and .. return only one value :-)
> As in something like this (not compile tested):
>
>
> diff --git a/xen/arch/x86/test/xen_hello_world_func.c b/xen/arch/x86/test/xen_hello_world_func.c
> index 2e4af9c..3572600 100644
> --- a/xen/arch/x86/test/xen_hello_world_func.c
> +++ b/xen/arch/x86/test/xen_hello_world_func.c
> @@ -10,7 +10,7 @@
> static unsigned long *non_canonical_addr = (unsigned long *)0xdead000000000000ULL;
>
> /* Our replacement function for xen_extra_version. */
> -const char *xen_hello_world(void)
> +const char *xen_version_hello_world(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
> {
> unsigned long tmp;
> int rc;
> @@ -21,7 +21,19 @@ const char *xen_hello_world(void)
> rc = __get_user(tmp, non_canonical_addr);
> BUG_ON(rc != -EFAULT);
>
> - return "Hello World";
> + if ( cmd == XENVER_extraversion )
> + {
> + xen_extraversion_t extraversion = "Hello World";
> +
> + if ( copy_to_guest(arg, extraversion, ARRAY_SIZE(extraversion)) )
> + return -EFAULT;
> + return 0;
> + }
> + /*
> + * Can't return -EPERM as certain subversions can't deal with negative
> + * values.
> + */
> + return 0;
> }
>
>
> That will make three of the test-patches work but not for the last
> one - the NOP one (which needs to patch the _whole_ function).
>
> Argh.
>
> Is this patch of yours that neccessary? Could at least some of the
> functions still exist?
Well. This patch is manually doing what LTO would do automatically when
it has a cross-translation-unit view of things, and come to the
conclusion that in all cases, inlining cross-unit will make the calling
code shorter, and allow the functions to be elided entirely.
I take it from this that noone has tried livepatching an LTO build of Xen.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] xen/common: Drop function calls for Xen compile/version information
2017-01-17 20:39 ` Andrew Cooper
@ 2017-01-17 20:46 ` Konrad Rzeszutek Wilk
2017-01-18 10:13 ` Roger Pau Monné
0 siblings, 1 reply; 17+ messages in thread
From: Konrad Rzeszutek Wilk @ 2017-01-17 20:46 UTC (permalink / raw)
To: Andrew Cooper, roger.pau
Cc: Julien Grall, Stefano Stabellini, Jan Beulich, Xen-devel
> > Is this patch of yours that neccessary? Could at least some of the
> > functions still exist?
>
> Well. This patch is manually doing what LTO would do automatically when
> it has a cross-translation-unit view of things, and come to the
> conclusion that in all cases, inlining cross-unit will make the calling
> code shorter, and allow the functions to be elided entirely.
>
> I take it from this that noone has tried livepatching an LTO build of Xen.
Roger did build it (FreeBSD uses it), but I don't think he tried
the test-cases.
Wait, I think he may have as his patch: f8c66c2ad2efdb281e4ebf15bf329d73c4f02ce7
Author: Roger Pau Monne <roger.pau@citrix.com>
Date: Tue May 3 12:55:09 2016 +0200
xen/xsplice: add ELFOSABI_FREEBSD as a supported OSABI for payloads
implies that he did build an payload (and I remember us trying to figure
out if the issues he had been hitting were due to LTO or something else).
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] xen/common: Drop function calls for Xen compile/version information
2017-01-17 20:46 ` Konrad Rzeszutek Wilk
@ 2017-01-18 10:13 ` Roger Pau Monné
2017-01-18 16:10 ` Konrad Rzeszutek Wilk
0 siblings, 1 reply; 17+ messages in thread
From: Roger Pau Monné @ 2017-01-18 10:13 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk
Cc: Andrew Cooper, Julien Grall, Stefano Stabellini, Jan Beulich, Xen-devel
On Tue, Jan 17, 2017 at 03:46:30PM -0500, Konrad Rzeszutek Wilk wrote:
> > > Is this patch of yours that neccessary? Could at least some of the
> > > functions still exist?
> >
> > Well. This patch is manually doing what LTO would do automatically when
> > it has a cross-translation-unit view of things, and come to the
> > conclusion that in all cases, inlining cross-unit will make the calling
> > code shorter, and allow the functions to be elided entirely.
> >
> > I take it from this that noone has tried livepatching an LTO build of Xen.
>
> Roger did build it (FreeBSD uses it), but I don't think he tried
> the test-cases.
I've tried livepatch, but not LTO, neither on it's own.
> Wait, I think he may have as his patch: f8c66c2ad2efdb281e4ebf15bf329d73c4f02ce7
> Author: Roger Pau Monne <roger.pau@citrix.com>
> Date: Tue May 3 12:55:09 2016 +0200
>
> xen/xsplice: add ELFOSABI_FREEBSD as a supported OSABI for payloads
>
> implies that he did build an payload (and I remember us trying to figure
> out if the issues he had been hitting were due to LTO or something else).
IIRC issues where related to not translating error codes from the return of the
livepatch hypercalls, and the patch upload tool refusing to upload patches with
ELFOSABI_FREEBSD instead of ELFOSABI_NONE (which I assume is what Linux uses).
It's been sometime since I've tried livepatch with FreeBSD, so I could give it
a spin if you think there's a chance things might have broken.
Roger.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] xen/common: Drop function calls for Xen compile/version information
2017-01-18 10:13 ` Roger Pau Monné
@ 2017-01-18 16:10 ` Konrad Rzeszutek Wilk
2017-01-18 16:21 ` Roger Pau Monné
0 siblings, 1 reply; 17+ messages in thread
From: Konrad Rzeszutek Wilk @ 2017-01-18 16:10 UTC (permalink / raw)
To: Roger Pau Monné
Cc: Andrew Cooper, Julien Grall, Stefano Stabellini, Jan Beulich, Xen-devel
On Wed, Jan 18, 2017 at 10:13:34AM +0000, Roger Pau Monné wrote:
> On Tue, Jan 17, 2017 at 03:46:30PM -0500, Konrad Rzeszutek Wilk wrote:
> > > > Is this patch of yours that neccessary? Could at least some of the
> > > > functions still exist?
> > >
> > > Well. This patch is manually doing what LTO would do automatically when
> > > it has a cross-translation-unit view of things, and come to the
> > > conclusion that in all cases, inlining cross-unit will make the calling
> > > code shorter, and allow the functions to be elided entirely.
> > >
> > > I take it from this that noone has tried livepatching an LTO build of Xen.
> >
> > Roger did build it (FreeBSD uses it), but I don't think he tried
> > the test-cases.
>
> I've tried livepatch, but not LTO, neither on it's own.
Gotcha.
>
> > Wait, I think he may have as his patch: f8c66c2ad2efdb281e4ebf15bf329d73c4f02ce7
> > Author: Roger Pau Monne <roger.pau@citrix.com>
> > Date: Tue May 3 12:55:09 2016 +0200
> >
> > xen/xsplice: add ELFOSABI_FREEBSD as a supported OSABI for payloads
> >
> > implies that he did build an payload (and I remember us trying to figure
> > out if the issues he had been hitting were due to LTO or something else).
>
> IIRC issues where related to not translating error codes from the return of the
> livepatch hypercalls, and the patch upload tool refusing to upload patches with
> ELFOSABI_FREEBSD instead of ELFOSABI_NONE (which I assume is what Linux uses).
>
> It's been sometime since I've tried livepatch with FreeBSD, so I could give it
> a spin if you think there's a chance things might have broken.
That is OK.
I think I can also try myself to build Xen with LTO on Linux and see how
the test-cases fare.
Not sure what is involved but I will find out.
And to answer Andrew's question - No, nobody tried LTO with livepatching then :-(
>
> Roger.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] xen/common: Drop function calls for Xen compile/version information
2017-01-18 16:10 ` Konrad Rzeszutek Wilk
@ 2017-01-18 16:21 ` Roger Pau Monné
2017-01-18 16:30 ` Wei Liu
0 siblings, 1 reply; 17+ messages in thread
From: Roger Pau Monné @ 2017-01-18 16:21 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk
Cc: Stefano Stabellini, Wei Liu, Andrew Cooper, Xen-devel,
Julien Grall, Jan Beulich
On Wed, Jan 18, 2017 at 11:10:40AM -0500, Konrad Rzeszutek Wilk wrote:
> On Wed, Jan 18, 2017 at 10:13:34AM +0000, Roger Pau Monné wrote:
> > On Tue, Jan 17, 2017 at 03:46:30PM -0500, Konrad Rzeszutek Wilk wrote:
> > > > > Is this patch of yours that neccessary? Could at least some of the
> > > > > functions still exist?
> > > >
> > > > Well. This patch is manually doing what LTO would do automatically when
> > > > it has a cross-translation-unit view of things, and come to the
> > > > conclusion that in all cases, inlining cross-unit will make the calling
> > > > code shorter, and allow the functions to be elided entirely.
> > > >
> > > > I take it from this that noone has tried livepatching an LTO build of Xen.
> > >
> > > Roger did build it (FreeBSD uses it), but I don't think he tried
> > > the test-cases.
> >
> > I've tried livepatch, but not LTO, neither on it's own.
>
> Gotcha.
> >
> > > Wait, I think he may have as his patch: f8c66c2ad2efdb281e4ebf15bf329d73c4f02ce7
> > > Author: Roger Pau Monne <roger.pau@citrix.com>
> > > Date: Tue May 3 12:55:09 2016 +0200
> > >
> > > xen/xsplice: add ELFOSABI_FREEBSD as a supported OSABI for payloads
> > >
> > > implies that he did build an payload (and I remember us trying to figure
> > > out if the issues he had been hitting were due to LTO or something else).
> >
> > IIRC issues where related to not translating error codes from the return of the
> > livepatch hypercalls, and the patch upload tool refusing to upload patches with
> > ELFOSABI_FREEBSD instead of ELFOSABI_NONE (which I assume is what Linux uses).
> >
> > It's been sometime since I've tried livepatch with FreeBSD, so I could give it
> > a spin if you think there's a chance things might have broken.
>
> That is OK.
>
> I think I can also try myself to build Xen with LTO on Linux and see how
> the test-cases fare.
>
> Not sure what is involved but I will find out.
>
> And to answer Andrew's question - No, nobody tried LTO with livepatching then :-(
TBH, I think nobody has tried LTO on it's own in a long time, much less with
livepatch. Adding Wei who recently sent some LTO makefile fixes, maybe he tried
it.
Roger.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] xen/common: Drop function calls for Xen compile/version information
2017-01-18 16:21 ` Roger Pau Monné
@ 2017-01-18 16:30 ` Wei Liu
2017-01-18 17:04 ` Andrew Cooper
0 siblings, 1 reply; 17+ messages in thread
From: Wei Liu @ 2017-01-18 16:30 UTC (permalink / raw)
To: Roger Pau Monné
Cc: Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk,
Andrew Cooper, Xen-devel, Julien Grall, Jan Beulich
On Wed, Jan 18, 2017 at 04:21:06PM +0000, Roger Pau Monné wrote:
> On Wed, Jan 18, 2017 at 11:10:40AM -0500, Konrad Rzeszutek Wilk wrote:
> > On Wed, Jan 18, 2017 at 10:13:34AM +0000, Roger Pau Monné wrote:
> > > On Tue, Jan 17, 2017 at 03:46:30PM -0500, Konrad Rzeszutek Wilk wrote:
> > > > > > Is this patch of yours that neccessary? Could at least some of the
> > > > > > functions still exist?
> > > > >
> > > > > Well. This patch is manually doing what LTO would do automatically when
> > > > > it has a cross-translation-unit view of things, and come to the
> > > > > conclusion that in all cases, inlining cross-unit will make the calling
> > > > > code shorter, and allow the functions to be elided entirely.
> > > > >
> > > > > I take it from this that noone has tried livepatching an LTO build of Xen.
> > > >
> > > > Roger did build it (FreeBSD uses it), but I don't think he tried
> > > > the test-cases.
> > >
> > > I've tried livepatch, but not LTO, neither on it's own.
> >
> > Gotcha.
> > >
> > > > Wait, I think he may have as his patch: f8c66c2ad2efdb281e4ebf15bf329d73c4f02ce7
> > > > Author: Roger Pau Monne <roger.pau@citrix.com>
> > > > Date: Tue May 3 12:55:09 2016 +0200
> > > >
> > > > xen/xsplice: add ELFOSABI_FREEBSD as a supported OSABI for payloads
> > > >
> > > > implies that he did build an payload (and I remember us trying to figure
> > > > out if the issues he had been hitting were due to LTO or something else).
> > >
> > > IIRC issues where related to not translating error codes from the return of the
> > > livepatch hypercalls, and the patch upload tool refusing to upload patches with
> > > ELFOSABI_FREEBSD instead of ELFOSABI_NONE (which I assume is what Linux uses).
> > >
> > > It's been sometime since I've tried livepatch with FreeBSD, so I could give it
> > > a spin if you think there's a chance things might have broken.
> >
> > That is OK.
> >
> > I think I can also try myself to build Xen with LTO on Linux and see how
> > the test-cases fare.
> >
> > Not sure what is involved but I will find out.
> >
> > And to answer Andrew's question - No, nobody tried LTO with livepatching then :-(
>
> TBH, I think nobody has tried LTO on it's own in a long time, much less with
> livepatch. Adding Wei who recently sent some LTO makefile fixes, maybe he tried
> it.
>
LTO is now controlled by CONFIG_LTO in kconfig.
Xen build system is broken when LTO is enabled. Furthermore GCC
toolchain can't handle LTO yet.
Konrad, feel free to fix LTO build. :-)
Wei.
> Roger.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] xen/common: Drop function calls for Xen compile/version information
2017-01-18 16:30 ` Wei Liu
@ 2017-01-18 17:04 ` Andrew Cooper
2017-01-18 19:01 ` Doug Goldstein
0 siblings, 1 reply; 17+ messages in thread
From: Andrew Cooper @ 2017-01-18 17:04 UTC (permalink / raw)
To: Wei Liu, Roger Pau Monné
Cc: Julien Grall, Stefano Stabellini, Xen-devel, Jan Beulich,
Konrad Rzeszutek Wilk
On 18/01/17 16:30, Wei Liu wrote:
> On Wed, Jan 18, 2017 at 04:21:06PM +0000, Roger Pau Monné wrote:
>> On Wed, Jan 18, 2017 at 11:10:40AM -0500, Konrad Rzeszutek Wilk wrote:
>>> On Wed, Jan 18, 2017 at 10:13:34AM +0000, Roger Pau Monné wrote:
>>>> On Tue, Jan 17, 2017 at 03:46:30PM -0500, Konrad Rzeszutek Wilk wrote:
>>>>>>> Is this patch of yours that neccessary? Could at least some of the
>>>>>>> functions still exist?
>>>>>> Well. This patch is manually doing what LTO would do automatically when
>>>>>> it has a cross-translation-unit view of things, and come to the
>>>>>> conclusion that in all cases, inlining cross-unit will make the calling
>>>>>> code shorter, and allow the functions to be elided entirely.
>>>>>>
>>>>>> I take it from this that noone has tried livepatching an LTO build of Xen.
>>>>> Roger did build it (FreeBSD uses it), but I don't think he tried
>>>>> the test-cases.
>>>> I've tried livepatch, but not LTO, neither on it's own.
>>> Gotcha.
>>>>> Wait, I think he may have as his patch: f8c66c2ad2efdb281e4ebf15bf329d73c4f02ce7
>>>>> Author: Roger Pau Monne <roger.pau@citrix.com>
>>>>> Date: Tue May 3 12:55:09 2016 +0200
>>>>>
>>>>> xen/xsplice: add ELFOSABI_FREEBSD as a supported OSABI for payloads
>>>>>
>>>>> implies that he did build an payload (and I remember us trying to figure
>>>>> out if the issues he had been hitting were due to LTO or something else).
>>>> IIRC issues where related to not translating error codes from the return of the
>>>> livepatch hypercalls, and the patch upload tool refusing to upload patches with
>>>> ELFOSABI_FREEBSD instead of ELFOSABI_NONE (which I assume is what Linux uses).
>>>>
>>>> It's been sometime since I've tried livepatch with FreeBSD, so I could give it
>>>> a spin if you think there's a chance things might have broken.
>>> That is OK.
>>>
>>> I think I can also try myself to build Xen with LTO on Linux and see how
>>> the test-cases fare.
>>>
>>> Not sure what is involved but I will find out.
>>>
>>> And to answer Andrew's question - No, nobody tried LTO with livepatching then :-(
>> TBH, I think nobody has tried LTO on it's own in a long time, much less with
>> livepatch. Adding Wei who recently sent some LTO makefile fixes, maybe he tried
>> it.
>>
> LTO is now controlled by CONFIG_LTO in kconfig.
>
> Xen build system is broken when LTO is enabled. Furthermore GCC
> toolchain can't handle LTO yet.
>
> Konrad, feel free to fix LTO build. :-)
Last time I tried LTO:
* The GCC build did work, but the net binary was bigger rather than
smaller, and it successfully boot
* The Clang build worked, made a much smaller binary, but due to a clang
bug, "optimised" code into using SSE, and blew up spectacularly when
booting.
From experiments with XTF, Clang 3.9 LTO is far more comprehensive, and
is basically capable of optimising entire XTF tests into one single test
function, and properly respects the "no FPU ever" options used during boot.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] xen/common: Drop function calls for Xen compile/version information
2017-01-18 17:04 ` Andrew Cooper
@ 2017-01-18 19:01 ` Doug Goldstein
0 siblings, 0 replies; 17+ messages in thread
From: Doug Goldstein @ 2017-01-18 19:01 UTC (permalink / raw)
To: Andrew Cooper, Wei Liu, Roger Pau Monné
Cc: Julien Grall, Stefano Stabellini, Konrad Rzeszutek Wilk,
Jan Beulich, Xen-devel
[-- Attachment #1.1.1: Type: text/plain, Size: 759 bytes --]
On 1/18/17 12:04 PM, Andrew Cooper wrote:
> Last time I tried LTO:
>
> * The GCC build did work, but the net binary was bigger rather than
> smaller, and it successfully boot
> * The Clang build worked, made a much smaller binary, but due to a clang
> bug, "optimised" code into using SSE, and blew up spectacularly when
> booting.
>
> From experiments with XTF, Clang 3.9 LTO is far more comprehensive, and
> is basically capable of optimising entire XTF tests into one single test
> function, and properly respects the "no FPU ever" options used during boot.
>
> ~Andrew
FWIW, some of the clang issues I reported against LTO and FPU bits are
marked as fixed in 3.9 and 4.0. Not sure if things got backported.
--
Doug Goldstein
[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 959 bytes --]
[-- Attachment #2: Type: text/plain, Size: 127 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2017-01-18 19:01 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-16 13:04 [PATCH] xen/common: Drop function calls for Xen compile/version information Andrew Cooper
2017-01-16 13:18 ` Jan Beulich
2017-01-16 13:26 ` Andrew Cooper
2017-01-16 18:23 ` Stefano Stabellini
2017-01-16 19:44 ` Doug Goldstein
2017-01-17 18:05 ` Konrad Rzeszutek Wilk
2017-01-17 18:16 ` Andrew Cooper
2017-01-17 18:42 ` Konrad Rzeszutek Wilk
2017-01-17 19:00 ` Konrad Rzeszutek Wilk
2017-01-17 20:39 ` Andrew Cooper
2017-01-17 20:46 ` Konrad Rzeszutek Wilk
2017-01-18 10:13 ` Roger Pau Monné
2017-01-18 16:10 ` Konrad Rzeszutek Wilk
2017-01-18 16:21 ` Roger Pau Monné
2017-01-18 16:30 ` Wei Liu
2017-01-18 17:04 ` Andrew Cooper
2017-01-18 19:01 ` Doug Goldstein
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.