Hi Andre,
On 02/05/2018 04:19 PM, Andre Przywara wrote:
At the moment we happily access VGIC internal data structures like
the rank and struct pending_irq in gic.c, which should be VGIC agnostic.
Factor out a new function vgic_connect_hw_irq(), which allows a virtual
IRQ to be connected to a hardware IRQ (using the hw bit in the LR).
This removes said accesses to VGIC data structures and improves abstraction.
I was expecting some explanation regarding the new locking order in the
commit message.
Signed-off-by: Andre Przywara <andre.przyw...@arm.com>
---
xen/arch/arm/gic-vgic.c | 36 ++++++++++++++++++++++++++++++++++++
xen/arch/arm/gic.c | 44 ++++++++++----------------------------------
xen/include/asm-arm/vgic.h | 2 ++
3 files changed, 48 insertions(+), 34 deletions(-)
diff --git a/xen/arch/arm/gic-vgic.c b/xen/arch/arm/gic-vgic.c
index 1d5744ecc8..fff7c01ee8 100644
--- a/xen/arch/arm/gic-vgic.c
+++ b/xen/arch/arm/gic-vgic.c
@@ -397,6 +397,42 @@ void gic_dump_vgic_info(struct vcpu *v)
printk("Pending irq=%d\n", p->irq);
}
+int vgic_connect_hw_irq(struct domain *d, struct vcpu *v, unsigned int virq,
+ struct irq_desc *desc, bool connect)
+{
+ unsigned long flags;
+ /* Use vcpu0 to retrieve the pending_irq struct. Given that we only
+ * route SPIs to guests, it doesn't make any difference. */
Please fix the coding style as requested in v3.
+ struct vcpu *v_target = vgic_get_target_vcpu(d->vcpu[0], virq);
+ struct vgic_irq_rank *rank = vgic_rank_irq(v_target, virq);
+ struct pending_irq *p = irq_to_pending(v_target, virq);
+ int ret = 0;
+
+ ASSERT(connect && desc);
I am not sure why you allow desc to be non-NULL when disconnecting it
(see more below).
+
+ /* We are taking to rank lock to prevent parallel connections. */
+ vgic_lock_rank(v_target, rank, flags);
+
+ if ( connect )
+ {
+ /* The VIRQ should not be already enabled by the guest */
+ if ( !p->desc &&
+ !test_bit(GIC_IRQ_GUEST_ENABLED, &p->status) )
+ p->desc = desc;
+ else
+ ret = -EBUSY;
+ }
+ else
+ {
+ if ( !desc || p->desc == desc )
From a quick glance, no caller will have desc is NULL. Even if it was,
it will not harm because p->desc will be set to NULL.
+ p->desc = NULL;
+ }
But likely you want to return an error if p->desc != desc as this is a
user input error. Ignoring it is a pretty bad.
+
+ vgic_unlock_rank(v_target, rank, flags);
+
+ return ret;
+}
+
/*
* Local variables:
* mode: C
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
