| Message ID | 20211217233437.13791-3-dpsmith@apertussolutions.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Hyperlaunch x86 Dom0 launch | expand |
On 17/12/2021 23:34, Daniel P. Smith wrote: > From: Christopher Clark <christopher.w.clark@gmail.com> > > 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@gmail.com> > Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com> 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
On Fri, 2021-12-17 at 18:34 -0500, Daniel P. Smith wrote: > From: Christopher Clark <christopher.w.clark@gmail.com> > > 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@gmail.com> > Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com> > --- > xen/arch/x86/cpu/vpmu.c | 2 +- > xen/common/domctl.c | 2 +- > xen/common/sched/core.c | 4 ++-- > xen/include/xen/sched.h | 5 +++++ > The */sched* bits: Acked-by: Dario Faggioli <dfaggioli@suse.com> But with a strong preference for renaming is_system_domain_id() to is_system_domid(), as Andrew suggested. Regards
On 12/17/21 2:50 PM, Andrew Cooper wrote: > On 17/12/2021 23:34, Daniel P. Smith wrote: >> From: Christopher Clark <christopher.w.clark@gmail.com> >> >> 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@gmail.com> >> Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com> > > 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` confirmed and replaced >> 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. ack >> 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(). ack > That said, could I talk you into is_system_domid() as a better name? ack > 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. I will send the revised version later today as a standalone patch. v/r, dps
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)) ) { sampling = choose_hwdom_vcpu(); if ( !sampling ) diff --git a/xen/common/domctl.c b/xen/common/domctl.c index 879a2adcbe..67021cc54b 100644 --- a/xen/common/domctl.c +++ b/xen/common/domctl.c @@ -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_domain_id(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..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;