From patchwork Tue Jul 15 17:08:28 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Daniel Thompson X-Patchwork-Id: 4556151 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork1.web.kernel.org (Postfix) with ESMTP id D18F29F295 for ; Tue, 15 Jul 2014 17:11:11 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id CF1BB200F3 for ; Tue, 15 Jul 2014 17:11:10 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.9]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id C6FBE20170 for ; Tue, 15 Jul 2014 17:11:09 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1X76EE-0007Ak-47; Tue, 15 Jul 2014 17:09:22 +0000 Received: from mail-we0-f176.google.com ([74.125.82.176]) by bombadil.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1X76EB-00071O-BZ for linux-arm-kernel@lists.infradead.org; Tue, 15 Jul 2014 17:09:20 +0000 Received: by mail-we0-f176.google.com with SMTP id q58so2499440wes.21 for ; Tue, 15 Jul 2014 10:08:56 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:message-id:date:from:user-agent:mime-version:to :cc:subject:references:in-reply-to:content-type :content-transfer-encoding; bh=P0d7vbqZrPD5wRzrC85lFA1RndfHI8yTZ7zpeB0TzZs=; b=RTQiSYIYhzB1EOcBkCF+4+WQTJVf9gGwAJHmqxDH8FG9JgRKy3BefYWu0RTC8nuQXL zxos4xNr3BRWiYJNsiNBa+duFMjA40tWUqolr1IjbJDFCpBerFI10rwZdip38EdCpwW7 Ukhn2pG/8n5AYLlSFkP7DksFppXgHHD//99Y14O8UJr7fc6SaV7eHfQcTKsz+kYoJ2LS Sub9xiYTSZl9fzqpsX56OckjTQLxuYrAfLnjukOWgY2jLvI6OR2Nrv4MsSUUyBFjD13m qtsHhDULihuXPmfJiKWChwhcfVHx+njBMtr9PeCkffzMXvlPSW8c8CY9C1YtwH398DLk AOKQ== X-Gm-Message-State: ALoCoQmUoOt2Mj4Pwpl7wtSawGshnaOSxhJHz4JE5cfCHqJnp+PJOhKSHOAE7W24tf8grJtmCs82 X-Received: by 10.180.75.230 with SMTP id f6mr7284022wiw.5.1405444136832; Tue, 15 Jul 2014 10:08:56 -0700 (PDT) Received: from harvey.bri.st.com (cpc4-aztw19-0-0-cust157.18-1.cable.virginm.net. [82.33.25.158]) by mx.google.com with ESMTPSA id s14sm11958819wij.1.2014.07.15.10.08.43 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Tue, 15 Jul 2014 10:08:55 -0700 (PDT) Message-ID: <53C5600C.1080004@linaro.org> Date: Tue, 15 Jul 2014 18:08:28 +0100 From: Daniel Thompson User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.6.0 MIME-Version: 1.0 To: Harro Haan Subject: Re: [PATCH v8 0/4] arm: KGDB NMI/FIQ support References: <1404118391-3850-1-git-send-email-daniel.thompson@linaro.org> <1404979427-12943-1-git-send-email-daniel.thompson@linaro.org> <53C4F745.3070701@linaro.org> <53C54042.2030507@linaro.org> In-Reply-To: X-Enigmail-Version: 1.6 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20140715_100919_701931_D9AA57CB X-CRM114-Status: GOOD ( 27.28 ) X-Spam-Score: -0.7 (/) Cc: linaro-kernel@lists.linaro.org, Russell King , patches@linaro.org, kgdb-bugreport@lists.sourceforge.net, Linus Walleij , Nicolas Pitre , linux-kernel@vger.kernel.org, Colin Cross , Anton Vorontsov , Ben Dooks , John Stultz , Fabio Estevam , Catalin Marinas , kernel-team@android.com, Frederic Weisbecker , Dave Martin , "linux-arm-kernel@lists.infradead.org" X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00,RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On 15/07/14 16:59, Harro Haan wrote: > On 15 July 2014 16:52, Daniel Thompson wrote: >> On 15/07/14 14:04, Harro Haan wrote: >>>> Makes sense. >>>> >>>> Avoiding this problem on GICv2 is easy (thanks to the aliased intack >>>> register) but on iMX.6 we have only a GICv1. >>>> >>>> >>>>> I can reduce the number of occurrences (not prevent it) by adding the >>>>> following hack to irq-gic.c >>>>> @@ -297,10 +309,12 @@ static asmlinkage void __exception_irq_entry >>>>> gic_handle_irq(struct pt_regs *regs >>>>> u32 irqstat, irqnr; >>>>> struct gic_chip_data *gic = &gic_data[0]; >>>>> void __iomem *cpu_base = gic_data_cpu_base(gic); >>>>> >>>>> do { >>>>> + while(readl_relaxed(gic_data_dist_base(gic) + GIC_DIST_PENDING_SET) >>>>> & (1 << 30)) >>>>> + printk(KERN_ERR "TEMP: gic_handle_irq: wait for FIQ exception\n"); >>>>> irqstat = readl_relaxed(cpu_base + GIC_CPU_INTACK); >>>>> irqnr = irqstat & ~0x1c00; >>>> >>>> I've made a more complete attempt to fix this. Could you test the >>>> following? (and be prepared to fuzz the line numbers) >>> >>> Thanks Daniel, I have tested it, see the comments below. >>> >>>> >>>> diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c >>>> index 73ae896..309bf2c 100644 >>>> --- a/drivers/irqchip/irq-gic.c >>>> +++ b/drivers/irqchip/irq-gic.c >>>> @@ -303,6 +303,28 @@ static int gic_set_wake(struct irq_data *d, >>>> unsigned int on) >>>> #define gic_set_wake NULL >>>> #endif >>>> >>>> +/* Check for group 0 interrupt spuriously acked as a normal IRQ. This >>>> + * workaround will only work for level triggered interrupts (and in >>>> + * its current form is actively harmful on systems that don't support >>>> + * FIQ). >>>> + */ >>>> +static u32 gic_handle_spurious_group_0(struct gic_chip_data *gic, u32 >>>> irqstat) >>>> +{ >>>> + u32 irqnr = irqstat & GICC_IAR_INT_ID_MASK; >>>> + >>>> + if (!gic_data_fiq_enable(gic) || irqnr >= 1021) >>>> + return irqnr; >>>> + >>>> + if (readl_relaxed(gic_data_dist_base(gic) + GIC_DIST_IGROUP + >>>> + (irqnr / 32 * 4)) & >>>> + BIT(irqnr % 32)) >>>> + return irqnr; >>>> + >>>> + /* this interrupt was spurious (needs to be handled as FIQ) */ >>>> + writel_relaxed(irqstat, gic_data_cpu_base(gic) + GIC_CPU_EOI); >>> >>> This will NOT work, because of the note I mentioned above: >>> "The FIQ exception will not occur anymore after gic_handle_irq >>> read/acknowledges the FIQ by reading the GIC_CPU_INTACK register" >>> So with this code you will say End Of Interrupt at the GIC level, >>> without actually handling the interrupt, so you are missing an >>> interrupt. >>> I did the following test to confirm the missing interrupt: >>> I have changed the periodic timer interrupt by an one-shot timer >>> interrupt. The one-shot timer interrupt is programmed by the FIQ >>> handler for the next FIQ interrupt. As expected: When the problem >>> occurs, no more FIQ interrupts are generated. >> >> Can you confirm whether your timer interrupts are configured level or >> edge triggered? Or whether EOIing the GIC causes them to be cleared by >> some other means. > > From page 48 of DDI0407I_cortex_a9_mpcore_r4p1_trm.pdf: > Watchdog timers, PPI(3) > Each Cortex-A9 processor has its own watchdog timers that can generate > interrupts, using ID30. > > From page 56: > PPI[0], [2],and[3]:b11 > interrupt is rising-edge sensitive. Thanks. This is clear. >> If we can't get level triggering to work then we have to: >> >> 1. Write code to jump correctly into FIQ mode. >> >> 2. Modify the gic's ack_fiq() callback to automatically avoid reading >> intack when the workaround is deployed. >> >> The above is why I wanted to see if we can make do with level triggering >> (and automatic re-triggering for interrupts such as SGIs that are >> cleared by EOI). > > But the re-triggering introduces extra latencies and a lot of use > cases of FIQ's try to avoid that. I'm not really clear why retriggering should be significantly more expensive than the dance required to fake up entry the FIQ handler. On the other hand retriggering allows us to avoid hacks in the FIQ handler to stop it acknowledging the interrupt. Given hacks like that won't be needed on A7/A15 this seems like a good outcome. Anyhow I've put together a new version of my earlier patch that I think will retrigger all interrupts except SGIs (I'll look at SGIs and compatibility with non-Freescale parts only if this improved approach works). Reported-by: Harro Haan Signed-off-by: Daniel Thompson --- drivers/irqchip/irq-gic.c | 31 ++++++++++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) static void __exception_irq_entry gic_handle_irq(struct pt_regs *regs) { u32 irqstat, irqnr; @@ -310,8 +337,10 @@ static void __exception_irq_entry gic_handle_irq(struct pt_regs *regs) void __iomem *cpu_base = gic_data_cpu_base(gic); do { + local_fiq_disable(); irqstat = readl_relaxed(cpu_base + GIC_CPU_INTACK); - irqnr = irqstat & GICC_IAR_INT_ID_MASK; + irqnr = gic_handle_spurious_group_0(gic, irqstat); + local_fiq_enable(); if (likely(irqnr > 15 && irqnr < 1021)) { irqnr = irq_find_mapping(gic->domain, irqnr); diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c index 73ae896..88f92e6 100644 --- a/drivers/irqchip/irq-gic.c +++ b/drivers/irqchip/irq-gic.c @@ -303,6 +303,33 @@ static int gic_set_wake(struct irq_data *d, unsigned int on) #define gic_set_wake NULL #endif +/* This is a software emulation of the Aliased Interrupt Acknowledge Register + * (GIC_AIAR) found in GICv2+. + */ +static u32 gic_handle_spurious_group_0(struct gic_chip_data *gic, u32 irqstat) +{ + u32 irqnr = irqstat & GICC_IAR_INT_ID_MASK; + void __iomem *dist_base = gic_data_dist_base(gic); + u32 offset, mask; + + if (!gic_data_fiq_enable(gic) || irqnr >= 1021) + return irqnr; + + offset = irqnr / 32 * 4; + mask = 1 << (irqnr % 32); + if (readl_relaxed(dist_base + GIC_DIST_IGROUP + offset) & mask) + return irqnr; + + /* this interrupt must be taken as a FIQ so put it back into the + * pending state and end our own servicing of it. + */ + writel_relaxed(mask, dist_base + GIC_DIST_PENDING_SET + offset); + readl_relaxed(dist_base + GIC_DIST_PENDING_SET + offset); + writel_relaxed(irqstat, gic_data_cpu_base(gic) + GIC_CPU_EOI); + + return 1023; +} +