From patchwork Fri Sep 2 13:08:01 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Thomas Gleixner X-Patchwork-Id: 9311013 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 0075560760 for ; Fri, 2 Sep 2016 13:12:30 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id E4B63297A0 for ; Fri, 2 Sep 2016 13:12:29 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id D6F98297A9; Fri, 2 Sep 2016 13:12:29 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.2 required=2.0 tests=BAYES_00, RCVD_IN_DNSWL_MED autolearn=unavailable version=3.3.1 Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.9]) (using TLSv1.2 with cipher AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 4D13E297A0 for ; Fri, 2 Sep 2016 13:12:29 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.85_2 #1 (Red Hat Linux)) id 1bfoF8-00066w-Ul; Fri, 02 Sep 2016 13:10:50 +0000 Received: from galois.linutronix.de ([2a01:7a0:2:106d:700::1]) by bombadil.infradead.org with esmtps (Exim 4.85_2 #1 (Red Hat Linux)) id 1bfoF4-0005fi-71 for linux-arm-kernel@lists.infradead.org; Fri, 02 Sep 2016 13:10:47 +0000 Received: from localhost ([127.0.0.1]) by Galois.linutronix.de with esmtps (TLS1.2:DHE_RSA_AES_256_CBC_SHA256:256) (Exim 4.80) (envelope-from ) id 1bfoEc-0002pv-Cy; Fri, 02 Sep 2016 15:10:19 +0200 Date: Fri, 2 Sep 2016 15:08:01 +0200 (CEST) From: Thomas Gleixner To: Marc Zyngier Subject: Re: [PATCH] generic: Add the exception case checking routine for ppi interrupt In-Reply-To: <57C7EEE2.7080205@arm.com> Message-ID: References: <1472530639-21616-1-git-send-email-majun258@huawei.com> <57C548D0.3090700@arm.com> <57C5617B.6080801@huawei.com> <57C568F8.20802@arm.com> <20160830112113.GE1223@leverpostej> <57C67ABE.908@huawei.com> <57C696DA.4090301@arm.com> <57C7E3B6.1040605@huawei.com> <57C7EEE2.7080205@arm.com> User-Agent: Alpine 2.20 (DEB 67 2015-01-07) MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20160902_061046_453215_E24B70D9 X-CRM114-Status: GOOD ( 23.65 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Mark Rutland , guohanjun@huawei.com, linux-kernel@vger.kernel.org, dingtianhong@huawei.com, linux-arm-kernel@lists.infradead.org, "majun \(F\)" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Virus-Scanned: ClamAV using ClamSMTP On Thu, 1 Sep 2016, Marc Zyngier wrote: > On 01/09/16 09:15, majun (F) wrote: > Well, this issue goes way beyond the hack you wanted to add to the > generic code, and it should probably be addressed in the GIC code > itself, as an implementation specific workaround. Without knowing the > details of the erratum, it is difficult to think of that would be > required. I can come up with something like this: > > irqnr = gic_read_iar(); > if (unlikely(!is_enabled(irqnr))) { > gic_write_eoir(irqnr); > if (static_key_true(&supports_deactivate)) > gic_write_dir(irqnr); > set_pending(irqnr); > continue; > } > > Performance will suffer (an extra MMIO access on the fast path). If LPIs > are also affected, then the ITS code also needs to be involved, and > that's not going to be pretty either. This code will have to be enabled > at runtime, and handled like other erratum we have in this code. So that's certainly a required workaround at the gic level. Though I really think that we should make handle_percpu_devid_irq robust against a spurious interrupt. > void handle_percpu_devid_irq(unsigned int irq, struct irq_desc *desc) > { > - struct irq_chip *chip = irq_desc_get_chip(desc); > - struct irqaction *action = desc->action; > - void *dev_id = raw_cpu_ptr(action->percpu_dev_id); > + struct irq_chip *chip = NULL; > + struct irqaction *action; > + void *dev_id; > irqreturn_t res; > > + action = desc->action; > + > + /* Unexpected interrupt in some execption case > + * we just send eoi to end this interrupt > + */ > + if (unlikely(!action)) { > + mask_irq(desc); This is wrong. mask_irq() does not work for percpu interrupts. Aside of that this completely lacks any debug information which tells us that there is something wrong in the system. I'm going to apply the patch below for robustness sake. Thanks, tglx 8<---------------------- Subject: genirq: Robustify handle_percpu_devid_irq() From: Thomas Gleixner Date: Fri, 02 Sep 2016 14:45:19 +0200 The percpu_devid handler is not robust against spurious interrupts. If a spurious interrupt happens and no action is installed then the handler crashes with a NULL pointer dereference. Add a sanity check for this and log the wreckage once in dmesg. Reported-by: Majun Signed-off-by: Thomas Gleixner Acked-by: Marc Zyngier --- kernel/irq/chip.c | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) --- a/kernel/irq/chip.c +++ b/kernel/irq/chip.c @@ -756,7 +756,6 @@ void handle_percpu_devid_irq(struct irq_ { struct irq_chip *chip = irq_desc_get_chip(desc); struct irqaction *action = desc->action; - void *dev_id = raw_cpu_ptr(action->percpu_dev_id); unsigned int irq = irq_desc_get_irq(desc); irqreturn_t res; @@ -765,9 +764,20 @@ void handle_percpu_devid_irq(struct irq_ if (chip->irq_ack) chip->irq_ack(&desc->irq_data); - trace_irq_handler_entry(irq, action); - res = action->handler(irq, dev_id); - trace_irq_handler_exit(irq, action, res); + if (likely(action)) { + trace_irq_handler_entry(irq, action); + res = action->handler(irq, raw_cpu_ptr(action->percpu_dev_id)); + trace_irq_handler_exit(irq, action, res); + } else { + unsigned int cpu = smp_processor_id(); + bool enabled = cpumask_test_cpu(cpu, desc->percpu_enabled); + + if (enabled) + irq_percpu_disable(desc, cpu); + + pr_err_once("Spurious%s percpu IRQ%u on CPU%u\n", + enabled ? " and unmasked" : "", irq, cpu); + } if (chip->irq_eoi) chip->irq_eoi(&desc->irq_data);