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 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 73288C433EF for ; Wed, 17 Nov 2021 11:01:26 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 58B5261BD3 for ; Wed, 17 Nov 2021 11:01:26 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232746AbhKQLEY (ORCPT ); Wed, 17 Nov 2021 06:04:24 -0500 Received: from mail.kernel.org ([198.145.29.99]:56570 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231307AbhKQLEX (ORCPT ); Wed, 17 Nov 2021 06:04:23 -0500 Received: from disco-boy.misterjones.org (disco-boy.misterjones.org [51.254.78.96]) (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 6F2576126A; Wed, 17 Nov 2021 11:01:25 +0000 (UTC) Received: from sofa.misterjones.org ([185.219.108.64] helo=why.misterjones.org) by disco-boy.misterjones.org with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1mnIgt-0063RW-Ii; Wed, 17 Nov 2021 11:01:23 +0000 Date: Wed, 17 Nov 2021 11:01:23 +0000 Message-ID: <87y25n6o70.wl-maz@kernel.org> From: Marc Zyngier To: Pingfan Liu Cc: linux-arm-kernel@lists.infradead.org, Catalin Marinas , Will Deacon , Mark Rutland , Joey Gouly , Sami Tolvanen , Julien Thierry , Yuichi Ito , rcu@vger.kernel.org Subject: Re: [PATCHv3 3/4] irqchip: GICv3: expose pNMI discriminator In-Reply-To: References: <20211116082450.10357-1-kernelfans@gmail.com> <20211116082450.10357-4-kernelfans@gmail.com> <878rxo8m0a.wl-maz@kernel.org> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue) FLIM-LB/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL-LB/10.8 EasyPG/1.0.0 Emacs/27.1 (x86_64-pc-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Content-Type: text/plain; charset=US-ASCII X-SA-Exim-Connect-IP: 185.219.108.64 X-SA-Exim-Rcpt-To: kernelfans@gmail.com, linux-arm-kernel@lists.infradead.org, catalin.marinas@arm.com, will@kernel.org, mark.rutland@arm.com, joey.gouly@arm.com, samitolvanen@google.com, julien.thierry@arm.com, ito-yuichi@fujitsu.com, rcu@vger.kernel.org X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on disco-boy.misterjones.org); SAEximRunCond expanded to false Precedence: bulk List-ID: X-Mailing-List: rcu@vger.kernel.org On Wed, 17 Nov 2021 10:16:53 +0000, Pingfan Liu wrote: > > On Tue, Nov 16, 2021 at 09:53:25AM +0000, Marc Zyngier wrote: > > Hi Pingfan, > > > > On Tue, 16 Nov 2021 08:24:49 +0000, > > Pingfan Liu wrote: > > > > > > Arch level code is ready to take over the nmi_enter()/nmi_exit() > > > housekeeping. > > > > > > GICv3 can expose the pNMI discriminator, then simply remove the > > > housekeeping. > > > > > > Signed-off-by: Pingfan Liu > > > Cc: Catalin Marinas > > > Cc: Will Deacon > > > Cc: Mark Rutland > > > Cc: Marc Zyngier > > > Cc: Joey Gouly > > > Cc: Sami Tolvanen > > > Cc: Julien Thierry > > > Cc: Yuichi Ito > > > Cc: rcu@vger.kernel.org > > > To: linux-arm-kernel@lists.infradead.org > > > --- > > > drivers/irqchip/irq-gic-v3.c | 18 ++++++++++++------ > > > 1 file changed, 12 insertions(+), 6 deletions(-) > > > > > > diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c > > > index daec3309b014..aa2bcb47b47e 100644 > > > --- a/drivers/irqchip/irq-gic-v3.c > > > +++ b/drivers/irqchip/irq-gic-v3.c > > > @@ -646,12 +646,8 @@ static void gic_deactivate_unhandled(u32 irqnr) > > > > > > static inline void gic_handle_nmi(u32 irqnr, struct pt_regs *regs) > > > { > > > - bool irqs_enabled = interrupts_enabled(regs); > > > int err; > > > > > > - if (irqs_enabled) > > > - nmi_enter(); > > > - > > > if (static_branch_likely(&supports_deactivate_key)) > > > gic_write_eoir(irqnr); > > > /* > > > @@ -664,8 +660,6 @@ static inline void gic_handle_nmi(u32 irqnr, struct pt_regs *regs) > > > if (err) > > > gic_deactivate_unhandled(irqnr); > > > > > > - if (irqs_enabled) > > > - nmi_exit(); > > > } > > > > > > static u32 do_read_iar(struct pt_regs *regs) > > > @@ -702,6 +696,15 @@ static u32 do_read_iar(struct pt_regs *regs) > > > return iar; > > > } > > > > > > +static bool gic_is_in_nmi(void) > > > +{ > > > + if (gic_supports_nmi() && > > > + unlikely(gic_read_rpr() == GICD_INT_RPR_PRI(GICD_INT_NMI_PRI))) > > > + return true; > > > > I don't think this fixes anything. > > > > RPR stands for 'Running Priority Register', which in GIC speak reports > > the priority of the most recently Ack'ed interrupt. > > > > You cannot use this to find out whether the interrupt that you /will/ > > ack is a NMI or not. Actually, you cannot find out about *any* > > priority until you actually ack the interrupt. What you are asking for > > is the equivalent of a crystal ball, and we're in short supply... ;-) > > > > The only case where ICC_RPR_EL1 will return something that is equal to > > GICD_INT_NMI_PRI is when you are *already* in an NMI context. So > > unless I have completely misunderstood your approach (which is always > > possible), I don't see how this can work. > > > > Thank you for the clear explanation. Also I revist this part in "GIC v3 > and v4 overview" and have a deeper understanding. (Need to spare time to > go through all later) > > You totally got my idea, and I need to find a bail-out. > > As all kinds of PIC at least have two parts of functions: active (Ack) and > deactive(EOI), is it possible to split handle_arch_irq into two parts? > I.e let irqchip expose two interfaces: > u32 (*read_irqinfo*)(struct pt_regs *regs, bool *is_nmi) > void (*handle_arch_irq)(struct pt_regs *regs, u32 irqnr) > to replace the current interface: No. There is no way we will move to such a scheme. We want to isolate the irqchip stuff in its own corner, and not propagate it into the arch code. If the pseudo-NMI is such a problem, I'm all for removing it *now*, never to come back again. > void (*handle_arch_irq)(struct pt_regs *regs) > > I have thought about such stuff for some days. And the benefits include: > -1. For this bugfix (by the parameter 'is_nmi') > -2. IPI_RESCHEDULE performance drop issue can be resolved at arch > code level. (by irqnr - ipi_irq_base == IPI_RESCHEDULE ?) > -3. The arch level can provide a similar loop as aic_handle_irq() in irq-apple-aic.c, > which can save cpu by avoiding heavy context sync when irq is > intensive. > > Do you think it is doable? I don't think this is even needed, because I don't believe that the whole thing is a real problem. In patch #1, you are claiming that handling a NMI as an IRQ first, and then upgrading to NMI once we know it really is an NMI is a problem. How different is this from an IRQ being preempted by a NMI? Your own conclusion is that the this later case isn't a problem. So why is the former an issue? I'm not saying that there is no issue at all, and it could well be that you have spotted something that I cannot see yet. But if that's the case, it means that the core code is broken as well. M. -- Without deviation from the norm, progress is not possible. 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 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 8F845C433EF for ; Wed, 17 Nov 2021 11:03:06 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (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 5986D61C12 for ; Wed, 17 Nov 2021 11:03:06 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 5986D61C12 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To: Subject:Cc:To:From:Message-ID:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=p83ybWEYmsR8FqAstfIb+uzfLhbYsiBAsfj5ba0AMog=; b=szDhx+yrC8JjHb oo2vifx3k3oXH8H6mNkgl7Qg0Bi7DfkS6iKfmUeQXV7bMD1XRFgvZg0P3VRGxIJLoKPp195okJcC7 3l4BOymlUDiSU4HgSDhFZvwfnNAeHRs4LaRzK3EMsm+TeejSWytNL3TN1jik12zl1PMpFYv2bOoca amDzxHbZEHxso9T4w6Y0ffLsWbh3NFYFP1gpxFm72PAOUJOEZ2WQ0vIBBpy8IBvPhafVsl6I+6OOt LmMrA+aK8uGCOVqSkcrYr1tlCctyzoNqLwpwvbamL0t+SzCutL/bS8GFHt5olJ7O0fO5Bjt71PHit gm4WaYhJ/q4FhKuZy7eQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1mnIh4-004Wiw-Gm; Wed, 17 Nov 2021 11:01:34 +0000 Received: from mail.kernel.org ([198.145.29.99]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1mnIgw-004Whb-3C for linux-arm-kernel@lists.infradead.org; Wed, 17 Nov 2021 11:01:27 +0000 Received: from disco-boy.misterjones.org (disco-boy.misterjones.org [51.254.78.96]) (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 6F2576126A; Wed, 17 Nov 2021 11:01:25 +0000 (UTC) Received: from sofa.misterjones.org ([185.219.108.64] helo=why.misterjones.org) by disco-boy.misterjones.org with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1mnIgt-0063RW-Ii; Wed, 17 Nov 2021 11:01:23 +0000 Date: Wed, 17 Nov 2021 11:01:23 +0000 Message-ID: <87y25n6o70.wl-maz@kernel.org> From: Marc Zyngier To: Pingfan Liu Cc: linux-arm-kernel@lists.infradead.org, Catalin Marinas , Will Deacon , Mark Rutland , Joey Gouly , Sami Tolvanen , Julien Thierry , Yuichi Ito , rcu@vger.kernel.org Subject: Re: [PATCHv3 3/4] irqchip: GICv3: expose pNMI discriminator In-Reply-To: References: <20211116082450.10357-1-kernelfans@gmail.com> <20211116082450.10357-4-kernelfans@gmail.com> <878rxo8m0a.wl-maz@kernel.org> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue) FLIM-LB/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL-LB/10.8 EasyPG/1.0.0 Emacs/27.1 (x86_64-pc-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") X-SA-Exim-Connect-IP: 185.219.108.64 X-SA-Exim-Rcpt-To: kernelfans@gmail.com, linux-arm-kernel@lists.infradead.org, catalin.marinas@arm.com, will@kernel.org, mark.rutland@arm.com, joey.gouly@arm.com, samitolvanen@google.com, julien.thierry@arm.com, ito-yuichi@fujitsu.com, rcu@vger.kernel.org X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on disco-boy.misterjones.org); SAEximRunCond expanded to false X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20211117_030126_212350_AB279689 X-CRM114-Status: GOOD ( 47.83 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , 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 On Wed, 17 Nov 2021 10:16:53 +0000, Pingfan Liu wrote: > > On Tue, Nov 16, 2021 at 09:53:25AM +0000, Marc Zyngier wrote: > > Hi Pingfan, > > > > On Tue, 16 Nov 2021 08:24:49 +0000, > > Pingfan Liu wrote: > > > > > > Arch level code is ready to take over the nmi_enter()/nmi_exit() > > > housekeeping. > > > > > > GICv3 can expose the pNMI discriminator, then simply remove the > > > housekeeping. > > > > > > Signed-off-by: Pingfan Liu > > > Cc: Catalin Marinas > > > Cc: Will Deacon > > > Cc: Mark Rutland > > > Cc: Marc Zyngier > > > Cc: Joey Gouly > > > Cc: Sami Tolvanen > > > Cc: Julien Thierry > > > Cc: Yuichi Ito > > > Cc: rcu@vger.kernel.org > > > To: linux-arm-kernel@lists.infradead.org > > > --- > > > drivers/irqchip/irq-gic-v3.c | 18 ++++++++++++------ > > > 1 file changed, 12 insertions(+), 6 deletions(-) > > > > > > diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c > > > index daec3309b014..aa2bcb47b47e 100644 > > > --- a/drivers/irqchip/irq-gic-v3.c > > > +++ b/drivers/irqchip/irq-gic-v3.c > > > @@ -646,12 +646,8 @@ static void gic_deactivate_unhandled(u32 irqnr) > > > > > > static inline void gic_handle_nmi(u32 irqnr, struct pt_regs *regs) > > > { > > > - bool irqs_enabled = interrupts_enabled(regs); > > > int err; > > > > > > - if (irqs_enabled) > > > - nmi_enter(); > > > - > > > if (static_branch_likely(&supports_deactivate_key)) > > > gic_write_eoir(irqnr); > > > /* > > > @@ -664,8 +660,6 @@ static inline void gic_handle_nmi(u32 irqnr, struct pt_regs *regs) > > > if (err) > > > gic_deactivate_unhandled(irqnr); > > > > > > - if (irqs_enabled) > > > - nmi_exit(); > > > } > > > > > > static u32 do_read_iar(struct pt_regs *regs) > > > @@ -702,6 +696,15 @@ static u32 do_read_iar(struct pt_regs *regs) > > > return iar; > > > } > > > > > > +static bool gic_is_in_nmi(void) > > > +{ > > > + if (gic_supports_nmi() && > > > + unlikely(gic_read_rpr() == GICD_INT_RPR_PRI(GICD_INT_NMI_PRI))) > > > + return true; > > > > I don't think this fixes anything. > > > > RPR stands for 'Running Priority Register', which in GIC speak reports > > the priority of the most recently Ack'ed interrupt. > > > > You cannot use this to find out whether the interrupt that you /will/ > > ack is a NMI or not. Actually, you cannot find out about *any* > > priority until you actually ack the interrupt. What you are asking for > > is the equivalent of a crystal ball, and we're in short supply... ;-) > > > > The only case where ICC_RPR_EL1 will return something that is equal to > > GICD_INT_NMI_PRI is when you are *already* in an NMI context. So > > unless I have completely misunderstood your approach (which is always > > possible), I don't see how this can work. > > > > Thank you for the clear explanation. Also I revist this part in "GIC v3 > and v4 overview" and have a deeper understanding. (Need to spare time to > go through all later) > > You totally got my idea, and I need to find a bail-out. > > As all kinds of PIC at least have two parts of functions: active (Ack) and > deactive(EOI), is it possible to split handle_arch_irq into two parts? > I.e let irqchip expose two interfaces: > u32 (*read_irqinfo*)(struct pt_regs *regs, bool *is_nmi) > void (*handle_arch_irq)(struct pt_regs *regs, u32 irqnr) > to replace the current interface: No. There is no way we will move to such a scheme. We want to isolate the irqchip stuff in its own corner, and not propagate it into the arch code. If the pseudo-NMI is such a problem, I'm all for removing it *now*, never to come back again. > void (*handle_arch_irq)(struct pt_regs *regs) > > I have thought about such stuff for some days. And the benefits include: > -1. For this bugfix (by the parameter 'is_nmi') > -2. IPI_RESCHEDULE performance drop issue can be resolved at arch > code level. (by irqnr - ipi_irq_base == IPI_RESCHEDULE ?) > -3. The arch level can provide a similar loop as aic_handle_irq() in irq-apple-aic.c, > which can save cpu by avoiding heavy context sync when irq is > intensive. > > Do you think it is doable? I don't think this is even needed, because I don't believe that the whole thing is a real problem. In patch #1, you are claiming that handling a NMI as an IRQ first, and then upgrading to NMI once we know it really is an NMI is a problem. How different is this from an IRQ being preempted by a NMI? Your own conclusion is that the this later case isn't a problem. So why is the former an issue? I'm not saying that there is no issue at all, and it could well be that you have spotted something that I cannot see yet. But if that's the case, it means that the core code is broken as well. M. -- Without deviation from the norm, progress is not possible. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel