| Message ID | 20250213214054.1745501-1-dmukhin@ford.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v3] xen/console: print Xen version via keyhandler | expand |
On 13.02.2025 22:41, dmkhn@proton.me wrote: > Add Xen version printout to 'h' keyhandler output. > > That is useful for debugging systems that have been left intact for a long > time. > > Signed-off-by: Denis Mukhin <dmukhin@ford.com> > --- > Changes since v2: > - moved new function declarations to xen/lib.h > - dropped xen_ prefix in new functions > --- > xen/common/keyhandler.c | 4 ++++ > xen/common/version.c | 20 ++++++++++++++++++-- > xen/drivers/char/console.c | 8 +++----- > xen/include/xen/lib.h | 3 +++ > 4 files changed, 28 insertions(+), 7 deletions(-) > > diff --git a/xen/common/keyhandler.c b/xen/common/keyhandler.c > index 6ea54838d4..0bb842ec00 100644 > --- a/xen/common/keyhandler.c > +++ b/xen/common/keyhandler.c > @@ -129,6 +129,10 @@ static void cf_check show_handlers(unsigned char key) > unsigned int i; > > printk("'%c' pressed -> showing installed handlers\n", key); > + > + print_version(); > + print_build_id(); > + > for ( i = 0; i < ARRAY_SIZE(key_table); i++ ) > if ( key_table[i].fn ) > printk(" key '%c' (ascii '%02x') => %s\n", > diff --git a/xen/common/version.c b/xen/common/version.c > index bc3714b45f..8672d5544e 100644 > --- a/xen/common/version.c > +++ b/xen/common/version.c > @@ -210,9 +210,25 @@ void __init xen_build_init(void) > } > } > #endif /* CONFIG_X86 */ > - if ( !rc ) > - printk(XENLOG_INFO "build-id: %*phN\n", build_id_len, build_id_p); > } > + > +void print_version(void) > +{ > + printk("Xen version %d.%d%s (%s@%s) (%s) %s %s\n", > + xen_major_version(), xen_minor_version(), xen_extra_version(), > + xen_compile_by(), xen_compile_domain(), xen_compiler(), > + xen_build_info(), xen_compile_date()); > + > + printk("Latest ChangeSet: %s\n", xen_changeset()); > +} > + > +void print_build_id(void) > +{ > + BUG_ON(!build_id_p); > + BUG_ON(!build_id_len); Technically one of the two would likely suffice, if we need such checks at all. Question is why you added them. After all ... > + printk(XENLOG_INFO "build-id: %*phN\n", build_id_len, build_id_p); ... this isn't supposed to malfunction when passed a NULL pointer (with zero length). If it can malfunction, it wants fixing there imo. And that extends to the case of non-zero length as well: Any extensions to %p that we add would better retain the basic property of %p printing when NULL is passed as argument. (I'm sorry for not paying enough attention to this on v2 already.) (Later) Oh, actually these BUG_ON() are both wrong. They would trigger when building with a build-id-incapable linker. See the detection logic in the top level Makefile (search for XEN_HAS_BUILD_ID). To retain original behavior you will want to make the printk() conditional upon e.g. build_id_len being non-zero. Jan
On Thursday, February 13th, 2025 at 11:54 PM, Jan Beulich <jbeulich@suse.com> wrote: > > > On 13.02.2025 22:41, dmkhn@proton.me wrote: > > > Add Xen version printout to 'h' keyhandler output. > > > > That is useful for debugging systems that have been left intact for a long > > time. > > > > Signed-off-by: Denis Mukhin dmukhin@ford.com > > --- > > Changes since v2: > > - moved new function declarations to xen/lib.h > > - dropped xen_ prefix in new functions > > --- > > xen/common/keyhandler.c | 4 ++++ > > xen/common/version.c | 20 ++++++++++++++++++-- > > xen/drivers/char/console.c | 8 +++----- > > xen/include/xen/lib.h | 3 +++ > > 4 files changed, 28 insertions(+), 7 deletions(-) > > > > diff --git a/xen/common/keyhandler.c b/xen/common/keyhandler.c > > index 6ea54838d4..0bb842ec00 100644 > > --- a/xen/common/keyhandler.c > > +++ b/xen/common/keyhandler.c > > @@ -129,6 +129,10 @@ static void cf_check show_handlers(unsigned char key) > > unsigned int i; > > > > printk("'%c' pressed -> showing installed handlers\n", key); > > + > > + print_version(); > > + print_build_id(); > > + > > for ( i = 0; i < ARRAY_SIZE(key_table); i++ ) > > if ( key_table[i].fn ) > > printk(" key '%c' (ascii '%02x') => %s\n", > > diff --git a/xen/common/version.c b/xen/common/version.c > > index bc3714b45f..8672d5544e 100644 > > --- a/xen/common/version.c > > +++ b/xen/common/version.c > > @@ -210,9 +210,25 @@ void __init xen_build_init(void) > > } > > } > > #endif /* CONFIG_X86 */ > > - if ( !rc ) > > - printk(XENLOG_INFO "build-id: %*phN\n", build_id_len, build_id_p); > > } > > + > > +void print_version(void) > > +{ > > + printk("Xen version %d.%d%s (%s@%s) (%s) %s %s\n", > > + xen_major_version(), xen_minor_version(), xen_extra_version(), > > + xen_compile_by(), xen_compile_domain(), xen_compiler(), > > + xen_build_info(), xen_compile_date()); > > + > > + printk("Latest ChangeSet: %s\n", xen_changeset()); > > +} > > + > > +void print_build_id(void) > > +{ > > + BUG_ON(!build_id_p); > > + BUG_ON(!build_id_len); > > > Technically one of the two would likely suffice, if we need such checks > at all. Question is why you added them. After all ... I added assertions so that any misuse can bee easily seen. But I did not know about XEN_HAS_BUILD_ID switch. > > > + printk(XENLOG_INFO "build-id: %*phN\n", build_id_len, build_id_p); > > > ... this isn't supposed to malfunction when passed a NULL pointer (with > zero length). If it can malfunction, it wants fixing there imo. And that > extends to the case of non-zero length as well: Any extensions to %p > that we add would better retain the basic property of %p printing when > NULL is passed as argument. (I'm sorry for not paying enough attention > to this on v2 already.) No problem! Thanks for the feedback, much appreciated. > > (Later) Oh, actually these BUG_ON() are both wrong. They would trigger > when building with a build-id-incapable linker. See the detection logic > in the top level Makefile (search for XEN_HAS_BUILD_ID). To retain > original behavior you will want to make the printk() conditional upon > e.g. build_id_len being non-zero. Oh, I see. %p in printk() behaves correctly when passing NULL pointer - it prints nothing. > > Jan
diff --git a/xen/common/keyhandler.c b/xen/common/keyhandler.c index 6ea54838d4..0bb842ec00 100644 --- a/xen/common/keyhandler.c +++ b/xen/common/keyhandler.c @@ -129,6 +129,10 @@ static void cf_check show_handlers(unsigned char key) unsigned int i; printk("'%c' pressed -> showing installed handlers\n", key); + + print_version(); + print_build_id(); + for ( i = 0; i < ARRAY_SIZE(key_table); i++ ) if ( key_table[i].fn ) printk(" key '%c' (ascii '%02x') => %s\n", diff --git a/xen/common/version.c b/xen/common/version.c index bc3714b45f..8672d5544e 100644 --- a/xen/common/version.c +++ b/xen/common/version.c @@ -210,9 +210,25 @@ void __init xen_build_init(void) } } #endif /* CONFIG_X86 */ - if ( !rc ) - printk(XENLOG_INFO "build-id: %*phN\n", build_id_len, build_id_p); } + +void print_version(void) +{ + printk("Xen version %d.%d%s (%s@%s) (%s) %s %s\n", + xen_major_version(), xen_minor_version(), xen_extra_version(), + xen_compile_by(), xen_compile_domain(), xen_compiler(), + xen_build_info(), xen_compile_date()); + + printk("Latest ChangeSet: %s\n", xen_changeset()); +} + +void print_build_id(void) +{ + BUG_ON(!build_id_p); + BUG_ON(!build_id_len); + printk(XENLOG_INFO "build-id: %*phN\n", build_id_len, build_id_p); +} + #endif /* BUILD_ID */ /* * Local variables: diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c index 73d24a7821..235d55bbff 100644 --- a/xen/drivers/char/console.c +++ b/xen/drivers/char/console.c @@ -1024,14 +1024,12 @@ void __init console_init_preirq(void) nrspin_lock(&console_lock); __putstr(xen_banner()); nrspin_unlock(&console_lock); - printk("Xen version %d.%d%s (%s@%s) (%s) %s %s\n", - xen_major_version(), xen_minor_version(), xen_extra_version(), - xen_compile_by(), xen_compile_domain(), xen_compiler(), - xen_build_info(), xen_compile_date()); - printk("Latest ChangeSet: %s\n", xen_changeset()); + + print_version(); /* Locate and print the buildid, if applicable. */ xen_build_init(); + print_build_id(); if ( opt_sync_console ) { diff --git a/xen/include/xen/lib.h b/xen/include/xen/lib.h index 81b722ea3e..686899a63e 100644 --- a/xen/include/xen/lib.h +++ b/xen/include/xen/lib.h @@ -47,6 +47,9 @@ int parse_signed_integer(const char *name, const char *s, const char *e, */ int cmdline_strcmp(const char *frag, const char *name); +void print_version(void); +void print_build_id(void); + #ifdef CONFIG_DEBUG_TRACE extern void debugtrace_dump(void); extern void debugtrace_printk(const char *fmt, ...)
Add Xen version printout to 'h' keyhandler output. That is useful for debugging systems that have been left intact for a long time. Signed-off-by: Denis Mukhin <dmukhin@ford.com> --- Changes since v2: - moved new function declarations to xen/lib.h - dropped xen_ prefix in new functions --- xen/common/keyhandler.c | 4 ++++ xen/common/version.c | 20 ++++++++++++++++++-- xen/drivers/char/console.c | 8 +++----- xen/include/xen/lib.h | 3 +++ 4 files changed, 28 insertions(+), 7 deletions(-)