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.
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)
---
xen/common/domain.c | 59 ++++++++++++++++++++++++++++++++++++++
xen/common/event_channel.c | 20 +++++++++++--
xen/include/xen/sched.h | 3 ++
3 files changed, 79 insertions(+), 3 deletions(-)
diff --git a/xen/common/domain.c b/xen/common/domain.c
index e33a0a5a21..4e3a593a9d 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -138,6 +138,60 @@ bool __read_mostly vmtrace_available;
bool __read_mostly vpmu_is_available;
+static DEFINE_SPINLOCK(dom_state_changed_lock);
+static unsigned long *dom_state_changed;
+
+int domain_init_states(void)
+{
+ const struct domain *d;
+ int rc = -ENOMEM;
+
+ spin_lock(&dom_state_changed_lock);
+
+ if ( dom_state_changed )
+ bitmap_zero(dom_state_changed, DOMID_FIRST_RESERVED);
+ else
+ {
+ dom_state_changed = xzalloc_array(unsigned long,
+ BITS_TO_LONGS(DOMID_FIRST_RESERVED));
+ if ( !dom_state_changed )
+ goto unlock;
+ }
+
+ rcu_read_lock(&domlist_read_lock);
+
+ for_each_domain ( d )
+ set_bit(d->domain_id, dom_state_changed);
+
+ rcu_read_unlock(&domlist_read_lock);
+
+ rc = 0;
+
+ unlock:
+ spin_unlock(&dom_state_changed_lock);
+
+ return rc;
+}
+
+void domain_deinit_states(void)
+{
+ spin_lock(&dom_state_changed_lock);
+
+ XFREE(dom_state_changed);
+
+ spin_unlock(&dom_state_changed_lock);
+}
+
+static void domain_changed_state(const struct domain *d)
+{
+ spin_lock(&dom_state_changed_lock);
+
+ if ( dom_state_changed )
+ set_bit(d->domain_id, dom_state_changed);
+
+ spin_unlock(&dom_state_changed_lock);
+}
+
static void __domain_finalise_shutdown(struct domain *d)
{
struct vcpu *v;
@@ -152,6 +206,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
@@ -839,6 +894,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;
@@ -1104,6 +1160,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 */
@@ -1293,6 +1350,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 8db2ca4ba2..cd7bad68c0 100644
--- a/xen/common/event_channel.c
+++ b/xen/common/event_channel.c
@@ -485,20 +485,27 @@ int evtchn_bind_virq(evtchn_bind_virq_t *bind, evtchn_port_t port)
if ( (v = domain_vcpu(d, vcpu)) == NULL )
return -ENOENT;
+ if ( virq == VIRQ_DOM_EXC )
+ {
+ rc = domain_init_states();
+ if ( rc )
+ goto out;
+ }
+
write_lock(&d->event_lock);
if ( read_atomic(&v->virq_to_evtchn[virq]) )
{
rc = -EEXIST;
gdprintk(XENLOG_WARNING, "EVTCHNOP failure: error %d\n", rc);
- goto out;
+ goto unlock;
}
port = rc = evtchn_get_port(d, port);
if ( rc < 0 )
{
gdprintk(XENLOG_WARNING, "EVTCHNOP failure: error %d\n", rc);
- goto out;
+ goto unlock;
}
rc = 0;
@@ -524,9 +531,13 @@ int evtchn_bind_virq(evtchn_bind_virq_t *bind, evtchn_port_t port)
*/
write_atomic(&v->virq_to_evtchn[virq], port);
- out:
+ unlock:
write_unlock(&d->event_lock);
+ out:
+ if ( rc )
+ domain_deinit_states();
+
return rc;
}
@@ -730,6 +741,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();
+
v = d1->vcpu[virq_is_global(chn1->u.virq) ? 0 : chn1->notify_vcpu_id];
write_lock_irqsave(&v->virq_lock, flags);
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index 711668e028..16684bbaf9 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -800,6 +800,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(void);
+
int vcpu_start_shutdown_deferral(struct vcpu *v);
void vcpu_end_shutdown_deferral(struct vcpu *v);
--
2.43.0On 06.12.2024 14:02, 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.
>
> 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>
What I'm still missing is at least mention of the global-ness of all of
this, and why that's deemed okay for now.
> --- a/xen/common/domain.c
> +++ b/xen/common/domain.c
> @@ -138,6 +138,60 @@ bool __read_mostly vmtrace_available;
>
> bool __read_mostly vpmu_is_available;
>
> +static DEFINE_SPINLOCK(dom_state_changed_lock);
> +static unsigned long *dom_state_changed;
> +
> +int domain_init_states(void)
> +{
> + const struct domain *d;
> + int rc = -ENOMEM;
> +
> + spin_lock(&dom_state_changed_lock);
> +
> + if ( dom_state_changed )
> + bitmap_zero(dom_state_changed, DOMID_FIRST_RESERVED);
> + else
> + {
> + dom_state_changed = xzalloc_array(unsigned long,
> + BITS_TO_LONGS(DOMID_FIRST_RESERVED));
New code wants to use xvmalloc() et al.
> --- a/xen/common/event_channel.c
> +++ b/xen/common/event_channel.c
> @@ -485,20 +485,27 @@ int evtchn_bind_virq(evtchn_bind_virq_t *bind, evtchn_port_t port)
> if ( (v = domain_vcpu(d, vcpu)) == NULL )
> return -ENOENT;
>
> + if ( virq == VIRQ_DOM_EXC )
> + {
> + rc = domain_init_states();
> + if ( rc )
> + goto out;
> + }
> +
> write_lock(&d->event_lock);
>
> if ( read_atomic(&v->virq_to_evtchn[virq]) )
> {
> rc = -EEXIST;
> gdprintk(XENLOG_WARNING, "EVTCHNOP failure: error %d\n", rc);
> - goto out;
> + goto unlock;
> }
>
> port = rc = evtchn_get_port(d, port);
> if ( rc < 0 )
> {
> gdprintk(XENLOG_WARNING, "EVTCHNOP failure: error %d\n", rc);
> - goto out;
> + goto unlock;
> }
>
> rc = 0;
> @@ -524,9 +531,13 @@ int evtchn_bind_virq(evtchn_bind_virq_t *bind, evtchn_port_t port)
> */
> write_atomic(&v->virq_to_evtchn[virq], port);
>
> - out:
> + unlock:
> write_unlock(&d->event_lock);
>
> + out:
> + if ( rc )
> + domain_deinit_states();
> +
> return rc;
> }
Renaming the prior label (and hence needing to fiddle with existing goto-s)
feels a little fragile. How about keeping "out" as is and introducing "deinit"
or some such?
Jan
On 09.12.24 17:52, Jan Beulich wrote:
> On 06.12.2024 14:02, 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.
>>
>> 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>
>
> What I'm still missing is at least mention of the global-ness of all of
> this, and why that's deemed okay for now.
I'll add:
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.
>
>> --- a/xen/common/domain.c
>> +++ b/xen/common/domain.c
>> @@ -138,6 +138,60 @@ bool __read_mostly vmtrace_available;
>>
>> bool __read_mostly vpmu_is_available;
>>
>> +static DEFINE_SPINLOCK(dom_state_changed_lock);
>> +static unsigned long *dom_state_changed;
>> +
>> +int domain_init_states(void)
>> +{
>> + const struct domain *d;
>> + int rc = -ENOMEM;
>> +
>> + spin_lock(&dom_state_changed_lock);
>> +
>> + if ( dom_state_changed )
>> + bitmap_zero(dom_state_changed, DOMID_FIRST_RESERVED);
>> + else
>> + {
>> + dom_state_changed = xzalloc_array(unsigned long,
>> + BITS_TO_LONGS(DOMID_FIRST_RESERVED));
>
> New code wants to use xvmalloc() et al.
Okay.
>
>> --- a/xen/common/event_channel.c
>> +++ b/xen/common/event_channel.c
>> @@ -485,20 +485,27 @@ int evtchn_bind_virq(evtchn_bind_virq_t *bind, evtchn_port_t port)
>> if ( (v = domain_vcpu(d, vcpu)) == NULL )
>> return -ENOENT;
>>
>> + if ( virq == VIRQ_DOM_EXC )
>> + {
>> + rc = domain_init_states();
>> + if ( rc )
>> + goto out;
>> + }
>> +
>> write_lock(&d->event_lock);
>>
>> if ( read_atomic(&v->virq_to_evtchn[virq]) )
>> {
>> rc = -EEXIST;
>> gdprintk(XENLOG_WARNING, "EVTCHNOP failure: error %d\n", rc);
>> - goto out;
>> + goto unlock;
>> }
>>
>> port = rc = evtchn_get_port(d, port);
>> if ( rc < 0 )
>> {
>> gdprintk(XENLOG_WARNING, "EVTCHNOP failure: error %d\n", rc);
>> - goto out;
>> + goto unlock;
>> }
>>
>> rc = 0;
>> @@ -524,9 +531,13 @@ int evtchn_bind_virq(evtchn_bind_virq_t *bind, evtchn_port_t port)
>> */
>> write_atomic(&v->virq_to_evtchn[virq], port);
>>
>> - out:
>> + unlock:
>> write_unlock(&d->event_lock);
>>
>> + out:
>> + if ( rc )
>> + domain_deinit_states();
>> +
>> return rc;
>> }
>
> Renaming the prior label (and hence needing to fiddle with existing goto-s)
> feels a little fragile. How about keeping "out" as is and introducing "deinit"
> or some such?
Fine with me.
Juergen
© 2016 - 2025 Red Hat, Inc.