On Wednesday, March 5th, 2025 at 8:42 AM, Jan Beulich <jbeul...@suse.com> wrote:
>
>
> On 05.03.2025 02:12, dm...@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
