[PATCH v8 0/7] xen/console: some cleanups and configurable conring size

[PATCH v8 0/7] xen/console: some cleanups and configurable conring size

[dmukhin]

Series introduces some cleanups in console conring and serial
driver buffer management code ending up with adding compile-time
configuration knob for conring size.

Patch 1-2 correct the data types used in conring management code.

Patch 3 updates the conring buffer allocation code.

Patch 4 updates the serial driver buffer allocation code.

Patch 5 optimizes code in conring_puts().

Patch 6 hardens the checks in the serial driver buffer management code.

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

[1] v7: https://lore.kernel.org/xen-devel/20260713181619.672176-1-dmukhin@ford.com/
[2] CI: https://gitlab.com/xen-project/people/dmukhin/xen/-/pipelines/2710713367

Denis Mukhin (7):
  xen/console: do not use XENCONS_RING_IDX in console_init_ring()
  xen/console: use 'unsigned int' in contring_{flush,puts}()
  xen/console: switch conring runtime allocation to xvmalloc
  xen/serial: switch txbuf runtime allocation to xvmalloc
  xen/console: use memcpy() in conring_puts()
  xen/serial: harden serial_tx_buffer checks
  xen/console: make console buffer size configurable

 docs/misc/xen-command-line.pandoc | 10 ++++-
 xen/drivers/char/Kconfig          | 21 +++++++++++
 xen/drivers/char/console.c        | 61 +++++++++++++++++++++----------
 xen/drivers/char/serial.c         |  6 ++-
 4 files changed, 75 insertions(+), 23 deletions(-)

-- 
2.54.0

[PATCH v8 1/7] xen/console: do not use XENCONS_RING_IDX in console_init_ring()

[dmukhin]

From: Denis Mukhin <dmukhin@ford.com> 

Replace XENCONS_RING_IDX with unsigned int for the console ring indices,
as the console ring is not a Xen console (XENCONS) ring.

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

diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index ea4e3ff34178..37fdda93a4c1 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;
-    XENCONS_RING_IDX done, size, n;
+    unsigned int done, size, n;
     unsigned int order, memflags;
     unsigned long flags;
 
@@ -484,8 +484,8 @@ void __init console_init_ring(void)
     size = conringp - conringc;
     for ( done = 0; done < size; done += n )
     {
-        XENCONS_RING_IDX src = (conringc + done) & (conring_size - 1);
-        XENCONS_RING_IDX dst = (conringc + done) & (opt_conring_size - 1);
+        unsigned int src = (conringc + done) & (conring_size - 1);
+        unsigned int dst = (conringc + done) & (opt_conring_size - 1);
 
         n = min(opt_conring_size - dst, conring_size - src);
         n = min(size - done, n);
-- 
2.54.0

[PATCH v8 2/7] xen/console: use 'unsigned int' in contring_{flush,puts}()

[dmukhin]

From: Denis Mukhin <dmukhin@ford.com> 

contring_puts() and conring_flush() still use 'uint32_t' to access
indices.

Switch to 'unsigned int' to as required by CODING_STYLE.

Signed-off-by: Denis Mukhin <dmukhin@ford.com>
---
Changes since v7:
- 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 37fdda93a4c1..40355c1d14d6 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -375,7 +375,7 @@ static void conring_puts(const char *str, size_t len)
 long read_console_ring(struct xen_sysctl_readconsole *op)
 {
     XEN_GUEST_HANDLE_PARAM(char) str;
-    uint32_t idx, len, max, sofar, c, p;
+    unsigned int idx, len, max, sofar, c, p;
 
     str   = guest_handle_cast(op->buffer, char),
     max   = op->count;
@@ -421,7 +421,7 @@ long read_console_ring(struct xen_sysctl_readconsole *op)
  */
 static int conring_flush(unsigned int flags)
 {
-    uint32_t idx, len, sofar, c;
+    unsigned int idx, len, sofar, c;
     unsigned int order;
     char *buf;
 
-- 
2.54.0

[PATCH v8 3/7] xen/console: switch conring runtime allocation to xvmalloc

[dmukhin]

From: Denis Mukhin <dmukhin@ford.com> 

The console ring only needs to be virtually contiguous; it does not need
a naturally aligned or physically contiguous allocation. Replace the
runtime xenheap allocation in console_init_ring() with an xvmalloc-backed
buffer.

Also clamp the user-configured ring size to the supported range and emit
warning when the requested size is adjusted.

Drop full stops in all diagnostic messages in console_init_ring() to align
code with the common code pattern.

Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Denis Mukhin <dmukhin@ford.com>
---
Changes since v7:
- Jan's feedback from
  https://lore.kernel.org/xen-devel/0fefa50c-46aa-4ede-a8e2-8c2c619bc2ab@suse.com/
---
 xen/drivers/char/console.c | 27 +++++++++++++++++++--------
 1 file changed, 19 insertions(+), 8 deletions(-)

diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index 40355c1d14d6..09282a7a4f8e 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -33,6 +33,7 @@
 #include <asm/setup.h>
 #include <xen/sections.h>
 #include <xen/consoled.h>
+#include <xen/xvmalloc.h>
 
 #ifdef CONFIG_X86
 #include <asm/guest.h>
@@ -464,20 +465,30 @@ void __init console_init_ring(void)
 {
     char *ring;
     unsigned int done, size, n;
-    unsigned int 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 )
+    if ( opt_conring_size < GB(2) )
     {
-        BUG_ON(order == 0);
-        order--;
+        unsigned int order = get_order_from_bytes(max(opt_conring_size,
+                                                      conring_size));
+
+        opt_conring_size = PAGE_SIZE << order;
+    }
+    else
+    {
+        printk(XENLOG_WARNING
+               "Limiting user-configured console ring size to 2 GiB\n");
+        opt_conring_size = GB(2);
+    }
+
+    while ( (ring = xvmalloc_array(char, opt_conring_size)) == NULL )
+    {
+        BUG_ON(opt_conring_size == 0);
+        opt_conring_size >>= 1;
     }
-    opt_conring_size = PAGE_SIZE << order;
 
     nrspin_lock_irqsave(&console_lock, flags);
 
@@ -498,7 +509,7 @@ void __init console_init_ring(void)
     conring_size = opt_conring_size;
     nrspin_unlock_irqrestore(&console_lock, flags);
 
-    printk("Allocated console ring of %u KiB.\n", opt_conring_size >> 10);
+    printk("Allocated console ring of %u KiB\n", opt_conring_size >> 10);
 }
 
 /*
-- 
2.54.0

[PATCH v8 4/7] xen/serial: switch txbuf runtime allocation to xvmalloc

[dmukhin]

From: Denis Mukhin <dmukhin@ford.com> 

Switch 'txbuf' allocation to xvmalloc_array() since there is no
hard requirement to have buffer physically contiguous.

Suggested-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Denis Mukhin <dmukhin@ford.com>
---
Changes since v7:
- new patch
---
 xen/drivers/char/serial.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/xen/drivers/char/serial.c b/xen/drivers/char/serial.c
index e3c356408987..cf0abf1893e5 100644
--- a/xen/drivers/char/serial.c
+++ b/xen/drivers/char/serial.c
@@ -12,6 +12,7 @@
 #include <xen/param.h>
 #include <xen/sections.h>
 #include <xen/serial.h>
+#include <xen/xvmalloc.h>
 
 #include <asm/processor.h>
 
@@ -524,8 +525,7 @@ void __init serial_async_transmit(struct serial_port *port)
         serial_txbufsz = PAGE_SIZE;
     while ( serial_txbufsz & (serial_txbufsz - 1) )
         serial_txbufsz &= serial_txbufsz - 1;
-    port->txbuf = alloc_xenheap_pages(
-        get_order_from_bytes(serial_txbufsz), 0);
+    port->txbuf = xvmalloc_array(char, serial_txbufsz);
 }
 
 /*
-- 
2.54.0

[PATCH v8 5/7] xen/console: use memcpy() in conring_puts()

[dmukhin]

From: Denis Mukhin <dmukhin@ford.com> 

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

No functional change intended.

Signed-off-by: Denis Mukhin <dmukhin@ford.com>
---
Changes since v7:
- hardended len check in conring_puts()
---
 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 09282a7a4f8e..a1b8e5f5b507 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -361,12 +361,24 @@ static DECLARE_SOFTIRQ_TASKLET(conring_tasklet, conring_notify, NULL);
 /* NB: Do not send conring VIRQs during panic. */
 static bool conring_no_notify;
 
-static void conring_puts(const char *str, size_t len)
+static void conring_puts(const char *str, unsigned int len)
 {
+    unsigned int src = len;
+
+    /* There are no callers with strings longer than PAGE_SIZE. */
+    BUG_ON(len > PAGE_SIZE);
     ASSERT(rspin_is_locked(&console_lock));
 
-    while ( len-- )
-        conring[CONRING_IDX_MASK(conringp++)] = *str++;
+    while ( src < len )
+    {
+        unsigned int dst = CONRING_IDX_MASK(conringp + src);
+        unsigned int n = min(conring_size - dst, len - src);
+
+        memcpy(&conring[dst], &str[src], n);
+        src += n;
+    }
+
+    conringp += len;
 
     if ( conringp - conringc > conring_size )
         conringc = conringp - conring_size;
-- 
2.54.0

[PATCH v8 6/7] xen/serial: harden serial_tx_buffer checks

[dmukhin]

From: Denis Mukhin <dmukhin@ford.com> 

Ensure the user-defined value never crosses 2GB boundary and always
rounded to the next power of 2 to align logic with console driver
conring buffer management code.

Signed-off-by: Denis Mukhin <dmukhin@ford.com>
---
Changes since v7:
- addressed Jan's feedback:
  https://lore.kernel.org/xen-devel/89029dbd-df1f-45d4-8a02-720cd6a42cab@suse.com/
- kept only check for large buffer in serial_async_transmit()
  and a doc update.
---
 docs/misc/xen-command-line.pandoc | 2 ++
 xen/drivers/char/serial.c         | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/docs/misc/xen-command-line.pandoc b/docs/misc/xen-command-line.pandoc
index 1c711fa98086..2be8772b329a 100644
--- a/docs/misc/xen-command-line.pandoc
+++ b/docs/misc/xen-command-line.pandoc
@@ -2396,6 +2396,8 @@ accidentally leaking secrets by releasing pages without proper sanitization.
 
 Set the serial transmit buffer size.
 
+The value provided will be rounded down to the nearest power of 2.
+
 ### serrors (ARM)
 > `= diverse | panic`
 
diff --git a/xen/drivers/char/serial.c b/xen/drivers/char/serial.c
index cf0abf1893e5..ba1647309ab8 100644
--- a/xen/drivers/char/serial.c
+++ b/xen/drivers/char/serial.c
@@ -523,6 +523,8 @@ void __init serial_async_transmit(struct serial_port *port)
         return;
     if ( serial_txbufsz < PAGE_SIZE )
         serial_txbufsz = PAGE_SIZE;
+    if ( serial_txbufsz > GB(2) )
+        serial_txbufsz = CONFIG_SERIAL_TX_BUFSIZE;
     while ( serial_txbufsz & (serial_txbufsz - 1) )
         serial_txbufsz &= serial_txbufsz - 1;
     port->txbuf = xvmalloc_array(char, serial_txbufsz);
-- 
2.54.0

[PATCH v8 7/7] 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).

Update the documentation for 'conring_size=' command line option.

Resolves: https://gitlab.com/xen-project/xen/-/issues/185
Signed-off-by: Denis Mukhin <dmukhin@ford.com>
---
Changes since v7:
- n/a
---
 docs/misc/xen-command-line.pandoc |  8 ++++++--
 xen/drivers/char/Kconfig          | 21 +++++++++++++++++++++
 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 2be8772b329a..448c9bdb8254 100644
--- a/docs/misc/xen-command-line.pandoc
+++ b/docs/misc/xen-command-line.pandoc
@@ -425,10 +425,14 @@ 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.
+
+The run-time console ring buffer size is the maximum of the build-time value
+and the value specified by the `conring_size=` command-line option.
+
 ### 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..a40a9929132b 100644
--- a/xen/drivers/char/Kconfig
+++ b/xen/drivers/char/Kconfig
@@ -95,6 +95,27 @@ 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.
+
+	  The run-time console ring buffer is the maximum of the build-time
+	  value and the value specified by the `conring_size=` command-line
+	  option.
+
+	  If `conring_size=` is not specified on the command line, the run-time
+	  console ring buffer size is the maximum of this value and
+	  `num_present_cpus() << (9 + xenlog_lower_thresh)`.
+
+	    27 => 128 MiB
+	    26 =>  64 MiB
+	    ...
+	    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 a1b8e5f5b507..76367c1dd705 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -340,12 +340,12 @@ static void cf_check do_dec_thresh(unsigned char key, bool unused)
  * ********************************************************
  */
 
-/* conring_size: allows a larger console ring than default (16kB). */
+/* conring_size: override build-time CONFIG_CONRING_SHIFT setting. */
 static unsigned int __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 *__ro_after_init conring = _conring;
 static unsigned int __ro_after_init conring_size = _CONRING_SIZE;
-- 
2.54.0

Re: [PATCH v8 1/7] xen/console: do not use XENCONS_RING_IDX in console_init_ring()

[Stefano Stabellini]

On Mon, 27 Jul 2026, dmukhin@ford.com wrote:
> From: Denis Mukhin <dmukhin@ford.com> 
> 
> Replace XENCONS_RING_IDX with unsigned int for the console ring indices,
> as the console ring is not a Xen console (XENCONS) ring.
> 
> Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Signed-off-by: Denis Mukhin <dmukhin@ford.com>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>


> ---
> Changes since v7:
> - new patch
> ---
>  xen/drivers/char/console.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
> index ea4e3ff34178..37fdda93a4c1 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;
> -    XENCONS_RING_IDX done, size, n;
> +    unsigned int done, size, n;
>      unsigned int order, memflags;
>      unsigned long flags;
>  
> @@ -484,8 +484,8 @@ void __init console_init_ring(void)
>      size = conringp - conringc;
>      for ( done = 0; done < size; done += n )
>      {
> -        XENCONS_RING_IDX src = (conringc + done) & (conring_size - 1);
> -        XENCONS_RING_IDX dst = (conringc + done) & (opt_conring_size - 1);
> +        unsigned int src = (conringc + done) & (conring_size - 1);
> +        unsigned int dst = (conringc + done) & (opt_conring_size - 1);
>  
>          n = min(opt_conring_size - dst, conring_size - src);
>          n = min(size - done, n);
> -- 
> 2.54.0
> 

Re: [PATCH v8 2/7] xen/console: use 'unsigned int' in contring_{flush,puts}()

[Stefano Stabellini]

On Mon, 27 Jul 2026, dmukhin@ford.com wrote:
> From: Denis Mukhin <dmukhin@ford.com> 
> 
> contring_puts() and conring_flush() still use 'uint32_t' to access
> indices.
> 
> Switch to 'unsigned int' to as required by CODING_STYLE.
> 
> Signed-off-by: Denis Mukhin <dmukhin@ford.com>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>

> ---
> Changes since v7:
> - 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 37fdda93a4c1..40355c1d14d6 100644
> --- a/xen/drivers/char/console.c
> +++ b/xen/drivers/char/console.c
> @@ -375,7 +375,7 @@ static void conring_puts(const char *str, size_t len)
>  long read_console_ring(struct xen_sysctl_readconsole *op)
>  {
>      XEN_GUEST_HANDLE_PARAM(char) str;
> -    uint32_t idx, len, max, sofar, c, p;
> +    unsigned int idx, len, max, sofar, c, p;
>  
>      str   = guest_handle_cast(op->buffer, char),
>      max   = op->count;
> @@ -421,7 +421,7 @@ long read_console_ring(struct xen_sysctl_readconsole *op)
>   */
>  static int conring_flush(unsigned int flags)
>  {
> -    uint32_t idx, len, sofar, c;
> +    unsigned int idx, len, sofar, c;
>      unsigned int order;
>      char *buf;
>  
> -- 
> 2.54.0
> 

Re: [PATCH v8 3/7] xen/console: switch conring runtime allocation to xvmalloc

[Stefano Stabellini]

On Mon, 27 Jul 2026, dmukhin@ford.com wrote:
> From: Denis Mukhin <dmukhin@ford.com> 
> 
> The console ring only needs to be virtually contiguous; it does not need
> a naturally aligned or physically contiguous allocation. Replace the
> runtime xenheap allocation in console_init_ring() with an xvmalloc-backed
> buffer.
> 
> Also clamp the user-configured ring size to the supported range and emit
> warning when the requested size is adjusted.
> 
> Drop full stops in all diagnostic messages in console_init_ring() to align
> code with the common code pattern.
> 
> Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Signed-off-by: Denis Mukhin <dmukhin@ford.com>

There is another alloc_xenheap_pages in the same file, in conring_flush.
It would probably need to be changed as well.

> ---
> Changes since v7:
> - Jan's feedback from
>   https://lore.kernel.org/xen-devel/0fefa50c-46aa-4ede-a8e2-8c2c619bc2ab@suse.com/
> ---
>  xen/drivers/char/console.c | 27 +++++++++++++++++++--------
>  1 file changed, 19 insertions(+), 8 deletions(-)
> 
> diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
> index 40355c1d14d6..09282a7a4f8e 100644
> --- a/xen/drivers/char/console.c
> +++ b/xen/drivers/char/console.c
> @@ -33,6 +33,7 @@
>  #include <asm/setup.h>
>  #include <xen/sections.h>
>  #include <xen/consoled.h>
> +#include <xen/xvmalloc.h>
>  
>  #ifdef CONFIG_X86
>  #include <asm/guest.h>
> @@ -464,20 +465,30 @@ void __init console_init_ring(void)
>  {
>      char *ring;
>      unsigned int done, size, n;
> -    unsigned int 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);

The original code had MEMF_bits(crashinfo_maxaddr_bits).
crashinfo_maxaddr_bits is 64-bit by default but can be changed via
command line options. Now, the memflags is going away and there is no
way to bring it back because xvmalloc_array doesn't take memflags as a
parameter.

Andrew, Jan, is that OK?


> -    while ( (ring = alloc_xenheap_pages(order, memflags)) == NULL )
> +    if ( opt_conring_size < GB(2) )
>      {
> -        BUG_ON(order == 0);
> -        order--;
> +        unsigned int order = get_order_from_bytes(max(opt_conring_size,
> +                                                      conring_size));
> +
> +        opt_conring_size = PAGE_SIZE << order;
> +    }
> +    else
> +    {
> +        printk(XENLOG_WARNING
> +               "Limiting user-configured console ring size to 2 GiB\n");
> +        opt_conring_size = GB(2);
> +    }
> +
> +    while ( (ring = xvmalloc_array(char, opt_conring_size)) == NULL )

It looks like that if opt_conring_size is zero, then xvmalloc_array
would return ZERO_BLOCK_PTR which is != NULL. We need to have a
different check here for that condition


> +    {
> +        BUG_ON(opt_conring_size == 0);
> +        opt_conring_size >>= 1;
>      }
> -    opt_conring_size = PAGE_SIZE << order;
>  
>      nrspin_lock_irqsave(&console_lock, flags);
>  
> @@ -498,7 +509,7 @@ void __init console_init_ring(void)
>      conring_size = opt_conring_size;
>      nrspin_unlock_irqrestore(&console_lock, flags);
>  
> -    printk("Allocated console ring of %u KiB.\n", opt_conring_size >> 10);
> +    printk("Allocated console ring of %u KiB\n", opt_conring_size >> 10);
>  }
>  
>  /*
> -- 
> 2.54.0
> 

Re: [PATCH v8 4/7] xen/serial: switch txbuf runtime allocation to xvmalloc

[Stefano Stabellini]

On Mon, 27 Jul 2026, dmukhin@ford.com wrote:

> From: Denis Mukhin <dmukhin@ford.com> 
> 
> Switch 'txbuf' allocation to xvmalloc_array() since there is no
> hard requirement to have buffer physically contiguous.
> 
> Suggested-by: Jan Beulich <jbeulich@suse.com>
> Signed-off-by: Denis Mukhin <dmukhin@ford.com>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>


> ---
> Changes since v7:
> - new patch
> ---
>  xen/drivers/char/serial.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/drivers/char/serial.c b/xen/drivers/char/serial.c
> index e3c356408987..cf0abf1893e5 100644
> --- a/xen/drivers/char/serial.c
> +++ b/xen/drivers/char/serial.c
> @@ -12,6 +12,7 @@
>  #include <xen/param.h>
>  #include <xen/sections.h>
>  #include <xen/serial.h>
> +#include <xen/xvmalloc.h>
>  
>  #include <asm/processor.h>
>  
> @@ -524,8 +525,7 @@ void __init serial_async_transmit(struct serial_port *port)
>          serial_txbufsz = PAGE_SIZE;
>      while ( serial_txbufsz & (serial_txbufsz - 1) )
>          serial_txbufsz &= serial_txbufsz - 1;
> -    port->txbuf = alloc_xenheap_pages(
> -        get_order_from_bytes(serial_txbufsz), 0);
> +    port->txbuf = xvmalloc_array(char, serial_txbufsz);
>  }
>  
>  /*
> -- 
> 2.54.0
> 

Re: [PATCH v8 5/7] xen/console: use memcpy() in conring_puts()

[Stefano Stabellini]

On Mon, 27 Jul 2026, dmukhin@ford.com wrote:
> From: Denis Mukhin <dmukhin@ford.com> 
> 
> Make conring_puts() more efficient by using memcpy()'s, rather than
> copying the ring a byte at a time.
> 
> No functional change intended.
> 
> Signed-off-by: Denis Mukhin <dmukhin@ford.com>
> ---
> Changes since v7:
> - hardended len check in conring_puts()
> ---
>  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 09282a7a4f8e..a1b8e5f5b507 100644
> --- a/xen/drivers/char/console.c
> +++ b/xen/drivers/char/console.c
> @@ -361,12 +361,24 @@ static DECLARE_SOFTIRQ_TASKLET(conring_tasklet, conring_notify, NULL);
>  /* NB: Do not send conring VIRQs during panic. */
>  static bool conring_no_notify;
>  
> -static void conring_puts(const char *str, size_t len)
> +static void conring_puts(const char *str, unsigned int len)
>  {
> +    unsigned int src = len;
> +
> +    /* There are no callers with strings longer than PAGE_SIZE. */
> +    BUG_ON(len > PAGE_SIZE);

Should be an ASSERT


>      ASSERT(rspin_is_locked(&console_lock));
>  
> -    while ( len-- )
> -        conring[CONRING_IDX_MASK(conringp++)] = *str++;
> +    while ( src < len )

src is initialized to len, so this is a problem?


> +    {
> +        unsigned int dst = CONRING_IDX_MASK(conringp + src);
> +        unsigned int n = min(conring_size - dst, len - src);
> +
> +        memcpy(&conring[dst], &str[src], n);
> +        src += n;
> +    }
> +
> +    conringp += len;
>  
>      if ( conringp - conringc > conring_size )
>          conringc = conringp - conring_size;
> -- 
> 2.54.0
> 

Re: [PATCH v8 6/7] xen/serial: harden serial_tx_buffer checks

[Stefano Stabellini]

On Mon, 27 Jul 2026, dmukhin@ford.com wrote:
> From: Denis Mukhin <dmukhin@ford.com> 
> 
> Ensure the user-defined value never crosses 2GB boundary and always
> rounded to the next power of 2 to align logic with console driver
> conring buffer management code.
> 
> Signed-off-by: Denis Mukhin <dmukhin@ford.com>
> ---
> Changes since v7:
> - addressed Jan's feedback:
>   https://lore.kernel.org/xen-devel/89029dbd-df1f-45d4-8a02-720cd6a42cab@suse.com/
> - kept only check for large buffer in serial_async_transmit()
>   and a doc update.
> ---
>  docs/misc/xen-command-line.pandoc | 2 ++
>  xen/drivers/char/serial.c         | 2 ++
>  2 files changed, 4 insertions(+)
> 
> diff --git a/docs/misc/xen-command-line.pandoc b/docs/misc/xen-command-line.pandoc
> index 1c711fa98086..2be8772b329a 100644
> --- a/docs/misc/xen-command-line.pandoc
> +++ b/docs/misc/xen-command-line.pandoc
> @@ -2396,6 +2396,8 @@ accidentally leaking secrets by releasing pages without proper sanitization.
>  
>  Set the serial transmit buffer size.
>  
> +The value provided will be rounded down to the nearest power of 2.
> +
>  ### serrors (ARM)
>  > `= diverse | panic`
>  
> diff --git a/xen/drivers/char/serial.c b/xen/drivers/char/serial.c
> index cf0abf1893e5..ba1647309ab8 100644
> --- a/xen/drivers/char/serial.c
> +++ b/xen/drivers/char/serial.c
> @@ -523,6 +523,8 @@ void __init serial_async_transmit(struct serial_port *port)
>          return;
>      if ( serial_txbufsz < PAGE_SIZE )
>          serial_txbufsz = PAGE_SIZE;
> +    if ( serial_txbufsz > GB(2) )
> +        serial_txbufsz = CONFIG_SERIAL_TX_BUFSIZE;
>      while ( serial_txbufsz & (serial_txbufsz - 1) )
>          serial_txbufsz &= serial_txbufsz - 1;

My understanding of this loop is that, given that serial_txbufsz is
unsigned int, it is already clamping it to 2GB max


>      port->txbuf = xvmalloc_array(char, serial_txbufsz);
> -- 
> 2.54.0
> 

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

[Stefano Stabellini]

On Mon, 27 Jul 2026, dmukhin@ford.com 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).
> 
> Update the documentation for 'conring_size=' command line option.
> 
> Resolves: https://gitlab.com/xen-project/xen/-/issues/185
> Signed-off-by: Denis Mukhin <dmukhin@ford.com>
> ---
> Changes since v7:
> - n/a
> ---
>  docs/misc/xen-command-line.pandoc |  8 ++++++--
>  xen/drivers/char/Kconfig          | 21 +++++++++++++++++++++
>  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 2be8772b329a..448c9bdb8254 100644
> --- a/docs/misc/xen-command-line.pandoc
> +++ b/docs/misc/xen-command-line.pandoc
> @@ -425,10 +425,14 @@ 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.
> +
> +The run-time console ring buffer size is the maximum of the build-time value
> +and the value specified by the `conring_size=` command-line option.
> +
>  ### 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..a40a9929132b 100644
> --- a/xen/drivers/char/Kconfig
> +++ b/xen/drivers/char/Kconfig
> @@ -95,6 +95,27 @@ config SERIAL_TX_BUFSIZE
>  
>  	  Default value is 32768 (32KiB).
>  
> +config CONRING_SHIFT
> +	int "Console ring buffer size (power of 2)"
> +	range 14 27

anything above 20 would fail to build on arm


> +	default 15

this is OK but is double than the previous default and would be nice to
keep a note about it in xen-command-line.pandoc


> +	help
> +	  Select the boot console ring buffer size as a power of 2.
> +
> +	  The run-time console ring buffer is the maximum of the build-time
> +	  value and the value specified by the `conring_size=` command-line
> +	  option.
> +
> +	  If `conring_size=` is not specified on the command line, the run-time
> +	  console ring buffer size is the maximum of this value and
> +	  `num_present_cpus() << (9 + xenlog_lower_thresh)`.
> +
> +	    27 => 128 MiB
> +	    26 =>  64 MiB
> +	    ...
> +	    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 a1b8e5f5b507..76367c1dd705 100644
> --- a/xen/drivers/char/console.c
> +++ b/xen/drivers/char/console.c
> @@ -340,12 +340,12 @@ static void cf_check do_dec_thresh(unsigned char key, bool unused)
>   * ********************************************************
>   */
>  
> -/* conring_size: allows a larger console ring than default (16kB). */
> +/* conring_size: override build-time CONFIG_CONRING_SHIFT setting. */
>  static unsigned int __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 *__ro_after_init conring = _conring;
>  static unsigned int __ro_after_init conring_size = _CONRING_SIZE;
> -- 
> 2.54.0
> 

Re: [PATCH v8 5/7] xen/console: use memcpy() in conring_puts()

[Stefano Stabellini]

On Mon, 27 Jul 2026, dmukhin@ford.com wrote:
> From: Denis Mukhin <dmukhin@ford.com> 
> 
> Make conring_puts() more efficient by using memcpy()'s, rather than
> copying the ring a byte at a time.
> 
> No functional change intended.
> 
> Signed-off-by: Denis Mukhin <dmukhin@ford.com>
> ---
> Changes since v7:
> - hardended len check in conring_puts()
> ---
>  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 09282a7a4f8e..a1b8e5f5b507 100644
> --- a/xen/drivers/char/console.c
> +++ b/xen/drivers/char/console.c
> @@ -361,12 +361,24 @@ static DECLARE_SOFTIRQ_TASKLET(conring_tasklet, conring_notify, NULL);
>  /* NB: Do not send conring VIRQs during panic. */
>  static bool conring_no_notify;
>  
> -static void conring_puts(const char *str, size_t len)
> +static void conring_puts(const char *str, unsigned int len)

Here, I think it would be better to keep it size_t


>  {
> +    unsigned int src = len;
> +
> +    /* There are no callers with strings longer than PAGE_SIZE. */
> +    BUG_ON(len > PAGE_SIZE);
>      ASSERT(rspin_is_locked(&console_lock));
>  
> -    while ( len-- )
> -        conring[CONRING_IDX_MASK(conringp++)] = *str++;
> +    while ( src < len )
> +    {
> +        unsigned int dst = CONRING_IDX_MASK(conringp + src);
> +        unsigned int n = min(conring_size - dst, len - src);
> +
> +        memcpy(&conring[dst], &str[src], n);
> +        src += n;
> +    }
> +
> +    conringp += len;
>  
>      if ( conringp - conringc > conring_size )
>          conringc = conringp - conring_size;
> -- 
> 2.54.0
> 

Re: [PATCH v8 5/7] xen/console: use memcpy() in conring_puts()

[Andrew Cooper]

On 28/07/2026 7:50 am, dmukhin@ford.com wrote:
> From: Denis Mukhin <dmukhin@ford.com> 
>
> Make conring_puts() more efficient by using memcpy()'s, rather than
> copying the ring a byte at a time.
>
> No functional change intended.
>
> Signed-off-by: Denis Mukhin <dmukhin@ford.com>
> ---
> Changes since v7:
> - hardended len check in conring_puts()
> ---
>  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 09282a7a4f8e..a1b8e5f5b507 100644
> --- a/xen/drivers/char/console.c
> +++ b/xen/drivers/char/console.c
> @@ -361,12 +361,24 @@ static DECLARE_SOFTIRQ_TASKLET(conring_tasklet, conring_notify, NULL);
>  /* NB: Do not send conring VIRQs during panic. */
>  static bool conring_no_notify;
>  
> -static void conring_puts(const char *str, size_t len)
> +static void conring_puts(const char *str, unsigned int len)

size_t is the only correct type to be using here.  Anything else is
buggy and a ...

>  {
> +    unsigned int src = len;
> +
> +    /* There are no callers with strings longer than PAGE_SIZE. */
> +    BUG_ON(len > PAGE_SIZE);

... bug waiting to happen.  Switching to ASSERT() ok either; it is fine
to pass more than a page here, and all this does is screw over some
future person who has a complicated debugging scenario.

I have 0 remaining patients for the avoidance of size_t.  I will nack
any further patches I see doing it, as well as any further advise I see
given from anyone in the community.


The rest of the patch is fine.  Please resubmit while keeping len as size_t.

~Andrew