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

[Denis Mukhin <dmkhn@xxxxxxxxx>]

On Monday, February 24th, 2025 at 11:23 PM, Jan Beulich <jbeulich@xxxxxxxx> 
wrote:

> 
> 
> On 25.02.2025 03:45, Denis Mukhin wrote:
> 
> > On Monday, February 24th, 2025 at 2:44 AM, Jan Beulich jbeulich@xxxxxxxx 
> > wrote:
> > 
> > > On 21.02.2025 21:52, Denis Mukhin wrote:
> > > 
> > > > On Tuesday, February 18th, 2025 at 8:05 AM, Jan Beulich 
> > > > jbeulich@xxxxxxxx wrote:
> > > > 
> > > > > On 12.02.2025 23:31, dmkhn@xxxxxxxxx wrote:
> > > > > 
> > > > > > --- a/xen/drivers/char/Kconfig
> > > > > > +++ b/xen/drivers/char/Kconfig
> > > > > > @@ -96,6 +96,18 @@ config SERIAL_TX_BUFSIZE
> > > > > > 
> > > > > > Default value is 32768 (32KiB).
> > > > > > 
> > > > > > +config CONRING_SIZE
> > > > > > + int "Console buffer size"
> > > > > > + default 32768
> > > > > > + range 16384 134217728
> > > > > > + help
> > > > > > + Select the boot console buffer size (in bytes).
> > > > > 
> > > > > Why in bytes when ...
> > > > > 
> > > > > > + Note, the value provided will be rounded down to the nearest 
> > > > > > power of 2.
> > > > > 
> > > > > ... this rounding is done anyway? Why have people type in complicated 
> > > > > numbers?
> > > > > A granularity of 1k would already be an improvement; yet better would 
> > > > > be if
> > > > > this was a power-of-two value altogether.
> > > > 
> > > > My understanding that the semantics of new CONFIG_CONRING_SIZE 
> > > > build-time setting
> > > > should be consistent with existing boot-time conring_size= behavior 
> > > > (string value
> > > > converted to number of bytes).
> > > > 
> > > > I can update both to round up to 1k boundary.
> > > > 
> > > > I also agree that having power of 2s for both (e.g. similar to Linux'es 
> > > > CONFIG_LOG_BUF_SHIFT)
> > > > will be the simplest (implementation) and non-ambigous.
> > > > I had it done earlier:
> > > > https://lore.kernel.org/xen-devel/20241205-vuart-ns8250-v1-26-e9aa923127eb@xxxxxxxx/
> > > 
> > > I'd prefer the power-of-2 approach, yet I could live with the Kb-based 
> > > one as
> > > was suggested by Roger.
> > 
> > Just to double check: I think it makes sense to switch both build-time and 
> > run-time
> > settings to use the same size calculation algorithm (e.g. Kb-based) to avoid
> > confusion during building hypervisor configuration.
> > 
> > Will that be OK to do such change?
> 
> 
> No, you can't change existing command line options like this, at least not
> without a very good reason. You'd break existing uses. Plus there is
> 
> size_param("conring_size", opt_conring_size);
> 
> already anyway, so the command line option can be used with Kb and other
> granularities without any adjustments to the code.

OK, thanks.

> 
> > > > Also, since there's a build-time configuration parameter along with the 
> > > > boot-time
> > > > configuration, perhaps it makes sense to retire boot-time setting in 
> > > > favor of
> > > > build-time setting?
> > > 
> > > Why would that be? Build-time settings can only ever be defaults. We don't
> > > know what people need in their configurations.
> > 
> > I was thinking about few reasons.
> > In embedded setup run-time settings are unlikely to change, it is mostly
> > built-time configuration.
> > On a server setup, bumping the size of console buffer morelikely means some
> > debugging, which to me means new xen binary can be re-generated and 
> > re-deployed.
> 
> 
> That depends on who's doing the debugging and who's giving the instructions.
> A developer telling a customer to increase the buffer size is certainly
> easier when it simply means adding/changing a command line option, for
> example.

Ack.

> 
> Jan