From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [PATCH v9 04/12] xen/arm: support HW interrupts, do not request maintenance_interrupts Date: Thu, 12 Jun 2014 22:59:39 +0100 Message-ID: <539A22CB.2050905@linaro.org> References: <1402409240-28114-4-git-send-email-stefano.stabellini@eu.citrix.com> <53985C3F.4030307@linaro.org> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: 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 Cc: julien.grall@citrix.com, xen-devel@lists.xensource.com, Ian.Campbell@citrix.com List-Id: xen-devel@lists.xenproject.org On 12/06/14 16:57, Stefano Stabellini wrote: > On Wed, 11 Jun 2014, Julien Grall wrote: >> Hi Stefano, >> >> On 06/10/2014 03:07 PM, Stefano Stabellini wrote: >>> +static void gic_update_one_lr(struct vcpu *v, int i) >>> +{ >>> + struct pending_irq *p; >>> + uint32_t lr; >>> + int irq; >>> + >>> + ASSERT(spin_is_locked(&v->arch.vgic.lock)); >>> + >>> + lr = GICH[GICH_LR + i]; >>> + if ( !(lr & (GICH_LR_PENDING|GICH_LR_ACTIVE)) ) >>> + { >>> + GICH[GICH_LR + i] = 0; >>> + clear_bit(i, &this_cpu(lr_mask)); >>> + >>> + irq = (lr >> GICH_LR_VIRTUAL_SHIFT) & GICH_LR_VIRTUAL_MASK; >>> + p = irq_to_pending(v, irq); >>> + if ( p->desc != NULL ) >>> + p->desc->status &= ~IRQ_INPROGRESS; >> >> Reading again this patch... shouldn't we take the desc->lock here? >> >> It's possible to receive the same interrupt while we update the LRs >> (depending how the IRQ has been physically route) so we may overwrite >> the IRQ_INPROGRESS bit. > > It is not possible, because we have interrupts disabled at this point. > A later patch introduce > > ASSERT(!local_irq_is_enabled()); > > at the beginning of gic_update_one_lr. You are assuming this IRQ is routed to the current pCPU. It's not always that case, at least without your GICD_ITARGETSR. Regards, -- Julien Grall