| Message ID | 20250107101711.5980-5-jgross@suse.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | remove libxenctrl usage from xenstored | expand |
On 07.01.2025 11:17, Juergen Gross wrote: > Add a bitmap with one bit per possible domid indicating the respective > domain has changed its state (created, deleted, dying, crashed, > shutdown). > > Registering the VIRQ_DOM_EXC event will result in setting the bits for > all existing domains and resetting all other bits. > > As the usage of this bitmap is tightly coupled with the VIRQ_DOM_EXC > event, it is meant to be used only by a single consumer in the system, > just like the VIRQ_DOM_EXC event. > > Resetting a bit will be done in a future patch. > > This information is needed for Xenstore to keep track of all domains. > > Signed-off-by: Juergen Gross <jgross@suse.com> > --- > V2: > - use DOMID_FIRST_RESERVED instead of DOMID_MASK + 1 (Jan Beulich) > - use const (Jan Beulich) > - move call of domain_reset_states() into evtchn_bind_virq() (Jan Beulich) > - dynamically allocate dom_state_changed bitmap (Jan Beulich) > V3: > - use xvzalloc_array() (Jan Beulich) > - don't rename existing label (Jan Beulich) > V4: > - add __read_mostly (Jan Beulich) > - use __set_bit() (Jan Beulich) This change looks to have been lost, ... > - fix error handling in evtchn_bind_virq() (Jan Beulich) > V5: > - domain_init_states() may be called only if evtchn_bind_virq() has been > called validly (Jan Beulich) > V6: > - guard dom_state_changed bitmap with d->event_lock (Jan Beulich) ... without it being mentioned anywhere, and without it becoming clear why it would have needed undoing. > --- a/xen/common/domain.c > +++ b/xen/common/domain.c > @@ -35,6 +35,7 @@ > #include <xen/irq.h> > #include <xen/argo.h> > #include <xen/llc-coloring.h> > +#include <xen/xvmalloc.h> > #include <asm/p2m.h> > #include <asm/processor.h> > #include <public/sched.h> > @@ -139,6 +140,51 @@ bool __read_mostly vmtrace_available; > > bool __read_mostly vpmu_is_available; > > +static unsigned long *__read_mostly dom_state_changed; > + > +int domain_init_states(void) > +{ > + const struct domain *d; > + > + ASSERT(!dom_state_changed); > + ASSERT(rw_is_write_locked(¤t->domain->event_lock)); rw_is_write_locked_by_me()? > + dom_state_changed = xvzalloc_array(unsigned long, > + BITS_TO_LONGS(DOMID_FIRST_RESERVED)); > + if ( !dom_state_changed ) > + return -ENOMEM; > + > + rcu_read_lock(&domlist_read_lock); > + > + for_each_domain ( d ) > + set_bit(d->domain_id, dom_state_changed); > + > + rcu_read_unlock(&domlist_read_lock); > + > + return 0; > +} > + > +void domain_deinit_states(const struct domain *d) > +{ > + ASSERT(rw_is_write_locked(&d->event_lock)); Again. Jan
On 07.01.25 17:14, Jan Beulich wrote: > On 07.01.2025 11:17, Juergen Gross wrote: >> Add a bitmap with one bit per possible domid indicating the respective >> domain has changed its state (created, deleted, dying, crashed, >> shutdown). >> >> Registering the VIRQ_DOM_EXC event will result in setting the bits for >> all existing domains and resetting all other bits. >> >> As the usage of this bitmap is tightly coupled with the VIRQ_DOM_EXC >> event, it is meant to be used only by a single consumer in the system, >> just like the VIRQ_DOM_EXC event. >> >> Resetting a bit will be done in a future patch. >> >> This information is needed for Xenstore to keep track of all domains. >> >> Signed-off-by: Juergen Gross <jgross@suse.com> >> --- >> V2: >> - use DOMID_FIRST_RESERVED instead of DOMID_MASK + 1 (Jan Beulich) >> - use const (Jan Beulich) >> - move call of domain_reset_states() into evtchn_bind_virq() (Jan Beulich) >> - dynamically allocate dom_state_changed bitmap (Jan Beulich) >> V3: >> - use xvzalloc_array() (Jan Beulich) >> - don't rename existing label (Jan Beulich) >> V4: >> - add __read_mostly (Jan Beulich) >> - use __set_bit() (Jan Beulich) > > This change looks to have been lost, ... > >> - fix error handling in evtchn_bind_virq() (Jan Beulich) >> V5: >> - domain_init_states() may be called only if evtchn_bind_virq() has been >> called validly (Jan Beulich) >> V6: >> - guard dom_state_changed bitmap with d->event_lock (Jan Beulich) > > ... without it being mentioned anywhere, and without it becoming clear why > it would have needed undoing. Oh, thanks for spotting. I changed that by accident and missed to undo this change. > >> --- a/xen/common/domain.c >> +++ b/xen/common/domain.c >> @@ -35,6 +35,7 @@ >> #include <xen/irq.h> >> #include <xen/argo.h> >> #include <xen/llc-coloring.h> >> +#include <xen/xvmalloc.h> >> #include <asm/p2m.h> >> #include <asm/processor.h> >> #include <public/sched.h> >> @@ -139,6 +140,51 @@ bool __read_mostly vmtrace_available; >> >> bool __read_mostly vpmu_is_available; >> >> +static unsigned long *__read_mostly dom_state_changed; >> + >> +int domain_init_states(void) >> +{ >> + const struct domain *d; >> + >> + ASSERT(!dom_state_changed); >> + ASSERT(rw_is_write_locked(¤t->domain->event_lock)); > > rw_is_write_locked_by_me()? Yes, probably better. > >> + dom_state_changed = xvzalloc_array(unsigned long, >> + BITS_TO_LONGS(DOMID_FIRST_RESERVED)); >> + if ( !dom_state_changed ) >> + return -ENOMEM; >> + >> + rcu_read_lock(&domlist_read_lock); >> + >> + for_each_domain ( d ) >> + set_bit(d->domain_id, dom_state_changed); >> + >> + rcu_read_unlock(&domlist_read_lock); >> + >> + return 0; >> +} >> + >> +void domain_deinit_states(const struct domain *d) >> +{ >> + ASSERT(rw_is_write_locked(&d->event_lock)); > > Again. Yes. Juergen
diff --git a/xen/common/domain.c b/xen/common/domain.c index 0c4cc77111..78e2732e94 100644 --- a/xen/common/domain.c +++ b/xen/common/domain.c @@ -35,6 +35,7 @@ #include <xen/irq.h> #include <xen/argo.h> #include <xen/llc-coloring.h> +#include <xen/xvmalloc.h> #include <asm/p2m.h> #include <asm/processor.h> #include <public/sched.h> @@ -139,6 +140,51 @@ bool __read_mostly vmtrace_available; bool __read_mostly vpmu_is_available; +static unsigned long *__read_mostly dom_state_changed; + +int domain_init_states(void) +{ + const struct domain *d; + + ASSERT(!dom_state_changed); + ASSERT(rw_is_write_locked(¤t->domain->event_lock)); + + dom_state_changed = xvzalloc_array(unsigned long, + BITS_TO_LONGS(DOMID_FIRST_RESERVED)); + if ( !dom_state_changed ) + return -ENOMEM; + + rcu_read_lock(&domlist_read_lock); + + for_each_domain ( d ) + set_bit(d->domain_id, dom_state_changed); + + rcu_read_unlock(&domlist_read_lock); + + return 0; +} + +void domain_deinit_states(const struct domain *d) +{ + ASSERT(rw_is_write_locked(&d->event_lock)); + + XVFREE(dom_state_changed); +} + +static void domain_changed_state(const struct domain *d) +{ + struct domain *hdl; + + hdl = lock_dom_exc_handler(); + if ( unlikely(!hdl) ) + return; + + if ( dom_state_changed ) + set_bit(d->domain_id, dom_state_changed); + + unlock_dom_exc_handler(hdl); +} + static void __domain_finalise_shutdown(struct domain *d) { struct vcpu *v; @@ -153,6 +199,7 @@ static void __domain_finalise_shutdown(struct domain *d) return; d->is_shut_down = 1; + domain_changed_state(d); if ( (d->shutdown_code == SHUTDOWN_suspend) && d->suspend_evtchn ) evtchn_send(d, d->suspend_evtchn); else @@ -840,6 +887,7 @@ struct domain *domain_create(domid_t domid, */ domlist_insert(d); + domain_changed_state(d); memcpy(d->handle, config->handle, sizeof(d->handle)); return d; @@ -1105,6 +1153,7 @@ int domain_kill(struct domain *d) /* Mem event cleanup has to go here because the rings * have to be put before we call put_domain. */ vm_event_cleanup(d); + domain_changed_state(d); put_domain(d); send_global_virq(VIRQ_DOM_EXC); /* fallthrough */ @@ -1294,6 +1343,8 @@ static void cf_check complete_domain_destroy(struct rcu_head *head) xfree(d->vcpu); + domain_changed_state(d); + _domain_destroy(d); send_global_virq(VIRQ_DOM_EXC); diff --git a/xen/common/event_channel.c b/xen/common/event_channel.c index 341221d420..c247efc4b1 100644 --- a/xen/common/event_channel.c +++ b/xen/common/event_channel.c @@ -506,10 +506,18 @@ int evtchn_bind_virq(evtchn_bind_virq_t *bind, evtchn_port_t port) goto out; } + if ( virq == VIRQ_DOM_EXC ) + { + rc = domain_init_states(); + if ( rc ) + goto out; + } + port = rc = evtchn_get_port(d, port); if ( rc < 0 ) { gdprintk(XENLOG_WARNING, "EVTCHNOP failure: error %d\n", rc); + domain_deinit_states(d); goto out; } @@ -742,6 +750,9 @@ int evtchn_close(struct domain *d1, int port1, bool guest) struct vcpu *v; unsigned long flags; + if ( chn1->u.virq == VIRQ_DOM_EXC ) + domain_deinit_states(d1); + v = d1->vcpu[virq_is_global(chn1->u.virq) ? 0 : chn1->notify_vcpu_id]; write_lock_irqsave(&v->virq_lock, flags); @@ -1063,6 +1074,26 @@ static void clear_global_virq_handlers(struct domain *d) } } +struct domain *lock_dom_exc_handler(void) +{ + struct domain *d; + + d = get_global_virq_handler(VIRQ_DOM_EXC); + if ( unlikely(!get_domain(d)) ) + return NULL; + + read_lock(&d->event_lock); + + return d; +} + +void unlock_dom_exc_handler(struct domain *d) +{ + read_unlock(&d->event_lock); + + put_domain(d); +} + int evtchn_status(evtchn_status_t *status) { struct domain *d; diff --git a/xen/include/xen/event.h b/xen/include/xen/event.h index 48b79f3d62..5c0ba90c9f 100644 --- a/xen/include/xen/event.h +++ b/xen/include/xen/event.h @@ -100,6 +100,10 @@ bool evtchn_virq_enabled(const struct vcpu *v, unsigned int virq); /* Notify remote end of a Xen-attached event channel.*/ void notify_via_xen_event_channel(struct domain *ld, int lport); +/* Lock/unlock of VIRQ_DOM_EXC associated data (read_lock(d->event_lock)). */ +struct domain *lock_dom_exc_handler(void); +void unlock_dom_exc_handler(struct domain *d); + /* * Internal event channel object storage. * diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h index 037c83fda2..9d9b89ec27 100644 --- a/xen/include/xen/sched.h +++ b/xen/include/xen/sched.h @@ -805,6 +805,9 @@ void domain_resume(struct domain *d); int domain_soft_reset(struct domain *d, bool resuming); +int domain_init_states(void); +void domain_deinit_states(const struct domain *d); + int vcpu_start_shutdown_deferral(struct vcpu *v); void vcpu_end_shutdown_deferral(struct vcpu *v);
Add a bitmap with one bit per possible domid indicating the respective domain has changed its state (created, deleted, dying, crashed, shutdown). Registering the VIRQ_DOM_EXC event will result in setting the bits for all existing domains and resetting all other bits. As the usage of this bitmap is tightly coupled with the VIRQ_DOM_EXC event, it is meant to be used only by a single consumer in the system, just like the VIRQ_DOM_EXC event. Resetting a bit will be done in a future patch. This information is needed for Xenstore to keep track of all domains. Signed-off-by: Juergen Gross <jgross@suse.com> --- V2: - use DOMID_FIRST_RESERVED instead of DOMID_MASK + 1 (Jan Beulich) - use const (Jan Beulich) - move call of domain_reset_states() into evtchn_bind_virq() (Jan Beulich) - dynamically allocate dom_state_changed bitmap (Jan Beulich) V3: - use xvzalloc_array() (Jan Beulich) - don't rename existing label (Jan Beulich) V4: - add __read_mostly (Jan Beulich) - use __set_bit() (Jan Beulich) - fix error handling in evtchn_bind_virq() (Jan Beulich) V5: - domain_init_states() may be called only if evtchn_bind_virq() has been called validly (Jan Beulich) V6: - guard dom_state_changed bitmap with d->event_lock (Jan Beulich) --- xen/common/domain.c | 51 ++++++++++++++++++++++++++++++++++++++ xen/common/event_channel.c | 31 +++++++++++++++++++++++ xen/include/xen/event.h | 4 +++ xen/include/xen/sched.h | 3 +++ 4 files changed, 89 insertions(+)