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: 33678 Return-Path: X-Original-To: linaro@patches.linaro.org Delivered-To: linaro@patches.linaro.org Received: from mail-ie0-f199.google.com (mail-ie0-f199.google.com [209.85.223.199]) by ip-10-151-82-157.ec2.internal (Postfix) with ESMTPS id C848A201F1 for ; Tue, 15 Jul 2014 17:08:59 +0000 (UTC) Received: by mail-ie0-f199.google.com with SMTP id tr6sf33187681ieb.6 for ; Tue, 15 Jul 2014 10:08:59 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:delivered-to:message-id:date:from:user-agent :mime-version:to:cc:subject:references:in-reply-to:x-original-sender :x-original-authentication-results:precedence:mailing-list:list-id :list-post:list-help:list-archive:list-unsubscribe:content-type :content-transfer-encoding; bh=P0d7vbqZrPD5wRzrC85lFA1RndfHI8yTZ7zpeB0TzZs=; b=fnO3TGXRmZPdGsGSiAEY2DRQKRhXuUleH07pd568no7I6nOlgBl9HbhLVFSwrK4El/ 5ZPp5fXyFKQOkym43diwI7xLkQnXyFsDEOCy4yR/71rBoX0h1NdHqK3YltsZX/2h1t7n 0luG4cKxOqmWxQQ/PBW0Hu2Gp7MrWV/fbHFvzCjq+dK0p3Fau3ThAkfO/IzINA+X+K3d ml4g8sEGdJaGYU7O8tO+8d8yKclVsWuZxJVZh/PshGu8Okg0FgM5o46LmmLIj3HT6WFe OttHxgjG+RZqMlfs+j6mJGy2tK5fFipUE1avg9pmPsZnUNIIFkHOa0f6NoaMdR+CsOvr EvOw== X-Gm-Message-State: ALoCoQmoDFJONLMY1BBKE8jJw8zxXH+ChgL+VaR73oNWYKwLaRwObjDpyX8sMM3bmAmCy9SeZ/qO X-Received: by 10.182.119.129 with SMTP id ku1mr12109830obb.20.1405444139221; Tue, 15 Jul 2014 10:08:59 -0700 (PDT) X-BeenThere: patchwork-forward@linaro.org Received: by 10.140.30.36 with SMTP id c33ls178482qgc.72.gmail; Tue, 15 Jul 2014 10:08:59 -0700 (PDT) X-Received: by 10.52.157.41 with SMTP id wj9mr19387585vdb.1.1405444139128; Tue, 15 Jul 2014 10:08:59 -0700 (PDT) Received: from mail-vc0-f171.google.com (mail-vc0-f171.google.com [209.85.220.171]) by mx.google.com with ESMTPS id cj10si7040340vdc.68.2014.07.15.10.08.59 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Tue, 15 Jul 2014 10:08:59 -0700 (PDT) Received-SPF: pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 209.85.220.171 as permitted sender) client-ip=209.85.220.171; Received: by mail-vc0-f171.google.com with SMTP id id10so10858243vcb.16 for ; Tue, 15 Jul 2014 10:08:59 -0700 (PDT) X-Received: by 10.52.23.71 with SMTP id k7mr11907010vdf.27.1405444139039; Tue, 15 Jul 2014 10:08:59 -0700 (PDT) X-Forwarded-To: patchwork-forward@linaro.org X-Forwarded-For: patch@linaro.org patchwork-forward@linaro.org Delivered-To: patches@linaro.org Received: by 10.221.37.5 with SMTP id tc5csp230962vcb; Tue, 15 Jul 2014 10:08:58 -0700 (PDT) X-Received: by 10.180.105.202 with SMTP id go10mr7184018wib.66.1405444137564; Tue, 15 Jul 2014 10:08:57 -0700 (PDT) Received: from mail-wi0-f179.google.com (mail-wi0-f179.google.com [209.85.212.179]) by mx.google.com with ESMTPS id r10si16484249wiw.102.2014.07.15.10.08.56 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Tue, 15 Jul 2014 10:08:57 -0700 (PDT) Received-SPF: pass (google.com: domain of daniel.thompson@linaro.org designates 209.85.212.179 as permitted sender) client-ip=209.85.212.179; Received: by mail-wi0-f179.google.com with SMTP id f8so3791543wiw.12 for ; Tue, 15 Jul 2014 10:08:56 -0700 (PDT) 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 CC: Russell King , linaro-kernel@lists.linaro.org, Catalin Marinas , patches@linaro.org, kgdb-bugreport@lists.sourceforge.net, Linus Walleij , Nicolas Pitre , linux-kernel@vger.kernel.org, Frederic Weisbecker , Anton Vorontsov , Ben Dooks , John Stultz , Fabio Estevam , Colin Cross , kernel-team@android.com, Dave Martin , "linux-arm-kernel@lists.infradead.org" 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-Removed-Original-Auth: Dkim didn't pass. X-Original-Sender: daniel.thompson@linaro.org X-Original-Authentication-Results: mx.google.com; spf=pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 209.85.220.171 as permitted sender) smtp.mail=patch+caf_=patchwork-forward=linaro.org@linaro.org Precedence: list Mailing-list: list patchwork-forward@linaro.org; contact patchwork-forward+owners@linaro.org List-ID: X-Google-Group-Id: 836684582541 List-Post: , List-Help: , List-Archive: List-Unsubscribe: , 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; +} +