[PATCH v3] arinc653: don't assume Dom0 is the control domain

Jan Beulich posted 1 patch 6 hours ago
Failed in applying to current master (apply log)
[PATCH v3] arinc653: don't assume Dom0 is the control domain
Posted by Jan Beulich 6 hours ago
Leaving aside highly disaggregated environments, the control domain is
what will invoke XEN_SYSCTL_SCHEDOP_putinfo. Its vCPU-s therefore need to
be able to run unconditionally, not those of the domain with ID 0 (which
may not exist at all).

Fixes: 9f0c658baedc ("arinc: add cpu-pool support to scheduler")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
There being no "else" to the if(), what about other control domain vCPU-s?
---
v3: Don't mistakenly include the idle domain.
v2: New.

--- a/xen/common/sched/arinc653.c
+++ b/xen/common/sched/arinc653.c
@@ -411,10 +411,10 @@ a653sched_alloc_udata(const struct sched
     spin_lock_irqsave(&sched_priv->lock, flags);
 
     /*
-     * Add every one of dom0's units to the schedule, as long as there are
-     * slots available.
+     * Add every one of the control domain's units to the schedule, as long as
+     * there are slots available.
      */
-    if ( unit->domain->domain_id == 0 )
+    if ( is_control_domain(unit->domain) && !is_idle_domain(unit->domain) )
     {
         entry = sched_priv->num_schedule_entries;
Re: [PATCH v3] arinc653: don't assume Dom0 is the control domain
Posted by Jürgen Groß 6 hours ago
On 01.04.26 14:29, Jan Beulich wrote:
> Leaving aside highly disaggregated environments, the control domain is
> what will invoke XEN_SYSCTL_SCHEDOP_putinfo. Its vCPU-s therefore need to
> be able to run unconditionally, not those of the domain with ID 0 (which
> may not exist at all).
> 
> Fixes: 9f0c658baedc ("arinc: add cpu-pool support to scheduler")
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> ---
> There being no "else" to the if(), what about other control domain vCPU-s?

I guess this is a stale leftover. Doesn't matter for committing anyway.

> ---
> v3: Don't mistakenly include the idle domain.
> v2: New.
> 
> --- a/xen/common/sched/arinc653.c
> +++ b/xen/common/sched/arinc653.c
> @@ -411,10 +411,10 @@ a653sched_alloc_udata(const struct sched
>       spin_lock_irqsave(&sched_priv->lock, flags);
>   
>       /*
> -     * Add every one of dom0's units to the schedule, as long as there are
> -     * slots available.
> +     * Add every one of the control domain's units to the schedule, as long as
> +     * there are slots available.
>        */
> -    if ( unit->domain->domain_id == 0 )
> +    if ( is_control_domain(unit->domain) && !is_idle_domain(unit->domain) )
>       {
>           entry = sched_priv->num_schedule_entries;
>   

Hmm, is it really the control domain only which wants to be scheduled initially?
I would think that at least the hardware domain and probably a Xenstore domain
would want to be included, too.

In the end it might even be that other domains created via dom0less would want
to be able to run initially. They could be part of a mandatory infrastructure.
Why would they need to be created at boot if they are NOT important?

The question is whether the arinc653 scheduler is really meant for such setups.
OTOH just modifying the test to:

     if ( system_state < SYS_STATE_active &&
          unit->domain->domain_id < DOMID_FIRST_RESERVED )

seems to be fine for catching all those cases.

With or without this modification:

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


Juergen
Re: [PATCH v3] arinc653: don't assume Dom0 is the control domain
Posted by Jan Beulich 6 hours ago
On 01.04.2026 14:57, Jürgen Groß wrote:
> On 01.04.26 14:29, Jan Beulich wrote:
>> Leaving aside highly disaggregated environments, the control domain is
>> what will invoke XEN_SYSCTL_SCHEDOP_putinfo. Its vCPU-s therefore need to
>> be able to run unconditionally, not those of the domain with ID 0 (which
>> may not exist at all).
>>
>> Fixes: 9f0c658baedc ("arinc: add cpu-pool support to scheduler")
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> ---
>> There being no "else" to the if(), what about other control domain vCPU-s?
> 
> I guess this is a stale leftover. Doesn't matter for committing anyway.
> 
>> ---
>> v3: Don't mistakenly include the idle domain.
>> v2: New.
>>
>> --- a/xen/common/sched/arinc653.c
>> +++ b/xen/common/sched/arinc653.c
>> @@ -411,10 +411,10 @@ a653sched_alloc_udata(const struct sched
>>       spin_lock_irqsave(&sched_priv->lock, flags);
>>   
>>       /*
>> -     * Add every one of dom0's units to the schedule, as long as there are
>> -     * slots available.
>> +     * Add every one of the control domain's units to the schedule, as long as
>> +     * there are slots available.
>>        */
>> -    if ( unit->domain->domain_id == 0 )
>> +    if ( is_control_domain(unit->domain) && !is_idle_domain(unit->domain) )
>>       {
>>           entry = sched_priv->num_schedule_entries;
>>   
> 
> Hmm, is it really the control domain only which wants to be scheduled initially?
> I would think that at least the hardware domain and probably a Xenstore domain
> would want to be included, too.
> 
> In the end it might even be that other domains created via dom0less would want
> to be able to run initially. They could be part of a mandatory infrastructure.
> Why would they need to be created at boot if they are NOT important?

This part is easy to answer: Because in a dom0less setup you simply may have
no toolstack at all. (At which point there may also be nothing to set a
schedule, yes.)

> The question is whether the arinc653 scheduler is really meant for such setups.
> OTOH just modifying the test to:
> 
>      if ( system_state < SYS_STATE_active &&
>           unit->domain->domain_id < DOMID_FIRST_RESERVED )
> 
> seems to be fine for catching all those cases.
> 
> With or without this modification:
> 
> Reviewed-by: Juergen Gross <jgross@suse.com>

Thanks, yet I'll have to leave to the maintainers to decide which form it
should ultimately take. One remark: A restartable control domain wouldn't
pass that conditional. Granted that's looking far into the future.

Jan