From: Christopher Clark <christopher.w.clark@gmail.com>

This is a split out of the hyperlaunch dom0 series.

There were several instances of open-coded domid range checking. This commit
replaces those with the is_system_domain or is_system_domid inline function.

Signed-off-by: Christopher Clark <christopher.w.clark@gmail.com>
Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
Acked-by: Dario Faggioli <dfaggioli@suse.com>
---
 xen/arch/x86/cpu/mcheck/mce.c | 2 +-
 xen/arch/x86/cpu/vpmu.c       | 2 +-
 xen/common/domain.c           | 2 +-
 xen/common/domctl.c           | 4 ++--
 xen/common/sched/core.c       | 4 ++--
 xen/include/xen/sched.h       | 7 ++++++-
 6 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/xen/arch/x86/cpu/mcheck/mce.c b/xen/arch/x86/cpu/mcheck/mce.c
index 7f433343bc..5c1df39075 100644
--- a/xen/arch/x86/cpu/mcheck/mce.c
+++ b/xen/arch/x86/cpu/mcheck/mce.c
@@ -1518,7 +1518,7 @@ long do_mca(XEN_GUEST_HANDLE_PARAM(xen_mc_t) u_xen_mc)
             d = rcu_lock_domain_by_any_id(mc_msrinject->mcinj_domid);
             if ( d == NULL )
             {
-                if ( mc_msrinject->mcinj_domid >= DOMID_FIRST_RESERVED )
+                if ( is_system_domid(mc_msrinject->mcinj_domid) )
                     return x86_mcerr("do_mca inject: incompatible flag "
                                      "MC_MSRINJ_F_GPADDR with domain %d",
                                      -EINVAL, domid);
diff --git a/xen/arch/x86/cpu/vpmu.c b/xen/arch/x86/cpu/vpmu.c
index 8ec4547bed..c6bfa5a00e 100644
--- a/xen/arch/x86/cpu/vpmu.c
+++ b/xen/arch/x86/cpu/vpmu.c
@@ -188,7 +188,7 @@ void vpmu_do_interrupt(struct cpu_user_regs *regs)
      * in XENPMU_MODE_ALL, for everyone.
      */
     if ( (vpmu_mode & XENPMU_MODE_ALL) ||
-         (sampled->domain->domain_id >= DOMID_FIRST_RESERVED) )
+         is_system_domain(sampled->domain) )
     {
         sampling = choose_hwdom_vcpu();
         if ( !sampling )
diff --git a/xen/common/domain.c b/xen/common/domain.c
index 093bb4403f..347cc073aa 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -583,7 +583,7 @@ struct domain *domain_create(domid_t domid,
     /* Sort out our idea of is_hardware_domain(). */
     if ( domid == 0 || domid == hardware_domid )
     {
-        if ( hardware_domid < 0 || hardware_domid >= DOMID_FIRST_RESERVED )
+        if ( hardware_domid < 0 || is_system_domid(hardware_domid) )
             panic("The value of hardware_dom must be a valid domain ID\n");
 
         old_hwdom = hardware_domain;
diff --git a/xen/common/domctl.c b/xen/common/domctl.c
index 879a2adcbe..a3ad1f62b6 100644
--- a/xen/common/domctl.c
+++ b/xen/common/domctl.c
@@ -52,7 +52,7 @@ static inline int is_free_domid(domid_t dom)
 {
     struct domain *d;
 
-    if ( dom >= DOMID_FIRST_RESERVED )
+    if ( is_system_domid(dom) )
         return 0;
 
     if ( (d = rcu_lock_domain_by_id(dom)) == NULL )
@@ -536,7 +536,7 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
         if ( !d )
         {
             ret = -EINVAL;
-            if ( op->domain >= DOMID_FIRST_RESERVED )
+            if ( is_system_domid(op->domain) )
                 break;
 
             rcu_read_lock(&domlist_read_lock);
diff --git a/xen/common/sched/core.c b/xen/common/sched/core.c
index 8f4b1ca10d..6ea8bcf62f 100644
--- a/xen/common/sched/core.c
+++ b/xen/common/sched/core.c
@@ -821,7 +821,7 @@ int sched_init_domain(struct domain *d, unsigned int poolid)
     int ret;
 
     ASSERT(d->cpupool == NULL);
-    ASSERT(d->domain_id < DOMID_FIRST_RESERVED);
+    ASSERT(!is_system_domain(d));
 
     if ( (ret = cpupool_add_domain(d, poolid)) )
         return ret;
@@ -845,7 +845,7 @@ int sched_init_domain(struct domain *d, unsigned int poolid)
 
 void sched_destroy_domain(struct domain *d)
 {
-    ASSERT(d->domain_id < DOMID_FIRST_RESERVED);
+    ASSERT(!is_system_domain(d));
 
     if ( d->cpupool )
     {
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index 28146ee404..0df72baf2e 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -613,9 +613,14 @@ extern struct vcpu *idle_vcpu[NR_CPUS];
 #define is_idle_domain(d) ((d)->domain_id == DOMID_IDLE)
 #define is_idle_vcpu(v)   (is_idle_domain((v)->domain))
 
+static inline bool is_system_domid(domid_t id)
+{
+    return (id >= DOMID_FIRST_RESERVED);
+}
+
 static inline bool is_system_domain(const struct domain *d)
 {
-    return d->domain_id >= DOMID_FIRST_RESERVED;
+    return is_system_domid(d->domain_id);
 }
 
 #define DOMAIN_DESTROYED (1u << 31) /* assumes atomic_t is >= 32 bits */
-- 
2.20.1

On 20.12.2021 17:28, Daniel P. Smith wrote:
> From: Christopher Clark <christopher.w.clark@gmail.com>
> 
> This is a split out of the hyperlaunch dom0 series.
> 
> There were several instances of open-coded domid range checking. This commit
> replaces those with the is_system_domain or is_system_domid inline function.
> 
> Signed-off-by: Christopher Clark <christopher.w.clark@gmail.com>
> Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
> Acked-by: Dario Faggioli <dfaggioli@suse.com>

While I'm not outright opposed, I'd still like to raise the question whether
we really want to intermix "is system domain" and "is in-range domain ID"
predicates. Personally I'd prefer the latter to remain open-coded range
checks.

> --- a/xen/include/xen/sched.h
> +++ b/xen/include/xen/sched.h
> @@ -613,9 +613,14 @@ extern struct vcpu *idle_vcpu[NR_CPUS];
>  #define is_idle_domain(d) ((d)->domain_id == DOMID_IDLE)
>  #define is_idle_vcpu(v)   (is_idle_domain((v)->domain))
>  
> +static inline bool is_system_domid(domid_t id)
> +{
> +    return (id >= DOMID_FIRST_RESERVED);

Nit: Generally we omit parentheses in cases like this, ...

> +}
> +
>  static inline bool is_system_domain(const struct domain *d)
>  {
> -    return d->domain_id >= DOMID_FIRST_RESERVED;

... just like was the case here.

Jan

On 12/21/21 4:20 AM, Jan Beulich wrote:
> On 20.12.2021 17:28, Daniel P. Smith wrote:
>> From: Christopher Clark <christopher.w.clark@gmail.com>
>>
>> This is a split out of the hyperlaunch dom0 series.
>>
>> There were several instances of open-coded domid range checking. This commit
>> replaces those with the is_system_domain or is_system_domid inline function.
>>
>> Signed-off-by: Christopher Clark <christopher.w.clark@gmail.com>
>> Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
>> Acked-by: Dario Faggioli <dfaggioli@suse.com>
> 
> While I'm not outright opposed, I'd still like to raise the question whether
> we really want to intermix "is system domain" and "is in-range domain ID"
> predicates. Personally I'd prefer the latter to remain open-coded range
> checks.
> 
>> --- a/xen/include/xen/sched.h
>> +++ b/xen/include/xen/sched.h
>> @@ -613,9 +613,14 @@ extern struct vcpu *idle_vcpu[NR_CPUS];
>>   #define is_idle_domain(d) ((d)->domain_id == DOMID_IDLE)
>>   #define is_idle_vcpu(v)   (is_idle_domain((v)->domain))
>>   
>> +static inline bool is_system_domid(domid_t id)
>> +{
>> +    return (id >= DOMID_FIRST_RESERVED);
> 
> Nit: Generally we omit parentheses in cases like this, ...

ack

>> +}
>> +
>>   static inline bool is_system_domain(const struct domain *d)
>>   {
>> -    return d->domain_id >= DOMID_FIRST_RESERVED;
> 
> ... just like was the case here.
> 
> Jan
>