From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [PATCH v3 5/6] xen/arm: Only enable physical IRQs when the guest asks Date: Wed, 11 Dec 2013 13:38:31 +0000 Message-ID: <52A86AD7.6020807@linaro.org> References: <1386701456-20307-5-git-send-email-stefano.stabellini@eu.citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1386701456-20307-5-git-send-email-stefano.stabellini@eu.citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Stefano Stabellini , xen-devel@lists.xensource.com Cc: julien.grall@citrix.com, Ian.Campbell@citrix.com List-Id: xen-devel@lists.xenproject.org On 12/10/2013 06:50 PM, Stefano Stabellini wrote: > From: Julien Grall > > 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 > Signed-off-by: Julien Grall > Acked-by: Ian Campbell > > 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 &= ... dsb 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