[v4,6/9] xen/arm/gic: Allow removing interrupt to running VMs

Commit Message

Henry Wang May 23, 2024, 7:40 a.m. UTC

From: Vikram Garhwal <[e-mail]>

Currently, removing physical interrupts are only allowed at
the domain destroy time. For use cases such as dynamic device
tree overlay removing, the removing of physical IRQ to
running domains should be allowed.

Move the above-mentioned domain dying check to vgic_connect_hw_irq().
Similarly as the routing interrupt to running domains, reject the
operation if the IRQ is active or pending in the guest. Do it for
both new and old vGIC implementations. Since now vgic_connect_hw_irq()
may reject the invalid operation case, move the clear of
_IRQ_INPROGRESS flag in gic_remove_irq_from_guest() to after the
successful execution of vgic_connect_hw_irq().

Signed-off-by: Vikram Garhwal <[e-mail]>
Signed-off-by: Stefano Stabellini <[e-mail]>
Signed-off-by: Henry Wang <[e-mail]>
---
v4:
- Split the original patch, only do the removing IRQ stuff in this
  patch.
- Move the clear of _IRQ_INPROGRESS flag in gic_remove_irq_from_guest()
  to after the successful execution of vgic_connect_hw_irq().
- Special case the d->is_dying check.
---
 xen/arch/arm/gic-vgic.c  | 27 ++++++++++++++++++++++++---
 xen/arch/arm/gic.c       |  9 +--------
 xen/arch/arm/vgic/vgic.c | 24 ++++++++++++++++++++----
 3 files changed, 45 insertions(+), 15 deletions(-)

Patch

diff --git a/xen/arch/arm/gic-vgic.c b/xen/arch/arm/gic-vgic.c
index b99e287224..56b6a3d5b0 100644
--- a/xen/arch/arm/gic-vgic.c
+++ b/xen/arch/arm/gic-vgic.c
@@ -439,6 +439,14 @@  int vgic_connect_hw_irq(struct domain *d, struct vcpu *v, unsigned int virq,
 
     /* We are taking to rank lock to prevent parallel connections. */
     vgic_lock_rank(v_target, rank, flags);
+    /* Return with error if the IRQ is being migrated. */
+    if( test_bit(GIC_IRQ_GUEST_MIGRATING, &p->status) )
+    {
+        vgic_unlock_rank(v_target, rank, flags);
+        return -EBUSY;
+    }
+
+    spin_lock(&v_target->arch.vgic.lock);
 
     if ( connect )
     {
@@ -456,12 +464,25 @@  int vgic_connect_hw_irq(struct domain *d, struct vcpu *v, unsigned int virq,
     }
     else
     {
-        if ( desc && p->desc != desc )
-            ret = -EINVAL;
+        if ( d->is_dying )
+        {
+            if ( desc && p->desc != desc )
+                ret = -EINVAL;
+            else
+                p->desc = NULL;
+        }
         else
-            p->desc = NULL;
+        {
+            if ( (desc && p->desc != desc) ||
+                 test_bit(GIC_IRQ_GUEST_VISIBLE, &p->status) ||
+                 test_bit(GIC_IRQ_GUEST_ACTIVE, &p->status) )
+                ret = -EINVAL;
+            else
+                p->desc = NULL;
+        }
     }
 
+    spin_unlock(&v_target->arch.vgic.lock);
     vgic_unlock_rank(v_target, rank, flags);
 
     return ret;
diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index b3467a76ae..8633f14bdd 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -159,24 +159,17 @@  int gic_remove_irq_from_guest(struct domain *d, unsigned int virq,
     ASSERT(test_bit(_IRQ_GUEST, &desc->status));
     ASSERT(!is_lpi(virq));
 
-    /*
-     * Removing an interrupt while the domain is running may have
-     * undesirable effect on the vGIC emulation.
-     */
-    if ( !d->is_dying )
-        return -EBUSY;
-
     desc->handler->shutdown(desc);
 
     /* EOI the IRQ if it has not been done by the guest */
     if ( test_bit(_IRQ_INPROGRESS, &desc->status) )
         gic_hw_ops->deactivate_irq(desc);
-    clear_bit(_IRQ_INPROGRESS, &desc->status);
 
     ret = vgic_connect_hw_irq(d, NULL, virq, desc, false);
     if ( ret )
         return ret;
 
+    clear_bit(_IRQ_INPROGRESS, &desc->status);
     clear_bit(_IRQ_GUEST, &desc->status);
     desc->handler = &no_irq_type;
 
diff --git a/xen/arch/arm/vgic/vgic.c b/xen/arch/arm/vgic/vgic.c
index 048e12c562..0c324b58f7 100644
--- a/xen/arch/arm/vgic/vgic.c
+++ b/xen/arch/arm/vgic/vgic.c
@@ -890,14 +890,30 @@  int vgic_connect_hw_irq(struct domain *d, struct vcpu *vcpu,
     }
     else                                /* remove a mapped IRQ */
     {
-        if ( desc && irq->hwintid != desc->irq )
+        if ( d->is_dying )
         {
-            ret = -EINVAL;
+            if ( desc && irq->hwintid != desc->irq )
+            {
+                ret = -EINVAL;
+            }
+            else
+            {
+                irq->hw = false;
+                irq->hwintid = 0;
+            }
         }
         else
         {
-            irq->hw = false;
-            irq->hwintid = 0;
+            if ( (desc && irq->hwintid != desc->irq) ||
+                 irq->active || irq->pending_latch )
+            {
+                ret = -EINVAL;
+            }
+            else
+            {
+                irq->hw = false;
+                irq->hwintid = 0;
+            }
         }
     }