Received: by 2002:a25:ad19:0:0:0:0:0 with SMTP id y25csp1702284ybi; Wed, 17 Jul 2019 20:24:35 -0700 (PDT) X-Google-Smtp-Source: APXvYqwMly8YRVZp35RefNkzgD31EEWKvZV75Q3WzCy8v1RdpC0rs4bUjEPmkoAwUzhYB4p3uX2G X-Received: by 2002:a17:902:1101:: with SMTP id d1mr2686486pla.212.1563420275095; Wed, 17 Jul 2019 20:24:35 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1563420275; cv=none; d=google.com; s=arc-20160816; b=goJ+hPWvTN0fcVf8DtGOUY/Sr6CcwGhJ0mnzws1jqINAEg9Jp5l8nz1KkOdR/ngxg3 AWuUFUGewSO0jpfC0v/24ukzv6KAQnSM1TAQ7c3eTjqOmEcqb2O88UN6slwQ2PfUMP3y 4p4iYisRMiEE/uYL/pQKQOJPD4C8vMlKPlKOHbDHteH1dK4wRwS3uQ3DFoDNpYlTHvyc HDI6ylf3BwgLf3Glfy+vkAJyQCbl+OhxAZcBGXDioYN9YbGL14kathQBhylDHoTVgyzK wp8xjKeolKsrX7//uYCusqZYscyFdA+Kyn6BL4Ib3KazsxmmbxnngjdkMyFpQYuKgASl wlRQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:content-transfer-encoding:mime-version :user-agent:references:in-reply-to:message-id:date:subject:cc:to :from:dkim-signature; bh=QrCfi/iALS219+KyVzRDrN4JUk6PShwRe68h/ERXJBA=; b=xxvsDFFtGJVwSo2QK0f4qCjK+zwvtqCbETbzF7pq57+Vc5z0RiYaWDLBZHcrHerQ7p 4JG08Y3Q96MSHvR19F9DIaVGeWb3oJgLxGUItO/NtXDUiHg63RQTesqMeL7/QgSoGtW9 ko645NO+mvu4rDyTXVI+xM8zBCDgMum/XXDxxexPwbOuocG/e1COahHNPhivoozyK2Ur 7/nA+EYjCgqMQMABlYZfZAXCPKCEzsQz2P8rUbymDFfrqEyhdwsRLwK3DuJyCVL76eXE NmxveIP/P2WlIMdi56bnE9xek02jpr0qZ7D/yKqM0vW0aGeEmI3DI5P4CdlhQ3HbQTOO NU2g== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@kernel.org header.s=default header.b=YEdnVRGs; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id z23si1301615pgi.116.2019.07.17.20.24.18; Wed, 17 Jul 2019 20:24:35 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; dkim=pass header.i=@kernel.org header.s=default header.b=YEdnVRGs; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2390695AbfGRDIS (ORCPT + 99 others); Wed, 17 Jul 2019 23:08:18 -0400 Received: from mail.kernel.org ([198.145.29.99]:40162 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2390665AbfGRDIQ (ORCPT ); Wed, 17 Jul 2019 23:08:16 -0400 Received: from localhost (115.42.148.210.bf.2iij.net [210.148.42.115]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id D75082173B; Thu, 18 Jul 2019 03:08:14 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1563419295; bh=uwB4vb3z/m1k+bVykQxmfhxl1uup4a6Q5Lxogz7dBKU=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=YEdnVRGsO9GYQAGTdvoz7RLWbHtqYdprglbgWhgm3pDOgU7e/lKvNTCRqD8/NT+CK vJesO8L3hgQj+QaPq2svuc23tcvsCe+ZTv5/vjgT892oUEZZBe0vPXC0uqhXKQMaqN QBnPrttXDQsk5OGNOVZ+A0Qa7DZgSrzl1ZMKqB7U= From: Greg Kroah-Hartman To: linux-kernel@vger.kernel.org Cc: Greg Kroah-Hartman , stable@vger.kernel.org, Robert Hodaszi , Thomas Gleixner , Marc Zyngier Subject: [PATCH 4.19 32/47] genirq: Add optional hardware synchronization for shutdown Date: Thu, 18 Jul 2019 12:01:46 +0900 Message-Id: <20190718030051.450770137@linuxfoundation.org> X-Mailer: git-send-email 2.22.0 In-Reply-To: <20190718030045.780672747@linuxfoundation.org> References: <20190718030045.780672747@linuxfoundation.org> User-Agent: quilt/0.66 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org From: Thomas Gleixner tglx@linutronix.de commit 62e0468650c30f0298822c580f382b16328119f6 upstream free_irq() ensures that no hardware interrupt handler is executing on a different CPU before actually releasing resources and deactivating the interrupt completely in a domain hierarchy. But that does not catch the case where the interrupt is on flight at the hardware level but not yet serviced by the target CPU. That creates an interesing race condition: CPU 0 CPU 1 IRQ CHIP interrupt is raised sent to CPU1 Unable to handle immediately (interrupts off, deep idle delay) mask() ... free() shutdown() synchronize_irq() release_resources() do_IRQ() -> resources are not available That might be harmless and just trigger a spurious interrupt warning, but some interrupt chips might get into a wedged state. Utilize the existing irq_get_irqchip_state() callback for the synchronization in free_irq(). synchronize_hardirq() is not using this mechanism as it might actually deadlock unter certain conditions, e.g. when called with interrupts disabled and the target CPU is the one on which the synchronization is invoked. synchronize_irq() uses it because that function cannot be called from non preemtible contexts as it might sleep. No functional change intended and according to Marc the existing GIC implementations where the driver supports the callback should be able to cope with that core change. Famous last words. Fixes: 464d12309e1b ("x86/vector: Switch IOAPIC to global reservation mode") Reported-by: Robert Hodaszi Signed-off-by: Thomas Gleixner Reviewed-by: Marc Zyngier Tested-by: Marc Zyngier Link: https://lkml.kernel.org/r/20190628111440.279463375@linutronix.de Signed-off-by: Greg Kroah-Hartman --- kernel/irq/internals.h | 4 ++ kernel/irq/manage.c | 75 ++++++++++++++++++++++++++++++++++++------------- 2 files changed, 60 insertions(+), 19 deletions(-) --- a/kernel/irq/internals.h +++ b/kernel/irq/internals.h @@ -95,6 +95,10 @@ static inline void irq_mark_irq(unsigned extern void irq_mark_irq(unsigned int irq); #endif +extern int __irq_get_irqchip_state(struct irq_data *data, + enum irqchip_irq_state which, + bool *state); + extern void init_kstat_irqs(struct irq_desc *desc, int node, int nr); irqreturn_t __handle_irq_event_percpu(struct irq_desc *desc, unsigned int *flags); --- a/kernel/irq/manage.c +++ b/kernel/irq/manage.c @@ -35,8 +35,9 @@ static int __init setup_forced_irqthread early_param("threadirqs", setup_forced_irqthreads); #endif -static void __synchronize_hardirq(struct irq_desc *desc) +static void __synchronize_hardirq(struct irq_desc *desc, bool sync_chip) { + struct irq_data *irqd = irq_desc_get_irq_data(desc); bool inprogress; do { @@ -52,6 +53,20 @@ static void __synchronize_hardirq(struct /* Ok, that indicated we're done: double-check carefully. */ raw_spin_lock_irqsave(&desc->lock, flags); inprogress = irqd_irq_inprogress(&desc->irq_data); + + /* + * If requested and supported, check at the chip whether it + * is in flight at the hardware level, i.e. already pending + * in a CPU and waiting for service and acknowledge. + */ + if (!inprogress && sync_chip) { + /* + * Ignore the return code. inprogress is only updated + * when the chip supports it. + */ + __irq_get_irqchip_state(irqd, IRQCHIP_STATE_ACTIVE, + &inprogress); + } raw_spin_unlock_irqrestore(&desc->lock, flags); /* Oops, that failed? */ @@ -74,13 +89,18 @@ static void __synchronize_hardirq(struct * Returns: false if a threaded handler is active. * * This function may be called - with care - from IRQ context. + * + * It does not check whether there is an interrupt in flight at the + * hardware level, but not serviced yet, as this might deadlock when + * called with interrupts disabled and the target CPU of the interrupt + * is the current CPU. */ bool synchronize_hardirq(unsigned int irq) { struct irq_desc *desc = irq_to_desc(irq); if (desc) { - __synchronize_hardirq(desc); + __synchronize_hardirq(desc, false); return !atomic_read(&desc->threads_active); } @@ -98,13 +118,17 @@ EXPORT_SYMBOL(synchronize_hardirq); * * Can only be called from preemptible code as it might sleep when * an interrupt thread is associated to @irq. + * + * It optionally makes sure (when the irq chip supports that method) + * that the interrupt is not pending in any CPU and waiting for + * service. */ void synchronize_irq(unsigned int irq) { struct irq_desc *desc = irq_to_desc(irq); if (desc) { - __synchronize_hardirq(desc); + __synchronize_hardirq(desc, true); /* * We made sure that no hardirq handler is * running. Now verify that no threaded handlers are @@ -1650,8 +1674,12 @@ static struct irqaction *__free_irq(stru unregister_handler_proc(irq, action); - /* Make sure it's not being used on another CPU: */ - synchronize_hardirq(irq); + /* + * Make sure it's not being used on another CPU and if the chip + * supports it also make sure that there is no (not yet serviced) + * interrupt in flight at the hardware level. + */ + __synchronize_hardirq(desc, true); #ifdef CONFIG_DEBUG_SHIRQ /* @@ -2184,6 +2212,28 @@ int __request_percpu_irq(unsigned int ir } EXPORT_SYMBOL_GPL(__request_percpu_irq); +int __irq_get_irqchip_state(struct irq_data *data, enum irqchip_irq_state which, + bool *state) +{ + struct irq_chip *chip; + int err = -EINVAL; + + do { + chip = irq_data_get_irq_chip(data); + if (chip->irq_get_irqchip_state) + break; +#ifdef CONFIG_IRQ_DOMAIN_HIERARCHY + data = data->parent_data; +#else + data = NULL; +#endif + } while (data); + + if (data) + err = chip->irq_get_irqchip_state(data, which, state); + return err; +} + /** * irq_get_irqchip_state - returns the irqchip state of a interrupt. * @irq: Interrupt line that is forwarded to a VM @@ -2202,7 +2252,6 @@ int irq_get_irqchip_state(unsigned int i { struct irq_desc *desc; struct irq_data *data; - struct irq_chip *chip; unsigned long flags; int err = -EINVAL; @@ -2212,19 +2261,7 @@ int irq_get_irqchip_state(unsigned int i data = irq_desc_get_irq_data(desc); - do { - chip = irq_data_get_irq_chip(data); - if (chip->irq_get_irqchip_state) - break; -#ifdef CONFIG_IRQ_DOMAIN_HIERARCHY - data = data->parent_data; -#else - data = NULL; -#endif - } while (data); - - if (data) - err = chip->irq_get_irqchip_state(data, which, state); + err = __irq_get_irqchip_state(data, which, state); irq_put_desc_busunlock(desc, flags); return err;