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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 10.08.2026 22:19, Stefano Stabellini wrote:
> On Mon, 27 Jul 2026, dmukhin@xxxxxxxx wrote:
>> --- 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?

I don't think it is. While the description of 3355c1a2a60c ("KEXEC: Allocate
crash structures in low memory") only mentions 32-bit, the same split can
occur on 64-bit. The one thing I'm not sure about is why / when dumping
would be limited to parts of memory only (both for the original 32-bit case
and for today's 64-bit only situation) - Andrew?

>> -    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

But opt_conring_size can't be 0 when making it here, can it? (See the
check early in the function, even visible in context above, plus the
use of max() in the hunk here.)

Jan