Hi,
On 17/05/2024 07:03, Henry Wang wrote:
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 */
This comment needs to be updated.
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) )
Looking at gic_set_lr(), we first check p->desc, before setting
IRQ_GUEST_VISIBLE.
I can't find a common lock, so what would guarantee that p->desc is not
going to be used or IRQ_GUEST_VISIBLE set afterwards?
@@ -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 )
I think the new vGIC doesn't have the same problem because it looks like
the IRQ lock will be taken for any access to 'irq'.
Cheers,
--
Julien Grall
