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

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Thu, Dec 05, 2024 at 08:41:56PM -0800, Denis Mukhin via B4 Relay wrote:
> From: Denis Mukhin <dmukhin@xxxxxxxx>
> 
> Add new CONRING_LOG_SHIFT Kconfig parameter to specify the boot console buffer
> size as a power of 2.
> 
> Bump default size to 32 KiB.
> 
> Link: https://gitlab.com/xen-project/xen/-/issues/185
> Signed-off-by: Denis Mukhin <dmukhin@xxxxxxxx>

Thanks for taking care of this.

> ---
>  xen/drivers/char/Kconfig   | 23 +++++++++++++++++++++++
>  xen/drivers/char/console.c |  4 ++--
>  2 files changed, 25 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/drivers/char/Kconfig b/xen/drivers/char/Kconfig
> index 
> e6e12bb4139717f9319031f51f5d20155d2caee2..3bc892fc38d8cdeb3c76ea44d747f712a8d0d372
>  100644
> --- a/xen/drivers/char/Kconfig
> +++ b/xen/drivers/char/Kconfig
> @@ -96,6 +96,29 @@ config SERIAL_TX_BUFSIZE
>  
>         Default value is 32768 (32KiB).
>  
> +config CONRING_LOG_SHIFT
> +     int "Console buffer size"
> +     range 14 25
> +     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 enforced via 'conring_size=' boot parameter.
> +
> +       Examples:
> +         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

It might be better to do something similar to what we do in
SERIAL_TX_BUFSIZE, that the user provides a value in KiB which is
rounded down to the nearest power of 2?

> +
>  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 
> d22fb4a253af26f9b51d91bd408e1dbf4bb5a7c1..581ee22b85302a54db5b9d5d28e8b2d689d31403
>  100644
> --- a/xen/drivers/char/console.c
> +++ b/xen/drivers/char/console.c
> @@ -104,11 +104,11 @@ 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: allows a large console ring than default (32 KiB). */
>  static uint32_t __initdata opt_conring_size;
>  size_param("conring_size", opt_conring_size);

You also need to update xen-command-line.pandoc to mention the default
size is now set in Kconfig.  And here I would mention
CONFIG_CONRING_SIZE rather than explicit 32 KiB, because that's just
the default in Kconfig, but might not be the default in the build
itself.

FWIW, you could define:

#define _CONRING_SIZE        (CONFIG_CONRING_SIZE & (CONFIG_CONRING_SIZE - 1))

Thanks, Roger.