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

["Roger Pau Monné" <roger.pau@citrix.com>]

On Thu, Dec 05, 2024 at 08:41:56PM -0800, Denis Mukhin via B4 Relay wrote:
> From: Denis Mukhin <dmukhin@ford.com>
> 
> 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@ford.com>

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.