Re: [RFC 02/10] is_system_domain: replace open-coded instances

[Andrew Cooper <amc96@xxxxxxxx>]

On 17/12/2021 23:34, Daniel P. Smith wrote:
> From: Christopher Clark <christopher.w.clark@xxxxxxxxx>
>
> There were several instances of open-coded domid range checking. This commit
> replaces those with the is_system_domain inline function.
>
> Signed-off-by: Christopher Clark <christopher.w.clark@xxxxxxxxx>
> Signed-off-by: Daniel P. Smith <dpsmith@xxxxxxxxxxxxxxxxxxxx>

Ah - probably my fault.  When I added is_system_domain(), I didn't think
to scan for other opencodes - I was guts deep in the domain creation logic.

In addition to the ones you've got here...

xen/arch/x86/cpu/mcheck/mce.c:1521
xen/common/domain.c:586
common/domctl.c:55, 411 and 421

according to `git grep DOMID_FIRST_RESERVED`

> diff --git a/xen/arch/x86/cpu/vpmu.c b/xen/arch/x86/cpu/vpmu.c
> index 8ec4547bed..179f3dcc5a 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)) )

Can drop one set of brackets now.

> diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
> index 28146ee404..1df09bcb77 100644
> --- a/xen/include/xen/sched.h
> +++ b/xen/include/xen/sched.h
> @@ -613,6 +613,11 @@ 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_domain_id(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;

is_system_domain() wants implementing in terms of is_system_domain_id().

That said, could I talk you into is_system_domid() as a better name?

This is all sufficiently trivial that I'm tempted to fix on commit if
you'd like.  This patch is cleanup that stands on its own merit, and
isn't tied to hyperlaunch specifically.

~Andrew