* [PATCH v4 3/4] irqchip: GIC: Convert to EOImode == 1
@ 2015-08-26 16:00 ` Marc Zyngier
0 siblings, 0 replies; 49+ messages in thread
From: Marc Zyngier @ 2015-08-26 16:00 UTC (permalink / raw)
To: Thomas Gleixner, Jason Cooper
Cc: Christoffer Dall, Jiang Liu, Eric Auger, linux-arm-kernel, kvmarm,
kvm, linux-kernel
So far, GICv2 has been used with EOImode == 0. The effect of this
mode is to perform the priority drop and the deactivation of the
interrupt at the same time.
While this works perfectly for Linux (we only have a single priority),
it causes issues when an interrupt is forwarded to a guest, and when
we want the guest to perform the EOI itself.
For this case, the GIC architecture provides EOImode == 1, where:
- A write to the EOI register drops the priority of the interrupt
and leaves it active. Other interrupts at the same priority level
can now be taken, but the active interrupt cannot be taken again
- A write to the DIR marks the interrupt as inactive, meaning it can
now be taken again.
We only enable this feature when booted in HYP mode and that
the device-tree reported a suitable CPU interface. Observable behaviour
should remain unchanged.
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
drivers/irqchip/irq-gic.c | 69 +++++++++++++++++++++++++++++++++++++++--
include/linux/irqchip/arm-gic.h | 4 +++
2 files changed, 70 insertions(+), 3 deletions(-)
diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index 4dd8826..9215897 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -46,6 +46,7 @@
#include <asm/irq.h>
#include <asm/exception.h>
#include <asm/smp_plat.h>
+#include <asm/virt.h>
#include "irq-gic-common.h"
#include "irqchip.h"
@@ -82,6 +83,8 @@ static DEFINE_RAW_SPINLOCK(irq_controller_lock);
#define NR_GIC_CPU_IF 8
static u8 gic_cpu_map[NR_GIC_CPU_IF] __read_mostly;
+static struct static_key supports_deactivate = STATIC_KEY_INIT_TRUE;
+
#ifndef MAX_GIC_NR
#define MAX_GIC_NR 1
#endif
@@ -157,6 +160,11 @@ static void gic_mask_irq(struct irq_data *d)
gic_poke_irq(d, GIC_DIST_ENABLE_CLEAR);
}
+static void gic_eoimode1_mask_irq(struct irq_data *d)
+{
+ gic_mask_irq(d);
+}
+
static void gic_unmask_irq(struct irq_data *d)
{
gic_poke_irq(d, GIC_DIST_ENABLE_SET);
@@ -167,6 +175,11 @@ static void gic_eoi_irq(struct irq_data *d)
writel_relaxed(gic_irq(d), gic_cpu_base(d) + GIC_CPU_EOI);
}
+static void gic_eoimode1_eoi_irq(struct irq_data *d)
+{
+ writel_relaxed(gic_irq(d), gic_cpu_base(d) + GIC_CPU_DEACTIVATE);
+}
+
static int gic_irq_set_irqchip_state(struct irq_data *d,
enum irqchip_irq_state which, bool val)
{
@@ -272,11 +285,15 @@ static void __exception_irq_entry gic_handle_irq(struct pt_regs *regs)
irqnr = irqstat & GICC_IAR_INT_ID_MASK;
if (likely(irqnr > 15 && irqnr < 1021)) {
+ if (static_key_true(&supports_deactivate))
+ writel_relaxed(irqstat, cpu_base + GIC_CPU_EOI);
handle_domain_irq(gic->domain, irqnr, regs);
continue;
}
if (irqnr < 16) {
writel_relaxed(irqstat, cpu_base + GIC_CPU_EOI);
+ if (static_key_true(&supports_deactivate))
+ writel_relaxed(irqstat, cpu_base + GIC_CPU_DEACTIVATE);
#ifdef CONFIG_SMP
handle_IPI(irqnr, regs);
#endif
@@ -327,6 +344,20 @@ static struct irq_chip gic_chip = {
.flags = IRQCHIP_SET_TYPE_MASKED,
};
+static struct irq_chip gic_eoimode1_chip = {
+ .name = "GICv2",
+ .irq_mask = gic_eoimode1_mask_irq,
+ .irq_unmask = gic_unmask_irq,
+ .irq_eoi = gic_eoimode1_eoi_irq,
+ .irq_set_type = gic_set_type,
+#ifdef CONFIG_SMP
+ .irq_set_affinity = gic_set_affinity,
+#endif
+ .irq_get_irqchip_state = gic_irq_get_irqchip_state,
+ .irq_set_irqchip_state = gic_irq_set_irqchip_state,
+ .flags = IRQCHIP_SET_TYPE_MASKED,
+};
+
void __init gic_cascade_irq(unsigned int gic_nr, unsigned int irq)
{
if (gic_nr >= MAX_GIC_NR)
@@ -359,6 +390,10 @@ static void gic_cpu_if_up(void)
{
void __iomem *cpu_base = gic_data_cpu_base(&gic_data[0]);
u32 bypass = 0;
+ u32 mode = 0;
+
+ if (static_key_true(&supports_deactivate))
+ mode = GIC_CPU_CTRL_EOImodeNS;
/*
* Preserve bypass disable bits to be written back later
@@ -366,7 +401,7 @@ static void gic_cpu_if_up(void)
bypass = readl(cpu_base + GIC_CPU_CTRL);
bypass &= GICC_DIS_BYPASS_MASK;
- writel_relaxed(bypass | GICC_ENABLE, cpu_base + GIC_CPU_CTRL);
+ writel_relaxed(bypass | mode | GICC_ENABLE, cpu_base + GIC_CPU_CTRL);
}
@@ -789,13 +824,20 @@ void __init gic_init_physaddr(struct device_node *node)
static int gic_irq_domain_map(struct irq_domain *d, unsigned int irq,
irq_hw_number_t hw)
{
+ struct irq_chip *chip = &gic_chip;
+
+ if (static_key_true(&supports_deactivate)) {
+ if (d->host_data == (void *)&gic_data[0])
+ chip = &gic_eoimode1_chip;
+ }
+
if (hw < 32) {
irq_set_percpu_devid(irq);
- irq_domain_set_info(d, irq, hw, &gic_chip, d->host_data,
+ irq_domain_set_info(d, irq, hw, chip, d->host_data,
handle_percpu_devid_irq, NULL, NULL);
set_irq_flags(irq, IRQF_VALID | IRQF_NOAUTOEN);
} else {
- irq_domain_set_info(d, irq, hw, &gic_chip, d->host_data,
+ irq_domain_set_info(d, irq, hw, chip, d->host_data,
handle_fasteoi_irq, NULL, NULL);
set_irq_flags(irq, IRQF_VALID | IRQF_PROBE);
}
@@ -986,6 +1028,8 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start,
register_cpu_notifier(&gic_cpu_notifier);
#endif
set_handle_irq(gic_handle_irq);
+ if (static_key_true(&supports_deactivate))
+ pr_info("GIC: Using split EOI/Deactivate mode\n");
}
gic_dist_init(gic);
@@ -1001,6 +1045,7 @@ gic_of_init(struct device_node *node, struct device_node *parent)
{
void __iomem *cpu_base;
void __iomem *dist_base;
+ struct resource cpu_res;
u32 percpu_offset;
int irq;
@@ -1013,6 +1058,16 @@ gic_of_init(struct device_node *node, struct device_node *parent)
cpu_base = of_iomap(node, 1);
WARN(!cpu_base, "unable to map gic cpu registers\n");
+ of_address_to_resource(node, 1, &cpu_res);
+
+ /*
+ * Disable split EOI/Deactivate if either HYP is not available
+ * or the CPU interface is too small.
+ */
+ if (gic_cnt == 0 && (!is_hyp_mode_available() ||
+ resource_size(&cpu_res) < SZ_8K))
+ static_key_slow_dec(&supports_deactivate);
+
if (of_property_read_u32(node, "cpu-offset", &percpu_offset))
percpu_offset = 0;
@@ -1132,6 +1187,14 @@ gic_v2_acpi_init(struct acpi_table_header *table)
}
/*
+ * Disable split EOI/Deactivate if HYP is not available. ACPI
+ * guarantees that we'll always have a GICv2, so the CPU
+ * interface will always be the right size.
+ */
+ if (!is_hyp_mode_available())
+ static_key_slow_dec(&supports_deactivate);
+
+ /*
* Initialize zero GIC instance (no multi-GIC support). Also, set GIC
* as default IRQ domain to allow for GSI registration and GSI to IRQ
* number translation (see acpi_register_gsi() and acpi_gsi_to_irq()).
diff --git a/include/linux/irqchip/arm-gic.h b/include/linux/irqchip/arm-gic.h
index 9de976b..b1533c0 100644
--- a/include/linux/irqchip/arm-gic.h
+++ b/include/linux/irqchip/arm-gic.h
@@ -20,9 +20,13 @@
#define GIC_CPU_ALIAS_BINPOINT 0x1c
#define GIC_CPU_ACTIVEPRIO 0xd0
#define GIC_CPU_IDENT 0xfc
+#define GIC_CPU_DEACTIVATE 0x1000
#define GICC_ENABLE 0x1
#define GICC_INT_PRI_THRESHOLD 0xf0
+
+#define GIC_CPU_CTRL_EOImodeNS (1 << 9)
+
#define GICC_IAR_INT_ID_MASK 0x3ff
#define GICC_INT_SPURIOUS 1023
#define GICC_DIS_BYPASS_MASK 0x1e0
--
2.1.4
^ permalink raw reply related [flat|nested] 49+ messages in thread* [PATCH v4 3/4] irqchip: GIC: Convert to EOImode == 1
@ 2015-08-26 16:00 ` Marc Zyngier
0 siblings, 0 replies; 49+ messages in thread
From: Marc Zyngier @ 2015-08-26 16:00 UTC (permalink / raw)
To: linux-arm-kernel
So far, GICv2 has been used with EOImode == 0. The effect of this
mode is to perform the priority drop and the deactivation of the
interrupt at the same time.
While this works perfectly for Linux (we only have a single priority),
it causes issues when an interrupt is forwarded to a guest, and when
we want the guest to perform the EOI itself.
For this case, the GIC architecture provides EOImode == 1, where:
- A write to the EOI register drops the priority of the interrupt
and leaves it active. Other interrupts at the same priority level
can now be taken, but the active interrupt cannot be taken again
- A write to the DIR marks the interrupt as inactive, meaning it can
now be taken again.
We only enable this feature when booted in HYP mode and that
the device-tree reported a suitable CPU interface. Observable behaviour
should remain unchanged.
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
drivers/irqchip/irq-gic.c | 69 +++++++++++++++++++++++++++++++++++++++--
include/linux/irqchip/arm-gic.h | 4 +++
2 files changed, 70 insertions(+), 3 deletions(-)
diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index 4dd8826..9215897 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -46,6 +46,7 @@
#include <asm/irq.h>
#include <asm/exception.h>
#include <asm/smp_plat.h>
+#include <asm/virt.h>
#include "irq-gic-common.h"
#include "irqchip.h"
@@ -82,6 +83,8 @@ static DEFINE_RAW_SPINLOCK(irq_controller_lock);
#define NR_GIC_CPU_IF 8
static u8 gic_cpu_map[NR_GIC_CPU_IF] __read_mostly;
+static struct static_key supports_deactivate = STATIC_KEY_INIT_TRUE;
+
#ifndef MAX_GIC_NR
#define MAX_GIC_NR 1
#endif
@@ -157,6 +160,11 @@ static void gic_mask_irq(struct irq_data *d)
gic_poke_irq(d, GIC_DIST_ENABLE_CLEAR);
}
+static void gic_eoimode1_mask_irq(struct irq_data *d)
+{
+ gic_mask_irq(d);
+}
+
static void gic_unmask_irq(struct irq_data *d)
{
gic_poke_irq(d, GIC_DIST_ENABLE_SET);
@@ -167,6 +175,11 @@ static void gic_eoi_irq(struct irq_data *d)
writel_relaxed(gic_irq(d), gic_cpu_base(d) + GIC_CPU_EOI);
}
+static void gic_eoimode1_eoi_irq(struct irq_data *d)
+{
+ writel_relaxed(gic_irq(d), gic_cpu_base(d) + GIC_CPU_DEACTIVATE);
+}
+
static int gic_irq_set_irqchip_state(struct irq_data *d,
enum irqchip_irq_state which, bool val)
{
@@ -272,11 +285,15 @@ static void __exception_irq_entry gic_handle_irq(struct pt_regs *regs)
irqnr = irqstat & GICC_IAR_INT_ID_MASK;
if (likely(irqnr > 15 && irqnr < 1021)) {
+ if (static_key_true(&supports_deactivate))
+ writel_relaxed(irqstat, cpu_base + GIC_CPU_EOI);
handle_domain_irq(gic->domain, irqnr, regs);
continue;
}
if (irqnr < 16) {
writel_relaxed(irqstat, cpu_base + GIC_CPU_EOI);
+ if (static_key_true(&supports_deactivate))
+ writel_relaxed(irqstat, cpu_base + GIC_CPU_DEACTIVATE);
#ifdef CONFIG_SMP
handle_IPI(irqnr, regs);
#endif
@@ -327,6 +344,20 @@ static struct irq_chip gic_chip = {
.flags = IRQCHIP_SET_TYPE_MASKED,
};
+static struct irq_chip gic_eoimode1_chip = {
+ .name = "GICv2",
+ .irq_mask = gic_eoimode1_mask_irq,
+ .irq_unmask = gic_unmask_irq,
+ .irq_eoi = gic_eoimode1_eoi_irq,
+ .irq_set_type = gic_set_type,
+#ifdef CONFIG_SMP
+ .irq_set_affinity = gic_set_affinity,
+#endif
+ .irq_get_irqchip_state = gic_irq_get_irqchip_state,
+ .irq_set_irqchip_state = gic_irq_set_irqchip_state,
+ .flags = IRQCHIP_SET_TYPE_MASKED,
+};
+
void __init gic_cascade_irq(unsigned int gic_nr, unsigned int irq)
{
if (gic_nr >= MAX_GIC_NR)
@@ -359,6 +390,10 @@ static void gic_cpu_if_up(void)
{
void __iomem *cpu_base = gic_data_cpu_base(&gic_data[0]);
u32 bypass = 0;
+ u32 mode = 0;
+
+ if (static_key_true(&supports_deactivate))
+ mode = GIC_CPU_CTRL_EOImodeNS;
/*
* Preserve bypass disable bits to be written back later
@@ -366,7 +401,7 @@ static void gic_cpu_if_up(void)
bypass = readl(cpu_base + GIC_CPU_CTRL);
bypass &= GICC_DIS_BYPASS_MASK;
- writel_relaxed(bypass | GICC_ENABLE, cpu_base + GIC_CPU_CTRL);
+ writel_relaxed(bypass | mode | GICC_ENABLE, cpu_base + GIC_CPU_CTRL);
}
@@ -789,13 +824,20 @@ void __init gic_init_physaddr(struct device_node *node)
static int gic_irq_domain_map(struct irq_domain *d, unsigned int irq,
irq_hw_number_t hw)
{
+ struct irq_chip *chip = &gic_chip;
+
+ if (static_key_true(&supports_deactivate)) {
+ if (d->host_data == (void *)&gic_data[0])
+ chip = &gic_eoimode1_chip;
+ }
+
if (hw < 32) {
irq_set_percpu_devid(irq);
- irq_domain_set_info(d, irq, hw, &gic_chip, d->host_data,
+ irq_domain_set_info(d, irq, hw, chip, d->host_data,
handle_percpu_devid_irq, NULL, NULL);
set_irq_flags(irq, IRQF_VALID | IRQF_NOAUTOEN);
} else {
- irq_domain_set_info(d, irq, hw, &gic_chip, d->host_data,
+ irq_domain_set_info(d, irq, hw, chip, d->host_data,
handle_fasteoi_irq, NULL, NULL);
set_irq_flags(irq, IRQF_VALID | IRQF_PROBE);
}
@@ -986,6 +1028,8 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start,
register_cpu_notifier(&gic_cpu_notifier);
#endif
set_handle_irq(gic_handle_irq);
+ if (static_key_true(&supports_deactivate))
+ pr_info("GIC: Using split EOI/Deactivate mode\n");
}
gic_dist_init(gic);
@@ -1001,6 +1045,7 @@ gic_of_init(struct device_node *node, struct device_node *parent)
{
void __iomem *cpu_base;
void __iomem *dist_base;
+ struct resource cpu_res;
u32 percpu_offset;
int irq;
@@ -1013,6 +1058,16 @@ gic_of_init(struct device_node *node, struct device_node *parent)
cpu_base = of_iomap(node, 1);
WARN(!cpu_base, "unable to map gic cpu registers\n");
+ of_address_to_resource(node, 1, &cpu_res);
+
+ /*
+ * Disable split EOI/Deactivate if either HYP is not available
+ * or the CPU interface is too small.
+ */
+ if (gic_cnt == 0 && (!is_hyp_mode_available() ||
+ resource_size(&cpu_res) < SZ_8K))
+ static_key_slow_dec(&supports_deactivate);
+
if (of_property_read_u32(node, "cpu-offset", &percpu_offset))
percpu_offset = 0;
@@ -1132,6 +1187,14 @@ gic_v2_acpi_init(struct acpi_table_header *table)
}
/*
+ * Disable split EOI/Deactivate if HYP is not available. ACPI
+ * guarantees that we'll always have a GICv2, so the CPU
+ * interface will always be the right size.
+ */
+ if (!is_hyp_mode_available())
+ static_key_slow_dec(&supports_deactivate);
+
+ /*
* Initialize zero GIC instance (no multi-GIC support). Also, set GIC
* as default IRQ domain to allow for GSI registration and GSI to IRQ
* number translation (see acpi_register_gsi() and acpi_gsi_to_irq()).
diff --git a/include/linux/irqchip/arm-gic.h b/include/linux/irqchip/arm-gic.h
index 9de976b..b1533c0 100644
--- a/include/linux/irqchip/arm-gic.h
+++ b/include/linux/irqchip/arm-gic.h
@@ -20,9 +20,13 @@
#define GIC_CPU_ALIAS_BINPOINT 0x1c
#define GIC_CPU_ACTIVEPRIO 0xd0
#define GIC_CPU_IDENT 0xfc
+#define GIC_CPU_DEACTIVATE 0x1000
#define GICC_ENABLE 0x1
#define GICC_INT_PRI_THRESHOLD 0xf0
+
+#define GIC_CPU_CTRL_EOImodeNS (1 << 9)
+
#define GICC_IAR_INT_ID_MASK 0x3ff
#define GICC_INT_SPURIOUS 1023
#define GICC_DIS_BYPASS_MASK 0x1e0
--
2.1.4
^ permalink raw reply related [flat|nested] 49+ messages in thread* [tip:irq/core] irqchip/GIC: Convert to EOImode == 1
2015-08-26 16:00 ` Marc Zyngier
(?)
(?)
@ 2015-08-27 15:16 ` tip-bot for Marc Zyngier
-1 siblings, 0 replies; 49+ messages in thread
From: tip-bot for Marc Zyngier @ 2015-08-27 15:16 UTC (permalink / raw)
To: linux-tip-commits
Cc: hpa, linux-kernel, marc.zyngier, jiang.liu, tglx, mingo, jason,
linux-arm-kernel, eric.auger, christoffer.dall
Commit-ID: 0b996fd35957a30568cddbce05b917c1897966e0
Gitweb: http://git.kernel.org/tip/0b996fd35957a30568cddbce05b917c1897966e0
Author: Marc Zyngier <marc.zyngier@arm.com>
AuthorDate: Wed, 26 Aug 2015 17:00:44 +0100
Committer: Thomas Gleixner <tglx@linutronix.de>
CommitDate: Thu, 27 Aug 2015 17:13:49 +0200
irqchip/GIC: Convert to EOImode == 1
So far, GICv2 has been used with EOImode == 0. The effect of this
mode is to perform the priority drop and the deactivation of the
interrupt at the same time.
While this works perfectly for Linux (we only have a single priority),
it causes issues when an interrupt is forwarded to a guest, and when
we want the guest to perform the EOI itself.
For this case, the GIC architecture provides EOImode == 1, where:
- A write to the EOI register drops the priority of the interrupt
and leaves it active. Other interrupts at the same priority level
can now be taken, but the active interrupt cannot be taken again
- A write to the DIR marks the interrupt as inactive, meaning it can
now be taken again.
We only enable this feature when booted in HYP mode and that
the device-tree reported a suitable CPU interface. Observable behaviour
should remain unchanged.
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
Reviewed-and-tested-by: Eric Auger <eric.auger@linaro.org>
Cc: Christoffer Dall <christoffer.dall@linaro.org>
Cc: Jiang Liu <jiang.liu@linux.intel.com>
Cc: <linux-arm-kernel@lists.infradead.org>
Cc: kvmarm@lists.cs.columbia.edu
Cc: Jason Cooper <jason@lakedaemon.net>
Link: http://lkml.kernel.org/r/1440604845-28229-4-git-send-email-marc.zyngier@arm.com
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
drivers/irqchip/irq-gic.c | 71 +++++++++++++++++++++++++++++++++++++++--
include/linux/irqchip/arm-gic.h | 4 +++
2 files changed, 72 insertions(+), 3 deletions(-)
diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index aa3e7b8..c835f4c 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -47,6 +47,7 @@
#include <asm/irq.h>
#include <asm/exception.h>
#include <asm/smp_plat.h>
+#include <asm/virt.h>
#include "irq-gic-common.h"
@@ -82,6 +83,8 @@ static DEFINE_RAW_SPINLOCK(irq_controller_lock);
#define NR_GIC_CPU_IF 8
static u8 gic_cpu_map[NR_GIC_CPU_IF] __read_mostly;
+static struct static_key supports_deactivate = STATIC_KEY_INIT_TRUE;
+
#ifndef MAX_GIC_NR
#define MAX_GIC_NR 1
#endif
@@ -157,6 +160,11 @@ static void gic_mask_irq(struct irq_data *d)
gic_poke_irq(d, GIC_DIST_ENABLE_CLEAR);
}
+static void gic_eoimode1_mask_irq(struct irq_data *d)
+{
+ gic_mask_irq(d);
+}
+
static void gic_unmask_irq(struct irq_data *d)
{
gic_poke_irq(d, GIC_DIST_ENABLE_SET);
@@ -167,6 +175,11 @@ static void gic_eoi_irq(struct irq_data *d)
writel_relaxed(gic_irq(d), gic_cpu_base(d) + GIC_CPU_EOI);
}
+static void gic_eoimode1_eoi_irq(struct irq_data *d)
+{
+ writel_relaxed(gic_irq(d), gic_cpu_base(d) + GIC_CPU_DEACTIVATE);
+}
+
static int gic_irq_set_irqchip_state(struct irq_data *d,
enum irqchip_irq_state which, bool val)
{
@@ -272,11 +285,15 @@ static void __exception_irq_entry gic_handle_irq(struct pt_regs *regs)
irqnr = irqstat & GICC_IAR_INT_ID_MASK;
if (likely(irqnr > 15 && irqnr < 1021)) {
+ if (static_key_true(&supports_deactivate))
+ writel_relaxed(irqstat, cpu_base + GIC_CPU_EOI);
handle_domain_irq(gic->domain, irqnr, regs);
continue;
}
if (irqnr < 16) {
writel_relaxed(irqstat, cpu_base + GIC_CPU_EOI);
+ if (static_key_true(&supports_deactivate))
+ writel_relaxed(irqstat, cpu_base + GIC_CPU_DEACTIVATE);
#ifdef CONFIG_SMP
handle_IPI(irqnr, regs);
#endif
@@ -329,6 +346,22 @@ static struct irq_chip gic_chip = {
IRQCHIP_MASK_ON_SUSPEND,
};
+static struct irq_chip gic_eoimode1_chip = {
+ .name = "GICv2",
+ .irq_mask = gic_eoimode1_mask_irq,
+ .irq_unmask = gic_unmask_irq,
+ .irq_eoi = gic_eoimode1_eoi_irq,
+ .irq_set_type = gic_set_type,
+#ifdef CONFIG_SMP
+ .irq_set_affinity = gic_set_affinity,
+#endif
+ .irq_get_irqchip_state = gic_irq_get_irqchip_state,
+ .irq_set_irqchip_state = gic_irq_set_irqchip_state,
+ .flags = IRQCHIP_SET_TYPE_MASKED |
+ IRQCHIP_SKIP_SET_WAKE |
+ IRQCHIP_MASK_ON_SUSPEND,
+};
+
void __init gic_cascade_irq(unsigned int gic_nr, unsigned int irq)
{
if (gic_nr >= MAX_GIC_NR)
@@ -360,6 +393,10 @@ static void gic_cpu_if_up(struct gic_chip_data *gic)
{
void __iomem *cpu_base = gic_data_cpu_base(gic);
u32 bypass = 0;
+ u32 mode = 0;
+
+ if (static_key_true(&supports_deactivate))
+ mode = GIC_CPU_CTRL_EOImodeNS;
/*
* Preserve bypass disable bits to be written back later
@@ -367,7 +404,7 @@ static void gic_cpu_if_up(struct gic_chip_data *gic)
bypass = readl(cpu_base + GIC_CPU_CTRL);
bypass &= GICC_DIS_BYPASS_MASK;
- writel_relaxed(bypass | GICC_ENABLE, cpu_base + GIC_CPU_CTRL);
+ writel_relaxed(bypass | mode | GICC_ENABLE, cpu_base + GIC_CPU_CTRL);
}
@@ -803,13 +840,20 @@ void __init gic_init_physaddr(struct device_node *node)
static int gic_irq_domain_map(struct irq_domain *d, unsigned int irq,
irq_hw_number_t hw)
{
+ struct irq_chip *chip = &gic_chip;
+
+ if (static_key_true(&supports_deactivate)) {
+ if (d->host_data == (void *)&gic_data[0])
+ chip = &gic_eoimode1_chip;
+ }
+
if (hw < 32) {
irq_set_percpu_devid(irq);
- irq_domain_set_info(d, irq, hw, &gic_chip, d->host_data,
+ irq_domain_set_info(d, irq, hw, chip, d->host_data,
handle_percpu_devid_irq, NULL, NULL);
set_irq_flags(irq, IRQF_VALID | IRQF_NOAUTOEN);
} else {
- irq_domain_set_info(d, irq, hw, &gic_chip, d->host_data,
+ irq_domain_set_info(d, irq, hw, chip, d->host_data,
handle_fasteoi_irq, NULL, NULL);
set_irq_flags(irq, IRQF_VALID | IRQF_PROBE);
}
@@ -995,6 +1039,8 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start,
register_cpu_notifier(&gic_cpu_notifier);
#endif
set_handle_irq(gic_handle_irq);
+ if (static_key_true(&supports_deactivate))
+ pr_info("GIC: Using split EOI/Deactivate mode\n");
}
gic_dist_init(gic);
@@ -1010,6 +1056,7 @@ gic_of_init(struct device_node *node, struct device_node *parent)
{
void __iomem *cpu_base;
void __iomem *dist_base;
+ struct resource cpu_res;
u32 percpu_offset;
int irq;
@@ -1022,6 +1069,16 @@ gic_of_init(struct device_node *node, struct device_node *parent)
cpu_base = of_iomap(node, 1);
WARN(!cpu_base, "unable to map gic cpu registers\n");
+ of_address_to_resource(node, 1, &cpu_res);
+
+ /*
+ * Disable split EOI/Deactivate if either HYP is not available
+ * or the CPU interface is too small.
+ */
+ if (gic_cnt == 0 && (!is_hyp_mode_available() ||
+ resource_size(&cpu_res) < SZ_8K))
+ static_key_slow_dec(&supports_deactivate);
+
if (of_property_read_u32(node, "cpu-offset", &percpu_offset))
percpu_offset = 0;
@@ -1141,6 +1198,14 @@ gic_v2_acpi_init(struct acpi_table_header *table)
}
/*
+ * Disable split EOI/Deactivate if HYP is not available. ACPI
+ * guarantees that we'll always have a GICv2, so the CPU
+ * interface will always be the right size.
+ */
+ if (!is_hyp_mode_available())
+ static_key_slow_dec(&supports_deactivate);
+
+ /*
* Initialize zero GIC instance (no multi-GIC support). Also, set GIC
* as default IRQ domain to allow for GSI registration and GSI to IRQ
* number translation (see acpi_register_gsi() and acpi_gsi_to_irq()).
diff --git a/include/linux/irqchip/arm-gic.h b/include/linux/irqchip/arm-gic.h
index 65da435..af3d29f 100644
--- a/include/linux/irqchip/arm-gic.h
+++ b/include/linux/irqchip/arm-gic.h
@@ -20,9 +20,13 @@
#define GIC_CPU_ALIAS_BINPOINT 0x1c
#define GIC_CPU_ACTIVEPRIO 0xd0
#define GIC_CPU_IDENT 0xfc
+#define GIC_CPU_DEACTIVATE 0x1000
#define GICC_ENABLE 0x1
#define GICC_INT_PRI_THRESHOLD 0xf0
+
+#define GIC_CPU_CTRL_EOImodeNS (1 << 9)
+
#define GICC_IAR_INT_ID_MASK 0x3ff
#define GICC_INT_SPURIOUS 1023
#define GICC_DIS_BYPASS_MASK 0x1e0
^ permalink raw reply related [flat|nested] 49+ messages in thread* Re: [PATCH v4 3/4] irqchip: GIC: Convert to EOImode == 1
2015-08-26 16:00 ` Marc Zyngier
(?)
@ 2015-09-09 19:23 ` Julien Grall
-1 siblings, 0 replies; 49+ messages in thread
From: Julien Grall @ 2015-09-09 19:23 UTC (permalink / raw)
To: Marc Zyngier, Thomas Gleixner, Jason Cooper,
Pranavkumar Sawargaonkar
Cc: Ian Campbell, kvm, Eric Auger, Stefano Stabellini, linux-kernel,
kvmarm, linux-arm-kernel, Jiang Liu, Christoffer Dall
Hi,
I've been trying the latest linus/master (a794b4f), which include this
patch, as baremetal kernel on X-gene. This is failing on early boot
without much log.
After bisecting the tree, I found the error coming from this patch.
While this patch is valid, it made me remembered that X-Gene (at least
the first version) as an odd GICv2.
The GICC is divided in 2 area of 4K, each one aligned at a 64KB address.
This means that, the address of GICC_DIR won't be 0x1000 but 0x10000.
We had the same issue on Xen when we did the first port of X-gene [1].
Although, we choose to add a quirk in Xen for this platform in order to
map contiguously in the virtual memory the 2 part of GICC.
Note that, back then, Ian suggested to extend the bindings to support a
such platform [2]. AFAICT, there was no follow-up on it.
Regards,
[1] http://lists.xen.org/archives/html/xen-devel/2013-11/msg03526.html
[2] http://www.spinics.net/lists/devicetree/msg10478.html
On 26/08/15 17:00, Marc Zyngier wrote:
> So far, GICv2 has been used with EOImode == 0. The effect of this
> mode is to perform the priority drop and the deactivation of the
> interrupt at the same time.
>
> While this works perfectly for Linux (we only have a single priority),
> it causes issues when an interrupt is forwarded to a guest, and when
> we want the guest to perform the EOI itself.
>
> For this case, the GIC architecture provides EOImode == 1, where:
> - A write to the EOI register drops the priority of the interrupt
> and leaves it active. Other interrupts at the same priority level
> can now be taken, but the active interrupt cannot be taken again
> - A write to the DIR marks the interrupt as inactive, meaning it can
> now be taken again.
>
> We only enable this feature when booted in HYP mode and that
> the device-tree reported a suitable CPU interface. Observable behaviour
> should remain unchanged.
>
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
> drivers/irqchip/irq-gic.c | 69 +++++++++++++++++++++++++++++++++++++++--
> include/linux/irqchip/arm-gic.h | 4 +++
> 2 files changed, 70 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
> index 4dd8826..9215897 100644
> --- a/drivers/irqchip/irq-gic.c
> +++ b/drivers/irqchip/irq-gic.c
> @@ -46,6 +46,7 @@
> #include <asm/irq.h>
> #include <asm/exception.h>
> #include <asm/smp_plat.h>
> +#include <asm/virt.h>
>
> #include "irq-gic-common.h"
> #include "irqchip.h"
> @@ -82,6 +83,8 @@ static DEFINE_RAW_SPINLOCK(irq_controller_lock);
> #define NR_GIC_CPU_IF 8
> static u8 gic_cpu_map[NR_GIC_CPU_IF] __read_mostly;
>
> +static struct static_key supports_deactivate = STATIC_KEY_INIT_TRUE;
> +
> #ifndef MAX_GIC_NR
> #define MAX_GIC_NR 1
> #endif
> @@ -157,6 +160,11 @@ static void gic_mask_irq(struct irq_data *d)
> gic_poke_irq(d, GIC_DIST_ENABLE_CLEAR);
> }
>
> +static void gic_eoimode1_mask_irq(struct irq_data *d)
> +{
> + gic_mask_irq(d);
> +}
> +
> static void gic_unmask_irq(struct irq_data *d)
> {
> gic_poke_irq(d, GIC_DIST_ENABLE_SET);
> @@ -167,6 +175,11 @@ static void gic_eoi_irq(struct irq_data *d)
> writel_relaxed(gic_irq(d), gic_cpu_base(d) + GIC_CPU_EOI);
> }
>
> +static void gic_eoimode1_eoi_irq(struct irq_data *d)
> +{
> + writel_relaxed(gic_irq(d), gic_cpu_base(d) + GIC_CPU_DEACTIVATE);
> +}
> +
> static int gic_irq_set_irqchip_state(struct irq_data *d,
> enum irqchip_irq_state which, bool val)
> {
> @@ -272,11 +285,15 @@ static void __exception_irq_entry gic_handle_irq(struct pt_regs *regs)
> irqnr = irqstat & GICC_IAR_INT_ID_MASK;
>
> if (likely(irqnr > 15 && irqnr < 1021)) {
> + if (static_key_true(&supports_deactivate))
> + writel_relaxed(irqstat, cpu_base + GIC_CPU_EOI);
> handle_domain_irq(gic->domain, irqnr, regs);
> continue;
> }
> if (irqnr < 16) {
> writel_relaxed(irqstat, cpu_base + GIC_CPU_EOI);
> + if (static_key_true(&supports_deactivate))
> + writel_relaxed(irqstat, cpu_base + GIC_CPU_DEACTIVATE);
> #ifdef CONFIG_SMP
> handle_IPI(irqnr, regs);
> #endif
> @@ -327,6 +344,20 @@ static struct irq_chip gic_chip = {
> .flags = IRQCHIP_SET_TYPE_MASKED,
> };
>
> +static struct irq_chip gic_eoimode1_chip = {
> + .name = "GICv2",
> + .irq_mask = gic_eoimode1_mask_irq,
> + .irq_unmask = gic_unmask_irq,
> + .irq_eoi = gic_eoimode1_eoi_irq,
> + .irq_set_type = gic_set_type,
> +#ifdef CONFIG_SMP
> + .irq_set_affinity = gic_set_affinity,
> +#endif
> + .irq_get_irqchip_state = gic_irq_get_irqchip_state,
> + .irq_set_irqchip_state = gic_irq_set_irqchip_state,
> + .flags = IRQCHIP_SET_TYPE_MASKED,
> +};
> +
> void __init gic_cascade_irq(unsigned int gic_nr, unsigned int irq)
> {
> if (gic_nr >= MAX_GIC_NR)
> @@ -359,6 +390,10 @@ static void gic_cpu_if_up(void)
> {
> void __iomem *cpu_base = gic_data_cpu_base(&gic_data[0]);
> u32 bypass = 0;
> + u32 mode = 0;
> +
> + if (static_key_true(&supports_deactivate))
> + mode = GIC_CPU_CTRL_EOImodeNS;
>
> /*
> * Preserve bypass disable bits to be written back later
> @@ -366,7 +401,7 @@ static void gic_cpu_if_up(void)
> bypass = readl(cpu_base + GIC_CPU_CTRL);
> bypass &= GICC_DIS_BYPASS_MASK;
>
> - writel_relaxed(bypass | GICC_ENABLE, cpu_base + GIC_CPU_CTRL);
> + writel_relaxed(bypass | mode | GICC_ENABLE, cpu_base + GIC_CPU_CTRL);
> }
>
>
> @@ -789,13 +824,20 @@ void __init gic_init_physaddr(struct device_node *node)
> static int gic_irq_domain_map(struct irq_domain *d, unsigned int irq,
> irq_hw_number_t hw)
> {
> + struct irq_chip *chip = &gic_chip;
> +
> + if (static_key_true(&supports_deactivate)) {
> + if (d->host_data == (void *)&gic_data[0])
> + chip = &gic_eoimode1_chip;
> + }
> +
> if (hw < 32) {
> irq_set_percpu_devid(irq);
> - irq_domain_set_info(d, irq, hw, &gic_chip, d->host_data,
> + irq_domain_set_info(d, irq, hw, chip, d->host_data,
> handle_percpu_devid_irq, NULL, NULL);
> set_irq_flags(irq, IRQF_VALID | IRQF_NOAUTOEN);
> } else {
> - irq_domain_set_info(d, irq, hw, &gic_chip, d->host_data,
> + irq_domain_set_info(d, irq, hw, chip, d->host_data,
> handle_fasteoi_irq, NULL, NULL);
> set_irq_flags(irq, IRQF_VALID | IRQF_PROBE);
> }
> @@ -986,6 +1028,8 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start,
> register_cpu_notifier(&gic_cpu_notifier);
> #endif
> set_handle_irq(gic_handle_irq);
> + if (static_key_true(&supports_deactivate))
> + pr_info("GIC: Using split EOI/Deactivate mode\n");
> }
>
> gic_dist_init(gic);
> @@ -1001,6 +1045,7 @@ gic_of_init(struct device_node *node, struct device_node *parent)
> {
> void __iomem *cpu_base;
> void __iomem *dist_base;
> + struct resource cpu_res;
> u32 percpu_offset;
> int irq;
>
> @@ -1013,6 +1058,16 @@ gic_of_init(struct device_node *node, struct device_node *parent)
> cpu_base = of_iomap(node, 1);
> WARN(!cpu_base, "unable to map gic cpu registers\n");
>
> + of_address_to_resource(node, 1, &cpu_res);
> +
> + /*
> + * Disable split EOI/Deactivate if either HYP is not available
> + * or the CPU interface is too small.
> + */
> + if (gic_cnt == 0 && (!is_hyp_mode_available() ||
> + resource_size(&cpu_res) < SZ_8K))
> + static_key_slow_dec(&supports_deactivate);
> +
> if (of_property_read_u32(node, "cpu-offset", &percpu_offset))
> percpu_offset = 0;
>
> @@ -1132,6 +1187,14 @@ gic_v2_acpi_init(struct acpi_table_header *table)
> }
>
> /*
> + * Disable split EOI/Deactivate if HYP is not available. ACPI
> + * guarantees that we'll always have a GICv2, so the CPU
> + * interface will always be the right size.
> + */
> + if (!is_hyp_mode_available())
> + static_key_slow_dec(&supports_deactivate);
> +
> + /*
> * Initialize zero GIC instance (no multi-GIC support). Also, set GIC
> * as default IRQ domain to allow for GSI registration and GSI to IRQ
> * number translation (see acpi_register_gsi() and acpi_gsi_to_irq()).
> diff --git a/include/linux/irqchip/arm-gic.h b/include/linux/irqchip/arm-gic.h
> index 9de976b..b1533c0 100644
> --- a/include/linux/irqchip/arm-gic.h
> +++ b/include/linux/irqchip/arm-gic.h
> @@ -20,9 +20,13 @@
> #define GIC_CPU_ALIAS_BINPOINT 0x1c
> #define GIC_CPU_ACTIVEPRIO 0xd0
> #define GIC_CPU_IDENT 0xfc
> +#define GIC_CPU_DEACTIVATE 0x1000
>
> #define GICC_ENABLE 0x1
> #define GICC_INT_PRI_THRESHOLD 0xf0
> +
> +#define GIC_CPU_CTRL_EOImodeNS (1 << 9)
> +
> #define GICC_IAR_INT_ID_MASK 0x3ff
> #define GICC_INT_SPURIOUS 1023
> #define GICC_DIS_BYPASS_MASK 0x1e0
>
--
Julien Grall
^ permalink raw reply [flat|nested] 49+ messages in thread* Re: [PATCH v4 3/4] irqchip: GIC: Convert to EOImode == 1
@ 2015-09-09 19:23 ` Julien Grall
0 siblings, 0 replies; 49+ messages in thread
From: Julien Grall @ 2015-09-09 19:23 UTC (permalink / raw)
To: Marc Zyngier, Thomas Gleixner, Jason Cooper,
Pranavkumar Sawargaonkar
Cc: kvm, Eric Auger, linux-kernel, Jiang Liu, Christoffer Dall,
kvmarm, linux-arm-kernel, Stefano Stabellini, Ian Campbell
Hi,
I've been trying the latest linus/master (a794b4f), which include this
patch, as baremetal kernel on X-gene. This is failing on early boot
without much log.
After bisecting the tree, I found the error coming from this patch.
While this patch is valid, it made me remembered that X-Gene (at least
the first version) as an odd GICv2.
The GICC is divided in 2 area of 4K, each one aligned at a 64KB address.
This means that, the address of GICC_DIR won't be 0x1000 but 0x10000.
We had the same issue on Xen when we did the first port of X-gene [1].
Although, we choose to add a quirk in Xen for this platform in order to
map contiguously in the virtual memory the 2 part of GICC.
Note that, back then, Ian suggested to extend the bindings to support a
such platform [2]. AFAICT, there was no follow-up on it.
Regards,
[1] http://lists.xen.org/archives/html/xen-devel/2013-11/msg03526.html
[2] http://www.spinics.net/lists/devicetree/msg10478.html
On 26/08/15 17:00, Marc Zyngier wrote:
> So far, GICv2 has been used with EOImode == 0. The effect of this
> mode is to perform the priority drop and the deactivation of the
> interrupt at the same time.
>
> While this works perfectly for Linux (we only have a single priority),
> it causes issues when an interrupt is forwarded to a guest, and when
> we want the guest to perform the EOI itself.
>
> For this case, the GIC architecture provides EOImode == 1, where:
> - A write to the EOI register drops the priority of the interrupt
> and leaves it active. Other interrupts at the same priority level
> can now be taken, but the active interrupt cannot be taken again
> - A write to the DIR marks the interrupt as inactive, meaning it can
> now be taken again.
>
> We only enable this feature when booted in HYP mode and that
> the device-tree reported a suitable CPU interface. Observable behaviour
> should remain unchanged.
>
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
> drivers/irqchip/irq-gic.c | 69 +++++++++++++++++++++++++++++++++++++++--
> include/linux/irqchip/arm-gic.h | 4 +++
> 2 files changed, 70 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
> index 4dd8826..9215897 100644
> --- a/drivers/irqchip/irq-gic.c
> +++ b/drivers/irqchip/irq-gic.c
> @@ -46,6 +46,7 @@
> #include <asm/irq.h>
> #include <asm/exception.h>
> #include <asm/smp_plat.h>
> +#include <asm/virt.h>
>
> #include "irq-gic-common.h"
> #include "irqchip.h"
> @@ -82,6 +83,8 @@ static DEFINE_RAW_SPINLOCK(irq_controller_lock);
> #define NR_GIC_CPU_IF 8
> static u8 gic_cpu_map[NR_GIC_CPU_IF] __read_mostly;
>
> +static struct static_key supports_deactivate = STATIC_KEY_INIT_TRUE;
> +
> #ifndef MAX_GIC_NR
> #define MAX_GIC_NR 1
> #endif
> @@ -157,6 +160,11 @@ static void gic_mask_irq(struct irq_data *d)
> gic_poke_irq(d, GIC_DIST_ENABLE_CLEAR);
> }
>
> +static void gic_eoimode1_mask_irq(struct irq_data *d)
> +{
> + gic_mask_irq(d);
> +}
> +
> static void gic_unmask_irq(struct irq_data *d)
> {
> gic_poke_irq(d, GIC_DIST_ENABLE_SET);
> @@ -167,6 +175,11 @@ static void gic_eoi_irq(struct irq_data *d)
> writel_relaxed(gic_irq(d), gic_cpu_base(d) + GIC_CPU_EOI);
> }
>
> +static void gic_eoimode1_eoi_irq(struct irq_data *d)
> +{
> + writel_relaxed(gic_irq(d), gic_cpu_base(d) + GIC_CPU_DEACTIVATE);
> +}
> +
> static int gic_irq_set_irqchip_state(struct irq_data *d,
> enum irqchip_irq_state which, bool val)
> {
> @@ -272,11 +285,15 @@ static void __exception_irq_entry gic_handle_irq(struct pt_regs *regs)
> irqnr = irqstat & GICC_IAR_INT_ID_MASK;
>
> if (likely(irqnr > 15 && irqnr < 1021)) {
> + if (static_key_true(&supports_deactivate))
> + writel_relaxed(irqstat, cpu_base + GIC_CPU_EOI);
> handle_domain_irq(gic->domain, irqnr, regs);
> continue;
> }
> if (irqnr < 16) {
> writel_relaxed(irqstat, cpu_base + GIC_CPU_EOI);
> + if (static_key_true(&supports_deactivate))
> + writel_relaxed(irqstat, cpu_base + GIC_CPU_DEACTIVATE);
> #ifdef CONFIG_SMP
> handle_IPI(irqnr, regs);
> #endif
> @@ -327,6 +344,20 @@ static struct irq_chip gic_chip = {
> .flags = IRQCHIP_SET_TYPE_MASKED,
> };
>
> +static struct irq_chip gic_eoimode1_chip = {
> + .name = "GICv2",
> + .irq_mask = gic_eoimode1_mask_irq,
> + .irq_unmask = gic_unmask_irq,
> + .irq_eoi = gic_eoimode1_eoi_irq,
> + .irq_set_type = gic_set_type,
> +#ifdef CONFIG_SMP
> + .irq_set_affinity = gic_set_affinity,
> +#endif
> + .irq_get_irqchip_state = gic_irq_get_irqchip_state,
> + .irq_set_irqchip_state = gic_irq_set_irqchip_state,
> + .flags = IRQCHIP_SET_TYPE_MASKED,
> +};
> +
> void __init gic_cascade_irq(unsigned int gic_nr, unsigned int irq)
> {
> if (gic_nr >= MAX_GIC_NR)
> @@ -359,6 +390,10 @@ static void gic_cpu_if_up(void)
> {
> void __iomem *cpu_base = gic_data_cpu_base(&gic_data[0]);
> u32 bypass = 0;
> + u32 mode = 0;
> +
> + if (static_key_true(&supports_deactivate))
> + mode = GIC_CPU_CTRL_EOImodeNS;
>
> /*
> * Preserve bypass disable bits to be written back later
> @@ -366,7 +401,7 @@ static void gic_cpu_if_up(void)
> bypass = readl(cpu_base + GIC_CPU_CTRL);
> bypass &= GICC_DIS_BYPASS_MASK;
>
> - writel_relaxed(bypass | GICC_ENABLE, cpu_base + GIC_CPU_CTRL);
> + writel_relaxed(bypass | mode | GICC_ENABLE, cpu_base + GIC_CPU_CTRL);
> }
>
>
> @@ -789,13 +824,20 @@ void __init gic_init_physaddr(struct device_node *node)
> static int gic_irq_domain_map(struct irq_domain *d, unsigned int irq,
> irq_hw_number_t hw)
> {
> + struct irq_chip *chip = &gic_chip;
> +
> + if (static_key_true(&supports_deactivate)) {
> + if (d->host_data == (void *)&gic_data[0])
> + chip = &gic_eoimode1_chip;
> + }
> +
> if (hw < 32) {
> irq_set_percpu_devid(irq);
> - irq_domain_set_info(d, irq, hw, &gic_chip, d->host_data,
> + irq_domain_set_info(d, irq, hw, chip, d->host_data,
> handle_percpu_devid_irq, NULL, NULL);
> set_irq_flags(irq, IRQF_VALID | IRQF_NOAUTOEN);
> } else {
> - irq_domain_set_info(d, irq, hw, &gic_chip, d->host_data,
> + irq_domain_set_info(d, irq, hw, chip, d->host_data,
> handle_fasteoi_irq, NULL, NULL);
> set_irq_flags(irq, IRQF_VALID | IRQF_PROBE);
> }
> @@ -986,6 +1028,8 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start,
> register_cpu_notifier(&gic_cpu_notifier);
> #endif
> set_handle_irq(gic_handle_irq);
> + if (static_key_true(&supports_deactivate))
> + pr_info("GIC: Using split EOI/Deactivate mode\n");
> }
>
> gic_dist_init(gic);
> @@ -1001,6 +1045,7 @@ gic_of_init(struct device_node *node, struct device_node *parent)
> {
> void __iomem *cpu_base;
> void __iomem *dist_base;
> + struct resource cpu_res;
> u32 percpu_offset;
> int irq;
>
> @@ -1013,6 +1058,16 @@ gic_of_init(struct device_node *node, struct device_node *parent)
> cpu_base = of_iomap(node, 1);
> WARN(!cpu_base, "unable to map gic cpu registers\n");
>
> + of_address_to_resource(node, 1, &cpu_res);
> +
> + /*
> + * Disable split EOI/Deactivate if either HYP is not available
> + * or the CPU interface is too small.
> + */
> + if (gic_cnt == 0 && (!is_hyp_mode_available() ||
> + resource_size(&cpu_res) < SZ_8K))
> + static_key_slow_dec(&supports_deactivate);
> +
> if (of_property_read_u32(node, "cpu-offset", &percpu_offset))
> percpu_offset = 0;
>
> @@ -1132,6 +1187,14 @@ gic_v2_acpi_init(struct acpi_table_header *table)
> }
>
> /*
> + * Disable split EOI/Deactivate if HYP is not available. ACPI
> + * guarantees that we'll always have a GICv2, so the CPU
> + * interface will always be the right size.
> + */
> + if (!is_hyp_mode_available())
> + static_key_slow_dec(&supports_deactivate);
> +
> + /*
> * Initialize zero GIC instance (no multi-GIC support). Also, set GIC
> * as default IRQ domain to allow for GSI registration and GSI to IRQ
> * number translation (see acpi_register_gsi() and acpi_gsi_to_irq()).
> diff --git a/include/linux/irqchip/arm-gic.h b/include/linux/irqchip/arm-gic.h
> index 9de976b..b1533c0 100644
> --- a/include/linux/irqchip/arm-gic.h
> +++ b/include/linux/irqchip/arm-gic.h
> @@ -20,9 +20,13 @@
> #define GIC_CPU_ALIAS_BINPOINT 0x1c
> #define GIC_CPU_ACTIVEPRIO 0xd0
> #define GIC_CPU_IDENT 0xfc
> +#define GIC_CPU_DEACTIVATE 0x1000
>
> #define GICC_ENABLE 0x1
> #define GICC_INT_PRI_THRESHOLD 0xf0
> +
> +#define GIC_CPU_CTRL_EOImodeNS (1 << 9)
> +
> #define GICC_IAR_INT_ID_MASK 0x3ff
> #define GICC_INT_SPURIOUS 1023
> #define GICC_DIS_BYPASS_MASK 0x1e0
>
--
Julien Grall
^ permalink raw reply [flat|nested] 49+ messages in thread* [PATCH v4 3/4] irqchip: GIC: Convert to EOImode == 1
@ 2015-09-09 19:23 ` Julien Grall
0 siblings, 0 replies; 49+ messages in thread
From: Julien Grall @ 2015-09-09 19:23 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
I've been trying the latest linus/master (a794b4f), which include this
patch, as baremetal kernel on X-gene. This is failing on early boot
without much log.
After bisecting the tree, I found the error coming from this patch.
While this patch is valid, it made me remembered that X-Gene (at least
the first version) as an odd GICv2.
The GICC is divided in 2 area of 4K, each one aligned at a 64KB address.
This means that, the address of GICC_DIR won't be 0x1000 but 0x10000.
We had the same issue on Xen when we did the first port of X-gene [1].
Although, we choose to add a quirk in Xen for this platform in order to
map contiguously in the virtual memory the 2 part of GICC.
Note that, back then, Ian suggested to extend the bindings to support a
such platform [2]. AFAICT, there was no follow-up on it.
Regards,
[1] http://lists.xen.org/archives/html/xen-devel/2013-11/msg03526.html
[2] http://www.spinics.net/lists/devicetree/msg10478.html
On 26/08/15 17:00, Marc Zyngier wrote:
> So far, GICv2 has been used with EOImode == 0. The effect of this
> mode is to perform the priority drop and the deactivation of the
> interrupt at the same time.
>
> While this works perfectly for Linux (we only have a single priority),
> it causes issues when an interrupt is forwarded to a guest, and when
> we want the guest to perform the EOI itself.
>
> For this case, the GIC architecture provides EOImode == 1, where:
> - A write to the EOI register drops the priority of the interrupt
> and leaves it active. Other interrupts at the same priority level
> can now be taken, but the active interrupt cannot be taken again
> - A write to the DIR marks the interrupt as inactive, meaning it can
> now be taken again.
>
> We only enable this feature when booted in HYP mode and that
> the device-tree reported a suitable CPU interface. Observable behaviour
> should remain unchanged.
>
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
> drivers/irqchip/irq-gic.c | 69 +++++++++++++++++++++++++++++++++++++++--
> include/linux/irqchip/arm-gic.h | 4 +++
> 2 files changed, 70 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
> index 4dd8826..9215897 100644
> --- a/drivers/irqchip/irq-gic.c
> +++ b/drivers/irqchip/irq-gic.c
> @@ -46,6 +46,7 @@
> #include <asm/irq.h>
> #include <asm/exception.h>
> #include <asm/smp_plat.h>
> +#include <asm/virt.h>
>
> #include "irq-gic-common.h"
> #include "irqchip.h"
> @@ -82,6 +83,8 @@ static DEFINE_RAW_SPINLOCK(irq_controller_lock);
> #define NR_GIC_CPU_IF 8
> static u8 gic_cpu_map[NR_GIC_CPU_IF] __read_mostly;
>
> +static struct static_key supports_deactivate = STATIC_KEY_INIT_TRUE;
> +
> #ifndef MAX_GIC_NR
> #define MAX_GIC_NR 1
> #endif
> @@ -157,6 +160,11 @@ static void gic_mask_irq(struct irq_data *d)
> gic_poke_irq(d, GIC_DIST_ENABLE_CLEAR);
> }
>
> +static void gic_eoimode1_mask_irq(struct irq_data *d)
> +{
> + gic_mask_irq(d);
> +}
> +
> static void gic_unmask_irq(struct irq_data *d)
> {
> gic_poke_irq(d, GIC_DIST_ENABLE_SET);
> @@ -167,6 +175,11 @@ static void gic_eoi_irq(struct irq_data *d)
> writel_relaxed(gic_irq(d), gic_cpu_base(d) + GIC_CPU_EOI);
> }
>
> +static void gic_eoimode1_eoi_irq(struct irq_data *d)
> +{
> + writel_relaxed(gic_irq(d), gic_cpu_base(d) + GIC_CPU_DEACTIVATE);
> +}
> +
> static int gic_irq_set_irqchip_state(struct irq_data *d,
> enum irqchip_irq_state which, bool val)
> {
> @@ -272,11 +285,15 @@ static void __exception_irq_entry gic_handle_irq(struct pt_regs *regs)
> irqnr = irqstat & GICC_IAR_INT_ID_MASK;
>
> if (likely(irqnr > 15 && irqnr < 1021)) {
> + if (static_key_true(&supports_deactivate))
> + writel_relaxed(irqstat, cpu_base + GIC_CPU_EOI);
> handle_domain_irq(gic->domain, irqnr, regs);
> continue;
> }
> if (irqnr < 16) {
> writel_relaxed(irqstat, cpu_base + GIC_CPU_EOI);
> + if (static_key_true(&supports_deactivate))
> + writel_relaxed(irqstat, cpu_base + GIC_CPU_DEACTIVATE);
> #ifdef CONFIG_SMP
> handle_IPI(irqnr, regs);
> #endif
> @@ -327,6 +344,20 @@ static struct irq_chip gic_chip = {
> .flags = IRQCHIP_SET_TYPE_MASKED,
> };
>
> +static struct irq_chip gic_eoimode1_chip = {
> + .name = "GICv2",
> + .irq_mask = gic_eoimode1_mask_irq,
> + .irq_unmask = gic_unmask_irq,
> + .irq_eoi = gic_eoimode1_eoi_irq,
> + .irq_set_type = gic_set_type,
> +#ifdef CONFIG_SMP
> + .irq_set_affinity = gic_set_affinity,
> +#endif
> + .irq_get_irqchip_state = gic_irq_get_irqchip_state,
> + .irq_set_irqchip_state = gic_irq_set_irqchip_state,
> + .flags = IRQCHIP_SET_TYPE_MASKED,
> +};
> +
> void __init gic_cascade_irq(unsigned int gic_nr, unsigned int irq)
> {
> if (gic_nr >= MAX_GIC_NR)
> @@ -359,6 +390,10 @@ static void gic_cpu_if_up(void)
> {
> void __iomem *cpu_base = gic_data_cpu_base(&gic_data[0]);
> u32 bypass = 0;
> + u32 mode = 0;
> +
> + if (static_key_true(&supports_deactivate))
> + mode = GIC_CPU_CTRL_EOImodeNS;
>
> /*
> * Preserve bypass disable bits to be written back later
> @@ -366,7 +401,7 @@ static void gic_cpu_if_up(void)
> bypass = readl(cpu_base + GIC_CPU_CTRL);
> bypass &= GICC_DIS_BYPASS_MASK;
>
> - writel_relaxed(bypass | GICC_ENABLE, cpu_base + GIC_CPU_CTRL);
> + writel_relaxed(bypass | mode | GICC_ENABLE, cpu_base + GIC_CPU_CTRL);
> }
>
>
> @@ -789,13 +824,20 @@ void __init gic_init_physaddr(struct device_node *node)
> static int gic_irq_domain_map(struct irq_domain *d, unsigned int irq,
> irq_hw_number_t hw)
> {
> + struct irq_chip *chip = &gic_chip;
> +
> + if (static_key_true(&supports_deactivate)) {
> + if (d->host_data == (void *)&gic_data[0])
> + chip = &gic_eoimode1_chip;
> + }
> +
> if (hw < 32) {
> irq_set_percpu_devid(irq);
> - irq_domain_set_info(d, irq, hw, &gic_chip, d->host_data,
> + irq_domain_set_info(d, irq, hw, chip, d->host_data,
> handle_percpu_devid_irq, NULL, NULL);
> set_irq_flags(irq, IRQF_VALID | IRQF_NOAUTOEN);
> } else {
> - irq_domain_set_info(d, irq, hw, &gic_chip, d->host_data,
> + irq_domain_set_info(d, irq, hw, chip, d->host_data,
> handle_fasteoi_irq, NULL, NULL);
> set_irq_flags(irq, IRQF_VALID | IRQF_PROBE);
> }
> @@ -986,6 +1028,8 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start,
> register_cpu_notifier(&gic_cpu_notifier);
> #endif
> set_handle_irq(gic_handle_irq);
> + if (static_key_true(&supports_deactivate))
> + pr_info("GIC: Using split EOI/Deactivate mode\n");
> }
>
> gic_dist_init(gic);
> @@ -1001,6 +1045,7 @@ gic_of_init(struct device_node *node, struct device_node *parent)
> {
> void __iomem *cpu_base;
> void __iomem *dist_base;
> + struct resource cpu_res;
> u32 percpu_offset;
> int irq;
>
> @@ -1013,6 +1058,16 @@ gic_of_init(struct device_node *node, struct device_node *parent)
> cpu_base = of_iomap(node, 1);
> WARN(!cpu_base, "unable to map gic cpu registers\n");
>
> + of_address_to_resource(node, 1, &cpu_res);
> +
> + /*
> + * Disable split EOI/Deactivate if either HYP is not available
> + * or the CPU interface is too small.
> + */
> + if (gic_cnt == 0 && (!is_hyp_mode_available() ||
> + resource_size(&cpu_res) < SZ_8K))
> + static_key_slow_dec(&supports_deactivate);
> +
> if (of_property_read_u32(node, "cpu-offset", &percpu_offset))
> percpu_offset = 0;
>
> @@ -1132,6 +1187,14 @@ gic_v2_acpi_init(struct acpi_table_header *table)
> }
>
> /*
> + * Disable split EOI/Deactivate if HYP is not available. ACPI
> + * guarantees that we'll always have a GICv2, so the CPU
> + * interface will always be the right size.
> + */
> + if (!is_hyp_mode_available())
> + static_key_slow_dec(&supports_deactivate);
> +
> + /*
> * Initialize zero GIC instance (no multi-GIC support). Also, set GIC
> * as default IRQ domain to allow for GSI registration and GSI to IRQ
> * number translation (see acpi_register_gsi() and acpi_gsi_to_irq()).
> diff --git a/include/linux/irqchip/arm-gic.h b/include/linux/irqchip/arm-gic.h
> index 9de976b..b1533c0 100644
> --- a/include/linux/irqchip/arm-gic.h
> +++ b/include/linux/irqchip/arm-gic.h
> @@ -20,9 +20,13 @@
> #define GIC_CPU_ALIAS_BINPOINT 0x1c
> #define GIC_CPU_ACTIVEPRIO 0xd0
> #define GIC_CPU_IDENT 0xfc
> +#define GIC_CPU_DEACTIVATE 0x1000
>
> #define GICC_ENABLE 0x1
> #define GICC_INT_PRI_THRESHOLD 0xf0
> +
> +#define GIC_CPU_CTRL_EOImodeNS (1 << 9)
> +
> #define GICC_IAR_INT_ID_MASK 0x3ff
> #define GICC_INT_SPURIOUS 1023
> #define GICC_DIS_BYPASS_MASK 0x1e0
>
--
Julien Grall
^ permalink raw reply [flat|nested] 49+ messages in thread* Re: [PATCH v4 3/4] irqchip: GIC: Convert to EOImode == 1
2015-09-09 19:23 ` Julien Grall
@ 2015-09-10 9:54 ` Marc Zyngier
-1 siblings, 0 replies; 49+ messages in thread
From: Marc Zyngier @ 2015-09-10 9:54 UTC (permalink / raw)
To: Julien Grall, Thomas Gleixner, Jason Cooper,
Pranavkumar Sawargaonkar
Cc: kvm, Eric Auger, linux-kernel, Jiang Liu, Christoffer Dall,
kvmarm, linux-arm-kernel, Stefano Stabellini, Ian Campbell
Hi Julian,
On 09/09/15 20:23, Julien Grall wrote:
> Hi,
>
> I've been trying the latest linus/master (a794b4f), which include this
> patch, as baremetal kernel on X-gene. This is failing on early boot
> without much log.
>
> After bisecting the tree, I found the error coming from this patch.
> While this patch is valid, it made me remembered that X-Gene (at least
> the first version) as an odd GICv2.
>
> The GICC is divided in 2 area of 4K, each one aligned at a 64KB address.
> This means that, the address of GICC_DIR won't be 0x1000 but 0x10000.
Not really. I already mentioned that one a while ago:
http://lists.infradead.org/pipermail/linux-arm-kernel/2015-March/332249.html
The first page of GIC is aliased over the first 64kB, and the second
page aliased over the second 64kB. So you get a consistent mapping if
you use (base + 0xF000) to address GICC. Also, the DT that's in
mainline is showing a 4kB CPU interface, which doesn't enable
EOImode==1. You must be using a firmware that's newer than mine, since
I'm perfectly able to boot my Mustang with these patches.
> We had the same issue on Xen when we did the first port of X-gene [1].
> Although, we choose to add a quirk in Xen for this platform in order to
> map contiguously in the virtual memory the 2 part of GICC.
>
> Note that, back then, Ian suggested to extend the bindings to support a
> such platform [2]. AFAICT, there was no follow-up on it.
The main problem here is not to update the binding, but the fact that
you *cannot* update the DT on x-gene (the firmware will replace your
GIC node with what it thinks it is), and the APM guys can't be bothered
to fix their stuff.
In the meantime, can you give the following patch a shot? My Mustang is
wired to a 4kB CPU interface, so I'll need your help to test it.
Thanks,
M.
>From f0f086a4462198a5a2ac840901d9b8fd23b25134 Mon Sep 17 00:00:00 2001
From: Marc Zyngier <marc.zyngier@arm.com>
Date: Thu, 10 Sep 2015 10:23:45 +0100
Subject: [PATCH] irqchip/GIC: Add workaround for aliased GIC400
The GICv2 architecture mandates that the two 4kB GIC regions are
contiguous, and on two separate physical pages. This doesn't work
very well when PAGE_SIZE is 64kB.
A relatively common hack to work around this is to alias each 4kB
region over its own 64kB page. Of course in this case, the base
address you want to use is not really the begining of the region,
but base + 60kB (so that you get a contiguous 8kB region over two
distinct pages).
Normally, this would be describe in DT with a new property, but
some HW is already out there, and the firmware makes sure that
it will override whatever you put in the GIC node. Duh. And of course,
said firmware source code is not available, despite being based
on u-boot.
The workaround is to detect the case where the CPU interface size
is set to 128kB, and verify the aliasing by checking that the ID
register for GIC400 (which is the only GIC wired this way so far)
is the same at base and base + 0xF000. In this case, we update
the GIC base address and let it roll.
And if you feel slightly sick by looking at this, rest assured that
I do too...
Reported-by: Julien Grall <julien.grall@citrix.com>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
drivers/irqchip/irq-gic.c | 45 ++++++++++++++++++++++++++++++++++++++++-----
1 file changed, 40 insertions(+), 5 deletions(-)
diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index e6b7ed5..b62f2b2 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -1119,12 +1119,50 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start,
#ifdef CONFIG_OF
static int gic_cnt __initdata;
+static bool gic_check_eoimode(struct device_node *node, void __iomem **base)
+{
+ struct resource cpuif_res;
+
+ of_address_to_resource(node, 1, &cpuif_res);
+
+ if (!is_hyp_mode_available())
+ return false;
+ if (resource_size(&cpuif_res) < SZ_8K)
+ return false;
+ if (resource_size(&cpuif_res) == SZ_128K) {
+ u32 val;
+
+ /*
+ * Verify that we have a GIC400 aliased over the first
+ * 64kB by checking the GICC_IIDR register.
+ */
+ val = readl_relaxed(*base + GIC_CPU_IDENT);
+ if (val != 0x0202043B)
+ return false;
+
+ val = readl_relaxed(*base + GIC_CPU_IDENT + 0xF000);
+ if (val != 0x0202043B)
+ return false;
+
+ /*
+ * Move the base up by 60kB, so that we have a 8kB
+ * contiguous region, which allows us to use GICC_DIR
+ * at its normal offset.
+ */
+ *base += 0xF000;
+ cpuif_res.start += 0xF000;
+ pr_warn("GIC: Adjusting CPU interface base to %pa",
+ &cpuif_res.start);
+ }
+
+ return true;
+}
+
static int __init
gic_of_init(struct device_node *node, struct device_node *parent)
{
void __iomem *cpu_base;
void __iomem *dist_base;
- struct resource cpu_res;
u32 percpu_offset;
int irq;
@@ -1137,14 +1175,11 @@ gic_of_init(struct device_node *node, struct device_node *parent)
cpu_base = of_iomap(node, 1);
WARN(!cpu_base, "unable to map gic cpu registers\n");
- of_address_to_resource(node, 1, &cpu_res);
-
/*
* Disable split EOI/Deactivate if either HYP is not available
* or the CPU interface is too small.
*/
- if (gic_cnt == 0 && (!is_hyp_mode_available() ||
- resource_size(&cpu_res) < SZ_8K))
+ if (gic_cnt == 0 && !gic_check_eoimode(node, &cpu_base))
static_key_slow_dec(&supports_deactivate);
if (of_property_read_u32(node, "cpu-offset", &percpu_offset))
--
2.1.4
--
Jazz is not dead. It just smells funny...
^ permalink raw reply related [flat|nested] 49+ messages in thread* [PATCH v4 3/4] irqchip: GIC: Convert to EOImode == 1
@ 2015-09-10 9:54 ` Marc Zyngier
0 siblings, 0 replies; 49+ messages in thread
From: Marc Zyngier @ 2015-09-10 9:54 UTC (permalink / raw)
To: linux-arm-kernel
Hi Julian,
On 09/09/15 20:23, Julien Grall wrote:
> Hi,
>
> I've been trying the latest linus/master (a794b4f), which include this
> patch, as baremetal kernel on X-gene. This is failing on early boot
> without much log.
>
> After bisecting the tree, I found the error coming from this patch.
> While this patch is valid, it made me remembered that X-Gene (at least
> the first version) as an odd GICv2.
>
> The GICC is divided in 2 area of 4K, each one aligned at a 64KB address.
> This means that, the address of GICC_DIR won't be 0x1000 but 0x10000.
Not really. I already mentioned that one a while ago:
http://lists.infradead.org/pipermail/linux-arm-kernel/2015-March/332249.html
The first page of GIC is aliased over the first 64kB, and the second
page aliased over the second 64kB. So you get a consistent mapping if
you use (base + 0xF000) to address GICC. Also, the DT that's in
mainline is showing a 4kB CPU interface, which doesn't enable
EOImode==1. You must be using a firmware that's newer than mine, since
I'm perfectly able to boot my Mustang with these patches.
> We had the same issue on Xen when we did the first port of X-gene [1].
> Although, we choose to add a quirk in Xen for this platform in order to
> map contiguously in the virtual memory the 2 part of GICC.
>
> Note that, back then, Ian suggested to extend the bindings to support a
> such platform [2]. AFAICT, there was no follow-up on it.
The main problem here is not to update the binding, but the fact that
you *cannot* update the DT on x-gene (the firmware will replace your
GIC node with what it thinks it is), and the APM guys can't be bothered
to fix their stuff.
In the meantime, can you give the following patch a shot? My Mustang is
wired to a 4kB CPU interface, so I'll need your help to test it.
Thanks,
M.
>From f0f086a4462198a5a2ac840901d9b8fd23b25134 Mon Sep 17 00:00:00 2001
From: Marc Zyngier <marc.zyngier@arm.com>
Date: Thu, 10 Sep 2015 10:23:45 +0100
Subject: [PATCH] irqchip/GIC: Add workaround for aliased GIC400
The GICv2 architecture mandates that the two 4kB GIC regions are
contiguous, and on two separate physical pages. This doesn't work
very well when PAGE_SIZE is 64kB.
A relatively common hack to work around this is to alias each 4kB
region over its own 64kB page. Of course in this case, the base
address you want to use is not really the begining of the region,
but base + 60kB (so that you get a contiguous 8kB region over two
distinct pages).
Normally, this would be describe in DT with a new property, but
some HW is already out there, and the firmware makes sure that
it will override whatever you put in the GIC node. Duh. And of course,
said firmware source code is not available, despite being based
on u-boot.
The workaround is to detect the case where the CPU interface size
is set to 128kB, and verify the aliasing by checking that the ID
register for GIC400 (which is the only GIC wired this way so far)
is the same at base and base + 0xF000. In this case, we update
the GIC base address and let it roll.
And if you feel slightly sick by looking at this, rest assured that
I do too...
Reported-by: Julien Grall <julien.grall@citrix.com>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
drivers/irqchip/irq-gic.c | 45 ++++++++++++++++++++++++++++++++++++++++-----
1 file changed, 40 insertions(+), 5 deletions(-)
diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index e6b7ed5..b62f2b2 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -1119,12 +1119,50 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start,
#ifdef CONFIG_OF
static int gic_cnt __initdata;
+static bool gic_check_eoimode(struct device_node *node, void __iomem **base)
+{
+ struct resource cpuif_res;
+
+ of_address_to_resource(node, 1, &cpuif_res);
+
+ if (!is_hyp_mode_available())
+ return false;
+ if (resource_size(&cpuif_res) < SZ_8K)
+ return false;
+ if (resource_size(&cpuif_res) == SZ_128K) {
+ u32 val;
+
+ /*
+ * Verify that we have a GIC400 aliased over the first
+ * 64kB by checking the GICC_IIDR register.
+ */
+ val = readl_relaxed(*base + GIC_CPU_IDENT);
+ if (val != 0x0202043B)
+ return false;
+
+ val = readl_relaxed(*base + GIC_CPU_IDENT + 0xF000);
+ if (val != 0x0202043B)
+ return false;
+
+ /*
+ * Move the base up by 60kB, so that we have a 8kB
+ * contiguous region, which allows us to use GICC_DIR
+ *@its normal offset.
+ */
+ *base += 0xF000;
+ cpuif_res.start += 0xF000;
+ pr_warn("GIC: Adjusting CPU interface base to %pa",
+ &cpuif_res.start);
+ }
+
+ return true;
+}
+
static int __init
gic_of_init(struct device_node *node, struct device_node *parent)
{
void __iomem *cpu_base;
void __iomem *dist_base;
- struct resource cpu_res;
u32 percpu_offset;
int irq;
@@ -1137,14 +1175,11 @@ gic_of_init(struct device_node *node, struct device_node *parent)
cpu_base = of_iomap(node, 1);
WARN(!cpu_base, "unable to map gic cpu registers\n");
- of_address_to_resource(node, 1, &cpu_res);
-
/*
* Disable split EOI/Deactivate if either HYP is not available
* or the CPU interface is too small.
*/
- if (gic_cnt == 0 && (!is_hyp_mode_available() ||
- resource_size(&cpu_res) < SZ_8K))
+ if (gic_cnt == 0 && !gic_check_eoimode(node, &cpu_base))
static_key_slow_dec(&supports_deactivate);
if (of_property_read_u32(node, "cpu-offset", &percpu_offset))
--
2.1.4
--
Jazz is not dead. It just smells funny...
^ permalink raw reply related [flat|nested] 49+ messages in thread* Re: [PATCH v4 3/4] irqchip: GIC: Convert to EOImode == 1
2015-09-10 9:54 ` Marc Zyngier
@ 2015-09-10 10:55 ` Marc Zyngier
-1 siblings, 0 replies; 49+ messages in thread
From: Marc Zyngier @ 2015-09-10 10:55 UTC (permalink / raw)
To: Julien Grall, Thomas Gleixner, Jason Cooper,
Pranavkumar Sawargaonkar
Cc: kvm, Eric Auger, linux-kernel, Jiang Liu, Christoffer Dall,
kvmarm, linux-arm-kernel, Stefano Stabellini, Ian Campbell
On 10/09/15 10:54, Marc Zyngier wrote:
> Hi Julian,
>
> On 09/09/15 20:23, Julien Grall wrote:
>> Hi,
>>
>> I've been trying the latest linus/master (a794b4f), which include this
>> patch, as baremetal kernel on X-gene. This is failing on early boot
>> without much log.
>>
>> After bisecting the tree, I found the error coming from this patch.
>> While this patch is valid, it made me remembered that X-Gene (at least
>> the first version) as an odd GICv2.
>>
>> The GICC is divided in 2 area of 4K, each one aligned at a 64KB address.
>> This means that, the address of GICC_DIR won't be 0x1000 but 0x10000.
>
> Not really. I already mentioned that one a while ago:
>
> http://lists.infradead.org/pipermail/linux-arm-kernel/2015-March/332249.html
>
> The first page of GIC is aliased over the first 64kB, and the second
> page aliased over the second 64kB. So you get a consistent mapping if
> you use (base + 0xF000) to address GICC. Also, the DT that's in
> mainline is showing a 4kB CPU interface, which doesn't enable
> EOImode==1. You must be using a firmware that's newer than mine, since
> I'm perfectly able to boot my Mustang with these patches.
>
>> We had the same issue on Xen when we did the first port of X-gene [1].
>> Although, we choose to add a quirk in Xen for this platform in order to
>> map contiguously in the virtual memory the 2 part of GICC.
>>
>> Note that, back then, Ian suggested to extend the bindings to support a
>> such platform [2]. AFAICT, there was no follow-up on it.
>
> The main problem here is not to update the binding, but the fact that
> you *cannot* update the DT on x-gene (the firmware will replace your
> GIC node with what it thinks it is), and the APM guys can't be bothered
> to fix their stuff.
>
> In the meantime, can you give the following patch a shot? My Mustang is
> wired to a 4kB CPU interface, so I'll need your help to test it.
>
> Thanks,
>
> M.
>
> From f0f086a4462198a5a2ac840901d9b8fd23b25134 Mon Sep 17 00:00:00 2001
> From: Marc Zyngier <marc.zyngier@arm.com>
> Date: Thu, 10 Sep 2015 10:23:45 +0100
> Subject: [PATCH] irqchip/GIC: Add workaround for aliased GIC400
>
> The GICv2 architecture mandates that the two 4kB GIC regions are
> contiguous, and on two separate physical pages. This doesn't work
> very well when PAGE_SIZE is 64kB.
>
> A relatively common hack to work around this is to alias each 4kB
> region over its own 64kB page. Of course in this case, the base
> address you want to use is not really the begining of the region,
> but base + 60kB (so that you get a contiguous 8kB region over two
> distinct pages).
>
> Normally, this would be describe in DT with a new property, but
> some HW is already out there, and the firmware makes sure that
> it will override whatever you put in the GIC node. Duh. And of course,
> said firmware source code is not available, despite being based
> on u-boot.
>
> The workaround is to detect the case where the CPU interface size
> is set to 128kB, and verify the aliasing by checking that the ID
> register for GIC400 (which is the only GIC wired this way so far)
> is the same at base and base + 0xF000. In this case, we update
> the GIC base address and let it roll.
>
> And if you feel slightly sick by looking at this, rest assured that
> I do too...
>
> Reported-by: Julien Grall <julien.grall@citrix.com>
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
> drivers/irqchip/irq-gic.c | 45 ++++++++++++++++++++++++++++++++++++++++-----
> 1 file changed, 40 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
> index e6b7ed5..b62f2b2 100644
> --- a/drivers/irqchip/irq-gic.c
> +++ b/drivers/irqchip/irq-gic.c
> @@ -1119,12 +1119,50 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start,
> #ifdef CONFIG_OF
> static int gic_cnt __initdata;
>
> +static bool gic_check_eoimode(struct device_node *node, void __iomem **base)
> +{
> + struct resource cpuif_res;
> +
> + of_address_to_resource(node, 1, &cpuif_res);
> +
> + if (!is_hyp_mode_available())
> + return false;
> + if (resource_size(&cpuif_res) < SZ_8K)
> + return false;
> + if (resource_size(&cpuif_res) == SZ_128K) {
> + u32 val;
> +
> + /*
> + * Verify that we have a GIC400 aliased over the first
> + * 64kB by checking the GICC_IIDR register.
> + */
> + val = readl_relaxed(*base + GIC_CPU_IDENT);
> + if (val != 0x0202043B)
> + return false;
> +
> + val = readl_relaxed(*base + GIC_CPU_IDENT + 0xF000);
> + if (val != 0x0202043B)
> + return false;
> +
> + /*
> + * Move the base up by 60kB, so that we have a 8kB
> + * contiguous region, which allows us to use GICC_DIR
> + * at its normal offset.
> + */
> + *base += 0xF000;
> + cpuif_res.start += 0xF000;
> + pr_warn("GIC: Adjusting CPU interface base to %pa",
> + &cpuif_res.start);
> + }
> +
> + return true;
> +}
> +
> static int __init
> gic_of_init(struct device_node *node, struct device_node *parent)
> {
> void __iomem *cpu_base;
> void __iomem *dist_base;
> - struct resource cpu_res;
> u32 percpu_offset;
> int irq;
>
> @@ -1137,14 +1175,11 @@ gic_of_init(struct device_node *node, struct device_node *parent)
> cpu_base = of_iomap(node, 1);
> WARN(!cpu_base, "unable to map gic cpu registers\n");
>
> - of_address_to_resource(node, 1, &cpu_res);
> -
> /*
> * Disable split EOI/Deactivate if either HYP is not available
> * or the CPU interface is too small.
> */
> - if (gic_cnt == 0 && (!is_hyp_mode_available() ||
> - resource_size(&cpu_res) < SZ_8K))
> + if (gic_cnt == 0 && !gic_check_eoimode(node, &cpu_base))
> static_key_slow_dec(&supports_deactivate);
>
> if (of_property_read_u32(node, "cpu-offset", &percpu_offset))
>
Meh. Multiple revisions of GIC400. This patchlet on top of the above
should take care of it:
diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index b62f2b2..a9ecb29 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -1122,6 +1122,8 @@ static int gic_cnt __initdata;
static bool gic_check_eoimode(struct device_node *node, void __iomem **base)
{
struct resource cpuif_res;
+ u32 mask = 0xffff0fff;
+ u32 gic400_id = 0x0202043B;
of_address_to_resource(node, 1, &cpuif_res);
@@ -1137,11 +1139,11 @@ static bool gic_check_eoimode(struct device_node *node, void __iomem **base)
* 64kB by checking the GICC_IIDR register.
*/
val = readl_relaxed(*base + GIC_CPU_IDENT);
- if (val != 0x0202043B)
+ if ((val & mask) != gic400_id)
return false;
val = readl_relaxed(*base + GIC_CPU_IDENT + 0xF000);
- if (val != 0x0202043B)
+ if ((val & mask) != gic400_id)
return false;
/*
Thanks,
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply related [flat|nested] 49+ messages in thread* [PATCH v4 3/4] irqchip: GIC: Convert to EOImode == 1
@ 2015-09-10 10:55 ` Marc Zyngier
0 siblings, 0 replies; 49+ messages in thread
From: Marc Zyngier @ 2015-09-10 10:55 UTC (permalink / raw)
To: linux-arm-kernel
On 10/09/15 10:54, Marc Zyngier wrote:
> Hi Julian,
>
> On 09/09/15 20:23, Julien Grall wrote:
>> Hi,
>>
>> I've been trying the latest linus/master (a794b4f), which include this
>> patch, as baremetal kernel on X-gene. This is failing on early boot
>> without much log.
>>
>> After bisecting the tree, I found the error coming from this patch.
>> While this patch is valid, it made me remembered that X-Gene (at least
>> the first version) as an odd GICv2.
>>
>> The GICC is divided in 2 area of 4K, each one aligned at a 64KB address.
>> This means that, the address of GICC_DIR won't be 0x1000 but 0x10000.
>
> Not really. I already mentioned that one a while ago:
>
> http://lists.infradead.org/pipermail/linux-arm-kernel/2015-March/332249.html
>
> The first page of GIC is aliased over the first 64kB, and the second
> page aliased over the second 64kB. So you get a consistent mapping if
> you use (base + 0xF000) to address GICC. Also, the DT that's in
> mainline is showing a 4kB CPU interface, which doesn't enable
> EOImode==1. You must be using a firmware that's newer than mine, since
> I'm perfectly able to boot my Mustang with these patches.
>
>> We had the same issue on Xen when we did the first port of X-gene [1].
>> Although, we choose to add a quirk in Xen for this platform in order to
>> map contiguously in the virtual memory the 2 part of GICC.
>>
>> Note that, back then, Ian suggested to extend the bindings to support a
>> such platform [2]. AFAICT, there was no follow-up on it.
>
> The main problem here is not to update the binding, but the fact that
> you *cannot* update the DT on x-gene (the firmware will replace your
> GIC node with what it thinks it is), and the APM guys can't be bothered
> to fix their stuff.
>
> In the meantime, can you give the following patch a shot? My Mustang is
> wired to a 4kB CPU interface, so I'll need your help to test it.
>
> Thanks,
>
> M.
>
> From f0f086a4462198a5a2ac840901d9b8fd23b25134 Mon Sep 17 00:00:00 2001
> From: Marc Zyngier <marc.zyngier@arm.com>
> Date: Thu, 10 Sep 2015 10:23:45 +0100
> Subject: [PATCH] irqchip/GIC: Add workaround for aliased GIC400
>
> The GICv2 architecture mandates that the two 4kB GIC regions are
> contiguous, and on two separate physical pages. This doesn't work
> very well when PAGE_SIZE is 64kB.
>
> A relatively common hack to work around this is to alias each 4kB
> region over its own 64kB page. Of course in this case, the base
> address you want to use is not really the begining of the region,
> but base + 60kB (so that you get a contiguous 8kB region over two
> distinct pages).
>
> Normally, this would be describe in DT with a new property, but
> some HW is already out there, and the firmware makes sure that
> it will override whatever you put in the GIC node. Duh. And of course,
> said firmware source code is not available, despite being based
> on u-boot.
>
> The workaround is to detect the case where the CPU interface size
> is set to 128kB, and verify the aliasing by checking that the ID
> register for GIC400 (which is the only GIC wired this way so far)
> is the same at base and base + 0xF000. In this case, we update
> the GIC base address and let it roll.
>
> And if you feel slightly sick by looking at this, rest assured that
> I do too...
>
> Reported-by: Julien Grall <julien.grall@citrix.com>
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
> drivers/irqchip/irq-gic.c | 45 ++++++++++++++++++++++++++++++++++++++++-----
> 1 file changed, 40 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
> index e6b7ed5..b62f2b2 100644
> --- a/drivers/irqchip/irq-gic.c
> +++ b/drivers/irqchip/irq-gic.c
> @@ -1119,12 +1119,50 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start,
> #ifdef CONFIG_OF
> static int gic_cnt __initdata;
>
> +static bool gic_check_eoimode(struct device_node *node, void __iomem **base)
> +{
> + struct resource cpuif_res;
> +
> + of_address_to_resource(node, 1, &cpuif_res);
> +
> + if (!is_hyp_mode_available())
> + return false;
> + if (resource_size(&cpuif_res) < SZ_8K)
> + return false;
> + if (resource_size(&cpuif_res) == SZ_128K) {
> + u32 val;
> +
> + /*
> + * Verify that we have a GIC400 aliased over the first
> + * 64kB by checking the GICC_IIDR register.
> + */
> + val = readl_relaxed(*base + GIC_CPU_IDENT);
> + if (val != 0x0202043B)
> + return false;
> +
> + val = readl_relaxed(*base + GIC_CPU_IDENT + 0xF000);
> + if (val != 0x0202043B)
> + return false;
> +
> + /*
> + * Move the base up by 60kB, so that we have a 8kB
> + * contiguous region, which allows us to use GICC_DIR
> + * at its normal offset.
> + */
> + *base += 0xF000;
> + cpuif_res.start += 0xF000;
> + pr_warn("GIC: Adjusting CPU interface base to %pa",
> + &cpuif_res.start);
> + }
> +
> + return true;
> +}
> +
> static int __init
> gic_of_init(struct device_node *node, struct device_node *parent)
> {
> void __iomem *cpu_base;
> void __iomem *dist_base;
> - struct resource cpu_res;
> u32 percpu_offset;
> int irq;
>
> @@ -1137,14 +1175,11 @@ gic_of_init(struct device_node *node, struct device_node *parent)
> cpu_base = of_iomap(node, 1);
> WARN(!cpu_base, "unable to map gic cpu registers\n");
>
> - of_address_to_resource(node, 1, &cpu_res);
> -
> /*
> * Disable split EOI/Deactivate if either HYP is not available
> * or the CPU interface is too small.
> */
> - if (gic_cnt == 0 && (!is_hyp_mode_available() ||
> - resource_size(&cpu_res) < SZ_8K))
> + if (gic_cnt == 0 && !gic_check_eoimode(node, &cpu_base))
> static_key_slow_dec(&supports_deactivate);
>
> if (of_property_read_u32(node, "cpu-offset", &percpu_offset))
>
Meh. Multiple revisions of GIC400. This patchlet on top of the above
should take care of it:
diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index b62f2b2..a9ecb29 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -1122,6 +1122,8 @@ static int gic_cnt __initdata;
static bool gic_check_eoimode(struct device_node *node, void __iomem **base)
{
struct resource cpuif_res;
+ u32 mask = 0xffff0fff;
+ u32 gic400_id = 0x0202043B;
of_address_to_resource(node, 1, &cpuif_res);
@@ -1137,11 +1139,11 @@ static bool gic_check_eoimode(struct device_node *node, void __iomem **base)
* 64kB by checking the GICC_IIDR register.
*/
val = readl_relaxed(*base + GIC_CPU_IDENT);
- if (val != 0x0202043B)
+ if ((val & mask) != gic400_id)
return false;
val = readl_relaxed(*base + GIC_CPU_IDENT + 0xF000);
- if (val != 0x0202043B)
+ if ((val & mask) != gic400_id)
return false;
/*
Thanks,
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply related [flat|nested] 49+ messages in thread
* Re: [PATCH v4 3/4] irqchip: GIC: Convert to EOImode == 1
2015-09-10 9:54 ` Marc Zyngier
(?)
@ 2015-09-10 16:23 ` Julien Grall
-1 siblings, 0 replies; 49+ messages in thread
From: Julien Grall @ 2015-09-10 16:23 UTC (permalink / raw)
To: Marc Zyngier, Thomas Gleixner, Jason Cooper,
Pranavkumar Sawargaonkar
Cc: Ian Campbell, kvm, linux-kernel, kvmarm, linux-arm-kernel,
Jiang Liu
On 10/09/15 10:54, Marc Zyngier wrote:
> Hi Julian,
Hi Marc,
> On 09/09/15 20:23, Julien Grall wrote:
>> Hi,
>>
>> I've been trying the latest linus/master (a794b4f), which include this
>> patch, as baremetal kernel on X-gene. This is failing on early boot
>> without much log.
>>
>> After bisecting the tree, I found the error coming from this patch.
>> While this patch is valid, it made me remembered that X-Gene (at least
>> the first version) as an odd GICv2.
>>
>> The GICC is divided in 2 area of 4K, each one aligned at a 64KB address.
>> This means that, the address of GICC_DIR won't be 0x1000 but 0x10000.
>
> Not really. I already mentioned that one a while ago:
>
> http://lists.infradead.org/pipermail/linux-arm-kernel/2015-March/332249.html
Sorry I haven't seen this thread on the ML.
> The first page of GIC is aliased over the first 64kB, and the second
> page aliased over the second 64kB. So you get a consistent mapping if
> you use (base + 0xF000) to address GICC. Also, the DT that's in
> mainline is showing a 4kB CPU interface, which doesn't enable
> EOImode==1.
> You must be using a firmware that's newer than mine, since
> I'm perfectly able to boot my Mustang with these patches.
My U-boot firmware is:
U-Boot 2013.04-mustang_sw_1.15.12 (May 20 2015 - 10:03:33)
The interrupt controller node looks like:
interrupt-controller@78090000 {
reg = <0x0 0x78090000 0x0 0x10000 0x0 0x780a0000 0x0
0x20000 0x0 0x780c0000 0x0 0x10000 0x0 0x780e0000 0x0 0x20000>;
interrupts = <0x1 0x9 0xf04>;
compatible = "arm,cortex-a15-gic";
#interrupt-cells = <0x3>;
phandle = <0x1>;
interrupt-controller;
linux,phandle = <0x1>;
};
Note that we have a recent firmware which correct the GICD region to use
the non-secure one rather than the secure. See [1] for more details.
>
>> We had the same issue on Xen when we did the first port of X-gene [1].
>> Although, we choose to add a quirk in Xen for this platform in order to
>> map contiguously in the virtual memory the 2 part of GICC.
>>
>> Note that, back then, Ian suggested to extend the bindings to support a
>> such platform [2]. AFAICT, there was no follow-up on it.
>
> The main problem here is not to update the binding, but the fact that
> you *cannot* update the DT on x-gene (the firmware will replace your
> GIC node with what it thinks it is), and the APM guys can't be bothered
> to fix their stuff.
>
> In the meantime, can you give the following patch a shot? My Mustang is
> wired to a 4kB CPU interface, so I'll need your help to test it.
I applied the two patches on top of linus/master and I'm able to boot
correctly on X-gene. Thank you!
Regards,
[1] http://lists.xen.org/archives/html/xen-devel/2015-04/msg02816.html
--
Julien Grall
^ permalink raw reply [flat|nested] 49+ messages in thread* Re: [PATCH v4 3/4] irqchip: GIC: Convert to EOImode == 1
@ 2015-09-10 16:23 ` Julien Grall
0 siblings, 0 replies; 49+ messages in thread
From: Julien Grall @ 2015-09-10 16:23 UTC (permalink / raw)
To: Marc Zyngier, Thomas Gleixner, Jason Cooper,
Pranavkumar Sawargaonkar
Cc: kvm, Eric Auger, linux-kernel, Jiang Liu, Christoffer Dall,
kvmarm, linux-arm-kernel, Stefano Stabellini, Ian Campbell
On 10/09/15 10:54, Marc Zyngier wrote:
> Hi Julian,
Hi Marc,
> On 09/09/15 20:23, Julien Grall wrote:
>> Hi,
>>
>> I've been trying the latest linus/master (a794b4f), which include this
>> patch, as baremetal kernel on X-gene. This is failing on early boot
>> without much log.
>>
>> After bisecting the tree, I found the error coming from this patch.
>> While this patch is valid, it made me remembered that X-Gene (at least
>> the first version) as an odd GICv2.
>>
>> The GICC is divided in 2 area of 4K, each one aligned at a 64KB address.
>> This means that, the address of GICC_DIR won't be 0x1000 but 0x10000.
>
> Not really. I already mentioned that one a while ago:
>
> http://lists.infradead.org/pipermail/linux-arm-kernel/2015-March/332249.html
Sorry I haven't seen this thread on the ML.
> The first page of GIC is aliased over the first 64kB, and the second
> page aliased over the second 64kB. So you get a consistent mapping if
> you use (base + 0xF000) to address GICC. Also, the DT that's in
> mainline is showing a 4kB CPU interface, which doesn't enable
> EOImode==1.
> You must be using a firmware that's newer than mine, since
> I'm perfectly able to boot my Mustang with these patches.
My U-boot firmware is:
U-Boot 2013.04-mustang_sw_1.15.12 (May 20 2015 - 10:03:33)
The interrupt controller node looks like:
interrupt-controller@78090000 {
reg = <0x0 0x78090000 0x0 0x10000 0x0 0x780a0000 0x0
0x20000 0x0 0x780c0000 0x0 0x10000 0x0 0x780e0000 0x0 0x20000>;
interrupts = <0x1 0x9 0xf04>;
compatible = "arm,cortex-a15-gic";
#interrupt-cells = <0x3>;
phandle = <0x1>;
interrupt-controller;
linux,phandle = <0x1>;
};
Note that we have a recent firmware which correct the GICD region to use
the non-secure one rather than the secure. See [1] for more details.
>
>> We had the same issue on Xen when we did the first port of X-gene [1].
>> Although, we choose to add a quirk in Xen for this platform in order to
>> map contiguously in the virtual memory the 2 part of GICC.
>>
>> Note that, back then, Ian suggested to extend the bindings to support a
>> such platform [2]. AFAICT, there was no follow-up on it.
>
> The main problem here is not to update the binding, but the fact that
> you *cannot* update the DT on x-gene (the firmware will replace your
> GIC node with what it thinks it is), and the APM guys can't be bothered
> to fix their stuff.
>
> In the meantime, can you give the following patch a shot? My Mustang is
> wired to a 4kB CPU interface, so I'll need your help to test it.
I applied the two patches on top of linus/master and I'm able to boot
correctly on X-gene. Thank you!
Regards,
[1] http://lists.xen.org/archives/html/xen-devel/2015-04/msg02816.html
--
Julien Grall
^ permalink raw reply [flat|nested] 49+ messages in thread* [PATCH v4 3/4] irqchip: GIC: Convert to EOImode == 1
@ 2015-09-10 16:23 ` Julien Grall
0 siblings, 0 replies; 49+ messages in thread
From: Julien Grall @ 2015-09-10 16:23 UTC (permalink / raw)
To: linux-arm-kernel
On 10/09/15 10:54, Marc Zyngier wrote:
> Hi Julian,
Hi Marc,
> On 09/09/15 20:23, Julien Grall wrote:
>> Hi,
>>
>> I've been trying the latest linus/master (a794b4f), which include this
>> patch, as baremetal kernel on X-gene. This is failing on early boot
>> without much log.
>>
>> After bisecting the tree, I found the error coming from this patch.
>> While this patch is valid, it made me remembered that X-Gene (at least
>> the first version) as an odd GICv2.
>>
>> The GICC is divided in 2 area of 4K, each one aligned at a 64KB address.
>> This means that, the address of GICC_DIR won't be 0x1000 but 0x10000.
>
> Not really. I already mentioned that one a while ago:
>
> http://lists.infradead.org/pipermail/linux-arm-kernel/2015-March/332249.html
Sorry I haven't seen this thread on the ML.
> The first page of GIC is aliased over the first 64kB, and the second
> page aliased over the second 64kB. So you get a consistent mapping if
> you use (base + 0xF000) to address GICC. Also, the DT that's in
> mainline is showing a 4kB CPU interface, which doesn't enable
> EOImode==1.
> You must be using a firmware that's newer than mine, since
> I'm perfectly able to boot my Mustang with these patches.
My U-boot firmware is:
U-Boot 2013.04-mustang_sw_1.15.12 (May 20 2015 - 10:03:33)
The interrupt controller node looks like:
interrupt-controller at 78090000 {
reg = <0x0 0x78090000 0x0 0x10000 0x0 0x780a0000 0x0
0x20000 0x0 0x780c0000 0x0 0x10000 0x0 0x780e0000 0x0 0x20000>;
interrupts = <0x1 0x9 0xf04>;
compatible = "arm,cortex-a15-gic";
#interrupt-cells = <0x3>;
phandle = <0x1>;
interrupt-controller;
linux,phandle = <0x1>;
};
Note that we have a recent firmware which correct the GICD region to use
the non-secure one rather than the secure. See [1] for more details.
>
>> We had the same issue on Xen when we did the first port of X-gene [1].
>> Although, we choose to add a quirk in Xen for this platform in order to
>> map contiguously in the virtual memory the 2 part of GICC.
>>
>> Note that, back then, Ian suggested to extend the bindings to support a
>> such platform [2]. AFAICT, there was no follow-up on it.
>
> The main problem here is not to update the binding, but the fact that
> you *cannot* update the DT on x-gene (the firmware will replace your
> GIC node with what it thinks it is), and the APM guys can't be bothered
> to fix their stuff.
>
> In the meantime, can you give the following patch a shot? My Mustang is
> wired to a 4kB CPU interface, so I'll need your help to test it.
I applied the two patches on top of linus/master and I'm able to boot
correctly on X-gene. Thank you!
Regards,
[1] http://lists.xen.org/archives/html/xen-devel/2015-04/msg02816.html
--
Julien Grall
^ permalink raw reply [flat|nested] 49+ messages in thread* Re: [PATCH v4 3/4] irqchip: GIC: Convert to EOImode == 1
2015-09-10 16:23 ` Julien Grall
@ 2015-09-10 16:30 ` Marc Zyngier
-1 siblings, 0 replies; 49+ messages in thread
From: Marc Zyngier @ 2015-09-10 16:30 UTC (permalink / raw)
To: Julien Grall, Thomas Gleixner, Jason Cooper,
Pranavkumar Sawargaonkar
Cc: kvm, Eric Auger, linux-kernel, Jiang Liu, Christoffer Dall,
kvmarm, linux-arm-kernel, Stefano Stabellini, Ian Campbell
On 10/09/15 17:23, Julien Grall wrote:
> On 10/09/15 10:54, Marc Zyngier wrote:
[...]
>> In the meantime, can you give the following patch a shot? My Mustang is
>> wired to a 4kB CPU interface, so I'll need your help to test it.
>
> I applied the two patches on top of linus/master and I'm able to boot
> correctly on X-gene. Thank you!
Thanks for testing. Can I put your Tested-by tag on the patch when I
send it to Thomas?
Thanks,
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH v4 3/4] irqchip: GIC: Convert to EOImode == 1
@ 2015-09-10 16:30 ` Marc Zyngier
0 siblings, 0 replies; 49+ messages in thread
From: Marc Zyngier @ 2015-09-10 16:30 UTC (permalink / raw)
To: linux-arm-kernel
On 10/09/15 17:23, Julien Grall wrote:
> On 10/09/15 10:54, Marc Zyngier wrote:
[...]
>> In the meantime, can you give the following patch a shot? My Mustang is
>> wired to a 4kB CPU interface, so I'll need your help to test it.
>
> I applied the two patches on top of linus/master and I'm able to boot
> correctly on X-gene. Thank you!
Thanks for testing. Can I put your Tested-by tag on the patch when I
send it to Thomas?
Thanks,
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH v4 3/4] irqchip: GIC: Convert to EOImode == 1
2015-09-10 16:30 ` Marc Zyngier
(?)
@ 2015-09-10 16:30 ` Julien Grall
-1 siblings, 0 replies; 49+ messages in thread
From: Julien Grall @ 2015-09-10 16:30 UTC (permalink / raw)
To: Marc Zyngier, Thomas Gleixner, Jason Cooper,
Pranavkumar Sawargaonkar
Cc: Ian Campbell, kvm, linux-kernel, kvmarm, linux-arm-kernel,
Jiang Liu
On 10/09/15 17:30, Marc Zyngier wrote:
> On 10/09/15 17:23, Julien Grall wrote:
>> On 10/09/15 10:54, Marc Zyngier wrote:
>
> [...]
>
>>> In the meantime, can you give the following patch a shot? My Mustang is
>>> wired to a 4kB CPU interface, so I'll need your help to test it.
>>
>> I applied the two patches on top of linus/master and I'm able to boot
>> correctly on X-gene. Thank you!
>
> Thanks for testing. Can I put your Tested-by tag on the patch when I
> send it to Thomas?
Sure:
Tested-by: Julien Grall <julien.grall@citrix.com>
Regards,
--
Julien Grall
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH v4 3/4] irqchip: GIC: Convert to EOImode == 1
@ 2015-09-10 16:30 ` Julien Grall
0 siblings, 0 replies; 49+ messages in thread
From: Julien Grall @ 2015-09-10 16:30 UTC (permalink / raw)
To: Marc Zyngier, Thomas Gleixner, Jason Cooper,
Pranavkumar Sawargaonkar
Cc: kvm, Eric Auger, linux-kernel, Jiang Liu, Christoffer Dall,
kvmarm, linux-arm-kernel, Stefano Stabellini, Ian Campbell
On 10/09/15 17:30, Marc Zyngier wrote:
> On 10/09/15 17:23, Julien Grall wrote:
>> On 10/09/15 10:54, Marc Zyngier wrote:
>
> [...]
>
>>> In the meantime, can you give the following patch a shot? My Mustang is
>>> wired to a 4kB CPU interface, so I'll need your help to test it.
>>
>> I applied the two patches on top of linus/master and I'm able to boot
>> correctly on X-gene. Thank you!
>
> Thanks for testing. Can I put your Tested-by tag on the patch when I
> send it to Thomas?
Sure:
Tested-by: Julien Grall <julien.grall@citrix.com>
Regards,
--
Julien Grall
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH v4 3/4] irqchip: GIC: Convert to EOImode == 1
@ 2015-09-10 16:30 ` Julien Grall
0 siblings, 0 replies; 49+ messages in thread
From: Julien Grall @ 2015-09-10 16:30 UTC (permalink / raw)
To: linux-arm-kernel
On 10/09/15 17:30, Marc Zyngier wrote:
> On 10/09/15 17:23, Julien Grall wrote:
>> On 10/09/15 10:54, Marc Zyngier wrote:
>
> [...]
>
>>> In the meantime, can you give the following patch a shot? My Mustang is
>>> wired to a 4kB CPU interface, so I'll need your help to test it.
>>
>> I applied the two patches on top of linus/master and I'm able to boot
>> correctly on X-gene. Thank you!
>
> Thanks for testing. Can I put your Tested-by tag on the patch when I
> send it to Thomas?
Sure:
Tested-by: Julien Grall <julien.grall@citrix.com>
Regards,
--
Julien Grall
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH v4 3/4] irqchip: GIC: Convert to EOImode == 1
2015-09-10 16:23 ` Julien Grall
(?)
@ 2015-09-11 10:54 ` Ian Campbell
-1 siblings, 0 replies; 49+ messages in thread
From: Ian Campbell @ 2015-09-11 10:54 UTC (permalink / raw)
To: Julien Grall, Marc Zyngier, Thomas Gleixner, Jason Cooper,
Pranavkumar Sawargaonkar
Cc: kvm, Eric Auger, linux-kernel, Jiang Liu, Christoffer Dall,
kvmarm, linux-arm-kernel, Stefano Stabellini
On Thu, 2015-09-10 at 17:23 +0100, Julien Grall wrote:
> I applied the two patches on top of linus/master and I'm able to boot
> correctly on X-gene. Thank you!
Perhaps we should replicate this approach in Xen and get rid of
PLATFORM_QUIRK_GIC_64K_STRIDE?
Ian.
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH v4 3/4] irqchip: GIC: Convert to EOImode == 1
@ 2015-09-11 10:54 ` Ian Campbell
0 siblings, 0 replies; 49+ messages in thread
From: Ian Campbell @ 2015-09-11 10:54 UTC (permalink / raw)
To: Julien Grall, Marc Zyngier, Thomas Gleixner, Jason Cooper,
Pranavkumar Sawargaonkar
Cc: kvm, Eric Auger, linux-kernel, Jiang Liu, Christoffer Dall,
kvmarm, linux-arm-kernel, Stefano Stabellini
On Thu, 2015-09-10 at 17:23 +0100, Julien Grall wrote:
> I applied the two patches on top of linus/master and I'm able to boot
> correctly on X-gene. Thank you!
Perhaps we should replicate this approach in Xen and get rid of
PLATFORM_QUIRK_GIC_64K_STRIDE?
Ian.
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH v4 3/4] irqchip: GIC: Convert to EOImode == 1
@ 2015-09-11 10:54 ` Ian Campbell
0 siblings, 0 replies; 49+ messages in thread
From: Ian Campbell @ 2015-09-11 10:54 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, 2015-09-10 at 17:23 +0100, Julien Grall wrote:
> I applied the two patches on top of linus/master and I'm able to boot
> correctly on X-gene. Thank you!
Perhaps we should replicate this approach in Xen and get rid of
PLATFORM_QUIRK_GIC_64K_STRIDE?
Ian.
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH v4 3/4] irqchip: GIC: Convert to EOImode == 1
2015-09-11 10:54 ` Ian Campbell
(?)
@ 2015-09-11 10:59 ` Julien Grall
-1 siblings, 0 replies; 49+ messages in thread
From: Julien Grall @ 2015-09-11 10:59 UTC (permalink / raw)
To: Ian Campbell, Marc Zyngier, Pranavkumar Sawargaonkar
Cc: Thomas Gleixner, Jason Cooper, kvm, Eric Auger, linux-kernel,
Jiang Liu, Christoffer Dall, kvmarm, linux-arm-kernel,
Stefano Stabellini
On 11/09/2015 11:54, Ian Campbell wrote:
> On Thu, 2015-09-10 at 17:23 +0100, Julien Grall wrote:
>> I applied the two patches on top of linus/master and I'm able to boot
>> correctly on X-gene. Thank you!
>
> Perhaps we should replicate this approach in Xen and get rid of
> PLATFORM_QUIRK_GIC_64K_STRIDE?
I was thinking to do it. But, I wasn't sure if it was worth to get a
such "ugly" patch compare to the quirk.
Regards,
--
Julien Grall
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH v4 3/4] irqchip: GIC: Convert to EOImode == 1
@ 2015-09-11 10:59 ` Julien Grall
0 siblings, 0 replies; 49+ messages in thread
From: Julien Grall @ 2015-09-11 10:59 UTC (permalink / raw)
To: Ian Campbell, Marc Zyngier, Pranavkumar Sawargaonkar
Cc: Thomas Gleixner, Jason Cooper, kvm, Eric Auger, linux-kernel,
Jiang Liu, Christoffer Dall, kvmarm, linux-arm-kernel,
Stefano Stabellini
On 11/09/2015 11:54, Ian Campbell wrote:
> On Thu, 2015-09-10 at 17:23 +0100, Julien Grall wrote:
>> I applied the two patches on top of linus/master and I'm able to boot
>> correctly on X-gene. Thank you!
>
> Perhaps we should replicate this approach in Xen and get rid of
> PLATFORM_QUIRK_GIC_64K_STRIDE?
I was thinking to do it. But, I wasn't sure if it was worth to get a
such "ugly" patch compare to the quirk.
Regards,
--
Julien Grall
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH v4 3/4] irqchip: GIC: Convert to EOImode == 1
@ 2015-09-11 10:59 ` Julien Grall
0 siblings, 0 replies; 49+ messages in thread
From: Julien Grall @ 2015-09-11 10:59 UTC (permalink / raw)
To: linux-arm-kernel
On 11/09/2015 11:54, Ian Campbell wrote:
> On Thu, 2015-09-10 at 17:23 +0100, Julien Grall wrote:
>> I applied the two patches on top of linus/master and I'm able to boot
>> correctly on X-gene. Thank you!
>
> Perhaps we should replicate this approach in Xen and get rid of
> PLATFORM_QUIRK_GIC_64K_STRIDE?
I was thinking to do it. But, I wasn't sure if it was worth to get a
such "ugly" patch compare to the quirk.
Regards,
--
Julien Grall
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH v4 3/4] irqchip: GIC: Convert to EOImode == 1
2015-09-11 10:59 ` Julien Grall
@ 2015-09-11 11:09 ` Marc Zyngier
-1 siblings, 0 replies; 49+ messages in thread
From: Marc Zyngier @ 2015-09-11 11:09 UTC (permalink / raw)
To: Julien Grall, Ian Campbell, Pranavkumar Sawargaonkar
Cc: Thomas Gleixner, Jason Cooper, kvm, Eric Auger, linux-kernel,
Jiang Liu, Christoffer Dall, kvmarm, linux-arm-kernel,
Stefano Stabellini
On 11/09/15 11:59, Julien Grall wrote:
>
>
> On 11/09/2015 11:54, Ian Campbell wrote:
>> On Thu, 2015-09-10 at 17:23 +0100, Julien Grall wrote:
>>> I applied the two patches on top of linus/master and I'm able to boot
>>> correctly on X-gene. Thank you!
>>
>> Perhaps we should replicate this approach in Xen and get rid of
>> PLATFORM_QUIRK_GIC_64K_STRIDE?
>
> I was thinking to do it. But, I wasn't sure if it was worth to get a
> such "ugly" patch compare to the quirk.
It is not a quirk. It is actually recommended in the SBSA spec. The
patch is ugly because we can't do the right thing on the one platform
that actually implemented ARM's own recommendation (we can't tell the
bloody firmware to stop overriding our DT).
I would otherwise have added a "arm,use-sbsa-aliasing" property (or
something similar) instead of trying to guess things...
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH v4 3/4] irqchip: GIC: Convert to EOImode == 1
@ 2015-09-11 11:09 ` Marc Zyngier
0 siblings, 0 replies; 49+ messages in thread
From: Marc Zyngier @ 2015-09-11 11:09 UTC (permalink / raw)
To: linux-arm-kernel
On 11/09/15 11:59, Julien Grall wrote:
>
>
> On 11/09/2015 11:54, Ian Campbell wrote:
>> On Thu, 2015-09-10 at 17:23 +0100, Julien Grall wrote:
>>> I applied the two patches on top of linus/master and I'm able to boot
>>> correctly on X-gene. Thank you!
>>
>> Perhaps we should replicate this approach in Xen and get rid of
>> PLATFORM_QUIRK_GIC_64K_STRIDE?
>
> I was thinking to do it. But, I wasn't sure if it was worth to get a
> such "ugly" patch compare to the quirk.
It is not a quirk. It is actually recommended in the SBSA spec. The
patch is ugly because we can't do the right thing on the one platform
that actually implemented ARM's own recommendation (we can't tell the
bloody firmware to stop overriding our DT).
I would otherwise have added a "arm,use-sbsa-aliasing" property (or
something similar) instead of trying to guess things...
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH v4 3/4] irqchip: GIC: Convert to EOImode == 1
2015-09-11 11:09 ` Marc Zyngier
(?)
@ 2015-09-11 12:53 ` Julien Grall
-1 siblings, 0 replies; 49+ messages in thread
From: Julien Grall @ 2015-09-11 12:53 UTC (permalink / raw)
To: Marc Zyngier, Ian Campbell, Pranavkumar Sawargaonkar
Cc: Jason Cooper, kvm, Eric Auger, Stefano Stabellini, linux-kernel,
kvmarm, linux-arm-kernel, Thomas Gleixner, Jiang Liu,
Christoffer Dall
On 11/09/15 12:09, Marc Zyngier wrote:
> On 11/09/15 11:59, Julien Grall wrote:
>>
>>
>> On 11/09/2015 11:54, Ian Campbell wrote:
>>> On Thu, 2015-09-10 at 17:23 +0100, Julien Grall wrote:
>>>> I applied the two patches on top of linus/master and I'm able to boot
>>>> correctly on X-gene. Thank you!
>>>
>>> Perhaps we should replicate this approach in Xen and get rid of
>>> PLATFORM_QUIRK_GIC_64K_STRIDE?
>>
>> I was thinking to do it. But, I wasn't sure if it was worth to get a
>> such "ugly" patch compare to the quirk.
>
> It is not a quirk. It is actually recommended in the SBSA spec. The
> patch is ugly because we can't do the right thing on the one platform
> that actually implemented ARM's own recommendation (we can't tell the
> bloody firmware to stop overriding our DT).
>
> I would otherwise have added a "arm,use-sbsa-aliasing" property (or
> something similar) instead of trying to guess things...
I will give a look to port this patch on Xen.
Regards,
--
Julien Grall
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH v4 3/4] irqchip: GIC: Convert to EOImode == 1
@ 2015-09-11 12:53 ` Julien Grall
0 siblings, 0 replies; 49+ messages in thread
From: Julien Grall @ 2015-09-11 12:53 UTC (permalink / raw)
To: Marc Zyngier, Ian Campbell, Pranavkumar Sawargaonkar
Cc: Jason Cooper, kvm, Eric Auger, Stefano Stabellini, linux-kernel,
kvmarm, linux-arm-kernel, Thomas Gleixner, Jiang Liu,
Christoffer Dall
On 11/09/15 12:09, Marc Zyngier wrote:
> On 11/09/15 11:59, Julien Grall wrote:
>>
>>
>> On 11/09/2015 11:54, Ian Campbell wrote:
>>> On Thu, 2015-09-10 at 17:23 +0100, Julien Grall wrote:
>>>> I applied the two patches on top of linus/master and I'm able to boot
>>>> correctly on X-gene. Thank you!
>>>
>>> Perhaps we should replicate this approach in Xen and get rid of
>>> PLATFORM_QUIRK_GIC_64K_STRIDE?
>>
>> I was thinking to do it. But, I wasn't sure if it was worth to get a
>> such "ugly" patch compare to the quirk.
>
> It is not a quirk. It is actually recommended in the SBSA spec. The
> patch is ugly because we can't do the right thing on the one platform
> that actually implemented ARM's own recommendation (we can't tell the
> bloody firmware to stop overriding our DT).
>
> I would otherwise have added a "arm,use-sbsa-aliasing" property (or
> something similar) instead of trying to guess things...
I will give a look to port this patch on Xen.
Regards,
--
Julien Grall
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH v4 3/4] irqchip: GIC: Convert to EOImode == 1
@ 2015-09-11 12:53 ` Julien Grall
0 siblings, 0 replies; 49+ messages in thread
From: Julien Grall @ 2015-09-11 12:53 UTC (permalink / raw)
To: linux-arm-kernel
On 11/09/15 12:09, Marc Zyngier wrote:
> On 11/09/15 11:59, Julien Grall wrote:
>>
>>
>> On 11/09/2015 11:54, Ian Campbell wrote:
>>> On Thu, 2015-09-10 at 17:23 +0100, Julien Grall wrote:
>>>> I applied the two patches on top of linus/master and I'm able to boot
>>>> correctly on X-gene. Thank you!
>>>
>>> Perhaps we should replicate this approach in Xen and get rid of
>>> PLATFORM_QUIRK_GIC_64K_STRIDE?
>>
>> I was thinking to do it. But, I wasn't sure if it was worth to get a
>> such "ugly" patch compare to the quirk.
>
> It is not a quirk. It is actually recommended in the SBSA spec. The
> patch is ugly because we can't do the right thing on the one platform
> that actually implemented ARM's own recommendation (we can't tell the
> bloody firmware to stop overriding our DT).
>
> I would otherwise have added a "arm,use-sbsa-aliasing" property (or
> something similar) instead of trying to guess things...
I will give a look to port this patch on Xen.
Regards,
--
Julien Grall
^ permalink raw reply [flat|nested] 49+ messages in thread