Mailing List Archive: [RFC PATCH 02/10] control domain: refactor is_control

Move to using the new Domain Control role as the backing to the
is_control_domain check.

Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
---
xen/common/domain.c | 3 +++
xen/include/xen/sched.h | 4 +++-
2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/xen/common/domain.c b/xen/common/domain.c
index cdda0d1f29..26bba8666d 100644
--- 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;

+ if (is_priv)
+ d->xsm_roles = CLASSIC_DOM0_PRIVS;
+
/* Sort out our idea of is_hardware_domain(). */
if ( domid == 0 || domid == hardware_domid )
{
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index 9b2c277ede..66b79d9c9f 100644
--- 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)
uint32_t xsm_roles;

/* Which guest this guest has privileges on */
@@ -1049,7 +1051,7 @@ static always_inline bool is_control_domain(const struct domain *d)
if ( IS_ENABLED(CONFIG_PV_SHIM_EXCLUSIVE) )
return false;

- return evaluate_nospec(d->is_privileged);
+ return evaluate_nospec(d->xsm_roles & XSM_DOM_SUPER);
}

#define VM_ASSIST(d, t) (test_bit(VMASST_TYPE_ ## t, &(d)->vm_assist))
--
2.20.1

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