On Fri, Mar 03, 2023 at 11:28:21PM +0530, Sunil V L wrote: > On Fri, Mar 03, 2023 at 05:05:56PM +0100, Andrew Jones wrote: > > On Fri, Mar 03, 2023 at 07:06:38PM +0530, Sunil V L wrote: > > > RINTC structures in the MADT provide mapping between the hartid > > > and the CPU. This is required many times even at run time like > > > cpuinfo. So, instead of parsing the ACPI table every time, cache > > > the RINTC structures and provide a function to get the correct > > > RINTC structure for a given cpu. > > > > > > Signed-off-by: Sunil V L <sunilvl@xxxxxxxxxxxxxxxx> > > > Acked-by: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx> > > > --- > > > arch/riscv/include/asm/acpi.h | 9 ++++++ > > > arch/riscv/kernel/acpi.c | 56 +++++++++++++++++++++++++++++++++++ > > > 2 files changed, 65 insertions(+) > > > > > > diff --git a/arch/riscv/include/asm/acpi.h b/arch/riscv/include/asm/acpi.h > > > index 111a8ed10af1..8be16c1ef7da 100644 > > > --- a/arch/riscv/include/asm/acpi.h > > > +++ b/arch/riscv/include/asm/acpi.h > > > @@ -61,6 +61,10 @@ static inline void arch_fix_phys_package_id(int num, u32 slot) { } > > > > > > int acpi_get_riscv_isa(struct acpi_table_header *table, > > > unsigned int cpu, const char **isa); > > > + > > > +struct acpi_madt_rintc *acpi_cpu_get_madt_rintc(int cpu); > > > + > > > +u32 get_acpi_id_for_cpu(int cpu); > > > #else > > > static inline int acpi_get_riscv_isa(struct acpi_table_header *table, > > > unsigned int cpu, const char **isa) > > > @@ -68,6 +72,11 @@ static inline int acpi_get_riscv_isa(struct acpi_table_header *table, > > > return -EINVAL; > > > } > > > > > > +static inline u32 get_acpi_id_for_cpu(int cpu) > > > +{ > > > + return -1; > > > +} > > > > Why do we need this stub? I wouldn't expect non-ACPI code to need an ACPI > > ID. > > > > > + > > > #endif /* CONFIG_ACPI */ > > > > > > #endif /*_ASM_ACPI_H*/ > > > diff --git a/arch/riscv/kernel/acpi.c b/arch/riscv/kernel/acpi.c > > > index 81d448c41714..8b3d68d8225f 100644 > > > --- a/arch/riscv/kernel/acpi.c > > > +++ b/arch/riscv/kernel/acpi.c > > > @@ -24,6 +24,62 @@ EXPORT_SYMBOL(acpi_disabled); > > > int acpi_pci_disabled = 1; /* skip ACPI PCI scan and IRQ initialization */ > > > EXPORT_SYMBOL(acpi_pci_disabled); > > > > > > +static struct acpi_madt_rintc cpu_madt_rintc[NR_CPUS]; > > > + > > > +static int acpi_parse_madt_rintc(union acpi_subtable_headers *header, const unsigned long end) > > > +{ > > > + struct acpi_madt_rintc *rintc = (struct acpi_madt_rintc *)header; > > > + int cpuid; > > > + > > > + if (!(rintc->flags & ACPI_MADT_ENABLED)) > > > + return 0; > > > > Why not cache the data even when its disabled? We also cache the flags so > > we can always check later too. > > > Okay, doesn't harm. > > > > + > > > + cpuid = riscv_hartid_to_cpuid(rintc->hart_id); > > > + if (cpuid >= 0 && cpuid < NR_CPUS) > > > > What does it mean for the above check to fail? Bad ACPI tables? > > > This can happen when SMP is disabled but platform has more CPUs. Ah yes, NR_CPUS can be too small for the platform. Maybe a comment explaining that we ignore all additional cpus in the ACPI tables that we can't manage with the kernel's limits would be helpful here. Thanks, drew