[PATCH] irqchip/sifive-plic: use hartid as context_id in OF to fix AMP conflicts

[PATCH] irqchip/sifive-plic: use hartid as context_id in OF to fix AMP conflicts

[Troy Mitchell]

In asymmetric multi-processing (AMP) scenarios, the original PLIC
driver used the context loop index 'i' as context_id for OF (device
tree) platforms. This caused multiple contexts from different harts
(e.g., core0 and core4) to share the same enable_base, leading to
conflicts when initializing the PLIC.

This patch resolves enable_base conflicts on AMP platforms while
maintaining SMP/UP behavior.

Signed-off-by: Troy Mitchell <troy.mitchell@linux.dev>
---
 drivers/irqchip/irq-sifive-plic.c | 23 ++++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/drivers/irqchip/irq-sifive-plic.c b/drivers/irqchip/irq-sifive-plic.c
index cbd7697bc14819cbe3b77096b26901b605491f75..79867afcd321e42ad837adb5c15d0e1dc9e0e1b4 100644
--- a/drivers/irqchip/irq-sifive-plic.c
+++ b/drivers/irqchip/irq-sifive-plic.c
@@ -487,18 +487,18 @@ static int plic_parse_nr_irqs_and_contexts(struct fwnode_handle *fwnode,
 }
 
 static int plic_parse_context_parent(struct fwnode_handle *fwnode, u32 context,
-				     u32 *parent_hwirq, int *parent_cpu, u32 id)
+				     u32 *parent_hwirq, int *parent_cpu, u32 id,
+				     unsigned long *hartid)
 {
 	struct of_phandle_args parent;
-	unsigned long hartid;
 	int rc;
 
 	if (!is_of_node(fwnode)) {
-		hartid = acpi_rintc_ext_parent_to_hartid(id, context);
-		if (hartid == INVALID_HARTID)
+		*hartid = acpi_rintc_ext_parent_to_hartid(id, context);
+		if (*hartid == INVALID_HARTID)
 			return -EINVAL;
 
-		*parent_cpu = riscv_hartid_to_cpuid(hartid);
+		*parent_cpu = riscv_hartid_to_cpuid(*hartid);
 		*parent_hwirq = RV_IRQ_EXT;
 		return 0;
 	}
@@ -507,19 +507,19 @@ static int plic_parse_context_parent(struct fwnode_handle *fwnode, u32 context,
 	if (rc)
 		return rc;
 
-	rc = riscv_of_parent_hartid(parent.np, &hartid);
+	rc = riscv_of_parent_hartid(parent.np, hartid);
 	if (rc)
 		return rc;
 
 	*parent_hwirq = parent.args[0];
-	*parent_cpu = riscv_hartid_to_cpuid(hartid);
+	*parent_cpu = riscv_hartid_to_cpuid(*hartid);
 	return 0;
 }
 
 static int plic_probe(struct fwnode_handle *fwnode)
 {
 	int error = 0, nr_contexts, nr_handlers = 0, cpu, i;
-	unsigned long plic_quirks = 0;
+	unsigned long plic_quirks = 0, hartid;
 	struct plic_handler *handler;
 	u32 nr_irqs, parent_hwirq;
 	struct plic_priv *priv;
@@ -569,14 +569,14 @@ static int plic_probe(struct fwnode_handle *fwnode)
 
 	for (i = 0; i < nr_contexts; i++) {
 		error = plic_parse_context_parent(fwnode, i, &parent_hwirq, &cpu,
-						  priv->acpi_plic_id);
+						  priv->acpi_plic_id, &hartid);
 		if (error) {
 			pr_warn("%pfwP: hwirq for context%d not found\n", fwnode, i);
 			continue;
 		}
 
 		if (is_of_node(fwnode)) {
-			context_id = i;
+			context_id = hartid * 2 + i % 2;
 		} else {
 			context_id = acpi_rintc_get_plic_context(priv->acpi_plic_id, i);
 			if (context_id == INVALID_CONTEXT) {
@@ -694,7 +694,8 @@ static int plic_probe(struct fwnode_handle *fwnode)
 
 fail_cleanup_contexts:
 	for (i = 0; i < nr_contexts; i++) {
-		if (plic_parse_context_parent(fwnode, i, &parent_hwirq, &cpu, priv->acpi_plic_id))
+		if (plic_parse_context_parent(fwnode, i, &parent_hwirq, &cpu,
+					      priv->acpi_plic_id, &hartid))
 			continue;
 		if (parent_hwirq != RV_IRQ_EXT || cpu < 0)
 			continue;

---
base-commit: fdbc36b17e9f2fa24c940959e39df90f53ccce2b
change-id: 20251020-fix-plic-amp-72bd94969ca4

Best regards,
-- 
Troy Mitchell <troy.mitchell@linux.dev>


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH] irqchip/sifive-plic: use hartid as context_id in OF to fix AMP conflicts

[Thomas Gleixner]

On Mon, Oct 20 2025 at 11:49, Troy Mitchell wrote:
> In asymmetric multi-processing (AMP) scenarios, the original PLIC
> driver used the context loop index 'i' as context_id for OF (device

Which original driver and when did it stop to use the context loop index?

> tree) platforms. This caused multiple contexts from different harts
> (e.g., core0 and core4) to share the same enable_base, leading to
> conflicts when initializing the PLIC.

When did it stop to cause the issues? And if the issues have been
already resolved, what is this patch about?

> This patch resolves enable_base conflicts on AMP platforms while

# git grep 'This patch' Documentation/process/

> maintaining SMP/UP behavior.

There is zero explanation what this patch does to resolve the issue.

See also:

  https://www.kernel.org/doc/html/latest/process/maintainer-tip.html#changelog

>  
>  		if (is_of_node(fwnode)) {
> -			context_id = i;
> +			context_id = hartid * 2 + i % 2;

This is incomprehensible and will cause head scratching 6 weeks down the
road. This needs a proper comment with an explanation what this is
about and why it is correct under all circumstances.

Thanks,

        tglx

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH] irqchip/sifive-plic: use hartid as context_id in OF to fix AMP conflicts

[Troy Mitchell]

On Mon, Oct 20, 2025 at 07:44:55PM +0200, Thomas Gleixner wrote:
> On Mon, Oct 20 2025 at 11:49, Troy Mitchell wrote:
> > In asymmetric multi-processing (AMP) scenarios, the original PLIC
> > driver used the context loop index 'i' as context_id for OF (device
> 
> Which original driver and when did it stop to use the context loop index?
> 
> > tree) platforms. This caused multiple contexts from different harts
> > (e.g., core0 and core4) to share the same enable_base, leading to
> > conflicts when initializing the PLIC.
> 
> When did it stop to cause the issues? And if the issues have been
> already resolved, what is this patch about?
The issue still exists in the current driver.
The PLIC driver for OF-based platforms assigns context_id = i inside
the context loop, which assumes that all harts are numbered contiguously
starting from 0.

In AMP systems (e.g., when Linux boots from hart4 while hart0 runs another OS),
this assumption breaks — multiple contexts from different clusters share the same
enable_base(e.g., core4's enable_base = core0's enable_base), causing conflicts.

This patch fixes the problem by assigning context_id based on the actual hartid,
ensuring that each hart context maps to a unique enable region,
while preserving behavior on SMP and UP systems.

Does it make sense? I'll update my commit message in the next version.
> 
> > This patch resolves enable_base conflicts on AMP platforms while
> 
> # git grep 'This patch' Documentation/process/
> 
> > maintaining SMP/UP behavior.
> 
> There is zero explanation what this patch does to resolve the issue.
> 
> See also:
> 
>   https://www.kernel.org/doc/html/latest/process/maintainer-tip.html#changelog
Thanks for your link!
> 
> >  
> >  		if (is_of_node(fwnode)) {
> > -			context_id = i;
> > +			context_id = hartid * 2 + i % 2;
> 
> This is incomprehensible and will cause head scratching 6 weeks down the
> road. This needs a proper comment with an explanation what this is
> about and why it is correct under all circumstances.
I will add comments here. Thanks!

                        - Troy
> 
> Thanks,
> 
>         tglx

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH] irqchip/sifive-plic: use hartid as context_id in OF to fix AMP conflicts

[Thomas Gleixner]

On Tue, Oct 21 2025 at 09:14, Troy Mitchell wrote:
> On Mon, Oct 20, 2025 at 07:44:55PM +0200, Thomas Gleixner wrote:
>> On Mon, Oct 20 2025 at 11:49, Troy Mitchell wrote:
>> > In asymmetric multi-processing (AMP) scenarios, the original PLIC
>> > driver used the context loop index 'i' as context_id for OF (device
>> 
>> Which original driver and when did it stop to use the context loop index?
>> 
>> > tree) platforms. This caused multiple contexts from different harts
>> > (e.g., core0 and core4) to share the same enable_base, leading to
>> > conflicts when initializing the PLIC.
>> 
>> When did it stop to cause the issues? And if the issues have been
>> already resolved, what is this patch about?
> The issue still exists in the current driver.
> The PLIC driver for OF-based platforms assigns context_id = i inside
> the context loop, which assumes that all harts are numbered contiguously
> starting from 0.
>
> In AMP systems (e.g., when Linux boots from hart4 while hart0 runs another OS),
> this assumption breaks — multiple contexts from different clusters share the same
> enable_base(e.g., core4's enable_base = core0's enable_base), causing conflicts.
>
> This patch fixes the problem by assigning context_id based on the actual hartid,
> ensuring that each hart context maps to a unique enable region,
> while preserving behavior on SMP and UP systems.
>
> Does it make sense? I'll update my commit message in the next version.

Yes. Though this still applies:
 
>> # git grep 'This patch' Documentation/process/

Thanks,

        tglx

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH] irqchip/sifive-plic: use hartid as context_id in OF to fix AMP conflicts

[Troy Mitchell]

On Tue, Oct 21, 2025 at 08:23:58PM +0200, Thomas Gleixner wrote:
> On Tue, Oct 21 2025 at 09:14, Troy Mitchell wrote:
> > On Mon, Oct 20, 2025 at 07:44:55PM +0200, Thomas Gleixner wrote:
> >> On Mon, Oct 20 2025 at 11:49, Troy Mitchell wrote:
> >> > In asymmetric multi-processing (AMP) scenarios, the original PLIC
> >> > driver used the context loop index 'i' as context_id for OF (device
> >> 
> >> Which original driver and when did it stop to use the context loop index?
> >> 
> >> > tree) platforms. This caused multiple contexts from different harts
> >> > (e.g., core0 and core4) to share the same enable_base, leading to
> >> > conflicts when initializing the PLIC.
> >> 
> >> When did it stop to cause the issues? And if the issues have been
> >> already resolved, what is this patch about?
> > The issue still exists in the current driver.
> > The PLIC driver for OF-based platforms assigns context_id = i inside
> > the context loop, which assumes that all harts are numbered contiguously
> > starting from 0.
> >
> > In AMP systems (e.g., when Linux boots from hart4 while hart0 runs another OS),
> > this assumption breaks — multiple contexts from different clusters share the same
> > enable_base(e.g., core4's enable_base = core0's enable_base), causing conflicts.
> >
> > This patch fixes the problem by assigning context_id based on the actual hartid,
> > ensuring that each hart context maps to a unique enable region,
> > while preserving behavior on SMP and UP systems.
> >
> > Does it make sense? I'll update my commit message in the next version.
> 
> Yes. Though this still applies:
Thank you for your reminder again.

                - Troy
>  
> >> # git grep 'This patch' Documentation/process/
> 
> Thanks,
> 
>         tglx

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv