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

dmkhn@proton.me posted 1 patch 9 months, 1 week ago
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/20250311070912.730334-1-dmkhn@proton.me
docs/misc/xen-command-line.pandoc |  5 +++--
xen/drivers/char/Kconfig          | 24 ++++++++++++++++++++++++
xen/drivers/char/console.c        |  6 +++---
3 files changed, 30 insertions(+), 5 deletions(-)
[PATCH v4] xen/console: make console buffer size configurable
Posted by dmkhn@proton.me 9 months, 1 week ago
From: Denis Mukhin <dmukhin@ford.com>

Add new CONRING_SHIFT Kconfig parameter to specify the boot console buffer size
as a power of 2.

The supported range is [14..27] -> [16KiB..128MiB].

Set default to 15 (32 KiB).

Resolves: https://gitlab.com/xen-project/xen/-/issues/185
Signed-off-by: Denis Mukhin <dmukhin@ford.com>
---
Changes v3->v4:
- s/Link:/Resolves:/ in commit message to auto-close the gitlab ticket
---
 docs/misc/xen-command-line.pandoc |  5 +++--
 xen/drivers/char/Kconfig          | 24 ++++++++++++++++++++++++
 xen/drivers/char/console.c        |  6 +++---
 3 files changed, 30 insertions(+), 5 deletions(-)

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.
+
 ### console
 > `= List of [ vga | com1[H,L] | com2[H,L] | pv | dbgp | ehci | xhci | none ]`
 
diff --git a/xen/drivers/char/Kconfig b/xen/drivers/char/Kconfig
index e6e12bb413..e238d4eccf 100644
--- a/xen/drivers/char/Kconfig
+++ b/xen/drivers/char/Kconfig
@@ -96,6 +96,30 @@ config SERIAL_TX_BUFSIZE
 
 	  Default value is 32768 (32KiB).
 
+config CONRING_SHIFT
+	int "Console ring buffer size (power of 2)"
+	range 14 27
+	default 15
+	help
+	  Select the boot console ring buffer size as a power of 2.
+	  Run-time console ring buffer size is the same as the boot console ring
+	  buffer size, unless overridden via 'conring_size=' boot parameter.
+
+	    27 => 128 MiB
+	    26 =>  64 MiB
+	    25 =>  32 MiB
+	    24 =>  16 MiB
+	    23 =>   8 MiB
+	    22 =>   4 MiB
+	    21 =>   2 MiB
+	    20 =>   1 MiB
+	    19 => 512 KiB
+	    18 => 256 KiB
+	    17 => 128 KiB
+	    16 =>  64 KiB
+	    15 =>  32 KiB (default)
+	    14 =>  16 KiB
+
 config XHCI
 	bool "XHCI DbC UART driver"
 	depends on X86
diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index ba428199d2..78794b74e9 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -101,12 +101,12 @@ static int cf_check parse_console_timestamps(const char *s);
 custom_runtime_param("console_timestamps", parse_console_timestamps,
                      con_timestamp_mode_upd);
 
-/* conring_size: allows a large console ring than default (16kB). */
+/* conring_size: override build-time CONFIG_CONRING_SHIFT setting. */
 static uint32_t __initdata opt_conring_size;
 size_param("conring_size", opt_conring_size);
 
-#define _CONRING_SIZE 16384
-#define CONRING_IDX_MASK(i) ((i)&(conring_size-1))
+#define _CONRING_SIZE       (1U << CONFIG_CONRING_SHIFT)
+#define CONRING_IDX_MASK(i) ((i) & (conring_size - 1))
 static char __initdata _conring[_CONRING_SIZE];
 static char *__read_mostly conring = _conring;
 static uint32_t __read_mostly conring_size = _CONRING_SIZE;
-- 
2.34.1
Re: [PATCH v4] xen/console: make console buffer size configurable
Posted by Andrew Cooper 9 months ago
On 11/03/2025 7:09 am, dmkhn@proton.me 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

Re: [PATCH v4] xen/console: make console buffer size configurable
Posted by Jason Andryuk 9 months, 1 week ago
On 2025-03-11 03:09, dmkhn@proton.me wrote:
> From: Denis Mukhin <dmukhin@ford.com>
> 
> Add new CONRING_SHIFT Kconfig parameter to specify the boot console buffer size
> as a power of 2.
> 
> The supported range is [14..27] -> [16KiB..128MiB].
> 
> Set default to 15 (32 KiB).
> 
> Resolves: https://gitlab.com/xen-project/xen/-/issues/185
> Signed-off-by: Denis Mukhin <dmukhin@ford.com>
> ---

Reviewed-by: Jason Andryuk <jason.andryuk@amd.com>