On systems supporting GICv3 and above, the field of GICR Base Address holds the 64-bit physical address of the associated Redistributor if the GIC Redistributors are not in the always-on power domain, so instead of init GICR regions via GIC redistributor stricture, init it with GICR base address in GICC structures.
Signed-off-by: Hanjun Guo hanjun.guo@linaro.org --- drivers/irqchip/irq-gic-acpi.c | 34 ++++++- drivers/irqchip/irq-gic-v3.c | 181 ++++++++++++++++++++++++++++++++--- include/linux/irqchip/arm-gic-acpi.h | 2 + 3 files changed, 200 insertions(+), 17 deletions(-)
diff --git a/drivers/irqchip/irq-gic-acpi.c b/drivers/irqchip/irq-gic-acpi.c index 236bba5..732efa0 100644 --- a/drivers/irqchip/irq-gic-acpi.c +++ b/drivers/irqchip/irq-gic-acpi.c @@ -21,6 +21,11 @@ static u8 gic_version __initdata = ACPI_MADT_GIC_VERSION_NONE;
static phys_addr_t dist_phy_base __initdata;
+u8 __init acpi_gic_version(void) +{ + return gic_version; +} + static int __init acpi_gic_parse_distributor(struct acpi_subtable_header *header, const unsigned long end) @@ -38,6 +43,27 @@ acpi_gic_parse_distributor(struct acpi_subtable_header *header, }
static int __init +gic_acpi_parse_madt_gicc(struct acpi_subtable_header *header, + const unsigned long end) +{ + struct acpi_madt_generic_interrupt *gicc; + + gicc = (struct acpi_madt_generic_interrupt *)header; + + if (BAD_MADT_ENTRY(gicc, end)) + return -EINVAL; + + /* + *If GICC is enabled but no gicr base address, which means GICR is + * not presented via GICC + */ + if ((gicc->flags & ACPI_MADT_ENABLED) && !gicc->gicr_base_address) + return -ENODEV; + + return 0; +} + +static int __init match_gic_redist(struct acpi_subtable_header *header, const unsigned long end) { return 0; @@ -54,8 +80,12 @@ static bool __init acpi_gic_redist_is_present(void) /* has at least one GIC redistributor entry */ if (count > 0) return true; - else - return false; + + /* else try to find GICR base in GICC entries */ + count = acpi_table_parse_madt(ACPI_MADT_TYPE_GENERIC_INTERRUPT, + gic_acpi_parse_madt_gicc, 0); + + return count > 0; }
static int __init acpi_gic_version_init(void) diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c index a1c4c74..755c940 100644 --- a/drivers/irqchip/irq-gic-v3.c +++ b/drivers/irqchip/irq-gic-v3.c @@ -389,9 +389,8 @@ static void __init gic_dist_init(void) writeq_relaxed(affinity, base + GICD_IROUTER + i * 8); }
-static int gic_populate_rdist(void) +static int gic_populate_rdist_with_regions(u64 mpidr) { - u64 mpidr = cpu_logical_map(smp_processor_id()); u64 typer; u32 aff; int i; @@ -439,6 +438,33 @@ static int gic_populate_rdist(void) } while (!(typer & GICR_TYPER_LAST)); }
+ return -ENODEV; +} + +#ifdef CONFIG_ACPI +/* + * Populate redist when GIC redistributor address is presented in ACPI + * MADT GICC entries + */ +static int gic_populate_rdist_with_gicr_base(u64 mpidr); +#else +static inline int gic_populate_rdist_with_gicr_base(u64 mpidr) +{ + return -ENODEV; +} +#endif +static int gic_populate_rdist(void) +{ + u64 mpidr = cpu_logical_map(smp_processor_id()); + + if (!gic_data.nr_redist_regions) { + if (!gic_populate_rdist_with_gicr_base(mpidr)) + return 0; + } else { + if (!gic_populate_rdist_with_regions(mpidr)) + return 0; + } + /* We couldn't even deal with ourselves... */ WARN(true, "CPU%d: mpidr %llx has no re-distributor!\n", smp_processor_id(), (unsigned long long)mpidr); @@ -906,6 +932,16 @@ IRQCHIP_DECLARE(gic_v3, "arm,gic-v3", gic_of_init); #endif
#ifdef CONFIG_ACPI + +struct acpi_gicc_redist { + struct list_head list; + u64 mpidr; + phys_addr_t phys_base; + void __iomem *redist_base; +}; + +static LIST_HEAD(redist_list); + static struct redist_region *redist_regs __initdata; static u32 nr_redist_regions __initdata; static phys_addr_t dist_phy_base __initdata; @@ -938,6 +974,17 @@ gic_acpi_register_redist(u64 phys_base, u64 size) return 0; }
+static void __init +gic_acpi_release_redist_regions(void) +{ + int i; + + for (i = 0; i < nr_redist_regions; i++) + if (redist_regs[i].redist_base) + iounmap(redist_regs[i].redist_base); + kfree(redist_regs); +} + static int __init gic_acpi_parse_madt_redist(struct acpi_subtable_header *header, const unsigned long end) @@ -970,10 +1017,98 @@ gic_acpi_parse_madt_distributor(struct acpi_subtable_header *header, }
static int __init -gic_v3_acpi_init(struct acpi_table_header *table) +gic_acpi_add_gicc_redist(phys_addr_t phys_base, u64 mpidr) { - int count, i, err = 0; + struct acpi_gicc_redist *redist; + + redist = kzalloc(sizeof(*redist), GFP_KERNEL); + if (!redist) + return -ENOMEM; + + redist->mpidr = mpidr; + redist->phys_base = phys_base; + + if (acpi_gic_version() == ACPI_MADT_GIC_VERSION_V3) + /* RD_base + SGI_base */ + redist->redist_base = ioremap(phys_base, 2 * SZ_64K); + else + /* + * RD_base + SGI_base + VLPI_base, + * we don't map reserved page as it's buggy to access it + */ + redist->redist_base = ioremap(phys_base, 3 * SZ_64K); + + if (!redist->redist_base) { + kfree(redist); + return -ENOMEM; + } + + list_add(&redist->list, &redist_list); + return 0; +} + +static void __init +gic_acpi_release_gicc_redist(void) +{ + struct acpi_gicc_redist *redist, *t; + + list_for_each_entry_safe(redist, t, &redist_list, list) { + list_del(&redist->list); + iounmap(redist->redist_base); + kfree(redist); + } +} + +static int __init +gic_acpi_parse_madt_gicc(struct acpi_subtable_header *header, + const unsigned long end) +{ + struct acpi_madt_generic_interrupt *gicc; + + gicc = (struct acpi_madt_generic_interrupt *)header; + + if (BAD_MADT_ENTRY(gicc, end)) + return -EINVAL; + + return gic_acpi_add_gicc_redist(gicc->gicr_base_address, + gicc->arm_mpidr); +} + +static int gic_populate_rdist_with_gicr_base(u64 mpidr) +{ + struct acpi_gicc_redist *redist; + void __iomem *ptr; + u32 reg; + + list_for_each_entry(redist, &redist_list, list) { + if (redist->mpidr != mpidr) + continue; + + ptr = redist->redist_base; + reg = readl_relaxed(ptr + GICR_PIDR2) & GIC_PIDR2_ARCH_MASK; + if (reg != GIC_PIDR2_ARCH_GICv3 && reg != GIC_PIDR2_ARCH_GICv4) { + pr_warn("No redistributor present @%p\n", ptr); + return -ENODEV; + } + + gic_data_rdist_rd_base() = ptr; + gic_data_rdist()->phys_base = redist->phys_base; + pr_info("CPU%d: found redistributor %llx phys base:%pa\n", + smp_processor_id(), + (unsigned long long)mpidr, + &gic_data_rdist()->phys_base); + return 0; + } + + return -ENODEV; +} + +static int __init +gic_acpi_init(struct acpi_table_header *table) +{ + int count, err = 0; void __iomem *dist_base; + bool no_gicr_entries = false;
/* Get distributor base address */ count = acpi_parse_entries(ACPI_SIG_MADT, @@ -1000,32 +1135,48 @@ gic_v3_acpi_init(struct acpi_table_header *table) goto out_dist_unmap; }
- /* Collect redistributor base addresses */ + /* Collect redistributor base addresses in GICR entries */ count = acpi_parse_entries(ACPI_SIG_MADT, sizeof(struct acpi_table_madt), gic_acpi_parse_madt_redist, table, ACPI_MADT_TYPE_GENERIC_REDISTRIBUTOR, 0); if (count <= 0) { - pr_info("No valid GICR entries exist\n"); - err = -EINVAL; - goto out_redist_unmap; + pr_info("No valid GICR entries exist, try GICC entries\n"); + gic_acpi_release_redist_regions(); + no_gicr_entries = true; + } else { + goto init_base; + } + + /* Collect redistributor base addresses in GICC entries */ + count = acpi_parse_entries(ACPI_SIG_MADT, + sizeof(struct acpi_table_madt), + gic_acpi_parse_madt_gicc, table, + ACPI_MADT_TYPE_GENERIC_INTERRUPT, 0); + if (count <= 0) { + pr_info("No valid GICC entries exist\n"); + goto out_release_redist; }
+init_base: err = gic_init_bases(dist_base, redist_regs, nr_redist_regions, 0, NULL); if (err) - goto out_redist_unmap; + goto out_release_redist;
acpi_irq_domain = gic_data.domain; return 0;
-out_redist_unmap: - for (i = 0; i < nr_redist_regions; i++) - if (redist_regs[i].redist_base) - iounmap(redist_regs[i].redist_base); - kfree(redist_regs); +out_release_redist: + if (no_gicr_entries) { + if (!list_empty(&redist_list)) + gic_acpi_release_gicc_redist(); + } else { + gic_acpi_release_redist_regions(); + } out_dist_unmap: iounmap(dist_base); return err; } -IRQCHIP_ACPI_DECLARE(gic_v3, ACPI_MADT_GIC_VERSION_V3, gic_v3_acpi_init); +IRQCHIP_ACPI_DECLARE(gic_v3, ACPI_MADT_GIC_VERSION_V3, gic_acpi_init); +IRQCHIP_ACPI_DECLARE(gic_v4, ACPI_MADT_GIC_VERSION_V4, gic_acpi_init); #endif diff --git a/include/linux/irqchip/arm-gic-acpi.h b/include/linux/irqchip/arm-gic-acpi.h index a4a5edb..d54fe95 100644 --- a/include/linux/irqchip/arm-gic-acpi.h +++ b/include/linux/irqchip/arm-gic-acpi.h @@ -23,5 +23,7 @@
extern struct irq_domain *acpi_irq_domain;
+u8 acpi_gic_version(void); + #endif /* CONFIG_ACPI */ #endif /* ARM_GIC_ACPI_H_ */
Hi Hanjun,
On 26 June 2015 at 09:51, Hanjun Guo hanjun.guo@linaro.org wrote:
On systems supporting GICv3 and above, the field of GICR Base Address holds the 64-bit physical address of the associated Redistributor if the GIC Redistributors are not in the always-on power domain, so instead of init GICR regions via GIC redistributor stricture, init it with GICR base address in GICC structures.
Unless I misunderstand, I think you want the opposite statement. i.e. If the redists are in always power-on domain, then the base addresses should be in GICR structs and not in GICC.
Signed-off-by: Hanjun Guo hanjun.guo@linaro.org
drivers/irqchip/irq-gic-acpi.c | 34 ++++++- drivers/irqchip/irq-gic-v3.c | 181 ++++++++++++++++++++++++++++++++--- include/linux/irqchip/arm-gic-acpi.h | 2 + 3 files changed, 200 insertions(+), 17 deletions(-)
diff --git a/drivers/irqchip/irq-gic-acpi.c b/drivers/irqchip/irq-gic-acpi.c index 236bba5..732efa0 100644 --- a/drivers/irqchip/irq-gic-acpi.c +++ b/drivers/irqchip/irq-gic-acpi.c @@ -21,6 +21,11 @@ static u8 gic_version __initdata = ACPI_MADT_GIC_VERSION_NONE;
static phys_addr_t dist_phy_base __initdata;
+u8 __init acpi_gic_version(void) +{
return gic_version;+}
static int __init acpi_gic_parse_distributor(struct acpi_subtable_header *header, const unsigned long end) @@ -38,6 +43,27 @@ acpi_gic_parse_distributor(struct acpi_subtable_header *header, }
static int __init +gic_acpi_parse_madt_gicc(struct acpi_subtable_header *header,
const unsigned long end)+{
struct acpi_madt_generic_interrupt *gicc;gicc = (struct acpi_madt_generic_interrupt *)header;if (BAD_MADT_ENTRY(gicc, end))return -EINVAL;/**If GICC is enabled but no gicr base address, which means GICR is* not presented via GICC*/if ((gicc->flags & ACPI_MADT_ENABLED) && !gicc->gicr_base_address)return -ENODEV;return 0;+}
+static int __init match_gic_redist(struct acpi_subtable_header *header, const unsigned long end) { return 0; @@ -54,8 +80,12 @@ static bool __init acpi_gic_redist_is_present(void) /* has at least one GIC redistributor entry */ if (count > 0) return true;
elsereturn false;
/* else try to find GICR base in GICC entries */count = acpi_table_parse_madt(ACPI_MADT_TYPE_GENERIC_INTERRUPT,gic_acpi_parse_madt_gicc, 0);return count > 0;}
static int __init acpi_gic_version_init(void) diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c index a1c4c74..755c940 100644 --- a/drivers/irqchip/irq-gic-v3.c +++ b/drivers/irqchip/irq-gic-v3.c @@ -389,9 +389,8 @@ static void __init gic_dist_init(void) writeq_relaxed(affinity, base + GICD_IROUTER + i * 8); }
-static int gic_populate_rdist(void) +static int gic_populate_rdist_with_regions(u64 mpidr) {
u64 mpidr = cpu_logical_map(smp_processor_id()); u64 typer; u32 aff; int i;@@ -439,6 +438,33 @@ static int gic_populate_rdist(void) } while (!(typer & GICR_TYPER_LAST)); }
return -ENODEV;+}
+#ifdef CONFIG_ACPI +/*
- Populate redist when GIC redistributor address is presented in ACPI
- MADT GICC entries
- */
+static int gic_populate_rdist_with_gicr_base(u64 mpidr); +#else +static inline int gic_populate_rdist_with_gicr_base(u64 mpidr) +{
return -ENODEV;+} +#endif +static int gic_populate_rdist(void) +{
u64 mpidr = cpu_logical_map(smp_processor_id());if (!gic_data.nr_redist_regions) {if (!gic_populate_rdist_with_gicr_base(mpidr))return 0;} else {if (!gic_populate_rdist_with_regions(mpidr))return 0;}/* We couldn't even deal with ourselves... */ WARN(true, "CPU%d: mpidr %llx has no re-distributor!\n", smp_processor_id(), (unsigned long long)mpidr);@@ -906,6 +932,16 @@ IRQCHIP_DECLARE(gic_v3, "arm,gic-v3", gic_of_init); #endif
#ifdef CONFIG_ACPI
+struct acpi_gicc_redist {
struct list_head list;u64 mpidr;phys_addr_t phys_base;void __iomem *redist_base;+};
+static LIST_HEAD(redist_list);
static struct redist_region *redist_regs __initdata; static u32 nr_redist_regions __initdata; static phys_addr_t dist_phy_base __initdata; @@ -938,6 +974,17 @@ gic_acpi_register_redist(u64 phys_base, u64 size) return 0; }
+static void __init +gic_acpi_release_redist_regions(void) +{
int i;for (i = 0; i < nr_redist_regions; i++)if (redist_regs[i].redist_base)iounmap(redist_regs[i].redist_base);kfree(redist_regs);+}
static int __init gic_acpi_parse_madt_redist(struct acpi_subtable_header *header, const unsigned long end) @@ -970,10 +1017,98 @@ gic_acpi_parse_madt_distributor(struct acpi_subtable_header *header, }
static int __init -gic_v3_acpi_init(struct acpi_table_header *table) +gic_acpi_add_gicc_redist(phys_addr_t phys_base, u64 mpidr) {
int count, i, err = 0;
struct acpi_gicc_redist *redist;redist = kzalloc(sizeof(*redist), GFP_KERNEL);if (!redist)return -ENOMEM;redist->mpidr = mpidr;redist->phys_base = phys_base;if (acpi_gic_version() == ACPI_MADT_GIC_VERSION_V3)/* RD_base + SGI_base */redist->redist_base = ioremap(phys_base, 2 * SZ_64K);else/** RD_base + SGI_base + VLPI_base,* we don't map reserved page as it's buggy to access it*/redist->redist_base = ioremap(phys_base, 3 * SZ_64K);
This will break things in the gic_populate_rdist() loop for the case where there is only one rdist entry per CPU and each entry is not delimited by the LAST bit in the GICR_TYPER. This is possible when the hardware only sets the LAST bit for the last rdist entry. In such a case, the __iomem *ptr in the loop will increment endlessly until it triggers some kind of illegal memory read.
Need to think some more, but 2 options come to mind:
- Figure a way to allocate contiguous memory for all rdist_regions. You already do that in GICR case: gic_acpi_register_redist().
- Find a way to break the loop in gic_populate_rdist() if the hardware doesn't have the LAST bit set. If we know number of entries in each rdist region this could be possible.
Cheers, Ashwin.
Hi Ashwin,
On 07/01/2015 04:23 AM, Ashwin Chaugule wrote:
Hi Hanjun,
On 26 June 2015 at 09:51, Hanjun Guo hanjun.guo@linaro.org wrote:
On systems supporting GICv3 and above, the field of GICR Base Address holds the 64-bit physical address of the associated Redistributor if the GIC Redistributors are not in the always-on power domain, so instead of init GICR regions via GIC redistributor stricture, init it with GICR base address in GICC structures.
Unless I misunderstand, I think you want the opposite statement. i.e. If the redists are in always power-on domain, then the base addresses should be in GICR structs and not in GICC.
I mean the same thing if I under stand correctly :)
Signed-off-by: Hanjun Guo hanjun.guo@linaro.org
drivers/irqchip/irq-gic-acpi.c | 34 ++++++- drivers/irqchip/irq-gic-v3.c | 181 ++++++++++++++++++++++++++++++++--- include/linux/irqchip/arm-gic-acpi.h | 2 + 3 files changed, 200 insertions(+), 17 deletions(-)
diff --git a/drivers/irqchip/irq-gic-acpi.c b/drivers/irqchip/irq-gic-acpi.c index 236bba5..732efa0 100644 --- a/drivers/irqchip/irq-gic-acpi.c +++ b/drivers/irqchip/irq-gic-acpi.c @@ -21,6 +21,11 @@ static u8 gic_version __initdata = ACPI_MADT_GIC_VERSION_NONE;
static phys_addr_t dist_phy_base __initdata;
+u8 __init acpi_gic_version(void) +{
return gic_version;+}
- static int __init acpi_gic_parse_distributor(struct acpi_subtable_header *header, const unsigned long end)
@@ -38,6 +43,27 @@ acpi_gic_parse_distributor(struct acpi_subtable_header *header, }
static int __init +gic_acpi_parse_madt_gicc(struct acpi_subtable_header *header,
const unsigned long end)+{
struct acpi_madt_generic_interrupt *gicc;gicc = (struct acpi_madt_generic_interrupt *)header;if (BAD_MADT_ENTRY(gicc, end))return -EINVAL;/**If GICC is enabled but no gicr base address, which means GICR is* not presented via GICC*/if ((gicc->flags & ACPI_MADT_ENABLED) && !gicc->gicr_base_address)return -ENODEV;return 0;+}
+static int __init match_gic_redist(struct acpi_subtable_header *header, const unsigned long end) { return 0; @@ -54,8 +80,12 @@ static bool __init acpi_gic_redist_is_present(void) /* has at least one GIC redistributor entry */ if (count > 0) return true;
elsereturn false;
/* else try to find GICR base in GICC entries */count = acpi_table_parse_madt(ACPI_MADT_TYPE_GENERIC_INTERRUPT,gic_acpi_parse_madt_gicc, 0);return count > 0;}
static int __init acpi_gic_version_init(void)
diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c index a1c4c74..755c940 100644 --- a/drivers/irqchip/irq-gic-v3.c +++ b/drivers/irqchip/irq-gic-v3.c @@ -389,9 +389,8 @@ static void __init gic_dist_init(void) writeq_relaxed(affinity, base + GICD_IROUTER + i * 8); }
-static int gic_populate_rdist(void) +static int gic_populate_rdist_with_regions(u64 mpidr) {
u64 mpidr = cpu_logical_map(smp_processor_id()); u64 typer; u32 aff; int i;@@ -439,6 +438,33 @@ static int gic_populate_rdist(void) } while (!(typer & GICR_TYPER_LAST)); }
return -ENODEV;+}
+#ifdef CONFIG_ACPI +/*
- Populate redist when GIC redistributor address is presented in ACPI
- MADT GICC entries
- */
+static int gic_populate_rdist_with_gicr_base(u64 mpidr); +#else +static inline int gic_populate_rdist_with_gicr_base(u64 mpidr) +{
return -ENODEV;+} +#endif +static int gic_populate_rdist(void) +{
u64 mpidr = cpu_logical_map(smp_processor_id());if (!gic_data.nr_redist_regions) {if (!gic_populate_rdist_with_gicr_base(mpidr))return 0;} else {if (!gic_populate_rdist_with_regions(mpidr))return 0;}/* We couldn't even deal with ourselves... */ WARN(true, "CPU%d: mpidr %llx has no re-distributor!\n", smp_processor_id(), (unsigned long long)mpidr);@@ -906,6 +932,16 @@ IRQCHIP_DECLARE(gic_v3, "arm,gic-v3", gic_of_init); #endif
#ifdef CONFIG_ACPI
+struct acpi_gicc_redist {
struct list_head list;u64 mpidr;phys_addr_t phys_base;void __iomem *redist_base;+};
+static LIST_HEAD(redist_list);
- static struct redist_region *redist_regs __initdata; static u32 nr_redist_regions __initdata; static phys_addr_t dist_phy_base __initdata;
@@ -938,6 +974,17 @@ gic_acpi_register_redist(u64 phys_base, u64 size) return 0; }
+static void __init +gic_acpi_release_redist_regions(void) +{
int i;for (i = 0; i < nr_redist_regions; i++)if (redist_regs[i].redist_base)iounmap(redist_regs[i].redist_base);kfree(redist_regs);+}
- static int __init gic_acpi_parse_madt_redist(struct acpi_subtable_header *header, const unsigned long end)
@@ -970,10 +1017,98 @@ gic_acpi_parse_madt_distributor(struct acpi_subtable_header *header, }
static int __init -gic_v3_acpi_init(struct acpi_table_header *table) +gic_acpi_add_gicc_redist(phys_addr_t phys_base, u64 mpidr) {
int count, i, err = 0;
struct acpi_gicc_redist *redist;redist = kzalloc(sizeof(*redist), GFP_KERNEL);if (!redist)return -ENOMEM;redist->mpidr = mpidr;redist->phys_base = phys_base;if (acpi_gic_version() == ACPI_MADT_GIC_VERSION_V3)/* RD_base + SGI_base */redist->redist_base = ioremap(phys_base, 2 * SZ_64K);else/** RD_base + SGI_base + VLPI_base,* we don't map reserved page as it's buggy to access it*/redist->redist_base = ioremap(phys_base, 3 * SZ_64K);This will break things in the gic_populate_rdist() loop for the case where there is only one rdist entry per CPU and each entry is not delimited by the LAST bit in the GICR_TYPER. This is possible when the hardware only sets the LAST bit for the last rdist entry. In such a case, the __iomem *ptr in the loop will increment endlessly until it triggers some kind of illegal memory read.
Need to think some more, but 2 options come to mind:
- Figure a way to allocate contiguous memory for all rdist_regions.
You already do that in GICR case: gic_acpi_register_redist().
- Find a way to break the loop in gic_populate_rdist() if the hardware
doesn't have the LAST bit set. If we know number of entries in each rdist region this could be possible.
I'm not using the normal dist population mechanism, I'm use a list to link all the GICR base in GICC and match it with the MPIDR.
For the gicV3 init using GIC redistributor regions, will be the same as you said, hope I didn't misunderstand your question.
Thanks Hanjun
On 1 July 2015 at 10:24, Hanjun Guo hanjun.guo@linaro.org wrote:
Hi Ashwin,
On 07/01/2015 04:23 AM, Ashwin Chaugule wrote:
Hi Hanjun,
On 26 June 2015 at 09:51, Hanjun Guo hanjun.guo@linaro.org wrote:
On systems supporting GICv3 and above, the field of GICR Base Address holds the 64-bit physical address of the associated Redistributor if the GIC Redistributors are not in the always-on power domain, so instead of init GICR regions via GIC redistributor stricture, init it with GICR base address in GICC structures.
Unless I misunderstand, I think you want the opposite statement. i.e. If the redists are in always power-on domain, then the base addresses should be in GICR structs and not in GICC.
I mean the same thing if I under stand correctly :)
Yea, sorry I just re-read it. Was reviewing this during meetings. ;)
Signed-off-by: Hanjun Guo hanjun.guo@linaro.org
drivers/irqchip/irq-gic-acpi.c | 34 ++++++- drivers/irqchip/irq-gic-v3.c | 181 ++++++++++++++++++++++++++++++++--- include/linux/irqchip/arm-gic-acpi.h | 2 + 3 files changed, 200 insertions(+), 17 deletions(-)
diff --git a/drivers/irqchip/irq-gic-acpi.c b/drivers/irqchip/irq-gic-acpi.c index 236bba5..732efa0 100644 --- a/drivers/irqchip/irq-gic-acpi.c +++ b/drivers/irqchip/irq-gic-acpi.c @@ -21,6 +21,11 @@ static u8 gic_version __initdata = ACPI_MADT_GIC_VERSION_NONE;
static phys_addr_t dist_phy_base __initdata;
+u8 __init acpi_gic_version(void) +{
return gic_version;+}
- static int __init acpi_gic_parse_distributor(struct acpi_subtable_header *header, const unsigned long end)
@@ -38,6 +43,27 @@ acpi_gic_parse_distributor(struct acpi_subtable_header *header, }
static int __init +gic_acpi_parse_madt_gicc(struct acpi_subtable_header *header,
const unsigned long end)+{
struct acpi_madt_generic_interrupt *gicc;gicc = (struct acpi_madt_generic_interrupt *)header;if (BAD_MADT_ENTRY(gicc, end))return -EINVAL;/**If GICC is enabled but no gicr base address, which means GICRis
* not presented via GICC*/if ((gicc->flags & ACPI_MADT_ENABLED) &&!gicc->gicr_base_address)
return -ENODEV;return 0;+}
+static int __init match_gic_redist(struct acpi_subtable_header *header, const unsigned long end) { return 0; @@ -54,8 +80,12 @@ static bool __init acpi_gic_redist_is_present(void) /* has at least one GIC redistributor entry */ if (count > 0) return true;
elsereturn false;
/* else try to find GICR base in GICC entries */count = acpi_table_parse_madt(ACPI_MADT_TYPE_GENERIC_INTERRUPT,gic_acpi_parse_madt_gicc, 0);return count > 0;}
static int __init acpi_gic_version_init(void)
diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c index a1c4c74..755c940 100644 --- a/drivers/irqchip/irq-gic-v3.c +++ b/drivers/irqchip/irq-gic-v3.c @@ -389,9 +389,8 @@ static void __init gic_dist_init(void) writeq_relaxed(affinity, base + GICD_IROUTER + i * 8); }
-static int gic_populate_rdist(void) +static int gic_populate_rdist_with_regions(u64 mpidr) {
u64 mpidr = cpu_logical_map(smp_processor_id()); u64 typer; u32 aff; int i;@@ -439,6 +438,33 @@ static int gic_populate_rdist(void) } while (!(typer & GICR_TYPER_LAST)); }
return -ENODEV;+}
+#ifdef CONFIG_ACPI +/*
- Populate redist when GIC redistributor address is presented in ACPI
- MADT GICC entries
- */
+static int gic_populate_rdist_with_gicr_base(u64 mpidr); +#else +static inline int gic_populate_rdist_with_gicr_base(u64 mpidr) +{
return -ENODEV;+} +#endif +static int gic_populate_rdist(void) +{
u64 mpidr = cpu_logical_map(smp_processor_id());if (!gic_data.nr_redist_regions) {if (!gic_populate_rdist_with_gicr_base(mpidr))return 0;} else {if (!gic_populate_rdist_with_regions(mpidr))return 0;}/* We couldn't even deal with ourselves... */ WARN(true, "CPU%d: mpidr %llx has no re-distributor!\n", smp_processor_id(), (unsigned long long)mpidr);@@ -906,6 +932,16 @@ IRQCHIP_DECLARE(gic_v3, "arm,gic-v3", gic_of_init); #endif
#ifdef CONFIG_ACPI
+struct acpi_gicc_redist {
struct list_head list;u64 mpidr;phys_addr_t phys_base;void __iomem *redist_base;+};
+static LIST_HEAD(redist_list);
- static struct redist_region *redist_regs __initdata; static u32 nr_redist_regions __initdata; static phys_addr_t dist_phy_base __initdata;
@@ -938,6 +974,17 @@ gic_acpi_register_redist(u64 phys_base, u64 size) return 0; }
+static void __init +gic_acpi_release_redist_regions(void) +{
int i;for (i = 0; i < nr_redist_regions; i++)if (redist_regs[i].redist_base)iounmap(redist_regs[i].redist_base);kfree(redist_regs);+}
- static int __init gic_acpi_parse_madt_redist(struct acpi_subtable_header *header, const unsigned long end)
@@ -970,10 +1017,98 @@ gic_acpi_parse_madt_distributor(struct acpi_subtable_header *header, }
static int __init -gic_v3_acpi_init(struct acpi_table_header *table) +gic_acpi_add_gicc_redist(phys_addr_t phys_base, u64 mpidr) {
int count, i, err = 0;
struct acpi_gicc_redist *redist;redist = kzalloc(sizeof(*redist), GFP_KERNEL);if (!redist)return -ENOMEM;redist->mpidr = mpidr;redist->phys_base = phys_base;if (acpi_gic_version() == ACPI_MADT_GIC_VERSION_V3)/* RD_base + SGI_base */redist->redist_base = ioremap(phys_base, 2 * SZ_64K);else/** RD_base + SGI_base + VLPI_base,* we don't map reserved page as it's buggy to access it*/redist->redist_base = ioremap(phys_base, 3 * SZ_64K);This will break things in the gic_populate_rdist() loop for the case where there is only one rdist entry per CPU and each entry is not delimited by the LAST bit in the GICR_TYPER. This is possible when the hardware only sets the LAST bit for the last rdist entry. In such a case, the __iomem *ptr in the loop will increment endlessly until it triggers some kind of illegal memory read.
Need to think some more, but 2 options come to mind:
- Figure a way to allocate contiguous memory for all rdist_regions.
You already do that in GICR case: gic_acpi_register_redist().
- Find a way to break the loop in gic_populate_rdist() if the hardware
doesn't have the LAST bit set. If we know number of entries in each rdist region this could be possible.
I'm not using the normal dist population mechanism, I'm use a list to link all the GICR base in GICC and match it with the MPIDR.
For the gicV3 init using GIC redistributor regions, will be the same as you said, hope I didn't misunderstand your question.
Ah. I see. So does each redist_base only point to one entry? IIUC, Marc's code doesn't assume that. He loops until he finds the LAST bit. Honestly I dont understand why we can't have all the redist regions listed in one table. That would make things much simpler but maybe I'm missing some details though.
Thanks, Ashwin.
On 07/02/2015 02:01 AM, Ashwin Chaugule wrote:
static int __init
>-gic_v3_acpi_init(struct acpi_table_header *table) >+gic_acpi_add_gicc_redist(phys_addr_t phys_base, u64 mpidr) > { >- int count, i, err = 0; >+ struct acpi_gicc_redist *redist; >+ >+ redist = kzalloc(sizeof(*redist), GFP_KERNEL); >+ if (!redist) >+ return -ENOMEM; >+ >+ redist->mpidr = mpidr; >+ redist->phys_base = phys_base; >+ >+ if (acpi_gic_version() == ACPI_MADT_GIC_VERSION_V3) >+ /* RD_base + SGI_base */ >+ redist->redist_base = ioremap(phys_base, 2 * SZ_64K); >+ else >+ /* >+ * RD_base + SGI_base + VLPI_base, >+ * we don't map reserved page as it's buggy to access it >+ */ >+ redist->redist_base = ioremap(phys_base, 3 * SZ_64K);
This will break things in the gic_populate_rdist() loop for the case where there is only one rdist entry per CPU and each entry is not delimited by the LAST bit in the GICR_TYPER. This is possible when the hardware only sets the LAST bit for the last rdist entry. In such a case, the __iomem *ptr in the loop will increment endlessly until it triggers some kind of illegal memory read.
Need to think some more, but 2 options come to mind:
- Figure a way to allocate contiguous memory for all rdist_regions.
You already do that in GICR case: gic_acpi_register_redist().
- Find a way to break the loop in gic_populate_rdist() if the hardware
doesn't have the LAST bit set. If we know number of entries in each rdist region this could be possible.
I'm not using the normal dist population mechanism, I'm use a list to link all the GICR base in GICC and match it with the MPIDR.
For the gicV3 init using GIC redistributor regions, will be the same as you said, hope I didn't misunderstand your question.
Ah. I see. So does each redist_base only point to one entry? IIUC,
yes.
Marc's code doesn't assume that. He loops until he finds the LAST bit.
And yes :)
Honestly I dont understand why we can't have all the redist regions listed in one table. That would make things much simpler but maybe I'm missing some details though.
I already handled the GICR regions just as DT does in another GICv3 patch [1], there are GICR regions entries in MADT and we can reuse Marc's code.
[1]: http://lkml.iu.edu/hypermail/linux/kernel/1506.2/03033.html
Thanks Hanjun
-
Ashwin Chaugule -
Hanjun Guo