Cpu0 must remain in cpupool0, otherwise some operations like moving cpus
between cpupools, cpu hotplug, destroying cpupools, shutdown of the host,
might not work in a sane way.

Signed-off-by: Luca Fancellu <luca.fancellu@arm.com>
---
Changes in v7:
- new patch
---
 xen/common/sched/cpupool.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/xen/common/sched/cpupool.c b/xen/common/sched/cpupool.c
index 86a175f99cd5..0a93bcc631bf 100644
--- a/xen/common/sched/cpupool.c
+++ b/xen/common/sched/cpupool.c
@@ -572,6 +572,7 @@ static long cf_check cpupool_unassign_cpu_helper(void *info)
  * possible failures:
  * - last cpu and still active domains in cpupool
  * - cpu just being unplugged
+ * - Attempt to remove boot cpu from cpupool0
  */
 static int cpupool_unassign_cpu(struct cpupool *c, unsigned int cpu)
 {
@@ -582,7 +583,12 @@ static int cpupool_unassign_cpu(struct cpupool *c, unsigned int cpu)
     debugtrace_printk("cpupool_unassign_cpu(pool=%u,cpu=%d)\n",
                       c->cpupool_id, cpu);
 
-    if ( !cpu_online(cpu) )
+    /*
+     * Cpu0 must remain in cpupool0, otherwise some operations like moving cpus
+     * between cpupools, cpu hotplug, destroying cpupools, shutdown of the host,
+     * might not work in a sane way.
+     */
+    if ( (!c->cpupool_id && !cpu) || !cpu_online(cpu) )
         return -EINVAL;
 
     master_cpu = sched_get_resource_cpu(cpu);
-- 
2.17.1

> On 11 Apr 2022, at 16:20, Luca Fancellu <Luca.Fancellu@arm.com> wrote:
> 
> Cpu0 must remain in cpupool0, otherwise some operations like moving cpus
> between cpupools, cpu hotplug, destroying cpupools, shutdown of the host,
> might not work in a sane way.
> 
> Signed-off-by: Luca Fancellu <luca.fancellu@arm.com>
> ---
> Changes in v7:
> - new patch
> ---
> xen/common/sched/cpupool.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/common/sched/cpupool.c b/xen/common/sched/cpupool.c
> index 86a175f99cd5..0a93bcc631bf 100644
> --- a/xen/common/sched/cpupool.c
> +++ b/xen/common/sched/cpupool.c
> @@ -572,6 +572,7 @@ static long cf_check cpupool_unassign_cpu_helper(void *info)
>  * possible failures:
>  * - last cpu and still active domains in cpupool
>  * - cpu just being unplugged
> + * - Attempt to remove boot cpu from cpupool0
>  */
> static int cpupool_unassign_cpu(struct cpupool *c, unsigned int cpu)
> {
> @@ -582,7 +583,12 @@ static int cpupool_unassign_cpu(struct cpupool *c, unsigned int cpu)
>     debugtrace_printk("cpupool_unassign_cpu(pool=%u,cpu=%d)\n",
>                       c->cpupool_id, cpu);
> 
> -    if ( !cpu_online(cpu) )
> +    /*
> +     * Cpu0 must remain in cpupool0, otherwise some operations like moving cpus
> +     * between cpupools, cpu hotplug, destroying cpupools, shutdown of the host,
> +     * might not work in a sane way.
> +     */
> +    if ( (!c->cpupool_id && !cpu) || !cpu_online(cpu) )
>         return -EINVAL;
> 
>     master_cpu = sched_get_resource_cpu(cpu);
> -- 
> 2.17.1
> 

Hi,

I’m going to address the comment on this serie before re-pushing it, I see there
are no comments on this patch, so I’m wondering, when you have time, if you
can give me some feedback on this one.

Cheers,
Luca

On 20.04.22 16:57, Luca Fancellu wrote:
> 
> 
>> On 11 Apr 2022, at 16:20, Luca Fancellu <Luca.Fancellu@arm.com> wrote:
>>
>> Cpu0 must remain in cpupool0, otherwise some operations like moving cpus
>> between cpupools, cpu hotplug, destroying cpupools, shutdown of the host,
>> might not work in a sane way.
>>
>> Signed-off-by: Luca Fancellu <luca.fancellu@arm.com>
>> ---
>> Changes in v7:
>> - new patch
>> ---
>> xen/common/sched/cpupool.c | 8 +++++++-
>> 1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/xen/common/sched/cpupool.c b/xen/common/sched/cpupool.c
>> index 86a175f99cd5..0a93bcc631bf 100644
>> --- a/xen/common/sched/cpupool.c
>> +++ b/xen/common/sched/cpupool.c
>> @@ -572,6 +572,7 @@ static long cf_check cpupool_unassign_cpu_helper(void *info)
>>   * possible failures:
>>   * - last cpu and still active domains in cpupool
>>   * - cpu just being unplugged
>> + * - Attempt to remove boot cpu from cpupool0
>>   */
>> static int cpupool_unassign_cpu(struct cpupool *c, unsigned int cpu)
>> {
>> @@ -582,7 +583,12 @@ static int cpupool_unassign_cpu(struct cpupool *c, unsigned int cpu)
>>      debugtrace_printk("cpupool_unassign_cpu(pool=%u,cpu=%d)\n",
>>                        c->cpupool_id, cpu);
>>
>> -    if ( !cpu_online(cpu) )
>> +    /*
>> +     * Cpu0 must remain in cpupool0, otherwise some operations like moving cpus
>> +     * between cpupools, cpu hotplug, destroying cpupools, shutdown of the host,
>> +     * might not work in a sane way.
>> +     */
>> +    if ( (!c->cpupool_id && !cpu) || !cpu_online(cpu) )
>>          return -EINVAL;
>>
>>      master_cpu = sched_get_resource_cpu(cpu);
>> -- 
>> 2.17.1
>>
> 
> Hi,
> 
> I’m going to address the comment on this serie before re-pushing it, I see there
> are no comments on this patch, so I’m wondering, when you have time, if you
> can give me some feedback on this one.

Sorry, I seem to have missed this one.

Reviewed-by: Juergen Gross <jgross@suse.com>


Juergen