Re: [PATCH v1 3/3] xen/console: introduce conring_flush()

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Thu, 3 Apr 2025, dmkhn@xxxxxxxxx wrote:
> From: Denis Mukhin <dmukhin@xxxxxxxx>
> 
> Introduce conring_flush() to ensure all messages kept in the internal
> console ring are sent to all physical consoles (serial, VGA (x86))
> after their initialization is completed.
> 
> Resolves: https://gitlab.com/xen-project/xen/-/issues/184
> Signed-off-by: Denis Mukhin <dmukhin@xxxxxxxx>
> ---
> The patch conflicts with
>   https://lore.kernel.org/xen-devel/20250331230508.440198-7-dmukhin@xxxxxxxx/
> in console_init_preirq()
> ---
>  xen/drivers/char/console.c | 34 ++++++++++++++++++++++++----------
>  1 file changed, 24 insertions(+), 10 deletions(-)
> 
> diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
> index 2618c2e47d..18eb66df89 100644
> --- a/xen/drivers/char/console.c
> +++ b/xen/drivers/char/console.c
> @@ -446,24 +446,22 @@ void console_serial_puts(const char *s, size_t nr)
>          serial_puts(sercon_handle, s, nr);
>  }
>  
> -static void cf_check dump_console_ring_key(unsigned char key)
> +/*
> + * Flush contents of the conring to the physical console devices.
> + */
> +static int conring_flush(void)
>  {
>      uint32_t idx, len, sofar, c;
>      unsigned int order;
>      char *buf;
> +    unsigned long flags;
>  
> -    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;
>  
> +    flags = console_lock_recursive_irqsave();

This patch is OK but it looks like this bit might have to change to
rebase on top of a modified patch #2


>      c = conringc;
>      sofar = 0;
>      while ( (c != conringp) )
> @@ -478,8 +476,21 @@ static void cf_check dump_console_ring_key(unsigned char 
> key)
>      }
>  
>      console_puts(buf, sofar, CONSOLE_SERIAL | CONSOLE_VIDEO | CONSOLE_PV);
> +    console_unlock_recursive_irqrestore(flags);
>  
>      free_xenheap_pages(buf, order);
> +
> +    return 0;
> +}
> +
> +static void cf_check conring_dump_keyhandler(unsigned char key)
> +{
> +    int rc;
> +
> +    printk("'%c' pressed -> dumping console ring buffer (dmesg)\n", key);
> +    rc = conring_flush();
> +    if ( rc )
> +        printk("failed to dump console ring buffer: %d\n", rc);
>  }
>  
>  /*
> @@ -1044,6 +1055,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 */
> +    conring_flush();
> +
>      /* HELLO WORLD --- start-of-day banner text. */
>      nrspin_lock(&console_lock);
>      __putstr(xen_banner());
> @@ -1134,7 +1148,7 @@ void __init console_endboot(void)
>      if ( opt_conswitch[1] == 'x' )
>          console_rx = max_console_rx;
>  
> -    register_keyhandler('w', dump_console_ring_key,
> +    register_keyhandler('w', conring_dump_keyhandler,
>                          "synchronously dump console ring buffer (dmesg)", 0);
>      register_irq_keyhandler('+', &do_inc_thresh,
>                              "increase log level threshold", 0);
> -- 
> 2.34.1
> 
>