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

[Jürgen Groß <jgross@xxxxxxxx>]

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?

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>


Juergen

Attachment: OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature.asc
Description: OpenPGP digital signature