Re: [PATCH v2 5/8] xen/arm/gic: Allow routing/removing interrupt to running VMs

[Henry Wang]

On 5/16/2024 6:03 PM, Henry Wang wrote:

From: Vikram Garhwal <fnu.vik...@xilinx.com>

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

Removing the above-mentioned domain creation/dying check. Since this
will introduce interrupt state unsync issues for cases when the
interrupt is active or pending in the guest, therefore for these cases
we simply reject the operation. Do it for both new and old vGIC
implementations.

Signed-off-by: Vikram Garhwal <fnu.vik...@xilinx.com>
Signed-off-by: Stefano Stabellini <stefano.stabell...@xilinx.com>
Signed-off-by: Henry Wang <xin.wa...@amd.com>
---
v2:
- Reject the case where the IRQ is active or pending in guest.
---
  xen/arch/arm/gic-vgic.c  |  8 ++++++--
  xen/arch/arm/gic.c       | 15 ---------------
  xen/arch/arm/vgic/vgic.c |  5 +++--
  3 files changed, 9 insertions(+), 19 deletions(-)

diff --git a/xen/arch/arm/gic-vgic.c b/xen/arch/arm/gic-vgic.c
index 56490dbc43..d1608415f8 100644
--- a/xen/arch/arm/gic-vgic.c
+++ b/xen/arch/arm/gic-vgic.c
@@ -444,14 +444,18 @@ int vgic_connect_hw_irq(struct domain *d, struct vcpu *v, 
unsigned int virq,
      {
          /* The VIRQ should not be already enabled by the guest */
          if ( !p->desc &&
-             !test_bit(GIC_IRQ_GUEST_ENABLED, &p->status) )
+             !test_bit(GIC_IRQ_GUEST_ENABLED, &p->status) &&
+             !test_bit(GIC_IRQ_GUEST_VISIBLE, &p->status) &&
+             !test_bit(GIC_IRQ_GUEST_ACTIVE, &p->status) )
              p->desc = desc;
          else
              ret = -EBUSY;
      }
      else
      {
-        if ( desc && p->desc != desc )
+        if ( desc && p->desc != desc &&
+             (test_bit(GIC_IRQ_GUEST_VISIBLE, &p->status) ||
+              test_bit(GIC_IRQ_GUEST_ACTIVE, &p->status)) )

This should be

+        if ( (desc && p->desc != desc) ||
+             test_bit(GIC_IRQ_GUEST_VISIBLE, &p->status) ||
+             test_bit(GIC_IRQ_GUEST_ACTIVE, &p->status) )

@@ -887,7 +887,8 @@ int vgic_connect_hw_irq(struct domain *d, struct vcpu *vcpu,
      }
      else                                /* remove a mapped IRQ */
      {
-        if ( desc && irq->hwintid != desc->irq )
+        if ( desc && irq->hwintid != desc->irq &&
+             (irq->active || irq->pending_latch) )

Same here, this should be

+        if ( (desc && irq->hwintid != desc->irq) ||
+             irq->active || irq->pending_latch )

Kind regards,
Henry