Re: [PATCH v3 4/7] xen: add bitmap to indicate per-domain state changes

[Jürgen Groß]

On 16.12.24 11:21, Jan Beulich wrote:

On 13.12.2024 17:24, Juergen Gross wrote:

@@ -138,6 +139,60 @@ bool __read_mostly vmtrace_available;

+static unsigned long *dom_state_changed;

bool __read_mostly vpmu_is_available; +static DEFINE_SPINLOCK(dom_state_changed_lock);

__read_mostly?

Yes.

+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 = xvzalloc_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);

Use the cheaper __set_bit() here?

Okay.

+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);

And perhaps even here, considering everything's under lock now?

True.

--- a/xen/common/event_channel.c
+++ b/xen/common/event_channel.c
@@ -485,6 +485,13 @@ int evtchn_bind_virq(evtchn_bind_virq_t *bind, 
evtchn_port_t port)
      if ( (v = domain_vcpu(d, vcpu)) == NULL )
          return -ENOENT;

+    {
+        rc = domain_init_states();
+        if ( rc )
+            goto deinit;
+    }

+ if ( virq == VIRQ_DOM_EXC )

This is tied to VIRQ_DOM_EXC. How come ...

@@ -527,6 +534,10 @@ int evtchn_bind_virq(evtchn_bind_virq_t *bind, 
evtchn_port_t port)
   out:
      write_unlock(&d->event_lock);

+    if ( rc )
+        domain_deinit_states();
+
      return rc;
  }

+ deinit:

... de-init happens upon any error, regardless of vIRQ? Even checking
the virq isn't sufficient, as we also need to gracefully deal with the
-EEXIST path.

Good catch. Will change that.


Juergen

OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

OpenPGP_signature.asc
Description: OpenPGP digital signature