All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/4] irqchip/gic: Deal with the active state across kexec and suspend/resume
@ 2015-11-16 19:13 ` Marc Zyngier
  0 siblings, 0 replies; 13+ messages in thread
From: Marc Zyngier @ 2015-11-16 19:13 UTC (permalink / raw)
  To: Thomas Gleixner, Jason Cooper, Russell King
  Cc: linux-arm-kernel, linux-kernel

Now that the kernel uses EOImode==1 for the GIC, this uncovers a
number of cases where the active state is not correctly handled:

- When doing kexec, we directly call the irq_eoi() method, which may
  violate ordering constraints, or leave interrupts active.
- When booting, we don't reset the active state, trusting whatever was
  there before.
- When doing suspend/resume, we're not saving/restoring the active
  state, which could result in lost interrupts if VMs were running at
  this precise moment.

This has also uncovered a small bug in the way we restore enabled
interrupts (which could result in a fix to stable).

These patches are on top of v4.4-rc1.

Marc Zyngier (4):
  arm: kexec: Deactivate in-flight interrupts
  irqchip/gic: Make sure all interrupts are deactivated at boot
  irqchip/gic: Clear enable bits before restoring them
  irqchip/gic: Add save/restore of the active state

 arch/arm/kernel/machine_kexec.c  | 11 ++++++++++-
 drivers/irqchip/irq-gic-common.c | 13 +++++++++----
 drivers/irqchip/irq-gic.c        | 38 ++++++++++++++++++++++++++++++++++++--
 3 files changed, 55 insertions(+), 7 deletions(-)

-- 
2.1.4


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 0/4] irqchip/gic: Deal with the active state across kexec and suspend/resume
@ 2015-11-16 19:13 ` Marc Zyngier
  0 siblings, 0 replies; 13+ messages in thread
From: Marc Zyngier @ 2015-11-16 19:13 UTC (permalink / raw)
  To: linux-arm-kernel

Now that the kernel uses EOImode==1 for the GIC, this uncovers a
number of cases where the active state is not correctly handled:

- When doing kexec, we directly call the irq_eoi() method, which may
  violate ordering constraints, or leave interrupts active.
- When booting, we don't reset the active state, trusting whatever was
  there before.
- When doing suspend/resume, we're not saving/restoring the active
  state, which could result in lost interrupts if VMs were running at
  this precise moment.

This has also uncovered a small bug in the way we restore enabled
interrupts (which could result in a fix to stable).

These patches are on top of v4.4-rc1.

Marc Zyngier (4):
  arm: kexec: Deactivate in-flight interrupts
  irqchip/gic: Make sure all interrupts are deactivated at boot
  irqchip/gic: Clear enable bits before restoring them
  irqchip/gic: Add save/restore of the active state

 arch/arm/kernel/machine_kexec.c  | 11 ++++++++++-
 drivers/irqchip/irq-gic-common.c | 13 +++++++++----
 drivers/irqchip/irq-gic.c        | 38 ++++++++++++++++++++++++++++++++++++--
 3 files changed, 55 insertions(+), 7 deletions(-)

-- 
2.1.4

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 1/4] arm: kexec: Deactivate in-flight interrupts
  2015-11-16 19:13 ` Marc Zyngier
@ 2015-11-16 19:13   ` Marc Zyngier
  -1 siblings, 0 replies; 13+ messages in thread
From: Marc Zyngier @ 2015-11-16 19:13 UTC (permalink / raw)
  To: Thomas Gleixner, Jason Cooper, Russell King
  Cc: linux-arm-kernel, linux-kernel

machine_kexec_mask_interrupts iterates over the system interrupts
and tries to mask all interrupts, including those that are currently
being handled.

The current method includes finding out if an interrupt is in progress,
and call the EOI method if that's the case. This methods has a few
issues when used with the GIC:

- In a hypothetical GIC centric world where we can handle
  interrupts at different priorities, nothing guarantees that
  we're going to EOI the interrupts in the mandated reverse order
  we have taken them.

- With the split EOI/Deactivate mode the GIC runs in when using
  virtualization, an interrupt can be EOIed, and still be active.
  The current code would not recognize that state (the interrupt
  is not flagged as being in progress from a host PoV).

A sensible way of avoiding these issues is to forcefully deactivate
the interrupt at the distributor level, and to only use EOI if
the deactivation has failed (which probably means that the irqchip
is not a GIC).

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm/kernel/machine_kexec.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/arch/arm/kernel/machine_kexec.c b/arch/arm/kernel/machine_kexec.c
index 8bf3b7c..66662e6 100644
--- a/arch/arm/kernel/machine_kexec.c
+++ b/arch/arm/kernel/machine_kexec.c
@@ -8,6 +8,7 @@
 #include <linux/reboot.h>
 #include <linux/io.h>
 #include <linux/irq.h>
+#include <linux/interrupt.h>
 #include <linux/memblock.h>
 #include <asm/pgtable.h>
 #include <linux/of_fdt.h>
@@ -98,12 +99,20 @@ static void machine_kexec_mask_interrupts(void)
 
 	for_each_irq_desc(i, desc) {
 		struct irq_chip *chip;
+		int ret;
 
 		chip = irq_desc_get_chip(desc);
 		if (!chip)
 			continue;
 
-		if (chip->irq_eoi && irqd_irq_inprogress(&desc->irq_data))
+		/*
+		 * First try to remove the active state. If this
+		 * fails, try to EOI the interrupt.
+		 */
+		ret = irq_set_irqchip_state(i, IRQCHIP_STATE_ACTIVE, false);
+
+		if (ret && irqd_irq_inprogress(&desc->irq_data) &&
+		    chip->irq_eoi)
 			chip->irq_eoi(&desc->irq_data);
 
 		if (chip->irq_mask)
-- 
2.1.4


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 1/4] arm: kexec: Deactivate in-flight interrupts
@ 2015-11-16 19:13   ` Marc Zyngier
  0 siblings, 0 replies; 13+ messages in thread
From: Marc Zyngier @ 2015-11-16 19:13 UTC (permalink / raw)
  To: linux-arm-kernel

machine_kexec_mask_interrupts iterates over the system interrupts
and tries to mask all interrupts, including those that are currently
being handled.

The current method includes finding out if an interrupt is in progress,
and call the EOI method if that's the case. This methods has a few
issues when used with the GIC:

- In a hypothetical GIC centric world where we can handle
  interrupts at different priorities, nothing guarantees that
  we're going to EOI the interrupts in the mandated reverse order
  we have taken them.

- With the split EOI/Deactivate mode the GIC runs in when using
  virtualization, an interrupt can be EOIed, and still be active.
  The current code would not recognize that state (the interrupt
  is not flagged as being in progress from a host PoV).

A sensible way of avoiding these issues is to forcefully deactivate
the interrupt at the distributor level, and to only use EOI if
the deactivation has failed (which probably means that the irqchip
is not a GIC).

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm/kernel/machine_kexec.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/arch/arm/kernel/machine_kexec.c b/arch/arm/kernel/machine_kexec.c
index 8bf3b7c..66662e6 100644
--- a/arch/arm/kernel/machine_kexec.c
+++ b/arch/arm/kernel/machine_kexec.c
@@ -8,6 +8,7 @@
 #include <linux/reboot.h>
 #include <linux/io.h>
 #include <linux/irq.h>
+#include <linux/interrupt.h>
 #include <linux/memblock.h>
 #include <asm/pgtable.h>
 #include <linux/of_fdt.h>
@@ -98,12 +99,20 @@ static void machine_kexec_mask_interrupts(void)
 
 	for_each_irq_desc(i, desc) {
 		struct irq_chip *chip;
+		int ret;
 
 		chip = irq_desc_get_chip(desc);
 		if (!chip)
 			continue;
 
-		if (chip->irq_eoi && irqd_irq_inprogress(&desc->irq_data))
+		/*
+		 * First try to remove the active state. If this
+		 * fails, try to EOI the interrupt.
+		 */
+		ret = irq_set_irqchip_state(i, IRQCHIP_STATE_ACTIVE, false);
+
+		if (ret && irqd_irq_inprogress(&desc->irq_data) &&
+		    chip->irq_eoi)
 			chip->irq_eoi(&desc->irq_data);
 
 		if (chip->irq_mask)
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 2/4] irqchip/gic: Make sure all interrupts are deactivated at boot
  2015-11-16 19:13 ` Marc Zyngier
@ 2015-11-16 19:13   ` Marc Zyngier
  -1 siblings, 0 replies; 13+ messages in thread
From: Marc Zyngier @ 2015-11-16 19:13 UTC (permalink / raw)
  To: Thomas Gleixner, Jason Cooper, Russell King
  Cc: linux-arm-kernel, linux-kernel

When booting a GIC/GICv3 based system, we have no idea what
state the firmware (or previous kernel in the case of kexec)
has left the GIC, and some interrupts may still be active.

In order to garantee that we have a clean state, make sure
the active bits are cleared at init time.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 drivers/irqchip/irq-gic-common.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/irqchip/irq-gic-common.c b/drivers/irqchip/irq-gic-common.c
index 44a077f..f174ce0 100644
--- a/drivers/irqchip/irq-gic-common.c
+++ b/drivers/irqchip/irq-gic-common.c
@@ -84,12 +84,15 @@ void __init gic_dist_config(void __iomem *base, int gic_irqs,
 		writel_relaxed(GICD_INT_DEF_PRI_X4, base + GIC_DIST_PRI + i);
 
 	/*
-	 * Disable all interrupts.  Leave the PPI and SGIs alone
-	 * as they are enabled by redistributor registers.
+	 * Deactivate and disable all SPIs. Leave the PPI and SGIs
+	 * alone as they are in the redistributor registers on GICv3.
 	 */
-	for (i = 32; i < gic_irqs; i += 32)
+	for (i = 32; i < gic_irqs; i += 32) {
 		writel_relaxed(GICD_INT_EN_CLR_X32,
-					base + GIC_DIST_ENABLE_CLEAR + i / 8);
+			       base + GIC_DIST_ACTIVE_CLEAR + i / 8);
+		writel_relaxed(GICD_INT_EN_CLR_X32,
+			       base + GIC_DIST_ENABLE_CLEAR + i / 8);
+	}
 
 	if (sync_access)
 		sync_access();
@@ -102,7 +105,9 @@ void gic_cpu_config(void __iomem *base, void (*sync_access)(void))
 	/*
 	 * Deal with the banked PPI and SGI interrupts - disable all
 	 * PPI interrupts, ensure all SGI interrupts are enabled.
+	 * Make sure everything is deactivated.
 	 */
+	writel_relaxed(GICD_INT_EN_CLR_X32, base + GIC_DIST_ACTIVE_CLEAR);
 	writel_relaxed(GICD_INT_EN_CLR_PPI, base + GIC_DIST_ENABLE_CLEAR);
 	writel_relaxed(GICD_INT_EN_SET_SGI, base + GIC_DIST_ENABLE_SET);
 
-- 
2.1.4


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 2/4] irqchip/gic: Make sure all interrupts are deactivated at boot
@ 2015-11-16 19:13   ` Marc Zyngier
  0 siblings, 0 replies; 13+ messages in thread
From: Marc Zyngier @ 2015-11-16 19:13 UTC (permalink / raw)
  To: linux-arm-kernel

When booting a GIC/GICv3 based system, we have no idea what
state the firmware (or previous kernel in the case of kexec)
has left the GIC, and some interrupts may still be active.

In order to garantee that we have a clean state, make sure
the active bits are cleared at init time.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 drivers/irqchip/irq-gic-common.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/irqchip/irq-gic-common.c b/drivers/irqchip/irq-gic-common.c
index 44a077f..f174ce0 100644
--- a/drivers/irqchip/irq-gic-common.c
+++ b/drivers/irqchip/irq-gic-common.c
@@ -84,12 +84,15 @@ void __init gic_dist_config(void __iomem *base, int gic_irqs,
 		writel_relaxed(GICD_INT_DEF_PRI_X4, base + GIC_DIST_PRI + i);
 
 	/*
-	 * Disable all interrupts.  Leave the PPI and SGIs alone
-	 * as they are enabled by redistributor registers.
+	 * Deactivate and disable all SPIs. Leave the PPI and SGIs
+	 * alone as they are in the redistributor registers on GICv3.
 	 */
-	for (i = 32; i < gic_irqs; i += 32)
+	for (i = 32; i < gic_irqs; i += 32) {
 		writel_relaxed(GICD_INT_EN_CLR_X32,
-					base + GIC_DIST_ENABLE_CLEAR + i / 8);
+			       base + GIC_DIST_ACTIVE_CLEAR + i / 8);
+		writel_relaxed(GICD_INT_EN_CLR_X32,
+			       base + GIC_DIST_ENABLE_CLEAR + i / 8);
+	}
 
 	if (sync_access)
 		sync_access();
@@ -102,7 +105,9 @@ void gic_cpu_config(void __iomem *base, void (*sync_access)(void))
 	/*
 	 * Deal with the banked PPI and SGI interrupts - disable all
 	 * PPI interrupts, ensure all SGI interrupts are enabled.
+	 * Make sure everything is deactivated.
 	 */
+	writel_relaxed(GICD_INT_EN_CLR_X32, base + GIC_DIST_ACTIVE_CLEAR);
 	writel_relaxed(GICD_INT_EN_CLR_PPI, base + GIC_DIST_ENABLE_CLEAR);
 	writel_relaxed(GICD_INT_EN_SET_SGI, base + GIC_DIST_ENABLE_SET);
 
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 3/4] irqchip/gic: Clear enable bits before restoring them
  2015-11-16 19:13 ` Marc Zyngier
@ 2015-11-16 19:13   ` Marc Zyngier
  -1 siblings, 0 replies; 13+ messages in thread
From: Marc Zyngier @ 2015-11-16 19:13 UTC (permalink / raw)
  To: Thomas Gleixner, Jason Cooper, Russell King
  Cc: linux-arm-kernel, linux-kernel

When restoring the GIC state (after a suspend/resume cycle,
for example), the driver directly writes the 'enabled' state
it has saved by accessing GICD_ISENABLERn, which performs
an OR operation between the value present in the register
and the value we write.

If whatever code that has run before we reentered the kernel
has enabled an interrupt that was previously disabled, we won't
restore that disabled state.

Making sure we first clear the register (by writting to
GICD_ICENABLERn) before restoring the enabled state.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 drivers/irqchip/irq-gic.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index 515c823..bc846e7 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -604,9 +604,12 @@ static void gic_dist_restore(unsigned int gic_nr)
 		writel_relaxed(gic_data[gic_nr].saved_spi_target[i],
 			dist_base + GIC_DIST_TARGET + i * 4);
 
-	for (i = 0; i < DIV_ROUND_UP(gic_irqs, 32); i++)
+	for (i = 0; i < DIV_ROUND_UP(gic_irqs, 32); i++) {
+		writel_relaxed(GICD_INT_EN_CLR_X32,
+			dist_base + GIC_DIST_ENABLE_CLEAR + i * 4);
 		writel_relaxed(gic_data[gic_nr].saved_spi_enable[i],
 			dist_base + GIC_DIST_ENABLE_SET + i * 4);
+	}
 
 	writel_relaxed(GICD_ENABLE, dist_base + GIC_DIST_CTRL);
 }
@@ -654,8 +657,11 @@ static void gic_cpu_restore(unsigned int gic_nr)
 		return;
 
 	ptr = raw_cpu_ptr(gic_data[gic_nr].saved_ppi_enable);
-	for (i = 0; i < DIV_ROUND_UP(32, 32); i++)
+	for (i = 0; i < DIV_ROUND_UP(32, 32); i++) {
+		writel_relaxed(GICD_INT_EN_CLR_X32,
+			       dist_base + GIC_DIST_ENABLE_CLEAR + i * 4);
 		writel_relaxed(ptr[i], dist_base + GIC_DIST_ENABLE_SET + i * 4);
+	}
 
 	ptr = raw_cpu_ptr(gic_data[gic_nr].saved_ppi_conf);
 	for (i = 0; i < DIV_ROUND_UP(32, 16); i++)
-- 
2.1.4


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 3/4] irqchip/gic: Clear enable bits before restoring them
@ 2015-11-16 19:13   ` Marc Zyngier
  0 siblings, 0 replies; 13+ messages in thread
From: Marc Zyngier @ 2015-11-16 19:13 UTC (permalink / raw)
  To: linux-arm-kernel

When restoring the GIC state (after a suspend/resume cycle,
for example), the driver directly writes the 'enabled' state
it has saved by accessing GICD_ISENABLERn, which performs
an OR operation between the value present in the register
and the value we write.

If whatever code that has run before we reentered the kernel
has enabled an interrupt that was previously disabled, we won't
restore that disabled state.

Making sure we first clear the register (by writting to
GICD_ICENABLERn) before restoring the enabled state.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 drivers/irqchip/irq-gic.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index 515c823..bc846e7 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -604,9 +604,12 @@ static void gic_dist_restore(unsigned int gic_nr)
 		writel_relaxed(gic_data[gic_nr].saved_spi_target[i],
 			dist_base + GIC_DIST_TARGET + i * 4);
 
-	for (i = 0; i < DIV_ROUND_UP(gic_irqs, 32); i++)
+	for (i = 0; i < DIV_ROUND_UP(gic_irqs, 32); i++) {
+		writel_relaxed(GICD_INT_EN_CLR_X32,
+			dist_base + GIC_DIST_ENABLE_CLEAR + i * 4);
 		writel_relaxed(gic_data[gic_nr].saved_spi_enable[i],
 			dist_base + GIC_DIST_ENABLE_SET + i * 4);
+	}
 
 	writel_relaxed(GICD_ENABLE, dist_base + GIC_DIST_CTRL);
 }
@@ -654,8 +657,11 @@ static void gic_cpu_restore(unsigned int gic_nr)
 		return;
 
 	ptr = raw_cpu_ptr(gic_data[gic_nr].saved_ppi_enable);
-	for (i = 0; i < DIV_ROUND_UP(32, 32); i++)
+	for (i = 0; i < DIV_ROUND_UP(32, 32); i++) {
+		writel_relaxed(GICD_INT_EN_CLR_X32,
+			       dist_base + GIC_DIST_ENABLE_CLEAR + i * 4);
 		writel_relaxed(ptr[i], dist_base + GIC_DIST_ENABLE_SET + i * 4);
+	}
 
 	ptr = raw_cpu_ptr(gic_data[gic_nr].saved_ppi_conf);
 	for (i = 0; i < DIV_ROUND_UP(32, 16); i++)
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 4/4] irqchip/gic: Add save/restore of the active state
  2015-11-16 19:13 ` Marc Zyngier
@ 2015-11-16 19:13   ` Marc Zyngier
  -1 siblings, 0 replies; 13+ messages in thread
From: Marc Zyngier @ 2015-11-16 19:13 UTC (permalink / raw)
  To: Thomas Gleixner, Jason Cooper, Russell King
  Cc: linux-arm-kernel, linux-kernel

When using EOImode==1, we may mark interrupts as being forwarded
to a virtual machine. In that case, the interrupt is left active
while being passed to the VM.

If we suspend the system before the VM has deactivated the interrupt,
the active state will be lost (which may be very annoying, as this
may result in spurious interrupts and a confused guest).

To avoid this, save and restore the active state together with the
rest of the GIC registers.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 drivers/irqchip/irq-gic.c | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index bc846e7..abf2ffa 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -73,9 +73,11 @@ struct gic_chip_data {
 	union gic_base cpu_base;
 #ifdef CONFIG_CPU_PM
 	u32 saved_spi_enable[DIV_ROUND_UP(1020, 32)];
+	u32 saved_spi_active[DIV_ROUND_UP(1020, 32)];
 	u32 saved_spi_conf[DIV_ROUND_UP(1020, 16)];
 	u32 saved_spi_target[DIV_ROUND_UP(1020, 4)];
 	u32 __percpu *saved_ppi_enable;
+	u32 __percpu *saved_ppi_active;
 	u32 __percpu *saved_ppi_conf;
 #endif
 	struct irq_domain *domain;
@@ -566,6 +568,10 @@ static void gic_dist_save(unsigned int gic_nr)
 	for (i = 0; i < DIV_ROUND_UP(gic_irqs, 32); i++)
 		gic_data[gic_nr].saved_spi_enable[i] =
 			readl_relaxed(dist_base + GIC_DIST_ENABLE_SET + i * 4);
+
+	for (i = 0; i < DIV_ROUND_UP(gic_irqs, 32); i++)
+		gic_data[gic_nr].saved_spi_active[i] =
+			readl_relaxed(dist_base + GIC_DIST_ACTIVE_SET + i * 4);
 }
 
 /*
@@ -611,6 +617,13 @@ static void gic_dist_restore(unsigned int gic_nr)
 			dist_base + GIC_DIST_ENABLE_SET + i * 4);
 	}
 
+	for (i = 0; i < DIV_ROUND_UP(gic_irqs, 32); i++) {
+		writel_relaxed(GICD_INT_EN_CLR_X32,
+			dist_base + GIC_DIST_ACTIVE_CLEAR + i * 4);
+		writel_relaxed(gic_data[gic_nr].saved_spi_active[i],
+			dist_base + GIC_DIST_ACTIVE_SET + i * 4);
+	}
+
 	writel_relaxed(GICD_ENABLE, dist_base + GIC_DIST_CTRL);
 }
 
@@ -634,6 +647,10 @@ static void gic_cpu_save(unsigned int gic_nr)
 	for (i = 0; i < DIV_ROUND_UP(32, 32); i++)
 		ptr[i] = readl_relaxed(dist_base + GIC_DIST_ENABLE_SET + i * 4);
 
+	ptr = raw_cpu_ptr(gic_data[gic_nr].saved_ppi_active);
+	for (i = 0; i < DIV_ROUND_UP(32, 32); i++)
+		ptr[i] = readl_relaxed(dist_base + GIC_DIST_ACTIVE_SET + i * 4);
+
 	ptr = raw_cpu_ptr(gic_data[gic_nr].saved_ppi_conf);
 	for (i = 0; i < DIV_ROUND_UP(32, 16); i++)
 		ptr[i] = readl_relaxed(dist_base + GIC_DIST_CONFIG + i * 4);
@@ -663,6 +680,13 @@ static void gic_cpu_restore(unsigned int gic_nr)
 		writel_relaxed(ptr[i], dist_base + GIC_DIST_ENABLE_SET + i * 4);
 	}
 
+	ptr = raw_cpu_ptr(gic_data[gic_nr].saved_ppi_active);
+	for (i = 0; i < DIV_ROUND_UP(32, 32); i++) {
+		writel_relaxed(GICD_INT_EN_CLR_X32,
+			       dist_base + GIC_DIST_ACTIVE_CLEAR + i * 4);
+		writel_relaxed(ptr[i], dist_base + GIC_DIST_ACTIVE_SET + i * 4);
+	}
+
 	ptr = raw_cpu_ptr(gic_data[gic_nr].saved_ppi_conf);
 	for (i = 0; i < DIV_ROUND_UP(32, 16); i++)
 		writel_relaxed(ptr[i], dist_base + GIC_DIST_CONFIG + i * 4);
@@ -716,6 +740,10 @@ static void __init gic_pm_init(struct gic_chip_data *gic)
 		sizeof(u32));
 	BUG_ON(!gic->saved_ppi_enable);
 
+	gic->saved_ppi_active = __alloc_percpu(DIV_ROUND_UP(32, 32) * 4,
+		sizeof(u32));
+	BUG_ON(!gic->saved_ppi_active);
+
 	gic->saved_ppi_conf = __alloc_percpu(DIV_ROUND_UP(32, 16) * 4,
 		sizeof(u32));
 	BUG_ON(!gic->saved_ppi_conf);
-- 
2.1.4


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 4/4] irqchip/gic: Add save/restore of the active state
@ 2015-11-16 19:13   ` Marc Zyngier
  0 siblings, 0 replies; 13+ messages in thread
From: Marc Zyngier @ 2015-11-16 19:13 UTC (permalink / raw)
  To: linux-arm-kernel

When using EOImode==1, we may mark interrupts as being forwarded
to a virtual machine. In that case, the interrupt is left active
while being passed to the VM.

If we suspend the system before the VM has deactivated the interrupt,
the active state will be lost (which may be very annoying, as this
may result in spurious interrupts and a confused guest).

To avoid this, save and restore the active state together with the
rest of the GIC registers.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 drivers/irqchip/irq-gic.c | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index bc846e7..abf2ffa 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -73,9 +73,11 @@ struct gic_chip_data {
 	union gic_base cpu_base;
 #ifdef CONFIG_CPU_PM
 	u32 saved_spi_enable[DIV_ROUND_UP(1020, 32)];
+	u32 saved_spi_active[DIV_ROUND_UP(1020, 32)];
 	u32 saved_spi_conf[DIV_ROUND_UP(1020, 16)];
 	u32 saved_spi_target[DIV_ROUND_UP(1020, 4)];
 	u32 __percpu *saved_ppi_enable;
+	u32 __percpu *saved_ppi_active;
 	u32 __percpu *saved_ppi_conf;
 #endif
 	struct irq_domain *domain;
@@ -566,6 +568,10 @@ static void gic_dist_save(unsigned int gic_nr)
 	for (i = 0; i < DIV_ROUND_UP(gic_irqs, 32); i++)
 		gic_data[gic_nr].saved_spi_enable[i] =
 			readl_relaxed(dist_base + GIC_DIST_ENABLE_SET + i * 4);
+
+	for (i = 0; i < DIV_ROUND_UP(gic_irqs, 32); i++)
+		gic_data[gic_nr].saved_spi_active[i] =
+			readl_relaxed(dist_base + GIC_DIST_ACTIVE_SET + i * 4);
 }
 
 /*
@@ -611,6 +617,13 @@ static void gic_dist_restore(unsigned int gic_nr)
 			dist_base + GIC_DIST_ENABLE_SET + i * 4);
 	}
 
+	for (i = 0; i < DIV_ROUND_UP(gic_irqs, 32); i++) {
+		writel_relaxed(GICD_INT_EN_CLR_X32,
+			dist_base + GIC_DIST_ACTIVE_CLEAR + i * 4);
+		writel_relaxed(gic_data[gic_nr].saved_spi_active[i],
+			dist_base + GIC_DIST_ACTIVE_SET + i * 4);
+	}
+
 	writel_relaxed(GICD_ENABLE, dist_base + GIC_DIST_CTRL);
 }
 
@@ -634,6 +647,10 @@ static void gic_cpu_save(unsigned int gic_nr)
 	for (i = 0; i < DIV_ROUND_UP(32, 32); i++)
 		ptr[i] = readl_relaxed(dist_base + GIC_DIST_ENABLE_SET + i * 4);
 
+	ptr = raw_cpu_ptr(gic_data[gic_nr].saved_ppi_active);
+	for (i = 0; i < DIV_ROUND_UP(32, 32); i++)
+		ptr[i] = readl_relaxed(dist_base + GIC_DIST_ACTIVE_SET + i * 4);
+
 	ptr = raw_cpu_ptr(gic_data[gic_nr].saved_ppi_conf);
 	for (i = 0; i < DIV_ROUND_UP(32, 16); i++)
 		ptr[i] = readl_relaxed(dist_base + GIC_DIST_CONFIG + i * 4);
@@ -663,6 +680,13 @@ static void gic_cpu_restore(unsigned int gic_nr)
 		writel_relaxed(ptr[i], dist_base + GIC_DIST_ENABLE_SET + i * 4);
 	}
 
+	ptr = raw_cpu_ptr(gic_data[gic_nr].saved_ppi_active);
+	for (i = 0; i < DIV_ROUND_UP(32, 32); i++) {
+		writel_relaxed(GICD_INT_EN_CLR_X32,
+			       dist_base + GIC_DIST_ACTIVE_CLEAR + i * 4);
+		writel_relaxed(ptr[i], dist_base + GIC_DIST_ACTIVE_SET + i * 4);
+	}
+
 	ptr = raw_cpu_ptr(gic_data[gic_nr].saved_ppi_conf);
 	for (i = 0; i < DIV_ROUND_UP(32, 16); i++)
 		writel_relaxed(ptr[i], dist_base + GIC_DIST_CONFIG + i * 4);
@@ -716,6 +740,10 @@ static void __init gic_pm_init(struct gic_chip_data *gic)
 		sizeof(u32));
 	BUG_ON(!gic->saved_ppi_enable);
 
+	gic->saved_ppi_active = __alloc_percpu(DIV_ROUND_UP(32, 32) * 4,
+		sizeof(u32));
+	BUG_ON(!gic->saved_ppi_active);
+
 	gic->saved_ppi_conf = __alloc_percpu(DIV_ROUND_UP(32, 16) * 4,
 		sizeof(u32));
 	BUG_ON(!gic->saved_ppi_conf);
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [tip:irq/urgent] irqchip/gic: Make sure all interrupts are deactivated at boot
  2015-11-16 19:13   ` Marc Zyngier
  (?)
@ 2015-11-17 13:30   ` tip-bot for Marc Zyngier
  -1 siblings, 0 replies; 13+ messages in thread
From: tip-bot for Marc Zyngier @ 2015-11-17 13:30 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: hpa, linux-kernel, jason, linux, mingo, tglx, marc.zyngier,
	linux-arm-kernel

Commit-ID:  0eece2b22849c90b730815c893425a36b9d10fd5
Gitweb:     http://git.kernel.org/tip/0eece2b22849c90b730815c893425a36b9d10fd5
Author:     Marc Zyngier <marc.zyngier@arm.com>
AuthorDate: Mon, 16 Nov 2015 19:13:26 +0000
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Tue, 17 Nov 2015 14:25:58 +0100

irqchip/gic: Make sure all interrupts are deactivated at boot

When booting a GIC/GICv3 based system, we have no idea what
state the firmware (or previous kernel in the case of kexec)
has left the GIC, and some interrupts may still be active.

In order to garantee that we have a clean state, make sure
the active bits are cleared at init time.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
Cc: <linux-arm-kernel@lists.infradead.org>
Cc: Jason Cooper <jason@lakedaemon.net>
Cc: Russell King <linux@arm.linux.org.uk>
Link: http://lkml.kernel.org/r/1447701208-18150-3-git-send-email-marc.zyngier@arm.com
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 drivers/irqchip/irq-gic-common.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/irqchip/irq-gic-common.c b/drivers/irqchip/irq-gic-common.c
index 44a077f..f174ce0 100644
--- a/drivers/irqchip/irq-gic-common.c
+++ b/drivers/irqchip/irq-gic-common.c
@@ -84,12 +84,15 @@ void __init gic_dist_config(void __iomem *base, int gic_irqs,
 		writel_relaxed(GICD_INT_DEF_PRI_X4, base + GIC_DIST_PRI + i);
 
 	/*
-	 * Disable all interrupts.  Leave the PPI and SGIs alone
-	 * as they are enabled by redistributor registers.
+	 * Deactivate and disable all SPIs. Leave the PPI and SGIs
+	 * alone as they are in the redistributor registers on GICv3.
 	 */
-	for (i = 32; i < gic_irqs; i += 32)
+	for (i = 32; i < gic_irqs; i += 32) {
 		writel_relaxed(GICD_INT_EN_CLR_X32,
-					base + GIC_DIST_ENABLE_CLEAR + i / 8);
+			       base + GIC_DIST_ACTIVE_CLEAR + i / 8);
+		writel_relaxed(GICD_INT_EN_CLR_X32,
+			       base + GIC_DIST_ENABLE_CLEAR + i / 8);
+	}
 
 	if (sync_access)
 		sync_access();
@@ -102,7 +105,9 @@ void gic_cpu_config(void __iomem *base, void (*sync_access)(void))
 	/*
 	 * Deal with the banked PPI and SGI interrupts - disable all
 	 * PPI interrupts, ensure all SGI interrupts are enabled.
+	 * Make sure everything is deactivated.
 	 */
+	writel_relaxed(GICD_INT_EN_CLR_X32, base + GIC_DIST_ACTIVE_CLEAR);
 	writel_relaxed(GICD_INT_EN_CLR_PPI, base + GIC_DIST_ENABLE_CLEAR);
 	writel_relaxed(GICD_INT_EN_SET_SGI, base + GIC_DIST_ENABLE_SET);
 

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [tip:irq/urgent] irqchip/gic: Clear enable bits before restoring them
  2015-11-16 19:13   ` Marc Zyngier
  (?)
@ 2015-11-17 13:30   ` tip-bot for Marc Zyngier
  -1 siblings, 0 replies; 13+ messages in thread
From: tip-bot for Marc Zyngier @ 2015-11-17 13:30 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: tglx, linux, linux-kernel, linux-arm-kernel, marc.zyngier, mingo,
	jason, hpa

Commit-ID:  92eda4ad4371225d6ccf9cded74315547e9a3153
Gitweb:     http://git.kernel.org/tip/92eda4ad4371225d6ccf9cded74315547e9a3153
Author:     Marc Zyngier <marc.zyngier@arm.com>
AuthorDate: Mon, 16 Nov 2015 19:13:27 +0000
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Tue, 17 Nov 2015 14:25:58 +0100

irqchip/gic: Clear enable bits before restoring them

When restoring the GIC state (after a suspend/resume cycle,
for example), the driver directly writes the 'enabled' state
it has saved by accessing GICD_ISENABLERn, which performs
an OR operation between the value present in the register
and the value we write.

If whatever code that has run before we reentered the kernel
has enabled an interrupt that was previously disabled, we won't
restore that disabled state.

Making sure we first clear the register (by writting to
GICD_ICENABLERn) before restoring the enabled state.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
Cc: <linux-arm-kernel@lists.infradead.org>
Cc: Jason Cooper <jason@lakedaemon.net>
Cc: Russell King <linux@arm.linux.org.uk>
Link: http://lkml.kernel.org/r/1447701208-18150-4-git-send-email-marc.zyngier@arm.com
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 drivers/irqchip/irq-gic.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index 515c823..bc846e7 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -604,9 +604,12 @@ static void gic_dist_restore(unsigned int gic_nr)
 		writel_relaxed(gic_data[gic_nr].saved_spi_target[i],
 			dist_base + GIC_DIST_TARGET + i * 4);
 
-	for (i = 0; i < DIV_ROUND_UP(gic_irqs, 32); i++)
+	for (i = 0; i < DIV_ROUND_UP(gic_irqs, 32); i++) {
+		writel_relaxed(GICD_INT_EN_CLR_X32,
+			dist_base + GIC_DIST_ENABLE_CLEAR + i * 4);
 		writel_relaxed(gic_data[gic_nr].saved_spi_enable[i],
 			dist_base + GIC_DIST_ENABLE_SET + i * 4);
+	}
 
 	writel_relaxed(GICD_ENABLE, dist_base + GIC_DIST_CTRL);
 }
@@ -654,8 +657,11 @@ static void gic_cpu_restore(unsigned int gic_nr)
 		return;
 
 	ptr = raw_cpu_ptr(gic_data[gic_nr].saved_ppi_enable);
-	for (i = 0; i < DIV_ROUND_UP(32, 32); i++)
+	for (i = 0; i < DIV_ROUND_UP(32, 32); i++) {
+		writel_relaxed(GICD_INT_EN_CLR_X32,
+			       dist_base + GIC_DIST_ENABLE_CLEAR + i * 4);
 		writel_relaxed(ptr[i], dist_base + GIC_DIST_ENABLE_SET + i * 4);
+	}
 
 	ptr = raw_cpu_ptr(gic_data[gic_nr].saved_ppi_conf);
 	for (i = 0; i < DIV_ROUND_UP(32, 16); i++)

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [tip:irq/urgent] irqchip/gic: Add save/ restore of the active state
  2015-11-16 19:13   ` Marc Zyngier
  (?)
@ 2015-11-17 13:31   ` tip-bot for Marc Zyngier
  -1 siblings, 0 replies; 13+ messages in thread
From: tip-bot for Marc Zyngier @ 2015-11-17 13:31 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: mingo, linux, jason, linux-kernel, tglx, linux-arm-kernel,
	marc.zyngier, hpa

Commit-ID:  1c7d4dd46ee048afe19e1294634df6fa66862519
Gitweb:     http://git.kernel.org/tip/1c7d4dd46ee048afe19e1294634df6fa66862519
Author:     Marc Zyngier <marc.zyngier@arm.com>
AuthorDate: Mon, 16 Nov 2015 19:13:28 +0000
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Tue, 17 Nov 2015 14:25:59 +0100

irqchip/gic: Add save/restore of the active state

When using EOImode==1, we may mark interrupts as being forwarded
to a virtual machine. In that case, the interrupt is left active
while being passed to the VM.

If we suspend the system before the VM has deactivated the interrupt,
the active state will be lost (which may be very annoying, as this
may result in spurious interrupts and a confused guest).

To avoid this, save and restore the active state together with the
rest of the GIC registers.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
Cc: <linux-arm-kernel@lists.infradead.org>
Cc: Jason Cooper <jason@lakedaemon.net>
Cc: Russell King <linux@arm.linux.org.uk>
Link: http://lkml.kernel.org/r/1447701208-18150-5-git-send-email-marc.zyngier@arm.com
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 drivers/irqchip/irq-gic.c | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index bc846e7..abf2ffa 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -73,9 +73,11 @@ struct gic_chip_data {
 	union gic_base cpu_base;
 #ifdef CONFIG_CPU_PM
 	u32 saved_spi_enable[DIV_ROUND_UP(1020, 32)];
+	u32 saved_spi_active[DIV_ROUND_UP(1020, 32)];
 	u32 saved_spi_conf[DIV_ROUND_UP(1020, 16)];
 	u32 saved_spi_target[DIV_ROUND_UP(1020, 4)];
 	u32 __percpu *saved_ppi_enable;
+	u32 __percpu *saved_ppi_active;
 	u32 __percpu *saved_ppi_conf;
 #endif
 	struct irq_domain *domain;
@@ -566,6 +568,10 @@ static void gic_dist_save(unsigned int gic_nr)
 	for (i = 0; i < DIV_ROUND_UP(gic_irqs, 32); i++)
 		gic_data[gic_nr].saved_spi_enable[i] =
 			readl_relaxed(dist_base + GIC_DIST_ENABLE_SET + i * 4);
+
+	for (i = 0; i < DIV_ROUND_UP(gic_irqs, 32); i++)
+		gic_data[gic_nr].saved_spi_active[i] =
+			readl_relaxed(dist_base + GIC_DIST_ACTIVE_SET + i * 4);
 }
 
 /*
@@ -611,6 +617,13 @@ static void gic_dist_restore(unsigned int gic_nr)
 			dist_base + GIC_DIST_ENABLE_SET + i * 4);
 	}
 
+	for (i = 0; i < DIV_ROUND_UP(gic_irqs, 32); i++) {
+		writel_relaxed(GICD_INT_EN_CLR_X32,
+			dist_base + GIC_DIST_ACTIVE_CLEAR + i * 4);
+		writel_relaxed(gic_data[gic_nr].saved_spi_active[i],
+			dist_base + GIC_DIST_ACTIVE_SET + i * 4);
+	}
+
 	writel_relaxed(GICD_ENABLE, dist_base + GIC_DIST_CTRL);
 }
 
@@ -634,6 +647,10 @@ static void gic_cpu_save(unsigned int gic_nr)
 	for (i = 0; i < DIV_ROUND_UP(32, 32); i++)
 		ptr[i] = readl_relaxed(dist_base + GIC_DIST_ENABLE_SET + i * 4);
 
+	ptr = raw_cpu_ptr(gic_data[gic_nr].saved_ppi_active);
+	for (i = 0; i < DIV_ROUND_UP(32, 32); i++)
+		ptr[i] = readl_relaxed(dist_base + GIC_DIST_ACTIVE_SET + i * 4);
+
 	ptr = raw_cpu_ptr(gic_data[gic_nr].saved_ppi_conf);
 	for (i = 0; i < DIV_ROUND_UP(32, 16); i++)
 		ptr[i] = readl_relaxed(dist_base + GIC_DIST_CONFIG + i * 4);
@@ -663,6 +680,13 @@ static void gic_cpu_restore(unsigned int gic_nr)
 		writel_relaxed(ptr[i], dist_base + GIC_DIST_ENABLE_SET + i * 4);
 	}
 
+	ptr = raw_cpu_ptr(gic_data[gic_nr].saved_ppi_active);
+	for (i = 0; i < DIV_ROUND_UP(32, 32); i++) {
+		writel_relaxed(GICD_INT_EN_CLR_X32,
+			       dist_base + GIC_DIST_ACTIVE_CLEAR + i * 4);
+		writel_relaxed(ptr[i], dist_base + GIC_DIST_ACTIVE_SET + i * 4);
+	}
+
 	ptr = raw_cpu_ptr(gic_data[gic_nr].saved_ppi_conf);
 	for (i = 0; i < DIV_ROUND_UP(32, 16); i++)
 		writel_relaxed(ptr[i], dist_base + GIC_DIST_CONFIG + i * 4);
@@ -716,6 +740,10 @@ static void __init gic_pm_init(struct gic_chip_data *gic)
 		sizeof(u32));
 	BUG_ON(!gic->saved_ppi_enable);
 
+	gic->saved_ppi_active = __alloc_percpu(DIV_ROUND_UP(32, 32) * 4,
+		sizeof(u32));
+	BUG_ON(!gic->saved_ppi_active);
+
 	gic->saved_ppi_conf = __alloc_percpu(DIV_ROUND_UP(32, 16) * 4,
 		sizeof(u32));
 	BUG_ON(!gic->saved_ppi_conf);

^ permalink raw reply related	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2015-11-17 13:32 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-11-16 19:13 [PATCH 0/4] irqchip/gic: Deal with the active state across kexec and suspend/resume Marc Zyngier
2015-11-16 19:13 ` Marc Zyngier
2015-11-16 19:13 ` [PATCH 1/4] arm: kexec: Deactivate in-flight interrupts Marc Zyngier
2015-11-16 19:13   ` Marc Zyngier
2015-11-16 19:13 ` [PATCH 2/4] irqchip/gic: Make sure all interrupts are deactivated at boot Marc Zyngier
2015-11-16 19:13   ` Marc Zyngier
2015-11-17 13:30   ` [tip:irq/urgent] " tip-bot for Marc Zyngier
2015-11-16 19:13 ` [PATCH 3/4] irqchip/gic: Clear enable bits before restoring them Marc Zyngier
2015-11-16 19:13   ` Marc Zyngier
2015-11-17 13:30   ` [tip:irq/urgent] " tip-bot for Marc Zyngier
2015-11-16 19:13 ` [PATCH 4/4] irqchip/gic: Add save/restore of the active state Marc Zyngier
2015-11-16 19:13   ` Marc Zyngier
2015-11-17 13:31   ` [tip:irq/urgent] irqchip/gic: Add save/ restore " tip-bot for Marc Zyngier

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.