| Message ID | 20240426135126.12802-6-Jonathan.Cameron@huawei.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | ACPI/arm64: add support for virtual cpu hotplug | expand |
On 4/26/24 23:51, Jonathan Cameron wrote: > If CONFIG_ACPI_PROCESSOR provide a helper to retrieve the > acpi_handle for a given CPU allowing access to methods > in DSDT. > > Tested-by: Miguel Luis <miguel.luis@oracle.com> > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > > --- > v8: Code simplification suggested by Rafael. > Fixup ;; spotted by Gavin > --- > drivers/acpi/acpi_processor.c | 11 +++++++++++ > include/linux/acpi.h | 7 +++++++ > 2 files changed, 18 insertions(+) > Reviewed-by: Gavin Shan <gshan@redhat.com> > diff --git a/drivers/acpi/acpi_processor.c b/drivers/acpi/acpi_processor.c > index 3b180e21f325..ecc2721fecae 100644 > --- a/drivers/acpi/acpi_processor.c > +++ b/drivers/acpi/acpi_processor.c > @@ -35,6 +35,17 @@ EXPORT_PER_CPU_SYMBOL(processors); > struct acpi_processor_errata errata __read_mostly; > EXPORT_SYMBOL_GPL(errata); > > +acpi_handle acpi_get_processor_handle(int cpu) > +{ > + struct acpi_processor *pr; > + > + pr = per_cpu(processors, cpu); > + if (pr) > + return pr->handle; > + > + return NULL; > +} > + Maybe it's worthy to have more check here, something like below. However, it's also fine without the extra check. if (cpu >= nr_cpu_ids || !cpu_possible(cpu)) return NULL; > static int acpi_processor_errata_piix4(struct pci_dev *dev) > { > u8 value1 = 0; > diff --git a/include/linux/acpi.h b/include/linux/acpi.h > index 34829f2c517a..9844a3f9c4e5 100644 > --- a/include/linux/acpi.h > +++ b/include/linux/acpi.h > @@ -309,6 +309,8 @@ int acpi_map_cpu(acpi_handle handle, phys_cpuid_t physid, u32 acpi_id, > int acpi_unmap_cpu(int cpu); > #endif /* CONFIG_ACPI_HOTPLUG_CPU */ > > +acpi_handle acpi_get_processor_handle(int cpu); > + > #ifdef CONFIG_ACPI_HOTPLUG_IOAPIC > int acpi_get_ioapic_id(acpi_handle handle, u32 gsi_base, u64 *phys_addr); > #endif > @@ -1077,6 +1079,11 @@ static inline bool acpi_sleep_state_supported(u8 sleep_state) > return false; > } > > +static inline acpi_handle acpi_get_processor_handle(int cpu) > +{ > + return NULL; > +} > + > #endif /* !CONFIG_ACPI */ > > extern void arch_post_acpi_subsys_init(void); Thanks, Gavin
On Tue, 30 Apr 2024 14:26:06 +1000 Gavin Shan <gshan@redhat.com> wrote: > On 4/26/24 23:51, Jonathan Cameron wrote: > > If CONFIG_ACPI_PROCESSOR provide a helper to retrieve the > > acpi_handle for a given CPU allowing access to methods > > in DSDT. > > > > Tested-by: Miguel Luis <miguel.luis@oracle.com> > > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > > > > --- > > v8: Code simplification suggested by Rafael. > > Fixup ;; spotted by Gavin > > --- > > drivers/acpi/acpi_processor.c | 11 +++++++++++ > > include/linux/acpi.h | 7 +++++++ > > 2 files changed, 18 insertions(+) > > > > Reviewed-by: Gavin Shan <gshan@redhat.com> > Thanks, > > diff --git a/drivers/acpi/acpi_processor.c b/drivers/acpi/acpi_processor.c > > index 3b180e21f325..ecc2721fecae 100644 > > --- a/drivers/acpi/acpi_processor.c > > +++ b/drivers/acpi/acpi_processor.c > > @@ -35,6 +35,17 @@ EXPORT_PER_CPU_SYMBOL(processors); > > struct acpi_processor_errata errata __read_mostly; > > EXPORT_SYMBOL_GPL(errata); > > > > +acpi_handle acpi_get_processor_handle(int cpu) > > +{ > > + struct acpi_processor *pr; > > + > > + pr = per_cpu(processors, cpu); > > + if (pr) > > + return pr->handle; > > + > > + return NULL; > > +} > > + > > Maybe it's worthy to have more check here, something like below. > However, it's also fine without the extra check. We could harden this, but for now the call sites are only int arch_(un)register_cpu() so if we get there with either of these failing something went very wrong. Maybe if it gets used more widely this defense would be wise. Jonathan > > if (cpu >= nr_cpu_ids || !cpu_possible(cpu)) > return NULL; > > > static int acpi_processor_errata_piix4(struct pci_dev *dev) > > { > > u8 value1 = 0; > > diff --git a/include/linux/acpi.h b/include/linux/acpi.h > > index 34829f2c517a..9844a3f9c4e5 100644 > > --- a/include/linux/acpi.h > > +++ b/include/linux/acpi.h > > @@ -309,6 +309,8 @@ int acpi_map_cpu(acpi_handle handle, phys_cpuid_t physid, u32 acpi_id, > > int acpi_unmap_cpu(int cpu); > > #endif /* CONFIG_ACPI_HOTPLUG_CPU */ > > > > +acpi_handle acpi_get_processor_handle(int cpu); > > + > > #ifdef CONFIG_ACPI_HOTPLUG_IOAPIC > > int acpi_get_ioapic_id(acpi_handle handle, u32 gsi_base, u64 *phys_addr); > > #endif > > @@ -1077,6 +1079,11 @@ static inline bool acpi_sleep_state_supported(u8 sleep_state) > > return false; > > } > > > > +static inline acpi_handle acpi_get_processor_handle(int cpu) > > +{ > > + return NULL; > > +} > > + > > #endif /* !CONFIG_ACPI */ > > > > extern void arch_post_acpi_subsys_init(void); > > Thanks, > Gavin >
diff --git a/drivers/acpi/acpi_processor.c b/drivers/acpi/acpi_processor.c index 3b180e21f325..ecc2721fecae 100644 --- a/drivers/acpi/acpi_processor.c +++ b/drivers/acpi/acpi_processor.c @@ -35,6 +35,17 @@ EXPORT_PER_CPU_SYMBOL(processors); struct acpi_processor_errata errata __read_mostly; EXPORT_SYMBOL_GPL(errata); +acpi_handle acpi_get_processor_handle(int cpu) +{ + struct acpi_processor *pr; + + pr = per_cpu(processors, cpu); + if (pr) + return pr->handle; + + return NULL; +} + static int acpi_processor_errata_piix4(struct pci_dev *dev) { u8 value1 = 0; diff --git a/include/linux/acpi.h b/include/linux/acpi.h index 34829f2c517a..9844a3f9c4e5 100644 --- a/include/linux/acpi.h +++ b/include/linux/acpi.h @@ -309,6 +309,8 @@ int acpi_map_cpu(acpi_handle handle, phys_cpuid_t physid, u32 acpi_id, int acpi_unmap_cpu(int cpu); #endif /* CONFIG_ACPI_HOTPLUG_CPU */ +acpi_handle acpi_get_processor_handle(int cpu); + #ifdef CONFIG_ACPI_HOTPLUG_IOAPIC int acpi_get_ioapic_id(acpi_handle handle, u32 gsi_base, u64 *phys_addr); #endif @@ -1077,6 +1079,11 @@ static inline bool acpi_sleep_state_supported(u8 sleep_state) return false; } +static inline acpi_handle acpi_get_processor_handle(int cpu) +{ + return NULL; +} + #endif /* !CONFIG_ACPI */ extern void arch_post_acpi_subsys_init(void);