[RFC PATCH 0/2] irqchip: GIC: check and clear GIC interupt active status

[RFC PATCH 0/2] irqchip: GIC: check and clear GIC interupt active status

[Liu Hua]

For this version of GIC codes, kernel assumes that all the interrupt
status of GIC is inactive. So the kernel does not check this when 
booting.

This is no problem on must sitations. But when kdump is deplayed.
And a panic occurs when a interrupt is being handled (may be PPI 
and SPI). We have no chance to write relative bit to GICC_EOIR.
So this interrupt remains active. And GIC will not deliver this
type interrupt to cpu interface. And the capture kernel may 
fail to boot becase of lacking of certain interrupt (such as timer 
interupt).


I glanced over the GIC Architecture Specification, but did not 
find a simple way to deactive state of all interrupts. For GICv1,
I can only deal with one abnormal interrupt state one time. For 
GICv2, I can deactive 32 one time.


So guys, Do you know a better way to do that? 


Liu Hua (2):
  irqchip: gic: introduce ICPIDR2 register interface
  irqchip: GIC: introduce method to deactive interupts

 drivers/irqchip/irq-gic.c       | 57 +++++++++++++++++++++++++++++++++++++++++
 include/linux/irqchip/arm-gic.h |  1 +
 2 files changed, 58 insertions(+)

-- 
1.9.0

[RFC PATCH 1/2] irqchip: GIC: introduce ICPIDR2 register interface

[Liu Hua]

Peripheral ID2 Register provides a four-bit architecturally-defined
architecture revision field. So we can identify the GIC verison from
this register. It is useful sometimes.

Signed-off-by: Liu Hua <sdu.liu@huawei.com>
---
 include/linux/irqchip/arm-gic.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/linux/irqchip/arm-gic.h b/include/linux/irqchip/arm-gic.h
index 45e2d8c..872a562 100644
--- a/include/linux/irqchip/arm-gic.h
+++ b/include/linux/irqchip/arm-gic.h
@@ -38,6 +38,7 @@
 #define GIC_DIST_SOFTINT		0xf00
 #define GIC_DIST_SGI_PENDING_CLEAR	0xf10
 #define GIC_DIST_SGI_PENDING_SET	0xf20
+#define GIC_DIST_ICPIDR2		0xfe8
 
 #define GICH_HCR			0x0
 #define GICH_VTR			0x4
-- 
1.9.0

[RFC PATCH 2/2] irqchip: GIC: introduce method to deactive interupts

[Liu Hua]

When using kdump on ARM platform, if kernel panics in interrupt handler
(maybe PPI or SPI), the capture kernel can not recive certain interrupt,
and fails to boot.

On this situation, We have read register GICC_IAR. But we have no chance
to write relative bit to register GICC_EOIR(kernel paniced before). So
the state of this type interrupt remains active. Ant that makes gic not
deliver this type interrupt to cpu interface.

So we should not assume that all interruts state of GIC is inactive when
kernel initailize the GIC. This patch identifies this type interrupts
and deactives them;

Signed-off-by: Liu Hua <sdu.liu@huawei.com>
---
 drivers/irqchip/irq-gic.c | 57 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 57 insertions(+)

diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index ddee133..d8620cf 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -352,12 +352,68 @@ static u8 gic_get_cpumask(struct gic_chip_data *gic)
 	return mask;
 }
 
+void gic_v1_eois(u32 active, void __iomem *cpu_base)
+{
+	int bit = -1;
+
+	while ((bit = find_next_bit((unsigned long *) &active,
+						32, bit + 1)) < 32) {
+		writel_relaxed(bit, cpu_base + GIC_CPU_EOI);
+	}
+}
+
+void gic_v1_clear_active(void __iomem *dist_base,
+			void __iomem *cpu_base, int gic_irqs)
+{
+	int irq, offset;
+	u32 active;
+
+	for (irq = 0; irq < gic_irqs; irq += 32) {
+		offset = GIC_DIST_ACTIVE_SET + irq * 4 / 32;
+		active = readl_relaxed(dist_base + offset);
+		if (!active)
+			continue;
+		gic_v1_eois(active, cpu_base);
+	}
+}
+
+void gic_v2_clear_active(void __iomem *dist_base, int gic_irqs)
+{
+	int irq, offset;
+	u32 active;
+
+	for (irq = 0; irq < gic_irqs; irq += 32) {
+		offset = irq * 4 / 32 + GIC_DIST_ACTIVE_SET;
+		active = readl_relaxed(dist_base + offset);
+		if (!active)
+			continue;
+		offset = irq * 4 / 32 + GIC_DIST_ACTIVE_CLEAR;
+		writel_relaxed(active, dist_base + offset);
+	}
+}
+
+void __init gic_dist_clear_active(void __iomem *dist_base,
+					void __iomem *cpu_base, int gic_irqs)
+{
+	u32 ArchRev;
+
+	ArchRev = readl_relaxed(dist_base + GIC_DIST_ICPIDR2);
+	ArchRev = (ArchRev >> 4) & 0xF;
+
+	if (ArchRev == 0x1) {
+		gic_v1_clear_active(dist_base, cpu_base, gic_irqs);
+	} else {
+		gic_v2_clear_active(dist_base, gic_irqs);
+	}
+}
+
 static void __init gic_dist_init(struct gic_chip_data *gic)
 {
 	unsigned int i;
 	u32 cpumask;
 	unsigned int gic_irqs = gic->gic_irqs;
 	void __iomem *base = gic_data_dist_base(gic);
+	void __iomem *cpu_base = gic_data_cpu_base(gic);
 
 	writel_relaxed(0, base + GIC_DIST_CTRL);
 
@@ -371,6 +427,7 @@ static void __init gic_dist_init(struct gic_chip_data *gic)
 		writel_relaxed(cpumask, base + GIC_DIST_TARGET + i * 4 / 4);
 
 	gic_dist_config(base, gic_irqs, NULL);
+	gic_dist_clear_active(base, cpu_base, gic_irqs);
 
 	writel_relaxed(1, base + GIC_DIST_CTRL);
 }
-- 
1.9.0

Re: [RFC PATCH 0/2] irqchip: GIC: check and clear GIC interupt active status

[Will Deacon]

[adding Marc]

On Fri, Jul 11, 2014 at 07:46:15AM +0100, Liu Hua wrote:
> For this version of GIC codes, kernel assumes that all the interrupt
> status of GIC is inactive. So the kernel does not check this when 
> booting.
> 
> This is no problem on must sitations. But when kdump is deplayed.
> And a panic occurs when a interrupt is being handled (may be PPI 
> and SPI). We have no chance to write relative bit to GICC_EOIR.
> So this interrupt remains active. And GIC will not deliver this
> type interrupt to cpu interface. And the capture kernel may 
> fail to boot becase of lacking of certain interrupt (such as timer 
> interupt).
> 
> 
> I glanced over the GIC Architecture Specification, but did not 
> find a simple way to deactive state of all interrupts. For GICv1,
> I can only deal with one abnormal interrupt state one time. For 
> GICv2, I can deactive 32 one time.
> 
> 
> So guys, Do you know a better way to do that? 

What happens if, in the crash kernel, you disable the CPU interfaces
(GICC_CTLR.ENABLE) then disable the distributor (GICD_CTLR.ENABLE) before
enabling everything again in the reverse order? Is that enough to cause the
GIC to drop any active states? It's not clear to me from a quick look at
the TRM.

Will

Re: [RFC PATCH 0/2] irqchip: GIC: check and clear GIC interupt active status

[Liu hua]

On 2014/7/11 20:35, Will Deacon wrote:
> [adding Marc]
> 
> On Fri, Jul 11, 2014 at 07:46:15AM +0100, Liu Hua wrote:
>> For this version of GIC codes, kernel assumes that all the interrupt
>> status of GIC is inactive. So the kernel does not check this when 
>> booting.
>>
>> This is no problem on must sitations. But when kdump is deplayed.
>> And a panic occurs when a interrupt is being handled (may be PPI 
>> and SPI). We have no chance to write relative bit to GICC_EOIR.
>> So this interrupt remains active. And GIC will not deliver this
>> type interrupt to cpu interface. And the capture kernel may 
>> fail to boot becase of lacking of certain interrupt (such as timer 
>> interupt).
>>
>>
>> I glanced over the GIC Architecture Specification, but did not 
>> find a simple way to deactive state of all interrupts. For GICv1,
>> I can only deal with one abnormal interrupt state one time. For 
>> GICv2, I can deactive 32 one time.
>>
>>
>> So guys, Do you know a better way to do that? 
> 
> What happens if, in the crash kernel, you disable the CPU interfaces
> (GICC_CTLR.ENABLE) then disable the distributor (GICD_CTLR.ENABLE) before
> enabling everything again in the reverse order? Is that enough to cause the
> GIC to drop any active states? It's not clear to me from a quick look at
> the TRM.
> 
Hi Will,

Thanks for your reply!

I did what you said at the beginning of "gic_dist_init". The active states
remained (panic in local timer interrupt (PPI))and the kernel failed to boot,
Did I do that at wrong place?

-------------------
diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index b6b0a81..94d6352 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -454,6 +455,7 @@ static void __init gic_dist_init(struct gic_chip_data *gic)
        void __iomem *base = gic_data_dist_base(gic);
        void __iomem *cpu_base = gic_data_cpu_base(gic);

+ writel_relaxed(0, base + GIC_CPU_CTRL);
        writel_relaxed(0, base + GIC_DIST_CTRL);

        /*
------------------------

As shown in GIC Architecture Specification manual,I think that the GICC_CTLR.ENABLE
and GICD_CTLR.ENABLE only control the delivering of the interrupt, not the active
states.

As GIC manual says "For every read of a valid Interrupt ID from the GICC_IAR, the
connected processor must perform a matching write to the GICC_EOIR". So we should
find a way to drop the active states when booting, if we do not remain these active
states by design.

Thanks,
Liu Hua




> Will
> 
> .
> 

Re: [RFC PATCH 0/2] irqchip: GIC: check and clear GIC interupt active status

[Will Deacon]

On Mon, Jul 14, 2014 at 04:22:26AM +0100, Liu hua wrote:
> On 2014/7/11 20:35, Will Deacon wrote:
> > On Fri, Jul 11, 2014 at 07:46:15AM +0100, Liu Hua wrote:
> >> So guys, Do you know a better way to do that? 
> > What happens if, in the crash kernel, you disable the CPU interfaces
> > (GICC_CTLR.ENABLE) then disable the distributor (GICD_CTLR.ENABLE) before
> > enabling everything again in the reverse order? Is that enough to cause the
> > GIC to drop any active states? It's not clear to me from a quick look at
> > the TRM.
> > 
> 
> I did what you said at the beginning of "gic_dist_init". The active states
> remained (panic in local timer interrupt (PPI))and the kernel failed to boot,
> Did I do that at wrong place?
> 
> -------------------
> diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
> index b6b0a81..94d6352 100644
> --- a/drivers/irqchip/irq-gic.c
> +++ b/drivers/irqchip/irq-gic.c
> @@ -454,6 +455,7 @@ static void __init gic_dist_init(struct gic_chip_data *gic)
>         void __iomem *base = gic_data_dist_base(gic);
>         void __iomem *cpu_base = gic_data_cpu_base(gic);
> 
> + writel_relaxed(0, base + GIC_CPU_CTRL);
>         writel_relaxed(0, base + GIC_DIST_CTRL);
> 
>         /*
> ------------------------
> 
> As shown in GIC Architecture Specification manual,I think that the GICC_CTLR.ENABLE
> and GICD_CTLR.ENABLE only control the delivering of the interrupt, not the active
> states.
> 
> As GIC manual says "For every read of a valid Interrupt ID from the GICC_IAR, the
> connected processor must perform a matching write to the GICC_EOIR". So we should
> find a way to drop the active states when booting, if we do not remain these active
> states by design.

Understood, my suggestion above was a speculative effort to see what the
hardware would do. Something along the lines of what you've done in your
patch makes sense to me, but we'll need to wait for Marc's input (he's on
holiday at the moment).

Will