[v3,17/24] xen/console: flush console ring to physical console

Commit Message

Denis Mukhin via B4 Relay Jan. 4, 2025, 1:58 a.m. UTC

From: Denis Mukhin <[e-mail]>

Internal console ring is used to hold Xen messages before external consoles
(serial, VGA) consoles are fully initialized.

Ensure all messages are sent to all external consoles after their
initialization is completed.

Link: https://gitlab.com/xen-project/xen/-/issues/184
Signed-off-by: Denis Mukhin <[e-mail]>
---
 xen/drivers/char/console.c | 56 +++++++++++++++++++++++++++++++++-------------
 1 file changed, 41 insertions(+), 15 deletions(-)

Comments

Jan Beulich Jan. 28, 2025, 4:56 p.m. UTC | #1

On 04.01.2025 02:58, Denis Mukhin via B4 Relay wrote:
> --- a/xen/drivers/char/console.c
> +++ b/xen/drivers/char/console.c
> @@ -430,23 +430,36 @@ void console_serial_puts(const char *s, size_t nr)
>      pv_console_puts(s, nr);
>  }
>  
> -static void cf_check dump_console_ring_key(unsigned char key)
> +/*
> + * Write characters to physical console(s).
> + * That covers:
> + * - serial console;
> + * - video output.
> + */
> +static void console_puts(const char *str, size_t len)
> +{
> +    ASSERT(rspin_is_locked(&console_lock));
> +
> +    console_serial_puts(str, len);
> +    video_puts(str, len);
> +}
> +
> +/*
> + * Flush contents of the conring to the physical console devices.
> + */
> +static int console_flush(void)
>  {
>      uint32_t idx, len, sofar, c;
>      unsigned int order;
>      char *buf;
>  
> -    printk("'%c' pressed -> dumping console ring buffer (dmesg)\n", key);
> -
> -    /* create a buffer in which we'll copy the ring in the correct
> -       order and NUL terminate */
>      order = get_order_from_bytes(conring_size + 1);
>      buf = alloc_xenheap_pages(order, 0);
>      if ( buf == NULL )
> -    {
> -        printk("unable to allocate memory!\n");
> -        return;
> -    }
> +        return -ENOMEM;
> +
> +
> +    nrspin_lock(&console_lock);

Nit: No double blank lines please.

> @@ -681,10 +707,7 @@ static void xen_console_write(const char *str, size_t len)
>   */
>  static void console_write(const char *str, size_t len, unsigned int flags)
>  {
> -    ASSERT(rspin_is_locked(&console_lock));

Why is this being dropped? Hmm, I see, it moves into console_puts().

> @@ -1061,6 +1084,9 @@ void __init console_init_preirq(void)
>      serial_set_rx_handler(sercon_handle, serial_rx);
>      pv_console_set_rx_handler(serial_rx);
>  
> +    /* NB: send conring contents to all enabled physical consoles, if any */
> +    console_flush();

Aren't you potentially emitting messages a 2nd time this way? Imo
such flushing needs to come immediately after any particular console
is ready.

Jan

Patch

diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index 2f776b2f07cb15fae8060f3574db73234a38727a..96375067164980a138b1a93161712c0758f4f7fe 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -430,23 +430,36 @@  void console_serial_puts(const char *s, size_t nr)
     pv_console_puts(s, nr);
 }
 
-static void cf_check dump_console_ring_key(unsigned char key)
+/*
+ * Write characters to physical console(s).
+ * That covers:
+ * - serial console;
+ * - video output.
+ */
+static void console_puts(const char *str, size_t len)
+{
+    ASSERT(rspin_is_locked(&console_lock));
+
+    console_serial_puts(str, len);
+    video_puts(str, len);
+}
+
+/*
+ * Flush contents of the conring to the physical console devices.
+ */
+static int console_flush(void)
 {
     uint32_t idx, len, sofar, c;
     unsigned int order;
     char *buf;
 
-    printk("'%c' pressed -> dumping console ring buffer (dmesg)\n", key);
-
-    /* create a buffer in which we'll copy the ring in the correct
-       order and NUL terminate */
     order = get_order_from_bytes(conring_size + 1);
     buf = alloc_xenheap_pages(order, 0);
     if ( buf == NULL )
-    {
-        printk("unable to allocate memory!\n");
-        return;
-    }
+        return -ENOMEM;
+
+
+    nrspin_lock(&console_lock);
 
     c = conringc;
     sofar = 0;
@@ -461,10 +474,23 @@  static void cf_check dump_console_ring_key(unsigned char key)
         c += len;
     }
 
-    console_serial_puts(buf, sofar);
-    video_puts(buf, sofar);
+    console_puts(buf, sofar);
+
+    nrspin_unlock(&console_lock);
 
     free_xenheap_pages(buf, order);
+
+    return 0;
+}
+
+static void cf_check dump_console_ring_key(unsigned char key)
+{
+    int rc;
+
+    printk("'%c' pressed -> dumping console ring buffer (dmesg)\n", key);
+    rc = console_flush();
+    if ( rc )
+        printk("failed to dump console ring buffer: %d\n", rc);
 }
 
 /*
@@ -681,10 +707,7 @@  static void xen_console_write(const char *str, size_t len)
  */
 static void console_write(const char *str, size_t len, unsigned int flags)
 {
-    ASSERT(rspin_is_locked(&console_lock));
-
-    console_serial_puts(str, len);
-    video_puts(str, len);
+    console_puts(str, len);
 
 #ifdef CONFIG_X86
     if ( opt_console_xen )
@@ -1061,6 +1084,9 @@  void __init console_init_preirq(void)
     serial_set_rx_handler(sercon_handle, serial_rx);
     pv_console_set_rx_handler(serial_rx);
 
+    /* NB: send conring contents to all enabled physical consoles, if any */
+    console_flush();
+
     /* HELLO WORLD --- start-of-day banner text. */
     nrspin_lock(&console_lock);
     __putstr(xen_banner());