keyhandler: add handler to show Xen command line

[PATCH v1] keyhandler: add handler to show Xen command line

Posted by dmukhin@ford.com 3 weeks, 5 days ago

From: Denis Mukhin <dmukhin@ford.com> 

Currently there's no way to print Xen command line on the emergency console
for debugging purposes.

Add new keyhander 'C' to add (saved) command line printout for debugging
the system.

Also, fix indentation for KEYHANDLER() entries in 'keyhandler' array.

Signed-off-by: Denis Mukhin <dmukhin@ford.com>
---
CI: https://gitlab.com/xen-project/people/dmukhin/xen/-/pipelines/2717339023
---
 xen/common/kernel.c     |  5 +++++
 xen/common/keyhandler.c | 18 +++++++++++++-----
 xen/include/xen/lib.h   |  1 +
 3 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/xen/common/kernel.c b/xen/common/kernel.c
index fb45f8139995..9718a89d8088 100644
--- a/xen/common/kernel.c
+++ b/xen/common/kernel.c
@@ -232,6 +232,11 @@ void __init cmdline_parse(const char *cmdline)
 #endif
 }
 
+const char *cmdline_get(void)
+{
+    return saved_cmdline;
+}
+
 int parse_bool(const char *s, const char *e)
 {
     size_t len = e ? ({ ASSERT(e >= s); e - s; }) : strlen(s);
diff --git a/xen/common/keyhandler.c b/xen/common/keyhandler.c
index cb6df2823b00..55a370ac1790 100644
--- a/xen/common/keyhandler.c
+++ b/xen/common/keyhandler.c
@@ -28,7 +28,7 @@ static unsigned char keypress_key;
 static bool alt_key_handling;
 
 static keyhandler_fn_t cf_check show_handlers, cf_check dump_hwdom_registers,
-    cf_check dump_domains, cf_check read_clocks;
+    cf_check dump_domains, cf_check read_clocks, show_cmdline;
 static irq_keyhandler_fn_t cf_check do_toggle_alt_key, cf_check dump_registers,
     cf_check reboot_machine, cf_check run_all_keyhandlers;
 
@@ -60,15 +60,17 @@ static struct keyhandler {
     IRQ_KEYHANDLER('*', run_all_keyhandlers, "print all diagnostics", 0),
 
 #ifdef CONFIG_PERF_COUNTERS
-    KEYHANDLER('p', perfc_printall, "print performance counters", 1),
-    KEYHANDLER('P', perfc_reset, "reset performance counters", 0),
+        KEYHANDLER('p', perfc_printall, "print performance counters", 1),
+        KEYHANDLER('P', perfc_reset, "reset performance counters", 0),
 #endif
 
 #ifdef CONFIG_DEBUG_LOCK_PROFILE
-    KEYHANDLER('l', spinlock_profile_printall, "print lock profile info", 1),
-    KEYHANDLER('L', spinlock_profile_reset, "reset lock profile info", 0),
+        KEYHANDLER('l', spinlock_profile_printall, "print lock profile info", 1),
+        KEYHANDLER('L', spinlock_profile_reset, "reset lock profile info", 0),
 #endif
 
+        KEYHANDLER('C', show_cmdline, "show saved command line", 0),
+
 #undef IRQ_KEYHANDLER
 #undef KEYHANDLER
 };
@@ -512,6 +514,12 @@ static void cf_check do_toggle_alt_key(unsigned char key, bool unused)
            alt_key_handling ? "alternative" : "normal");
 }
 
+static void cf_check show_cmdline(unsigned char key)
+{
+    printk("'%c' pressed -> showing saved command line\n", key);
+    printk("%s\n", cmdline_get());
+}
+
 void __init initialize_keytable(void)
 {
     if ( num_present_cpus() > 16 )
diff --git a/xen/include/xen/lib.h b/xen/include/xen/lib.h
index 3c545ff33c61..094873dc8ba6 100644
--- a/xen/include/xen/lib.h
+++ b/xen/include/xen/lib.h
@@ -19,6 +19,7 @@
 
 struct domain;
 
+const char *cmdline_get(void);
 void cmdline_parse(const char *cmdline);
 int parse_bool(const char *s, const char *e);
 
-- 
2.54.0

Re: [PATCH v1] keyhandler: add handler to show Xen command line

Posted by Jan Beulich 3 weeks, 5 days ago

On 30.07.2026 08:15, dmukhin@ford.com wrote:
> From: Denis Mukhin <dmukhin@ford.com> 

First, you want to update Roger's email address that you use. This is now the
2nd patch that you sent to his old address, and the latest after sending the 1st
you should have noticed.

> Currently there's no way to print Xen command line on the emergency console
> for debugging purposes.
> 
> Add new keyhander 'C' to add (saved) command line printout for debugging
> the system.

If others think spending a precious character on this, I think I won't attempt
to veto this, but I'm definitely not going to ack such. More importantly though
you shouldn't be using a character that's already in use.

Nevertheless a few comments below.

> Also, fix indentation for KEYHANDLER() entries in 'keyhandler' array.

"fix" as in "break"? Patch context alone doesn't make clear at all why you'd
do this. Looking at the full source I think I can see why you think something
may be wrong there, but I don't think it is - the earlier block outside of the
#ifdef-s is different in this regard, imo.

> --- a/xen/common/kernel.c
> +++ b/xen/common/kernel.c
> @@ -232,6 +232,11 @@ void __init cmdline_parse(const char *cmdline)
>  #endif
>  }
>  
> +const char *cmdline_get(void)
> +{
> +    return saved_cmdline;
> +}

To accompany both this use and the pre-existing ones, perhaps it's time to
mark saved_cmdline[] __ro_after_init (in a separate patch preferably)?

> --- a/xen/common/keyhandler.c
> +++ b/xen/common/keyhandler.c
> @@ -28,7 +28,7 @@ static unsigned char keypress_key;
>  static bool alt_key_handling;
>  
>  static keyhandler_fn_t cf_check show_handlers, cf_check dump_hwdom_registers,
> -    cf_check dump_domains, cf_check read_clocks;
> +    cf_check dump_domains, cf_check read_clocks, show_cmdline;
>  static irq_keyhandler_fn_t cf_check do_toggle_alt_key, cf_check dump_registers,
>      cf_check reboot_machine, cf_check run_all_keyhandlers;
>  
> @@ -60,15 +60,17 @@ static struct keyhandler {
>      IRQ_KEYHANDLER('*', run_all_keyhandlers, "print all diagnostics", 0),
>  
>  #ifdef CONFIG_PERF_COUNTERS
> -    KEYHANDLER('p', perfc_printall, "print performance counters", 1),
> -    KEYHANDLER('P', perfc_reset, "reset performance counters", 0),
> +        KEYHANDLER('p', perfc_printall, "print performance counters", 1),
> +        KEYHANDLER('P', perfc_reset, "reset performance counters", 0),
>  #endif
>  
>  #ifdef CONFIG_DEBUG_LOCK_PROFILE
> -    KEYHANDLER('l', spinlock_profile_printall, "print lock profile info", 1),
> -    KEYHANDLER('L', spinlock_profile_reset, "reset lock profile info", 0),
> +        KEYHANDLER('l', spinlock_profile_printall, "print lock profile info", 1),
> +        KEYHANDLER('L', spinlock_profile_reset, "reset lock profile info", 0),
>  #endif
>  
> +        KEYHANDLER('C', show_cmdline, "show saved command line", 0),

Why not insert at the appropriate slot in the earlier block outside of any
#ifdef?

As to the help text - in how far is "saved" relevant there?

> @@ -512,6 +514,12 @@ static void cf_check do_toggle_alt_key(unsigned char key, bool unused)
>             alt_key_handling ? "alternative" : "normal");
>  }
>  
> +static void cf_check show_cmdline(unsigned char key)
> +{
> +    printk("'%c' pressed -> showing saved command line\n", key);
> +    printk("%s\n", cmdline_get());
> +}

If this lived in kernel.c, the new cmdline_get() wouldn't be needed at all.

Jan

Re: [PATCH v1] keyhandler: add handler to show Xen command line

Posted by Juergen Gross 3 weeks, 5 days ago

On 30.07.26 08:15, dmukhin@ford.com wrote:
> From: Denis Mukhin <dmukhin@ford.com>
> 
> Currently there's no way to print Xen command line on the emergency console
> for debugging purposes.
> 
> Add new keyhander 'C' to add (saved) command line printout for debugging
> the system.

'C' is already taken for "trigger a crashdump". Many keyhandlers are
registered dynamically via register_keyhandler().

What about 'X' for "General Xen information"?

This would allow for adding more information later, like e.g. physical
memory size, number of CPUs, number of active domains, etc., without the
need of burning another debug key for that.

Juergen