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

dmkhn@proton.me posted 1 patch 4 days, 13 hours ago
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/20250305011127.4079670-1-dmukhin@ford.com
There is a newer version of this series
docs/misc/xen-command-line.pandoc |  5 ++++-
xen/drivers/char/Kconfig          | 27 +++++++++++++++++++++++++++
xen/drivers/char/console.c        |  6 +++---
3 files changed, 34 insertions(+), 4 deletions(-)
[PATCH v2] xen/console: make console buffer size configurable
Posted by dmkhn@proton.me 4 days, 13 hours ago
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).

Link: https://gitlab.com/xen-project/xen/-/issues/185
Signed-off-by: Denis Mukhin <dmukhin@ford.com>
---
Changes v1->v2:
- Switched to using powers of 2 in new Kconfig knob
---
 docs/misc/xen-command-line.pandoc |  5 ++++-
 xen/drivers/char/Kconfig          | 27 +++++++++++++++++++++++++++
 xen/drivers/char/console.c        |  6 +++---
 3 files changed, 34 insertions(+), 4 deletions(-)

diff --git a/docs/misc/xen-command-line.pandoc b/docs/misc/xen-command-line.pandoc
index 0c6225391d..1e12d2c6b5 100644
--- a/docs/misc/xen-command-line.pandoc
+++ b/docs/misc/xen-command-line.pandoc
@@ -425,10 +425,13 @@ The following are examples of correct specifications:
 ### conring_size
 > `= <size>`
 
-> Default: `conring_size=16k`
+> Default: `conring_size=32k`
 
 Specify the size of the console ring buffer.
 
+The console ring buffer size can be selected at build time via
+CONFIG_CONRING_SHIFT.
+
 ### console
 > `= List of [ vga | com1[H,L] | com2[H,L] | pv | dbgp | ehci | xhci | none ]`
 
diff --git a/xen/drivers/char/Kconfig b/xen/drivers/char/Kconfig
index e6e12bb413..d3ddb7d87a 100644
--- a/xen/drivers/char/Kconfig
+++ b/xen/drivers/char/Kconfig
@@ -96,6 +96,33 @@ config SERIAL_TX_BUFSIZE
 
 	  Default value is 32768 (32KiB).
 
+config CONRING_SHIFT
+	int "Console buffer size (power of 2)"
+	range 14 27
+	default 15
+	help
+	  Select the boot console buffer size as a power of 2.
+	  Run-time console buffer size is the same as the boot console size,
+	  unless overridden via 'conring_size=' boot parameter.
+
+	  Default value is 15 (32KiB).
+
+	  Examples:
+	    27 => 128 MiB
+	    26 =>  64 MiB
+	    25 =>  32 MiB
+	    24 =>  16 MiB
+	    23 =>   8 MiB
+	    22 =>   4 MiB
+	    21 =>   2 MiB
+	    20 =>   1 MiB
+	    19 => 512 KiB
+	    18 => 256 KiB
+	    17 => 128 KiB
+	    16 =>  64 KiB
+	    15 =>  32 KiB
+	    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 a84932e384..65468109ba 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -101,12 +101,12 @@ static int cf_check parse_console_timestamps(const char *s);
 custom_runtime_param("console_timestamps", parse_console_timestamps,
                      con_timestamp_mode_upd);
 
-/* conring_size: allows a large console ring than default (16kB). */
+/* conring_size: override build-time CONFIG_CONRING_SHIFT setting. */
 static uint32_t __initdata opt_conring_size;
 size_param("conring_size", opt_conring_size);
 
-#define _CONRING_SIZE 16384
-#define CONRING_IDX_MASK(i) ((i)&(conring_size-1))
+#define _CONRING_SIZE       (1UL << CONFIG_CONRING_SHIFT)
+#define CONRING_IDX_MASK(i) ((i) & (conring_size - 1))
 static char __initdata _conring[_CONRING_SIZE];
 static char *__read_mostly conring = _conring;
 static uint32_t __read_mostly conring_size = _CONRING_SIZE;
-- 
2.34.1
Re: [PATCH v2] xen/console: make console buffer size configurable
Posted by Jan Beulich 3 days, 21 hours ago
On 05.03.2025 02:12, dmkhn@proton.me wrote:
> --- a/docs/misc/xen-command-line.pandoc
> +++ b/docs/misc/xen-command-line.pandoc
> @@ -425,10 +425,13 @@ The following are examples of correct specifications:
>  ### conring_size
>  > `= <size>`
>  
> -> Default: `conring_size=16k`
> +> Default: `conring_size=32k`
>  
>  Specify the size of the console ring buffer.
>  
> +The console ring buffer size can be selected at build time via
> +CONFIG_CONRING_SHIFT.

And hence, if a value other than 32 is used there, the default above is
going to be wrong. IOW it is only the compile time default which is 32k.
The run-time default (which is what matters in Default: above) is itself
configurable, and hence wants documenting that way.

> --- a/xen/drivers/char/Kconfig
> +++ b/xen/drivers/char/Kconfig
> @@ -96,6 +96,33 @@ config SERIAL_TX_BUFSIZE
>  
>  	  Default value is 32768 (32KiB).
>  
> +config CONRING_SHIFT
> +	int "Console buffer size (power of 2)"

Would you mind adding the word "ring" here?

> +	range 14 27
> +	default 15
> +	help
> +	  Select the boot console buffer size as a power of 2.
> +	  Run-time console buffer size is the same as the boot console size,
> +	  unless overridden via 'conring_size=' boot parameter.
> +
> +	  Default value is 15 (32KiB).
> +
> +	  Examples:
> +	    27 => 128 MiB
> +	    26 =>  64 MiB
> +	    25 =>  32 MiB
> +	    24 =>  16 MiB
> +	    23 =>   8 MiB
> +	    22 =>   4 MiB
> +	    21 =>   2 MiB
> +	    20 =>   1 MiB
> +	    19 => 512 KiB
> +	    18 => 256 KiB
> +	    17 => 128 KiB
> +	    16 =>  64 KiB
> +	    15 =>  32 KiB
> +	    14 =>  16 KiB

I don't think an exhaustive list is necessary here. This way it's not really
a set of examples, but an enumeration of all possible values.

> --- a/xen/drivers/char/console.c
> +++ b/xen/drivers/char/console.c
> @@ -101,12 +101,12 @@ static int cf_check parse_console_timestamps(const char *s);
>  custom_runtime_param("console_timestamps", parse_console_timestamps,
>                       con_timestamp_mode_upd);
>  
> -/* conring_size: allows a large console ring than default (16kB). */
> +/* conring_size: override build-time CONFIG_CONRING_SHIFT setting. */
>  static uint32_t __initdata opt_conring_size;
>  size_param("conring_size", opt_conring_size);
>  
> -#define _CONRING_SIZE 16384
> -#define CONRING_IDX_MASK(i) ((i)&(conring_size-1))
> +#define _CONRING_SIZE       (1UL << CONFIG_CONRING_SHIFT)

Imo this ought to be 1U only, seeing ...

> +#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;

... this use of the constant.

Jan
Re: [PATCH v2] xen/console: make console buffer size configurable
Posted by Denis Mukhin 3 days, 6 hours ago
On Wednesday, March 5th, 2025 at 8:42 AM, Jan Beulich <jbeulich@suse.com> wrote:

> 
> 
> On 05.03.2025 02:12, dmkhn@proton.me wrote:
> 
> > --- a/docs/misc/xen-command-line.pandoc
> > +++ b/docs/misc/xen-command-line.pandoc
> > @@ -425,10 +425,13 @@ The following are examples of correct specifications:
> > ### conring_size
> > 
> > > `= <size>`
> > 
> > -> Default: `conring_size=16k`
> > +> Default: `conring_size=32k`
> > 
> > Specify the size of the console ring buffer.
> > 
> > +The console ring buffer size can be selected at build time via
> > +CONFIG_CONRING_SHIFT.
> 
> 
> And hence, if a value other than 32 is used there, the default above is
> going to be wrong. IOW it is only the compile time default which is 32k.
> The run-time default (which is what matters in Default: above) is itself
> configurable, and hence wants documenting that way.
> 
> > --- a/xen/drivers/char/Kconfig
> > +++ b/xen/drivers/char/Kconfig
> > @@ -96,6 +96,33 @@ config SERIAL_TX_BUFSIZE
> > 
> > Default value is 32768 (32KiB).
> > 
> > +config CONRING_SHIFT
> > + int "Console buffer size (power of 2)"
> 
> 
> Would you mind adding the word "ring" here?
> 
> > + range 14 27
> > + default 15
> > + help
> > + Select the boot console buffer size as a power of 2.
> > + Run-time console buffer size is the same as the boot console size,
> > + unless overridden via 'conring_size=' boot parameter.
> > +
> > + Default value is 15 (32KiB).
> > +
> > + Examples:
> > + 27 => 128 MiB
> > + 26 => 64 MiB
> > + 25 => 32 MiB
> > + 24 => 16 MiB
> > + 23 => 8 MiB
> > + 22 => 4 MiB
> > + 21 => 2 MiB
> > + 20 => 1 MiB
> > + 19 => 512 KiB
> > + 18 => 256 KiB
> > + 17 => 128 KiB
> > + 16 => 64 KiB
> > + 15 => 32 KiB
> > + 14 => 16 KiB
> 
> 
> I don't think an exhaustive list is necessary here. This way it's not really
> a set of examples, but an enumeration of all possible values.

I decided to keep the exhaustive list so it is a bit easier to do setting
selection.

> 
> > --- a/xen/drivers/char/console.c
> > +++ b/xen/drivers/char/console.c
> > @@ -101,12 +101,12 @@ static int cf_check parse_console_timestamps(const char *s);
> > custom_runtime_param("console_timestamps", parse_console_timestamps,
> > con_timestamp_mode_upd);
> > 
> > -/* conring_size: allows a large console ring than default (16kB). /
> > +/ conring_size: override build-time CONFIG_CONRING_SHIFT setting. */
> > static uint32_t __initdata opt_conring_size;
> > size_param("conring_size", opt_conring_size);
> > 
> > -#define _CONRING_SIZE 16384
> > -#define CONRING_IDX_MASK(i) ((i)&(conring_size-1))
> > +#define _CONRING_SIZE (1UL << CONFIG_CONRING_SHIFT)
> 
> 
> Imo this ought to be 1U only, seeing ...
> 
> > +#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;
> 
> 
> ... this use of the constant.
> 
> Jan