Re: [PATCH v5 12/18] ACPI / processor: Make it possible to get CPU hardware ID via GICC

[Hanjun Guo]

On 2014-10-25 1:39, Lorenzo Pieralisi wrote:
> On Fri, Oct 17, 2014 at 02:37:08PM +0100, 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.
>>
>> Signed-off-by: Hanjun Guo <hanjun.guo@xxxxxxxxxx>
>> ---
>>  arch/arm64/include/asm/acpi.h |   29 +++++++++++++++++++++++++++++
>>  arch/arm64/kernel/acpi.c      |    1 -
>>  drivers/acpi/processor_core.c |   37 +++++++++++++++++++++++++++++++++++++
>>  3 files changed, 66 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm64/include/asm/acpi.h b/arch/arm64/include/asm/acpi.h
>> index c82d4a1..483ff45 100644
>> --- a/arch/arm64/include/asm/acpi.h
>> +++ b/arch/arm64/include/asm/acpi.h
>> @@ -12,6 +12,8 @@
>>  #ifndef _ASM_ACPI_H
>>  #define _ASM_ACPI_H
>>  
>> +#include <asm/smp_plat.h>
>> +
>>  /* Basic configuration for ACPI */
>>  #ifdef	CONFIG_ACPI
>>  #define acpi_strict 1	/* No out-of-spec workarounds on ARM64 */
>> @@ -45,6 +47,33 @@ static inline void enable_acpi(void)
>>  	acpi_noirq = 0;
>>  }
>>  
>> +/* MPIDR value provided in GICC structure is 64 bits, but the
>> + * existing apic_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.
>> + *
>> + * Only 32 bits of MPIDR are used:
>> + * 
>> + * 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))
>> +
>>  /*
>>   * It's used from ACPI core in kdump to boot UP system with SMP kernel,
>>   * with this check the ACPI core will not override the CPU index
>> diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
>> index 34837e0..e91ec76 100644
>> --- a/arch/arm64/kernel/acpi.c
>> +++ b/arch/arm64/kernel/acpi.c
>> @@ -24,7 +24,6 @@
>>  #include <linux/bootmem.h>
>>  #include <linux/smp.h>
>>  
>> -#include <asm/smp_plat.h>
>>  #include <asm/cputype.h>
>>  #include <asm/cpu_ops.h>
>>  
>> diff --git a/drivers/acpi/processor_core.c b/drivers/acpi/processor_core.c
>> index e32321c..4007313 100644
>> --- a/drivers/acpi/processor_core.c
>> +++ b/drivers/acpi/processor_core.c
>> @@ -64,6 +64,38 @@ static int map_lsapic_id(struct acpi_subtable_header *entry,
>>  	return 0;
>>  }
>>  
>> +/*
>> + * On ARM platform, MPIDR value is the hardware ID as apic ID
>> + * on Intel platforms
>> + */
>> +static int map_gicc_mpidr(struct acpi_subtable_header *entry,
>> +		int device_declaration, u32 acpi_id, int *mpidr)
>> +{
>> +	struct acpi_madt_generic_interrupt *gicc =
>> +	    container_of(entry, struct acpi_madt_generic_interrupt, header);
>> +
>> +	if (!(gicc->flags & ACPI_MADT_ENABLED))
>> +		return -ENODEV;
>> +
>> +	/* In the GIC interrupt model, logical processors are
>> +	 * required to have a Processor Device object in the DSDT,
>> +	 * so we should check device_declaration here
>> +	 */
>> +	if (device_declaration && (gicc->uid == acpi_id)) {
>> +		/*
>> +		 * Only bits [0:7] Aff0, bits [8:15] Aff1, bits [16:23] Aff2
>> +		 * and bits [32:39] Aff3 are meaningful, so pack the Affx
>> +		 * fields into a single 32 bit identifier to accommodate the
>> +		 * acpi processor drivers.
>> +		 */
>> +		*mpidr = ((gicc->arm_mpidr & 0xff00000000) >> 8)
>> +			 | gicc->arm_mpidr;
> 
> The simple fact that you define a function to pack the mpidr value and
> you can't use it here because this is *generic* code is telling, and
> a very bad omen. At the cost of sounding like a broken record, I do not
> like this mpidr->apic->logical_cpu song and dance at all.
> ACPI is peppered with code (eg hotplug is another example, CPUidle driver
> even worse) that is supposed to be generic but contains x86 code to carry
> out this cpuid conversion, I really think that in order to start an ARM64
> ACPI port properly we should at least try to factor out this physical to
> logical cpu id conversion, and it is not the first time that I mention this
> on the lists.

I know, thanks for pointing this out. As I replied in previous version
of this patch set, apic_id is x86/ia64 specific, but the meaning behind
it is not. It means the CPU hardware id to identify itself in the system,
it just like MPIDR on ARM.

I will send out a patch for RFC to convert apic_id to physid which
is generic for all platforms.

> 
> I will also talk to Rafael about this at the earliest opportunity, I
> guess that x86 code relies on apic-id because some ACPI versions could
> not rely on the acpi-id or some other reasons I have to investigate.

As ACPI spec (section 8.4, Declaring Processors) said, Each processor
in the system must be declared in the ACPI namespace, so each cpu will
have acpi_id in all ACPI versions, and in theory we can map acpi_id to
logical cpu id if we want to.

But things are complicated, apic_id is connected to many
tables, MADT for smp init, DSDT for device driver, and SRAT for NUMA (there
is no acpi_id in it, ONLY has apic_id in the table for x86/ia64), so if we
want to factor the code to map acpi_id to logical cpu id, we need to modify:

 - ACPI drivers;
 - SMP init for x86 and ia64
 - the mappings for NUMA init for x86 and ia64
that will be lots of work I think.

I'm willing to discuss this further and come out a solution, please
comment on what I said and share your ideas :)

Thanks
Hanjun
--
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/