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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 03.04.2026 10:47, Stewart Hildebrand wrote:
> On 4/1/26 09:03, Jan Beulich wrote:
>> 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@xxxxxxxx>
>>>> ---
>>>> 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.)
> 
> This is a known limitation. In a dom0less/hyperlaunch scenario, as future 
> work,
> I would like to see the ability to configure the ARINC653 schedule in device
> tree, which would likely extend the existing boot time cpu pool work.
> 
>>> 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@xxxxxxxx>
>>
>> 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.
> 
> It may not be desirable to schedule domUs until the control domain has had a 
> say
> in the matter, considering that the default schedule is unlikely to contain 
> the
> desired minor frame runtimes. It's less clear whether to include hardware and
> xenstore domains in the default schedule, though I'm leaning toward only
> including the domain with ability to invoke XEN_SYSCTL_SCHEDOP_putinfo for now
> (i.e. the control domain).
> 
> Hm, the suggested 'system_state < SYS_STATE_active' check is possibly a good
> addition. This reinforces that the default schedule's purpose is merely to 
> get a
> system booting until a user-provided schedule can be installed. Without this
> check, restarting the control domain could result in new entries being added
> while old entries remain, possibly ending up with duplicates and/or exhausting
> the schedule. However, the restarted domain would need to retain its uuid if 
> it
> expects to be scheduled after restart.
> 
> Lastly, we may consider restricting the default schedule to Pool-0, and
> eventually we may want a mechanism to disable the default schedule altogether
> (e.g. when boot time cpupools are in use), but I don't think it's necessary to
> conflate those with the current patch.

So what does all of this mean for the patch here? Should I switch to Jürgen's
suggestion? Should I merely add the system_state check, but otherwise keep as
is? Or should I not change anything?

Jan