[PATCH] irqchip/riscv-intc: Mark INTC nodes for secondary CPUs as initialized.

Dmitry Dunaev posted 1 patch 2 years, 2 months ago
drivers/irqchip/irq-riscv-intc.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
[PATCH] irqchip/riscv-intc: Mark INTC nodes for secondary CPUs as initialized.
Posted by Dmitry Dunaev 2 years, 2 months ago
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);
 		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) {
-- 
2.34.1
Re: [PATCH] irqchip/riscv-intc: Mark INTC nodes for secondary CPUs as initialized.
Posted by kernel test robot 2 years, 2 months ago
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	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Re: [PATCH] irqchip/riscv-intc: Mark INTC nodes for secondary CPUs as initialized.
Posted by Anup Patel 2 years, 2 months ago
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
Re: [PATCH] irqchip/riscv-intc: Mark INTC nodes for secondary CPUs as initialized.
Posted by Marc Zyngier 2 years, 2 months ago
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.

-- 
Without deviation from the norm, progress is not possible.
Re: [PATCH] irqchip/riscv-intc: Mark INTC nodes for secondary CPUs as initialized.
Posted by Anup Patel 2 years, 2 months ago
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.
Re: [PATCH] irqchip/riscv-intc: Mark INTC nodes for secondary CPUs as initialized.
Posted by Marc Zyngier 2 years, 2 months ago
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.

-- 
Without deviation from the norm, progress is not possible.
Re: [PATCH] irqchip/riscv-intc: Mark INTC nodes for secondary CPUs as initialized.
Posted by Anup Patel 2 years, 2 months ago
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.
[irqchip: irq/irqchip-fixes] irqchip/riscv-intc: Mark all INTC nodes as initialized
Posted by irqchip-bot for Anup Patel 2 years, 2 months ago
The following commit has been merged into the irq/irqchip-fixes branch of irqchip:

Commit-ID:     e13cd66bd821be417c498a34928652db4ac6b436
Gitweb:        https://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms/e13cd66bd821be417c498a34928652db4ac6b436
Author:        Anup Patel <apatel@ventanamicro.com>
AuthorDate:    Tue, 03 Oct 2023 10:13:51 +05:30
Committer:     Marc Zyngier <maz@kernel.org>
CommitterDate: Sat, 07 Oct 2023 12:47:12 +01:00

irqchip/riscv-intc: Mark all INTC nodes as initialized

The RISC-V INTC local interrupts are per-HART (or per-CPU) so we
create INTC IRQ domain only for the INTC node belonging to the boot
HART. This means only the boot HART INTC node will be marked as
initialized and other INTC nodes won't be marked which results
downstream interrupt controllers (such as PLIC, IMSIC and APLIC
direct-mode) not being probed due to missing device suppliers.

To address this issue, we mark all INTC node for which we don't
create IRQ domain as initialized.

Reported-by: Dmitry Dunaev <dunaev@tecon.ru>
Signed-off-by: Anup Patel <apatel@ventanamicro.com>
Signed-off-by: Marc Zyngier <maz@kernel.org>
Link: https://lore.kernel.org/r/20230926102801.1591126-1-dunaev@tecon.ru
Link: https://lore.kernel.org/r/20231003044403.1974628-4-apatel@ventanamicro.com
---
 drivers/irqchip/irq-riscv-intc.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/irqchip/irq-riscv-intc.c b/drivers/irqchip/irq-riscv-intc.c
index 4adeee1..e8d01b1 100644
--- a/drivers/irqchip/irq-riscv-intc.c
+++ b/drivers/irqchip/irq-riscv-intc.c
@@ -155,8 +155,16 @@ 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()) {
+		/*
+		 * The INTC nodes of each CPU are suppliers for downstream
+		 * interrupt controllers (such as PLIC, IMSIC and APLIC
+		 * direct-mode) so we should mark an INTC node as initialized
+		 * if we are not creating IRQ domain for it.
+		 */
+		fwnode_dev_initialized(of_fwnode_handle(node), true);
 		return 0;
+	}
 
 	return riscv_intc_init_common(of_node_to_fwnode(node));
 }