From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [PATCH for-4.5 6/8] xen/arm: IRQ: Add lock contrainst for gic_irq_{startup, shutdown} Date: Wed, 19 Feb 2014 14:35:40 +0000 Message-ID: <5304C13C.5060505@linaro.org> References: <1390581822-32624-1-git-send-email-julien.grall@linaro.org> <1390581822-32624-7-git-send-email-julien.grall@linaro.org> <1392810675.29739.15.camel@kazak.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta14.messagelabs.com ([193.109.254.103]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1WG8FW-0004ja-5y for xen-devel@lists.xenproject.org; Wed, 19 Feb 2014 14:35:46 +0000 Received: by mail-ee0-f42.google.com with SMTP id b15so272654eek.1 for ; Wed, 19 Feb 2014 06:35:44 -0800 (PST) In-Reply-To: <1392810675.29739.15.camel@kazak.uk.xensource.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: Ian Campbell Cc: xen-devel@lists.xenproject.org, tim@xen.org, stefano.stabellini@citrix.com, patches@linaro.org List-Id: xen-devel@lists.xenproject.org Hi Ian, On 02/19/2014 11:51 AM, Ian Campbell wrote: > On Fri, 2014-01-24 at 16:43 +0000, Julien Grall wrote: >> When multiple action will be supported, gic_irq_{startup,shutdown} will have >> to be called in the same critical zone has setup/release. > > s/has/as/? Yes. >> Otherwise it could have a race condition if at the same time CPU A is calling >> release_dt_irq and CPU B is calling setup_dt_irq. >> >> This could end up to the IRQ is not enabled. > > "This could end up with the IRQ not being enabled." > >> >> Signed-off-by: Julien Grall >> --- >> xen/arch/arm/gic.c | 40 +++++++++++++++++++++++++--------------- >> 1 file changed, 25 insertions(+), 15 deletions(-) >> >> diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c >> index 2643b46..ebd2b5f 100644 >> --- a/xen/arch/arm/gic.c >> +++ b/xen/arch/arm/gic.c >> @@ -129,43 +129,53 @@ void gic_restore_state(struct vcpu *v) >> gic_restore_pending_irqs(v); >> } >> >> -static void gic_irq_enable(struct irq_desc *desc) >> +static unsigned int gic_irq_startup(struct irq_desc *desc) > > unsigned? What are the error codes here going to be? This is the return type requested by hw_interrupt_type.startup. It seems that the return is never checked (even in x86 code). Maybe we should change the prototype of hw_interrupt_type.startup. Cheers, -- Julien Grall