| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAK9=C2XSmeCQLrQY51jJRD+Mnph4a0h8BiPEEaFai+5jBC6jHA@mail.gmail.com>
Date: Thu, 13 Nov 2025 20:48:11 +0530
From: Anup Patel <apatel@...tanamicro.com>
To: Troy Mitchell <troy.mitchell@...ux.dev>
Cc: Thomas Gleixner <tglx@...utronix.de>, Paul Walmsley <pjw@...nel.org>,
Samuel Holland <samuel.holland@...ive.com>, Palmer Dabbelt <palmer@...belt.com>,
Albert Ou <aou@...s.berkeley.edu>, Alexandre Ghiti <alex@...ti.fr>, linux-kernel@...r.kernel.org,
linux-riscv@...ts.infradead.org
Subject: Re: [PATCH v2] riscv/plic: assign context ID based on hartid
On Mon, Oct 27, 2025 at 12:35 PM Troy Mitchell <troy.mitchell@...ux.dev> wrote:
>
> The PLIC driver for OF-based platforms currently assigns 'context_id = i'
> within the context loop. This implies an assumption that all harts are
> numbered contiguously starting from 0.
>
> In Asymmetric Multi-Processing (AMP) systems, where Linux might boot on
> a non-zero hart ID (e.g., hart4), while other harts (e.g., hart0) are
> running a different OS, this assumption is violated. This can lead to
> different system inadvertently sharing the same
> PLIC enable_base register. Consequently, this causes configuration
> conflicts and incorrect interrupt handling.
>
> Assign the PLIC context ID based on the actual hart ID provided by the
> OF node. This ensures that each hart context maps to a unique enable
> region within the PLIC, thereby resolving conflicts in AMP setups. This
> change preserves the correct behavior on Symmetric Multi-Processing (SMP)
> and Uniprocessor (UP) systems.
>
> Signed-off-by: Troy Mitchell <troy.mitchell@...ux.dev>
> ---
> Changelog in v2:
> - add comments
> - modify commit message
> - use `context_id` instead of `i` when skip contexts other than external
> interrupts for our privilege level
> - Link to v1: https://lore.kernel.org/r/20251020-fix-plic-amp-v1-1-defe2a99ab80@linux.dev
> ---
> drivers/irqchip/irq-sifive-plic.c | 26 ++++++++++++++------------
> 1 file changed, 14 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/irqchip/irq-sifive-plic.c b/drivers/irqchip/irq-sifive-plic.c
> index cbd7697bc14819cbe3b77096b26901b605491f75..2289eb2f77bbd0da460d22ad3ffcd3e7ef2bde40 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,15 @@ 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;
> + /* each hart has two contexts: M-mode and S-mode */
> + context_id = hartid * 2 + i % 2;
Deriving PLIC context_id from hartid is totally broken because there
is simply no relation between context_id and hartid and PLIC spec
does not claim any such relation.
For example, in SiFive SoCs with one E-core and multiple S-cores.
The E-core only has M-mode so there is only M-mode PLIC context
for the E-core.
Due to this reason, the "interrupts-extended" property has one
entry for PLIC context.
To address the AMP use-case, you can simply disable the HARTs
not available in a partition. For example, if you have two partitions
(A: hart0, hart1 and B: hart2, hart3) then the FDT passed to both
partitions will have same PLIC DT node but partition A will have
hart2 and hart3 DT nodes disabled whereas partition B will have
hart0 and hart1 DT nodes disabled.
> } else {
> context_id = acpi_rintc_get_plic_context(priv->acpi_plic_id, i);
> if (context_id == INVALID_CONTEXT) {
> @@ -594,7 +595,7 @@ static int plic_probe(struct fwnode_handle *fwnode)
> if (IS_ENABLED(CONFIG_RISCV_M_MODE)) {
> void __iomem *enable_base = priv->regs +
> CONTEXT_ENABLE_BASE +
> - i * CONTEXT_ENABLE_SIZE;
> + context_id * CONTEXT_ENABLE_SIZE;
>
> for (hwirq = 1; hwirq <= nr_irqs; hwirq++)
> __plic_toggle(enable_base, hwirq, 0);
> @@ -694,7 +695,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@...ux.dev>
>
>
Regards,
Anup
Powered by blists - more mailing lists