* [PATCH v8 1/7] xen/console: do not use XENCONS_RING_IDX in console_init_ring()
2026-07-28 6:50 [PATCH v8 0/7] xen/console: some cleanups and configurable conring size dmukhin
@ 2026-07-28 6:50 ` dmukhin
2026-08-10 20:03 ` Stefano Stabellini
2026-07-28 6:50 ` [PATCH v8 2/7] xen/console: use 'unsigned int' in contring_{flush,puts}() dmukhin
` (5 subsequent siblings)
6 siblings, 1 reply; 17+ messages in thread
From: dmukhin @ 2026-07-28 6:50 UTC (permalink / raw)
To: xen-devel
Cc: andrew.cooper3, anthony.perard, jbeulich, julien, michal.orzel,
roger.pau, sstabellini, 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
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH v8 1/7] xen/console: do not use XENCONS_RING_IDX in console_init_ring()
2026-07-28 6:50 ` [PATCH v8 1/7] xen/console: do not use XENCONS_RING_IDX in console_init_ring() dmukhin
@ 2026-08-10 20:03 ` Stefano Stabellini
0 siblings, 0 replies; 17+ messages in thread
From: Stefano Stabellini @ 2026-08-10 20:03 UTC (permalink / raw)
To: dmukhin
Cc: xen-devel, andrew.cooper3, anthony.perard, jbeulich, julien,
michal.orzel, roger.pau, sstabellini
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
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v8 2/7] xen/console: use 'unsigned int' in contring_{flush,puts}()
2026-07-28 6:50 [PATCH v8 0/7] xen/console: some cleanups and configurable conring size dmukhin
2026-07-28 6:50 ` [PATCH v8 1/7] xen/console: do not use XENCONS_RING_IDX in console_init_ring() dmukhin
@ 2026-07-28 6:50 ` dmukhin
2026-08-10 20:05 ` Stefano Stabellini
2026-07-28 6:50 ` [PATCH v8 3/7] xen/console: switch conring runtime allocation to xvmalloc dmukhin
` (4 subsequent siblings)
6 siblings, 1 reply; 17+ messages in thread
From: dmukhin @ 2026-07-28 6:50 UTC (permalink / raw)
To: xen-devel
Cc: andrew.cooper3, anthony.perard, jbeulich, julien, michal.orzel,
roger.pau, sstabellini, 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
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH v8 2/7] xen/console: use 'unsigned int' in contring_{flush,puts}()
2026-07-28 6:50 ` [PATCH v8 2/7] xen/console: use 'unsigned int' in contring_{flush,puts}() dmukhin
@ 2026-08-10 20:05 ` Stefano Stabellini
0 siblings, 0 replies; 17+ messages in thread
From: Stefano Stabellini @ 2026-08-10 20:05 UTC (permalink / raw)
To: dmukhin
Cc: xen-devel, andrew.cooper3, anthony.perard, jbeulich, julien,
michal.orzel, roger.pau, sstabellini
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
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v8 3/7] xen/console: switch conring runtime allocation to xvmalloc
2026-07-28 6:50 [PATCH v8 0/7] xen/console: some cleanups and configurable conring size dmukhin
2026-07-28 6:50 ` [PATCH v8 1/7] xen/console: do not use XENCONS_RING_IDX in console_init_ring() dmukhin
2026-07-28 6:50 ` [PATCH v8 2/7] xen/console: use 'unsigned int' in contring_{flush,puts}() dmukhin
@ 2026-07-28 6:50 ` dmukhin
2026-08-10 20:19 ` Stefano Stabellini
2026-07-28 6:50 ` [PATCH v8 4/7] xen/serial: switch txbuf " dmukhin
` (3 subsequent siblings)
6 siblings, 1 reply; 17+ messages in thread
From: dmukhin @ 2026-07-28 6:50 UTC (permalink / raw)
To: xen-devel
Cc: andrew.cooper3, anthony.perard, jbeulich, julien, michal.orzel,
roger.pau, sstabellini, 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
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH v8 3/7] xen/console: switch conring runtime allocation to xvmalloc
2026-07-28 6:50 ` [PATCH v8 3/7] xen/console: switch conring runtime allocation to xvmalloc dmukhin
@ 2026-08-10 20:19 ` Stefano Stabellini
0 siblings, 0 replies; 17+ messages in thread
From: Stefano Stabellini @ 2026-08-10 20:19 UTC (permalink / raw)
To: dmukhin
Cc: xen-devel, andrew.cooper3, anthony.perard, jbeulich, julien,
michal.orzel, roger.pau, sstabellini
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
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v8 4/7] xen/serial: switch txbuf runtime allocation to xvmalloc
2026-07-28 6:50 [PATCH v8 0/7] xen/console: some cleanups and configurable conring size dmukhin
` (2 preceding siblings ...)
2026-07-28 6:50 ` [PATCH v8 3/7] xen/console: switch conring runtime allocation to xvmalloc dmukhin
@ 2026-07-28 6:50 ` dmukhin
2026-08-10 20:22 ` Stefano Stabellini
2026-07-28 6:50 ` [PATCH v8 5/7] xen/console: use memcpy() in conring_puts() dmukhin
` (2 subsequent siblings)
6 siblings, 1 reply; 17+ messages in thread
From: dmukhin @ 2026-07-28 6:50 UTC (permalink / raw)
To: xen-devel
Cc: andrew.cooper3, anthony.perard, jbeulich, julien, michal.orzel,
roger.pau, sstabellini, 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
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH v8 4/7] xen/serial: switch txbuf runtime allocation to xvmalloc
2026-07-28 6:50 ` [PATCH v8 4/7] xen/serial: switch txbuf " dmukhin
@ 2026-08-10 20:22 ` Stefano Stabellini
0 siblings, 0 replies; 17+ messages in thread
From: Stefano Stabellini @ 2026-08-10 20:22 UTC (permalink / raw)
To: dmukhin
Cc: xen-devel, andrew.cooper3, anthony.perard, jbeulich, julien,
michal.orzel, roger.pau, sstabellini
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
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v8 5/7] xen/console: use memcpy() in conring_puts()
2026-07-28 6:50 [PATCH v8 0/7] xen/console: some cleanups and configurable conring size dmukhin
` (3 preceding siblings ...)
2026-07-28 6:50 ` [PATCH v8 4/7] xen/serial: switch txbuf " dmukhin
@ 2026-07-28 6:50 ` dmukhin
2026-08-10 20:24 ` Stefano Stabellini
` (2 more replies)
2026-07-28 6:50 ` [PATCH v8 6/7] xen/serial: harden serial_tx_buffer checks dmukhin
2026-07-28 6:50 ` [PATCH v8 7/7] xen/console: make console buffer size configurable dmukhin
6 siblings, 3 replies; 17+ messages in thread
From: dmukhin @ 2026-07-28 6:50 UTC (permalink / raw)
To: xen-devel
Cc: andrew.cooper3, anthony.perard, jbeulich, julien, michal.orzel,
roger.pau, sstabellini, 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
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH v8 5/7] xen/console: use memcpy() in conring_puts()
2026-07-28 6:50 ` [PATCH v8 5/7] xen/console: use memcpy() in conring_puts() dmukhin
@ 2026-08-10 20:24 ` Stefano Stabellini
2026-08-10 21:36 ` Stefano Stabellini
2026-08-10 21:37 ` Andrew Cooper
2 siblings, 0 replies; 17+ messages in thread
From: Stefano Stabellini @ 2026-08-10 20:24 UTC (permalink / raw)
To: dmukhin
Cc: xen-devel, andrew.cooper3, anthony.perard, jbeulich, julien,
michal.orzel, roger.pau, sstabellini
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
>
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH v8 5/7] xen/console: use memcpy() in conring_puts()
2026-07-28 6:50 ` [PATCH v8 5/7] xen/console: use memcpy() in conring_puts() dmukhin
2026-08-10 20:24 ` Stefano Stabellini
@ 2026-08-10 21:36 ` Stefano Stabellini
2026-08-10 21:37 ` Andrew Cooper
2 siblings, 0 replies; 17+ messages in thread
From: Stefano Stabellini @ 2026-08-10 21:36 UTC (permalink / raw)
To: dmukhin
Cc: xen-devel, andrew.cooper3, anthony.perard, jbeulich, julien,
michal.orzel, roger.pau, sstabellini
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
>
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH v8 5/7] xen/console: use memcpy() in conring_puts()
2026-07-28 6:50 ` [PATCH v8 5/7] xen/console: use memcpy() in conring_puts() dmukhin
2026-08-10 20:24 ` Stefano Stabellini
2026-08-10 21:36 ` Stefano Stabellini
@ 2026-08-10 21:37 ` Andrew Cooper
2 siblings, 0 replies; 17+ messages in thread
From: Andrew Cooper @ 2026-08-10 21:37 UTC (permalink / raw)
To: dmukhin, xen-devel
Cc: Andrew Cooper, anthony.perard, jbeulich, julien, michal.orzel,
roger.pau, sstabellini
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
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v8 6/7] xen/serial: harden serial_tx_buffer checks
2026-07-28 6:50 [PATCH v8 0/7] xen/console: some cleanups and configurable conring size dmukhin
` (4 preceding siblings ...)
2026-07-28 6:50 ` [PATCH v8 5/7] xen/console: use memcpy() in conring_puts() dmukhin
@ 2026-07-28 6:50 ` dmukhin
2026-08-10 20:32 ` Stefano Stabellini
2026-07-28 6:50 ` [PATCH v8 7/7] xen/console: make console buffer size configurable dmukhin
6 siblings, 1 reply; 17+ messages in thread
From: dmukhin @ 2026-07-28 6:50 UTC (permalink / raw)
To: xen-devel
Cc: andrew.cooper3, anthony.perard, jbeulich, julien, michal.orzel,
roger.pau, sstabellini, 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
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH v8 6/7] xen/serial: harden serial_tx_buffer checks
2026-07-28 6:50 ` [PATCH v8 6/7] xen/serial: harden serial_tx_buffer checks dmukhin
@ 2026-08-10 20:32 ` Stefano Stabellini
0 siblings, 0 replies; 17+ messages in thread
From: Stefano Stabellini @ 2026-08-10 20:32 UTC (permalink / raw)
To: dmukhin
Cc: xen-devel, andrew.cooper3, anthony.perard, jbeulich, julien,
michal.orzel, roger.pau, sstabellini
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
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v8 7/7] xen/console: make console buffer size configurable
2026-07-28 6:50 [PATCH v8 0/7] xen/console: some cleanups and configurable conring size dmukhin
` (5 preceding siblings ...)
2026-07-28 6:50 ` [PATCH v8 6/7] xen/serial: harden serial_tx_buffer checks dmukhin
@ 2026-07-28 6:50 ` dmukhin
2026-08-10 20:42 ` Stefano Stabellini
6 siblings, 1 reply; 17+ messages in thread
From: dmukhin @ 2026-07-28 6:50 UTC (permalink / raw)
To: xen-devel
Cc: andrew.cooper3, anthony.perard, jbeulich, julien, michal.orzel,
roger.pau, sstabellini, 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
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH v8 7/7] xen/console: make console buffer size configurable
2026-07-28 6:50 ` [PATCH v8 7/7] xen/console: make console buffer size configurable dmukhin
@ 2026-08-10 20:42 ` Stefano Stabellini
0 siblings, 0 replies; 17+ messages in thread
From: Stefano Stabellini @ 2026-08-10 20:42 UTC (permalink / raw)
To: dmukhin
Cc: xen-devel, andrew.cooper3, anthony.perard, jbeulich, julien,
michal.orzel, roger.pau, sstabellini
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
>
^ permalink raw reply [flat|nested] 17+ messages in thread