Hi Hanjun,
I still maintain my original comment, the SMP code must reside in smp.c if possible. I have started looking at that.
On 18/02/15 13:53, Hanjun Guo wrote:
MADT contains the information for MPIDR which is essential for SMP initialization, parse the GIC cpu interface structures to get the MPIDR value and map it to cpu_logical_map(), and add enabled cpu with valid MPIDR into cpu_possible_map.
ACPI 5.1 only has two explicit methods to boot up SMP, PSCI and Parking protocol, but the Parking protocol is only specified for ARMv7 now, so make PSCI as the only way for the SMP boot protocol before some updates for the ACPI spec or the Parking protocol spec.
Parking protocol patches for SMP boot will be sent to upstream when the new version of Parking protocol is ready.
CC: Lorenzo Pieralisi lorenzo.pieralisi@arm.com CC: Catalin Marinas catalin.marinas@arm.com CC: Will Deacon will.deacon@arm.com CC: Mark Rutland mark.rutland@arm.com Tested-by: Suravee Suthikulpanit Suravee.Suthikulpanit@amd.com Tested-by: Yijing Wang wangyijing@huawei.com Tested-by: Mark Langsdorf mlangsdo@redhat.com Tested-by: Jon Masters jcm@redhat.com Tested-by: Timur Tabi timur@codeaurora.org Tested-by: Robert Richter rrichter@cavium.com Acked-by: Robert Richter rrichter@cavium.com Signed-off-by: Hanjun Guo hanjun.guo@linaro.org Signed-off-by: Tomasz Nowicki tomasz.nowicki@linaro.org
arch/arm64/include/asm/acpi.h | 2 + arch/arm64/include/asm/cpu_ops.h | 1 + arch/arm64/include/asm/smp.h | 5 +- arch/arm64/kernel/acpi.c | 149 ++++++++++++++++++++++++++++++++++++++- arch/arm64/kernel/cpu_ops.c | 2 +- arch/arm64/kernel/setup.c | 7 +- arch/arm64/kernel/smp.c | 2 +- 7 files changed, 160 insertions(+), 8 deletions(-)
[...]
diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c index bdcc9fc..3b4c44b 100644 --- a/arch/arm64/kernel/acpi.c +++ b/arch/arm64/kernel/acpi.c
[...]
+/**
- acpi_map_gic_cpu_interface - generates a logical cpu number
- and map to MPIDR represented by GICC structure
- @mpidr: CPU's hardware id to register, MPIDR represented in MADT
- @enabled: this cpu is enabled or not
- Returns the logical cpu number which maps to MPIDR
- */
+static int __init acpi_map_gic_cpu_interface(u64 mpidr, u8 enabled) +{
int i;if (mpidr == INVALID_HWID) {pr_info("Skip MADT cpu entry with invalid MPIDR\n");return -EINVAL;}total_cpus++;if (!enabled)return -EINVAL;if (enabled_cpus >= NR_CPUS) {pr_warn("NR_CPUS limit of %d reached, Processor %d/0x%llx ignored.\n",NR_CPUS, total_cpus, mpidr);return -EINVAL;}/* Check if GICC structure of boot CPU is available in the MADT */if (cpu_logical_map(0) == mpidr) {if (bootcpu_valid) {pr_err("Firmware bug, duplicate CPU MPIDR: 0x%llx in MADT\n",mpidr);return -EINVAL;}bootcpu_valid = true;}/** Duplicate MPIDRs are a recipe for disaster. Scan* all initialized entries and check for* duplicates. If any is found just ignore the CPU.*/for (i = 1; i < enabled_cpus; i++) {if (cpu_logical_map(i) == mpidr) {pr_err("Firmware bug, duplicate CPU MPIDR: 0x%llx in MADT\n",mpidr);return -EINVAL;}}if (!acpi_psci_present())return -EOPNOTSUPP;
I don't like this at all, psci_acpi_init will let you know about this, you ignore there and check here every-time, makes no sense to me.
IIUC if PSCI is not present, acpi_parse_fadt would have return error via acpi_boot_table_init and acpi_disabled is set. Right ?
cpu_ops[enabled_cpus] = cpu_get_ops("psci");/* CPU 0 was already initialized */if (enabled_cpus) {if (!cpu_ops[enabled_cpus])return -EINVAL;if (cpu_ops[enabled_cpus]->cpu_init(NULL, enabled_cpus))return -EOPNOTSUPP;/* map the logical cpu id to cpu MPIDR */cpu_logical_map(enabled_cpus) = mpidr;}enabled_cpus++;return enabled_cpus;+}
+static int __init +acpi_parse_gic_cpu_interface(struct acpi_subtable_header *header,
const unsigned long end)+{
struct acpi_madt_generic_interrupt *processor;processor = (struct acpi_madt_generic_interrupt *)header;if (BAD_MADT_ENTRY(processor, end))return -EINVAL;acpi_table_print_madt_entry(header);acpi_map_gic_cpu_interface(processor->arm_mpidr & MPIDR_HWID_BITMASK,processor->flags & ACPI_MADT_ENABLED);return 0;+}
+/* Parse GIC cpu interface entries in MADT for SMP init */ +void __init acpi_init_cpus(void) +{
int count, i;/** do a partial walk of MADT to determine how many CPUs* we have including disabled CPUs, and get information* we need for SMP init*/count = acpi_table_parse_madt(ACPI_MADT_TYPE_GENERIC_INTERRUPT,acpi_parse_gic_cpu_interface, 0);if (!count) {pr_err("No GIC CPU interface entries present\n");return;} else if (count < 0) {pr_err("Error parsing GIC CPU interface entry\n");return;}if (!bootcpu_valid) {pr_err("MADT missing boot CPU MPIDR, not enabling secondaries\n");return;}for (i = 0; i < enabled_cpus; i++)set_cpu_possible(i, true);/* Make boot-up look pretty */pr_info("%d CPUs enabled, %d CPUs total\n", enabled_cpus, total_cpus);+}
Lot of code duplicated with of_smp_init_cpus, there must be scope for unification. I will look at this as I mentioned above.
Regards, Sudeep