Re: [PATCH] irqchip/riscv-intc: Fix no-SMP boot with ACPI

[Alexandre Ghiti]

Hi Sunil,

On 14/10/2024 17:14, Björn Töpel wrote:
> Thanks for fixing this, Sunil!
>
> Sunil V L <sunilvl@xxxxxxxxxxxxxxxx> writes:
>
> > When CONFIG_SMP is disabled, the static array rintc_acpi_data with size
> > NR_CPUS will not be sufficient to hold all RINTC structures passed from
> > the firmware. All RINTC structures are required to configure
> > IMSIC/APLIC/PLIC properly irrespective of SMP in the OS. So, allocate
> > dynamic memory based on the number of RINTC structures in MADT to fix
> > this issue.
> To clarify; QEMU is constructing an ACPI table for multiple harts
> (multiple RINTC), but the kernel build is NO_SMP.
>
> > Fixes: f8619b66bdb1 ("irqchip/riscv-intc: Add ACPI support for AIA")
> > Reported-by: Björn Töpel <bjorn@xxxxxxxxxx>
> > Closes: https://github.com/linux-riscv/linux-riscv/actions/runs/11280997511/job/31375229012
> > Signed-off-by: Sunil V L <sunilvl@xxxxxxxxxxxxxxxx>
> > ---
> >   drivers/irqchip/irq-riscv-intc.c | 19 ++++++++++++++++++-
> >   1 file changed, 18 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/irqchip/irq-riscv-intc.c b/drivers/irqchip/irq-riscv-intc.c
> > index 8c5411386220..f653c13de62b 100644
> > --- a/drivers/irqchip/irq-riscv-intc.c
> > +++ b/drivers/irqchip/irq-riscv-intc.c
> > @@ -265,7 +265,7 @@ struct rintc_data {
> >   };
> >   
> >   static u32 nr_rintc;
> > -static struct rintc_data *rintc_acpi_data[NR_CPUS];
> > +static struct rintc_data **rintc_acpi_data;
> >   
> >   #define for_each_matching_plic(_plic_id)				\
> >   	unsigned int _plic;						\
> > @@ -329,13 +329,30 @@ int acpi_rintc_get_imsic_mmio_info(u32 index, struct resource *res)
> >   	return 0;
> >   }
> >   
> > +static int __init riscv_intc_acpi_match(union acpi_subtable_headers *header,
> > +					const unsigned long end)
> > +{
> > +	return 0;
> > +}
> > +
> >   static int __init riscv_intc_acpi_init(union acpi_subtable_headers *header,
> >   				       const unsigned long end)
> >   {
> >   	struct acpi_madt_rintc *rintc;
> >   	struct fwnode_handle *fn;
> > +	int count;
> >   	int rc;
> >   
> > +	if (!rintc_acpi_data) {
> > +		count = acpi_table_parse_madt(ACPI_MADT_TYPE_RINTC, riscv_intc_acpi_match, 0);
> > +		if (count <= 0)
> > +			return -EINVAL;
> > +
> > +		rintc_acpi_data = kcalloc(count, sizeof(*rintc_acpi_data), GFP_KERNEL);
> > +		if (!rintc_acpi_data)
> > +			return -ENOMEM;
> > +	}
> > +
> >   	rintc = (struct acpi_madt_rintc *)header;
> >   	rintc_acpi_data[nr_rintc] = kzalloc(sizeof(*rintc_acpi_data[0]), GFP_KERNEL);
> >   	if (!rintc_acpi_data[nr_rintc])
> > --
> > 2.43.0
> For context, this is the splat:
>
>    | Unable to handle kernel paging request at virtual address ff6000000000000c
>    | Oops [#1]
>    | Modules linked in:
>    | CPU: 0 UID: 0 PID: 0 Comm: swapper Not tainted 6.12.0-rc2-g2255fe696ded #1
>    | epc : acpi_rintc_index_to_hartid+0x2c/0x3c
>    |  ra : imsic_get_parent_hartid+0x54/0xb6
>    | epc : ffffffff8046b788 ra : ffffffff80a1fe74 sp : ffffffff81403af0
>    |  gp : ffffffff815181b8 tp : ffffffff8140c780 t0 : ffffffff81403b58
>    |  t1 : 00000000000003ff t2 : 0000000000000004 s0 : ffffffff81403b90
>    |  s1 : ff60000080057060 a0 : 0000000000000008 a1 : 0000000000000000
>    |  a2 : 0000000000000000 a3 : ffffffff81403b58 a4 : ffffffff80e0dd80
>    |  a5 : ff60000000000004 a6 : 0000000000000001 a7 : 0000000000000400
>    |  s2 : ffffffff81403bb0 s3 : 0000000000000001 s4 : ff1bfffffea790e8
>    |  s5 : ffffffff81403da0 s6 : 0000000000000000 s7 : ffffffff8151c9e8
>    |  s8 : 0000000000000001 s9 : 0000000000000003 s10: 0000000000000000
>    |  s11: ffffffff81403da0 t3 : 0000000000000400 t4 : 0000000000000401
>    |  t5 : 0000000000000001 t6 : 0000000000000400
>    | status: 0000000200000100 badaddr: ff6000000000000c cause: 000000000000000d
>    | [<ffffffff8046b788>] acpi_rintc_index_to_hartid+0x2c/0x3c
>    | [<ffffffff80a20008>] imsic_setup_state+0xe8/0x83a
>    | [<ffffffff80a208f8>] imsic_early_acpi_init+0x50/0xd2
>    | [<ffffffff80a23536>] acpi_match_madt+0x2e/0x5a
>    | [<ffffffff80a1f5f4>] acpi_parse_entries_array+0x164/0x230
>    | [<ffffffff80a222ba>] acpi_table_parse_entries_array+0x86/0xc2
>    | [<ffffffff80a2234a>] acpi_table_parse_entries+0x54/0x7c
>    | [<ffffffff80a22410>] acpi_table_parse_madt+0x22/0x2a
>    | [<ffffffff80a2389c>] __acpi_probe_device_table+0x78/0xc8
>    | [<ffffffff80a1f98a>] irqchip_init+0x36/0x3e
>    | [<ffffffff80a03d2e>] init_IRQ+0x30/0x52
>    | [<ffffffff80a00c08>] start_kernel+0x4b4/0x800
>    | Code: d513 01d7 1797 010b 8793 2507 97aa 639c 60a2 6402 (6788) 0141
>    | ---[ end trace 0000000000000000 ]---
>
> (Alex just kicked a build w/ this patch)


The tests ran here 
https://github.com/linux-riscv/linux-riscv/actions/runs/11330279867/job/31507748208?pr=1587 
and there were no "nosmp" failures compared to without this patch, so 
you can add:

Tested-by: Alexandre Ghiti <alexghiti@xxxxxxxxxxxx>

Thanks,

Alex


> Björn
>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@xxxxxxxxxxxxxxxxxxx
> http://lists.infradead.org/mailman/listinfo/linux-riscv