Re: [RFC 1/6] dom0: replace explict zero checks

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Tue, 1 Aug 2023, Daniel P. Smith wrote:
> A legacy concept is that the initial domain will have a domain id of zero. As 
> a
> result there are places where a check that a domain is the inital domain is
> determined by an explicit check that the domid is zero.
> 
> This commit seeks to abstract this check into a function call and replace all
> check locations with the function call.
> 
> Signed-off-by: Daniel P. Smith <dpsmith@xxxxxxxxxxxxxxxxxxxx>

Thanks for the patch, this is a good cleanup!


> ---
>  xen/common/domain.c         | 4 ++--
>  xen/common/sched/arinc653.c | 2 +-
>  xen/common/sched/core.c     | 4 ++--
>  xen/include/xen/sched.h     | 7 +++++++
>  4 files changed, 12 insertions(+), 5 deletions(-)
> 
> diff --git a/xen/common/domain.c b/xen/common/domain.c
> index 304aa04fa6..8fb3c052f5 100644
> --- a/xen/common/domain.c
> +++ b/xen/common/domain.c
> @@ -309,7 +309,7 @@ static int late_hwdom_init(struct domain *d)
>      struct domain *dom0;
>      int rv;
>  
> -    if ( d != hardware_domain || d->domain_id == 0 )
> +    if ( d != hardware_domain || is_initial_domain(d) )
>          return 0;
>  
>      rv = xsm_init_hardware_domain(XSM_HOOK, d);
> @@ -612,7 +612,7 @@ struct domain *domain_create(domid_t domid,
>      d->is_privileged = flags & CDF_privileged;
>  
>      /* Sort out our idea of is_hardware_domain(). */
> -    if ( domid == 0 || domid == hardware_domid )
> +    if ( is_initial_domain(d) || domid == hardware_domid )
>      {
>          if ( hardware_domid < 0 || hardware_domid >= DOMID_FIRST_RESERVED )
>              panic("The value of hardware_dom must be a valid domain ID\n");
> diff --git a/xen/common/sched/arinc653.c b/xen/common/sched/arinc653.c
> index a82c0d7314..31e8270af3 100644
> --- a/xen/common/sched/arinc653.c
> +++ b/xen/common/sched/arinc653.c
> @@ -404,7 +404,7 @@ a653sched_alloc_udata(const struct scheduler *ops, struct 
> sched_unit *unit,
>       * Add every one of dom0's units to the schedule, as long as there are
>       * slots available.
>       */
> -    if ( unit->domain->domain_id == 0 )
> +    if ( is_initial_domain(unit->domain) )
>      {
>          entry = sched_priv->num_schedule_entries;
>  
> diff --git a/xen/common/sched/core.c b/xen/common/sched/core.c
> index 022f548652..210ad30f94 100644
> --- a/xen/common/sched/core.c
> +++ b/xen/common/sched/core.c
> @@ -585,7 +585,7 @@ int sched_init_vcpu(struct vcpu *v)
>           */
>          sched_set_affinity(unit, cpumask_of(0), cpumask_of(0));
>      }
> -    else if ( d->domain_id == 0 && opt_dom0_vcpus_pin )
> +    else if ( is_initial_domain(d) && opt_dom0_vcpus_pin )
>      {
>          /*
>           * If dom0_vcpus_pin is specified, dom0 vCPUs are pinned 1:1 to
> @@ -594,7 +594,7 @@ int sched_init_vcpu(struct vcpu *v)
>          sched_set_affinity(unit, cpumask_of(processor), &cpumask_all);
>      }
>  #ifdef CONFIG_X86
> -    else if ( d->domain_id == 0 )
> +    else if ( is_initial_domain(d) )
>      {
>          /*
>           * In absence of dom0_vcpus_pin instead, the hard and soft affinity 
> of
> diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
> index 854f3e32c0..a9276a7bed 100644
> --- a/xen/include/xen/sched.h
> +++ b/xen/include/xen/sched.h
> @@ -1058,6 +1058,13 @@ void scheduler_disable(void);
>  void watchdog_domain_init(struct domain *d);
>  void watchdog_domain_destroy(struct domain *d);
>  
> +static always_inline bool is_initial_domain(const struct domain *d)

I know you used always_inline only because is_hardware_domain just below
also uses it, but I wonder if we need it.

Also, Robero, it looks like always_inline is missing from
docs/misra/C-language-toolchain.rst? Or do we plan to get rid of it?


> +{
> +    static int init_domain_id = 0;

I take it is done this way because you plan to make it configurable?


> +    return d->domain_id == init_domain_id;
> +}
> +
>  /*
>   * Use this check when the following are both true:
>   *  - Using this feature or interface requires full access to the hardware
> -- 
> 2.20.1
>