This patch set is based on part1 "Make ACPI core running on ARM64" patch set.
After we can get the ACPI tables from UEFI, we can use these tables to initialise the system now.
GIC (means GIC cpu interface) structure and GIC distributor structure in MADT table contains the information of GIC cpu interface base address and GIC distributor base address, which can be used to initialise GIC.
Further more, parked address in GIC structure can be used as cpu release address for spin table SMP initialisation.
This patch set use these information to init SMP and GIC.
Please refer to chapter 5.2.12.14/15 of ACPI 5.0 spec for GIC and GIC distributor structure information.
Amit Daniel Kachhap (1): irqdomain: Add a new API irq_create_acpi_mapping()
Hanjun Guo (8): ARM64 / ACPI: Implement core functions for parsing MADT table ARM64 / ACPI: Prefill cpu possible/present maps and map logical cpu id to APIC id ARM64 / ACPI: Introduce map_gic_id() to get apic id from MADT or _MAT method ARM64 / ACPI: Use Parked Address in GIC structure for spin table SMP initialisation ACPI: Define ACPI_IRQ_MODEL_GIC needed for arm Irqchip / gic: Set as default domain so we can access from ACPI ACPI / ARM64: Update acpi_register_gsi to register with the core IRQ subsystem ACPI / GIC: Initialize GIC using the information in MADT
arch/arm64/include/asm/acpi.h | 16 +- arch/arm64/kernel/irq.c | 5 + arch/arm64/kernel/setup.c | 2 + arch/arm64/kernel/smp.c | 2 + arch/arm64/kernel/smp_spin_table.c | 16 +- drivers/acpi/bus.c | 3 + drivers/acpi/plat/arm-core.c | 397 +++++++++++++++++++++++++++++++++++- drivers/acpi/processor_core.c | 26 +++ drivers/acpi/tables.c | 21 ++ drivers/irqchip/irq-gic.c | 7 + include/linux/acpi.h | 9 + kernel/irq/irqdomain.c | 27 +++ 12 files changed, 521 insertions(+), 10 deletions(-)
Implement core functions for parsing MADT table to get the information about GIC cpu interface and GIC distributor.
Signed-off-by: Hanjun Guo hanjun.guo@linaro.org --- arch/arm64/include/asm/acpi.h | 3 + drivers/acpi/plat/arm-core.c | 139 ++++++++++++++++++++++++++++++++++++++++- drivers/acpi/tables.c | 21 +++++++ 3 files changed, 162 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/include/asm/acpi.h b/arch/arm64/include/asm/acpi.h index c830adc..be2951c 100644 --- a/arch/arm64/include/asm/acpi.h +++ b/arch/arm64/include/asm/acpi.h @@ -83,6 +83,9 @@ void arch_fix_phys_package_id(int num, u32 slot); extern int (*acpi_suspend_lowlevel)(void); #define acpi_wakeup_address (0)
+#define MAX_GIC_CPU_INTERFACE 256 +#define MAX_GIC_DISTRIBUTOR 1 /* should be the same as MAX_GIC_NR */ + #else /* !CONFIG_ACPI */ #define acpi_disabled 1 /* ACPI sometimes enabled on ARM */ #define acpi_noirq 1 /* ACPI sometimes enabled on ARM */ diff --git a/drivers/acpi/plat/arm-core.c b/drivers/acpi/plat/arm-core.c index 2b6df56..45ff625 100644 --- a/drivers/acpi/plat/arm-core.c +++ b/drivers/acpi/plat/arm-core.c @@ -52,6 +52,16 @@ EXPORT_SYMBOL(acpi_disabled); int acpi_pci_disabled; /* skip ACPI PCI scan and IRQ initialization */ EXPORT_SYMBOL(acpi_pci_disabled);
+/* + * Local interrupt controller address, + * GIC cpu interface base address on ARM/ARM64 + */ +static u64 acpi_lapic_addr __initdata; + +#define BAD_MADT_ENTRY(entry, end) ( \ + (!entry) || (unsigned long)entry + sizeof(*entry) > end || \ + ((struct acpi_subtable_header *)entry)->length < sizeof(*entry)) + #define PREFIX "ACPI: "
/* FIXME: this function should be moved to topology.c when it is ready */ @@ -102,6 +112,115 @@ void __init __acpi_unmap_table(char *map, unsigned long size) return; }
+static int __init acpi_parse_madt(struct acpi_table_header *table) +{ + struct acpi_table_madt *madt = NULL; + + madt = (struct acpi_table_madt *)table; + if (!madt) { + pr_warn(PREFIX "Unable to map MADT\n"); + return -ENODEV; + } + + if (madt->address) { + acpi_lapic_addr = (u64) madt->address; + + pr_info(PREFIX "Local APIC address 0x%08x\n", madt->address); + } + + return 0; +} + +/* + * GIC structures on ARM are somthing like Local APIC structures on x86, + * which means GIC cpu interfaces for GICv2/v3. Every GIC structure in + * MADT table represents a cpu in the system. + * + * GIC distributor structures are somthing like IOAPIC on x86. GIC can + * be initialized with information in this structure. + * + * Please refer to chapter5.2.12.14/15 of ACPI 5.0 + */ + +static int __init +acpi_parse_gic(struct acpi_subtable_header *header, const unsigned long end) +{ + struct acpi_madt_generic_interrupt *processor = NULL; + + processor = (struct acpi_madt_generic_interrupt *)header; + + if (BAD_MADT_ENTRY(processor, end)) + return -EINVAL; + + acpi_table_print_madt_entry(header); + + return 0; +} + +static int __init +acpi_parse_gic_distributor(struct acpi_subtable_header *header, + const unsigned long end) +{ + struct acpi_madt_generic_distributor *distributor = NULL; + + distributor = (struct acpi_madt_generic_distributor *)header; + + if (BAD_MADT_ENTRY(distributor, end)) + return -EINVAL; + + acpi_table_print_madt_entry(header); + + return 0; +} + +/* + * Parse GIC cpu interface related entries in MADT + * returns 0 on success, < 0 on error + */ +static int __init acpi_parse_madt_gic_entries(void) +{ + int count; + + /* + * do a partial walk of MADT to determine how many CPUs + * we have including disabled CPUs + */ + count = acpi_table_parse_madt(ACPI_MADT_TYPE_GENERIC_INTERRUPT, + acpi_parse_gic, MAX_GIC_CPU_INTERFACE); + + if (!count) { + pr_err(PREFIX "No GIC entries present\n"); + return -ENODEV; + } else if (count < 0) { + pr_err(PREFIX "Error parsing GIC entry\n"); + return count; + } + + return 0; +} + +/* + * Parse GIC distributor related entries in MADT + * returns 0 on success, < 0 on error + */ +static int __init acpi_parse_madt_gic_distributor_entries(void) +{ + int count; + + count = acpi_table_parse_madt(ACPI_MADT_TYPE_GENERIC_DISTRIBUTOR, + acpi_parse_gic_distributor, MAX_GIC_DISTRIBUTOR); + + if (!count) { + pr_err(PREFIX "No GIC distributor entries present\n"); + return -ENODEV; + } else if (count < 0) { + pr_err(PREFIX "Error parsing GIC distributor entry\n"); + return count; + } + + return 0; +} + int acpi_gsi_to_irq(u32 gsi, unsigned int *irq) { *irq = gsi_to_irq(gsi); @@ -132,11 +251,29 @@ static int __init acpi_parse_fadt(struct acpi_table_header *table)
static void __init early_acpi_process_madt(void) { - return; + acpi_table_parse(ACPI_SIG_MADT, acpi_parse_madt); }
static void __init acpi_process_madt(void) { + int error; + + if (!acpi_table_parse(ACPI_SIG_MADT, acpi_parse_madt)) { + + /* + * Parse MADT GIC cpu interface entries + */ + error = acpi_parse_madt_gic_entries(); + if (!error) { + /* + * Parse MADT GIC distributor entries + */ + acpi_parse_madt_gic_distributor_entries(); + } + } + + pr_info("Using ACPI for processor (GIC) configuration information\n"); + return; }
diff --git a/drivers/acpi/tables.c b/drivers/acpi/tables.c index d67a1fe..b3e4615 100644 --- a/drivers/acpi/tables.c +++ b/drivers/acpi/tables.c @@ -191,6 +191,27 @@ 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; + printk(KERN_INFO PREFIX + "GIC (acpi_id[0x%04x] gic_id[0x%04x] %s)\n", + p->uid, p->gic_id, + (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; + printk(KERN_INFO PREFIX + "GIC Distributor (id[0x%04x] address[0x%08llx] gsi_base[%d])\n", + p->gic_id, p->base_address, p->global_irq_base); + } + break; + default: printk(KERN_WARNING PREFIX "Found unsupported MADT entry (type = 0x%x)\n",
When boot the kernel with MADT, the cpu possible and present maps should be prefilled for cpu topology and acpi based cpu hot-plug.
The logic cpu id maps to APIC id (GIC id) is also implemented, it is needed for acpi processor drivers.
Signed-off-by: Hanjun Guo hanjun.guo@linaro.org --- arch/arm64/include/asm/acpi.h | 10 ++-- arch/arm64/kernel/setup.c | 2 + arch/arm64/kernel/smp.c | 2 + drivers/acpi/plat/arm-core.c | 118 +++++++++++++++++++++++++++++++++++++++++ 4 files changed, 129 insertions(+), 3 deletions(-)
diff --git a/arch/arm64/include/asm/acpi.h b/arch/arm64/include/asm/acpi.h index be2951c..423a32c 100644 --- a/arch/arm64/include/asm/acpi.h +++ b/arch/arm64/include/asm/acpi.h @@ -76,9 +76,6 @@ static inline void acpi_disable_pci(void) /* FIXME: this function should be moved to topology.h when it's ready */ void arch_fix_phys_package_id(int num, u32 slot);
-/* temperally define -1 to make acpi core compilerable */ -#define cpu_physical_id(cpu) -1 - /* Low-level suspend routine. */ extern int (*acpi_suspend_lowlevel)(void); #define acpi_wakeup_address (0) @@ -86,6 +83,13 @@ extern int (*acpi_suspend_lowlevel)(void); #define MAX_GIC_CPU_INTERFACE 256 #define MAX_GIC_DISTRIBUTOR 1 /* should be the same as MAX_GIC_NR */
+/* map logic cpu id to physical GIC id */ +extern int arm_cpu_to_apicid[NR_CPUS]; +extern int boot_cpu_apic_id; +#define cpu_physical_id(cpu) arm_cpu_to_apicid[cpu] + +extern void prefill_possible_map(void); + #else /* !CONFIG_ACPI */ #define acpi_disabled 1 /* ACPI sometimes enabled on ARM */ #define acpi_noirq 1 /* ACPI sometimes enabled on ARM */ diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c index 8199360..08f11e2 100644 --- a/arch/arm64/kernel/setup.c +++ b/arch/arm64/kernel/setup.c @@ -231,7 +231,9 @@ void __init setup_arch(char **cmdline_p) */ acpi_boot_table_init(); early_acpi_boot_init(); + boot_cpu_apic_id = read_cpuid_mpidr() & MPIDR_HWID_BITMASK; acpi_boot_init(); + prefill_possible_map();
paging_init(); request_standard_resources(); diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c index a0c2ca6..1428024 100644 --- a/arch/arm64/kernel/smp.c +++ b/arch/arm64/kernel/smp.c @@ -420,7 +420,9 @@ void __init smp_prepare_cpus(unsigned int max_cpus) if (err) continue;
+#ifndef CONFIG_ACPI set_cpu_present(cpu, true); +#endif max_cpus--; } } diff --git a/drivers/acpi/plat/arm-core.c b/drivers/acpi/plat/arm-core.c index 45ff625..8527ecc 100644 --- a/drivers/acpi/plat/arm-core.c +++ b/drivers/acpi/plat/arm-core.c @@ -58,6 +58,13 @@ EXPORT_SYMBOL(acpi_pci_disabled); */ static u64 acpi_lapic_addr __initdata;
+/* available_cpus here means enabled cpu in MADT */ +int available_cpus; + +/* Map logic cpu id to physical GIC id. */ +int arm_cpu_to_apicid[NR_CPUS] = { [0 ... NR_CPUS-1] = -1 }; +int boot_cpu_apic_id = -1; + #define BAD_MADT_ENTRY(entry, end) ( \ (!entry) || (unsigned long)entry + sizeof(*entry) > end || \ ((struct acpi_subtable_header *)entry)->length < sizeof(*entry)) @@ -142,6 +149,39 @@ static int __init acpi_parse_madt(struct acpi_table_header *table) * Please refer to chapter5.2.12.14/15 of ACPI 5.0 */
+static void acpi_register_gic_cpu_interface(int id, u8 enabled) +{ + int cpu; + + if (id >= MAX_GIC_CPU_INTERFACE) { + pr_info(PREFIX "skipped apicid that is too big\n"); + return; + } + + total_cpus++; + if (!enabled) + return; + + available_cpus++; + + /* allocate a logic cpu id for the new comer */ + if (boot_cpu_apic_id == id) { + /* + * boot_cpu_init() already hold bit 0 in cpu_present_mask + * for BSP, no need to allocte again. + */ + cpu = 0; + } else { + cpu = cpumask_next_zero(-1, cpu_present_mask); + } + + /* map the logic cpu id to APIC id */ + arm_cpu_to_apicid[cpu] = id; + + set_cpu_present(cpu, true); + set_cpu_possible(cpu, true); +} + static int __init acpi_parse_gic(struct acpi_subtable_header *header, const unsigned long end) { @@ -154,6 +194,16 @@ acpi_parse_gic(struct acpi_subtable_header *header, const unsigned long end)
acpi_table_print_madt_entry(header);
+ /* + * We need to register disabled CPU as well to permit + * counting disabled CPUs. This allows us to size + * cpus_possible_map more accurately, to permit + * to not preallocating memory for all NR_CPUS + * when we use CPU hotplug. + */ + acpi_register_gic_cpu_interface(processor->gic_id, + processor->flags & ACPI_MADT_ENABLED); + return 0; }
@@ -196,6 +246,19 @@ static int __init acpi_parse_madt_gic_entries(void) return count; }
+#ifdef CONFIG_SMP + if (available_cpus == 0) { + pr_info(PREFIX "Found 0 CPUs; assuming 1\n"); + /* FIXME: should be the real GIC id read from hardware */ + arm_cpu_to_apicid[available_cpus] = 0; + available_cpus = 1; /* We've got at least one of these */ + } +#endif + + /* Make boot-up look pretty */ + pr_info("%d CPUs available, %d CPUs total\n", available_cpus, + total_cpus); + return 0; }
@@ -221,6 +284,61 @@ static int __init acpi_parse_madt_gic_distributor_entries(void) return 0; }
+static int setup_possible_cpus __initdata = -1; +static int __init _setup_possible_cpus(char *str) +{ + get_option(&str, &setup_possible_cpus); + return 0; +} +early_param("possible_cpus", _setup_possible_cpus); + +/* + * cpu_possible_mask should be static, it cannot change as cpu's + * are onlined, or offlined. The reason is per-cpu data-structures + * are allocated by some modules at init time, and dont expect to + * do this dynamically on cpu arrival/departure. + * cpu_present_mask on the other hand can change dynamically. + * In case when cpu_hotplug is not compiled, then we resort to current + * behaviour, which is cpu_possible == cpu_present. + * - Ashok Raj + * + * Three ways to find out the number of additional hotplug CPUs: + * - If the BIOS specified disabled CPUs in ACPI/mptables use that. + * - The user can overwrite it with possible_cpus=NUM + * - Otherwise don't reserve additional CPUs. + * We do this because additional CPUs waste a lot of memory. + * -AK + */ +void __init prefill_possible_map(void) +{ + int i; + int possible, disabled_cpus; + + disabled_cpus = total_cpus - available_cpus; + + if (setup_possible_cpus == -1) { + if (disabled_cpus > 0) + setup_possible_cpus = disabled_cpus; + else + setup_possible_cpus = 0; + } + + possible = available_cpus + setup_possible_cpus; + + pr_info("SMP: the system is limited to %d CPUs\n", nr_cpu_ids); + + if (possible > nr_cpu_ids) + possible = nr_cpu_ids; + + pr_info("SMP: Allowing %d CPUs, %d hotplug CPUs\n", + possible, max((possible - available_cpus), 0)); + + for (i = 0; i < possible; i++) + set_cpu_possible(i, true); + for (; i < NR_CPUS; i++) + set_cpu_possible(i, false); +} + int acpi_gsi_to_irq(u32 gsi, unsigned int *irq) { *irq = gsi_to_irq(gsi);
diff --git a/drivers/acpi/plat/arm-core.c b/drivers/acpi/plat/arm-core.c index 45ff625..8527ecc 100644 --- a/drivers/acpi/plat/arm-core.c +++ b/drivers/acpi/plat/arm-core.c @@ -58,6 +58,13 @@ EXPORT_SYMBOL(acpi_pci_disabled); */ static u64 acpi_lapic_addr __initdata; +/* available_cpus here means enabled cpu in MADT */ +int available_cpus;
+/* Map logic cpu id to physical GIC id. */ +int arm_cpu_to_apicid[NR_CPUS] = { [0 ... NR_CPUS-1] = -1 }; +int boot_cpu_apic_id = -1;
static ?
Really shouldn't be leaking names like "available_cpus" out of ACPI into the global namespace
+#ifdef CONFIG_SMP
- if (available_cpus == 0) {
pr_info(PREFIX "Found 0 CPUs; assuming 1\n");/* FIXME: should be the real GIC id read from hardware */arm_cpu_to_apicid[available_cpus] = 0;available_cpus = 1; /* We've got at least one of these */- }
+#endif
Isn't this true uniprocessor (by definition in fact)
- */
+void __init prefill_possible_map(void)
leaking more unprefixed names into the global namespace
On 2013年12月04日 00:57, One Thousand Gnomes wrote:
diff --git a/drivers/acpi/plat/arm-core.c b/drivers/acpi/plat/arm-core.c index 45ff625..8527ecc 100644 --- a/drivers/acpi/plat/arm-core.c +++ b/drivers/acpi/plat/arm-core.c @@ -58,6 +58,13 @@ EXPORT_SYMBOL(acpi_pci_disabled); */ static u64 acpi_lapic_addr __initdata; +/* available_cpus here means enabled cpu in MADT */ +int available_cpus;
+/* Map logic cpu id to physical GIC id. */ +int arm_cpu_to_apicid[NR_CPUS] = { [0 ... NR_CPUS-1] = -1 }; +int boot_cpu_apic_id = -1;
static ?
Really shouldn't be leaking names like "available_cpus" out of ACPI into the global namespace
Ok, will update in next version.
+#ifdef CONFIG_SMP
- if (available_cpus == 0) {
pr_info(PREFIX "Found 0 CPUs; assuming 1\n");/* FIXME: should be the real GIC id read from hardware */arm_cpu_to_apicid[available_cpus] = 0;available_cpus = 1; /* We've got at least one of these */- }
+#endif
Isn't this true uniprocessor (by definition in fact)
This code is intend to handle some buggy firmware I think.
- */
+void __init prefill_possible_map(void)
leaking more unprefixed names into the global namespace
prefill_possible_map() will be called in setup_arch() in setup.c, and should be gloabl, is this incorrect? Look forward to your advice
Thanks Hanjun
On Wed, Dec 4, 2013 at 8:21 AM, Hanjun Guo hanjun.guo@linaro.org wrote:
On 2013年12月04日 00:57, One Thousand Gnomes wrote:
diff --git a/drivers/acpi/plat/arm-core.c b/drivers/acpi/plat/arm-core.c index 45ff625..8527ecc 100644 --- a/drivers/acpi/plat/arm-core.c +++ b/drivers/acpi/plat/arm-core.c @@ -58,6 +58,13 @@ EXPORT_SYMBOL(acpi_pci_disabled); */ static u64 acpi_lapic_addr __initdata; +/* available_cpus here means enabled cpu in MADT */ +int available_cpus;
+/* Map logic cpu id to physical GIC id. */ +int arm_cpu_to_apicid[NR_CPUS] = { [0 ... NR_CPUS-1] = -1 }; +int boot_cpu_apic_id = -1;
static ?
Really shouldn't be leaking names like "available_cpus" out of ACPI into the global namespace
Ok, will update in next version.
+#ifdef CONFIG_SMP
if (available_cpus == 0) {pr_info(PREFIX "Found 0 CPUs; assuming 1\n");/* FIXME: should be the real GIC id read from hardware */arm_cpu_to_apicid[available_cpus] = 0;available_cpus = 1; /* We've got at least one ofthese */
}+#endif
Isn't this true uniprocessor (by definition in fact)
This code is intend to handle some buggy firmware I think.
Really? We have production firmware already that we need to work-around? That's impressive given there is no production h/w.
Rob
On Tue, Dec 3, 2013 at 10:39 AM, Hanjun Guo hanjun.guo@linaro.org wrote:
When boot the kernel with MADT, the cpu possible and present maps should be prefilled for cpu topology and acpi based cpu hot-plug.
The logic cpu id maps to APIC id (GIC id) is also implemented, it is needed for acpi processor drivers.
Signed-off-by: Hanjun Guo hanjun.guo@linaro.org
arch/arm64/include/asm/acpi.h | 10 ++-- arch/arm64/kernel/setup.c | 2 + arch/arm64/kernel/smp.c | 2 + drivers/acpi/plat/arm-core.c | 118 +++++++++++++++++++++++++++++++++++++++++ 4 files changed, 129 insertions(+), 3 deletions(-)
[snip]
diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c index a0c2ca6..1428024 100644 --- a/arch/arm64/kernel/smp.c +++ b/arch/arm64/kernel/smp.c @@ -420,7 +420,9 @@ void __init smp_prepare_cpus(unsigned int max_cpus) if (err) continue;
+#ifndef CONFIG_ACPI set_cpu_present(cpu, true); +#endif
Should this be moved to DT cpu topology related code?
max_cpus--; }} diff --git a/drivers/acpi/plat/arm-core.c b/drivers/acpi/plat/arm-core.c index 45ff625..8527ecc 100644 --- a/drivers/acpi/plat/arm-core.c +++ b/drivers/acpi/plat/arm-core.c
[snip]
@@ -221,6 +284,61 @@ static int __init acpi_parse_madt_gic_distributor_entries(void) return 0; }
+static int setup_possible_cpus __initdata = -1; +static int __init _setup_possible_cpus(char *str) +{
get_option(&str, &setup_possible_cpus);return 0;+} +early_param("possible_cpus", _setup_possible_cpus);
This does not seem ACPI or ARM specific.
+/*
- cpu_possible_mask should be static, it cannot change as cpu's
- are onlined, or offlined. The reason is per-cpu data-structures
- are allocated by some modules at init time, and dont expect to
- do this dynamically on cpu arrival/departure.
- cpu_present_mask on the other hand can change dynamically.
- In case when cpu_hotplug is not compiled, then we resort to current
- behaviour, which is cpu_possible == cpu_present.
- Ashok Raj
- Three ways to find out the number of additional hotplug CPUs:
- If the BIOS specified disabled CPUs in ACPI/mptables use that.
- The user can overwrite it with possible_cpus=NUM
- Otherwise don't reserve additional CPUs.
- We do this because additional CPUs waste a lot of memory.
- -AK
- */
+void __init prefill_possible_map(void) +{
int i;int possible, disabled_cpus;disabled_cpus = total_cpus - available_cpus;if (setup_possible_cpus == -1) {if (disabled_cpus > 0)setup_possible_cpus = disabled_cpus;elsesetup_possible_cpus = 0;}possible = available_cpus + setup_possible_cpus;pr_info("SMP: the system is limited to %d CPUs\n", nr_cpu_ids);if (possible > nr_cpu_ids)possible = nr_cpu_ids;pr_info("SMP: Allowing %d CPUs, %d hotplug CPUs\n",possible, max((possible - available_cpus), 0));for (i = 0; i < possible; i++)set_cpu_possible(i, true);for (; i < NR_CPUS; i++)set_cpu_possible(i, false);+}
This does not seem ACPI or ARM specific either.
Rob
On Wed, Dec 04, 2013 at 09:47:49AM -0600, Rob Herring wrote:
On Tue, Dec 3, 2013 at 10:39 AM, Hanjun Guo hanjun.guo@linaro.org wrote:
+#ifndef CONFIG_ACPI set_cpu_present(cpu, true); +#endif
Should this be moved to DT cpu topology related code?
Part of the trick there being that there isn't any CPU topology code for ARMv8 in mainline yet, DT or otherwise - I'm working with some patches for doing that with DT which I'm hoping to get out soon.
On 2013年12月04日 23:47, Rob Herring wrote:
On Tue, Dec 3, 2013 at 10:39 AM, Hanjun Guo hanjun.guo@linaro.org wrote:
When boot the kernel with MADT, the cpu possible and present maps should be prefilled for cpu topology and acpi based cpu hot-plug.
The logic cpu id maps to APIC id (GIC id) is also implemented, it is needed for acpi processor drivers.
Signed-off-by: Hanjun Guo hanjun.guo@linaro.org
arch/arm64/include/asm/acpi.h | 10 ++-- arch/arm64/kernel/setup.c | 2 + arch/arm64/kernel/smp.c | 2 + drivers/acpi/plat/arm-core.c | 118 +++++++++++++++++++++++++++++++++++++++++ 4 files changed, 129 insertions(+), 3 deletions(-)
[snip]
diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c index a0c2ca6..1428024 100644 --- a/arch/arm64/kernel/smp.c +++ b/arch/arm64/kernel/smp.c @@ -420,7 +420,9 @@ void __init smp_prepare_cpus(unsigned int max_cpus) if (err) continue;
+#ifndef CONFIG_ACPI set_cpu_present(cpu, true); +#endif
Should this be moved to DT cpu topology related code?
I didn't see the code in the mainline, Mark Brown is working on it now?
max_cpus--; }} diff --git a/drivers/acpi/plat/arm-core.c b/drivers/acpi/plat/arm-core.c index 45ff625..8527ecc 100644 --- a/drivers/acpi/plat/arm-core.c +++ b/drivers/acpi/plat/arm-core.c
[snip]
@@ -221,6 +284,61 @@ static int __init acpi_parse_madt_gic_distributor_entries(void) return 0; }
+static int setup_possible_cpus __initdata = -1; +static int __init _setup_possible_cpus(char *str) +{
get_option(&str, &setup_possible_cpus);return 0;+} +early_param("possible_cpus", _setup_possible_cpus);
This does not seem ACPI or ARM specific.
+/*
- cpu_possible_mask should be static, it cannot change as cpu's
- are onlined, or offlined. The reason is per-cpu data-structures
- are allocated by some modules at init time, and dont expect to
- do this dynamically on cpu arrival/departure.
- cpu_present_mask on the other hand can change dynamically.
- In case when cpu_hotplug is not compiled, then we resort to current
- behaviour, which is cpu_possible == cpu_present.
- Ashok Raj
- Three ways to find out the number of additional hotplug CPUs:
- If the BIOS specified disabled CPUs in ACPI/mptables use that.
- The user can overwrite it with possible_cpus=NUM
- Otherwise don't reserve additional CPUs.
- We do this because additional CPUs waste a lot of memory.
- -AK
- */
+void __init prefill_possible_map(void) +{
int i;int possible, disabled_cpus;disabled_cpus = total_cpus - available_cpus;if (setup_possible_cpus == -1) {if (disabled_cpus > 0)setup_possible_cpus = disabled_cpus;elsesetup_possible_cpus = 0;}possible = available_cpus + setup_possible_cpus;pr_info("SMP: the system is limited to %d CPUs\n", nr_cpu_ids);if (possible > nr_cpu_ids)possible = nr_cpu_ids;pr_info("SMP: Allowing %d CPUs, %d hotplug CPUs\n",possible, max((possible - available_cpus), 0));for (i = 0; i < possible; i++)set_cpu_possible(i, true);for (; i < NR_CPUS; i++)set_cpu_possible(i, false);+}
This does not seem ACPI or ARM specific either.
I think possible map here is related to ACPI based CPU hot-plug, that's why I introduce the code here, if anything I'm wrong, please correct me.
Thanks Hanjun
On Wednesday 04 December 2013, Rob Herring wrote:
index a0c2ca6..1428024 100644 --- a/arch/arm64/kernel/smp.c +++ b/arch/arm64/kernel/smp.c @@ -420,7 +420,9 @@ void __init smp_prepare_cpus(unsigned int max_cpus) if (err) continue;
+#ifndef CONFIG_ACPI set_cpu_present(cpu, true); +#endif
Should this be moved to DT cpu topology related code?
More importantly, the #ifndef is certainly wrong here: It is important that you can turn CONFIG_ACPI on or off without impacting the run-time code path for non-ACPI systems. The snippet above breaks this because we no longer set the cpu mask when ACPI is turned on but not used.
Arnd
On 2013-12-6 7:09, Arnd Bergmann wrote:
On Wednesday 04 December 2013, Rob Herring wrote:
index a0c2ca6..1428024 100644 --- a/arch/arm64/kernel/smp.c +++ b/arch/arm64/kernel/smp.c @@ -420,7 +420,9 @@ void __init smp_prepare_cpus(unsigned int max_cpus) if (err) continue;
+#ifndef CONFIG_ACPI set_cpu_present(cpu, true); +#endif
Should this be moved to DT cpu topology related code?
More importantly, the #ifndef is certainly wrong here: It is important that you can turn CONFIG_ACPI on or off without impacting the run-time code path for non-ACPI systems. The snippet above breaks this because we no longer set the cpu mask when ACPI is turned on but not used.
Good point, I'll rework this patch to find a better solution.
Thanks for your comments.
Hanjun
Get apic id from MADT or _MAT method is not implemented on arm/arm64, and ACPI 5.0 introduces GIC Structure for it, so this patch introduces map_gic_id() to get apic id followed the ACPI 5.0 spec.
Signed-off-by: Hanjun Guo hanjun.guo@linaro.org --- drivers/acpi/processor_core.c | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+)
diff --git a/drivers/acpi/processor_core.c b/drivers/acpi/processor_core.c index 9931435..a83b01c 100644 --- a/drivers/acpi/processor_core.c +++ b/drivers/acpi/processor_core.c @@ -89,6 +89,27 @@ static int map_lsapic_id(struct acpi_subtable_header *entry, return 1; }
+static int map_gic_id(struct acpi_subtable_header *entry, + int device_declaration, u32 acpi_id, int *apic_id) +{ + struct acpi_madt_generic_interrupt *gic = + (struct acpi_madt_generic_interrupt *)entry; + + if (!(gic->flags & ACPI_MADT_ENABLED)) + return 0; + + /* 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 && (gic->uid == acpi_id)) { + *apic_id = gic->gic_id; + return 1; + } + + return 0; +} + static int map_madt_entry(int type, u32 acpi_id) { unsigned long madt_end, entry; @@ -124,6 +145,9 @@ static int 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, &apic_id)) break; + } else if (header->type == ACPI_MADT_TYPE_GENERIC_INTERRUPT) { + if (map_gic_id(header, type, acpi_id, &apic_id)) + break; } entry += header->length; } @@ -154,6 +178,8 @@ static int map_mat_entry(acpi_handle handle, int type, u32 acpi_id) map_lapic_id(header, acpi_id, &apic_id); } else if (header->type == ACPI_MADT_TYPE_LOCAL_SAPIC) { map_lsapic_id(header, type, acpi_id, &apic_id); + } else if (header->type == ACPI_MADT_TYPE_GENERIC_INTERRUPT) { + map_gic_id(header, type, acpi_id, &apic_id); }
exit:
Parked Address in GIC structure can be used as cpu release address for spin table SMP initialisation.
This patch gets parked address from MADT and use it for SMP initialisation when DT is not available.
Signed-off-by: Hanjun Guo hanjun.guo@linaro.org --- arch/arm64/include/asm/acpi.h | 3 +++ arch/arm64/kernel/smp_spin_table.c | 16 +++++++++--- drivers/acpi/plat/arm-core.c | 50 ++++++++++++++++++++++++++++++++++++ 3 files changed, 66 insertions(+), 3 deletions(-)
diff --git a/arch/arm64/include/asm/acpi.h b/arch/arm64/include/asm/acpi.h index 423a32c..180de4a 100644 --- a/arch/arm64/include/asm/acpi.h +++ b/arch/arm64/include/asm/acpi.h @@ -90,6 +90,9 @@ extern int boot_cpu_apic_id;
extern void prefill_possible_map(void);
+extern int acpi_get_parked_address_with_gic_id(u32 gic_id, + u64 *parked_address); + #else /* !CONFIG_ACPI */ #define acpi_disabled 1 /* ACPI sometimes enabled on ARM */ #define acpi_noirq 1 /* ACPI sometimes enabled on ARM */ diff --git a/arch/arm64/kernel/smp_spin_table.c b/arch/arm64/kernel/smp_spin_table.c index 44c2280..7997873 100644 --- a/arch/arm64/kernel/smp_spin_table.c +++ b/arch/arm64/kernel/smp_spin_table.c @@ -25,6 +25,7 @@ #include <asm/cpu_ops.h> #include <asm/cputype.h> #include <asm/smp_plat.h> +#include <asm/acpi.h>
extern void secondary_holding_pen(void); volatile unsigned long secondary_holding_pen_release = INVALID_HWID; @@ -47,6 +48,11 @@ static void write_pen_release(u64 val) __flush_dcache_area(start, size); }
+static int get_cpu_release_addr_acpi(unsigned int cpu, u64 *parked_address) +{ + return acpi_get_parked_address_with_gic_id(arm_cpu_to_apicid[cpu], + parked_address); +}
static int smp_spin_table_cpu_init(struct device_node *dn, unsigned int cpu) { @@ -55,10 +61,14 @@ static int smp_spin_table_cpu_init(struct device_node *dn, unsigned int cpu) */ if (of_property_read_u64(dn, "cpu-release-addr", &cpu_release_addr[cpu])) { - pr_err("CPU %d: missing or invalid cpu-release-addr property\n", - cpu);
- return -1; + /* try ACPI way */ + if (get_cpu_release_addr_acpi(cpu, &cpu_release_addr[cpu])) { + pr_err("CPU %d: missing or invalid cpu-release-addr property\n", + cpu); + + return -1; + } }
return 0; diff --git a/drivers/acpi/plat/arm-core.c b/drivers/acpi/plat/arm-core.c index 8527ecc..c4c8c68 100644 --- a/drivers/acpi/plat/arm-core.c +++ b/drivers/acpi/plat/arm-core.c @@ -262,6 +262,56 @@ static int __init acpi_parse_madt_gic_entries(void) return 0; }
+/* Parked Address in ACPI GIC structure can be used as cpu release addr */ +int acpi_get_parked_address_with_gic_id(u32 gic_id, u64 *parked_address) +{ + struct acpi_table_header *table_header = NULL; + struct acpi_subtable_header *entry; + int err = 0; + unsigned long table_end; + acpi_size tbl_size; + struct acpi_madt_generic_interrupt *processor = NULL; + + if (!parked_address) + return -EINVAL; + + acpi_get_table_with_size(ACPI_SIG_MADT, 0, &table_header, &tbl_size); + if (!table_header) { + pr_warn(PREFIX "MADT table not present\n"); + return -ENODEV; + } + + table_end = (unsigned long)table_header + table_header->length; + + /* Parse all entries looking for a match. */ + entry = (struct acpi_subtable_header *) + ((unsigned long)table_header + sizeof(struct acpi_table_madt)); + + while (((unsigned long)entry) + sizeof(struct acpi_subtable_header) < + table_end) { + if (entry->type != ACPI_MADT_TYPE_GENERIC_INTERRUPT + || BAD_MADT_ENTRY(entry, table_end)) + continue; + + processor = (struct acpi_madt_generic_interrupt *)entry; + + if (processor->gic_id == gic_id) { + *parked_address = processor->parked_address; + goto out; + } + + entry = (struct acpi_subtable_header *) + ((unsigned long)entry + entry->length); + } + + err = -ENODEV; +out: + if (!acpi_gbl_permanent_mmap) + __acpi_unmap_table((char *)table_header, tbl_size); + + return err; +} + /* * Parse GIC distributor related entries in MADT * returns 0 on success, < 0 on error
Needed because arm uses GIC which is defined in ACPI 5.0 spec.
Signed-off-by: Al Stone al.stone@linaro.org Signed-off-by: Hanjun Guo hanjun.guo@linaro.org --- drivers/acpi/bus.c | 3 +++ drivers/acpi/plat/arm-core.c | 6 +++++- include/linux/acpi.h | 1 + 3 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c index a79273a..b1fed60 100644 --- a/drivers/acpi/bus.c +++ b/drivers/acpi/bus.c @@ -410,6 +410,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/drivers/acpi/plat/arm-core.c b/drivers/acpi/plat/arm-core.c index c4c8c68..9cc0208 100644 --- a/drivers/acpi/plat/arm-core.c +++ b/drivers/acpi/plat/arm-core.c @@ -82,7 +82,11 @@ EXPORT_SYMBOL_GPL(arch_fix_phys_package_id); * Boot-time Configuration */
-enum acpi_irq_model_id acpi_irq_model = ACPI_IRQ_MODEL_PLATFORM; +/* + * 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;
static unsigned int gsi_to_irq(unsigned int gsi) { diff --git a/include/linux/acpi.h b/include/linux/acpi.h index 115c610..1e6a0ac 100644 --- a/include/linux/acpi.h +++ b/include/linux/acpi.h @@ -69,6 +69,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 };
If we set the GIC as the default domain then we can access it for IRQ mapping within the ACPI code.
Signed-off-by: Graeme Gregory graeme.gregory@linaro.org Signed-off-by: Hanjun Guo hanjun.guo@linaro.org --- drivers/irqchip/irq-gic.c | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c index 9031171..f522c9a 100644 --- a/drivers/irqchip/irq-gic.c +++ b/drivers/irqchip/irq-gic.c @@ -957,6 +957,13 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start, if (WARN_ON(!gic->domain)) return;
+ /* + * do not set default host for GIC domain multi-times. + * FIXME: This probably needs revisited when multi GICs supported + */ + if (!gic_nr) + irq_set_default_host(gic->domain); + #ifdef CONFIG_SMP set_smp_cross_call(gic_raise_softirq); register_cpu_notifier(&gic_cpu_notifier);
From: Amit Daniel Kachhap amit.daniel@samsung.com
This patch introduces a new API for acpi based irq mapping.
[hanjun: Rework this patch to delete the reference to gic_irq_domain_xlate() which can simplify the code a lot.]
Signed-off-by: Amit Daniel Kachhap amit.daniel@samsung.com Signed-off-by: Hanjun Guo hanjun.guo@linaro.org --- include/linux/acpi.h | 2 ++ kernel/irq/irqdomain.c | 27 +++++++++++++++++++++++++++ 2 files changed, 29 insertions(+)
diff --git a/include/linux/acpi.h b/include/linux/acpi.h index 1e6a0ac..edd5806 100644 --- a/include/linux/acpi.h +++ b/include/linux/acpi.h @@ -141,6 +141,8 @@ int acpi_map_lsapic(acpi_handle handle, int physid, int *pcpu); int acpi_unmap_lsapic(int cpu); #endif /* CONFIG_ACPI_HOTPLUG_CPU */
+unsigned int irq_create_acpi_mapping(irq_hw_number_t hwirq, + unsigned int type); int acpi_register_ioapic(acpi_handle handle, u64 phys_addr, u32 gsi_base); int acpi_unregister_ioapic(acpi_handle handle, u32 gsi_base); void acpi_irq_stats_init(void); diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c index cf68bb3..c661552 100644 --- a/kernel/irq/irqdomain.c +++ b/kernel/irq/irqdomain.c @@ -501,6 +501,33 @@ unsigned int irq_create_of_mapping(struct of_phandle_args *irq_data) } EXPORT_SYMBOL_GPL(irq_create_of_mapping);
+#ifdef CONFIG_ACPI +unsigned int irq_create_acpi_mapping(irq_hw_number_t hwirq, + unsigned int type) +{ + struct irq_domain *domain; + unsigned int virq; + + domain = irq_default_domain; + if (!domain) { + pr_warn("no irq domain found !\n"); + return 0; + } + + /* Create mapping */ + virq = irq_create_mapping(domain, hwirq); + if (!virq) + return virq; + + /* Set type if specified and different than the current one */ + if (type != IRQ_TYPE_NONE && + type != irq_get_trigger_type(virq)) + irq_set_irq_type(virq, type); + return virq; +} +EXPORT_SYMBOL_GPL(irq_create_acpi_mapping); +#endif + /** * irq_dispose_mapping() - Unmap an interrupt * @virq: linux irq number of the interrupt to unmap
On Tue, Dec 3, 2013 at 10:39 AM, Hanjun Guo hanjun.guo@linaro.org wrote:
From: Amit Daniel Kachhap amit.daniel@samsung.com
This patch introduces a new API for acpi based irq mapping.
[hanjun: Rework this patch to delete the reference to gic_irq_domain_xlate() which can simplify the code a lot.]
Signed-off-by: Amit Daniel Kachhap amit.daniel@samsung.com Signed-off-by: Hanjun Guo hanjun.guo@linaro.org
include/linux/acpi.h | 2 ++ kernel/irq/irqdomain.c | 27 +++++++++++++++++++++++++++ 2 files changed, 29 insertions(+)
diff --git a/include/linux/acpi.h b/include/linux/acpi.h index 1e6a0ac..edd5806 100644 --- a/include/linux/acpi.h +++ b/include/linux/acpi.h @@ -141,6 +141,8 @@ int acpi_map_lsapic(acpi_handle handle, int physid, int *pcpu); int acpi_unmap_lsapic(int cpu); #endif /* CONFIG_ACPI_HOTPLUG_CPU */
+unsigned int irq_create_acpi_mapping(irq_hw_number_t hwirq,
unsigned int type);int acpi_register_ioapic(acpi_handle handle, u64 phys_addr, u32 gsi_base); int acpi_unregister_ioapic(acpi_handle handle, u32 gsi_base); void acpi_irq_stats_init(void); diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c index cf68bb3..c661552 100644 --- a/kernel/irq/irqdomain.c +++ b/kernel/irq/irqdomain.c @@ -501,6 +501,33 @@ unsigned int irq_create_of_mapping(struct of_phandle_args *irq_data) } EXPORT_SYMBOL_GPL(irq_create_of_mapping);
+#ifdef CONFIG_ACPI +unsigned int irq_create_acpi_mapping(irq_hw_number_t hwirq,
unsigned int type)+{
struct irq_domain *domain;unsigned int virq;domain = irq_default_domain;if (!domain) {pr_warn("no irq domain found !\n");return 0;}/* Create mapping */virq = irq_create_mapping(domain, hwirq);if (!virq)return virq;/* Set type if specified and different than the current one */if (type != IRQ_TYPE_NONE &&type != irq_get_trigger_type(virq))irq_set_irq_type(virq, type);return virq;+} +EXPORT_SYMBOL_GPL(irq_create_acpi_mapping);
There is nothing ACPI specific about this function. This is simply irq_create_of_mapping w/o translating of_phandle_args to a hwirq and type. So I expect the code to be re-factored here to mirror that.
Rob
On 2013年12月04日 01:25, Rob Herring wrote:
On Tue, Dec 3, 2013 at 10:39 AM, Hanjun Guo hanjun.guo@linaro.org wrote:
From: Amit Daniel Kachhap amit.daniel@samsung.com
This patch introduces a new API for acpi based irq mapping.
[hanjun: Rework this patch to delete the reference to gic_irq_domain_xlate() which can simplify the code a lot.]
Signed-off-by: Amit Daniel Kachhap amit.daniel@samsung.com Signed-off-by: Hanjun Guo hanjun.guo@linaro.org
include/linux/acpi.h | 2 ++ kernel/irq/irqdomain.c | 27 +++++++++++++++++++++++++++ 2 files changed, 29 insertions(+)
diff --git a/include/linux/acpi.h b/include/linux/acpi.h index 1e6a0ac..edd5806 100644 --- a/include/linux/acpi.h +++ b/include/linux/acpi.h @@ -141,6 +141,8 @@ int acpi_map_lsapic(acpi_handle handle, int physid, int *pcpu); int acpi_unmap_lsapic(int cpu); #endif /* CONFIG_ACPI_HOTPLUG_CPU */
+unsigned int irq_create_acpi_mapping(irq_hw_number_t hwirq,
unsigned int type);diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c index cf68bb3..c661552 100644 --- a/kernel/irq/irqdomain.c +++ b/kernel/irq/irqdomain.c @@ -501,6 +501,33 @@ unsigned int irq_create_of_mapping(struct of_phandle_args *irq_data) } EXPORT_SYMBOL_GPL(irq_create_of_mapping);
+#ifdef CONFIG_ACPI +unsigned int irq_create_acpi_mapping(irq_hw_number_t hwirq,
unsigned int type)+{
struct irq_domain *domain;unsigned int virq;domain = irq_default_domain;if (!domain) {pr_warn("no irq domain found !\n");return 0;}/* Create mapping */virq = irq_create_mapping(domain, hwirq);if (!virq)return virq;/* Set type if specified and different than the current one */if (type != IRQ_TYPE_NONE &&type != irq_get_trigger_type(virq))irq_set_irq_type(virq, type);return virq;+} +EXPORT_SYMBOL_GPL(irq_create_acpi_mapping);
There is nothing ACPI specific about this function. This is simply irq_create_of_mapping w/o translating of_phandle_args to a hwirq and type. So I expect the code to be re-factored here to mirror that.
Sorry for my bad english, do you mean create a OF free function and call that from the OF function ?
Thanks Hanjun
On Wed, Dec 4, 2013 at 4:38 PM, Hanjun Guo hanjun.guo@linaro.org wrote:
On 2013年12月04日 01:25, Rob Herring wrote:
On Tue, Dec 3, 2013 at 10:39 AM, Hanjun Guo hanjun.guo@linaro.org wrote:
From: Amit Daniel Kachhap amit.daniel@samsung.com
This patch introduces a new API for acpi based irq mapping.
[hanjun: Rework this patch to delete the reference to gic_irq_domain_xlate() which can simplify the code a lot.]
Signed-off-by: Amit Daniel Kachhap amit.daniel@samsung.com Signed-off-by: Hanjun Guo hanjun.guo@linaro.org
(...)
+EXPORT_SYMBOL_GPL(irq_create_acpi_mapping);
There is nothing ACPI specific about this function. This is simply irq_create_of_mapping w/o translating of_phandle_args to a hwirq and type. So I expect the code to be re-factored here to mirror that.
Sorry for my bad english, do you mean create a OF free function and call that from the OF function ?
Sounds like a good idea, like if you move the OF function into kernel/irq/irqdomain.c and get rid of the OF specific naming then use that same function from ACPI too. It's all about using the irq_default_domain in a standard way after all right?
Yours, Linus Walleij
This API is similar to DT based irq_of_parse_and_map but does link parent/child IRQ controllers. This is tested for primary GIC PPI and GIC SPI interrupts and not for secondary child irq controllers.
Signed-off-by: Amit Daniel Kachhap amit.daniel@samsung.com Signed-off-by: Hanjun Guo hanjun.guo@linaro.org --- drivers/acpi/plat/arm-core.c | 36 ++++++++++++++++++++++++++++++++++-- 1 file changed, 34 insertions(+), 2 deletions(-)
diff --git a/drivers/acpi/plat/arm-core.c b/drivers/acpi/plat/arm-core.c index 9cc0208..17c99e1 100644 --- a/drivers/acpi/plat/arm-core.c +++ b/drivers/acpi/plat/arm-core.c @@ -90,7 +90,7 @@ enum acpi_irq_model_id acpi_irq_model = ACPI_IRQ_MODEL_GIC;
static unsigned int gsi_to_irq(unsigned int gsi) { - int irq = irq_create_mapping(NULL, gsi); + int irq = irq_find_mapping(NULL, gsi);
return irq; } @@ -407,7 +407,39 @@ EXPORT_SYMBOL_GPL(acpi_gsi_to_irq); */ int acpi_register_gsi(struct device *dev, u32 gsi, int trigger, int polarity) { - return -1; + 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, using identity mapping for hwirq: + * 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; + + irq = irq_create_acpi_mapping(gsi, irq_type); + return irq; } EXPORT_SYMBOL_GPL(acpi_register_gsi);
On Tuesday 03 December 2013, Hanjun Guo wrote:
/** 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, using identity mapping for hwirq:* PPI interrupt: in the range [16, 31];* SPI interrupt: in the range [32, 1019];
This difference might cause endless confusion. Can't you register PPI and SPI as separate IRQ controllers to have the same number space that we normally have?
Arnd
On 2013年12月05日 11:48, Arnd Bergmann wrote:
On Tuesday 03 December 2013, Hanjun Guo wrote:
/** 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, using identity mapping for hwirq:* PPI interrupt: in the range [16, 31];* SPI interrupt: in the range [32, 1019];This difference might cause endless confusion. Can't you register PPI and SPI as separate IRQ controllers to have the same number space that we normally have?
In ACPI, they used a conception named GSI (Global System Interrupts) for irq, GSI number can not be the same even if there are muti GICs, so I use the identity mapping for hwirq for ACPI.
Thanks you very much for your comments :)
Hanjun
In MADT table, there are GIC cpu interface base address and GIC distributor base address, use them to convert GIC to ACPI.
Signed-off-by: Hanjun Guo hanjun.guo@linaro.org --- arch/arm64/kernel/irq.c | 5 ++++ drivers/acpi/plat/arm-core.c | 66 ++++++++++++++++++++++++++++++++++++------ include/linux/acpi.h | 6 ++++ 3 files changed, 68 insertions(+), 9 deletions(-)
diff --git a/arch/arm64/kernel/irq.c b/arch/arm64/kernel/irq.c index 473e5db..a9e68bf 100644 --- a/arch/arm64/kernel/irq.c +++ b/arch/arm64/kernel/irq.c @@ -25,6 +25,7 @@ #include <linux/irq.h> #include <linux/smp.h> #include <linux/init.h> +#include <linux/acpi.h> #include <linux/irqchip.h> #include <linux/seq_file.h> #include <linux/ratelimit.h> @@ -78,6 +79,10 @@ void __init set_handle_irq(void (*handle_irq)(struct pt_regs *)) void __init init_IRQ(void) { irqchip_init(); + + if (!handle_arch_irq) + acpi_gic_init(); + if (!handle_arch_irq) panic("No interrupt controller found."); } diff --git a/drivers/acpi/plat/arm-core.c b/drivers/acpi/plat/arm-core.c index 17c99e1..509b847 100644 --- a/drivers/acpi/plat/arm-core.c +++ b/drivers/acpi/plat/arm-core.c @@ -29,6 +29,7 @@ #include <linux/module.h> #include <linux/irq.h> #include <linux/irqdomain.h> +#include <linux/irqchip/arm-gic.h> #include <linux/slab.h> #include <linux/bootmem.h> #include <linux/ioport.h> @@ -211,11 +212,21 @@ acpi_parse_gic(struct acpi_subtable_header *header, const unsigned long end) return 0; }
+#ifdef CONFIG_ARM_GIC +/* + * Hard code here, we can not get memory size from MADT (but FDT does), + * this size is described in ARMv8 foudation model's User Guide + */ +#define GIC_DISTRIBUTOR_MEMORY_SIZE (SZ_8K) +#define GIC_CPU_INTERFACE_MEMORY_SIZE (SZ_4K) + static int __init acpi_parse_gic_distributor(struct acpi_subtable_header *header, const unsigned long end) { struct acpi_madt_generic_distributor *distributor = NULL; + void __iomem *dist_base = NULL; + void __iomem *cpu_base = NULL;
distributor = (struct acpi_madt_generic_distributor *)header;
@@ -224,8 +235,43 @@ acpi_parse_gic_distributor(struct acpi_subtable_header *header,
acpi_table_print_madt_entry(header);
+ /* GIC is initialised after page_init(), no need for early_ioremap */ + dist_base = ioremap(distributor->base_address, + GIC_CPU_INTERFACE_MEMORY_SIZE); + if (!dist_base) { + pr_warn(PREFIX "unable to map gic dist registers\n"); + return -ENOMEM; + } + + /* + * acpi_lapic_addr is stored in acpi_parse_madt(), + * so we can use it here for GIC init + */ + if (acpi_lapic_addr) { + iounmap(dist_base); + pr_warn(PREFIX "Invalid GIC cpu interface base address\n"); + return -EINVAL; + } + + cpu_base = ioremap(acpi_lapic_addr, GIC_CPU_INTERFACE_MEMORY_SIZE); + if (!cpu_base) { + iounmap(dist_base); + pr_warn(PREFIX "unable to map gic cpu registers\n"); + return -ENOMEM; + } + + gic_init(distributor->gic_id, -1, dist_base, cpu_base); + return 0; } +#else +static int __init +acpi_parse_gic_distributor(struct acpi_subtable_header *header, + const unsigned long end) +{ + return -ENODEV; +} +#endif /* CONFIG_ARM_GIC */
/* * Parse GIC cpu interface related entries in MADT @@ -234,7 +280,7 @@ acpi_parse_gic_distributor(struct acpi_subtable_header *header, static int __init acpi_parse_madt_gic_entries(void) { int count; - + /* * do a partial walk of MADT to determine how many CPUs * we have including disabled CPUs @@ -468,19 +514,21 @@ static void __init acpi_process_madt(void) * Parse MADT GIC cpu interface entries */ error = acpi_parse_madt_gic_entries(); - if (!error) { - /* - * Parse MADT GIC distributor entries - */ - acpi_parse_madt_gic_distributor_entries(); - } + if (!error) + pr_info("Using ACPI for processor (GIC) configuration information\n"); }
- pr_info("Using ACPI for processor (GIC) configuration information\n"); - return; }
+int __init acpi_gic_init(void) +{ + /* + * Parse MADT GIC distributor entries + */ + return acpi_parse_madt_gic_distributor_entries(); +} + /* * acpi_boot_table_init() and acpi_boot_init() * called from setup_arch(), always. diff --git a/include/linux/acpi.h b/include/linux/acpi.h index edd5806..58a4bf4 100644 --- a/include/linux/acpi.h +++ b/include/linux/acpi.h @@ -112,6 +112,7 @@ char * __acpi_map_table (unsigned long phys_addr, unsigned long size); void __acpi_unmap_table(char *map, unsigned long size); int early_acpi_boot_init(void); int acpi_boot_init (void); +int acpi_gic_init(void); void acpi_boot_table_init (void); int acpi_mps_check (void); int acpi_numa_init (void); @@ -444,6 +445,11 @@ static inline int acpi_boot_init(void) return 0; }
+static inline int acpi_gic_init(void) +{ + return -ENODEV; +} + static inline void acpi_boot_table_init(void) { return;
On Tue, Dec 3, 2013 at 10:39 AM, Hanjun Guo hanjun.guo@linaro.org wrote:
In MADT table, there are GIC cpu interface base address and GIC distributor base address, use them to convert GIC to ACPI.
Signed-off-by: Hanjun Guo hanjun.guo@linaro.org
arch/arm64/kernel/irq.c | 5 ++++ drivers/acpi/plat/arm-core.c | 66 ++++++++++++++++++++++++++++++++++++------ include/linux/acpi.h | 6 ++++ 3 files changed, 68 insertions(+), 9 deletions(-)
diff --git a/arch/arm64/kernel/irq.c b/arch/arm64/kernel/irq.c index 473e5db..a9e68bf 100644 --- a/arch/arm64/kernel/irq.c +++ b/arch/arm64/kernel/irq.c @@ -25,6 +25,7 @@ #include <linux/irq.h> #include <linux/smp.h> #include <linux/init.h> +#include <linux/acpi.h> #include <linux/irqchip.h> #include <linux/seq_file.h> #include <linux/ratelimit.h> @@ -78,6 +79,10 @@ void __init set_handle_irq(void (*handle_irq)(struct pt_regs *)) void __init init_IRQ(void) { irqchip_init();
if (!handle_arch_irq)acpi_gic_init();if (!handle_arch_irq) panic("No interrupt controller found.");} diff --git a/drivers/acpi/plat/arm-core.c b/drivers/acpi/plat/arm-core.c index 17c99e1..509b847 100644 --- a/drivers/acpi/plat/arm-core.c +++ b/drivers/acpi/plat/arm-core.c @@ -29,6 +29,7 @@ #include <linux/module.h> #include <linux/irq.h> #include <linux/irqdomain.h> +#include <linux/irqchip/arm-gic.h> #include <linux/slab.h> #include <linux/bootmem.h> #include <linux/ioport.h> @@ -211,11 +212,21 @@ acpi_parse_gic(struct acpi_subtable_header *header, const unsigned long end) return 0; }
+#ifdef CONFIG_ARM_GIC
Perhaps this should go in the GIC code? This is more of a general question of where init/probing code goes. For DT, this as been with the driver code.
+/*
- Hard code here, we can not get memory size from MADT (but FDT does),
- this size is described in ARMv8 foudation model's User Guide
- */
+#define GIC_DISTRIBUTOR_MEMORY_SIZE (SZ_8K) +#define GIC_CPU_INTERFACE_MEMORY_SIZE (SZ_4K)
You have the sizes swapped. The cpu interface has the DIR register at 0x1000.
static int __init acpi_parse_gic_distributor(struct acpi_subtable_header *header, const unsigned long end) { struct acpi_madt_generic_distributor *distributor = NULL;
void __iomem *dist_base = NULL;void __iomem *cpu_base = NULL;
Initialization here is unnecessary.
distributor = (struct acpi_madt_generic_distributor *)header;@@ -224,8 +235,43 @@ acpi_parse_gic_distributor(struct acpi_subtable_header *header,
acpi_table_print_madt_entry(header);
/* GIC is initialised after page_init(), no need for early_ioremap */dist_base = ioremap(distributor->base_address,GIC_CPU_INTERFACE_MEMORY_SIZE);
Should be GIC_DISTRIBUTOR_MEMORY_SIZE.
if (!dist_base) {pr_warn(PREFIX "unable to map gic dist registers\n");return -ENOMEM;}/** acpi_lapic_addr is stored in acpi_parse_madt(),* so we can use it here for GIC init*/if (acpi_lapic_addr) {
Checking this first would be cleaner.
iounmap(dist_base);pr_warn(PREFIX "Invalid GIC cpu interface base address\n");return -EINVAL;}cpu_base = ioremap(acpi_lapic_addr, GIC_CPU_INTERFACE_MEMORY_SIZE);
How are gic's with different cpu address per core going to be handled?
if (!cpu_base) {iounmap(dist_base);pr_warn(PREFIX "unable to map gic cpu registers\n");
All the printks are a bit verbose for my tastes. I think a single error print would suffice.
return -ENOMEM;}gic_init(distributor->gic_id, -1, dist_base, cpu_base);return 0;} +#else +static int __init +acpi_parse_gic_distributor(struct acpi_subtable_header *header,
const unsigned long end)+{
return -ENODEV;+} +#endif /* CONFIG_ARM_GIC */
A "if (!IS_ENABLED(CONFIG_ARM_GIC)) return;" in the above function would eliminate this ifdef.
/*
- Parse GIC cpu interface related entries in MADT
@@ -234,7 +280,7 @@ acpi_parse_gic_distributor(struct acpi_subtable_header *header, static int __init acpi_parse_madt_gic_entries(void) { int count;
Unnecessary whitespace change.
/* * do a partial walk of MADT to determine how many CPUs * we have including disabled CPUs@@ -468,19 +514,21 @@ static void __init acpi_process_madt(void) * Parse MADT GIC cpu interface entries */ error = acpi_parse_madt_gic_entries();
if (!error) {/** Parse MADT GIC distributor entries*/acpi_parse_madt_gic_distributor_entries();}
if (!error)pr_info("Using ACPI for processor (GIC) configuration information\n"); }
pr_info("Using ACPI for processor (GIC) configuration information\n");return;}
+int __init acpi_gic_init(void) +{
/** Parse MADT GIC distributor entries*/return acpi_parse_madt_gic_distributor_entries();+}
/*
- acpi_boot_table_init() and acpi_boot_init()
- called from setup_arch(), always.
diff --git a/include/linux/acpi.h b/include/linux/acpi.h index edd5806..58a4bf4 100644 --- a/include/linux/acpi.h +++ b/include/linux/acpi.h @@ -112,6 +112,7 @@ char * __acpi_map_table (unsigned long phys_addr, unsigned long size); void __acpi_unmap_table(char *map, unsigned long size); int early_acpi_boot_init(void); int acpi_boot_init (void); +int acpi_gic_init(void); void acpi_boot_table_init (void); int acpi_mps_check (void); int acpi_numa_init (void); @@ -444,6 +445,11 @@ static inline int acpi_boot_init(void) return 0; }
+static inline int acpi_gic_init(void) +{
return -ENODEV;+}
static inline void acpi_boot_table_init(void) { return; -- 1.7.9.5
linaro-kernel mailing list linaro-kernel@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-kernel
On 2013年12月04日 01:09, Rob Herring wrote:
On Tue, Dec 3, 2013 at 10:39 AM, Hanjun Guo hanjun.guo@linaro.org wrote:
In MADT table, there are GIC cpu interface base address and GIC distributor base address, use them to convert GIC to ACPI.
Signed-off-by: Hanjun Guo hanjun.guo@linaro.org
arch/arm64/kernel/irq.c | 5 ++++ drivers/acpi/plat/arm-core.c | 66 ++++++++++++++++++++++++++++++++++++------ include/linux/acpi.h | 6 ++++ 3 files changed, 68 insertions(+), 9 deletions(-)
diff --git a/arch/arm64/kernel/irq.c b/arch/arm64/kernel/irq.c index 473e5db..a9e68bf 100644 --- a/arch/arm64/kernel/irq.c +++ b/arch/arm64/kernel/irq.c @@ -25,6 +25,7 @@ #include <linux/irq.h> #include <linux/smp.h> #include <linux/init.h> +#include <linux/acpi.h> #include <linux/irqchip.h> #include <linux/seq_file.h> #include <linux/ratelimit.h> @@ -78,6 +79,10 @@ void __init set_handle_irq(void (*handle_irq)(struct pt_regs *)) void __init init_IRQ(void) { irqchip_init();
if (!handle_arch_irq)acpi_gic_init();if (!handle_arch_irq) panic("No interrupt controller found.");diff --git a/drivers/acpi/plat/arm-core.c b/drivers/acpi/plat/arm-core.c index 17c99e1..509b847 100644 --- a/drivers/acpi/plat/arm-core.c +++ b/drivers/acpi/plat/arm-core.c @@ -29,6 +29,7 @@ #include <linux/module.h> #include <linux/irq.h> #include <linux/irqdomain.h> +#include <linux/irqchip/arm-gic.h> #include <linux/slab.h> #include <linux/bootmem.h> #include <linux/ioport.h> @@ -211,11 +212,21 @@ acpi_parse_gic(struct acpi_subtable_header *header, const unsigned long end) return 0; }
+#ifdef CONFIG_ARM_GIC
Perhaps this should go in the GIC code? This is more of a general question of where init/probing code goes. For DT, this as been with the driver code.
I'm ok with your suggestion, how about move the code to drivers/irqchip/irq-gic.c ? is this make sense to you?
+/*
- Hard code here, we can not get memory size from MADT (but FDT does),
- this size is described in ARMv8 foudation model's User Guide
- */
+#define GIC_DISTRIBUTOR_MEMORY_SIZE (SZ_8K) +#define GIC_CPU_INTERFACE_MEMORY_SIZE (SZ_4K)
You have the sizes swapped. The cpu interface has the DIR register at 0x1000.
I will figure out the right size in next version.
- static int __init acpi_parse_gic_distributor(struct acpi_subtable_header *header, const unsigned long end) { struct acpi_madt_generic_distributor *distributor = NULL;
void __iomem *dist_base = NULL;void __iomem *cpu_base = NULL;Initialization here is unnecessary.
ok, will update in next version.
distributor = (struct acpi_madt_generic_distributor *)header;@@ -224,8 +235,43 @@ acpi_parse_gic_distributor(struct acpi_subtable_header *header,
acpi_table_print_madt_entry(header);
/* GIC is initialised after page_init(), no need for early_ioremap */dist_base = ioremap(distributor->base_address,GIC_CPU_INTERFACE_MEMORY_SIZE);Should be GIC_DISTRIBUTOR_MEMORY_SIZE.
Good catch
if (!dist_base) {pr_warn(PREFIX "unable to map gic dist registers\n");return -ENOMEM;}/** acpi_lapic_addr is stored in acpi_parse_madt(),* so we can use it here for GIC init*/if (acpi_lapic_addr) {Checking this first would be cleaner.
Agreed, thank you for the advice, will update it in next version.
iounmap(dist_base);pr_warn(PREFIX "Invalid GIC cpu interface base address\n");return -EINVAL;}cpu_base = ioremap(acpi_lapic_addr, GIC_CPU_INTERFACE_MEMORY_SIZE);How are gic's with different cpu address per core going to be handled?
do you mean some GIC without banked registers? if yes, ACPI can handle that, in the GIC (GIC cpu interface) structure, there is "Physical Base Address" per core, we can use it to handle gic's with different cpu address per core.
This part of code is not implemented yet, if needed, will send out in next version.
if (!cpu_base) {iounmap(dist_base);pr_warn(PREFIX "unable to map gic cpu registers\n");All the printks are a bit verbose for my tastes. I think a single error print would suffice.
do you mean if meet some error, then got to a single error printk?
return -ENOMEM;}gic_init(distributor->gic_id, -1, dist_base, cpu_base);return 0;+#else +static int __init +acpi_parse_gic_distributor(struct acpi_subtable_header *header,
const unsigned long end)+{
return -ENODEV;+} +#endif /* CONFIG_ARM_GIC */
A "if (!IS_ENABLED(CONFIG_ARM_GIC)) return;" in the above function would eliminate this ifdef.
Thanks for the suggestion, will do it
/*
- Parse GIC cpu interface related entries in MADT
@@ -234,7 +280,7 @@ acpi_parse_gic_distributor(struct acpi_subtable_header *header, static int __init acpi_parse_madt_gic_entries(void) { int count;
Unnecessary whitespace change.
will update it :)
Thanks Hanjun
Hi Hanjun,
On 03/12/13 16:39, Hanjun Guo wrote:
In MADT table, there are GIC cpu interface base address and GIC distributor base address, use them to convert GIC to ACPI.
Signed-off-by: Hanjun Guo hanjun.guo@linaro.org
arch/arm64/kernel/irq.c | 5 ++++ drivers/acpi/plat/arm-core.c | 66 ++++++++++++++++++++++++++++++++++++------ include/linux/acpi.h | 6 ++++ 3 files changed, 68 insertions(+), 9 deletions(-)
diff --git a/arch/arm64/kernel/irq.c b/arch/arm64/kernel/irq.c index 473e5db..a9e68bf 100644 --- a/arch/arm64/kernel/irq.c +++ b/arch/arm64/kernel/irq.c @@ -25,6 +25,7 @@ #include <linux/irq.h> #include <linux/smp.h> #include <linux/init.h> +#include <linux/acpi.h> #include <linux/irqchip.h> #include <linux/seq_file.h> #include <linux/ratelimit.h> @@ -78,6 +79,10 @@ void __init set_handle_irq(void (*handle_irq)(struct pt_regs *)) void __init init_IRQ(void) { irqchip_init();
- if (!handle_arch_irq)
acpi_gic_init();
Why is the GIC hardcoded? How are you going to support other interrupt controllers?
if (!handle_arch_irq) panic("No interrupt controller found."); } diff --git a/drivers/acpi/plat/arm-core.c b/drivers/acpi/plat/arm-core.c index 17c99e1..509b847 100644 --- a/drivers/acpi/plat/arm-core.c +++ b/drivers/acpi/plat/arm-core.c @@ -29,6 +29,7 @@ #include <linux/module.h> #include <linux/irq.h> #include <linux/irqdomain.h> +#include <linux/irqchip/arm-gic.h> #include <linux/slab.h> #include <linux/bootmem.h> #include <linux/ioport.h> @@ -211,11 +212,21 @@ acpi_parse_gic(struct acpi_subtable_header *header, const unsigned long end) return 0; } +#ifdef CONFIG_ARM_GIC +/*
- Hard code here, we can not get memory size from MADT (but FDT does),
- this size is described in ARMv8 foudation model's User Guide
- */
+#define GIC_DISTRIBUTOR_MEMORY_SIZE (SZ_8K) +#define GIC_CPU_INTERFACE_MEMORY_SIZE (SZ_4K)
Aside from the incorrect sizes, how do you plan to address the other regions that the GICv2 specification describes?
static int __init acpi_parse_gic_distributor(struct acpi_subtable_header *header, const unsigned long end) { struct acpi_madt_generic_distributor *distributor = NULL;
- void __iomem *dist_base = NULL;
- void __iomem *cpu_base = NULL;
distributor = (struct acpi_madt_generic_distributor *)header; @@ -224,8 +235,43 @@ acpi_parse_gic_distributor(struct acpi_subtable_header *header, acpi_table_print_madt_entry(header);
- /* GIC is initialised after page_init(), no need for early_ioremap */
- dist_base = ioremap(distributor->base_address,
GIC_CPU_INTERFACE_MEMORY_SIZE);- if (!dist_base) {
pr_warn(PREFIX "unable to map gic dist registers\n");return -ENOMEM;- }
- /*
* acpi_lapic_addr is stored in acpi_parse_madt(),* so we can use it here for GIC init*/- if (acpi_lapic_addr) {
iounmap(dist_base);pr_warn(PREFIX "Invalid GIC cpu interface base address\n");return -EINVAL;- }
- cpu_base = ioremap(acpi_lapic_addr, GIC_CPU_INTERFACE_MEMORY_SIZE);
- if (!cpu_base) {
iounmap(dist_base);pr_warn(PREFIX "unable to map gic cpu registers\n");return -ENOMEM;- }
- gic_init(distributor->gic_id, -1, dist_base, cpu_base);
- return 0;
} +#else +static int __init +acpi_parse_gic_distributor(struct acpi_subtable_header *header,
const unsigned long end)+{
- return -ENODEV;
+} +#endif /* CONFIG_ARM_GIC */ /*
- Parse GIC cpu interface related entries in MADT
@@ -234,7 +280,7 @@ acpi_parse_gic_distributor(struct acpi_subtable_header *header, static int __init acpi_parse_madt_gic_entries(void) { int count;
- /*
- do a partial walk of MADT to determine how many CPUs
- we have including disabled CPUs
@@ -468,19 +514,21 @@ static void __init acpi_process_madt(void) * Parse MADT GIC cpu interface entries */ error = acpi_parse_madt_gic_entries();
if (!error) {/** Parse MADT GIC distributor entries*/acpi_parse_madt_gic_distributor_entries();}
if (!error)pr_info("Using ACPI for processor (GIC) configuration information\n");
- pr_info("Using ACPI for processor (GIC) configuration information\n");
- return;
} +int __init acpi_gic_init(void) +{
- /*
* Parse MADT GIC distributor entries*/- return acpi_parse_madt_gic_distributor_entries();
+}
Why can't you do the GIC init in the GIC code? We've tried hard to make interrupt controllers discoverable and self contained. What are you going to do when ACPI adds GICv3 to the mix? I don't really think this model (shoving everything into the core ACPI code) is sustainable in the long run...
Cheers,
M.
On 2013年12月04日 01:26, Marc Zyngier wrote:
Hi Hanjun,
On 03/12/13 16:39, Hanjun Guo wrote:
In MADT table, there are GIC cpu interface base address and GIC distributor base address, use them to convert GIC to ACPI.
Signed-off-by: Hanjun Guo hanjun.guo@linaro.org
arch/arm64/kernel/irq.c | 5 ++++ drivers/acpi/plat/arm-core.c | 66 ++++++++++++++++++++++++++++++++++++------ include/linux/acpi.h | 6 ++++ 3 files changed, 68 insertions(+), 9 deletions(-)
diff --git a/arch/arm64/kernel/irq.c b/arch/arm64/kernel/irq.c index 473e5db..a9e68bf 100644 --- a/arch/arm64/kernel/irq.c +++ b/arch/arm64/kernel/irq.c @@ -25,6 +25,7 @@ #include <linux/irq.h> #include <linux/smp.h> #include <linux/init.h> +#include <linux/acpi.h> #include <linux/irqchip.h> #include <linux/seq_file.h> #include <linux/ratelimit.h> @@ -78,6 +79,10 @@ void __init set_handle_irq(void (*handle_irq)(struct pt_regs *)) void __init init_IRQ(void) { irqchip_init();
- if (!handle_arch_irq)
acpi_gic_init();Why is the GIC hardcoded?
Very good question, thanks. I considered GIC only in my patch set. I have no idea how to handle the GIC hardcoded problem here for now, but I will figure it out later.
If any suggestion, I will appreciate a lot.
How are you going to support other interrupt controllers?
ACPI 5.0 supports GICv2 only for now, if we want to support other interrupt controller, we should introduce some OEM table and parsing it, and it will not covered by this patch set.
if (!handle_arch_irq) panic("No interrupt controller found."); } diff --git a/drivers/acpi/plat/arm-core.c b/drivers/acpi/plat/arm-core.c index 17c99e1..509b847 100644 --- a/drivers/acpi/plat/arm-core.c +++ b/drivers/acpi/plat/arm-core.c @@ -29,6 +29,7 @@ #include <linux/module.h> #include <linux/irq.h> #include <linux/irqdomain.h> +#include <linux/irqchip/arm-gic.h> #include <linux/slab.h> #include <linux/bootmem.h> #include <linux/ioport.h> @@ -211,11 +212,21 @@ acpi_parse_gic(struct acpi_subtable_header *header, const unsigned long end) return 0; } +#ifdef CONFIG_ARM_GIC +/*
- Hard code here, we can not get memory size from MADT (but FDT does),
- this size is described in ARMv8 foudation model's User Guide
- */
+#define GIC_DISTRIBUTOR_MEMORY_SIZE (SZ_8K) +#define GIC_CPU_INTERFACE_MEMORY_SIZE (SZ_4K)
Aside from the incorrect sizes, how do you plan to address the other regions that the GICv2 specification describes?
Did these regions have the same base address? I mean the same as GIC distributor base address and GIC cpu interface base address.
if yes, since the base address is stored in gic_init(), it can be for furture use. if I misunderstood your question, please let me know.
static int __init acpi_parse_gic_distributor(struct acpi_subtable_header *header, const unsigned long end) { struct acpi_madt_generic_distributor *distributor = NULL;
- void __iomem *dist_base = NULL;
- void __iomem *cpu_base = NULL;
distributor = (struct acpi_madt_generic_distributor *)header; @@ -224,8 +235,43 @@ acpi_parse_gic_distributor(struct acpi_subtable_header *header, acpi_table_print_madt_entry(header);
- /* GIC is initialised after page_init(), no need for early_ioremap */
- dist_base = ioremap(distributor->base_address,
GIC_CPU_INTERFACE_MEMORY_SIZE);- if (!dist_base) {
pr_warn(PREFIX "unable to map gic dist registers\n");return -ENOMEM;- }
- /*
* acpi_lapic_addr is stored in acpi_parse_madt(),* so we can use it here for GIC init*/- if (acpi_lapic_addr) {
iounmap(dist_base);pr_warn(PREFIX "Invalid GIC cpu interface base address\n");return -EINVAL;- }
- cpu_base = ioremap(acpi_lapic_addr, GIC_CPU_INTERFACE_MEMORY_SIZE);
- if (!cpu_base) {
iounmap(dist_base);pr_warn(PREFIX "unable to map gic cpu registers\n");return -ENOMEM;- }
- gic_init(distributor->gic_id, -1, dist_base, cpu_base);
- return 0; }
+#else +static int __init +acpi_parse_gic_distributor(struct acpi_subtable_header *header,
const unsigned long end)+{
- return -ENODEV;
+} +#endif /* CONFIG_ARM_GIC */ /*
- Parse GIC cpu interface related entries in MADT
@@ -234,7 +280,7 @@ acpi_parse_gic_distributor(struct acpi_subtable_header *header, static int __init acpi_parse_madt_gic_entries(void) { int count;
- /*
- do a partial walk of MADT to determine how many CPUs
- we have including disabled CPUs
@@ -468,19 +514,21 @@ static void __init acpi_process_madt(void) * Parse MADT GIC cpu interface entries */ error = acpi_parse_madt_gic_entries();
if (!error) {/** Parse MADT GIC distributor entries*/acpi_parse_madt_gic_distributor_entries();}
if (!error)pr_info("Using ACPI for processor (GIC) configuration information\n");
- pr_info("Using ACPI for processor (GIC) configuration information\n");
- return; }
+int __init acpi_gic_init(void) +{
- /*
* Parse MADT GIC distributor entries*/- return acpi_parse_madt_gic_distributor_entries();
+}
Why can't you do the GIC init in the GIC code? We've tried hard to make interrupt controllers discoverable and self contained.
thanks for your suggestion, Rob also had the same suggestion, will try to update it in next version.
What are you going to do when ACPI adds GICv3 to the mix? I don't really think this model (shoving everything into the core ACPI code) is sustainable in the long run...
Since GICv3 related ACPI proposal is not public and not goes into ACPI spec, my suggestion is that we implement GICv2 only for now and post another patches for GICv3 when the new ACPI spec is available.
Thanks Hanjun
On 04/12/13 15:32, Hanjun Guo wrote:
On 2013年12月04日 01:26, Marc Zyngier wrote:
Hi Hanjun,
On 03/12/13 16:39, Hanjun Guo wrote:
In MADT table, there are GIC cpu interface base address and GIC distributor base address, use them to convert GIC to ACPI.
Signed-off-by: Hanjun Guo hanjun.guo@linaro.org
arch/arm64/kernel/irq.c | 5 ++++ drivers/acpi/plat/arm-core.c | 66 ++++++++++++++++++++++++++++++++++++------ include/linux/acpi.h | 6 ++++ 3 files changed, 68 insertions(+), 9 deletions(-)
diff --git a/arch/arm64/kernel/irq.c b/arch/arm64/kernel/irq.c index 473e5db..a9e68bf 100644 --- a/arch/arm64/kernel/irq.c +++ b/arch/arm64/kernel/irq.c @@ -25,6 +25,7 @@ #include <linux/irq.h> #include <linux/smp.h> #include <linux/init.h> +#include <linux/acpi.h> #include <linux/irqchip.h> #include <linux/seq_file.h> #include <linux/ratelimit.h> @@ -78,6 +79,10 @@ void __init set_handle_irq(void (*handle_irq)(struct pt_regs *)) void __init init_IRQ(void) { irqchip_init();
- if (!handle_arch_irq)
acpi_gic_init();Why is the GIC hardcoded?
Very good question, thanks. I considered GIC only in my patch set. I have no idea how to handle the GIC hardcoded problem here for now, but I will figure it out later.
If any suggestion, I will appreciate a lot.
How are you going to support other interrupt controllers?
ACPI 5.0 supports GICv2 only for now, if we want to support other interrupt controller, we should introduce some OEM table and parsing it, and it will not covered by this patch set.
if (!handle_arch_irq) panic("No interrupt controller found."); } diff --git a/drivers/acpi/plat/arm-core.c b/drivers/acpi/plat/arm-core.c index 17c99e1..509b847 100644 --- a/drivers/acpi/plat/arm-core.c +++ b/drivers/acpi/plat/arm-core.c @@ -29,6 +29,7 @@ #include <linux/module.h> #include <linux/irq.h> #include <linux/irqdomain.h> +#include <linux/irqchip/arm-gic.h> #include <linux/slab.h> #include <linux/bootmem.h> #include <linux/ioport.h> @@ -211,11 +212,21 @@ acpi_parse_gic(struct acpi_subtable_header *header, const unsigned long end) return 0; } +#ifdef CONFIG_ARM_GIC +/*
- Hard code here, we can not get memory size from MADT (but FDT does),
- this size is described in ARMv8 foudation model's User Guide
- */
+#define GIC_DISTRIBUTOR_MEMORY_SIZE (SZ_8K) +#define GIC_CPU_INTERFACE_MEMORY_SIZE (SZ_4K)
Aside from the incorrect sizes, how do you plan to address the other regions that the GICv2 specification describes?
Did these regions have the same base address? I mean the same as GIC distributor base address and GIC cpu interface base address.
if yes, since the base address is stored in gic_init(), it can be for furture use. if I misunderstood your question, please let me know.
Look at the VGIC implementation for KVM in virt/kvm/arm. It does its own probing of the additional regions used for virtualization.
The GIC and VGIC code are completely separate, and you'll need to find an acceptable solution for that too.
static int __init acpi_parse_gic_distributor(struct acpi_subtable_header *header, const unsigned long end) { struct acpi_madt_generic_distributor *distributor = NULL;
- void __iomem *dist_base = NULL;
- void __iomem *cpu_base = NULL;
distributor = (struct acpi_madt_generic_distributor *)header; @@ -224,8 +235,43 @@ acpi_parse_gic_distributor(struct acpi_subtable_header *header, acpi_table_print_madt_entry(header);
- /* GIC is initialised after page_init(), no need for early_ioremap */
- dist_base = ioremap(distributor->base_address,
GIC_CPU_INTERFACE_MEMORY_SIZE);- if (!dist_base) {
pr_warn(PREFIX "unable to map gic dist registers\n");return -ENOMEM;- }
- /*
* acpi_lapic_addr is stored in acpi_parse_madt(),* so we can use it here for GIC init*/- if (acpi_lapic_addr) {
iounmap(dist_base);pr_warn(PREFIX "Invalid GIC cpu interface base address\n");return -EINVAL;- }
- cpu_base = ioremap(acpi_lapic_addr, GIC_CPU_INTERFACE_MEMORY_SIZE);
- if (!cpu_base) {
iounmap(dist_base);pr_warn(PREFIX "unable to map gic cpu registers\n");return -ENOMEM;- }
- gic_init(distributor->gic_id, -1, dist_base, cpu_base);
- return 0; }
+#else +static int __init +acpi_parse_gic_distributor(struct acpi_subtable_header *header,
const unsigned long end)+{
- return -ENODEV;
+} +#endif /* CONFIG_ARM_GIC */ /*
- Parse GIC cpu interface related entries in MADT
@@ -234,7 +280,7 @@ acpi_parse_gic_distributor(struct acpi_subtable_header *header, static int __init acpi_parse_madt_gic_entries(void) { int count;
- /*
- do a partial walk of MADT to determine how many CPUs
- we have including disabled CPUs
@@ -468,19 +514,21 @@ static void __init acpi_process_madt(void) * Parse MADT GIC cpu interface entries */ error = acpi_parse_madt_gic_entries();
if (!error) {/** Parse MADT GIC distributor entries*/acpi_parse_madt_gic_distributor_entries();}
if (!error)pr_info("Using ACPI for processor (GIC) configuration information\n");
- pr_info("Using ACPI for processor (GIC) configuration information\n");
- return; }
+int __init acpi_gic_init(void) +{
- /*
* Parse MADT GIC distributor entries*/- return acpi_parse_madt_gic_distributor_entries();
+}
Why can't you do the GIC init in the GIC code? We've tried hard to make interrupt controllers discoverable and self contained.
thanks for your suggestion, Rob also had the same suggestion, will try to update it in next version.
What are you going to do when ACPI adds GICv3 to the mix? I don't really think this model (shoving everything into the core ACPI code) is sustainable in the long run...
Since GICv3 related ACPI proposal is not public and not goes into ACPI spec, my suggestion is that we implement GICv2 only for now and post another patches for GICv3 when the new ACPI spec is available.
Certainly. But I think you should aim for a scalable solution right away, instead of starting with something that we already know won't work for stuff that is already around the corner (which is what I infer from your "non public" statement).
Cheers,
M.
On 2013年12月04日 23:50, Marc Zyngier wrote:
On 04/12/13 15:32, Hanjun Guo wrote:
On 2013年12月04日 01:26, Marc Zyngier wrote:
Hi Hanjun,
On 03/12/13 16:39, Hanjun Guo wrote:
In MADT table, there are GIC cpu interface base address and GIC distributor base address, use them to convert GIC to ACPI.
Signed-off-by: Hanjun Guo hanjun.guo@linaro.org
arch/arm64/kernel/irq.c | 5 ++++ drivers/acpi/plat/arm-core.c | 66 ++++++++++++++++++++++++++++++++++++------ include/linux/acpi.h | 6 ++++ 3 files changed, 68 insertions(+), 9 deletions(-)
diff --git a/arch/arm64/kernel/irq.c b/arch/arm64/kernel/irq.c index 473e5db..a9e68bf 100644 --- a/arch/arm64/kernel/irq.c +++ b/arch/arm64/kernel/irq.c @@ -25,6 +25,7 @@ #include <linux/irq.h> #include <linux/smp.h> #include <linux/init.h> +#include <linux/acpi.h> #include <linux/irqchip.h> #include <linux/seq_file.h> #include <linux/ratelimit.h> @@ -78,6 +79,10 @@ void __init set_handle_irq(void (*handle_irq)(struct pt_regs *)) void __init init_IRQ(void) { irqchip_init();
- if (!handle_arch_irq)
acpi_gic_init();Why is the GIC hardcoded?
Very good question, thanks. I considered GIC only in my patch set. I have no idea how to handle the GIC hardcoded problem here for now, but I will figure it out later.
If any suggestion, I will appreciate a lot.
How are you going to support other interrupt controllers?
ACPI 5.0 supports GICv2 only for now, if we want to support other interrupt controller, we should introduce some OEM table and parsing it, and it will not covered by this patch set.
if (!handle_arch_irq) panic("No interrupt controller found.");} diff --git a/drivers/acpi/plat/arm-core.c b/drivers/acpi/plat/arm-core.c index 17c99e1..509b847 100644 --- a/drivers/acpi/plat/arm-core.c +++ b/drivers/acpi/plat/arm-core.c @@ -29,6 +29,7 @@ #include <linux/module.h> #include <linux/irq.h> #include <linux/irqdomain.h> +#include <linux/irqchip/arm-gic.h> #include <linux/slab.h> #include <linux/bootmem.h> #include <linux/ioport.h> @@ -211,11 +212,21 @@ acpi_parse_gic(struct acpi_subtable_header *header, const unsigned long end) return 0; } +#ifdef CONFIG_ARM_GIC +/*
- Hard code here, we can not get memory size from MADT (but FDT does),
- this size is described in ARMv8 foudation model's User Guide
- */
+#define GIC_DISTRIBUTOR_MEMORY_SIZE (SZ_8K) +#define GIC_CPU_INTERFACE_MEMORY_SIZE (SZ_4K)
Aside from the incorrect sizes, how do you plan to address the other regions that the GICv2 specification describes?
Did these regions have the same base address? I mean the same as GIC distributor base address and GIC cpu interface base address.
if yes, since the base address is stored in gic_init(), it can be for furture use. if I misunderstood your question, please let me know.
Look at the VGIC implementation for KVM in virt/kvm/arm. It does its own probing of the additional regions used for virtualization.
The GIC and VGIC code are completely separate, and you'll need to find an acceptable solution for that too.
Ok, will review the VGIC code for KVM, thanks for the guidance.
static int __init acpi_parse_gic_distributor(struct acpi_subtable_header *header, const unsigned long end) { struct acpi_madt_generic_distributor *distributor = NULL;
- void __iomem *dist_base = NULL;
- void __iomem *cpu_base = NULL; distributor = (struct acpi_madt_generic_distributor *)header;
@@ -224,8 +235,43 @@ acpi_parse_gic_distributor(struct acpi_subtable_header *header, acpi_table_print_madt_entry(header);
- /* GIC is initialised after page_init(), no need for early_ioremap */
- dist_base = ioremap(distributor->base_address,
GIC_CPU_INTERFACE_MEMORY_SIZE);- if (!dist_base) {
pr_warn(PREFIX "unable to map gic dist registers\n");return -ENOMEM;- }
- /*
* acpi_lapic_addr is stored in acpi_parse_madt(),* so we can use it here for GIC init*/- if (acpi_lapic_addr) {
iounmap(dist_base);pr_warn(PREFIX "Invalid GIC cpu interface base address\n");return -EINVAL;- }
- cpu_base = ioremap(acpi_lapic_addr, GIC_CPU_INTERFACE_MEMORY_SIZE);
- if (!cpu_base) {
iounmap(dist_base);pr_warn(PREFIX "unable to map gic cpu registers\n");return -ENOMEM;- }
- gic_init(distributor->gic_id, -1, dist_base, cpu_base);
- return 0; }
+#else +static int __init +acpi_parse_gic_distributor(struct acpi_subtable_header *header,
const unsigned long end)+{
- return -ENODEV;
+} +#endif /* CONFIG_ARM_GIC */ /* * Parse GIC cpu interface related entries in MADT @@ -234,7 +280,7 @@ acpi_parse_gic_distributor(struct acpi_subtable_header *header, static int __init acpi_parse_madt_gic_entries(void) { int count;
- /*
- do a partial walk of MADT to determine how many CPUs
- we have including disabled CPUs
@@ -468,19 +514,21 @@ static void __init acpi_process_madt(void) * Parse MADT GIC cpu interface entries */ error = acpi_parse_madt_gic_entries();
if (!error) {/** Parse MADT GIC distributor entries*/acpi_parse_madt_gic_distributor_entries();}
if (!error)pr_info("Using ACPI for processor (GIC) configuration information\n");
- pr_info("Using ACPI for processor (GIC) configuration information\n");
- return; }
+int __init acpi_gic_init(void) +{
- /*
* Parse MADT GIC distributor entries*/- return acpi_parse_madt_gic_distributor_entries();
+}
Why can't you do the GIC init in the GIC code? We've tried hard to make interrupt controllers discoverable and self contained.
thanks for your suggestion, Rob also had the same suggestion, will try to update it in next version.
What are you going to do when ACPI adds GICv3 to the mix? I don't really think this model (shoving everything into the core ACPI code) is sustainable in the long run...
Since GICv3 related ACPI proposal is not public and not goes into ACPI spec, my suggestion is that we implement GICv2 only for now and post another patches for GICv3 when the new ACPI spec is available.
Certainly. But I think you should aim for a scalable solution right away, instead of starting with something that we already know won't work for stuff that is already around the corner (which is what I infer from your "non public" statement).
yes, sure I will, thanks for your comments.
Thanks Hanjun
On Wed, Dec 04, 2013 at 03:50:17PM +0000, Marc Zyngier wrote:
On 04/12/13 15:32, Hanjun Guo wrote:
On 2013年12月04日 01:26, Marc Zyngier wrote:
Hi Hanjun,
On 03/12/13 16:39, Hanjun Guo wrote:
In MADT table, there are GIC cpu interface base address and GIC distributor base address, use them to convert GIC to ACPI.
Signed-off-by: Hanjun Guo hanjun.guo@linaro.org
arch/arm64/kernel/irq.c | 5 ++++ drivers/acpi/plat/arm-core.c | 66 ++++++++++++++++++++++++++++++++++++------ include/linux/acpi.h | 6 ++++ 3 files changed, 68 insertions(+), 9 deletions(-)
diff --git a/arch/arm64/kernel/irq.c b/arch/arm64/kernel/irq.c index 473e5db..a9e68bf 100644 --- a/arch/arm64/kernel/irq.c +++ b/arch/arm64/kernel/irq.c @@ -25,6 +25,7 @@ #include <linux/irq.h> #include <linux/smp.h> #include <linux/init.h> +#include <linux/acpi.h> #include <linux/irqchip.h> #include <linux/seq_file.h> #include <linux/ratelimit.h> @@ -78,6 +79,10 @@ void __init set_handle_irq(void (*handle_irq)(struct pt_regs *)) void __init init_IRQ(void) { irqchip_init();
- if (!handle_arch_irq)
acpi_gic_init();Why is the GIC hardcoded?
Very good question, thanks. I considered GIC only in my patch set. I have no idea how to handle the GIC hardcoded problem here for now, but I will figure it out later.
If any suggestion, I will appreciate a lot.
How are you going to support other interrupt controllers?
ACPI 5.0 supports GICv2 only for now, if we want to support other interrupt controller, we should introduce some OEM table and parsing it, and it will not covered by this patch set.
if (!handle_arch_irq) panic("No interrupt controller found."); } diff --git a/drivers/acpi/plat/arm-core.c b/drivers/acpi/plat/arm-core.c index 17c99e1..509b847 100644 --- a/drivers/acpi/plat/arm-core.c +++ b/drivers/acpi/plat/arm-core.c @@ -29,6 +29,7 @@ #include <linux/module.h> #include <linux/irq.h> #include <linux/irqdomain.h> +#include <linux/irqchip/arm-gic.h> #include <linux/slab.h> #include <linux/bootmem.h> #include <linux/ioport.h> @@ -211,11 +212,21 @@ acpi_parse_gic(struct acpi_subtable_header *header, const unsigned long end) return 0; } +#ifdef CONFIG_ARM_GIC +/*
- Hard code here, we can not get memory size from MADT (but FDT does),
- this size is described in ARMv8 foudation model's User Guide
- */
+#define GIC_DISTRIBUTOR_MEMORY_SIZE (SZ_8K) +#define GIC_CPU_INTERFACE_MEMORY_SIZE (SZ_4K)
Aside from the incorrect sizes, how do you plan to address the other regions that the GICv2 specification describes?
Did these regions have the same base address? I mean the same as GIC distributor base address and GIC cpu interface base address.
if yes, since the base address is stored in gic_init(), it can be for furture use. if I misunderstood your question, please let me know.
Look at the VGIC implementation for KVM in virt/kvm/arm. It does its own probing of the additional regions used for virtualization.
The GIC and VGIC code are completely separate, and you'll need to find an acceptable solution for that too.
static int __init acpi_parse_gic_distributor(struct acpi_subtable_header *header, const unsigned long end) { struct acpi_madt_generic_distributor *distributor = NULL;
- void __iomem *dist_base = NULL;
- void __iomem *cpu_base = NULL;
distributor = (struct acpi_madt_generic_distributor *)header; @@ -224,8 +235,43 @@ acpi_parse_gic_distributor(struct acpi_subtable_header *header, acpi_table_print_madt_entry(header);
- /* GIC is initialised after page_init(), no need for early_ioremap */
- dist_base = ioremap(distributor->base_address,
GIC_CPU_INTERFACE_MEMORY_SIZE);- if (!dist_base) {
pr_warn(PREFIX "unable to map gic dist registers\n");return -ENOMEM;- }
- /*
* acpi_lapic_addr is stored in acpi_parse_madt(),* so we can use it here for GIC init*/- if (acpi_lapic_addr) {
iounmap(dist_base);pr_warn(PREFIX "Invalid GIC cpu interface base address\n");return -EINVAL;- }
- cpu_base = ioremap(acpi_lapic_addr, GIC_CPU_INTERFACE_MEMORY_SIZE);
- if (!cpu_base) {
iounmap(dist_base);pr_warn(PREFIX "unable to map gic cpu registers\n");return -ENOMEM;- }
- gic_init(distributor->gic_id, -1, dist_base, cpu_base);
- return 0; }
+#else +static int __init +acpi_parse_gic_distributor(struct acpi_subtable_header *header,
const unsigned long end)+{
- return -ENODEV;
+} +#endif /* CONFIG_ARM_GIC */ /*
- Parse GIC cpu interface related entries in MADT
@@ -234,7 +280,7 @@ acpi_parse_gic_distributor(struct acpi_subtable_header *header, static int __init acpi_parse_madt_gic_entries(void) { int count;
- /*
- do a partial walk of MADT to determine how many CPUs
- we have including disabled CPUs
@@ -468,19 +514,21 @@ static void __init acpi_process_madt(void) * Parse MADT GIC cpu interface entries */ error = acpi_parse_madt_gic_entries();
if (!error) {/** Parse MADT GIC distributor entries*/acpi_parse_madt_gic_distributor_entries();}
if (!error)pr_info("Using ACPI for processor (GIC) configuration information\n");
- pr_info("Using ACPI for processor (GIC) configuration information\n");
- return; }
+int __init acpi_gic_init(void) +{
- /*
* Parse MADT GIC distributor entries*/- return acpi_parse_madt_gic_distributor_entries();
+}
Why can't you do the GIC init in the GIC code? We've tried hard to make interrupt controllers discoverable and self contained.
thanks for your suggestion, Rob also had the same suggestion, will try to update it in next version.
What are you going to do when ACPI adds GICv3 to the mix? I don't really think this model (shoving everything into the core ACPI code) is sustainable in the long run...
Since GICv3 related ACPI proposal is not public and not goes into ACPI spec, my suggestion is that we implement GICv2 only for now and post another patches for GICv3 when the new ACPI spec is available.
Certainly. But I think you should aim for a scalable solution right away, instead of starting with something that we already know won't work for stuff that is already around the corner (which is what I infer from your "non public" statement).
Again, I wonder if we might be better off converting GIC info (since it's likely to be there on all systems) in the EFI boot wrapper into FDT data when ACPI is provided.
Essentially, if we can describe: * Memory * Console uart (if one exists) for debug * Timers * Interrupts (Possibly PCI host controllers too but I'm less sure that can be done generically)
in the FDT stub, then we can keep a lot of the lowlevel init code common instead of diverging. We can also get away from having to update both ACPI and DT for GICv3, etc, reducing boilerplate code in the kernel to handle both.
If we have to add some properties to bring across some ACPI information we can of course do so, hopefully it'll be a small amount.
-Olof
linaro-kernel@lists.linaro.org
-
Arnd Bergmann -
Hanjun Guo -
Linus Walleij -
Marc Zyngier -
Mark Brown -
Olof Johansson -
One Thousand Gnomes -
Rob Herring