Re: [PATCH v8 14/21] ACPI / processor: Make it possible to get CPU hardware ID via GICC
From: Catalin Marinas
Date: Tue Feb 03 2015 - 15:10:03 EST
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 */
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/