On Wednesday, February 25, 2015 04:39:41 PM Hanjun Guo wrote:

For a normal 8 cpu sockets system, it will up to 240 cpu threads (Xeon E7 v2 family for now), and we need 240 entries for local apic or local x2apic in MADT table, so it will be much verbose information printed with a slow uart console when system booted, this will be even worse with large system with 16/32 cpu sockets.

This patch just use pr_debug() instead of pr_info() for ioapic/iosapic, local apic/x2apic/sapic structures when scanning the MADT table to remove those verbose information, but leave other structures unchanged.

CC: Rafael J Wysocki rjw@rjwysocki.net Signed-off-by: Hanjun Guo hanjun.guo@linaro.org

Acked-by: Rafael J. Wysocki rafael.j.wysocki@intel.com

---

drivers/acpi/tables.c | 30 ++++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-)

diff --git a/drivers/acpi/tables.c b/drivers/acpi/tables.c index 93b8152..f4e5b88 100644 --- a/drivers/acpi/tables.c +++ b/drivers/acpi/tables.c @@ -23,6 +23,8 @@

*/ +/* Uncomment next line to get verbose printout */ +/* #define DEBUG */ #define pr_fmt(fmt) "ACPI: " fmt #include <linux/init.h> @@ -61,9 +63,9 @@ void acpi_table_print_madt_entry(struct acpi_subtable_header *header) { struct acpi_madt_local_apic *p = (struct acpi_madt_local_apic *)header;

• 	pr_info("LAPIC (acpi_id[0x%02x] lapic_id[0x%02x] %s)\n",
  

• 		p->processor_id, p->id,
  

• 		(p->lapic_flags & ACPI_MADT_ENABLED) ? "enabled" : "disabled");
  

• 	pr_debug("LAPIC (acpi_id[0x%02x] lapic_id[0x%02x] %s)\n",
  

• 		 p->processor_id, p->id,
  

• 		 (p->lapic_flags & ACPI_MADT_ENABLED) ? "enabled" : "disabled");
  

  } break;

@@ -71,9 +73,9 @@ void acpi_table_print_madt_entry(struct acpi_subtable_header *header) { struct acpi_madt_local_x2apic *p = (struct acpi_madt_local_x2apic *)header;

• 	pr_info("X2APIC (apic_id[0x%02x] uid[0x%02x] %s)\n",
  

• 		p->local_apic_id, p->uid,
  

• 		(p->lapic_flags & ACPI_MADT_ENABLED) ? "enabled" : "disabled");
  

• 	pr_debug("X2APIC (apic_id[0x%02x] uid[0x%02x] %s)\n",
  

• 		 p->local_apic_id, p->uid,
  

• 		 (p->lapic_flags & ACPI_MADT_ENABLED) ? "enabled" : "disabled");
  

  } break;

@@ -81,8 +83,8 @@ void acpi_table_print_madt_entry(struct acpi_subtable_header *header) { struct acpi_madt_io_apic *p = (struct acpi_madt_io_apic *)header;

• 	pr_info("IOAPIC (id[0x%02x] address[0x%08x] gsi_base[%d])\n",
  

• 		p->id, p->address, p->global_irq_base);
  

• 	pr_debug("IOAPIC (id[0x%02x] address[0x%08x] gsi_base[%d])\n",
  

• 		 p->id, p->address, p->global_irq_base);
  

  } break;

@@ -155,9 +157,9 @@ void acpi_table_print_madt_entry(struct acpi_subtable_header *header) { struct acpi_madt_io_sapic *p = (struct acpi_madt_io_sapic *)header;

• 	pr_info("IOSAPIC (id[0x%x] address[%p] gsi_base[%d])\n",
  

• 		p->id, (void *)(unsigned long)p->address,
  

• 		p->global_irq_base);
  

• 	pr_debug("IOSAPIC (id[0x%x] address[%p] gsi_base[%d])\n",
  

• 		 p->id, (void *)(unsigned long)p->address,
  

• 		 p->global_irq_base);
  

  } break;

@@ -165,9 +167,9 @@ void acpi_table_print_madt_entry(struct acpi_subtable_header *header) { struct acpi_madt_local_sapic *p = (struct acpi_madt_local_sapic *)header;

• 	pr_info("LSAPIC (acpi_id[0x%02x] lsapic_id[0x%02x] lsapic_eid[0x%02x] %s)\n",
  

• 		p->processor_id, p->id, p->eid,
  

• 		(p->lapic_flags & ACPI_MADT_ENABLED) ? "enabled" : "disabled");
  

• 	pr_debug("LSAPIC (acpi_id[0x%02x] lsapic_id[0x%02x] lsapic_eid[0x%02x] %s)\n",
  

• 		 p->processor_id, p->id, p->eid,
  

• 		 (p->lapic_flags & ACPI_MADT_ENABLED) ? "enabled" : "disabled");
  

  } break;

On Wed, 25 Feb 2015 16:39:41 +0800 , Hanjun Guo hanjun.guo@linaro.org wrote:

For a normal 8 cpu sockets system, it will up to 240 cpu threads (Xeon E7 v2 family for now), and we need 240 entries for local apic or local x2apic in MADT table, so it will be much verbose information printed with a slow uart console when system booted, this will be even worse with large system with 16/32 cpu sockets.

This patch just use pr_debug() instead of pr_info() for ioapic/iosapic, local apic/x2apic/sapic structures when scanning the MADT table to remove those verbose information, but leave other structures unchanged.

CC: Rafael J Wysocki rjw@rjwysocki.net Signed-off-by: Hanjun Guo hanjun.guo@linaro.org

One nitpick below, but don't respin over this, and don't do a fixup.

Acked-by: Grant Likely grant.likely@linaro.org

---

drivers/acpi/tables.c | 30 ++++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-)

diff --git a/drivers/acpi/tables.c b/drivers/acpi/tables.c index 93b8152..f4e5b88 100644 --- a/drivers/acpi/tables.c +++ b/drivers/acpi/tables.c @@ -23,6 +23,8 @@

*/ +/* Uncomment next line to get verbose printout */ +/* #define DEBUG */ #define pr_fmt(fmt) "ACPI: " fmt #include <linux/init.h> @@ -61,9 +63,9 @@ void acpi_table_print_madt_entry(struct acpi_subtable_header *header) { struct acpi_madt_local_apic *p = (struct acpi_madt_local_apic *)header;

• 	pr_info("LAPIC (acpi_id[0x%02x] lapic_id[0x%02x] %s)\n",
  

• 		p->processor_id, p->id,
  

• 		(p->lapic_flags & ACPI_MADT_ENABLED) ? "enabled" : "disabled");
  

• 	pr_debug("LAPIC (acpi_id[0x%02x] lapic_id[0x%02x] %s)\n",
  

• 		 p->processor_id, p->id,
  

• 		 (p->lapic_flags & ACPI_MADT_ENABLED) ? "enabled" : "disabled");
  

The whitespace changes makes each 1 line change into 3 line changes. In these situations, I would chose to leave the whitespace alone to keep the diffstat as small as possible. It makes it less likely to conflict with other patches and easier to find context.

}
break;

@@ -71,9 +73,9 @@ void acpi_table_print_madt_entry(struct acpi_subtable_header *header) { struct acpi_madt_local_x2apic *p = (struct acpi_madt_local_x2apic *)header;

• 	pr_info("X2APIC (apic_id[0x%02x] uid[0x%02x] %s)\n",
  

• 		p->local_apic_id, p->uid,
  

• 		(p->lapic_flags & ACPI_MADT_ENABLED) ? "enabled" : "disabled");
  

• 	pr_debug("X2APIC (apic_id[0x%02x] uid[0x%02x] %s)\n",
  

• 		 p->local_apic_id, p->uid,
  

• 		 (p->lapic_flags & ACPI_MADT_ENABLED) ? "enabled" : "disabled");
  

  } break;

@@ -81,8 +83,8 @@ void acpi_table_print_madt_entry(struct acpi_subtable_header *header) { struct acpi_madt_io_apic *p = (struct acpi_madt_io_apic *)header;

• 	pr_info("IOAPIC (id[0x%02x] address[0x%08x] gsi_base[%d])\n",
  

• 		p->id, p->address, p->global_irq_base);
  

• 	pr_debug("IOAPIC (id[0x%02x] address[0x%08x] gsi_base[%d])\n",
  

• 		 p->id, p->address, p->global_irq_base);
  

  } break;

@@ -155,9 +157,9 @@ void acpi_table_print_madt_entry(struct acpi_subtable_header *header) { struct acpi_madt_io_sapic *p = (struct acpi_madt_io_sapic *)header;

• 	pr_info("IOSAPIC (id[0x%x] address[%p] gsi_base[%d])\n",
  

• 		p->id, (void *)(unsigned long)p->address,
  

• 		p->global_irq_base);
  

• 	pr_debug("IOSAPIC (id[0x%x] address[%p] gsi_base[%d])\n",
  

• 		 p->id, (void *)(unsigned long)p->address,
  

• 		 p->global_irq_base);
  

  } break;

@@ -165,9 +167,9 @@ void acpi_table_print_madt_entry(struct acpi_subtable_header *header) { struct acpi_madt_local_sapic *p = (struct acpi_madt_local_sapic *)header;

• 	pr_info("LSAPIC (acpi_id[0x%02x] lsapic_id[0x%02x] lsapic_eid[0x%02x] %s)\n",
  

• 		p->processor_id, p->id, p->eid,
  

• 		(p->lapic_flags & ACPI_MADT_ENABLED) ? "enabled" : "disabled");
  

• 	pr_debug("LSAPIC (acpi_id[0x%02x] lsapic_id[0x%02x] lsapic_eid[0x%02x] %s)\n",
  

• 		 p->processor_id, p->id, p->eid,
  

• 		 (p->lapic_flags & ACPI_MADT_ENABLED) ? "enabled" : "disabled");
  

  } break;

1.9.1

On 2015年03月07日 04:31, Joe Perches wrote:

On Fri, 2015-03-06 at 20:17 +0000, Grant Likely wrote:

...

On Wed, 25 Feb 2015 16:39:41 +0800 Hanjun Guo hanjun.guo@linaro.org wrote:

...

This patch just use pr_debug() instead of pr_info() for ioapic/iosapic, local apic/x2apic/sapic structures when scanning the MADT table to remove those verbose information, but leave other structures unchanged.

[]

...

One nitpick below, but don't respin over this, and don't do a fixup.

[]

...

...

diff --git a/drivers/acpi/tables.c b/drivers/acpi/tables.c

[]

...

...

@@ -61,9 +63,9 @@ void acpi_table_print_madt_entry(struct acpi_subtable_header *header) { struct acpi_madt_local_apic *p = (struct acpi_madt_local_apic *)header;

• 	pr_info("LAPIC (acpi_id[0x%02x] lapic_id[0x%02x] %s)\n",
  

• 		p->processor_id, p->id,
  

• 		(p->lapic_flags & ACPI_MADT_ENABLED) ? "enabled" : "disabled");
  

• 	pr_debug("LAPIC (acpi_id[0x%02x] lapic_id[0x%02x] %s)\n",
  

• 		 p->processor_id, p->id,
  

• 		 (p->lapic_flags & ACPI_MADT_ENABLED) ? "enabled" : "disabled");
  

The whitespace changes makes each 1 line change into 3 line changes. In these situations, I would chose to leave the whitespace alone to keep the diffstat as small as possible. It makes it less likely to conflict with other patches and easier to find context.

I think it's mostly better to use a consistent indentation style regardless of the number in whitespace changes surrounding the change.

I think both would be fine. Since this patch only touches acpi core code and ACPI maintainer Rafael already acked it, I will keep it as it is.

Thanks Hanjun

On Wednesday, February 25, 2015 04:39:42 PM Hanjun Guo wrote:

CPU hardware ID (phys_id) is defined as u32 in structure acpi_processor, but phys_id is used as int in acpi processor driver, so it will lead to some inconsistence for the drivers.

Furthermore, to cater for ACPI arch ports that implement 64 bits CPU ids a generic CPU physical id type is required.

So introduce typedef u32 phys_cpuid_t for x86 and ia64, and introduce a macro CPU_PHYS_ID_INVALID as (u32)(-1), use phys_cpuid_t when phys_id defined in acpi processor driver, and replace CPU_PHYS_ID_INVALID as -1 for phys_id, this will solve the inconsistence in acpi processor driver, and will prepare for the ACPI on ARM64 too.

CC: Rafael J Wysocki rjw@rjwysocki.net Suggested-by: Lorenzo Pieralisi lorenzo.pieralisi@arm.com Acked-by: Sudeep Holla sudeep.holla@arm.com Signed-off-by: Catalin Marinas catalin.marinas@arm.com [hj: reworked cpu physid map return codes] Signed-off-by: Hanjun Guo hanjun.guo@linaro.org

---

arch/ia64/include/asm/acpi.h | 4 ++++ arch/ia64/kernel/acpi.c | 2 +- arch/x86/include/asm/acpi.h | 4 ++++ arch/x86/kernel/acpi/boot.c | 2 +- drivers/acpi/acpi_processor.c | 7 ++++--- drivers/acpi/processor_core.c | 30 +++++++++++++++--------------- include/acpi/processor.h | 6 +++--- include/linux/acpi.h | 2 +- 8 files changed, 33 insertions(+), 24 deletions(-)

diff --git a/arch/ia64/include/asm/acpi.h b/arch/ia64/include/asm/acpi.h index a1d91ab..ca1f0e4 100644 --- a/arch/ia64/include/asm/acpi.h +++ b/arch/ia64/include/asm/acpi.h @@ -34,6 +34,10 @@ #include <linux/numa.h> #include <asm/numa.h> +typedef u32 phys_cpuid_t;

+#define CPU_PHYS_ID_INVALID (u32)(-1)

#ifdef CONFIG_ACPI extern int acpi_lapic; #define acpi_disabled 0 /* ACPI always enabled on IA64 */ diff --git a/arch/ia64/kernel/acpi.c b/arch/ia64/kernel/acpi.c index 2c44989..067ef44 100644 --- a/arch/ia64/kernel/acpi.c +++ b/arch/ia64/kernel/acpi.c @@ -887,7 +887,7 @@ static int _acpi_map_lsapic(acpi_handle handle, int physid, int *pcpu) } /* wrapper to silence section mismatch warning */ -int __ref acpi_map_cpu(acpi_handle handle, int physid, int *pcpu) +int __ref acpi_map_cpu(acpi_handle handle, phys_cpuid_t physid, int *pcpu) { return _acpi_map_lsapic(handle, physid, pcpu); } diff --git a/arch/x86/include/asm/acpi.h b/arch/x86/include/asm/acpi.h index 3a45668..cd788dd 100644 --- a/arch/x86/include/asm/acpi.h +++ b/arch/x86/include/asm/acpi.h @@ -32,6 +32,10 @@ #include <asm/mpspec.h> #include <asm/realmode.h> +typedef u32 phys_cpuid_t;

+#define CPU_PHYS_ID_INVALID (u32)(-1)

Can we define those things in one common place instead of having to duplicate the definitions for every arch?

Also, I'd like to have

static inline bool invalid_cpu_phys_id(phys_cpuid_t phys_id) { return (int)phys_id < 0; }

as that would cover error codes too.

Moreover, why don't you use (phys_cpuid_t)(-1) in the #define? And call is PHYS_CPUID_INVALID for that matter (so that the name of the symbol corresponds to the name of the type)?

#ifdef CONFIG_ACPI extern int acpi_lapic; extern int acpi_ioapic; diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c index 3d525c6..e4f8582 100644 --- a/arch/x86/kernel/acpi/boot.c +++ b/arch/x86/kernel/acpi/boot.c @@ -757,7 +757,7 @@ static int _acpi_map_lsapic(acpi_handle handle, int physid, int *pcpu) } /* wrapper to silence section mismatch warning */ -int __ref acpi_map_cpu(acpi_handle handle, int physid, int *pcpu) +int __ref acpi_map_cpu(acpi_handle handle, phys_cpuid_t physid, int *pcpu) { return _acpi_map_lsapic(handle, physid, pcpu); } diff --git a/drivers/acpi/acpi_processor.c b/drivers/acpi/acpi_processor.c index 1020b1b..e6c7c56 100644 --- a/drivers/acpi/acpi_processor.c +++ b/drivers/acpi/acpi_processor.c @@ -170,7 +170,7 @@ static int acpi_processor_hotadd_init(struct acpi_processor *pr) acpi_status status; int ret;

• if (pr->phys_id == -1)

• if (pr->phys_id == CPU_PHYS_ID_INVALID) return -ENODEV;

status = acpi_evaluate_integer(pr->handle, "_STA", NULL, &sta); @@ -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;

• phys_cpuid_t phys_id;
• int cpu_index, device_declaration = 0; acpi_status status = AE_OK; static int cpu0_initialized; unsigned long long value;

@@ -263,7 +264,7 @@ static int acpi_processor_get_info(struct acpi_device *device) } phys_id = acpi_get_phys_id(pr->handle, device_declaration, pr->acpi_id);

• if (phys_id < 0)

• if (phys_id == CPU_PHYS_ID_INVALID) acpi_handle_debug(pr->handle, "failed to get CPU physical ID.\n"); pr->phys_id = phys_id;

diff --git a/drivers/acpi/processor_core.c b/drivers/acpi/processor_core.c index 7962651..a5547dc 100644 --- a/drivers/acpi/processor_core.c +++ b/drivers/acpi/processor_core.c @@ -32,7 +32,7 @@ static struct acpi_table_madt *get_madt_table(void) } static int map_lapic_id(struct acpi_subtable_header *entry,

•  u32 acpi_id, int *apic_id)
  

•  u32 acpi_id, phys_cpuid_t *apic_id)
  

{ struct acpi_madt_local_apic *lapic = container_of(entry, struct acpi_madt_local_apic, header); @@ -48,7 +48,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, phys_cpuid_t *apic_id)
  

{ struct acpi_madt_local_x2apic *apic = container_of(entry, struct acpi_madt_local_x2apic, header); @@ -65,7 +65,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, phys_cpuid_t *apic_id)
  

{ struct acpi_madt_local_sapic *lsapic = container_of(entry, struct acpi_madt_local_sapic, header); @@ -83,10 +83,10 @@ static int map_lsapic_id(struct acpi_subtable_header *entry, return 0; } -static int map_madt_entry(int type, u32 acpi_id) +static phys_cpuid_t map_madt_entry(int type, u32 acpi_id) { unsigned long madt_end, entry;

• int phys_id = -1; /* CPU hardware ID */

• phys_cpuid_t phys_id = CPU_PHYS_ID_INVALID; /* CPU hardware ID */ struct acpi_table_madt *madt;

madt = get_madt_table(); @@ -117,12 +117,12 @@ static int map_madt_entry(int type, u32 acpi_id) return phys_id; } -static int map_mat_entry(acpi_handle handle, int type, u32 acpi_id) +static phys_cpuid_t 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;

• phys_cpuid_t phys_id = CPU_PHYS_ID_INVALID;

if (ACPI_FAILURE(acpi_evaluate_object(handle, "_MAT", NULL, &buffer))) goto exit; @@ -149,27 +149,27 @@ exit: return phys_id; } -int acpi_get_phys_id(acpi_handle handle, int type, u32 acpi_id) +phys_cpuid_t acpi_get_phys_id(acpi_handle handle, int type, u32 acpi_id) {

• int phys_id;

• phys_cpuid_t phys_id;

phys_id = map_mat_entry(handle, type, acpi_id);

• if (phys_id == -1)

• if (phys_id == CPU_PHYS_ID_INVALID) phys_id = map_madt_entry(type, acpi_id);

return phys_id; } -int acpi_map_cpuid(int phys_id, u32 acpi_id) +int acpi_map_cpuid(phys_cpuid_t phys_id, u32 acpi_id) { #ifdef CONFIG_SMP int i; #endif

• if (phys_id == -1) {

• if (phys_id == CPU_PHYS_ID_INVALID) { /*
  • On UP processor, there is no _MAT or MADT table.

•  * So above phys_id is always set to -1.
  

•  * So above phys_id is always set to CPU_PHYS_ID_INVALID.
  

  • BIOS may define multiple CPU handles even for UP processor.
  • For example,

@@ -190,7 +190,7 @@ int acpi_map_cpuid(int phys_id, u32 acpi_id) if (nr_cpu_ids <= 1 && acpi_id == 0) return acpi_id; else

• 	return phys_id;
  

• 	return -1;
  

Can we use a proper error code here?

} #ifdef CONFIG_SMP

On Thu, Mar 5, 2015 at 8:44 AM, Hanjun Guo guohanjun@huawei.com wrote:

On 2015/3/5 6:29, Rafael J. Wysocki wrote:

...

On Wednesday, February 25, 2015 04:39:42 PM Hanjun Guo wrote:

[cut]

...

...

@@ -190,7 +190,7 @@ int acpi_map_cpuid(int phys_id, u32 acpi_id) if (nr_cpu_ids <= 1 && acpi_id == 0) return acpi_id; else

•                return phys_id;
  

•                return -1;
  

Can we use a proper error code here?

I'm afraid not. In ACPI processor drivers, -1 will be deemed to invalid cpu logical number, if we return error code here, we need to modify multi places of "if (cpu_logical_num == -1)" to

Oh, silly stuff.

"if (! (cpu_logical_num < 0))" too, so for me, I prefer to keep it as -1, but I'm open for suggestions.

OK

I think we need something like invalid_logical_cpuid() and use it in all of those checks instead of the direct comparisons, but we can make those changes later.

Rafael

On Wed, 25 Feb 2015 16:39:42 +0800 , Hanjun Guo hanjun.guo@linaro.org wrote:

CPU hardware ID (phys_id) is defined as u32 in structure acpi_processor, but phys_id is used as int in acpi processor driver, so it will lead to some inconsistence for the drivers.

Furthermore, to cater for ACPI arch ports that implement 64 bits CPU ids a generic CPU physical id type is required.

So introduce typedef u32 phys_cpuid_t for x86 and ia64, and introduce a macro CPU_PHYS_ID_INVALID as (u32)(-1), use phys_cpuid_t when phys_id defined in acpi processor driver, and replace CPU_PHYS_ID_INVALID as -1 for phys_id, this will solve the inconsistence in acpi processor driver, and will prepare for the ACPI on ARM64 too.

CC: Rafael J Wysocki rjw@rjwysocki.net Suggested-by: Lorenzo Pieralisi lorenzo.pieralisi@arm.com Acked-by: Sudeep Holla sudeep.holla@arm.com Signed-off-by: Catalin Marinas catalin.marinas@arm.com [hj: reworked cpu physid map return codes] Signed-off-by: Hanjun Guo hanjun.guo@linaro.org

Looks okay to me.

Reviewed-by: Grant Likely grant.likely@linaro.org

---

arch/ia64/include/asm/acpi.h | 4 ++++ arch/ia64/kernel/acpi.c | 2 +- arch/x86/include/asm/acpi.h | 4 ++++ arch/x86/kernel/acpi/boot.c | 2 +- drivers/acpi/acpi_processor.c | 7 ++++--- drivers/acpi/processor_core.c | 30 +++++++++++++++--------------- include/acpi/processor.h | 6 +++--- include/linux/acpi.h | 2 +- 8 files changed, 33 insertions(+), 24 deletions(-)

diff --git a/arch/ia64/include/asm/acpi.h b/arch/ia64/include/asm/acpi.h index a1d91ab..ca1f0e4 100644 --- a/arch/ia64/include/asm/acpi.h +++ b/arch/ia64/include/asm/acpi.h @@ -34,6 +34,10 @@ #include <linux/numa.h> #include <asm/numa.h> +typedef u32 phys_cpuid_t;

+#define CPU_PHYS_ID_INVALID (u32)(-1)

#ifdef CONFIG_ACPI extern int acpi_lapic; #define acpi_disabled 0 /* ACPI always enabled on IA64 */ diff --git a/arch/ia64/kernel/acpi.c b/arch/ia64/kernel/acpi.c index 2c44989..067ef44 100644 --- a/arch/ia64/kernel/acpi.c +++ b/arch/ia64/kernel/acpi.c @@ -887,7 +887,7 @@ static int _acpi_map_lsapic(acpi_handle handle, int physid, int *pcpu) } /* wrapper to silence section mismatch warning */ -int __ref acpi_map_cpu(acpi_handle handle, int physid, int *pcpu) +int __ref acpi_map_cpu(acpi_handle handle, phys_cpuid_t physid, int *pcpu) { return _acpi_map_lsapic(handle, physid, pcpu); } diff --git a/arch/x86/include/asm/acpi.h b/arch/x86/include/asm/acpi.h index 3a45668..cd788dd 100644 --- a/arch/x86/include/asm/acpi.h +++ b/arch/x86/include/asm/acpi.h @@ -32,6 +32,10 @@ #include <asm/mpspec.h> #include <asm/realmode.h> +typedef u32 phys_cpuid_t;

+#define CPU_PHYS_ID_INVALID (u32)(-1)

#ifdef CONFIG_ACPI extern int acpi_lapic; extern int acpi_ioapic; diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c index 3d525c6..e4f8582 100644 --- a/arch/x86/kernel/acpi/boot.c +++ b/arch/x86/kernel/acpi/boot.c @@ -757,7 +757,7 @@ static int _acpi_map_lsapic(acpi_handle handle, int physid, int *pcpu) } /* wrapper to silence section mismatch warning */ -int __ref acpi_map_cpu(acpi_handle handle, int physid, int *pcpu) +int __ref acpi_map_cpu(acpi_handle handle, phys_cpuid_t physid, int *pcpu) { return _acpi_map_lsapic(handle, physid, pcpu); } diff --git a/drivers/acpi/acpi_processor.c b/drivers/acpi/acpi_processor.c index 1020b1b..e6c7c56 100644 --- a/drivers/acpi/acpi_processor.c +++ b/drivers/acpi/acpi_processor.c @@ -170,7 +170,7 @@ static int acpi_processor_hotadd_init(struct acpi_processor *pr) acpi_status status; int ret;

• if (pr->phys_id == -1)

• if (pr->phys_id == CPU_PHYS_ID_INVALID) return -ENODEV;

status = acpi_evaluate_integer(pr->handle, "_STA", NULL, &sta); @@ -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;

• phys_cpuid_t phys_id;
• int cpu_index, device_declaration = 0; acpi_status status = AE_OK; static int cpu0_initialized; unsigned long long value;

@@ -263,7 +264,7 @@ static int acpi_processor_get_info(struct acpi_device *device) } phys_id = acpi_get_phys_id(pr->handle, device_declaration, pr->acpi_id);

• if (phys_id < 0)

• if (phys_id == CPU_PHYS_ID_INVALID) acpi_handle_debug(pr->handle, "failed to get CPU physical ID.\n"); pr->phys_id = phys_id;

diff --git a/drivers/acpi/processor_core.c b/drivers/acpi/processor_core.c index 7962651..a5547dc 100644 --- a/drivers/acpi/processor_core.c +++ b/drivers/acpi/processor_core.c @@ -32,7 +32,7 @@ static struct acpi_table_madt *get_madt_table(void) } static int map_lapic_id(struct acpi_subtable_header *entry,

•  u32 acpi_id, int *apic_id)
  

•  u32 acpi_id, phys_cpuid_t *apic_id)
  

{ struct acpi_madt_local_apic *lapic = container_of(entry, struct acpi_madt_local_apic, header); @@ -48,7 +48,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, phys_cpuid_t *apic_id)
  

{ struct acpi_madt_local_x2apic *apic = container_of(entry, struct acpi_madt_local_x2apic, header); @@ -65,7 +65,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, phys_cpuid_t *apic_id)
  

{ struct acpi_madt_local_sapic *lsapic = container_of(entry, struct acpi_madt_local_sapic, header); @@ -83,10 +83,10 @@ static int map_lsapic_id(struct acpi_subtable_header *entry, return 0; } -static int map_madt_entry(int type, u32 acpi_id) +static phys_cpuid_t map_madt_entry(int type, u32 acpi_id) { unsigned long madt_end, entry;

• int phys_id = -1; /* CPU hardware ID */

• phys_cpuid_t phys_id = CPU_PHYS_ID_INVALID; /* CPU hardware ID */ struct acpi_table_madt *madt;

madt = get_madt_table(); @@ -117,12 +117,12 @@ static int map_madt_entry(int type, u32 acpi_id) return phys_id; } -static int map_mat_entry(acpi_handle handle, int type, u32 acpi_id) +static phys_cpuid_t 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;

• phys_cpuid_t phys_id = CPU_PHYS_ID_INVALID;

if (ACPI_FAILURE(acpi_evaluate_object(handle, "_MAT", NULL, &buffer))) goto exit; @@ -149,27 +149,27 @@ exit: return phys_id; } -int acpi_get_phys_id(acpi_handle handle, int type, u32 acpi_id) +phys_cpuid_t acpi_get_phys_id(acpi_handle handle, int type, u32 acpi_id) {

• int phys_id;

• phys_cpuid_t phys_id;

phys_id = map_mat_entry(handle, type, acpi_id);

• if (phys_id == -1)

• if (phys_id == CPU_PHYS_ID_INVALID) phys_id = map_madt_entry(type, acpi_id);

return phys_id; } -int acpi_map_cpuid(int phys_id, u32 acpi_id) +int acpi_map_cpuid(phys_cpuid_t phys_id, u32 acpi_id) { #ifdef CONFIG_SMP int i; #endif

• if (phys_id == -1) {

• if (phys_id == CPU_PHYS_ID_INVALID) { /*
  • On UP processor, there is no _MAT or MADT table.

•  * So above phys_id is always set to -1.
  

•  * So above phys_id is always set to CPU_PHYS_ID_INVALID.
  

  • BIOS may define multiple CPU handles even for UP processor.
  • For example,

@@ -190,7 +190,7 @@ int acpi_map_cpuid(int phys_id, u32 acpi_id) if (nr_cpu_ids <= 1 && acpi_id == 0) return acpi_id; else

• 	return phys_id;
  

• 	return -1;
  

  }

#ifdef CONFIG_SMP @@ -208,7 +208,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;

• phys_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 b95dc32..4188a4d 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 */

• phys_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); +phys_cpuid_t acpi_get_phys_id(acpi_handle, int type, u32 acpi_id); +int acpi_map_cpuid(phys_cpuid_t phys_id, u32 acpi_id); int acpi_get_cpuid(acpi_handle, int type, u32 acpi_id); /* in processor_pdc.c */ diff --git a/include/linux/acpi.h b/include/linux/acpi.h index 24c7aa8..0eabd15 100644 --- a/include/linux/acpi.h +++ b/include/linux/acpi.h @@ -148,7 +148,7 @@ void acpi_numa_arch_fixup(void); #ifdef CONFIG_ACPI_HOTPLUG_CPU /* Arch dependent functions for cpu hotplug support */ -int acpi_map_cpu(acpi_handle handle, int physid, int *pcpu); +int acpi_map_cpu(acpi_handle handle, phys_cpuid_t physid, int *pcpu); int acpi_unmap_cpu(int cpu); #endif /* CONFIG_ACPI_HOTPLUG_CPU */ -- 1.9.1

On Wednesday, February 25, 2015 04:39:43 PM Hanjun Guo wrote:

From: Graeme Gregory graeme.gregory@linaro.org

Now with the base changes to the arm memory mapping it is safe to convert to using ioremap to map in the tables after acpi_gbl_permanent_mmap is set.

CC: Rafael J Wysocki rjw@rjwysocki.net Tested-by: Robert Richter rrichter@cavium.com Acked-by: Robert Richter rrichter@cavium.com Signed-off-by: Al Stone al.stone@linaro.org Signed-off-by: Graeme Gregory graeme.gregory@linaro.org Signed-off-by: Hanjun Guo hanjun.guo@linaro.org

Acked-by: Rafael J. Wysocki rafael.j.wysocki@intel.com

---

drivers/acpi/osl.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c index f9eeae8..39748bb 100644 --- a/drivers/acpi/osl.c +++ b/drivers/acpi/osl.c @@ -336,11 +336,11 @@ acpi_map_lookup_virt(void __iomem *virt, acpi_size size) return NULL; } -#ifndef CONFIG_IA64 -#define should_use_kmap(pfn) page_is_ram(pfn) -#else +#if defined(CONFIG_IA64) || defined(CONFIG_ARM64) /* ioremap will take care of cache attributes */ #define should_use_kmap(pfn) 0 +#else +#define should_use_kmap(pfn) page_is_ram(pfn) #endif static void __iomem *acpi_map(acpi_physical_address pg_off, unsigned long pg_sz)

On Wed, Feb 25, 2015 at 08:39:45AM +0000, Hanjun Guo wrote:

From: Al Stone al.stone@linaro.org

As we want to get ACPI tables to parse and then use the information for system initialization, we should get the RSDP (Root System Description Pointer) first, it then locates Extended Root Description Table (XSDT) which contains all the 64-bit physical address that pointer to other boot-time tables.

Introduce acpi.c and its related head file in this patch to provide fundamental needs of extern variables and functions for ACPI core, and then get boot-time tables as needed.

• asm/acenv.h for arch specific ACPICA environments and implementation, It is needed unconditionally by ACPI core;

I am not sure we will ever need it since the global lock is not needed on HW reduced ACPI (ie ACPI on arm64), and instructions to flush the caches are not needed either on arm64.

Anyway, I will have a look if it is worth writing optimized div/shift, to give it a purpose to exist.

[...]

diff --git a/arch/arm64/include/asm/acpi.h b/arch/arm64/include/asm/acpi.h new file mode 100644 index 0000000..8b837ab --- /dev/null +++ b/arch/arm64/include/asm/acpi.h @@ -0,0 +1,45 @@ +/*

• • Copyright (C) 2013-2014, Linaro Ltd.
• • Author: Al Stone al.stone@linaro.org
• • Author: Graeme Gregory graeme.gregory@linaro.org
• • Author: Hanjun Guo hanjun.guo@linaro.org
• • This program is free software; you can redistribute it and/or modify
• • it under the terms of the GNU General Public License version 2 as
• • published by the Free Software Foundation;
• */

+#ifndef _ASM_ACPI_H +#define _ASM_ACPI_H

+/* Basic configuration for ACPI */ +#ifdef CONFIG_ACPI +#define acpi_strict 1 /* No out-of-spec workarounds on ARM64 */ +extern int acpi_disabled; +extern int acpi_noirq; +extern int acpi_pci_disabled;

+static inline void disable_acpi(void) +{

• acpi_disabled = 1;
• acpi_pci_disabled = 1;
• acpi_noirq = 1;

+}

+/*

• • 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
• • obtained from GICC with 0 and not print some error message as well.
• • Since MADT must provide at least one GICC structure for GIC
• • initialization, CPU will be always available in MADT on ARM64.
• */

+static inline bool acpi_has_cpu_in_madt(void) +{

• return true;

+}

I am still trying to make sense of what's this is used for, to me it looks like a legacy hack in acpi_processor.c. It is a shame that we need to define functions to paper over legacy hacks that make no sense whatsoever on arm64.

+static inline void arch_fix_phys_package_id(int num, u32 slot) { }

This is only used on ia64 to pass the _SUN value to arch code to fix up the socket id. See above.

You could define last two functions in generic code, and let the arch code that needs them override them.

Whether we do it in this series or later, this stuff needs cleaning up.

+#endif /* CONFIG_ACPI */

+#endif /*_ASM_ACPI_H*/ diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile index bef04af..218eb7e 100644 --- a/arch/arm64/kernel/Makefile +++ b/arch/arm64/kernel/Makefile @@ -34,6 +34,7 @@ arm64-obj-$(CONFIG_KGDB) += kgdb.o arm64-obj-$(CONFIG_EFI) += efi.o efi-stub.o efi-entry.o arm64-obj-$(CONFIG_PCI) += pci.o arm64-obj-$(CONFIG_ARMV8_DEPRECATED) += armv8_deprecated.o +arm64-obj-$(CONFIG_ACPI) += acpi.o obj-y += $(arm64-obj-y) vdso/ obj-m += $(arm64-obj-m) diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c new file mode 100644 index 0000000..f052e7a --- /dev/null +++ b/arch/arm64/kernel/acpi.c @@ -0,0 +1,101 @@ +/*

• • ARM64 Specific Low-Level ACPI Boot Support
• • Copyright (C) 2013-2014, Linaro Ltd.
• • Author: Al Stone al.stone@linaro.org
• • Author: Graeme Gregory graeme.gregory@linaro.org
• • Author: Hanjun Guo hanjun.guo@linaro.org
• • Author: Tomasz Nowicki tomasz.nowicki@linaro.org
• • Author: Naresh Bhat naresh.bhat@linaro.org
• • This program is free software; you can redistribute it and/or modify
• • it under the terms of the GNU General Public License version 2 as
• • published by the Free Software Foundation.
• */

+#define pr_fmt(fmt) "ACPI: " fmt

+#include <linux/acpi.h> +#include <linux/bootmem.h> +#include <linux/cpumask.h> +#include <linux/init.h> +#include <linux/irq.h> +#include <linux/irqdomain.h> +#include <linux/memblock.h> +#include <linux/smp.h>

+int acpi_noirq; /* skip ACPI IRQ initialization */ +int acpi_disabled; +EXPORT_SYMBOL(acpi_disabled);

+int acpi_pci_disabled; /* skip ACPI PCI scan and IRQ initialization */ +EXPORT_SYMBOL(acpi_pci_disabled);

+/*

• • __acpi_map_table() will be called before page_init(), so early_ioremap()
• • or early_memremap() should be called here to for ACPI table mapping.
• */

+char *__init __acpi_map_table(unsigned long phys, unsigned long size) +{

• if (!phys || !size)

Is there a reason to rule out physical address 0x0 ?

Thanks, Lorenzo

• return NULL;
  

• return early_memremap(phys, size);

+}

+void __init __acpi_unmap_table(char *map, unsigned long size) +{

• if (!map || !size)

• return;
  

• early_memunmap(map, size);

+}

+static int __init acpi_parse_fadt(struct acpi_table_header *table) +{

• struct acpi_table_fadt *fadt = (struct acpi_table_fadt *)table;
• /*

• * Revision in table header is the FADT Major revision, and there
  

• * is a minor revision of FADT which was introduced by ACPI 5.1,
  

• * we only deal with ACPI 5.1 or newer revision to get GIC and SMP
  

• * boot protocol configuration data, or we will disable ACPI.
  

• */
  

• if (table->revision > 5 ||

•    (table->revision == 5 && fadt->minor_revision >= 1))
  

• return 0;
  

• pr_warn("Unsupported FADT revision %d.%d, should be 5.1+, will disable ACPI\n",

• table->revision, fadt->minor_revision);
  

• disable_acpi();
• return -EINVAL;

+}

+/*

• • acpi_boot_table_init() called from setup_arch(), always.
• • 1. find RSDP and get its address, and then find XSDT
• • 2. extract all tables and checksums them all
• • 3. check ACPI FADT revision
• • We can parse ACPI boot-time tables such as MADT after
• • this function is called.
• */

+void __init acpi_boot_table_init(void) +{

• /* If acpi_disabled, bail out */
• if (acpi_disabled)

• return;
  

• /* Initialize the ACPI boot-time table parser. */
• if (acpi_table_init()) {

• disable_acpi();
  

• return;
  

• }
• if (acpi_table_parse(ACPI_SIG_FADT, acpi_parse_fadt)) {

• /* disable ACPI if no FADT is found */
  

• disable_acpi();
  

• pr_err("Can't find FADT\n");
  

• }

+} diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c index e8420f6..4f1a014 100644 --- a/arch/arm64/kernel/setup.c +++ b/arch/arm64/kernel/setup.c @@ -17,6 +17,7 @@

• along with this program. If not, see http://www.gnu.org/licenses/.

*/ +#include <linux/acpi.h> #include <linux/export.h> #include <linux/kernel.h> #include <linux/stddef.h> @@ -46,6 +47,7 @@ #include <linux/efi.h> #include <linux/personality.h> +#include <asm/acpi.h> #include <asm/fixmap.h> #include <asm/cpu.h> #include <asm/cputype.h> @@ -380,6 +382,9 @@ void __init setup_arch(char **cmdline_p) efi_init(); arm64_memblock_init();

• /* Parse the ACPI tables for possible boot-time configuration */
• acpi_boot_table_init();
• paging_init(); request_standard_resources();

1.9.1

On Tue, Mar 10, 2015 at 08:01:16AM +0000, Hanjun Guo wrote:

[...]

...

...

diff --git a/arch/arm64/include/asm/acpi.h b/arch/arm64/include/asm/acpi.h new file mode 100644 index 0000000..8b837ab --- /dev/null +++ b/arch/arm64/include/asm/acpi.h @@ -0,0 +1,45 @@ +/*

• • Copyright (C) 2013-2014, Linaro Ltd.
• • Author: Al Stone al.stone@linaro.org
• • Author: Graeme Gregory graeme.gregory@linaro.org
• • Author: Hanjun Guo hanjun.guo@linaro.org
• • This program is free software; you can redistribute it and/or modify
• • it under the terms of the GNU General Public License version 2 as
• • published by the Free Software Foundation;
• */

+#ifndef _ASM_ACPI_H +#define _ASM_ACPI_H

+/* Basic configuration for ACPI */ +#ifdef CONFIG_ACPI +#define acpi_strict 1 /* No out-of-spec workarounds on ARM64 */ +extern int acpi_disabled; +extern int acpi_noirq; +extern int acpi_pci_disabled;

+static inline void disable_acpi(void) +{

• acpi_disabled = 1;
• acpi_pci_disabled = 1;
• acpi_noirq = 1;

+}

+/*

• • 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
• • obtained from GICC with 0 and not print some error message as well.
• • Since MADT must provide at least one GICC structure for GIC
• • initialization, CPU will be always available in MADT on ARM64.
• */

+static inline bool acpi_has_cpu_in_madt(void) +{

• return true;

+}

I am still trying to make sense of what's this is used for, to me it looks like a legacy hack in acpi_processor.c. It is a shame that we need to define functions to paper over legacy hacks that make no sense whatsoever on arm64.

I'm sorry that I'm confused, why call this as a "hack"? If I remember correctly, this is a bugfix for x86 from Redhat folks to test kdump on UP system but without CPU entries in MADT table [1], and we make it arch independent to introduce acpi_has_cpu_in_madt().

I gathered that, do not tell me it is nice please. ACPI core code is chock full of things like this that force you to define functions in arch arm64 since they are called from "generic" code, and I do not like that, at all.

It can be cleaned up later.

[1]: commit c401eb8ee37, ACPI / processor: Check if LAPIC is present during initialization

...

...

+static inline void arch_fix_phys_package_id(int num, u32 slot) { }

This is only used on ia64 to pass the _SUN value to arch code to fix up the socket id. See above.

You could define last two functions in generic code, and let the arch code that needs them override them.

Whether we do it in this series or later, this stuff needs cleaning up.

...

+#endif /* CONFIG_ACPI */

+#endif /*_ASM_ACPI_H*/ diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile index bef04af..218eb7e 100644 --- a/arch/arm64/kernel/Makefile +++ b/arch/arm64/kernel/Makefile @@ -34,6 +34,7 @@ arm64-obj-$(CONFIG_KGDB) += kgdb.o arm64-obj-$(CONFIG_EFI) += efi.o efi-stub.o efi-entry.o arm64-obj-$(CONFIG_PCI) += pci.o arm64-obj-$(CONFIG_ARMV8_DEPRECATED) += armv8_deprecated.o +arm64-obj-$(CONFIG_ACPI) += acpi.o

obj-y += $(arm64-obj-y) vdso/ obj-m += $(arm64-obj-m) diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c new file mode 100644 index 0000000..f052e7a --- /dev/null +++ b/arch/arm64/kernel/acpi.c @@ -0,0 +1,101 @@ +/*

• • ARM64 Specific Low-Level ACPI Boot Support
• • Copyright (C) 2013-2014, Linaro Ltd.
• • Author: Al Stone al.stone@linaro.org
• • Author: Graeme Gregory graeme.gregory@linaro.org
• • Author: Hanjun Guo hanjun.guo@linaro.org
• • Author: Tomasz Nowicki tomasz.nowicki@linaro.org
• • Author: Naresh Bhat naresh.bhat@linaro.org
• • This program is free software; you can redistribute it and/or modify
• • it under the terms of the GNU General Public License version 2 as
• • published by the Free Software Foundation.
• */

+#define pr_fmt(fmt) "ACPI: " fmt

+#include <linux/acpi.h> +#include <linux/bootmem.h> +#include <linux/cpumask.h> +#include <linux/init.h> +#include <linux/irq.h> +#include <linux/irqdomain.h> +#include <linux/memblock.h> +#include <linux/smp.h>

+int acpi_noirq; /* skip ACPI IRQ initialization */ +int acpi_disabled; +EXPORT_SYMBOL(acpi_disabled);

+int acpi_pci_disabled; /* skip ACPI PCI scan and IRQ initialization */ +EXPORT_SYMBOL(acpi_pci_disabled);

+/*

• • __acpi_map_table() will be called before page_init(), so early_ioremap()
• • or early_memremap() should be called here to for ACPI table mapping.
• */

+char *__init __acpi_map_table(unsigned long phys, unsigned long size) +{

• if (!phys || !size)

Is there a reason to rule out physical address 0x0 ?

No particular reasons, unless some arch/firmware limits, I'm not sure if we need this check (x86 needs it), I'm CC Leif to confirm.

If there is no particular reason the check should not be there, "x86 needs it" is not a good reason, there is arch specific code for that in x86 directories.

Ok, let's wait for Leif to confirm that.

Thanks, Lorenzo

On 2015年03月10日 19:19, Leif Lindholm wrote:

On Tue, Mar 10, 2015 at 04:01:16PM +0800, Hanjun Guo wrote:

...

...

...

index 0000000..f052e7a --- /dev/null +++ b/arch/arm64/kernel/acpi.c @@ -0,0 +1,101 @@ +/*

• • ARM64 Specific Low-Level ACPI Boot Support
• • Copyright (C) 2013-2014, Linaro Ltd.
• • Author: Al Stone al.stone@linaro.org
• • Author: Graeme Gregory graeme.gregory@linaro.org
• • Author: Hanjun Guo hanjun.guo@linaro.org
• • Author: Tomasz Nowicki tomasz.nowicki@linaro.org
• • Author: Naresh Bhat naresh.bhat@linaro.org
• • This program is free software; you can redistribute it and/or modify
• • it under the terms of the GNU General Public License version 2 as
• • published by the Free Software Foundation.
• */

+#define pr_fmt(fmt) "ACPI: " fmt

+#include <linux/acpi.h> +#include <linux/bootmem.h> +#include <linux/cpumask.h> +#include <linux/init.h> +#include <linux/irq.h> +#include <linux/irqdomain.h> +#include <linux/memblock.h> +#include <linux/smp.h>

+int acpi_noirq; /* skip ACPI IRQ initialization */ +int acpi_disabled; +EXPORT_SYMBOL(acpi_disabled);

+int acpi_pci_disabled; /* skip ACPI PCI scan and IRQ initialization */ +EXPORT_SYMBOL(acpi_pci_disabled);

+/*

• • __acpi_map_table() will be called before page_init(), so early_ioremap()
• • or early_memremap() should be called here to for ACPI table mapping.
• */

+char *__init __acpi_map_table(unsigned long phys, unsigned long size) +{

• if (!phys || !size)

Is there a reason to rule out physical address 0x0 ?

No particular reasons, unless some arch/firmware limits, I'm not sure if we need this check (x86 needs it), I'm CC Leif to confirm.

Nothing in UEFI explicitly bans using physical address 0 for anything, and nothing in the architecture reserves it. So I don't think this check is necessary.

Thanks for the confirmation, will remove the !phys check.

Hanjun

On Wed, 25 Feb 2015 16:39:46 +0800 , Hanjun Guo hanjun.guo@linaro.org wrote:

From: Mark Salter msalter@redhat.com

The acpi_os_ioremap() function may be used to map normal RAM or IO regions. The current implementation simply uses ioremap_cache(). This will work for some architectures, but arm64 ioremap_cache() cannot be used to map IO regions which don't support caching. So for arm64, use ioremap() for non-RAM regions.

CC: Rafael J Wysocki rjw@rjwysocki.net CC: Catalin Marinas catalin.marinas@arm.com Tested-by: Robert Richter rrichter@cavium.com Acked-by: Robert Richter rrichter@cavium.com Signed-off-by: Mark Salter msalter@redhat.com Signed-off-by: Hanjun Guo hanjun.guo@linaro.org

Reviewed-by: Grant Likely grant.likely@linaro.org

---

arch/arm64/include/asm/acpi.h | 13 +++++++++++++ include/acpi/acpi_io.h | 4 ++++ 2 files changed, 17 insertions(+)

diff --git a/arch/arm64/include/asm/acpi.h b/arch/arm64/include/asm/acpi.h index 8b837ab..40e0924 100644 --- a/arch/arm64/include/asm/acpi.h +++ b/arch/arm64/include/asm/acpi.h @@ -12,8 +12,21 @@ #ifndef _ASM_ACPI_H #define _ASM_ACPI_H +#include <linux/mm.h>

/* Basic configuration for ACPI */ #ifdef CONFIG_ACPI +/* ACPI table mapping after acpi_gbl_permanent_mmap is set */ +static inline void __iomem *acpi_os_ioremap(acpi_physical_address phys,

• 			    acpi_size size)
  

+{

• if (!page_is_ram(phys >> PAGE_SHIFT))

• return ioremap(phys, size);
  

• return ioremap_cache(phys, size);

+} +#define acpi_os_ioremap acpi_os_ioremap

#define acpi_strict 1 /* No out-of-spec workarounds on ARM64 */ extern int acpi_disabled; extern int acpi_noirq; diff --git a/include/acpi/acpi_io.h b/include/acpi/acpi_io.h index 444671e..dd86c5f 100644 --- a/include/acpi/acpi_io.h +++ b/include/acpi/acpi_io.h @@ -3,11 +3,15 @@ #include <linux/io.h> +#include <asm/acpi.h>

+#ifndef acpi_os_ioremap static inline void __iomem *acpi_os_ioremap(acpi_physical_address phys, acpi_size size) { return ioremap_cache(phys, size); } +#endif void __iomem *__init_refok acpi_os_map_iomem(acpi_physical_address phys, acpi_size size); -- 1.9.1

On Wednesday, February 25, 2015 04:39:47 PM Hanjun Guo wrote:

From: Graeme Gregory graeme.gregory@linaro.org

ACPI 5.1 does not currently support S states for ARM64 hardware but ACPI code will call acpi_target_system_state() for device power management, so introduce acpi_sleep.c to allow other drivers to function until S states are defined.

Since it is arm64 specific stub holder, so let acpi_sleep.c to ARM64 specific.

TODO: merge this with drivers/acpi/sleep.c once we fix the specification.

CC: Rafael J. Wysocki rjw@rjwysocki.net 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: Graeme Gregory graeme.gregory@linaro.org Signed-off-by: Tomasz Nowicki tomasz.nowicki@linaro.org Signed-off-by: Hanjun Guo hanjun.guo@linaro.org

---

arch/arm64/kernel/Makefile | 2 +- arch/arm64/kernel/acpi_sleep.c | 28 ++++++++++++++++++++++++++++ drivers/acpi/Makefile | 2 ++ 3 files changed, 31 insertions(+), 1 deletion(-) create mode 100644 arch/arm64/kernel/acpi_sleep.c

diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile index 218eb7e..4435943 100644 --- a/arch/arm64/kernel/Makefile +++ b/arch/arm64/kernel/Makefile @@ -34,7 +34,7 @@ arm64-obj-$(CONFIG_KGDB) += kgdb.o arm64-obj-$(CONFIG_EFI) += efi.o efi-stub.o efi-entry.o arm64-obj-$(CONFIG_PCI) += pci.o arm64-obj-$(CONFIG_ARMV8_DEPRECATED) += armv8_deprecated.o -arm64-obj-$(CONFIG_ACPI) += acpi.o +arm64-obj-$(CONFIG_ACPI) += acpi.o acpi_sleep.o obj-y += $(arm64-obj-y) vdso/ obj-m += $(arm64-obj-m) diff --git a/arch/arm64/kernel/acpi_sleep.c b/arch/arm64/kernel/acpi_sleep.c new file mode 100644 index 0000000..54578ef --- /dev/null +++ b/arch/arm64/kernel/acpi_sleep.c @@ -0,0 +1,28 @@ +/*

• • ARM64 Specific Sleep Functionality
• • Copyright (C) 2013-2014, Linaro Ltd.

• •  Author: Graeme Gregory <graeme.gregory@linaro.org>
    

• • This program is free software; you can redistribute it and/or modify
• • it under the terms of the GNU General Public License version 2 as
• • published by the Free Software Foundation.
• */

+#include <linux/acpi.h>

+/*

• • Currently the ACPI 5.1 standard does not define S states in a
• • manner which is usable for ARM64. These two stubs are sufficient
• • that system initialises and device PM works.
• */

+u32 acpi_target_system_state(void) +{

• return ACPI_STATE_S0;

+} +EXPORT_SYMBOL_GPL(acpi_target_system_state);

+int __init acpi_sleep_init(void) +{

• return -ENOSYS;

+} diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile index 623b117..c32edf5 100644 --- a/drivers/acpi/Makefile +++ b/drivers/acpi/Makefile @@ -23,7 +23,9 @@ acpi-y += nvs.o # Power management related files acpi-y += wakeup.o +ifneq (,$(findstring $(ARCH),x86 ia64))

This is super-ugly. Isn't there a better way to do that?

acpi-y += sleep.o +endif acpi-y += device_pm.o acpi-$(CONFIG_ACPI_SLEEP) += proc.o

On Wednesday, March 04, 2015 10:49:25 PM G Gregory wrote:

On 4 March 2015 at 22:38, Rafael J. Wysocki rjw@rjwysocki.net wrote:

...

On Wednesday, February 25, 2015 04:39:47 PM Hanjun Guo wrote:

...

From: Graeme Gregory graeme.gregory@linaro.org

ACPI 5.1 does not currently support S states for ARM64 hardware but ACPI code will call acpi_target_system_state() for device power management, so introduce acpi_sleep.c to allow other drivers to function until S states are defined.

Since it is arm64 specific stub holder, so let acpi_sleep.c to ARM64 specific.

TODO: merge this with drivers/acpi/sleep.c once we fix the specification.

CC: Rafael J. Wysocki rjw@rjwysocki.net 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: Graeme Gregory graeme.gregory@linaro.org Signed-off-by: Tomasz Nowicki tomasz.nowicki@linaro.org Signed-off-by: Hanjun Guo hanjun.guo@linaro.org

---

arch/arm64/kernel/Makefile | 2 +- arch/arm64/kernel/acpi_sleep.c | 28 ++++++++++++++++++++++++++++ drivers/acpi/Makefile | 2 ++ 3 files changed, 31 insertions(+), 1 deletion(-) create mode 100644 arch/arm64/kernel/acpi_sleep.c

diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile index 218eb7e..4435943 100644 --- a/arch/arm64/kernel/Makefile +++ b/arch/arm64/kernel/Makefile @@ -34,7 +34,7 @@ arm64-obj-$(CONFIG_KGDB) += kgdb.o arm64-obj-$(CONFIG_EFI) += efi.o efi-stub.o efi-entry.o arm64-obj-$(CONFIG_PCI) += pci.o arm64-obj-$(CONFIG_ARMV8_DEPRECATED) += armv8_deprecated.o -arm64-obj-$(CONFIG_ACPI) += acpi.o +arm64-obj-$(CONFIG_ACPI) += acpi.o acpi_sleep.o

obj-y += $(arm64-obj-y) vdso/ obj-m += $(arm64-obj-m) diff --git a/arch/arm64/kernel/acpi_sleep.c b/arch/arm64/kernel/acpi_sleep.c new file mode 100644 index 0000000..54578ef --- /dev/null +++ b/arch/arm64/kernel/acpi_sleep.c @@ -0,0 +1,28 @@ +/*

• • ARM64 Specific Sleep Functionality
• • Copyright (C) 2013-2014, Linaro Ltd.

• •  Author: Graeme Gregory <graeme.gregory@linaro.org>
    

• • This program is free software; you can redistribute it and/or modify
• • it under the terms of the GNU General Public License version 2 as
• • published by the Free Software Foundation.
• */

+#include <linux/acpi.h>

+/*

• • Currently the ACPI 5.1 standard does not define S states in a
• • manner which is usable for ARM64. These two stubs are sufficient
• • that system initialises and device PM works.
• */

+u32 acpi_target_system_state(void) +{

• return ACPI_STATE_S0;
  

+} +EXPORT_SYMBOL_GPL(acpi_target_system_state);

+int __init acpi_sleep_init(void) +{

• return -ENOSYS;
  

+} diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile index 623b117..c32edf5 100644 --- a/drivers/acpi/Makefile +++ b/drivers/acpi/Makefile @@ -23,7 +23,9 @@ acpi-y += nvs.o

# Power management related files acpi-y += wakeup.o +ifneq (,$(findstring $(ARCH),x86 ia64))

This is super-ugly. Isn't there a better way to do that?

Its the standard example in the GNU Make manual,

It is never used in the kernel this way, though, and I'm not going to let that slip through.

I would certainly love if someone had a batter suggestion.

Define ACPI_GENERIC_SLEEP in drivers/acpi/Kconfig as

config ACPI_GENERIC_SLEEP bool depends on IA64 || X86

and build the file conditional on that.

On Thu, Mar 05, 2015 at 12:16:29AM +0000, Rafael J. Wysocki wrote:

On Thursday, March 05, 2015 12:25:54 AM Rafael J. Wysocki wrote:

...

On Wednesday, March 04, 2015 10:49:25 PM G Gregory wrote:

...

On 4 March 2015 at 22:38, Rafael J. Wysocki rjw@rjwysocki.net wrote:

...

On Wednesday, February 25, 2015 04:39:47 PM Hanjun Guo wrote:

...

From: Graeme Gregory graeme.gregory@linaro.org

ACPI 5.1 does not currently support S states for ARM64 hardware but ACPI code will call acpi_target_system_state() for device power management, so introduce acpi_sleep.c to allow other drivers to function until S states are defined.

Since it is arm64 specific stub holder, so let acpi_sleep.c to ARM64 specific.

TODO: merge this with drivers/acpi/sleep.c once we fix the specification.

CC: Rafael J. Wysocki rjw@rjwysocki.net 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: Graeme Gregory graeme.gregory@linaro.org Signed-off-by: Tomasz Nowicki tomasz.nowicki@linaro.org Signed-off-by: Hanjun Guo hanjun.guo@linaro.org

---

arch/arm64/kernel/Makefile | 2 +- arch/arm64/kernel/acpi_sleep.c | 28 ++++++++++++++++++++++++++++ drivers/acpi/Makefile | 2 ++ 3 files changed, 31 insertions(+), 1 deletion(-) create mode 100644 arch/arm64/kernel/acpi_sleep.c

diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile index 218eb7e..4435943 100644 --- a/arch/arm64/kernel/Makefile +++ b/arch/arm64/kernel/Makefile @@ -34,7 +34,7 @@ arm64-obj-$(CONFIG_KGDB) += kgdb.o arm64-obj-$(CONFIG_EFI) += efi.o efi-stub.o efi-entry.o arm64-obj-$(CONFIG_PCI) += pci.o arm64-obj-$(CONFIG_ARMV8_DEPRECATED) += armv8_deprecated.o -arm64-obj-$(CONFIG_ACPI) += acpi.o +arm64-obj-$(CONFIG_ACPI) += acpi.o acpi_sleep.o

obj-y += $(arm64-obj-y) vdso/ obj-m += $(arm64-obj-m) diff --git a/arch/arm64/kernel/acpi_sleep.c b/arch/arm64/kernel/acpi_sleep.c new file mode 100644 index 0000000..54578ef --- /dev/null +++ b/arch/arm64/kernel/acpi_sleep.c @@ -0,0 +1,28 @@ +/*

• • ARM64 Specific Sleep Functionality
• • Copyright (C) 2013-2014, Linaro Ltd.

• •  Author: Graeme Gregory <graeme.gregory@linaro.org>
    

• • This program is free software; you can redistribute it and/or modify
• • it under the terms of the GNU General Public License version 2 as
• • published by the Free Software Foundation.
• */

+#include <linux/acpi.h>

+/*

• • Currently the ACPI 5.1 standard does not define S states in a
• • manner which is usable for ARM64. These two stubs are sufficient
• • that system initialises and device PM works.
• */

+u32 acpi_target_system_state(void) +{

• return ACPI_STATE_S0;
  

+} +EXPORT_SYMBOL_GPL(acpi_target_system_state);

+int __init acpi_sleep_init(void) +{

• return -ENOSYS;
  

+} diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile index 623b117..c32edf5 100644 --- a/drivers/acpi/Makefile +++ b/drivers/acpi/Makefile @@ -23,7 +23,9 @@ acpi-y += nvs.o

# Power management related files acpi-y += wakeup.o +ifneq (,$(findstring $(ARCH),x86 ia64))

This is super-ugly. Isn't there a better way to do that?

Its the standard example in the GNU Make manual,

It is never used in the kernel this way, though, and I'm not going to let that slip through.

...

I would certainly love if someone had a batter suggestion.

Define ACPI_GENERIC_SLEEP in drivers/acpi/Kconfig as

config ACPI_GENERIC_SLEEP bool depends on IA64 || X86

and build the file conditional on that.

Or define

config ACPI_GENERIC_SLEEP bool

and then make both x86 and ia64 do

select ACPI_GENERIC_SLEEP if ACPI

which kind of will make it clear where the choice takes place.

While at it, use the newly defined config entry to add the required static inline in the respective headers and get rid of acpi_sleep.c from arm64.

Thanks, Lorenzo

On 2015年03月07日 02:31, Lorenzo Pieralisi wrote:

On Wed, Feb 25, 2015 at 08:39:48AM +0000, Hanjun Guo wrote:

[...]

...

diff --git a/arch/arm64/kernel/pci.c b/arch/arm64/kernel/pci.c index 6f93c24..4095379 100644 --- a/arch/arm64/kernel/pci.c +++ b/arch/arm64/kernel/pci.c @@ -10,6 +10,7 @@

*/

+#include <linux/acpi.h> #include <linux/init.h> #include <linux/io.h> #include <linux/kernel.h> @@ -46,3 +47,27 @@ int pcibios_add_device(struct pci_dev *dev)

return 0; }

+/*

• • raw_pci_read/write - Platform-specific PCI config space access.
• */

+int raw_pci_read(unsigned int domain, unsigned int bus,

•   unsigned int devfn, int reg, int len, u32 *val)
  

+{

• return -ENXIO;

+}

+int raw_pci_write(unsigned int domain, unsigned int bus,

• unsigned int devfn, int reg, int len, u32 val)
  

+{

• return -ENXIO;

+}

It would be nice to see:

https://lkml.org/lkml/2015/2/27/264

merged at the same time, so that the two functions above disappear :)

I hope so, but MMCONFIG patches are being taken care of by Bjorn, it would be nice to merge them through different tree (and it no harms), and remove those two function when introducing the implementation of pci_acpi_scan_root() in later patches.

Thanks Hanjun

On Wed, Feb 25, 2015 at 08:39:48AM +0000, Hanjun Guo wrote:

CONFIG_ACPI depends CONFIG_PCI on x86 and ia64, in ARM64 server world we will have PCIe in most cases, but some of them may not, make CONFIG_ACPI depend CONFIG_PCI on ARM64 will satisfy both.

With that case, we need some arch dependent PCI functions to access the config space before the PCI root bridge is created, and pci_acpi_scan_root() to create the PCI root bus. So introduce some stub function here to make ACPI core compile and revisit them later when implemented on ARM64.

CC: Liviu Dudau Liviu.Dudau@arm.com CC: Catalin Marinas catalin.marinas@arm.com CC: Will Deacon will.deacon@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

---

arch/arm64/include/asm/pci.h | 6 ++++++ arch/arm64/kernel/pci.c | 25 +++++++++++++++++++++++++ 2 files changed, 31 insertions(+)

diff --git a/arch/arm64/include/asm/pci.h b/arch/arm64/include/asm/pci.h index 872ba93..fded096 100644 --- a/arch/arm64/include/asm/pci.h +++ b/arch/arm64/include/asm/pci.h @@ -24,6 +24,12 @@ */ #define PCI_DMA_BUS_IS_PHYS (0)

Hanjun,

+static inline int pci_get_legacy_ide_irq(struct pci_dev *dev, int channel) +{

• /* no legacy IRQ on arm64 */
• return -ENODEV;

+}

Could you explain to me what situation you encountered where you needed to define this? I'm trying to figure out when this would be needed and the only thing I can come up with would be for drivers/pnp/resource.c, but that is guarded by #ifdef CONFIG_PCI, which would mean that you should move this a bit further down in the file, but also contradicts the last comment on this patch (i.e. PCI on ACPI is not really implemented, so presumably CONFIG_PCI is not enabled?)

Best regards, Liviu

extern int isa_dma_bridge_buggy; #ifdef CONFIG_PCI diff --git a/arch/arm64/kernel/pci.c b/arch/arm64/kernel/pci.c index 6f93c24..4095379 100644 --- a/arch/arm64/kernel/pci.c +++ b/arch/arm64/kernel/pci.c @@ -10,6 +10,7 @@

*/ +#include <linux/acpi.h> #include <linux/init.h> #include <linux/io.h> #include <linux/kernel.h> @@ -46,3 +47,27 @@ int pcibios_add_device(struct pci_dev *dev) return 0; }

+/*

• • raw_pci_read/write - Platform-specific PCI config space access.
• */

+int raw_pci_read(unsigned int domain, unsigned int bus,

•   unsigned int devfn, int reg, int len, u32 *val)
  

+{

• return -ENXIO;

+}

+int raw_pci_write(unsigned int domain, unsigned int bus,

• unsigned int devfn, int reg, int len, u32 val)
  

+{

• return -ENXIO;

+}

+#ifdef CONFIG_ACPI +/* Root bridge scanning */ +struct pci_bus *pci_acpi_scan_root(struct acpi_pci_root *root) +{

• /* TODO: Should be revisited when implementing PCI on ACPI */
• return NULL;

+}

+#endif

1.9.1

On Wed, 25 Feb 2015 16:39:49 +0800 , Hanjun Guo hanjun.guo@linaro.org wrote:

From: Al Stone al.stone@linaro.org

This implements the following policy to decide whether ACPI should be used to boot the system:

• acpi=off: ACPI will not be used to boot the system, even if there is no alternative available (e.g., device tree is empty)
• acpi=force: only ACPI will be used to boot the system; if that fails, there will be no fallback to alternative methods (such as device tree)
• otherwise, ACPI will be used as a fallback if the device tree turns out to lack a platform description; the heuristic to decide this is whether /chosen is the only node present at depth 1

CC: Catalin Marinas catalin.marinas@arm.com CC: Will Deacon will.deacon@arm.com CC: Rafael J. Wysocki rjw@rjwysocki.net Signed-off-by: Al Stone al.stone@linaro.org Signed-off-by: Graeme Gregory graeme.gregory@linaro.org Signed-off-by: Hanjun Guo hanjun.guo@linaro.org Signed-off-by: Ard Biesheuvel ard.biesheuvel@linaro.org

Acked-by: Grant Likely grant.likely@linaro.org

---

Documentation/kernel-parameters.txt | 3 ++- arch/arm64/include/asm/acpi.h | 7 +++++ arch/arm64/kernel/acpi.c | 52 +++++++++++++++++++++++++++++++++---- 3 files changed, 56 insertions(+), 6 deletions(-)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt index bfcb1a6..d6c35a7 100644 --- a/Documentation/kernel-parameters.txt +++ b/Documentation/kernel-parameters.txt @@ -165,7 +165,7 @@ multipliers 'Kilo', 'Mega', and 'Giga', equalling 2^10, 2^20, and 2^30 bytes respectively. Such letter suffixes can also be entirely omitted.

• acpi= [HW,ACPI,X86]

• acpi= [HW,ACPI,X86,ARM64] Advanced Configuration and Power Interface Format: { force | off | strict | noirq | rsdt } force -- enable ACPI if default was off

@@ -175,6 +175,7 @@ bytes respectively. Such letter suffixes can also be entirely omitted. strictly ACPI specification compliant. rsdt -- prefer RSDT over (default) XSDT copy_dsdt -- copy DSDT to memory

• 	For ARM64, ONLY "acpi=off" or "acpi=force" are available
  

See also Documentation/power/runtime_pm.txt, pci=noacpi diff --git a/arch/arm64/include/asm/acpi.h b/arch/arm64/include/asm/acpi.h index 40e0924..c5a9b97 100644 --- a/arch/arm64/include/asm/acpi.h +++ b/arch/arm64/include/asm/acpi.h @@ -39,6 +39,13 @@ static inline void disable_acpi(void) acpi_noirq = 1; } +static inline void enable_acpi(void) +{

• acpi_disabled = 0;
• acpi_pci_disabled = 0;
• acpi_noirq = 0;

+}

/*

• 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 f052e7a..bdcc9fc 100644 --- a/arch/arm64/kernel/acpi.c +++ b/arch/arm64/kernel/acpi.c @@ -22,15 +22,49 @@ #include <linux/irq.h> #include <linux/irqdomain.h> #include <linux/memblock.h> +#include <linux/of_fdt.h> #include <linux/smp.h> -int acpi_noirq; /* skip ACPI IRQ initialization */ -int acpi_disabled; +int acpi_noirq = 1; /* skip ACPI IRQ initialization */ +int acpi_disabled = 1; EXPORT_SYMBOL(acpi_disabled); -int acpi_pci_disabled; /* skip ACPI PCI scan and IRQ initialization */ +int acpi_pci_disabled = 1; /* skip ACPI PCI scan and IRQ initialization */ EXPORT_SYMBOL(acpi_pci_disabled); +static bool __initdata param_acpi_off; +static bool __initdata param_acpi_force;

+static int __init parse_acpi(char *arg) +{

• if (!arg)

• return -EINVAL;
  

• /* "acpi=off" disables both ACPI table parsing and interpreter */
• if (strcmp(arg, "off") == 0)

• param_acpi_off = true;
  

• else if (strcmp(arg, "force") == 0) /* force ACPI to be enabled */

• param_acpi_force = true;
  

• else

• return -EINVAL;	/* Core will print when we return error */
  

• return 0;

+} +early_param("acpi", parse_acpi);

+static int __init dt_scan_depth1_nodes(unsigned long node,

• 		       const char *uname, int depth,
  

• 		       void *data)
  

+{

• /*

• * Return 1 as soon as we encounter a node at depth 1 that is
  

• * not the /chosen node.
  

• */
  

• if (depth == 1 && (strcmp(uname, "chosen") != 0))

• return 1;
  

• return 0;

+}

/*

• __acpi_map_table() will be called before page_init(), so early_ioremap()
• or early_memremap() should be called here to for ACPI table mapping.

@@ -83,10 +117,18 @@ static int __init acpi_parse_fadt(struct acpi_table_header *table) */ void __init acpi_boot_table_init(void) {

• /* If acpi_disabled, bail out */
• if (acpi_disabled)

• /*

• * Enable ACPI instead of device tree unless
  

• * - ACPI has been disabled explicitly (acpi=off), or
  

• * - the device tree is not empty (it has more than just a /chosen node)
  

• *   and ACPI has not been force enabled (acpi=force)
  

• */
  

• if (param_acpi_off ||

•    (!param_acpi_force && of_scan_flat_dt(dt_scan_depth1_nodes, NULL)))
  

  return;

• enable_acpi();
• /* Initialize the ACPI boot-time table parser. */ if (acpi_table_init()) { disable_acpi();

-- 1.9.1

On Wed, 25 Feb 2015 16:39:51 +0800 , Hanjun Guo hanjun.guo@linaro.org wrote:

From: Graeme Gregory graeme.gregory@linaro.org

There are two flags: PSCI_COMPLIANT and PSCI_USE_HVC. When set, the former signals to the OS that the firmware is PSCI compliant. The latter selects the appropriate conduit for PSCI calls by toggling between Hypervisor Calls (HVC) and Secure Monitor Calls (SMC).

FADT table contains such information in ACPI 5.1, FADT table was parsed in ACPI table init and copy to struct acpi_gbl_FADT, so use the flags in struct acpi_gbl_FADT for PSCI init.

Since ACPI 5.1 doesn't support self defined PSCI function IDs, which means that only PSCI 0.2+ is supported in ACPI.

CC: Lorenzo Pieralisi lorenzo.pieralisi@arm.com CC: Catalin Marinas catalin.marinas@arm.com CC: Will Deacon will.deacon@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: Graeme Gregory graeme.gregory@linaro.org Signed-off-by: Tomasz Nowicki tomasz.nowicki@linaro.org Signed-off-by: Hanjun Guo hanjun.guo@linaro.org

Acked-by: Grant Likely grant.likely@linaro.org

---

arch/arm64/include/asm/acpi.h | 15 +++++++++ arch/arm64/include/asm/psci.h | 3 +- arch/arm64/kernel/psci.c | 78 ++++++++++++++++++++++++++++++------------- arch/arm64/kernel/setup.c | 8 +++-- 4 files changed, 76 insertions(+), 28 deletions(-)

diff --git a/arch/arm64/include/asm/acpi.h b/arch/arm64/include/asm/acpi.h index c5a9b97..9ea650c 100644 --- a/arch/arm64/include/asm/acpi.h +++ b/arch/arm64/include/asm/acpi.h @@ -32,6 +32,18 @@ extern int acpi_disabled; extern int acpi_noirq; extern int acpi_pci_disabled; +/* 1 to indicate PSCI 0.2+ is implemented */ +static inline bool acpi_psci_present(void) +{

• return acpi_gbl_FADT.arm_boot_flags & ACPI_FADT_PSCI_COMPLIANT;

+}

+/* 1 to indicate HVC must be used instead of SMC as the PSCI conduit */ +static inline bool acpi_psci_use_hvc(void) +{

• return acpi_gbl_FADT.arm_boot_flags & ACPI_FADT_PSCI_USE_HVC;

+}

static inline void disable_acpi(void) { acpi_disabled = 1; @@ -60,6 +72,9 @@ static inline bool acpi_has_cpu_in_madt(void) static inline void arch_fix_phys_package_id(int num, u32 slot) { } +#else +static inline bool acpi_psci_present(void) { return false; } +static inline bool acpi_psci_use_hvc(void) { return false; } #endif /* CONFIG_ACPI */ #endif /*_ASM_ACPI_H*/ diff --git a/arch/arm64/include/asm/psci.h b/arch/arm64/include/asm/psci.h index e5312ea..2454bc5 100644 --- a/arch/arm64/include/asm/psci.h +++ b/arch/arm64/include/asm/psci.h @@ -14,6 +14,7 @@ #ifndef __ASM_PSCI_H #define __ASM_PSCI_H -int psci_init(void); +int psci_dt_init(void); +int psci_acpi_init(void); #endif /* __ASM_PSCI_H */ diff --git a/arch/arm64/kernel/psci.c b/arch/arm64/kernel/psci.c index 3425f31..bab2bea 100644 --- a/arch/arm64/kernel/psci.c +++ b/arch/arm64/kernel/psci.c @@ -15,6 +15,7 @@ #define pr_fmt(fmt) "psci: " fmt +#include <linux/acpi.h> #include <linux/init.h> #include <linux/of.h> #include <linux/smp.h> @@ -24,6 +25,7 @@ #include <linux/slab.h> #include <uapi/linux/psci.h> +#include <asm/acpi.h> #include <asm/compiler.h> #include <asm/cpu_ops.h> #include <asm/errno.h> @@ -304,6 +306,33 @@ static void psci_sys_poweroff(void) invoke_psci_fn(PSCI_0_2_FN_SYSTEM_OFF, 0, 0, 0); } +static void __init psci_0_2_set_functions(void) +{

• pr_info("Using standard PSCI v0.2 function IDs\n");
• psci_function_id[PSCI_FN_CPU_SUSPEND] = PSCI_0_2_FN64_CPU_SUSPEND;
• psci_ops.cpu_suspend = psci_cpu_suspend;
• psci_function_id[PSCI_FN_CPU_OFF] = PSCI_0_2_FN_CPU_OFF;
• psci_ops.cpu_off = psci_cpu_off;
• psci_function_id[PSCI_FN_CPU_ON] = PSCI_0_2_FN64_CPU_ON;
• psci_ops.cpu_on = psci_cpu_on;
• psci_function_id[PSCI_FN_MIGRATE] = PSCI_0_2_FN64_MIGRATE;
• psci_ops.migrate = psci_migrate;
• psci_function_id[PSCI_FN_AFFINITY_INFO] = PSCI_0_2_FN64_AFFINITY_INFO;
• psci_ops.affinity_info = psci_affinity_info;
• psci_function_id[PSCI_FN_MIGRATE_INFO_TYPE] =

• PSCI_0_2_FN_MIGRATE_INFO_TYPE;
  

• psci_ops.migrate_info_type = psci_migrate_info_type;
• arm_pm_restart = psci_sys_reset;
• pm_power_off = psci_sys_poweroff;

+}

/*

• PSCI Function IDs for v0.2+ are well defined so use
• standard values.

@@ -337,29 +366,7 @@ static int __init psci_0_2_init(struct device_node *np) } }

• pr_info("Using standard PSCI v0.2 function IDs\n");
• psci_function_id[PSCI_FN_CPU_SUSPEND] = PSCI_0_2_FN64_CPU_SUSPEND;
• psci_ops.cpu_suspend = psci_cpu_suspend;
• psci_function_id[PSCI_FN_CPU_OFF] = PSCI_0_2_FN_CPU_OFF;
• psci_ops.cpu_off = psci_cpu_off;
• psci_function_id[PSCI_FN_CPU_ON] = PSCI_0_2_FN64_CPU_ON;
• psci_ops.cpu_on = psci_cpu_on;
• psci_function_id[PSCI_FN_MIGRATE] = PSCI_0_2_FN64_MIGRATE;
• psci_ops.migrate = psci_migrate;
• psci_function_id[PSCI_FN_AFFINITY_INFO] = PSCI_0_2_FN64_AFFINITY_INFO;
• psci_ops.affinity_info = psci_affinity_info;
• psci_function_id[PSCI_FN_MIGRATE_INFO_TYPE] =

• PSCI_0_2_FN_MIGRATE_INFO_TYPE;
  

• psci_ops.migrate_info_type = psci_migrate_info_type;
• arm_pm_restart = psci_sys_reset;
• pm_power_off = psci_sys_poweroff;

• psci_0_2_set_functions();

out_put_node: of_node_put(np); @@ -412,7 +419,7 @@ static const struct of_device_id psci_of_match[] __initconst = { {}, }; -int __init psci_init(void) +int __init psci_dt_init(void) { struct device_node *np; const struct of_device_id *matched_np; @@ -427,6 +434,29 @@ int __init psci_init(void) return init_fn(np); } +/*

• • We use PSCI 0.2+ when ACPI is deployed on ARM64 and it's
• • explicitly clarified in SBBR
• */

+int __init psci_acpi_init(void) +{

• if (!acpi_psci_present()) {

• pr_info("is not implemented in ACPI.\n");
  

• return -EOPNOTSUPP;
  

• }
• pr_info("probing for conduit method from ACPI.\n");
• if (acpi_psci_use_hvc())

• invoke_psci_fn = __invoke_psci_fn_hvc;
  

• else

• invoke_psci_fn = __invoke_psci_fn_smc;
  

• psci_0_2_set_functions();
• return 0;

+}

#ifdef CONFIG_SMP static int __init cpu_psci_cpu_init(struct device_node *dn, unsigned int cpu) diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c index e8c7000..97fa7f3 100644 --- a/arch/arm64/kernel/setup.c +++ b/arch/arm64/kernel/setup.c @@ -390,10 +390,12 @@ void __init setup_arch(char **cmdline_p) early_ioremap_reset();

• if (acpi_disabled)

• if (acpi_disabled) { unflatten_device_tree();

• psci_init();

• psci_dt_init();
  

• } else {

• psci_acpi_init();
  

• }

cpu_read_bootcpu_ops();

#ifdef CONFIG_SMP

1.9.1

On Wednesday, February 25, 2015 04:39:52 PM Hanjun Guo wrote:

When MADT is parsed, print GIC information as debug message:

ACPI: GICC (acpi_id[0x0000] address[00000000e112f000] MPIDR[0x0] enabled) ACPI: GICC (acpi_id[0x0001] address[00000000e112f000] MPIDR[0x1] enabled) ... ACPI: GICC (acpi_id[0x0201] address[00000000e112f000] MPIDR[0x201] enabled)

Those debug information will be very helpful to bring up early systems to see if acpi_id and MPIDR are matched or not as spec defined.

CC: Rafael J. Wysocki rjw@rjwysocki.net 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 Acked-by: Sudeep Holla sudeep.holla@arm.com Signed-off-by: Hanjun Guo hanjun.guo@linaro.org Signed-off-by: Tomasz Nowicki tomasz.nowicki@linaro.org

Acked-by: Rafael J. Wysocki rafael.j.wysocki@intel.com

---

drivers/acpi/tables.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+)

diff --git a/drivers/acpi/tables.c b/drivers/acpi/tables.c index f4e5b88..2e19189 100644 --- a/drivers/acpi/tables.c +++ b/drivers/acpi/tables.c @@ -185,6 +185,28 @@ void acpi_table_print_madt_entry(struct acpi_subtable_header *header) } break;

• case ACPI_MADT_TYPE_GENERIC_INTERRUPT:

• {
  

• 	struct acpi_madt_generic_interrupt *p =
  

• 		(struct acpi_madt_generic_interrupt *)header;
  

• 	pr_debug("GICC (acpi_id[0x%04x] address[%llx] MPIDR[0x%llx] %s)\n",
  

• 		 p->uid, p->base_address,
  

• 		 p->arm_mpidr,
  

• 		 (p->flags & ACPI_MADT_ENABLED) ? "enabled" : "disabled");
  

• }
  

• break;
  

• case ACPI_MADT_TYPE_GENERIC_DISTRIBUTOR:

• {
  

• 	struct acpi_madt_generic_distributor *p =
  

• 		(struct acpi_madt_generic_distributor *)header;
  

• 	pr_debug("GIC Distributor (gic_id[0x%04x] address[%llx] gsi_base[%d])\n",
  

• 		 p->gic_id, p->base_address,
  

• 		 p->global_irq_base);
  

• }
  

• break;
  

• default: pr_warn("Found unsupported MADT entry (type = 0x%x)\n", header->type);

On Wed, 25 Feb 2015 16:39:52 +0800 , Hanjun Guo hanjun.guo@linaro.org wrote:

When MADT is parsed, print GIC information as debug message:

ACPI: GICC (acpi_id[0x0000] address[00000000e112f000] MPIDR[0x0] enabled) ACPI: GICC (acpi_id[0x0001] address[00000000e112f000] MPIDR[0x1] enabled) ... ACPI: GICC (acpi_id[0x0201] address[00000000e112f000] MPIDR[0x201] enabled)

Those debug information will be very helpful to bring up early systems to see if acpi_id and MPIDR are matched or not as spec defined.

CC: Rafael J. Wysocki rjw@rjwysocki.net 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 Acked-by: Sudeep Holla sudeep.holla@arm.com Signed-off-by: Hanjun Guo hanjun.guo@linaro.org Signed-off-by: Tomasz Nowicki tomasz.nowicki@linaro.org

Acked-by: Grant Likely grant.likely@linaro.org

---

drivers/acpi/tables.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+)

diff --git a/drivers/acpi/tables.c b/drivers/acpi/tables.c index f4e5b88..2e19189 100644 --- a/drivers/acpi/tables.c +++ b/drivers/acpi/tables.c @@ -185,6 +185,28 @@ void acpi_table_print_madt_entry(struct acpi_subtable_header *header) } break;

• case ACPI_MADT_TYPE_GENERIC_INTERRUPT:

• {
  

• 	struct acpi_madt_generic_interrupt *p =
  

• 		(struct acpi_madt_generic_interrupt *)header;
  

• 	pr_debug("GICC (acpi_id[0x%04x] address[%llx] MPIDR[0x%llx] %s)\n",
  

• 		 p->uid, p->base_address,
  

• 		 p->arm_mpidr,
  

• 		 (p->flags & ACPI_MADT_ENABLED) ? "enabled" : "disabled");
  

• }
  

• break;
  

• case ACPI_MADT_TYPE_GENERIC_DISTRIBUTOR:

• {
  

• 	struct acpi_madt_generic_distributor *p =
  

• 		(struct acpi_madt_generic_distributor *)header;
  

• 	pr_debug("GIC Distributor (gic_id[0x%04x] address[%llx] gsi_base[%d])\n",
  

• 		 p->gic_id, p->base_address,
  

• 		 p->global_irq_base);
  

• }
  

• break;
  

• default: pr_warn("Found unsupported MADT entry (type = 0x%x)\n", header->type);

-- 1.9.1

Hi,

On Wed, Feb 25, 2015 at 04:39:53PM +0800, 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

Some nits below. Up to you if you fix incrementally or respin, but I'd like to see them fixed for consistency's sake.

Acked-by: Olof Johansson olof@lixom.net

diff --git a/arch/arm64/include/asm/smp.h b/arch/arm64/include/asm/smp.h index 780f82c..bf22650 100644 --- a/arch/arm64/include/asm/smp.h +++ b/arch/arm64/include/asm/smp.h @@ -39,9 +39,10 @@ extern void show_ipi_list(struct seq_file *p, int prec); extern void handle_IPI(int ipinr, struct pt_regs *regs); /*

• • Setup the set of possible CPUs (via set_cpu_possible)

• • Discover the set of possible CPUs and determine their
• • SMP operations.
  */

-extern void smp_init_cpus(void); +extern void of_smp_init_cpus(void);

This is inconsistent naming, we use dt some places, of elsewhere.

diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c index bdcc9fc..0f35d87 100644 --- a/arch/arm64/kernel/acpi.c +++ b/arch/arm64/kernel/acpi.c @@ -25,6 +25,10 @@ #include <linux/of_fdt.h> #include <linux/smp.h> +#include <asm/cputype.h> +#include <asm/cpu_ops.h> +#include <asm/smp_plat.h>

int acpi_noirq = 1; /* skip ACPI IRQ initialization */ int acpi_disabled = 1; EXPORT_SYMBOL(acpi_disabled); @@ -32,6 +36,12 @@ EXPORT_SYMBOL(acpi_disabled); int acpi_pci_disabled = 1; /* skip ACPI PCI scan and IRQ initialization */ EXPORT_SYMBOL(acpi_pci_disabled); +/* Processors with enabled flag and sane MPIDR */ +static int enabled_cpus;

+/* Boot CPU is valid or not in MADT */ +static bool __initdata bootcpu_valid;

static bool __initdata param_acpi_off; static bool __initdata param_acpi_force; @@ -85,6 +95,129 @@ void __init __acpi_unmap_table(char *map, unsigned long size) early_memunmap(map, size); } +/**

• • 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;
  

• 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);

+}

static int __init acpi_parse_fadt(struct acpi_table_header *table) { struct acpi_table_fadt *fadt = (struct acpi_table_fadt *)table; @@ -96,8 +229,20 @@ static int __init acpi_parse_fadt(struct acpi_table_header *table) * boot protocol configuration data, or we will disable ACPI. */ if (table->revision > 5 ||

•    (table->revision == 5 && fadt->minor_revision >= 1))
  

• return 0;
  

•    (table->revision == 5 && fadt->minor_revision >= 1)) {
  

• /*
  

•  * 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 Parking protocol spec.
  

•  */
  

• if (acpi_psci_present())
  

• 	return 0;
  

• pr_warn("No PSCI support, will not bring up secondary CPUs\n");
  

• return -EOPNOTSUPP;
  

• }

pr_warn("Unsupported FADT revision %d.%d, should be 5.1+, will disable ACPI\n", table->revision, fadt->minor_revision); diff --git a/arch/arm64/kernel/cpu_ops.c b/arch/arm64/kernel/cpu_ops.c index cce9524..fb8ff9b 100644 --- a/arch/arm64/kernel/cpu_ops.c +++ b/arch/arm64/kernel/cpu_ops.c @@ -35,7 +35,7 @@ static const struct cpu_operations *supported_cpu_ops[] __initconst = { NULL, }; -static const struct cpu_operations * __init cpu_get_ops(const char *name) +const struct cpu_operations * __init cpu_get_ops(const char *name) { const struct cpu_operations **ops = supported_cpu_ops; diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c index 97fa7f3..b278311 100644 --- a/arch/arm64/kernel/setup.c +++ b/arch/arm64/kernel/setup.c @@ -393,13 +393,16 @@ void __init setup_arch(char **cmdline_p) if (acpi_disabled) { unflatten_device_tree(); psci_dt_init();

• cpu_read_bootcpu_ops();
  

+#ifdef CONFIG_SMP

• of_smp_init_cpus();
  

+#endif } else { psci_acpi_init();

• acpi_init_cpus();
  

  }

• cpu_read_bootcpu_ops();

#ifdef CONFIG_SMP

• smp_init_cpus(); smp_build_mpidr_hash();

#endif

I'd rather do without another ifdef above and provide a stub for that function, but again it might make sense to just keep a small smp_init_cpus() just as with the psci setup.

The cpu_read_bootcpu_ops() should be pushed down to the dt implementation to keep the paths more common between acpi and dt.

-Olof

On Wed, 25 Feb 2015 16:39:53 +0800 , Hanjun Guo hanjun.guo@linaro.org 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

Reviewed-by: Grant Likely grant.likely@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/include/asm/acpi.h b/arch/arm64/include/asm/acpi.h index 9ea650c..9719921 100644 --- a/arch/arm64/include/asm/acpi.h +++ b/arch/arm64/include/asm/acpi.h @@ -71,10 +71,12 @@ static inline bool acpi_has_cpu_in_madt(void) } static inline void arch_fix_phys_package_id(int num, u32 slot) { } +void __init acpi_init_cpus(void); #else static inline bool acpi_psci_present(void) { return false; } static inline bool acpi_psci_use_hvc(void) { return false; } +static inline void acpi_init_cpus(void) { } #endif /* CONFIG_ACPI */ #endif /*_ASM_ACPI_H*/ diff --git a/arch/arm64/include/asm/cpu_ops.h b/arch/arm64/include/asm/cpu_ops.h index da301ee..5a31d67 100644 --- a/arch/arm64/include/asm/cpu_ops.h +++ b/arch/arm64/include/asm/cpu_ops.h @@ -66,5 +66,6 @@ struct cpu_operations { extern const struct cpu_operations *cpu_ops[NR_CPUS]; int __init cpu_read_ops(struct device_node *dn, int cpu); void __init cpu_read_bootcpu_ops(void); +const struct cpu_operations *cpu_get_ops(const char *name); #endif /* ifndef __ASM_CPU_OPS_H */ diff --git a/arch/arm64/include/asm/smp.h b/arch/arm64/include/asm/smp.h index 780f82c..bf22650 100644 --- a/arch/arm64/include/asm/smp.h +++ b/arch/arm64/include/asm/smp.h @@ -39,9 +39,10 @@ extern void show_ipi_list(struct seq_file *p, int prec); extern void handle_IPI(int ipinr, struct pt_regs *regs); /*

• • Setup the set of possible CPUs (via set_cpu_possible)

• • Discover the set of possible CPUs and determine their
• • SMP operations.
  */

-extern void smp_init_cpus(void); +extern void of_smp_init_cpus(void); /*

• Provide a function to raise an IPI cross call on CPUs in callmap.

diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c index bdcc9fc..0f35d87 100644 --- a/arch/arm64/kernel/acpi.c +++ b/arch/arm64/kernel/acpi.c @@ -25,6 +25,10 @@ #include <linux/of_fdt.h> #include <linux/smp.h> +#include <asm/cputype.h> +#include <asm/cpu_ops.h> +#include <asm/smp_plat.h>

int acpi_noirq = 1; /* skip ACPI IRQ initialization */ int acpi_disabled = 1; EXPORT_SYMBOL(acpi_disabled); @@ -32,6 +36,12 @@ EXPORT_SYMBOL(acpi_disabled); int acpi_pci_disabled = 1; /* skip ACPI PCI scan and IRQ initialization */ EXPORT_SYMBOL(acpi_pci_disabled); +/* Processors with enabled flag and sane MPIDR */ +static int enabled_cpus;

+/* Boot CPU is valid or not in MADT */ +static bool __initdata bootcpu_valid;

static bool __initdata param_acpi_off; static bool __initdata param_acpi_force; @@ -85,6 +95,129 @@ void __init __acpi_unmap_table(char *map, unsigned long size) early_memunmap(map, size); } +/**

• • 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;
  

• 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);

+}

static int __init acpi_parse_fadt(struct acpi_table_header *table) { struct acpi_table_fadt *fadt = (struct acpi_table_fadt *)table; @@ -96,8 +229,20 @@ static int __init acpi_parse_fadt(struct acpi_table_header *table) * boot protocol configuration data, or we will disable ACPI. */ if (table->revision > 5 ||

•    (table->revision == 5 && fadt->minor_revision >= 1))
  

• return 0;
  

•    (table->revision == 5 && fadt->minor_revision >= 1)) {
  

• /*
  

•  * 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 Parking protocol spec.
  

•  */
  

• if (acpi_psci_present())
  

• 	return 0;
  

• pr_warn("No PSCI support, will not bring up secondary CPUs\n");
  

• return -EOPNOTSUPP;
  

• }

pr_warn("Unsupported FADT revision %d.%d, should be 5.1+, will disable ACPI\n", table->revision, fadt->minor_revision); diff --git a/arch/arm64/kernel/cpu_ops.c b/arch/arm64/kernel/cpu_ops.c index cce9524..fb8ff9b 100644 --- a/arch/arm64/kernel/cpu_ops.c +++ b/arch/arm64/kernel/cpu_ops.c @@ -35,7 +35,7 @@ static const struct cpu_operations *supported_cpu_ops[] __initconst = { NULL, }; -static const struct cpu_operations * __init cpu_get_ops(const char *name) +const struct cpu_operations * __init cpu_get_ops(const char *name) { const struct cpu_operations **ops = supported_cpu_ops; diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c index 97fa7f3..b278311 100644 --- a/arch/arm64/kernel/setup.c +++ b/arch/arm64/kernel/setup.c @@ -393,13 +393,16 @@ void __init setup_arch(char **cmdline_p) if (acpi_disabled) { unflatten_device_tree(); psci_dt_init();

• cpu_read_bootcpu_ops();
  

+#ifdef CONFIG_SMP

• of_smp_init_cpus();
  

+#endif } else { psci_acpi_init();

• acpi_init_cpus();
  

  }

• cpu_read_bootcpu_ops();

#ifdef CONFIG_SMP

• smp_init_cpus(); smp_build_mpidr_hash();

#endif diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c index 328b8ce..52998b7 100644 --- a/arch/arm64/kernel/smp.c +++ b/arch/arm64/kernel/smp.c @@ -322,7 +322,7 @@ void __init smp_prepare_boot_cpu(void)

• cpu logical map array containing MPIDR values related to logical
• cpus. Assumes that cpu_logical_map(0) has already been initialized.

*/ -void __init smp_init_cpus(void) +void __init of_smp_init_cpus(void) { struct device_node *dn = NULL; unsigned int i, cpu = 1; -- 1.9.1

On Wednesday, February 25, 2015 04:39:54 PM 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.

The ARM architecture defines the MPIDR register as the CPU hardware identifier. This patch adds the code infrastructure to retrieve the MPIDR values from the ARM ACPI GICC structure in order to look-up the kernel CPU hardware ids required by the ACPI core code to identify CPUs.

CC: Rafael J. Wysocki rjw@rjwysocki.net CC: Catalin Marinas catalin.marinas@arm.com CC: Will Deacon will.deacon@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

---

arch/arm64/include/asm/acpi.h | 12 ++++++++++++ drivers/acpi/processor_core.c | 30 ++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+)

diff --git a/arch/arm64/include/asm/acpi.h b/arch/arm64/include/asm/acpi.h index 9719921..9a23369 100644 --- a/arch/arm64/include/asm/acpi.h +++ b/arch/arm64/include/asm/acpi.h @@ -13,6 +13,8 @@ #define _ASM_ACPI_H #include <linux/mm.h> +#include <asm/cputype.h> +#include <asm/smp_plat.h> /* Basic configuration for ACPI */ #ifdef CONFIG_ACPI @@ -27,6 +29,9 @@ static inline void __iomem *acpi_os_ioremap(acpi_physical_address phys, } #define acpi_os_ioremap acpi_os_ioremap +typedef u64 phys_cpuid_t; +#define CPU_PHYS_ID_INVALID INVALID_HWID

Any chance to combine this with patch [2/21]? Or at least put them next to each other in the series so as to indicate that they are related or *mention* patch [2/21] in the changelog here?

IMO, you really need to define phys_cpuid_t in a common place or people will forget that it may be 64-bit, because they'll only be looking at their arch.

#define acpi_strict 1 /* No out-of-spec workarounds on ARM64 */ extern int acpi_disabled; extern int acpi_noirq; @@ -59,6 +64,13 @@ static inline void enable_acpi(void) } /*

• • 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.
• */

+#define cpu_physical_id(cpu) 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
• obtained from GICC with 0 and not print some error message as well.

diff --git a/drivers/acpi/processor_core.c b/drivers/acpi/processor_core.c index a5547dc..8654adb 100644 --- a/drivers/acpi/processor_core.c +++ b/drivers/acpi/processor_core.c @@ -83,6 +83,31 @@ static int map_lsapic_id(struct acpi_subtable_header *entry, return 0; } +/*

• • Retrieve the ARM CPU physical identifier (MPIDR)
• */

+static int map_gicc_mpidr(struct acpi_subtable_header *entry,

• int device_declaration, u32 acpi_id, phys_cpuid_t *mpidr)
  

+{

• struct acpi_madt_generic_interrupt *gicc =

•    container_of(entry, struct acpi_madt_generic_interrupt, header);
  

• if (!(gicc->flags & ACPI_MADT_ENABLED))

• return -ENODEV;
  

• /* device_declaration means Device object in DSDT, 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)) {

• *mpidr = gicc->arm_mpidr;
  

• return 0;
  

• }
• return -EINVAL;

+}

static phys_cpuid_t map_madt_entry(int type, u32 acpi_id) { unsigned long madt_end, entry; @@ -111,6 +136,9 @@ static phys_cpuid_t map_madt_entry(int type, u32 acpi_id) } else if (header->type == ACPI_MADT_TYPE_LOCAL_SAPIC) { if (!map_lsapic_id(header, type, acpi_id, &phys_id)) break;

• } else if (header->type == ACPI_MADT_TYPE_GENERIC_INTERRUPT) {
  

• 	if (!map_gicc_mpidr(header, type, acpi_id, &phys_id))
  

• 		break;
  

  } entry += header->length; }

@@ -143,6 +171,8 @@ static phys_cpuid_t map_mat_entry(acpi_handle handle, int type, u32 acpi_id) map_lsapic_id(header, type, acpi_id, &phys_id); else if (header->type == ACPI_MADT_TYPE_LOCAL_X2APIC) map_x2apic_id(header, type, acpi_id, &phys_id);

• else if (header->type == ACPI_MADT_TYPE_GENERIC_INTERRUPT)

• map_gicc_mpidr(header, type, acpi_id, &phys_id);
  

exit: kfree(buffer.pointer);

On Wed, 25 Feb 2015 16:39:54 +0800 , Hanjun Guo hanjun.guo@linaro.org wrote:

Introduce a new function map_gicc_mpidr() to allow MPIDRs to be obtained from the GICC Structure introduced by ACPI 5.1.

The ARM architecture defines the MPIDR register as the CPU hardware identifier. This patch adds the code infrastructure to retrieve the MPIDR values from the ARM ACPI GICC structure in order to look-up the kernel CPU hardware ids required by the ACPI core code to identify CPUs.

CC: Rafael J. Wysocki rjw@rjwysocki.net CC: Catalin Marinas catalin.marinas@arm.com CC: Will Deacon will.deacon@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

Reviewed-by: Grant Likely grant.likely@linaro.org

---

arch/arm64/include/asm/acpi.h | 12 ++++++++++++ drivers/acpi/processor_core.c | 30 ++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+)

diff --git a/arch/arm64/include/asm/acpi.h b/arch/arm64/include/asm/acpi.h index 9719921..9a23369 100644 --- a/arch/arm64/include/asm/acpi.h +++ b/arch/arm64/include/asm/acpi.h @@ -13,6 +13,8 @@ #define _ASM_ACPI_H #include <linux/mm.h> +#include <asm/cputype.h> +#include <asm/smp_plat.h> /* Basic configuration for ACPI */ #ifdef CONFIG_ACPI @@ -27,6 +29,9 @@ static inline void __iomem *acpi_os_ioremap(acpi_physical_address phys, } #define acpi_os_ioremap acpi_os_ioremap +typedef u64 phys_cpuid_t; +#define CPU_PHYS_ID_INVALID INVALID_HWID

#define acpi_strict 1 /* No out-of-spec workarounds on ARM64 */ extern int acpi_disabled; extern int acpi_noirq; @@ -59,6 +64,13 @@ static inline void enable_acpi(void) } /*

• • 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.
• */

+#define cpu_physical_id(cpu) 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
• obtained from GICC with 0 and not print some error message as well.

diff --git a/drivers/acpi/processor_core.c b/drivers/acpi/processor_core.c index a5547dc..8654adb 100644 --- a/drivers/acpi/processor_core.c +++ b/drivers/acpi/processor_core.c @@ -83,6 +83,31 @@ static int map_lsapic_id(struct acpi_subtable_header *entry, return 0; } +/*

• • Retrieve the ARM CPU physical identifier (MPIDR)
• */

+static int map_gicc_mpidr(struct acpi_subtable_header *entry,

• int device_declaration, u32 acpi_id, phys_cpuid_t *mpidr)
  

+{

• struct acpi_madt_generic_interrupt *gicc =

•    container_of(entry, struct acpi_madt_generic_interrupt, header);
  

• if (!(gicc->flags & ACPI_MADT_ENABLED))

• return -ENODEV;
  

• /* device_declaration means Device object in DSDT, 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)) {

• *mpidr = gicc->arm_mpidr;
  

• return 0;
  

• }
• return -EINVAL;

+}

static phys_cpuid_t map_madt_entry(int type, u32 acpi_id) { unsigned long madt_end, entry; @@ -111,6 +136,9 @@ static phys_cpuid_t map_madt_entry(int type, u32 acpi_id) } else if (header->type == ACPI_MADT_TYPE_LOCAL_SAPIC) { if (!map_lsapic_id(header, type, acpi_id, &phys_id)) break;

• } else if (header->type == ACPI_MADT_TYPE_GENERIC_INTERRUPT) {
  

• 	if (!map_gicc_mpidr(header, type, acpi_id, &phys_id))
  

• 		break;
  

  } entry += header->length; }

@@ -143,6 +171,8 @@ static phys_cpuid_t map_mat_entry(acpi_handle handle, int type, u32 acpi_id) map_lsapic_id(header, type, acpi_id, &phys_id); else if (header->type == ACPI_MADT_TYPE_LOCAL_X2APIC) map_x2apic_id(header, type, acpi_id, &phys_id);

• else if (header->type == ACPI_MADT_TYPE_GENERIC_INTERRUPT)

• map_gicc_mpidr(header, type, acpi_id, &phys_id);
  

exit: kfree(buffer.pointer); -- 1.9.1

On Wednesday, February 25, 2015 04:39:55 PM Hanjun Guo wrote:

Introduce ACPI_IRQ_MODEL_GIC which is needed for ARM64 as GIC is used, and then register device's gsi with the core IRQ subsystem.

acpi_register_gsi() is similar to DT based irq_of_parse_and_map(), since gsi is unique in the system, so use hwirq number directly for the mapping.

We are going to implement stacked domains when GICv2m, GICv3, ITS support are added.

CC: Marc Zyngier marc.zyngier@arm.com Originally-by: Amit Daniel Kachhap amit.daniel@samsung.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

For the changes in bus.c:

Acked-by: Rafael J. Wysocki rafael.j.wysocki@intel.com

---

arch/arm64/kernel/acpi.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++++ drivers/acpi/bus.c | 3 ++ include/linux/acpi.h | 1 + 3 files changed, 77 insertions(+)

diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c index 0f35d87..31f080e 100644 --- a/arch/arm64/kernel/acpi.c +++ b/arch/arm64/kernel/acpi.c @@ -76,6 +76,12 @@ static int __init dt_scan_depth1_nodes(unsigned long node, } /*

• • Since we're on ARM, the default interrupt routing model
• • clearly has to be GIC.
• */

+enum acpi_irq_model_id acpi_irq_model = ACPI_IRQ_MODEL_GIC;

+/*

• __acpi_map_table() will be called before page_init(), so early_ioremap()
• or early_memremap() should be called here to for ACPI table mapping.

*/ @@ -218,6 +224,73 @@ void __init acpi_init_cpus(void) pr_info("%d CPUs enabled, %d CPUs total\n", enabled_cpus, total_cpus); } +int acpi_gsi_to_irq(u32 gsi, unsigned int *irq) +{

• *irq = irq_find_mapping(NULL, gsi);
• return 0;

+} +EXPORT_SYMBOL_GPL(acpi_gsi_to_irq);

+/*

• • success: return IRQ number (>0)
• • failure: return =< 0
• */

+int acpi_register_gsi(struct device *dev, u32 gsi, int trigger, int polarity) +{

• unsigned int irq;
• unsigned int irq_type;
• /*

• * ACPI have no bindings to indicate SPI or PPI, so we
  

• * use different mappings from DT in ACPI.
  

• *
  

• * For FDT
  

• * PPI interrupt: in the range [0, 15];
  

• * SPI interrupt: in the range [0, 987];
  

• *
  

• * For ACPI, GSI should be unique so using
  

• * the hwirq directly for the mapping:
  

• * PPI interrupt: in the range [16, 31];
  

• * SPI interrupt: in the range [32, 1019];
  

• */
  

• if (trigger == ACPI_EDGE_SENSITIVE &&

• 		polarity == ACPI_ACTIVE_LOW)
  

• irq_type = IRQ_TYPE_EDGE_FALLING;
  

• else if (trigger == ACPI_EDGE_SENSITIVE &&

• 		polarity == ACPI_ACTIVE_HIGH)
  

• irq_type = IRQ_TYPE_EDGE_RISING;
  

• else if (trigger == ACPI_LEVEL_SENSITIVE &&

• 		polarity == ACPI_ACTIVE_LOW)
  

• irq_type = IRQ_TYPE_LEVEL_LOW;
  

• else if (trigger == ACPI_LEVEL_SENSITIVE &&

• 		polarity == ACPI_ACTIVE_HIGH)
  

• irq_type = IRQ_TYPE_LEVEL_HIGH;
  

• else

• irq_type = IRQ_TYPE_NONE;
  

• /*

• * Since only one GIC is supported in ACPI 5.0, we can
  

• * create mapping refer to the default domain
  

• */
  

• irq = irq_create_mapping(NULL, gsi);
• if (!irq)

• return irq;
  

• /* Set irq type if specified and different than the current one */
• if (irq_type != IRQ_TYPE_NONE &&

• irq_type != irq_get_trigger_type(irq))
  

• irq_set_irq_type(irq, irq_type);
  

• return irq;

+} +EXPORT_SYMBOL_GPL(acpi_register_gsi);

+void acpi_unregister_gsi(u32 gsi) +{ +} +EXPORT_SYMBOL_GPL(acpi_unregister_gsi);

static int __init acpi_parse_fadt(struct acpi_table_header *table) { struct acpi_table_fadt *fadt = (struct acpi_table_fadt *)table; diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c index 8b67bd0..c412fdb 100644 --- a/drivers/acpi/bus.c +++ b/drivers/acpi/bus.c @@ -448,6 +448,9 @@ static int __init acpi_bus_init_irq(void) case ACPI_IRQ_MODEL_IOSAPIC: message = "IOSAPIC"; break;

• case ACPI_IRQ_MODEL_GIC:

• message = "GIC";
  

• break;
  

  case ACPI_IRQ_MODEL_PLATFORM: message = "platform specific model"; break;

diff --git a/include/linux/acpi.h b/include/linux/acpi.h index 0eabd15..c03d8d1 100644 --- a/include/linux/acpi.h +++ b/include/linux/acpi.h @@ -73,6 +73,7 @@ enum acpi_irq_model_id { ACPI_IRQ_MODEL_IOAPIC, ACPI_IRQ_MODEL_IOSAPIC, ACPI_IRQ_MODEL_PLATFORM,

• ACPI_IRQ_MODEL_GIC, ACPI_IRQ_MODEL_COUNT

};