Re: [RFC PATCH 02/10] control domain: refactor is_control_domain

[Jan Beulich <jbeulich@suse.com>]

On 14.05.2021 22:54, Daniel P. Smith wrote:
> --- a/xen/common/domain.c
> +++ b/xen/common/domain.c
> @@ -556,6 +556,9 @@ struct domain *domain_create(domid_t domid,
>      /* Sort out our idea of is_control_domain(). */
>      d->is_privileged = is_priv;

With the change to is_control_domain() this is the last use of the
field, so your patch should replace it rather than adding yet
another one. (For layout reasons, "replace" doesn't necessarily
mean "in place").

> +    if (is_priv)

Nit: Please add the missing blanks here.

> --- a/xen/include/xen/sched.h
> +++ b/xen/include/xen/sched.h
> @@ -473,6 +473,8 @@ struct domain
>  #define XSM_HW_CTRL   (1U<<8)  /* Hardware Control: domain with physical hardware access and its allocation for domain usage */
>  #define XSM_HW_SUPER  (1U<<9)  /* Hardware Supervisor: domain that control allocated physical hardware */
>  #define XSM_XENSTORE  (1U<<31) /* Xenstore: domain that can do privileged operations on xenstore */
> +#define CLASSIC_DOM0_PRIVS (XSM_PLAT_CTRL | XSM_DOM_BUILD | XSM_DOM_SUPER | \
> +		XSM_DEV_EMUL | XSM_HW_CTRL | XSM_HW_SUPER | XSM_XENSTORE)

The latest at this point I'm inclined to request that these #define-s
don't all live in the middle of struct domain. When you move them
elsewhere, simply have ...

>      uint32_t         xsm_roles;

... a brief comment next to this point at XSM_* as the values applicable
here.

Jan