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

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87cyk2zpgt.fsf@all.your.base.are.belong.to.us>
Date: Mon, 14 Oct 2024 17:14:26 +0200
From: Björn Töpel <bjorn@...nel.org>
To: Sunil V L <sunilvl@...tanamicro.com>, linux-riscv@...ts.infradead.org,
 linux-kernel@...r.kernel.org
Cc: Anup Patel <anup@...infault.org>, Thomas Gleixner <tglx@...utronix.de>,
 Paul Walmsley <paul.walmsley@...ive.com>, Palmer Dabbelt
 <palmer@...belt.com>, Albert Ou <aou@...s.berkeley.edu>, "Rafael J .
 Wysocki" <rafael.j.wysocki@...el.com>, Sunil V L
 <sunilvl@...tanamicro.com>, Alexandre Ghiti <alexghiti@...osinc.com>
Subject: Re: [PATCH] irqchip/riscv-intc: Fix no-SMP boot with ACPI

Thanks for fixing this, Sunil!

Sunil V L <sunilvl@...tanamicro.com> 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@...nel.org>
> Closes: https://github.com/linux-riscv/linux-riscv/actions/runs/11280997511/job/31375229012
> Signed-off-by: Sunil V L <sunilvl@...tanamicro.com>
> ---
>  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)


Björn