[PATCH v5 0/6] xen/console: configurable conring size

[PATCH v5 0/6] xen/console: configurable conring size

[dmukhin]

This series originates from [1] which addresses feedback [2].
Sending this as v5 since [1] was posted as v4...

Briefly, here is what the series covers:

Patch 1 groups conring code together to better maintainability.
There were conring bits all over the place in console.c.

Patch 2 introduces CONRING_CONRING_SHIFT to select compile-time
conring buffer size.

Patch 3 updates conring{,_size} annotations to __ro_after_init as per [2].

Patch 4 optimizes switch from early conring to permanent conring.

Patches 5-6 update the conring buffer allocation code and add some
verbose logging around buffer size selection.

[1] Link to v4: https://lore.kernel.org/xen-devel/20250311070912.730334-1-dmkhn@proton.me/
[2] https://lore.kernel.org/xen-devel/1a5ed8ad-0cc7-4e05-9b9c-cd6930d9b9ea@citrix.com
[3] Link to CI: https://gitlab.com/xen-project/people/dmukhin/xen/-/pipelines/2306728453

Denis Mukhin (6):
  xen/console: group conring code together
  xen/console: make console buffer size configurable
  xen/console: promote conring{,_size} to __ro_after_init
  xen/console: use memcpy() in console_init_ring()
  xen/console: update conring memory allocation
  xen/console: add conring buffer size alignment setting

 docs/misc/xen-command-line.pandoc |   5 +-
 xen/drivers/char/Kconfig          |  31 +++++
 xen/drivers/char/console.c        | 190 +++++++++++++++++-------------
 3 files changed, 143 insertions(+), 83 deletions(-)

-- 
2.52.0

[PATCH v5 1/6] xen/console: group conring code together

[dmukhin]

From: Denis Mukhin <dmukhin@ford.com> 

Groups conring buffer management code in the console driver for ease of
maintaining this code.

Not a functional change.

Signed-off-by: Denis Mukhin <dmukhin@ford.com>
---
Changes since v4:
- new patch
---
 xen/drivers/char/console.c | 161 ++++++++++++++++++-------------------
 1 file changed, 80 insertions(+), 81 deletions(-)

diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index 2bdb4d5fb417..86319600e0af 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -126,17 +126,6 @@ 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). */
-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))
-static char __initdata _conring[_CONRING_SIZE];
-static char *__read_mostly conring = _conring;
-static uint32_t __read_mostly conring_size = _CONRING_SIZE;
-static uint32_t conringc, conringp;
-
 static int __read_mostly sercon_handle = -1;
 
 #ifdef CONFIG_X86
@@ -350,6 +339,17 @@ static void cf_check do_dec_thresh(unsigned char key, bool unused)
  * ********************************************************
  */
 
+/* conring_size: allows a large console ring than default (16kB). */
+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))
+static char __initdata _conring[_CONRING_SIZE];
+static char *__read_mostly conring = _conring;
+static uint32_t __read_mostly conring_size = _CONRING_SIZE;
+static uint32_t conringc, conringp;
+
 static void cf_check conring_notify(void *unused)
 {
     send_global_virq(VIRQ_CON_RING);
@@ -416,47 +416,6 @@ long read_console_ring(struct xen_sysctl_readconsole *op)
 }
 #endif /* CONFIG_SYSCTL */
 
-
-/*
- * *******************************************************
- * *************** ACCESS TO SERIAL LINE *****************
- * *******************************************************
- */
-
-/* Characters received over the serial line are buffered for domain 0. */
-#define SERIAL_RX_SIZE 128
-#define SERIAL_RX_MASK(_i) ((_i)&(SERIAL_RX_SIZE-1))
-static char serial_rx_ring[SERIAL_RX_SIZE];
-static unsigned int serial_rx_cons, serial_rx_prod;
-
-static void (*serial_steal_fn)(const char *str, size_t nr) = early_puts;
-
-int console_steal(int handle, void (*fn)(const char *str, size_t nr))
-{
-    if ( (handle == -1) || (handle != sercon_handle) )
-        return 0;
-
-    if ( serial_steal_fn != NULL )
-        return -EBUSY;
-
-    serial_steal_fn = fn;
-    return 1;
-}
-
-void console_giveback(int id)
-{
-    if ( id == 1 )
-        serial_steal_fn = NULL;
-}
-
-void console_serial_puts(const char *s, size_t nr)
-{
-    if ( serial_steal_fn != NULL )
-        serial_steal_fn(s, nr);
-    else
-        serial_puts(sercon_handle, s, nr);
-}
-
 /*
  * Flush contents of the conring to the selected console devices.
  */
@@ -501,6 +460,75 @@ static void cf_check conring_dump_keyhandler(unsigned char key)
         printk("failed to dump console ring buffer: %d\n", rc);
 }
 
+void __init console_init_ring(void)
+{
+    char *ring;
+    unsigned int i, order, memflags;
+    unsigned long flags;
+
+    if ( !opt_conring_size )
+        return;
+
+    order = get_order_from_bytes(max(opt_conring_size, conring_size));
+    memflags = MEMF_bits(crashinfo_maxaddr_bits);
+    while ( (ring = alloc_xenheap_pages(order, memflags)) == NULL )
+    {
+        BUG_ON(order == 0);
+        order--;
+    }
+    opt_conring_size = PAGE_SIZE << order;
+
+    nrspin_lock_irqsave(&console_lock, flags);
+    for ( i = conringc ; i != conringp; i++ )
+        ring[i & (opt_conring_size - 1)] = conring[i & (conring_size - 1)];
+    conring = ring;
+    smp_wmb(); /* Allow users of console_force_unlock() to see larger buffer. */
+    conring_size = opt_conring_size;
+    nrspin_unlock_irqrestore(&console_lock, flags);
+
+    printk("Allocated console ring of %u KiB.\n", opt_conring_size >> 10);
+}
+
+/*
+ * *******************************************************
+ * *************** ACCESS TO SERIAL LINE *****************
+ * *******************************************************
+ */
+
+/* Characters received over the serial line are buffered for domain 0. */
+#define SERIAL_RX_SIZE 128
+#define SERIAL_RX_MASK(_i) ((_i)&(SERIAL_RX_SIZE-1))
+static char serial_rx_ring[SERIAL_RX_SIZE];
+static unsigned int serial_rx_cons, serial_rx_prod;
+
+static void (*serial_steal_fn)(const char *str, size_t nr) = early_puts;
+
+int console_steal(int handle, void (*fn)(const char *str, size_t nr))
+{
+    if ( (handle == -1) || (handle != sercon_handle) )
+        return 0;
+
+    if ( serial_steal_fn != NULL )
+        return -EBUSY;
+
+    serial_steal_fn = fn;
+    return 1;
+}
+
+void console_giveback(int id)
+{
+    if ( id == 1 )
+        serial_steal_fn = NULL;
+}
+
+void console_serial_puts(const char *s, size_t nr)
+{
+    if ( serial_steal_fn != NULL )
+        serial_steal_fn(s, nr);
+    else
+        serial_puts(sercon_handle, s, nr);
+}
+
 /*
  * CTRL-<switch_char> changes input direction, rotating among Xen, Dom0,
  * and the DomUs started from Xen at boot.
@@ -1125,35 +1153,6 @@ void __init console_init_preirq(void)
     }
 }
 
-void __init console_init_ring(void)
-{
-    char *ring;
-    unsigned int i, order, memflags;
-    unsigned long flags;
-
-    if ( !opt_conring_size )
-        return;
-
-    order = get_order_from_bytes(max(opt_conring_size, conring_size));
-    memflags = MEMF_bits(crashinfo_maxaddr_bits);
-    while ( (ring = alloc_xenheap_pages(order, memflags)) == NULL )
-    {
-        BUG_ON(order == 0);
-        order--;
-    }
-    opt_conring_size = PAGE_SIZE << order;
-
-    nrspin_lock_irqsave(&console_lock, flags);
-    for ( i = conringc ; i != conringp; i++ )
-        ring[i & (opt_conring_size - 1)] = conring[i & (conring_size - 1)];
-    conring = ring;
-    smp_wmb(); /* Allow users of console_force_unlock() to see larger buffer. */
-    conring_size = opt_conring_size;
-    nrspin_unlock_irqrestore(&console_lock, flags);
-
-    printk("Allocated console ring of %u KiB.\n", opt_conring_size >> 10);
-}
-
 void __init console_init_irq(void)
 {
     serial_init_irq();
-- 
2.52.0

[PATCH v5 2/6] xen/console: make console buffer size configurable

[dmukhin]

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>
---
Changes since v4:
- n/a
---
 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 c1f2def9f99c..87392142e8e9 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 8e49a52c735b..d083ba4c9cdf 100644
--- a/xen/drivers/char/Kconfig
+++ b/xen/drivers/char/Kconfig
@@ -95,6 +95,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 86319600e0af..522b2f489a53 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -339,12 +339,12 @@ static void cf_check do_dec_thresh(unsigned char key, bool unused)
  * ********************************************************
  */
 
-/* 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.52.0

[PATCH v5 3/6] xen/console: promote conring{,_size} to __ro_after_init

[dmukhin]

From: Denis Mukhin <dmukhin@ford.com> 

Both conring{,_size} should be RO after initialization is completed.

Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Denis Mukhin <dmukhin@ford.com>
---
Changes since v4:
- new patch
---
 xen/drivers/char/console.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index 522b2f489a53..ef9131439bba 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -346,8 +346,8 @@ size_param("conring_size", opt_conring_size);
 #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;
+static char *__ro_after_init conring = _conring;
+static uint32_t __ro_after_init conring_size = _CONRING_SIZE;
 static uint32_t conringc, conringp;
 
 static void cf_check conring_notify(void *unused)
-- 
2.52.0

[PATCH v5 4/6] xen/console: use memcpy() in console_init_ring()

[dmukhin]

From: Denis Mukhin <dmukhin@ford.com> 

Make console_init_ring() more efficient by using memcpy()'s, rather than
copying the ring a byte at a time.

Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Denis Mukhin <dmukhin@ford.com>
---
Changes since v4:
- new patch
---
 xen/drivers/char/console.c | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index ef9131439bba..3ad86fd436e2 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -463,7 +463,7 @@ static void cf_check conring_dump_keyhandler(unsigned char key)
 void __init console_init_ring(void)
 {
     char *ring;
-    unsigned int i, order, memflags;
+    unsigned int start, size, chunk, order, memflags;
     unsigned long flags;
 
     if ( !opt_conring_size )
@@ -479,11 +479,23 @@ void __init console_init_ring(void)
     opt_conring_size = PAGE_SIZE << order;
 
     nrspin_lock_irqsave(&console_lock, flags);
-    for ( i = conringc ; i != conringp; i++ )
-        ring[i & (opt_conring_size - 1)] = conring[i & (conring_size - 1)];
+
+    start = conringc & (conring_size - 1);
+    size = min(conringp - conringc, conring_size);
+    chunk = min(size, conring_size - start);
+
+    memcpy(&ring[0], &conring[start], chunk);
+    if ( size > chunk )
+        memcpy(&ring[chunk], &conring[0], size - chunk);
+
+    /* Data is moved to [0..size), re-position conring pointers. */
+    conringc = 0;
+    conringp = size;
+
     conring = ring;
     smp_wmb(); /* Allow users of console_force_unlock() to see larger buffer. */
     conring_size = opt_conring_size;
+
     nrspin_unlock_irqrestore(&console_lock, flags);
 
     printk("Allocated console ring of %u KiB.\n", opt_conring_size >> 10);
-- 
2.52.0

[PATCH v5 5/6] xen/console: update conring memory allocation

[dmukhin]

From: Denis Mukhin <dmukhin@ford.com> 

conring buffer doesn't need to be aligned to it's size; it just needs to be
contiguous. Use xmalloc_bytes() in console_init_preirq() for run-time
conring buffer allocation.

Warn user when the conring size is being changed behind the user's
back during the console initialization.

Also, limit the user-selectable conring buffer size to the maximum of 2GB
and minimum of _CONRING_SIZE.

Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Denis Mukhin <dmukhin@ford.com>
---
Changes since v4:
- new patch
---
 xen/drivers/char/console.c | 28 +++++++++++++++++++++-------
 1 file changed, 21 insertions(+), 7 deletions(-)

diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index 3ad86fd436e2..9394ab2a89eb 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -463,20 +463,34 @@ static void cf_check conring_dump_keyhandler(unsigned char key)
 void __init console_init_ring(void)
 {
     char *ring;
-    unsigned int start, size, chunk, order, memflags;
+    unsigned int start, size, chunk;
     unsigned long flags;
 
     if ( !opt_conring_size )
         return;
 
-    order = get_order_from_bytes(max(opt_conring_size, conring_size));
-    memflags = MEMF_bits(crashinfo_maxaddr_bits);
-    while ( (ring = alloc_xenheap_pages(order, memflags)) == NULL )
+    opt_conring_size = max(opt_conring_size, conring_size);
+    size = ROUNDDOWN(opt_conring_size, PAGE_SIZE);
+    if ( size != opt_conring_size )
     {
-        BUG_ON(order == 0);
-        order--;
+        opt_conring_size = size;
+        printk(XENLOG_WARNING "Rounding down console ring size to multiple of %lu KiB.\n",
+               PAGE_SIZE >> 10);
     }
-    opt_conring_size = PAGE_SIZE << order;
+    if ( opt_conring_size >= GB(2) )
+    {
+        opt_conring_size = GB(2);
+        printk(XENLOG_WARNING "Limiting user-configured console ring size.\n");
+    }
+    else if ( opt_conring_size < _CONRING_SIZE )
+    {
+        opt_conring_size = _CONRING_SIZE;
+        printk(XENLOG_WARNING "Using compile-time console ring size.\n");
+    }
+
+    /* Contiguous buffer; does not need to be naturally aligned. */
+    ring = xmalloc_bytes(opt_conring_size);
+    BUG_ON(ring == NULL);
 
     nrspin_lock_irqsave(&console_lock, flags);
 
-- 
2.52.0

[PATCH v5 6/6] xen/console: add conring buffer size alignment setting

[dmukhin]

From: Denis Mukhin <dmukhin@ford.com> 

Introduce CONFIG_CONRING_ALIGN_PAGE_SIZE to control rounding down of the
user-defined conring buffer size.

Also, update the logline reporting the final conring buffer size to report
bytes instead of kilobytes, since the user-defined size may not necessarily
be kilobyte-alined.

Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Denis Mukhin <dmukhin@ford.com>
---
Changes since v4:
- new patch
---
 xen/drivers/char/Kconfig   |  7 +++++++
 xen/drivers/char/console.c | 15 +++++++++------
 2 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/xen/drivers/char/Kconfig b/xen/drivers/char/Kconfig
index d083ba4c9cdf..1b96fbc3ed7c 100644
--- a/xen/drivers/char/Kconfig
+++ b/xen/drivers/char/Kconfig
@@ -95,6 +95,13 @@ config SERIAL_TX_BUFSIZE
 
 	  Default value is 32768 (32KiB).
 
+config CONRING_ALIGN_PAGE_SIZE
+	bool
+	default y
+	help
+	  This selects the console ring buffer size alignment (rounding down)
+	  to a multiple of PAGE_SIZE.
+
 config CONRING_SHIFT
 	int "Console ring buffer size (power of 2)"
 	range 14 27
diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index 9394ab2a89eb..c13818715a52 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -470,12 +470,15 @@ void __init console_init_ring(void)
         return;
 
     opt_conring_size = max(opt_conring_size, conring_size);
-    size = ROUNDDOWN(opt_conring_size, PAGE_SIZE);
-    if ( size != opt_conring_size )
+    if ( IS_ENABLED(CONFIG_CONRING_ALIGN_PAGE_SIZE) )
     {
-        opt_conring_size = size;
-        printk(XENLOG_WARNING "Rounding down console ring size to multiple of %lu KiB.\n",
-               PAGE_SIZE >> 10);
+        size = ROUNDDOWN(opt_conring_size, PAGE_SIZE);
+        if ( size != opt_conring_size )
+        {
+            opt_conring_size = size;
+            printk(XENLOG_WARNING "Rounding down console ring size to multiple of %lu KiB.\n",
+                   PAGE_SIZE >> 10);
+        }
     }
     if ( opt_conring_size >= GB(2) )
     {
@@ -512,7 +515,7 @@ void __init console_init_ring(void)
 
     nrspin_unlock_irqrestore(&console_lock, flags);
 
-    printk("Allocated console ring of %u KiB.\n", opt_conring_size >> 10);
+    printk("Allocated console ring of %u bytes.\n", opt_conring_size);
 }
 
 /*
-- 
2.52.0

Re: [PATCH v5 2/6] xen/console: make console buffer size configurable

[Jan Beulich]

On 05.02.2026 02:36, dmukhin@xen.org wrote:
> --- a/xen/drivers/char/Kconfig
> +++ b/xen/drivers/char/Kconfig
> @@ -95,6 +95,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

As I think I had indicated before - imo an exhaustive table goes too far here.
E.g.

	    27 => 128 MiB
	    26 =>  64 MiB
	    ...
	    15 =>  32 KiB (default)
	    14 =>  16 KiB

would do (if such is needed / wanted at all).

Jan

Re: [PATCH v5 2/6] xen/console: make console buffer size configurable

[dmukhin]

On Thu, Feb 05, 2026 at 09:43:31AM +0100, Jan Beulich wrote:
> On 05.02.2026 02:36, dmukhin@xen.org wrote:
> > --- a/xen/drivers/char/Kconfig
> > +++ b/xen/drivers/char/Kconfig
> > @@ -95,6 +95,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
> 
> As I think I had indicated before - imo an exhaustive table goes too far here.
> E.g.
> 
> 	    27 => 128 MiB
> 	    26 =>  64 MiB
> 	    ...
> 	    15 =>  32 KiB (default)
> 	    14 =>  16 KiB
> 
> would do (if such is needed / wanted at all).

OK, will adjust as suggested.
Thanks!

> 
> Jan

Re: [PATCH v5 1/6] xen/console: group conring code together

[Jan Beulich]

On 05.02.2026 02:36, dmukhin@xen.org wrote:
> From: Denis Mukhin <dmukhin@ford.com> 
> 
> Groups conring buffer management code in the console driver for ease of
> maintaining this code.
> 
> Not a functional change.
> 
> Signed-off-by: Denis Mukhin <dmukhin@ford.com>

Acked-by: Jan Beulich <jbeulich@suse.com>
with one minimal adjustment:

> --- a/xen/drivers/char/console.c
> +++ b/xen/drivers/char/console.c
> @@ -126,17 +126,6 @@ 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). */
> -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))
> -static char __initdata _conring[_CONRING_SIZE];
> -static char *__read_mostly conring = _conring;
> -static uint32_t __read_mostly conring_size = _CONRING_SIZE;
> -static uint32_t conringc, conringp;
> -
>  static int __read_mostly sercon_handle = -1;
>  
>  #ifdef CONFIG_X86
> @@ -350,6 +339,17 @@ static void cf_check do_dec_thresh(unsigned char key, bool unused)
>   * ********************************************************
>   */
>  
> +/* conring_size: allows a large console ring than default (16kB). */

As you move the comment, s/large/larger/. Will adjust while committing.

> +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))
> +static char __initdata _conring[_CONRING_SIZE];
> +static char *__read_mostly conring = _conring;
> +static uint32_t __read_mostly conring_size = _CONRING_SIZE;
> +static uint32_t conringc, conringp;

There are several other tidying things to be done here, but I'm not going
to request that you take care of those, too.

Jan

Re: [PATCH v5 3/6] xen/console: promote conring{,_size} to __ro_after_init

[Jan Beulich]

On 05.02.2026 02:36, dmukhin@xen.org wrote:
> From: Denis Mukhin <dmukhin@ford.com> 
> 
> Both conring{,_size} should be RO after initialization is completed.
> 
> Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Signed-off-by: Denis Mukhin <dmukhin@ford.com>

Ah yes, that's one of those things mentioned for patch 1.
Acked-by: Jan Beulich <jbeulich@suse.com>
preferably with ...

> --- a/xen/drivers/char/console.c
> +++ b/xen/drivers/char/console.c
> @@ -346,8 +346,8 @@ size_param("conring_size", opt_conring_size);
>  #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;
> +static char *__ro_after_init conring = _conring;
> +static uint32_t __ro_after_init conring_size = _CONRING_SIZE;

... the type here also changed to unsigned int (as set forth by ./CODING_STYLE).
Happy to adjust while committing.

Jan

Re: [PATCH v5 4/6] xen/console: use memcpy() in console_init_ring()

[Jan Beulich]

On 05.02.2026 02:36, dmukhin@xen.org wrote:
> @@ -479,11 +479,23 @@ void __init console_init_ring(void)
>      opt_conring_size = PAGE_SIZE << order;
>  
>      nrspin_lock_irqsave(&console_lock, flags);
> -    for ( i = conringc ; i != conringp; i++ )
> -        ring[i & (opt_conring_size - 1)] = conring[i & (conring_size - 1)];
> +
> +    start = conringc & (conring_size - 1);
> +    size = min(conringp - conringc, conring_size);

Is this correct when the ring size actually shrinks? In such a case you want
to copy the tail of the ring if not all of the original contents fits in the
new one. But ...

> +    chunk = min(size, conring_size - start);
> +
> +    memcpy(&ring[0], &conring[start], chunk);

... you start at its head.

> +    if ( size > chunk )
> +        memcpy(&ring[chunk], &conring[0], size - chunk);
> +
> +    /* Data is moved to [0..size), re-position conring pointers. */
> +    conringc = 0;
> +    conringp = size;

Why this unrelated change, which the description also doesn't mention? Since
this is in an __init function, there's no race with read_console_ring(), but
a static analysis tool may still (validly) spot one. Yet then there's also
conring_flush(), which doesn't look to be using any locking either. Have you
excluded that this function can run in a racing manner?

Jan

Re: [PATCH v5 5/6] xen/console: update conring memory allocation

[Jan Beulich]

On 05.02.2026 02:36, dmukhin@xen.org wrote:
> --- a/xen/drivers/char/console.c
> +++ b/xen/drivers/char/console.c
> @@ -463,20 +463,34 @@ static void cf_check conring_dump_keyhandler(unsigned char key)
>  void __init console_init_ring(void)
>  {
>      char *ring;
> -    unsigned int start, size, chunk, order, memflags;
> +    unsigned int start, size, chunk;
>      unsigned long flags;
>  
>      if ( !opt_conring_size )
>          return;
>  
> -    order = get_order_from_bytes(max(opt_conring_size, conring_size));
> -    memflags = MEMF_bits(crashinfo_maxaddr_bits);
> -    while ( (ring = alloc_xenheap_pages(order, memflags)) == NULL )
> +    opt_conring_size = max(opt_conring_size, conring_size);
> +    size = ROUNDDOWN(opt_conring_size, PAGE_SIZE);
> +    if ( size != opt_conring_size )
>      {
> -        BUG_ON(order == 0);
> -        order--;
> +        opt_conring_size = size;
> +        printk(XENLOG_WARNING "Rounding down console ring size to multiple of %lu KiB.\n",
> +               PAGE_SIZE >> 10);
>      }
> -    opt_conring_size = PAGE_SIZE << order;
> +    if ( opt_conring_size >= GB(2) )
> +    {
> +        opt_conring_size = GB(2);
> +        printk(XENLOG_WARNING "Limiting user-configured console ring size.\n");
> +    }
> +    else if ( opt_conring_size < _CONRING_SIZE )
> +    {
> +        opt_conring_size = _CONRING_SIZE;
> +        printk(XENLOG_WARNING "Using compile-time console ring size.\n");
> +    }
> +
> +    /* Contiguous buffer; does not need to be naturally aligned. */
> +    ring = xmalloc_bytes(opt_conring_size);

I'm sorry, but I'm going to veto any new uses of xmalloc_bytes(). As per the
comment at the top of xvmalloc.h, the family of functions there should be used
in new code. That family deliberately doesn't include a counterpart of
xmalloc_bytes(). You're wanting a multiple of page size anyway, so perhaps it
is warranted here to actually use vmalloc() directly.

Jan

Re: [PATCH v5 5/6] xen/console: update conring memory allocation

[Jan Beulich]

On 05.02.2026 02:36, dmukhin@xen.org wrote:
> --- a/xen/drivers/char/console.c
> +++ b/xen/drivers/char/console.c
> @@ -463,20 +463,34 @@ static void cf_check conring_dump_keyhandler(unsigned char key)
>  void __init console_init_ring(void)
>  {
>      char *ring;
> -    unsigned int start, size, chunk, order, memflags;
> +    unsigned int start, size, chunk;
>      unsigned long flags;
>  
>      if ( !opt_conring_size )
>          return;
>  
> -    order = get_order_from_bytes(max(opt_conring_size, conring_size));
> -    memflags = MEMF_bits(crashinfo_maxaddr_bits);
> -    while ( (ring = alloc_xenheap_pages(order, memflags)) == NULL )
> +    opt_conring_size = max(opt_conring_size, conring_size);
> +    size = ROUNDDOWN(opt_conring_size, PAGE_SIZE);
> +    if ( size != opt_conring_size )
>      {
> -        BUG_ON(order == 0);
> -        order--;
> +        opt_conring_size = size;
> +        printk(XENLOG_WARNING "Rounding down console ring size to multiple of %lu KiB.\n",
> +               PAGE_SIZE >> 10);
>      }
> -    opt_conring_size = PAGE_SIZE << order;

I've spotted this removal only while looking at patch 6: How does this
work? We require conring_size to be a power of 2, or else masking by
(conring_size - 1) isn't a valid thing to do. You even touch
CONRING_IDX_MASK() twice in this series, so you really should have
noticed.

> +    if ( opt_conring_size >= GB(2) )
> +    {
> +        opt_conring_size = GB(2);
> +        printk(XENLOG_WARNING "Limiting user-configured console ring size.\n");
> +    }
> +    else if ( opt_conring_size < _CONRING_SIZE )
> +    {
> +        opt_conring_size = _CONRING_SIZE;
> +        printk(XENLOG_WARNING "Using compile-time console ring size.\n");
> +    }
> +
> +    /* Contiguous buffer; does not need to be naturally aligned. */
> +    ring = xmalloc_bytes(opt_conring_size);
> +    BUG_ON(ring == NULL);
>  
>      nrspin_lock_irqsave(&console_lock, flags);
>  

Re: [PATCH v5 6/6] xen/console: add conring buffer size alignment setting

[Jan Beulich]

On 05.02.2026 02:36, dmukhin@xen.org wrote:
> From: Denis Mukhin <dmukhin@ford.com> 
> 
> Introduce CONFIG_CONRING_ALIGN_PAGE_SIZE to control rounding down of the
> user-defined conring buffer size.

What's wrong with the rounding? The more that, with the original behavior
properly restored in patch 5, it'll be a power-of-2 multiple of PAGE_SIZE
anyway?

> Also, update the logline reporting the final conring buffer size to report
> bytes instead of kilobytes, since the user-defined size may not necessarily
> be kilobyte-alined.

Yet making the number harder to grok.

> Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com>

Having talked to him, I don't think he meant what you're doing here. All he
apparently meant is to stop using alloc_*heap_pages(), which needlessly
supplies order-aligned memory.

> --- a/xen/drivers/char/Kconfig
> +++ b/xen/drivers/char/Kconfig
> @@ -95,6 +95,13 @@ config SERIAL_TX_BUFSIZE
>  
>  	  Default value is 32768 (32KiB).
>  
> +config CONRING_ALIGN_PAGE_SIZE
> +	bool
> +	default y

IOW "def_bool y". But what's the point of the option when there's no prompt?

> --- a/xen/drivers/char/console.c
> +++ b/xen/drivers/char/console.c
> @@ -470,12 +470,15 @@ void __init console_init_ring(void)
>          return;
>  
>      opt_conring_size = max(opt_conring_size, conring_size);
> -    size = ROUNDDOWN(opt_conring_size, PAGE_SIZE);
> -    if ( size != opt_conring_size )
> +    if ( IS_ENABLED(CONFIG_CONRING_ALIGN_PAGE_SIZE) )
>      {
> -        opt_conring_size = size;
> -        printk(XENLOG_WARNING "Rounding down console ring size to multiple of %lu KiB.\n",
> -               PAGE_SIZE >> 10);
> +        size = ROUNDDOWN(opt_conring_size, PAGE_SIZE);
> +        if ( size != opt_conring_size )
> +        {
> +            opt_conring_size = size;
> +            printk(XENLOG_WARNING "Rounding down console ring size to multiple of %lu KiB.\n",

This line wants splitting at the start of the string literal, and the full
stop wants removing if already it is being touched.

Jan

Re: [PATCH v5 1/6] xen/console: group conring code together

[dmukhin]

On Mon, Feb 09, 2026 at 05:38:36PM +0100, Jan Beulich wrote:
> On 05.02.2026 02:36, dmukhin@xen.org wrote:
> > From: Denis Mukhin <dmukhin@ford.com> 
> > 
> > Groups conring buffer management code in the console driver for ease of
> > maintaining this code.
> > 
> > Not a functional change.
> > 
> > Signed-off-by: Denis Mukhin <dmukhin@ford.com>
> 
> Acked-by: Jan Beulich <jbeulich@suse.com>
> with one minimal adjustment:
> 
> > --- a/xen/drivers/char/console.c
> > +++ b/xen/drivers/char/console.c
> > @@ -126,17 +126,6 @@ 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). */
> > -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))
> > -static char __initdata _conring[_CONRING_SIZE];
> > -static char *__read_mostly conring = _conring;
> > -static uint32_t __read_mostly conring_size = _CONRING_SIZE;
> > -static uint32_t conringc, conringp;
> > -
> >  static int __read_mostly sercon_handle = -1;
> >  
> >  #ifdef CONFIG_X86
> > @@ -350,6 +339,17 @@ static void cf_check do_dec_thresh(unsigned char key, bool unused)
> >   * ********************************************************
> >   */
> >  
> > +/* conring_size: allows a large console ring than default (16kB). */
> 
> As you move the comment, s/large/larger/. Will adjust while committing.

Thanks!

> 
> > +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))
> > +static char __initdata _conring[_CONRING_SIZE];
> > +static char *__read_mostly conring = _conring;
> > +static uint32_t __read_mostly conring_size = _CONRING_SIZE;
> > +static uint32_t conringc, conringp;
> 
> There are several other tidying things to be done here, but I'm not going
> to request that you take care of those, too.
> 
> Jan

Re: [PATCH v5 3/6] xen/console: promote conring{,_size} to __ro_after_init

[dmukhin]

On Mon, Feb 09, 2026 at 05:40:33PM +0100, Jan Beulich wrote:
> On 05.02.2026 02:36, dmukhin@xen.org wrote:
> > From: Denis Mukhin <dmukhin@ford.com> 
> > 
> > Both conring{,_size} should be RO after initialization is completed.
> > 
> > Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com>
> > Signed-off-by: Denis Mukhin <dmukhin@ford.com>
> 
> Ah yes, that's one of those things mentioned for patch 1.
> Acked-by: Jan Beulich <jbeulich@suse.com>
> preferably with ...
> 
> > --- a/xen/drivers/char/console.c
> > +++ b/xen/drivers/char/console.c
> > @@ -346,8 +346,8 @@ size_param("conring_size", opt_conring_size);
> >  #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;
> > +static char *__ro_after_init conring = _conring;
> > +static uint32_t __ro_after_init conring_size = _CONRING_SIZE;
> 
> ... the type here also changed to unsigned int (as set forth by ./CODING_STYLE).
> Happy to adjust while committing.

Will appreciate help here.
Thanks!

> 
> Jan

Re: [PATCH v5 5/6] xen/console: update conring memory allocation

[dmukhin]

On Mon, Feb 09, 2026 at 06:02:52PM +0100, Jan Beulich wrote:
> On 05.02.2026 02:36, dmukhin@xen.org wrote:
> > --- a/xen/drivers/char/console.c
> > +++ b/xen/drivers/char/console.c
> > @@ -463,20 +463,34 @@ static void cf_check conring_dump_keyhandler(unsigned char key)
> >  void __init console_init_ring(void)
> >  {
> >      char *ring;
> > -    unsigned int start, size, chunk, order, memflags;
> > +    unsigned int start, size, chunk;
> >      unsigned long flags;
> >  
> >      if ( !opt_conring_size )
> >          return;
> >  
> > -    order = get_order_from_bytes(max(opt_conring_size, conring_size));
> > -    memflags = MEMF_bits(crashinfo_maxaddr_bits);
> > -    while ( (ring = alloc_xenheap_pages(order, memflags)) == NULL )
> > +    opt_conring_size = max(opt_conring_size, conring_size);
> > +    size = ROUNDDOWN(opt_conring_size, PAGE_SIZE);
> > +    if ( size != opt_conring_size )
> >      {
> > -        BUG_ON(order == 0);
> > -        order--;
> > +        opt_conring_size = size;
> > +        printk(XENLOG_WARNING "Rounding down console ring size to multiple of %lu KiB.\n",
> > +               PAGE_SIZE >> 10);
> >      }
> > -    opt_conring_size = PAGE_SIZE << order;
> > +    if ( opt_conring_size >= GB(2) )
> > +    {
> > +        opt_conring_size = GB(2);
> > +        printk(XENLOG_WARNING "Limiting user-configured console ring size.\n");
> > +    }
> > +    else if ( opt_conring_size < _CONRING_SIZE )
> > +    {
> > +        opt_conring_size = _CONRING_SIZE;
> > +        printk(XENLOG_WARNING "Using compile-time console ring size.\n");
> > +    }
> > +
> > +    /* Contiguous buffer; does not need to be naturally aligned. */
> > +    ring = xmalloc_bytes(opt_conring_size);
> 
> I'm sorry, but I'm going to veto any new uses of xmalloc_bytes(). As per the
> comment at the top of xvmalloc.h, the family of functions there should be used
> in new code. That family deliberately doesn't include a counterpart of
> xmalloc_bytes(). You're wanting a multiple of page size anyway, so perhaps it
> is warranted here to actually use vmalloc() directly.

Thanks for taking a look at the series!

Sorry it's been a while, I am getting back to the series only now :-/

Turns out that vmalloc() is not available on MPU systems (*-mpu jobs in CI):
CONFIG_HAS_VMAP is disabled.

I will switch to xvmalloc-backed allocation, if that's OK.

--
Denis

Re: [PATCH v5 6/6] xen/console: add conring buffer size alignment setting

[dmukhin]

On Tue, Feb 10, 2026 at 04:10:42PM +0100, Jan Beulich wrote:
> On 05.02.2026 02:36, dmukhin@xen.org wrote:
> > From: Denis Mukhin <dmukhin@ford.com> 
> > 
> > Introduce CONFIG_CONRING_ALIGN_PAGE_SIZE to control rounding down of the
> > user-defined conring buffer size.
> 
> What's wrong with the rounding? The more that, with the original behavior
> properly restored in patch 5, it'll be a power-of-2 multiple of PAGE_SIZE
> anyway?
> 
> > Also, update the logline reporting the final conring buffer size to report
> > bytes instead of kilobytes, since the user-defined size may not necessarily
> > be kilobyte-alined.
> 
> Yet making the number harder to grok.
> 
> > Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com>
> 
> Having talked to him, I don't think he meant what you're doing here. All he
> apparently meant is to stop using alloc_*heap_pages(), which needlessly
> supplies order-aligned memory.

Thanks, I will drop that patch.

--
Denis