Re: [PATCH v5 5/6] xen/console: update conring memory allocation

[dmukhin@xxxxxxxx]

On Mon, Feb 09, 2026 at 06:02:52PM +0100, Jan Beulich wrote:
> On 05.02.2026 02:36, dmukhin@xxxxxxx wrote:
> > --- a/xen/drivers/char/console.c
> > +++ b/xen/drivers/char/console.c
> > @@ -463,20 +463,34 @@ static void cf_check conring_dump_keyhandler(unsigned 
> > char key)
> >  void __init console_init_ring(void)
> >  {
> >      char *ring;
> > -    unsigned int start, size, chunk, order, memflags;
> > +    unsigned int start, size, chunk;
> >      unsigned long flags;
> >  
> >      if ( !opt_conring_size )
> >          return;
> >  
> > -    order = get_order_from_bytes(max(opt_conring_size, conring_size));
> > -    memflags = MEMF_bits(crashinfo_maxaddr_bits);
> > -    while ( (ring = alloc_xenheap_pages(order, memflags)) == NULL )
> > +    opt_conring_size = max(opt_conring_size, conring_size);
> > +    size = ROUNDDOWN(opt_conring_size, PAGE_SIZE);
> > +    if ( size != opt_conring_size )
> >      {
> > -        BUG_ON(order == 0);
> > -        order--;
> > +        opt_conring_size = size;
> > +        printk(XENLOG_WARNING "Rounding down console ring size to multiple 
> > of %lu KiB.\n",
> > +               PAGE_SIZE >> 10);
> >      }
> > -    opt_conring_size = PAGE_SIZE << order;
> > +    if ( opt_conring_size >= GB(2) )
> > +    {
> > +        opt_conring_size = GB(2);
> > +        printk(XENLOG_WARNING "Limiting user-configured console ring 
> > size.\n");
> > +    }
> > +    else if ( opt_conring_size < _CONRING_SIZE )
> > +    {
> > +        opt_conring_size = _CONRING_SIZE;
> > +        printk(XENLOG_WARNING "Using compile-time console ring size.\n");
> > +    }
> > +
> > +    /* Contiguous buffer; does not need to be naturally aligned. */
> > +    ring = xmalloc_bytes(opt_conring_size);
> 
> I'm sorry, but I'm going to veto any new uses of xmalloc_bytes(). As per the
> comment at the top of xvmalloc.h, the family of functions there should be used
> in new code. That family deliberately doesn't include a counterpart of
> xmalloc_bytes(). You're wanting a multiple of page size anyway, so perhaps it
> is warranted here to actually use vmalloc() directly.

Thanks for taking a look at the series!

Sorry it's been a while, I am getting back to the series only now :-/

Turns out that vmalloc() is not available on MPU systems (*-mpu jobs in CI):
CONFIG_HAS_VMAP is disabled.

I will switch to xvmalloc-backed allocation, if that's OK.

--
Denis