> On 22 Mar 2022, at 14:01, Julien Grall <jul...@xen.org> wrote:
>
> Hi,
>
> On 22/03/2022 09:52, Luca Fancellu wrote:
>>>>>
>>>>> 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.
>>>
>>> What exactly did you test on Arm?
>>>
>> I have tested start/stop of some guest, moving cpus between cpupools,
>> create/destroy cpupools, shutdown of Dom0
>> [ from your last mail ]
>>>>>
>>>>> If dom0 must run on core0 and core0 is Little then you cannot build a
>>>>> system where dom0 is running on big cores.
>>>>> If the limitation is not there, you can build such a configuration
>>>>> without any dependency to the boot core type.
>>>> This might not be completely clear so let me rephrase:
>>>> In the current system:
>>>> - dom0 must run on cpupool-0
>>>
>>> I don't think we need this restriction. In fact, with this series it will
>>> become more a problem because the cpupool ID will based on how we parse the
>>> Device-Tree.
>>>
>>> So for dom0, we need to specify explicitely the cpupool to be used.
>>>
>>>> - cpupool-0 must contain the boot core
>>>> - consequence: dom0 must run on the boot core
>>>> If boot core is little, you cannot build as system where dom0 runs only on
>>>> the big cores.
>>>> Removing the second limitation (which is not required on arm) is making it
>>>> possible.
>>>
>>> IMHO removing the second restriction is a lot more risky than removing the
>>> first one.
>> I see your point, my concern about moving Dom0 on another cpupool, different
>> from cpupool0, is that we give the
>> opportunity to destroy the cpupool0 (we can’t let that happen), or remove
>> every cpu from cpupool0.
>
> From my understanding a cpupool can only be destroyed when there are no more
> CPUs in the pool. Given that cpu0 has to be in pool0 then this should prevent
> the pool to be destroyed.
>
> Now, it is quite possible that we don't have a check to prevent CPU0 to be
> removed from cpupool0. If so, then I would argue we should add the check
> otherwise it is pointless to prevent cpu0 to be initially added in another
> pool than pool0 but can be moved afterwards.
>
Hi Julien,
I’ve done a test on fvp, first finding is that cpu0 can be removed from Pool-0,
there is no check.
Afterwards I’ve created another pool and I’ve assigned a cpu to it, I’ve called
xl cpupool-destroy and the tool removes every cpu from the pool before
destroying.
Do you think the check that prevents CPU0 to be removed from Pool-0 should be
done in the tools or in Xen?
With this change it could be possible to protect cpu0:
diff --git a/xen/common/sched/cpupool.c b/xen/common/sched/cpupool.c
index a6da4970506a..703005839dd6 100644
--- a/xen/common/sched/cpupool.c
+++ b/xen/common/sched/cpupool.c
@@ -585,6 +585,12 @@ static int cpupool_unassign_cpu(struct cpupool *c,
unsigned int cpu)
if ( !cpu_online(cpu) )
return -EINVAL;
+ if ( !c->cpupool_id && !cpu )
+ {
+ debugtrace_printk("Cpu0 must be in pool with id 0.\n");
+ return -EINVAL;
+ }
+
master_cpu = sched_get_resource_cpu(cpu);
ret = cpupool_unassign_cpu_start(c, master_cpu);
if ( ret )
Cheers,
Luca
> Cheers,
>
> --
> Julien Grall
