On Tue, Feb 03, 2015 at 02:17:49PM +0000, Mark Rutland wrote:
On Mon, Feb 02, 2015 at 12:45:42PM +0000, Hanjun Guo wrote:
Introduce a new function map_gicc_mpidr() to allow MPIDRs to be obtained from the GICC Structure introduced by ACPI 5.1.
MPIDR is the CPU hardware ID as local APIC ID on x86 platform, so we use MPIDR not the GIC CPU interface ID to identify CPUs.
Further steps would typedef a phys_id_t for in arch code(with appropriate size and a corresponding invalid value, say ~0) and use that instead of an int in drivers/acpi/processor_core.c to store phys_id, then no need for mpidr packing.
I don't understand why we don't fix this now, and I'm very worried that this patch leaves much potential for FW bugs due to potential Linux bugs.
Having a function called cpu_physical_id which _does not_ return a physical ID makes no sense to me. Any time we really need a physical ID, we're still going to have to unpack it (in an architecture-specific manner).
Do you mean something like this? Only briefly tested on Juno and I may have missed other calls:
diff --git a/arch/arm64/include/asm/acpi.h b/arch/arm64/include/asm/acpi.h index ea4d2b35c57b..4fafd62b1b86 100644 --- a/arch/arm64/include/asm/acpi.h +++ b/arch/arm64/include/asm/acpi.h @@ -49,33 +49,12 @@ static inline void enable_acpi(void) acpi_noirq = 0; }
-/* MPIDR value provided in GICC structure is 64 bits, but the - * existing phys_id (CPU hardware ID) using in acpi processor - * driver is 32-bit, to conform to the same datatype we need - * to repack the GICC structure MPIDR. - * - * bits other than following 32 bits are defined as 0, so it - * will be no information lost after repacked. - * - * Bits [0:7] Aff0; - * Bits [8:15] Aff1; - * Bits [16:23] Aff2; - * Bits [32:39] Aff3; - */ -static inline u32 pack_mpidr(u64 mpidr) -{ - return (u32) ((mpidr & 0xff00000000) >> 8) | mpidr; -} - /* * The ACPI processor driver for ACPI core code needs this macro * to find out this cpu was already mapped (mapping from CPU hardware * ID to CPU logical ID) or not. - * - * cpu_logical_map(cpu) is the mapping of MPIDR and the logical cpu, - * and MPIDR is the cpu hardware ID we needed to pack. */ -#define cpu_physical_id(cpu) pack_mpidr(cpu_logical_map(cpu)) +#define cpu_physical_id(cpu) cpu_logical_map(cpu)
/* * It's used from ACPI core in kdump to boot UP system with SMP kernel, diff --git a/arch/arm64/include/asm/smp_plat.h b/arch/arm64/include/asm/smp_plat.h index 59e282311b58..a492276e008d 100644 --- a/arch/arm64/include/asm/smp_plat.h +++ b/arch/arm64/include/asm/smp_plat.h @@ -40,4 +40,6 @@ static inline u32 mpidr_hash_size(void) extern u64 __cpu_logical_map[NR_CPUS]; #define cpu_logical_map(cpu) __cpu_logical_map[cpu]
+typedef u64 cpuid_t; + #endif /* __ASM_SMP_PLAT_H */ diff --git a/arch/ia64/include/asm/smp.h b/arch/ia64/include/asm/smp.h index fea21e986022..251c6af899d6 100644 --- a/arch/ia64/include/asm/smp.h +++ b/arch/ia64/include/asm/smp.h @@ -41,6 +41,8 @@ ia64_get_lid (void)
#define hard_smp_processor_id() ia64_get_lid()
+typedef int cpuid_t; + #ifdef CONFIG_SMP
#define XTP_OFFSET 0x1e0008 diff --git a/arch/x86/include/asm/smp.h b/arch/x86/include/asm/smp.h index 8cd1cc3bc835..638f7562ba99 100644 --- a/arch/x86/include/asm/smp.h +++ b/arch/x86/include/asm/smp.h @@ -83,6 +83,8 @@ struct smp_ops { /* Globals due to paravirt */ extern void set_cpu_sibling_map(int cpu);
+typedef int cpuid_t; + #ifdef CONFIG_SMP #ifndef CONFIG_PARAVIRT #define startup_ipi_hook(phys_apicid, start_eip, start_esp) do { } while (0) diff --git a/drivers/acpi/acpi_processor.c b/drivers/acpi/acpi_processor.c index 1020b1b53a17..3e1a6b3ee3b8 100644 --- a/drivers/acpi/acpi_processor.c +++ b/drivers/acpi/acpi_processor.c @@ -215,7 +215,8 @@ static int acpi_processor_get_info(struct acpi_device *device) union acpi_object object = { 0 }; struct acpi_buffer buffer = { sizeof(union acpi_object), &object }; struct acpi_processor *pr = acpi_driver_data(device); - int phys_id, cpu_index, device_declaration = 0; + cpuid_t phys_id; + int cpu_index, device_declaration = 0; acpi_status status = AE_OK; static int cpu0_initialized; unsigned long long value; diff --git a/drivers/acpi/processor_core.c b/drivers/acpi/processor_core.c index 5ac12e40fedd..a2735c315ef7 100644 --- a/drivers/acpi/processor_core.c +++ b/drivers/acpi/processor_core.c @@ -13,7 +13,7 @@ ACPI_MODULE_NAME("processor_core");
static int map_lapic_id(struct acpi_subtable_header *entry, - u32 acpi_id, int *apic_id) + u32 acpi_id, cpuid_t *apic_id) { struct acpi_madt_local_apic *lapic = container_of(entry, struct acpi_madt_local_apic, header); @@ -29,7 +29,7 @@ static int map_lapic_id(struct acpi_subtable_header *entry, }
static int map_x2apic_id(struct acpi_subtable_header *entry, - int device_declaration, u32 acpi_id, int *apic_id) + int device_declaration, u32 acpi_id, cpuid_t *apic_id) { struct acpi_madt_local_x2apic *apic = container_of(entry, struct acpi_madt_local_x2apic, header); @@ -46,7 +46,7 @@ static int map_x2apic_id(struct acpi_subtable_header *entry, }
static int map_lsapic_id(struct acpi_subtable_header *entry, - int device_declaration, u32 acpi_id, int *apic_id) + int device_declaration, u32 acpi_id, cpuid_t *apic_id) { struct acpi_madt_local_sapic *lsapic = container_of(entry, struct acpi_madt_local_sapic, header); @@ -69,7 +69,7 @@ static int map_lsapic_id(struct acpi_subtable_header *entry, * on Intel platforms */ static int map_gicc_mpidr(struct acpi_subtable_header *entry, - int device_declaration, u32 acpi_id, int *mpidr) + int device_declaration, u32 acpi_id, cpuid_t *mpidr) { struct acpi_madt_generic_interrupt *gicc = container_of(entry, struct acpi_madt_generic_interrupt, header); @@ -84,24 +84,21 @@ static int map_gicc_mpidr(struct acpi_subtable_header *entry, if (device_declaration && (gicc->uid == acpi_id)) { /* * bits other than [0:7] Aff0, [8:15] Aff1, [16:23] Aff2 and - * [32:39] Aff3 must be 0 which is defined in ACPI 5.1, so pack - * the Affx fields into a single 32 bit identifier to accommodate - * the acpi processor drivers. + * [32:39] Aff3 must be 0 which is defined in ACPI 5.1 */ - *mpidr = ((gicc->arm_mpidr & 0xff00000000) >> 8) - | gicc->arm_mpidr; + *mpidr = gicc->arm_mpidr & 0xff00ffffffUL; return 0; }
return -EINVAL; }
-static int map_madt_entry(int type, u32 acpi_id) +static cpuid_t map_madt_entry(int type, u32 acpi_id) { unsigned long madt_end, entry; static struct acpi_table_madt *madt; static int read_madt; - int phys_id = -1; /* CPU hardware ID */ + cpuid_t phys_id = -1; /* CPU hardware ID */
if (!read_madt) { if (ACPI_FAILURE(acpi_get_table(ACPI_SIG_MADT, 0, @@ -145,7 +142,7 @@ static int map_mat_entry(acpi_handle handle, int type, u32 acpi_id) struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL }; union acpi_object *obj; struct acpi_subtable_header *header; - int phys_id = -1; + cpuid_t phys_id = -1;
if (ACPI_FAILURE(acpi_evaluate_object(handle, "_MAT", NULL, &buffer))) goto exit; @@ -174,7 +171,7 @@ exit: return phys_id; }
-int acpi_get_phys_id(acpi_handle handle, int type, u32 acpi_id) +cpuid_t acpi_get_phys_id(acpi_handle handle, int type, u32 acpi_id) { int phys_id;
@@ -185,7 +182,7 @@ int acpi_get_phys_id(acpi_handle handle, int type, u32 acpi_id) return phys_id; }
-int acpi_map_cpuid(int phys_id, u32 acpi_id) +int acpi_map_cpuid(cpuid_t phys_id, u32 acpi_id) { #ifdef CONFIG_SMP int i; @@ -213,9 +210,9 @@ int acpi_map_cpuid(int phys_id, u32 acpi_id) * Return -1 for other CPU's handle. */ if (nr_cpu_ids <= 1 && acpi_id == 0) - return acpi_id; + return 0; else - return phys_id; + return -1; }
#ifdef CONFIG_SMP @@ -233,7 +230,7 @@ int acpi_map_cpuid(int phys_id, u32 acpi_id)
int acpi_get_cpuid(acpi_handle handle, int type, u32 acpi_id) { - int phys_id; + cpuid_t phys_id;
phys_id = acpi_get_phys_id(handle, type, acpi_id);
diff --git a/include/acpi/processor.h b/include/acpi/processor.h index b95dc32a6e6b..30ebdbf0d961 100644 --- a/include/acpi/processor.h +++ b/include/acpi/processor.h @@ -196,7 +196,7 @@ struct acpi_processor_flags { struct acpi_processor { acpi_handle handle; u32 acpi_id; - u32 phys_id; /* CPU hardware ID such as APIC ID for x86 */ + cpuid_t phys_id; /* CPU hardware ID such as APIC ID for x86 */ u32 id; /* CPU logical ID allocated by OS */ u32 pblk; int performance_platform_limit; @@ -310,8 +310,8 @@ static inline int acpi_processor_get_bios_limit(int cpu, unsigned int *limit) #endif /* CONFIG_CPU_FREQ */
/* in processor_core.c */ -int acpi_get_phys_id(acpi_handle, int type, u32 acpi_id); -int acpi_map_cpuid(int phys_id, u32 acpi_id); +cpuid_t acpi_get_phys_id(acpi_handle, int type, u32 acpi_id); +int acpi_map_cpuid(cpuid_t phys_id, u32 acpi_id); int acpi_get_cpuid(acpi_handle, int type, u32 acpi_id);
/* in processor_pdc.c */