All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v6 0/4] xen/console: configurable conring size
@ 2026-05-09  0:57 dmukhin
  2026-05-09  0:57 ` [PATCH v6 1/4] xen/console: make console buffer size configurable dmukhin
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: dmukhin @ 2026-05-09  0:57 UTC (permalink / raw)
  To: xen-devel
  Cc: andrew.cooper3, anthony.perard, jbeulich, julien, michal.orzel,
	roger.pau, sstabellini, dmukhin

Series introduces compile-time configurable conring size along
with few cleanups in conring management.

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

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

Patch 3 optimizes switch from early conring to permanent conring.

Patch 4 update the conring buffer allocation code.

[1] Link to v6: https://lore.kernel.org/xen-devel/20260205013606.3384798-1-dmukhin@ford.com/
[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/2511597097

Denis Mukhin (4):
  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: switch conring runtime allocation to xvmalloc

 docs/misc/xen-command-line.pandoc |  5 +--
 xen/drivers/char/Kconfig          | 15 ++++++++
 xen/drivers/char/console.c        | 60 +++++++++++++++++++++++--------
 3 files changed, 63 insertions(+), 17 deletions(-)

-- 
2.54.0



^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v6 1/4] xen/console: make console buffer size configurable
  2026-05-09  0:57 [PATCH v6 0/4] xen/console: configurable conring size dmukhin
@ 2026-05-09  0:57 ` dmukhin
  2026-06-02  8:34   ` Roger Pau Monné
  2026-05-09  0:57 ` [PATCH v6 2/4] xen/console: promote conring{,_size} to __ro_after_init dmukhin
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 9+ messages in thread
From: dmukhin @ 2026-05-09  0:57 UTC (permalink / raw)
  To: xen-devel
  Cc: andrew.cooper3, anthony.perard, jbeulich, julien, michal.orzel,
	roger.pau, sstabellini, dmukhin, Jason Andryuk

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 v5:
- shortened the Kconfig option description
- kept Jason's R-b since the change is minimal - just dropping
  few lines from the Kconfig description
---
 docs/misc/xen-command-line.pandoc |  5 +++--
 xen/drivers/char/Kconfig          | 15 +++++++++++++++
 xen/drivers/char/console.c        |  6 +++---
 3 files changed, 21 insertions(+), 5 deletions(-)

diff --git a/docs/misc/xen-command-line.pandoc b/docs/misc/xen-command-line.pandoc
index 6c77129732bf..29393631d885 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..11f48415c12a 100644
--- a/xen/drivers/char/Kconfig
+++ b/xen/drivers/char/Kconfig
@@ -95,6 +95,21 @@ 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
+	    ...
+	    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 bcd6d261491b..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 larger 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.54.0



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v6 2/4] xen/console: promote conring{,_size} to __ro_after_init
  2026-05-09  0:57 [PATCH v6 0/4] xen/console: configurable conring size dmukhin
  2026-05-09  0:57 ` [PATCH v6 1/4] xen/console: make console buffer size configurable dmukhin
@ 2026-05-09  0:57 ` dmukhin
  2026-05-09  0:57 ` [PATCH v6 3/4] xen/console: use memcpy() in console_init_ring() dmukhin
  2026-05-09  0:57 ` [PATCH v6 4/4] xen/console: switch conring runtime allocation to xvmalloc dmukhin
  3 siblings, 0 replies; 9+ messages in thread
From: dmukhin @ 2026-05-09  0:57 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> 

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

Change the conring integer parameters type to `unsigned int` as required
by CODING_STYLE.

Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Denis Mukhin <dmukhin@ford.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
---
Changes since v5:
- used `unsigned int`
- added Jan's A-b
---
 xen/drivers/char/console.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index 522b2f489a53..5ab3b0de12d8 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -340,15 +340,15 @@ static void cf_check do_dec_thresh(unsigned char key, bool unused)
  */
 
 /* conring_size: override build-time CONFIG_CONRING_SHIFT setting. */
-static uint32_t __initdata opt_conring_size;
+static unsigned int __initdata opt_conring_size;
 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 uint32_t conringc, conringp;
+static char *__ro_after_init conring = _conring;
+static unsigned int __ro_after_init conring_size = _CONRING_SIZE;
+static unsigned int conringc, conringp;
 
 static void cf_check conring_notify(void *unused)
 {
-- 
2.54.0



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v6 3/4] xen/console: use memcpy() in console_init_ring()
  2026-05-09  0:57 [PATCH v6 0/4] xen/console: configurable conring size dmukhin
  2026-05-09  0:57 ` [PATCH v6 1/4] xen/console: make console buffer size configurable dmukhin
  2026-05-09  0:57 ` [PATCH v6 2/4] xen/console: promote conring{,_size} to __ro_after_init dmukhin
@ 2026-05-09  0:57 ` dmukhin
  2026-06-02  9:05   ` Roger Pau Monné
  2026-05-09  0:57 ` [PATCH v6 4/4] xen/console: switch conring runtime allocation to xvmalloc dmukhin
  3 siblings, 1 reply; 9+ messages in thread
From: dmukhin @ 2026-05-09  0:57 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 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 v5:
- fixed memcpy() logic
---
 xen/drivers/char/console.c | 21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index 5ab3b0de12d8..5cac87d052b9 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -463,7 +463,8 @@ static void cf_check conring_dump_keyhandler(unsigned char key)
 void __init console_init_ring(void)
 {
     char *ring;
-    unsigned int i, order, memflags;
+    XENCONS_RING_IDX i, size;
+    unsigned int order, memflags;
     unsigned long flags;
 
     if ( !opt_conring_size )
@@ -479,8 +480,22 @@ 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)];
+
+    i = 0;
+    size = conringp - conringc;
+    while ( i < size )
+    {
+        XENCONS_RING_IDX src = (conringc + i) & (conring_size - 1);
+        XENCONS_RING_IDX dst = (conringc + i) & (opt_conring_size - 1);
+        XENCONS_RING_IDX n;
+
+        n = min(opt_conring_size - dst, conring_size - src);
+        n = min(size - i, n);
+
+        memcpy(&ring[dst], &conring[src], n);
+        i += n;
+    }
+
     conring = ring;
     smp_wmb(); /* Allow users of console_force_unlock() to see larger buffer. */
     conring_size = opt_conring_size;
-- 
2.54.0



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v6 4/4] xen/console: switch conring runtime allocation to xvmalloc
  2026-05-09  0:57 [PATCH v6 0/4] xen/console: configurable conring size dmukhin
                   ` (2 preceding siblings ...)
  2026-05-09  0:57 ` [PATCH v6 3/4] xen/console: use memcpy() in console_init_ring() dmukhin
@ 2026-05-09  0:57 ` dmukhin
  2026-06-02  9:49   ` Roger Pau Monné
  3 siblings, 1 reply; 9+ messages in thread
From: dmukhin @ 2026-05-09  0:57 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
warnings when the requested size is adjusted.

Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Denis Mukhin <dmukhin@ford.com>
---
Changes since v5:
- switched to xvmalloc_array()
- fixed conring size checks
- corrected diagnostic messages
---
 xen/drivers/char/console.c | 27 +++++++++++++++++++++------
 1 file changed, 21 insertions(+), 6 deletions(-)

diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index 5cac87d052b9..29b9359468e7 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>
@@ -343,6 +344,7 @@ static void cf_check do_dec_thresh(unsigned char key, bool unused)
 static unsigned int __initdata opt_conring_size;
 size_param("conring_size", opt_conring_size);
 
+#define CONRING_SIZE_MIN    (1U << 14)
 #define _CONRING_SIZE       (1U << CONFIG_CONRING_SHIFT)
 #define CONRING_IDX_MASK(i) ((i) & (conring_size - 1))
 static char __initdata _conring[_CONRING_SIZE];
@@ -464,20 +466,33 @@ void __init console_init_ring(void)
 {
     char *ring;
     XENCONS_RING_IDX i, size;
-    unsigned int order, memflags;
+    unsigned int order;
     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 )
+    size = PAGE_SIZE << order;
+    if ( size != opt_conring_size )
     {
-        BUG_ON(order == 0);
-        order--;
+        opt_conring_size = size;
+        printk(XENLOG_WARNING "Normalizing console ring size.\n");
     }
-    opt_conring_size = PAGE_SIZE << order;
+    if ( opt_conring_size < CONRING_SIZE_MIN )
+    {
+        opt_conring_size = 0;
+        printk(XENLOG_WARNING "Ignoring too-small console ring size override.\n");
+        return;
+    }
+    else if ( opt_conring_size > GB(2) )
+    {
+        opt_conring_size = GB(2);
+        printk(XENLOG_WARNING "Limiting user-configured console ring size to 2 GiB.\n");
+    }
+
+    ring = xvmalloc_array(char, opt_conring_size);
+    BUG_ON(ring == NULL);
 
     nrspin_lock_irqsave(&console_lock, flags);
 
-- 
2.54.0



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v6 1/4] xen/console: make console buffer size configurable
  2026-05-09  0:57 ` [PATCH v6 1/4] xen/console: make console buffer size configurable dmukhin
@ 2026-06-02  8:34   ` Roger Pau Monné
  0 siblings, 0 replies; 9+ messages in thread
From: Roger Pau Monné @ 2026-06-02  8:34 UTC (permalink / raw)
  To: dmukhin
  Cc: xen-devel, andrew.cooper3, anthony.perard, jbeulich, julien,
	michal.orzel, sstabellini, Jason Andryuk

On Fri, May 08, 2026 at 05:57:11PM -0700, 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).
> 
> 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 v5:
> - shortened the Kconfig option description
> - kept Jason's R-b since the change is minimal - just dropping
>   few lines from the Kconfig description
> ---
>  docs/misc/xen-command-line.pandoc |  5 +++--
>  xen/drivers/char/Kconfig          | 15 +++++++++++++++
>  xen/drivers/char/console.c        |  6 +++---
>  3 files changed, 21 insertions(+), 5 deletions(-)
> 
> diff --git a/docs/misc/xen-command-line.pandoc b/docs/misc/xen-command-line.pandoc
> index 6c77129732bf..29393631d885 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.

Hm, since you are adjusting the text of the option anyway, the above
is not fully accurate.  The size of the console ring buffer will be
the maximum between the built-time value and the command line option.

>  
> +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..11f48415c12a 100644
> --- a/xen/drivers/char/Kconfig
> +++ b/xen/drivers/char/Kconfig
> @@ -95,6 +95,21 @@ config SERIAL_TX_BUFSIZE
>  
>  	  Default value is 32768 (32KiB).
>  
> +config CONRING_SHIFT

The option just above specifies the size in bytes, and then the code
rounds down to the nearest power of 2, but I don't think we can't do
the same here, due to how SERIAL_TX_BUFSIZE is used to size an array.

> +	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.

I don't think the above text is accurate, if `conring_size=` is not
specified on the command line the runtime console buffer size will be
the maximum between the build time value and `num_present_cpus() << (9
+ xenlog_lower_thresh)`.  See console_init_postirq().

Thanks, Roger.


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v6 3/4] xen/console: use memcpy() in console_init_ring()
  2026-05-09  0:57 ` [PATCH v6 3/4] xen/console: use memcpy() in console_init_ring() dmukhin
@ 2026-06-02  9:05   ` Roger Pau Monné
  0 siblings, 0 replies; 9+ messages in thread
From: Roger Pau Monné @ 2026-06-02  9:05 UTC (permalink / raw)
  To: dmukhin
  Cc: xen-devel, andrew.cooper3, anthony.perard, jbeulich, julien,
	michal.orzel, sstabellini

On Fri, May 08, 2026 at 05:57:13PM -0700, dmukhin@ford.com wrote:
> 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>

Acked-by: Roger Pau Monné <roger.pau@citrix.com>

A couple of notes below.

> ---
> Changes since v5:
> - fixed memcpy() logic
> ---
>  xen/drivers/char/console.c | 21 ++++++++++++++++++---
>  1 file changed, 18 insertions(+), 3 deletions(-)
> 
> diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
> index 5ab3b0de12d8..5cac87d052b9 100644
> --- a/xen/drivers/char/console.c
> +++ b/xen/drivers/char/console.c
> @@ -463,7 +463,8 @@ static void cf_check conring_dump_keyhandler(unsigned char key)
>  void __init console_init_ring(void)
>  {
>      char *ring;
> -    unsigned int i, order, memflags;
> +    XENCONS_RING_IDX i, size;
> +    unsigned int order, memflags;
>      unsigned long flags;
>  
>      if ( !opt_conring_size )
> @@ -479,8 +480,22 @@ 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)];
> +
> +    i = 0;
> +    size = conringp - conringc;
> +    while ( i < size )
> +    {
> +        XENCONS_RING_IDX src = (conringc + i) & (conring_size - 1);
> +        XENCONS_RING_IDX dst = (conringc + i) & (opt_conring_size - 1);
> +        XENCONS_RING_IDX n;
> +
> +        n = min(opt_conring_size - dst, conring_size - src);
> +        n = min(size - i, n);
> +
> +        memcpy(&ring[dst], &conring[src], n);
> +        i += n;
> +    }

It's a bit more complex logic for not much gain IMO: this will only be
used once to move from the init buffer to the dynamically allocated
one.  A couple of notes, feel free to ignore if you don't think it's
an improvement: I would rename `i` to `done`, as `i` is usually
associated with an index, and here it's just an accumulator of the
amount of characters processed.  I would also consider making this a
for loop, by pulling n to the outer context, ie:

XENCONS_RING_IDX done, size, n;
[...]

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);

    n = min(opt_conring_size - dst, conring_size - src);
    n = min(size - done, n);

    memcpy(&ring[dst], &conring[src], n);
}

Thanks, Roger.


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v6 4/4] xen/console: switch conring runtime allocation to xvmalloc
  2026-05-09  0:57 ` [PATCH v6 4/4] xen/console: switch conring runtime allocation to xvmalloc dmukhin
@ 2026-06-02  9:49   ` Roger Pau Monné
  2026-06-02  9:54     ` Jan Beulich
  0 siblings, 1 reply; 9+ messages in thread
From: Roger Pau Monné @ 2026-06-02  9:49 UTC (permalink / raw)
  To: dmukhin
  Cc: xen-devel, andrew.cooper3, anthony.perard, jbeulich, julien,
	michal.orzel, sstabellini

On Fri, May 08, 2026 at 05:57:14PM -0700, 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
> warnings when the requested size is adjusted.
> 
> Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Signed-off-by: Denis Mukhin <dmukhin@ford.com>
> ---
> Changes since v5:
> - switched to xvmalloc_array()
> - fixed conring size checks
> - corrected diagnostic messages
> ---
>  xen/drivers/char/console.c | 27 +++++++++++++++++++++------
>  1 file changed, 21 insertions(+), 6 deletions(-)
> 
> diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
> index 5cac87d052b9..29b9359468e7 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>
> @@ -343,6 +344,7 @@ static void cf_check do_dec_thresh(unsigned char key, bool unused)
>  static unsigned int __initdata opt_conring_size;
>  size_param("conring_size", opt_conring_size);
>  
> +#define CONRING_SIZE_MIN    (1U << 14)
>  #define _CONRING_SIZE       (1U << CONFIG_CONRING_SHIFT)
>  #define CONRING_IDX_MASK(i) ((i) & (conring_size - 1))
>  static char __initdata _conring[_CONRING_SIZE];
> @@ -464,20 +466,33 @@ void __init console_init_ring(void)
>  {
>      char *ring;
>      XENCONS_RING_IDX i, size;
> -    unsigned int order, memflags;
> +    unsigned int order;
>      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 )
> +    size = PAGE_SIZE << order;
> +    if ( size != opt_conring_size )
>      {
> -        BUG_ON(order == 0);
> -        order--;
> +        opt_conring_size = size;
> +        printk(XENLOG_WARNING "Normalizing console ring size.\n");

I think you want to also specify to what it has been normalized, ie:

"Normalizing command line console ring size %u to %u.\n", opt_conring_size, size

However, note how console_init_postirq() sets opt_conring_size if not
specified, and AFAICT that calculated value might not be a power of 2,
and it might be lower than the build time setting.  If you want to
print warning messages about the user-provided value (if any), you
need to split the checking from console_init_ring(), and do it
exclusively for the command line user-provided value, not for the
value generated by Xen in console_init_postirq().

Also, a user attempting to set a console buffer size below the
built-time value will get this "Normalizing size" message, when it
should otherwise get a message about the command line value being
smaller than the built time setting.

>      }
> -    opt_conring_size = PAGE_SIZE << order;
> +    if ( opt_conring_size < CONRING_SIZE_MIN )
> +    {
> +        opt_conring_size = 0;
> +        printk(XENLOG_WARNING "Ignoring too-small console ring size override.\n");
> +        return;
> +    }

I don't think this is possible, as the built time Kconfig value is
forced to be >= 14, and hence the order calculated above can never be
smaller than 14.

> +    else if ( opt_conring_size > GB(2) )
> +    {
> +        opt_conring_size = GB(2);
> +        printk(XENLOG_WARNING "Limiting user-configured console ring size to 2 GiB.\n");
> +    }
> +
> +    ring = xvmalloc_array(char, opt_conring_size);
> +    BUG_ON(ring == NULL);

Since you are already touching this, might I suggest to use a panic
rather than a BUG?

if ( !ring )
    panic("Unable to allocate console ring buffer of size %u\n",...);

Thanks, Roger.


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v6 4/4] xen/console: switch conring runtime allocation to xvmalloc
  2026-06-02  9:49   ` Roger Pau Monné
@ 2026-06-02  9:54     ` Jan Beulich
  0 siblings, 0 replies; 9+ messages in thread
From: Jan Beulich @ 2026-06-02  9:54 UTC (permalink / raw)
  To: Roger Pau Monné, dmukhin
  Cc: xen-devel, andrew.cooper3, anthony.perard, julien, michal.orzel,
	sstabellini

On 02.06.2026 11:49, Roger Pau Monné wrote:
> On Fri, May 08, 2026 at 05:57:14PM -0700, 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
>> warnings when the requested size is adjusted.
>>
>> Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com>
>> Signed-off-by: Denis Mukhin <dmukhin@ford.com>
>> ---
>> Changes since v5:
>> - switched to xvmalloc_array()
>> - fixed conring size checks
>> - corrected diagnostic messages
>> ---
>>  xen/drivers/char/console.c | 27 +++++++++++++++++++++------
>>  1 file changed, 21 insertions(+), 6 deletions(-)
>>
>> diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
>> index 5cac87d052b9..29b9359468e7 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>
>> @@ -343,6 +344,7 @@ static void cf_check do_dec_thresh(unsigned char key, bool unused)
>>  static unsigned int __initdata opt_conring_size;
>>  size_param("conring_size", opt_conring_size);
>>  
>> +#define CONRING_SIZE_MIN    (1U << 14)
>>  #define _CONRING_SIZE       (1U << CONFIG_CONRING_SHIFT)
>>  #define CONRING_IDX_MASK(i) ((i) & (conring_size - 1))
>>  static char __initdata _conring[_CONRING_SIZE];
>> @@ -464,20 +466,33 @@ void __init console_init_ring(void)
>>  {
>>      char *ring;
>>      XENCONS_RING_IDX i, size;
>> -    unsigned int order, memflags;
>> +    unsigned int order;
>>      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 )
>> +    size = PAGE_SIZE << order;
>> +    if ( size != opt_conring_size )
>>      {
>> -        BUG_ON(order == 0);
>> -        order--;
>> +        opt_conring_size = size;
>> +        printk(XENLOG_WARNING "Normalizing console ring size.\n");

Nit: Both here and ...

> I think you want to also specify to what it has been normalized, ie:
> 
> "Normalizing command line console ring size %u to %u.\n", opt_conring_size, size

... here: No full stop at the end of log messages please.

Jan


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2026-06-02  9:54 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-09  0:57 [PATCH v6 0/4] xen/console: configurable conring size dmukhin
2026-05-09  0:57 ` [PATCH v6 1/4] xen/console: make console buffer size configurable dmukhin
2026-06-02  8:34   ` Roger Pau Monné
2026-05-09  0:57 ` [PATCH v6 2/4] xen/console: promote conring{,_size} to __ro_after_init dmukhin
2026-05-09  0:57 ` [PATCH v6 3/4] xen/console: use memcpy() in console_init_ring() dmukhin
2026-06-02  9:05   ` Roger Pau Monné
2026-05-09  0:57 ` [PATCH v6 4/4] xen/console: switch conring runtime allocation to xvmalloc dmukhin
2026-06-02  9:49   ` Roger Pau Monné
2026-06-02  9:54     ` Jan Beulich

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.