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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 05.03.2025 02:12, dmkhn@xxxxxxxxx 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