Re: [PATCH v8 3/7] xen/console: switch conring runtime allocation to xvmalloc

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Mon, 27 Jul 2026, dmukhin@xxxxxxxx wrote:
> From: Denis Mukhin <dmukhin@xxxxxxxx> 
> 
> The console ring only needs to be virtually contiguous; it does not need
> a naturally aligned or physically contiguous allocation. Replace the
> runtime xenheap allocation in console_init_ring() with an xvmalloc-backed
> buffer.
> 
> Also clamp the user-configured ring size to the supported range and emit
> warning when the requested size is adjusted.
> 
> Drop full stops in all diagnostic messages in console_init_ring() to align
> code with the common code pattern.
> 
> Suggested-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> Signed-off-by: Denis Mukhin <dmukhin@xxxxxxxx>

There is another alloc_xenheap_pages in the same file, in conring_flush.
It would probably need to be changed as well.

> ---
> Changes since v7:
> - Jan's feedback from
>   
> https://lore.kernel.org/xen-devel/0fefa50c-46aa-4ede-a8e2-8c2c619bc2ab@xxxxxxxx/
> ---
>  xen/drivers/char/console.c | 27 +++++++++++++++++++--------
>  1 file changed, 19 insertions(+), 8 deletions(-)
> 
> diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
> index 40355c1d14d6..09282a7a4f8e 100644
> --- a/xen/drivers/char/console.c
> +++ b/xen/drivers/char/console.c
> @@ -33,6 +33,7 @@
>  #include <asm/setup.h>
>  #include <xen/sections.h>
>  #include <xen/consoled.h>
> +#include <xen/xvmalloc.h>
>  
>  #ifdef CONFIG_X86
>  #include <asm/guest.h>
> @@ -464,20 +465,30 @@ void __init console_init_ring(void)
>  {
>      char *ring;
>      unsigned int done, size, n;
> -    unsigned int order, memflags;
>      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);

The original code had MEMF_bits(crashinfo_maxaddr_bits).
crashinfo_maxaddr_bits is 64-bit by default but can be changed via
command line options. Now, the memflags is going away and there is no
way to bring it back because xvmalloc_array doesn't take memflags as a
parameter.

Andrew, Jan, is that OK?


> -    while ( (ring = alloc_xenheap_pages(order, memflags)) == NULL )
> +    if ( opt_conring_size < GB(2) )
>      {
> -        BUG_ON(order == 0);
> -        order--;
> +        unsigned int order = get_order_from_bytes(max(opt_conring_size,
> +                                                      conring_size));
> +
> +        opt_conring_size = PAGE_SIZE << order;
> +    }
> +    else
> +    {
> +        printk(XENLOG_WARNING
> +               "Limiting user-configured console ring size to 2 GiB\n");
> +        opt_conring_size = GB(2);
> +    }
> +
> +    while ( (ring = xvmalloc_array(char, opt_conring_size)) == NULL )

It looks like that if opt_conring_size is zero, then xvmalloc_array
would return ZERO_BLOCK_PTR which is != NULL. We need to have a
different check here for that condition


> +    {
> +        BUG_ON(opt_conring_size == 0);
> +        opt_conring_size >>= 1;
>      }
> -    opt_conring_size = PAGE_SIZE << order;
>  
>      nrspin_lock_irqsave(&console_lock, flags);
>  
> @@ -498,7 +509,7 @@ void __init console_init_ring(void)
>      conring_size = opt_conring_size;
>      nrspin_unlock_irqrestore(&console_lock, flags);
>  
> -    printk("Allocated console ring of %u KiB.\n", opt_conring_size >> 10);
> +    printk("Allocated console ring of %u KiB\n", opt_conring_size >> 10);
>  }
>  
>  /*
> -- 
> 2.54.0
>