From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-13.1 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED, USER_AGENT_SANE_1 autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 87D97C43465 for ; Fri, 18 Sep 2020 10:00:26 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id E56B821D20 for ; Fri, 18 Sep 2020 10:00:25 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="obdf8Cb8" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org E56B821D20 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=arm.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=merlin.20170209; h=Sender:Content-Transfer-Encoding: Content-Type:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:Date:Message-ID:From: References:To:Subject:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=S944vgVeitsPUDnBmRq4HGZDXO2TbxmUHb20dv86/e0=; b=obdf8Cb8QM34YTMzsS8w7d1eZ KLoXg0gOXO72YONRP6DJmdrjRWWcTI/vMy8AQIKykEADGlG7EqQm3kjMKEBkARQ3oZmuV2l6InbGC 3EwDkJYw0YcdPSyY42B7NgZML3X0emYqOsym6vewhHc6VqI+BAZIhB9FIRjtAvCB5kprST9ay5Tn/ 3+MqmsIdd8a5YQDNTMNcOFnm5TPtEteKV3ceR40CpQiZNSa9GoF+RnIF6rsRr6n51RuCrpfCcvMpR mAQfCFo7L2JNL0O3u4avvtVrBzeW3sad3WOMb7EDUE/ejhmW2EpXNnZzxv+Hs1gALMYr7KKk6uY/T CMGZTjbbw==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1kJDAS-0001mk-IX; Fri, 18 Sep 2020 09:59:00 +0000 Received: from foss.arm.com ([217.140.110.172]) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1kJDAQ-0001lp-G2 for linux-arm-kernel@lists.infradead.org; Fri, 18 Sep 2020 09:58:59 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 31C7711D4; Fri, 18 Sep 2020 02:58:55 -0700 (PDT) Received: from [172.16.1.113] (unknown [172.31.20.19]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id F30363F73B; Fri, 18 Sep 2020 02:58:52 -0700 (PDT) Subject: Re: [PATCH v3 08/16] irqchip/gic: Configure SGIs as standard interrupts To: Marc Zyngier , jonathanh@nvidia.com References: <20200901144324.1071694-1-maz@kernel.org> <20200901144324.1071694-9-maz@kernel.org> From: James Morse Message-ID: Date: Fri, 18 Sep 2020 10:58:45 +0100 User-Agent: Mozilla/5.0 (X11; Linux aarch64; rv:68.0) Gecko/20100101 Thunderbird/68.11.0 MIME-Version: 1.0 In-Reply-To: <20200901144324.1071694-9-maz@kernel.org> Content-Language: en-GB X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20200918_055858_622310_C0A51C2E X-CRM114-Status: GOOD ( 26.14 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Sumit Garg , Florian Fainelli , Russell King , Jason Cooper , Saravana Kannan , Andrew Lunn , Catalin Marinas , Gregory Clement , linux-kernel@vger.kernel.org, Will Deacon , Thomas Gleixner , kernel-team@android.com, Valentin Schneider , linux-arm-kernel@lists.infradead.org Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Hi Marc, (CC: +Jon) On 01/09/2020 15:43, Marc Zyngier wrote: > Change the way we deal with GIC SGIs by turning them into proper > IRQs, and calling into the arch code to register the interrupt range > instead of a callback. Your comment "This only works because we don't nest SGIs..." on this thread tripped some bad memories from adding the irq-stack. Softirq causes us to nest irqs, but only once. (I've messed with the below diff to remove the added stuff:) > diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c > index 4ffd62af888f..4be2b62f816f 100644 > --- a/drivers/irqchip/irq-gic.c > +++ b/drivers/irqchip/irq-gic.c > @@ -335,31 +335,22 @@ static void __exception_irq_entry gic_handle_irq(struct pt_regs *regs) > irqstat = readl_relaxed(cpu_base + GIC_CPU_INTACK); > irqnr = irqstat & GICC_IAR_INT_ID_MASK; > > - if (likely(irqnr > 15 && irqnr < 1020)) { > - if (static_branch_likely(&supports_deactivate_key)) > - writel_relaxed(irqstat, cpu_base + GIC_CPU_EOI); > - isb(); > - handle_domain_irq(gic->domain, irqnr, regs); > - continue; > - } > - if (irqnr < 16) { > writel_relaxed(irqstat, cpu_base + GIC_CPU_EOI); > - if (static_branch_likely(&supports_deactivate_key)) > - writel_relaxed(irqstat, cpu_base + GIC_CPU_DEACTIVATE); > -#ifdef CONFIG_SMP > - /* > - * Ensure any shared data written by the CPU sending > - * the IPI is read after we've read the ACK register > - * on the GIC. > - * > - * Pairs with the write barrier in gic_raise_softirq > - */ > smp_rmb(); > - handle_IPI(irqnr, regs); If I read this right, previously we would EOI the interrupt before calling handle_IPI(). Where as now with the version of this series in your tree, we stuff the to-be-EOId value in a percpu variable, which is only safe if these don't nest. Hidden in irq_exit(), kernel/softirq.c::__irq_exit_rcu() has this: | preempt_count_sub(HARDIRQ_OFFSET); | if (!in_interrupt() && local_softirq_pending()) | invoke_softirq(); The arch code doesn't raise the preempt counter by HARDIRQ, so once __irq_exit_rcu() has dropped it, in_interrupt() returns false, and we invoke_softirq(). invoke_softirq() -> __do_softirq() -> local_irq_enable()! Fortunately, __do_softirq() raises the softirq count first using __local_bh_disable_ip(), which in-interrupt() checks too, so this can only happen once per IRQ. Now the irq_exit() has moved from handle_IPI(), which ran after EOI, into handle_domain_irq(), which runs before. I think its possible SGIs nest, and the new percpu variable becomes corrupted. Presumably this isn't a problem for regular IRQ, as they don't need the sending-CPU in order to EOI, which is why it wasn't a problem before. Adding anything to preempt-count around the whole thing upsets RCU, and softirq seems to expect this nesting, but evidently the gic does not. I'm not sure what the right thing to do would be. A dirty hack like [0] would confirm the theory. /me runs Thanks, James [0] A dirty hack -----------%<----------- diff --git a/kernel/softirq.c b/kernel/softirq.c index bf88d7f62433..50e14d8cbec3 100644 --- a/kernel/softirq.c +++ b/kernel/softirq.c @@ -376,7 +376,7 @@ static inline void invoke_softirq(void) if (ksoftirqd_running(local_softirq_pending())) return; - if (!force_irqthreads) { + if (false) { #ifdef CONFIG_HAVE_IRQ_EXIT_ON_IRQ_STACK /* * We can safely execute softirq on the current stack if @@ -393,6 +393,7 @@ static inline void invoke_softirq(void) do_softirq_own_stack(); #endif } else { + /* hack: force this */ wakeup_softirqd(); } } -----------%<----------- _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel