Re: [Xen-devel] [RFC PATCH v2 03/22] ARM: vGIC: move gic_raise_inflight_irq() into vgic_vcpu_inject_irq()

[Julien Grall]

Hi Andre,

On 21/07/17 20:59, Andre Przywara wrote:

Currently there is a gic_raise_inflight_irq(), which serves the very
special purpose of handling a newly injected interrupt while an older
one is still handled. This has only one user, in vgic_vcpu_inject_irq().

Now with the introduction of the pending_irq lock this will later on
result in a nasty deadlock, which can only be solved properly by
actually embedding the function into the caller (and dropping the lock
later in-between).

This has the admittedly hideous consequence of needing to export
gic_update_one_lr(), but this will go away in a later stage of a rework.
In this respect this patch is more a temporary kludge.

Signed-off-by: Andre Przywara 
---
 xen/arch/arm/gic.c| 30 +-
 xen/arch/arm/vgic.c   | 11 ++-
 xen/include/asm-arm/gic.h |  2 +-
 3 files changed, 12 insertions(+), 31 deletions(-)

diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index 2c99d71..5bd66a2 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -44,8 +44,6 @@ static DEFINE_PER_CPU(uint64_t, lr_mask);

 #undef GIC_DEBUG

-static void gic_update_one_lr(struct vcpu *v, int i);
-
 static const struct gic_hw_operations *gic_hw_ops;

 void register_gic_ops(const struct gic_hw_operations *ops)
@@ -416,32 +414,6 @@ void gic_remove_irq_from_queues(struct vcpu *v, struct 
pending_irq *p)
 gic_remove_from_lr_pending(v, p);
 }

-void gic_raise_inflight_irq(struct vcpu *v, unsigned int virtual_irq)
-{
-struct pending_irq *n = irq_to_pending(v, virtual_irq);
-
-/* If an LPI has been removed meanwhile, there is nothing left to raise. */
-if ( unlikely(!n) )
-return;
-
-ASSERT(spin_is_locked(>arch.vgic.lock));
-
-/* Don't try to update the LR if the interrupt is disabled */
-if ( !test_bit(GIC_IRQ_GUEST_ENABLED, >status) )
-return;
-
-if ( list_empty(>lr_queue) )
-{
-if ( v == current )
-gic_update_one_lr(v, n->lr);
-}
-#ifdef GIC_DEBUG
-else
-gdprintk(XENLOG_DEBUG, "trying to inject irq=%u into d%dv%d, when it is 
still lr_pending\n",
- virtual_irq, v->domain->domain_id, v->vcpu_id);
-#endif
-}
-
 /*
  * Find an unused LR to insert an IRQ into, starting with the LR given
  * by @lr. If this new interrupt is a PRISTINE LPI, scan the other LRs to
@@ -503,7 +475,7 @@ void gic_raise_guest_irq(struct vcpu *v, unsigned int 
virtual_irq,
 gic_add_to_lr_pending(v, p);
 }

-static void gic_update_one_lr(struct vcpu *v, int i)
+void gic_update_one_lr(struct vcpu *v, int i)
 {
 struct pending_irq *p;
 int irq;
diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
index 38dacd3..7b122cd 100644
--- a/xen/arch/arm/vgic.c
+++ b/xen/arch/arm/vgic.c
@@ -536,7 +536,16 @@ void vgic_vcpu_inject_irq(struct vcpu *v, unsigned int 
virq)

 if ( !list_empty(>inflight) )
 {
-gic_raise_inflight_irq(v, virq);
+bool update = test_bit(GIC_IRQ_GUEST_ENABLED, >status) &&
+  list_empty(>lr_queue) && (v == current);
+
+if ( update )
+gic_update_one_lr(v, n->lr);
+#ifdef GIC_DEBUG
+else
+gdprintk(XENLOG_DEBUG, "trying to inject irq=%u into d%dv%d, when it is 
still lr_pending\n",
+ n->irq, v->domain->domain_id, v->vcpu_id);


The previous code was only printing this message when the pending_irq is 
queued. Now you will print any time you don't update the LR.



+#endif
 goto out;
 }

diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h
index 6203dc5..cf8b8fb 100644
--- a/xen/include/asm-arm/gic.h
+++ b/xen/include/asm-arm/gic.h
@@ -237,12 +237,12 @@ int gic_remove_irq_from_guest(struct domain *d, unsigned 
int virq,

 extern void gic_inject(void);
 extern void gic_clear_pending_irqs(struct vcpu *v);
+extern void gic_update_one_lr(struct vcpu *v, int lr);
 extern int gic_events_need_delivery(void);

 extern void init_maintenance_interrupt(void);
 extern void gic_raise_guest_irq(struct vcpu *v, unsigned int irq,
 unsigned int priority);
-extern void gic_raise_inflight_irq(struct vcpu *v, unsigned int virtual_irq);
 extern void gic_remove_from_lr_pending(struct vcpu *v, struct pending_irq *p);
 extern void gic_remove_irq_from_queues(struct vcpu *v, struct pending_irq *p);




Cheers,

--
Julien Grall

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [RFC PATCH v2 03/22] ARM: vGIC: move gic_raise_inflight_irq() into vgic_vcpu_inject_irq()

[Andre Przywara]

Currently there is a gic_raise_inflight_irq(), which serves the very
special purpose of handling a newly injected interrupt while an older
one is still handled. This has only one user, in vgic_vcpu_inject_irq().

Now with the introduction of the pending_irq lock this will later on
result in a nasty deadlock, which can only be solved properly by
actually embedding the function into the caller (and dropping the lock
later in-between).

This has the admittedly hideous consequence of needing to export
gic_update_one_lr(), but this will go away in a later stage of a rework.
In this respect this patch is more a temporary kludge.

Signed-off-by: Andre Przywara 
---
 xen/arch/arm/gic.c| 30 +-
 xen/arch/arm/vgic.c   | 11 ++-
 xen/include/asm-arm/gic.h |  2 +-
 3 files changed, 12 insertions(+), 31 deletions(-)

diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index 2c99d71..5bd66a2 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -44,8 +44,6 @@ static DEFINE_PER_CPU(uint64_t, lr_mask);
 
 #undef GIC_DEBUG
 
-static void gic_update_one_lr(struct vcpu *v, int i);
-
 static const struct gic_hw_operations *gic_hw_ops;
 
 void register_gic_ops(const struct gic_hw_operations *ops)
@@ -416,32 +414,6 @@ void gic_remove_irq_from_queues(struct vcpu *v, struct 
pending_irq *p)
 gic_remove_from_lr_pending(v, p);
 }
 
-void gic_raise_inflight_irq(struct vcpu *v, unsigned int virtual_irq)
-{
-struct pending_irq *n = irq_to_pending(v, virtual_irq);
-
-/* If an LPI has been removed meanwhile, there is nothing left to raise. */
-if ( unlikely(!n) )
-return;
-
-ASSERT(spin_is_locked(>arch.vgic.lock));
-
-/* Don't try to update the LR if the interrupt is disabled */
-if ( !test_bit(GIC_IRQ_GUEST_ENABLED, >status) )
-return;
-
-if ( list_empty(>lr_queue) )
-{
-if ( v == current )
-gic_update_one_lr(v, n->lr);
-}
-#ifdef GIC_DEBUG
-else
-gdprintk(XENLOG_DEBUG, "trying to inject irq=%u into d%dv%d, when it 
is still lr_pending\n",
- virtual_irq, v->domain->domain_id, v->vcpu_id);
-#endif
-}
-
 /*
  * Find an unused LR to insert an IRQ into, starting with the LR given
  * by @lr. If this new interrupt is a PRISTINE LPI, scan the other LRs to
@@ -503,7 +475,7 @@ void gic_raise_guest_irq(struct vcpu *v, unsigned int 
virtual_irq,
 gic_add_to_lr_pending(v, p);
 }
 
-static void gic_update_one_lr(struct vcpu *v, int i)
+void gic_update_one_lr(struct vcpu *v, int i)
 {
 struct pending_irq *p;
 int irq;
diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
index 38dacd3..7b122cd 100644
--- a/xen/arch/arm/vgic.c
+++ b/xen/arch/arm/vgic.c
@@ -536,7 +536,16 @@ void vgic_vcpu_inject_irq(struct vcpu *v, unsigned int 
virq)
 
 if ( !list_empty(>inflight) )
 {
-gic_raise_inflight_irq(v, virq);
+bool update = test_bit(GIC_IRQ_GUEST_ENABLED, >status) &&
+  list_empty(>lr_queue) && (v == current);
+
+if ( update )
+gic_update_one_lr(v, n->lr);
+#ifdef GIC_DEBUG
+else
+gdprintk(XENLOG_DEBUG, "trying to inject irq=%u into d%dv%d, when 
it is still lr_pending\n",
+ n->irq, v->domain->domain_id, v->vcpu_id);
+#endif
 goto out;
 }
 
diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h
index 6203dc5..cf8b8fb 100644
--- a/xen/include/asm-arm/gic.h
+++ b/xen/include/asm-arm/gic.h
@@ -237,12 +237,12 @@ int gic_remove_irq_from_guest(struct domain *d, unsigned 
int virq,
 
 extern void gic_inject(void);
 extern void gic_clear_pending_irqs(struct vcpu *v);
+extern void gic_update_one_lr(struct vcpu *v, int lr);
 extern int gic_events_need_delivery(void);
 
 extern void init_maintenance_interrupt(void);
 extern void gic_raise_guest_irq(struct vcpu *v, unsigned int irq,
 unsigned int priority);
-extern void gic_raise_inflight_irq(struct vcpu *v, unsigned int virtual_irq);
 extern void gic_remove_from_lr_pending(struct vcpu *v, struct pending_irq *p);
 extern void gic_remove_irq_from_queues(struct vcpu *v, struct pending_irq *p);
 
-- 
2.9.0


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel