Message ID | 20230926102801.1591126-1-dunaev@tecon.ru |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:cae8:0:b0:403:3b70:6f57 with SMTP id r8csp1817511vqu; Tue, 26 Sep 2023 03:48:14 -0700 (PDT) X-Google-Smtp-Source: AGHT+IEDz/ZnhYGF36Wt8Ihx1m82i/RVSHXEdKKqCwICeCPYDSJw36syJA0w3M2gVUcavqPkAtgE X-Received: by 2002:a05:6a20:429c:b0:134:d4d3:f0a8 with SMTP id o28-20020a056a20429c00b00134d4d3f0a8mr7828509pzj.3.1695725294139; Tue, 26 Sep 2023 03:48:14 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1695725294; cv=none; d=google.com; s=arc-20160816; b=ATUHutNkJ6H2xZj+quFLND5w9QoIXKP8gkKxmGOBjLv3RVs+DGPz7iT0imqhyHJ1C4 PrE1W3Kplq1V5wY+OEnB2O/I5K3yqg+eNayVJnZ/QSpqW8SBQDCD9OCRcw0N3VP/Qlad CtgOVXuPUgeUCMh3F7a5AJOw59uYADBpD0dDUBuUbM6rTzYWp2YGNNxo5RQHQlktvVk7 1Y8j1EFGjDYhUOQW1Yw4uSHEADaZwxhay4/3gez0FZm9FZJxNSLLiFhFXJoOQb+gQTUY vEVXeQRmq4HIe4HSXs9ogIMEwvvltM/QeQIfwuJeuk03RRJM3tMFG1JjnTAL4NWY8eGg lebg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:to:content-transfer-encoding:mime-version :message-id:date:subject:cc:from; bh=St/rwEgNNVfpjajXhk28jnRBR0V7n93DPQhC6p7Yz6k=; fh=ouShH18SvNcz8+UqWQ+hVw91nAHwJbhdjxRhJfe9A94=; b=UZ67sAXSvHM5MKnfM20776T9Qz9aTFmdVaouG8XIhWUVyRldZaAz1or0q3cKmFcPim roofKykaPOSBxKpK2+HZohzcDCX16ZjZwA6aL0VsOv2DHlzxDf2Ug49JrfYDY9OYFIyJ 2oPMUeJlowaMb3dmburzp1xYJjfxwXYvUHW9U/sZKsdYqYpmMGU+akwb4py8US7+s0pR dWhoyVidAbVKcHIMBDJ0fODsH1OtgJOBOvkRlV0yCAOlpzt4O9TtFIBAX73d54s4zcSD mHmRdtVWy+saTRgYHl/skDkhWjbgnLPEozQdHllGJHZKsHQIOZUHMELbZzV6Ek1eZWMO rZiQ== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=tecon.ru Received: from snail.vger.email (snail.vger.email. [2620:137:e000::3:7]) by mx.google.com with ESMTPS id kz4-20020a170902f9c400b001bda93595d9si4778707plb.604.2023.09.26.03.48.13 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 26 Sep 2023 03:48:14 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) client-ip=2620:137:e000::3:7; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=tecon.ru Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by snail.vger.email (Postfix) with ESMTP id 6FE84802DD00; Tue, 26 Sep 2023 03:33:58 -0700 (PDT) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.10 at snail.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234339AbjIZKd5 (ORCPT <rfc822;ruipengqi7@gmail.com> + 27 others); Tue, 26 Sep 2023 06:33:57 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:42122 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229445AbjIZKdz (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Tue, 26 Sep 2023 06:33:55 -0400 X-Greylist: delayed 302 seconds by postgrey-1.37 at lindbergh.monkeyblade.net; Tue, 26 Sep 2023 03:33:46 PDT Received: from mail.tecon.ru (mail.tecon.ru [82.112.190.120]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id F3793E9 for <linux-kernel@vger.kernel.org>; Tue, 26 Sep 2023 03:33:46 -0700 (PDT) From: Dmitry Dunaev <dunaev@tecon.ru> CC: <dunaich@mail.ru>, Dmitry Dunaev <dunaev@tecon.ru>, Thomas Gleixner <tglx@linutronix.de>, Marc Zyngier <maz@kernel.org>, Paul Walmsley <paul.walmsley@sifive.com>, Palmer Dabbelt <palmer@dabbelt.com>, Albert Ou <aou@eecs.berkeley.edu>, <linux-kernel@vger.kernel.org>, <linux-riscv@lists.infradead.org> Subject: [PATCH] irqchip/riscv-intc: Mark INTC nodes for secondary CPUs as initialized. Date: Tue, 26 Sep 2023 13:28:01 +0300 Message-ID: <20230926102801.1591126-1-dunaev@tecon.ru> MIME-Version: 1.0 Content-Transfer-Encoding: 7BIT Content-Type: text/plain; charset=US-ASCII X-Spam-Status: No, score=0.1 required=5.0 tests=BAYES_00,RCVD_IN_DNSWL_BLOCKED, SPF_HELO_NONE,SPF_PASS,TO_EQ_FM_DIRECT_MX autolearn=no autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net To: unlisted-recipients:; (no To-header on input) Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (snail.vger.email [0.0.0.0]); Tue, 26 Sep 2023 03:33:58 -0700 (PDT) X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1778096846425928413 X-GMAIL-MSGID: 1778096846425928413 |
Series |
irqchip/riscv-intc: Mark INTC nodes for secondary CPUs as initialized.
|
|
Commit Message
Dmitry Dunaev
Sept. 26, 2023, 10:28 a.m. UTC
The current Linux driver irq-riscv-intc initialize IRQ domain only once,
when init function called on primary hart. In other cases no IRQ domain is
created and no operation on interrupt-controller node is performed.
This is cause of that no common Linux driver can use per-cpu interrupts
mapped to several CPUs because fwnode of secondary cores INTC is not
marked as initialized. This device is always will be marked as deferred.
For example the system with devicetree
cpu0: cpu@0 {
cpu0_intc: interrupt-controller {
interrupt-controller;
compatible = riscv,cpu-intc;
};
};
cpu1: cpu@1 {
cpu1_intc: interrupt-controller {
interrupt-controller;
compatible = riscv,cpu-intc;
};
};
buserr {
compatible = riscv,buserr;
interrupts-extended = <&cpu0_intc 16 &cpu1_intc 16>;
};
will always report 'buserr' node as deferred without calling any
bus probe function.
This patch will mark all secondary nodes passed to irq-riscv-intc
driver init function as initialized to be able to act as correct
IRQ phandle node.
Signed-off-by: Dmitry Dunaev <dunaev@tecon.ru>
---
drivers/irqchip/irq-riscv-intc.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
Comments
On Tue, Sep 26, 2023 at 3:59 PM Dmitry Dunaev <dunaev@tecon.ru> wrote: > > The current Linux driver irq-riscv-intc initialize IRQ domain only once, > when init function called on primary hart. In other cases no IRQ domain is > created and no operation on interrupt-controller node is performed. > This is cause of that no common Linux driver can use per-cpu interrupts > mapped to several CPUs because fwnode of secondary cores INTC is not > marked as initialized. This device is always will be marked as deferred. > For example the system with devicetree > > cpu0: cpu@0 { > cpu0_intc: interrupt-controller { > interrupt-controller; > compatible = riscv,cpu-intc; > }; > }; > > cpu1: cpu@1 { > cpu1_intc: interrupt-controller { > interrupt-controller; > compatible = riscv,cpu-intc; > }; > }; > > buserr { > compatible = riscv,buserr; > interrupts-extended = <&cpu0_intc 16 &cpu1_intc 16>; > }; > > will always report 'buserr' node as deferred without calling any > bus probe function. > > This patch will mark all secondary nodes passed to irq-riscv-intc > driver init function as initialized to be able to act as correct > IRQ phandle node. > > Signed-off-by: Dmitry Dunaev <dunaev@tecon.ru> > --- > drivers/irqchip/irq-riscv-intc.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/drivers/irqchip/irq-riscv-intc.c b/drivers/irqchip/irq-riscv-intc.c > index 4adeee1bc391..c01a4e8d4983 100644 > --- a/drivers/irqchip/irq-riscv-intc.c > +++ b/drivers/irqchip/irq-riscv-intc.c > @@ -155,8 +155,10 @@ static int __init riscv_intc_init(struct device_node *node, > * for each INTC DT node. We only need to do INTC initialization > * for the INTC DT node belonging to boot CPU (or boot HART). > */ > - if (riscv_hartid_to_cpuid(hartid) != smp_processor_id()) > + if (riscv_hartid_to_cpuid(hartid) != smp_processor_id()) { > + fwnode_dev_initialized(of_node_to_fwnode(node), true); There is already a patch on LKML to address this. https://www.spinics.net/lists/kernel/msg4929886.html > return 0; > + } > > return riscv_intc_init_common(of_node_to_fwnode(node)); > } > @@ -179,8 +181,10 @@ static int __init riscv_intc_acpi_init(union acpi_subtable_headers *header, > * for each INTC. We only do INTC initialization > * for the INTC belonging to the boot CPU (or boot HART). > */ > - if (riscv_hartid_to_cpuid(rintc->hart_id) != smp_processor_id()) > + if (riscv_hartid_to_cpuid(rintc->hart_id) != smp_processor_id()) { > + fwnode_dev_initialized(of_node_to_fwnode(node), true); > return 0; > + } We don't need this change for ACPI because we don't have devlink dependency between devices and INTC. Regards, Anup > > fn = irq_domain_alloc_named_fwnode("RISCV-INTC"); > if (!fn) { > -- > 2.34.1 > > > _______________________________________________ > linux-riscv mailing list > linux-riscv@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-riscv
Hi Dmitry, kernel test robot noticed the following build errors: [auto build test ERROR on tip/irq/core] [also build test ERROR on linus/master v6.6-rc3 next-20230926] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Dmitry-Dunaev/irqchip-riscv-intc-Mark-INTC-nodes-for-secondary-CPUs-as-initialized/20230926-183500 base: tip/irq/core patch link: https://lore.kernel.org/r/20230926102801.1591126-1-dunaev%40tecon.ru patch subject: [PATCH] irqchip/riscv-intc: Mark INTC nodes for secondary CPUs as initialized. config: riscv-randconfig-001-20230926 (https://download.01.org/0day-ci/archive/20230927/202309270417.HR9Q4rJN-lkp@intel.com/config) compiler: riscv64-linux-gcc (GCC) 13.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20230927/202309270417.HR9Q4rJN-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202309270417.HR9Q4rJN-lkp@intel.com/ All errors (new ones prefixed by >>): drivers/irqchip/irq-riscv-intc.c: In function 'riscv_intc_acpi_init': >> drivers/irqchip/irq-riscv-intc.c:185:58: error: 'node' undeclared (first use in this function) 185 | fwnode_dev_initialized(of_node_to_fwnode(node), true); | ^~~~ drivers/irqchip/irq-riscv-intc.c:185:58: note: each undeclared identifier is reported only once for each function it appears in vim +/node +185 drivers/irqchip/irq-riscv-intc.c 169 170 static int __init riscv_intc_acpi_init(union acpi_subtable_headers *header, 171 const unsigned long end) 172 { 173 struct fwnode_handle *fn; 174 struct acpi_madt_rintc *rintc; 175 176 rintc = (struct acpi_madt_rintc *)header; 177 178 /* 179 * The ACPI MADT will have one INTC for each CPU (or HART) 180 * so riscv_intc_acpi_init() function will be called once 181 * for each INTC. We only do INTC initialization 182 * for the INTC belonging to the boot CPU (or boot HART). 183 */ 184 if (riscv_hartid_to_cpuid(rintc->hart_id) != smp_processor_id()) { > 185 fwnode_dev_initialized(of_node_to_fwnode(node), true); 186 return 0; 187 } 188 189 fn = irq_domain_alloc_named_fwnode("RISCV-INTC"); 190 if (!fn) { 191 pr_err("unable to allocate INTC FW node\n"); 192 return -ENOMEM; 193 } 194 195 return riscv_intc_init_common(fn); 196 } 197
On Tue, 26 Sep 2023 11:36:31 +0100, Anup Patel <apatel@ventanamicro.com> wrote: > > On Tue, Sep 26, 2023 at 3:59 PM Dmitry Dunaev <dunaev@tecon.ru> wrote: > > > > The current Linux driver irq-riscv-intc initialize IRQ domain only once, > > when init function called on primary hart. In other cases no IRQ domain is > > created and no operation on interrupt-controller node is performed. > > This is cause of that no common Linux driver can use per-cpu interrupts > > mapped to several CPUs because fwnode of secondary cores INTC is not > > marked as initialized. This device is always will be marked as deferred. > > For example the system with devicetree > > > > cpu0: cpu@0 { > > cpu0_intc: interrupt-controller { > > interrupt-controller; > > compatible = riscv,cpu-intc; > > }; > > }; > > > > cpu1: cpu@1 { > > cpu1_intc: interrupt-controller { > > interrupt-controller; > > compatible = riscv,cpu-intc; > > }; > > }; > > > > buserr { > > compatible = riscv,buserr; > > interrupts-extended = <&cpu0_intc 16 &cpu1_intc 16>; > > }; > > > > will always report 'buserr' node as deferred without calling any > > bus probe function. > > > > This patch will mark all secondary nodes passed to irq-riscv-intc > > driver init function as initialized to be able to act as correct > > IRQ phandle node. > > > > Signed-off-by: Dmitry Dunaev <dunaev@tecon.ru> > > --- > > drivers/irqchip/irq-riscv-intc.c | 8 ++++++-- > > 1 file changed, 6 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/irqchip/irq-riscv-intc.c b/drivers/irqchip/irq-riscv-intc.c > > index 4adeee1bc391..c01a4e8d4983 100644 > > --- a/drivers/irqchip/irq-riscv-intc.c > > +++ b/drivers/irqchip/irq-riscv-intc.c > > @@ -155,8 +155,10 @@ static int __init riscv_intc_init(struct device_node *node, > > * for each INTC DT node. We only need to do INTC initialization > > * for the INTC DT node belonging to boot CPU (or boot HART). > > */ > > - if (riscv_hartid_to_cpuid(hartid) != smp_processor_id()) > > + if (riscv_hartid_to_cpuid(hartid) != smp_processor_id()) { > > + fwnode_dev_initialized(of_node_to_fwnode(node), true); > > There is already a patch on LKML to address this. > https://www.spinics.net/lists/kernel/msg4929886.html If this is a fix, why is it buried in a huge series and not brought forward as an independent fix that needs to be picked early? M.
On Wed, Oct 4, 2023 at 3:48 PM Marc Zyngier <maz@kernel.org> wrote: > > On Tue, 26 Sep 2023 11:36:31 +0100, > Anup Patel <apatel@ventanamicro.com> wrote: > > > > On Tue, Sep 26, 2023 at 3:59 PM Dmitry Dunaev <dunaev@tecon.ru> wrote: > > > > > > The current Linux driver irq-riscv-intc initialize IRQ domain only once, > > > when init function called on primary hart. In other cases no IRQ domain is > > > created and no operation on interrupt-controller node is performed. > > > This is cause of that no common Linux driver can use per-cpu interrupts > > > mapped to several CPUs because fwnode of secondary cores INTC is not > > > marked as initialized. This device is always will be marked as deferred. > > > For example the system with devicetree > > > > > > cpu0: cpu@0 { > > > cpu0_intc: interrupt-controller { > > > interrupt-controller; > > > compatible = riscv,cpu-intc; > > > }; > > > }; > > > > > > cpu1: cpu@1 { > > > cpu1_intc: interrupt-controller { > > > interrupt-controller; > > > compatible = riscv,cpu-intc; > > > }; > > > }; > > > > > > buserr { > > > compatible = riscv,buserr; > > > interrupts-extended = <&cpu0_intc 16 &cpu1_intc 16>; > > > }; > > > > > > will always report 'buserr' node as deferred without calling any > > > bus probe function. > > > > > > This patch will mark all secondary nodes passed to irq-riscv-intc > > > driver init function as initialized to be able to act as correct > > > IRQ phandle node. > > > > > > Signed-off-by: Dmitry Dunaev <dunaev@tecon.ru> > > > --- > > > drivers/irqchip/irq-riscv-intc.c | 8 ++++++-- > > > 1 file changed, 6 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/irqchip/irq-riscv-intc.c b/drivers/irqchip/irq-riscv-intc.c > > > index 4adeee1bc391..c01a4e8d4983 100644 > > > --- a/drivers/irqchip/irq-riscv-intc.c > > > +++ b/drivers/irqchip/irq-riscv-intc.c > > > @@ -155,8 +155,10 @@ static int __init riscv_intc_init(struct device_node *node, > > > * for each INTC DT node. We only need to do INTC initialization > > > * for the INTC DT node belonging to boot CPU (or boot HART). > > > */ > > > - if (riscv_hartid_to_cpuid(hartid) != smp_processor_id()) > > > + if (riscv_hartid_to_cpuid(hartid) != smp_processor_id()) { > > > + fwnode_dev_initialized(of_node_to_fwnode(node), true); > > > > There is already a patch on LKML to address this. > > https://www.spinics.net/lists/kernel/msg4929886.html > > If this is a fix, why is it buried in a huge series and not brought > forward as an independent fix that needs to be picked early? Dmitry saw this issue in a totally different context which is not reproducible with existing DTS files in kernel sources. This issue only manifests when some platform driver DT node points to the per-HART INTC nodes. For example, RISC-V irqchip device DT nodes point to per-HART INTC nodes. Currently, all RISC-V irqchip drivers (INTC and PLIC) are probed early (not as platform drivers) so we don't see this issue with existing irqchip drivers. The Linux AIA series implements RISC-V irqchip drivers (PLIC, APLIC, and IMSIC) as regular platform drivers (like you suggested) so this issue is easily seen with Linux AIA series hence the corresponding fix is part of the Linux AIA series. (https://www.spinics.net/lists/devicetree/msg638068.html) Regards, Anup > > M. > > -- > Without deviation from the norm, progress is not possible.
On Wed, 04 Oct 2023 15:59:33 +0100, Anup Patel <apatel@ventanamicro.com> wrote: > > On Wed, Oct 4, 2023 at 3:48 PM Marc Zyngier <maz@kernel.org> wrote: > > > > On Tue, 26 Sep 2023 11:36:31 +0100, > > Anup Patel <apatel@ventanamicro.com> wrote: > > > > > > On Tue, Sep 26, 2023 at 3:59 PM Dmitry Dunaev <dunaev@tecon.ru> wrote: > > > > > > > > The current Linux driver irq-riscv-intc initialize IRQ domain only once, > > > > when init function called on primary hart. In other cases no IRQ domain is > > > > created and no operation on interrupt-controller node is performed. > > > > This is cause of that no common Linux driver can use per-cpu interrupts > > > > mapped to several CPUs because fwnode of secondary cores INTC is not > > > > marked as initialized. This device is always will be marked as deferred. > > > > For example the system with devicetree > > > > > > > > cpu0: cpu@0 { > > > > cpu0_intc: interrupt-controller { > > > > interrupt-controller; > > > > compatible = riscv,cpu-intc; > > > > }; > > > > }; > > > > > > > > cpu1: cpu@1 { > > > > cpu1_intc: interrupt-controller { > > > > interrupt-controller; > > > > compatible = riscv,cpu-intc; > > > > }; > > > > }; > > > > > > > > buserr { > > > > compatible = riscv,buserr; > > > > interrupts-extended = <&cpu0_intc 16 &cpu1_intc 16>; > > > > }; > > > > > > > > will always report 'buserr' node as deferred without calling any > > > > bus probe function. > > > > > > > > This patch will mark all secondary nodes passed to irq-riscv-intc > > > > driver init function as initialized to be able to act as correct > > > > IRQ phandle node. > > > > > > > > Signed-off-by: Dmitry Dunaev <dunaev@tecon.ru> > > > > --- > > > > drivers/irqchip/irq-riscv-intc.c | 8 ++++++-- > > > > 1 file changed, 6 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/drivers/irqchip/irq-riscv-intc.c b/drivers/irqchip/irq-riscv-intc.c > > > > index 4adeee1bc391..c01a4e8d4983 100644 > > > > --- a/drivers/irqchip/irq-riscv-intc.c > > > > +++ b/drivers/irqchip/irq-riscv-intc.c > > > > @@ -155,8 +155,10 @@ static int __init riscv_intc_init(struct device_node *node, > > > > * for each INTC DT node. We only need to do INTC initialization > > > > * for the INTC DT node belonging to boot CPU (or boot HART). > > > > */ > > > > - if (riscv_hartid_to_cpuid(hartid) != smp_processor_id()) > > > > + if (riscv_hartid_to_cpuid(hartid) != smp_processor_id()) { > > > > + fwnode_dev_initialized(of_node_to_fwnode(node), true); > > > > > > There is already a patch on LKML to address this. > > > https://www.spinics.net/lists/kernel/msg4929886.html > > > > If this is a fix, why is it buried in a huge series and not brought > > forward as an independent fix that needs to be picked early? > > Dmitry saw this issue in a totally different context which is not > reproducible with existing DTS files in kernel sources. I hope you're not suggesting that only the DTs that are present in the kernel tree are valid. Because as far as I'm concern, the DTs in the kernel tree are only some *examples*, and not a reference. I fully expect the vast majority of DTs to live *outside* of the kernel tree, provided by the firmware, and never upstreamed. Would you expect every PC vendor to upstream their ACPI tables? > This issue only manifests when some platform driver DT node > points to the per-HART INTC nodes. For example, RISC-V > irqchip device DT nodes point to per-HART INTC nodes. Is this configuration legal or not as per the DT binding? I don't see anything that suggests it isn't legal, and having per-CPU interrupts isn't exactly a new thing. > Currently, all RISC-V irqchip drivers (INTC and PLIC) are probed > early (not as platform drivers) so we don't see this issue with > existing irqchip drivers. You don't, but Dimitry does. Who wins? M.
On Wed, Oct 4, 2023 at 9:02 PM Marc Zyngier <maz@kernel.org> wrote: > > On Wed, 04 Oct 2023 15:59:33 +0100, > Anup Patel <apatel@ventanamicro.com> wrote: > > > > On Wed, Oct 4, 2023 at 3:48 PM Marc Zyngier <maz@kernel.org> wrote: > > > > > > On Tue, 26 Sep 2023 11:36:31 +0100, > > > Anup Patel <apatel@ventanamicro.com> wrote: > > > > > > > > On Tue, Sep 26, 2023 at 3:59 PM Dmitry Dunaev <dunaev@tecon.ru> wrote: > > > > > > > > > > The current Linux driver irq-riscv-intc initialize IRQ domain only once, > > > > > when init function called on primary hart. In other cases no IRQ domain is > > > > > created and no operation on interrupt-controller node is performed. > > > > > This is cause of that no common Linux driver can use per-cpu interrupts > > > > > mapped to several CPUs because fwnode of secondary cores INTC is not > > > > > marked as initialized. This device is always will be marked as deferred. > > > > > For example the system with devicetree > > > > > > > > > > cpu0: cpu@0 { > > > > > cpu0_intc: interrupt-controller { > > > > > interrupt-controller; > > > > > compatible = riscv,cpu-intc; > > > > > }; > > > > > }; > > > > > > > > > > cpu1: cpu@1 { > > > > > cpu1_intc: interrupt-controller { > > > > > interrupt-controller; > > > > > compatible = riscv,cpu-intc; > > > > > }; > > > > > }; > > > > > > > > > > buserr { > > > > > compatible = riscv,buserr; > > > > > interrupts-extended = <&cpu0_intc 16 &cpu1_intc 16>; > > > > > }; > > > > > > > > > > will always report 'buserr' node as deferred without calling any > > > > > bus probe function. > > > > > > > > > > This patch will mark all secondary nodes passed to irq-riscv-intc > > > > > driver init function as initialized to be able to act as correct > > > > > IRQ phandle node. > > > > > > > > > > Signed-off-by: Dmitry Dunaev <dunaev@tecon.ru> > > > > > --- > > > > > drivers/irqchip/irq-riscv-intc.c | 8 ++++++-- > > > > > 1 file changed, 6 insertions(+), 2 deletions(-) > > > > > > > > > > diff --git a/drivers/irqchip/irq-riscv-intc.c b/drivers/irqchip/irq-riscv-intc.c > > > > > index 4adeee1bc391..c01a4e8d4983 100644 > > > > > --- a/drivers/irqchip/irq-riscv-intc.c > > > > > +++ b/drivers/irqchip/irq-riscv-intc.c > > > > > @@ -155,8 +155,10 @@ static int __init riscv_intc_init(struct device_node *node, > > > > > * for each INTC DT node. We only need to do INTC initialization > > > > > * for the INTC DT node belonging to boot CPU (or boot HART). > > > > > */ > > > > > - if (riscv_hartid_to_cpuid(hartid) != smp_processor_id()) > > > > > + if (riscv_hartid_to_cpuid(hartid) != smp_processor_id()) { > > > > > + fwnode_dev_initialized(of_node_to_fwnode(node), true); > > > > > > > > There is already a patch on LKML to address this. > > > > https://www.spinics.net/lists/kernel/msg4929886.html > > > > > > If this is a fix, why is it buried in a huge series and not brought > > > forward as an independent fix that needs to be picked early? > > > > Dmitry saw this issue in a totally different context which is not > > reproducible with existing DTS files in kernel sources. > > I hope you're not suggesting that only the DTs that are present in the > kernel tree are valid. Because as far as I'm concern, the DTs in the > kernel tree are only some *examples*, and not a reference. I am only saying why this issue was not observed before. > > I fully expect the vast majority of DTs to live *outside* of the > kernel tree, provided by the firmware, and never upstreamed. Would you > expect every PC vendor to upstream their ACPI tables? I agree. We can't expect all vendors to submit DT to kernel sources. > > > This issue only manifests when some platform driver DT node > > points to the per-HART INTC nodes. For example, RISC-V > > irqchip device DT nodes point to per-HART INTC nodes. > > Is this configuration legal or not as per the DT binding? I don't see > anything that suggests it isn't legal, and having per-CPU interrupts > isn't exactly a new thing. This is a perfect legal configuration in the RISC-V world. We have similar DT binding for AIA drivers as well. > > > Currently, all RISC-V irqchip drivers (INTC and PLIC) are probed > > early (not as platform drivers) so we don't see this issue with > > existing irqchip drivers. > > You don't, but Dimitry does. Who wins? I am totally fine taking PATCH3 of the Linux AIA v10 series as a fix PATCH for 6.6-rcX. The PATCH3 is pretty self contained and does not depend on any other PATCH of Linux AIA v10 series. Do you want me to re-send it as an individual PATCH ? Regards, Anup > > M. > > -- > Without deviation from the norm, progress is not possible.
diff --git a/drivers/irqchip/irq-riscv-intc.c b/drivers/irqchip/irq-riscv-intc.c index 4adeee1bc391..c01a4e8d4983 100644 --- a/drivers/irqchip/irq-riscv-intc.c +++ b/drivers/irqchip/irq-riscv-intc.c @@ -155,8 +155,10 @@ static int __init riscv_intc_init(struct device_node *node, * for each INTC DT node. We only need to do INTC initialization * for the INTC DT node belonging to boot CPU (or boot HART). */ - if (riscv_hartid_to_cpuid(hartid) != smp_processor_id()) + if (riscv_hartid_to_cpuid(hartid) != smp_processor_id()) { + fwnode_dev_initialized(of_node_to_fwnode(node), true); return 0; + } return riscv_intc_init_common(of_node_to_fwnode(node)); } @@ -179,8 +181,10 @@ static int __init riscv_intc_acpi_init(union acpi_subtable_headers *header, * for each INTC. We only do INTC initialization * for the INTC belonging to the boot CPU (or boot HART). */ - if (riscv_hartid_to_cpuid(rintc->hart_id) != smp_processor_id()) + if (riscv_hartid_to_cpuid(rintc->hart_id) != smp_processor_id()) { + fwnode_dev_initialized(of_node_to_fwnode(node), true); return 0; + } fn = irq_domain_alloc_named_fwnode("RISCV-INTC"); if (!fn) {