From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [PATCH 3/5] xen/arm: Don't reinject the IRQ if it's already in LRs Date: Tue, 25 Jun 2013 14:55:12 +0100 Message-ID: <51C9A140.80702@linaro.org> References: <1372115067-17071-1-git-send-email-julien.grall@citrix.com> <1372115067-17071-4-git-send-email-julien.grall@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" 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: patches@linaro.org, ian.campbell@citrix.com, xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org On 06/25/2013 02:24 PM, Stefano Stabellini wrote: > On Tue, 25 Jun 2013, Julien Grall wrote: >> From: Julien Grall >> >> When an IRQ, marked as IRQS_ONESHOT, is injected Linux will: >> - Disable the IRQ >> - Call the interrupt handler >> - Conditionnally enable the IRQ >> - EOI the IRQ >> >> When Linux will re-enable the IRQ, Xen will inject again the IRQ because it's >> still inflight. Therefore, LRs will contains duplicated IRQs and Xen will >> EOI it twice if it's a physical IRQ. >> >> Signed-off-by: Julien Grall >> --- >> xen/arch/arm/gic.c | 27 +++++++++++++++++++++++++++ >> xen/arch/arm/vgic.c | 3 ++- >> xen/include/asm-arm/gic.h | 3 +++ >> 3 files changed, 32 insertions(+), 1 deletion(-) >> >> diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c >> index 21575df..bf05716 100644 >> --- a/xen/arch/arm/gic.c >> +++ b/xen/arch/arm/gic.c >> @@ -570,6 +570,33 @@ int __init setup_dt_irq(const struct dt_irq *irq, struct irqaction *new) >> return rc; >> } >> >> +/* Check if an IRQ was already injected to the current VCPU */ >> +bool_t gic_irq_injected(unsigned int irq) > > Can you rename it to something more specific, like gic_irq_inlr? > >> +{ >> + bool_t found = 0; >> + int i = 0; >> + unsigned int virq; >> + >> + spin_lock_irq(&gic.lock); >> + >> + while ( (i = find_next_bit((unsigned long *)&this_cpu(lr_mask), >> + nr_lrs, i)) < nr_lrs ) >> + { >> + virq = GICH[GICH_LR + i] & GICH_LR_VIRTUAL_MASK; >> + >> + if ( virq == irq ) >> + { >> + found = 1; >> + break; >> + } >> + i++; >> + } > > Instead of reading back all the GICH_LR registers, can't just just read > the ones that have a corresponding bit set in lr_mask? It's already the case, I use find_next_bit to find the next used LRs. > Also you should be able to avoid having to read the GICH_LR registers by > simply checking if the irq is in the lr_queue list: if an irq is in > inflight but not in lr_queue, it means that it is in one of the LRs. No. When a disabled IRQ is injected to the VCPU, Xen puts it in inflight but not in lr_queue. We don't have a way to know whether the IRQ is really in LRs or not. -- Julien