Add Xen version printout via keyhandler mechanism.

That is useful for debugging systems that have been left intact for a long
time.

Signed-off-by: Denis Mukhin <dmukhin@ford.com>
---
 xen/common/version.c       |  2 --
 xen/drivers/char/console.c | 31 ++++++++++++++++++++++++++-----
 2 files changed, 26 insertions(+), 7 deletions(-)

diff --git a/xen/common/version.c b/xen/common/version.c
index bc3714b45f..9e47ff0744 100644
--- a/xen/common/version.c
+++ b/xen/common/version.c
@@ -210,8 +210,6 @@ void __init xen_build_init(void)
         }
     }
 #endif /* CONFIG_X86 */
-    if ( !rc )
-        printk(XENLOG_INFO "build-id: %*phN\n", build_id_len, build_id_p);
 }
 #endif /* BUILD_ID */
 /*
diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index 73d24a7821..4f945f3bca 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -974,6 +974,28 @@ void guest_printk(const struct domain *d, const char *fmt, ...)
     va_end(args);
 }
 
+static void xen_print_version(void)
+{
+    const char *str = NULL;
+    unsigned int str_len;
+
+    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());
+
+    if ( !xen_build_id((const void **)&str, &str_len) )
+        printk(XENLOG_INFO "build-id: %*phN\n", str_len, str);
+}
+
+static void cf_check do_print_version(unsigned char key, bool unused)
+{
+    printk("'%c' pressed -> printing Xen version\n", key);
+    xen_print_version();
+}
+
 void __init console_init_preirq(void)
 {
     char *p;
@@ -1024,15 +1046,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());
 
     /* Locate and print the buildid, if applicable. */
     xen_build_init();
 
+    xen_print_version();
+
     if ( opt_sync_console )
     {
         serial_start_sync(sercon_handle);
@@ -1120,6 +1139,8 @@ void __init console_endboot(void)
                             "decrease log level threshold", 0);
     register_irq_keyhandler('G', &do_toggle_guest,
                             "toggle host/guest log level adjustment", 0);
+    register_irq_keyhandler('~', &do_print_version,
+                            "print Xen version", 0);
 
     /* Serial input is directed to DOM0 by default. */
     switch_serial_input();
-- 
2.34.1

On 12/02/2025 11:58 pm, dmkhn@proton.me wrote:
> Add Xen version printout via keyhandler mechanism.
>
> That is useful for debugging systems that have been left intact for a long
> time.
>
> Signed-off-by: Denis Mukhin <dmukhin@ford.com>

Hmm, it's a good point, but can't we add this to the top of the 'h'
(help) key rather than adding a(nother) handler?

Amongst other things, ~ is going to be problematic for any connection
serial running over SSH (usually IPMI), as it's the SSH escape
character, and use on Xen's console expects ~ with no following keypress.

On 13.02.2025 00:58, dmkhn@proton.me wrote:
> Add Xen version printout via keyhandler mechanism.
> 
> That is useful for debugging systems that have been left intact for a long
> time.
> 
> Signed-off-by: Denis Mukhin <dmukhin@ford.com>

First, +1 to what Andrew said.

> --- a/xen/drivers/char/console.c
> +++ b/xen/drivers/char/console.c
> @@ -974,6 +974,28 @@ void guest_printk(const struct domain *d, const char *fmt, ...)
>      va_end(args);
>  }
>  
> +static void xen_print_version(void)
> +{
> +    const char *str = NULL;
> +    unsigned int str_len;
> +
> +    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());
> +
> +    if ( !xen_build_id((const void **)&str, &str_len) )

Why the cast? What wrong with "str" being const void *? The only thing
is that it's then not really a good name for the variable, but that it
isn't anyway. It's not really a string you get back.

> @@ -1024,15 +1046,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());
>  
>      /* Locate and print the buildid, if applicable. */
>      xen_build_init();
>  
> +    xen_print_version();

It may seem minor, but I'm not happy about the reordering. What's logged
would better really be directly after the banner. Any present or future
output from xen_build_init() should come afterwards. Simply add
xen_print_buildid() along with xen_print_version()? And then have it in
version.c, where you can use build_id_{p,len} directly, eliminating the
point above regarding casts and naming.

Jan

On Wednesday, February 12th, 2025 at 11:25 PM, Jan Beulich <jbeulich@suse.com> wrote:

>
>
> On 13.02.2025 00:58, dmkhn@proton.me wrote:
>
> > Add Xen version printout via keyhandler mechanism.
> >
> > That is useful for debugging systems that have been left intact for a long
> > time.
> >
> > Signed-off-by: Denis Mukhin dmukhin@ford.com
>
>
> First, +1 to what Andrew said.
>
> > --- a/xen/drivers/char/console.c
> > +++ b/xen/drivers/char/console.c
> > @@ -974,6 +974,28 @@ void guest_printk(const struct domain *d, const char *fmt, ...)
> > va_end(args);
> > }
> >
> > +static void xen_print_version(void)
> > +{
> > + const char *str = NULL;
> > + unsigned int str_len;
> > +
> > + 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());
> > +
> > + if ( !xen_build_id((const void **)&str, &str_len) )
>
>
> Why the cast? What wrong with "str" being const void *? The only thing
> is that it's then not really a good name for the variable, but that it
> isn't anyway. It's not really a string you get back.

Sorry, that's because I used xenver_varbuf_op() as a reference.
Fixed in v2.

>
> > @@ -1024,15 +1046,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());
> >
> > /* Locate and print the buildid, if applicable. */
> > xen_build_init();
> >
> > + xen_print_version();
>
>
> It may seem minor, but I'm not happy about the reordering. What's logged
> would better really be directly after the banner. Any present or future
> output from xen_build_init() should come afterwards. Simply add
> xen_print_buildid() along with xen_print_version()? And then have it in
> version.c, where you can use build_id_{p,len} directly, eliminating the
> point above regarding casts and naming.

Thanks!
Addressed in v2.

>
> Jan