Re: [PATCH v3 4/6] xen/cpupool: Create different cpupools at boot time

[Luca Fancellu <Luca.Fancellu@xxxxxxx>]

> On 18 Mar 2022, at 16:12, Julien Grall <julien@xxxxxxx> wrote:
> 
> Hi Luca,
> 
> I only skimmed through the series. I have one question below:
> 
> On 18/03/2022 15:25, Luca Fancellu wrote:
>> +void __init btcpupools_allocate_pools(void)
>> +{
>> +    unsigned int i;
>> +    bool add_extra_cpupool = false;
>> +
>> +    /*
>> +     * If there are no cpupools, the value of next_pool_id is zero, so the 
>> code
>> +     * below will assign every cpu to cpupool0 as the default behavior.
>> +     * When there are cpupools, the code below is assigning all the not
>> +     * assigned cpu to a new pool (next_pool_id value is the last id + 1).
>> +     * In the same loop we check if there is any assigned cpu that is not
>> +     * online.
>> +     */
>> +    for ( i = 0; i < nr_cpu_ids; i++ )
>> +        if ( cpumask_test_cpu(i, &cpu_online_map) )
>> +        {
>> +            /* Unassigned cpu gets next_pool_id pool id value */
>> +            if ( pool_cpu_map[i] < 0 )
>> +            {
>> +                pool_cpu_map[i] = next_pool_id;
>> +                add_extra_cpupool = true;
>> +            }
>> +            printk(XENLOG_INFO "Logical CPU %u in Pool-%u.\n", i,
>> +                   pool_cpu_map[i]);
>> +        }
>> +        else
>> +        {
>> +            if ( pool_cpu_map[i] >= 0 )
>> +                panic("Pool-%d contains cpu%u that is not online!\n",
>> +                      pool_cpu_map[i], i);
>> +        }
>> +
>> +    if ( add_extra_cpupool )
>> +        next_pool_id++;
>> +
>> +    /* Create cpupools with selected schedulers */
>> +    for ( i = 0; i < next_pool_id; i++ )
>> +        cpupool_create_pool(i, pool_sched_map[i]);
>> +
>> +#ifdef CONFIG_X86
>> +    /* Cpu0 must be in cpupool0 for x86 */
>> +    if ( pool_cpu_map[0] != 0 )
>> +        panic("Cpu0 must be in Pool-0\n");
>> +#endif
> 
> Can you document why this is necessary on x86 but not on other architectures?

Hi Julien,

I received the warning by Juergen here: 
https://patchwork.kernel.org/comment/24740762/ that at least on x86 there could 
be
some problems if cpu0 is not in cpupool0, I tested it on arm and it was working 
fine and I didn’t find any restriction.

So I don’t know why on x86 we must have cpu0 in cpupool0, maybe x86 maintainer 
have more knowledge about that and
I can put a comment here.

Cheers,
Luca

> 
> Cheers,
> 
> -- 
> Julien Grall