[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v3 5/6] xen/arm: Only enable physical IRQs when the guest asks

On 12/10/2013 06:50 PM, Stefano Stabellini wrote:
From: Julien Grall <julien.grall@xxxxxxxxxx>

Set/Unset IRQ_DISABLED from gic_irq_enable and gic_irq_disable.
Enable IRQs when the guest requests it, not unconditionally at boot time.

Signed-off-by: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
Signed-off-by: Julien Grall <julien.grall@xxxxxxxxxx>
Acked-by: Ian Campbell <ian.campbell@xxxxxxxxxx>

Changes in v2:
- protect startup and shutdown with gic and desc locks.
  xen/arch/arm/gic.c  |   46 ++++++++++++++++++++++++++--------------------
  xen/arch/arm/vgic.c |    6 ++----
  2 files changed, 28 insertions(+), 24 deletions(-)

diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index 5e60c5a..62330dd 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -133,16 +133,26 @@ static void gic_irq_enable(struct irq_desc *desc)
      int irq = desc->irq;

+    spin_lock(&desc->lock);
+    spin_lock(&gic.lock);
      /* Enable routing */
      GICD[GICD_ISENABLER + irq / 32] = (1u << (irq % 32));
+    desc->status &= ~IRQ_DISABLED;
+    spin_unlock(&gic.lock);
+    spin_unlock(&desc->lock);

I think we should have something like that:

desc->status &= ...
GICD[...] = ...

As soon as the interrupt is enabled in the distributor, it can be fired. With your solution, another CPU (and even this one because the interrupt is not disabled), can receive the interrupt. But it won't work because the IRQ is marked as disabled.

So you also should disable interrupt here.

Julien Grall

Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.