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

[Jan Beulich <jbeulich@xxxxxxxx>]

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