Re: [PATCH] xen/events: fix get_global_virq_handler() usage without hardware domain

[Juergen Gross]

On 06.03.25 17:00, Jan Beulich wrote:

On 06.03.2025 16:54, Juergen Gross wrote:

--- a/xen/common/event_channel.c
+++ b/xen/common/event_channel.c
@@ -1036,7 +1036,9 @@ int set_global_virq_handler(struct domain *d, uint32_t 
virq)
      {
          old = global_virq_handlers[virq];
          hdl = get_global_virq_handler(virq);
-        if ( hdl != d )
+        if ( !hdl )
+            global_virq_handlers[virq] = d;
+        else if ( hdl != d )
          {
              read_lock(&hdl->event_lock);

      struct domain *d;

-    if ( unlikely(!get_domain(d)) )
+    if ( unlikely(!d || !get_domain(d)) )

@@ -1091,7 +1093,7 @@ struct domain *lock_dom_exc_handler(void) d = get_global_virq_handler(VIRQ_DOM_EXC);

According to my understanding of how unlikely() works this wants to be

     if ( unlikely(!d) || unlikely(!get_domain(d)) )

Of course I could also live with being given an explanation of why my
understanding is wrong.

I was unsure, so I looked through the code until I found a case where
2 conditions were tested as "unlikely". I might have found a wrong example.
I'll change it as you are suggesting.

          return NULL;

read_lock(&d->event_lock);

There's one more change needed: get_domain_state() needs to avoid calling
unlock_dom_exc_handler() when "hdl" is NULL.

Oh, right. Will add.


Juergen

OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

OpenPGP_signature.asc
Description: OpenPGP digital signature