| Message ID | 20230130182225.2471414-15-sunilvl@ventanamicro.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Palmer Dabbelt |
| Headers | show |
| Series | Add basic ACPI support for RISC-V | expand |
| Context | Check | Description |
|---|---|---|
| conchuod/tree_selection | fail | Failed to apply to next/pending-fixes or riscv/for-next |
Hey Sunil, Drew, @drew, a question below that I'm sorta aiming at you... On Mon, Jan 30, 2023 at 11:52:15PM +0530, Sunil V L wrote: > hartid is in RINTC structuire in MADT table. Instead of parsing Nit: missing articles before RINTC and MADT. Also typo "structure". Perhaps you'd benefit from a spell checker in your git editor. > the ACPI table every time we need for a cpu, cache it and provide > a function to read it. > > This is similar to acpi_get_madt_gicc() in arm64. -ENOTFOUND, do you mean acpi_cpu_get_madt_gicc()? > > Signed-off-by: Sunil V L <sunilvl@ventanamicro.com> > --- > arch/riscv/include/asm/acpi.h | 14 +++++++++++++- > arch/riscv/kernel/smpboot.c | 21 +++++++++++++++++++++ > 2 files changed, 34 insertions(+), 1 deletion(-) > > diff --git a/arch/riscv/include/asm/acpi.h b/arch/riscv/include/asm/acpi.h > index d1f1e53ec657..69a880b7257a 100644 > --- a/arch/riscv/include/asm/acpi.h > +++ b/arch/riscv/include/asm/acpi.h > @@ -65,6 +65,18 @@ int acpi_numa_get_nid(unsigned int cpu); > static inline int acpi_numa_get_nid(unsigned int cpu) { return NUMA_NO_NODE; } > #endif /* CONFIG_ACPI_NUMA */ > > -#endif > +struct acpi_madt_rintc *acpi_get_madt_rintc(int cpu); > +struct acpi_madt_rintc *acpi_cpu_get_madt_rintc(int cpu); > +static inline u32 get_acpi_id_for_cpu(int cpu) > +{ > + return acpi_cpu_get_madt_rintc(cpu)->uid; > +} > +#else > +static inline u32 get_acpi_id_for_cpu(int cpu) > +{ > + return -1; > +} > + > +#endif /* CONFIG_ACPI */ > > #endif /*_ASM_ACPI_H*/ > diff --git a/arch/riscv/kernel/smpboot.c b/arch/riscv/kernel/smpboot.c > index e48cf88d0bc1..3a8b7a9eb5ac 100644 > --- a/arch/riscv/kernel/smpboot.c > +++ b/arch/riscv/kernel/smpboot.c > @@ -73,6 +73,25 @@ void __init smp_prepare_cpus(unsigned int max_cpus) > > #ifdef CONFIG_ACPI > static unsigned int cpu_count = 1; > +static unsigned int intc_count; > +static struct acpi_madt_rintc cpu_madt_rintc[NR_CPUS]; > + > +struct acpi_madt_rintc *acpi_get_madt_rintc(int cpu) > +{ > + return &cpu_madt_rintc[cpu]; > +} > + > +struct acpi_madt_rintc *acpi_cpu_get_madt_rintc(int cpu) > +{ > + int i; Since we are C11 now, you don't even need to declare this outside of the loop, right? > + > + for (i = 0; i < NR_CPUS; i++) { @drew, perhaps you know since you were fiddling not too long ago with cpumask stuff - at what point does for_each_possible_cpu() become usable? I had a bit of a poke & couldn't immediately tell if it'd be okay to use it here. > + if (riscv_hartid_to_cpuid(cpu_madt_rintc[i].hart_id) == cpu) > + return &cpu_madt_rintc[i]; > + } > + return NULL; Another nit: newline before return please :) > +} > +EXPORT_SYMBOL_GPL(acpi_cpu_get_madt_rintc); > > static int __init > acpi_parse_rintc(union acpi_subtable_headers *header, > @@ -92,6 +111,8 @@ acpi_parse_rintc(union acpi_subtable_headers *header, > hart = processor->hart_id; > if (hart < 0) > return 0; > + > + cpu_madt_rintc[intc_count++] = *processor; > if (hart == cpuid_to_hartid_map(0)) { > BUG_ON(found_boot_cpu); > found_boot_cpu = 1; > -- > 2.38.0 >
On Thu, Feb 09, 2023 at 08:30:40PM +0000, Conor Dooley wrote: > Hey Sunil, Drew, > > @drew, a question below that I'm sorta aiming at you... > > On Mon, Jan 30, 2023 at 11:52:15PM +0530, Sunil V L wrote: > > hartid is in RINTC structuire in MADT table. Instead of parsing > > Nit: missing articles before RINTC and MADT. Also typo "structure". > > Perhaps you'd benefit from a spell checker in your git editor. > Okay. > > the ACPI table every time we need for a cpu, cache it and provide > > a function to read it. > > > > This is similar to acpi_get_madt_gicc() in arm64. > > -ENOTFOUND, do you mean acpi_cpu_get_madt_gicc()? > Yes. Will update. > > > > Signed-off-by: Sunil V L <sunilvl@ventanamicro.com> > > --- > > arch/riscv/include/asm/acpi.h | 14 +++++++++++++- > > arch/riscv/kernel/smpboot.c | 21 +++++++++++++++++++++ > > 2 files changed, 34 insertions(+), 1 deletion(-) > > > > diff --git a/arch/riscv/include/asm/acpi.h b/arch/riscv/include/asm/acpi.h > > index d1f1e53ec657..69a880b7257a 100644 > > --- a/arch/riscv/include/asm/acpi.h > > +++ b/arch/riscv/include/asm/acpi.h > > @@ -65,6 +65,18 @@ int acpi_numa_get_nid(unsigned int cpu); > > static inline int acpi_numa_get_nid(unsigned int cpu) { return NUMA_NO_NODE; } > > #endif /* CONFIG_ACPI_NUMA */ > > > > -#endif > > +struct acpi_madt_rintc *acpi_get_madt_rintc(int cpu); > > +struct acpi_madt_rintc *acpi_cpu_get_madt_rintc(int cpu); > > +static inline u32 get_acpi_id_for_cpu(int cpu) > > +{ > > + return acpi_cpu_get_madt_rintc(cpu)->uid; > > +} > > +#else > > +static inline u32 get_acpi_id_for_cpu(int cpu) > > +{ > > + return -1; > > +} > > + > > +#endif /* CONFIG_ACPI */ > > > > #endif /*_ASM_ACPI_H*/ > > diff --git a/arch/riscv/kernel/smpboot.c b/arch/riscv/kernel/smpboot.c > > index e48cf88d0bc1..3a8b7a9eb5ac 100644 > > --- a/arch/riscv/kernel/smpboot.c > > +++ b/arch/riscv/kernel/smpboot.c > > @@ -73,6 +73,25 @@ void __init smp_prepare_cpus(unsigned int max_cpus) > > > > #ifdef CONFIG_ACPI > > static unsigned int cpu_count = 1; > > +static unsigned int intc_count; > > +static struct acpi_madt_rintc cpu_madt_rintc[NR_CPUS]; > > + > > +struct acpi_madt_rintc *acpi_get_madt_rintc(int cpu) > > +{ > > + return &cpu_madt_rintc[cpu]; > > +} > > + > > +struct acpi_madt_rintc *acpi_cpu_get_madt_rintc(int cpu) > > +{ > > + int i; > > Since we are C11 now, you don't even need to declare this outside of the > loop, right? > Okay. > > + > > + for (i = 0; i < NR_CPUS; i++) { > > @drew, perhaps you know since you were fiddling not too long ago with > cpumask stuff - at what point does for_each_possible_cpu() become > usable? > I had a bit of a poke & couldn't immediately tell if it'd be okay to use > it here. > It should be possible. Thanks! > > + if (riscv_hartid_to_cpuid(cpu_madt_rintc[i].hart_id) == cpu) > > + return &cpu_madt_rintc[i]; > > + } > > + return NULL; > > Another nit: newline before return please :) > Sure. Thanks, Sunil
diff --git a/arch/riscv/include/asm/acpi.h b/arch/riscv/include/asm/acpi.h index d1f1e53ec657..69a880b7257a 100644 --- a/arch/riscv/include/asm/acpi.h +++ b/arch/riscv/include/asm/acpi.h @@ -65,6 +65,18 @@ int acpi_numa_get_nid(unsigned int cpu); static inline int acpi_numa_get_nid(unsigned int cpu) { return NUMA_NO_NODE; } #endif /* CONFIG_ACPI_NUMA */ -#endif +struct acpi_madt_rintc *acpi_get_madt_rintc(int cpu); +struct acpi_madt_rintc *acpi_cpu_get_madt_rintc(int cpu); +static inline u32 get_acpi_id_for_cpu(int cpu) +{ + return acpi_cpu_get_madt_rintc(cpu)->uid; +} +#else +static inline u32 get_acpi_id_for_cpu(int cpu) +{ + return -1; +} + +#endif /* CONFIG_ACPI */ #endif /*_ASM_ACPI_H*/ diff --git a/arch/riscv/kernel/smpboot.c b/arch/riscv/kernel/smpboot.c index e48cf88d0bc1..3a8b7a9eb5ac 100644 --- a/arch/riscv/kernel/smpboot.c +++ b/arch/riscv/kernel/smpboot.c @@ -73,6 +73,25 @@ void __init smp_prepare_cpus(unsigned int max_cpus) #ifdef CONFIG_ACPI static unsigned int cpu_count = 1; +static unsigned int intc_count; +static struct acpi_madt_rintc cpu_madt_rintc[NR_CPUS]; + +struct acpi_madt_rintc *acpi_get_madt_rintc(int cpu) +{ + return &cpu_madt_rintc[cpu]; +} + +struct acpi_madt_rintc *acpi_cpu_get_madt_rintc(int cpu) +{ + int i; + + for (i = 0; i < NR_CPUS; i++) { + if (riscv_hartid_to_cpuid(cpu_madt_rintc[i].hart_id) == cpu) + return &cpu_madt_rintc[i]; + } + return NULL; +} +EXPORT_SYMBOL_GPL(acpi_cpu_get_madt_rintc); static int __init acpi_parse_rintc(union acpi_subtable_headers *header, @@ -92,6 +111,8 @@ acpi_parse_rintc(union acpi_subtable_headers *header, hart = processor->hart_id; if (hart < 0) return 0; + + cpu_madt_rintc[intc_count++] = *processor; if (hart == cpuid_to_hartid_map(0)) { BUG_ON(found_boot_cpu); found_boot_cpu = 1;
hartid is in RINTC structuire in MADT table. Instead of parsing the ACPI table every time we need for a cpu, cache it and provide a function to read it. This is similar to acpi_get_madt_gicc() in arm64. Signed-off-by: Sunil V L <sunilvl@ventanamicro.com> --- arch/riscv/include/asm/acpi.h | 14 +++++++++++++- arch/riscv/kernel/smpboot.c | 21 +++++++++++++++++++++ 2 files changed, 34 insertions(+), 1 deletion(-)