Re: [PATCH v4] xen/console: make console buffer size configurable

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 11/03/2025 7:09 am, dmkhn@xxxxxxxxx wrote:
> diff --git a/docs/misc/xen-command-line.pandoc 
> b/docs/misc/xen-command-line.pandoc
> index 89db6e83be..a471a9f7ce 100644
> --- a/docs/misc/xen-command-line.pandoc
> +++ b/docs/misc/xen-command-line.pandoc
> @@ -425,10 +425,11 @@ The following are examples of correct specifications:
>  ### conring_size
>  > `= <size>`
>  
> -> Default: `conring_size=16k`
> -
>  Specify the size of the console ring buffer.
>  
> +The default console ring buffer size is selected at build time via
> +CONFIG_CONRING_SHIFT setting.

I am firmly in support of this option.  I've been carrying:

-#define _CONRING_SIZE 16384
+#define _CONRING_SIZE KB(64)

in the XenServer patchqueue for more than a decade now, seeing as the
default simply isn't big enough.


However, there's a subtlety which probably needs expanding on, now it's
being discussed in documentation.

The new CONFIG_CONRING_SHIFT controls the size of of the buffer in
.init.data.  We don't have .init.bss, so this affects the compiled size
of Xen.

The command line controls the size of the dynamic allocation.  This is
effectively a realloc() of the .init buffer, and happens unconditionally
whether the numbers are the same or not.

opt_conring_size is guestimated in console_init_postirq() if the user
hasn't chosen a value.  When allocating the runtime buffer, the larger
of conring_size and opt_conring_size is taken, and then the buffer is
progressively rounded by order until a successful allocation can be made.

i.e. there's no sane relationship between the options given, and the
eventual size of the buffer.

In order to not drop boot messages, the .init.data needs to be large
enough to contain the messages until console_init_ring() is run.  The
situation could be improved by moving this as early as possible.


Anyway, we obviously don't want to go into that detail, but it's also a
little more complicated than currently given.


Not for this patch, but if anyone is feeling at a loose end, `conring`
and `conring_size` should become __ro_after_init, and
console_init_ring() can become much more efficient by using 1 or 2
memcpy()'s, rather than copying the ring a byte at a time.

Also, "opt_conring_size = PAGE_SIZE << order" is UB when the user
selects 2G size, as PAGE_SIZE is signed, and will overflow to 0 if the
user selects 4G-or-more, and then all the masking logic breaks.

Given that the size is rounded down without the users consent anyway,
it's probably better to to just clamp a maximum.

Finally, the buffer doesn't need to be aligned on it's size; it just
needs to be contiguous (and even then, only for kexec).  Combined with
the rounding-down, this might result in the buffer being unnecessarily
smaller than requested.

IIRC, ARM has another case which wants contiguous but not page aligned,
and it would be nice to make this an available allocation option.

~Andrew