| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Sat, 28 Apr 2012 10:13:45 +0200 (CEST)
From: Thomas Gleixner <tglx@...utronix.de>
To: Jean-Francois Dagenais <jeff.dagenais@...il.com>
cc: Grant Likely <grant.likely@...retlab.ca>,
open list <linux-kernel@...r.kernel.org>,
alexander.stein@...tec-electronic.com, qi.wang@...el.com,
yong.y.wang@...el.com, joel.clark@...el.com,
kok.howg.ewe@...el.com, tomoya.rohm@...il.com
Subject: [PATCH] gpio: pch9: Use proper flow type handlers
Jean-Francois Dagenais reported:
Configuring a gpio pin with the gpio-pch driver with
"IRQF_TRIGGER_LOW | IRQF_ONESHOT" generates an interrupt storm for
threaded ISR until the ISR thread actually gets to physically clear
the interrupt on the triggering chip!! The immediate observable
symptom is the high CPU usage for my ISR thread task and the
interrupt count in /proc/interrupts incrementing radically.
The driver is wrong in several ways:
1) Using handle_simple_irq() does not provide proper flow control
handling. In the case of oneshot threaded handlers for the
demultiplexed interrupts this results in an interrupt storm because
the simple handler does not deal with masking/unmasking. Even
without threaded oneshot handlers an interrupt storm for level type
interrupts can easily be triggered when the interrupt is disabled
and the interrupt line is activated from the device.
2) Acknowlegding the demultiplexed interrupt before calling the
handler is wrong for level type interrupts.
3) The set_type function unconditionally enables the interrupt. It's
supposed to set the type and nothing else. The unmasking is done by
the core code.
Move the acknowledge code into a separate function and add it to the
demux irqchip callbacks.
Remove the unconditional enabling from the set_type() callback and set
the proper flow handlers depending on the selected type (level/edge).
Reported-and-tested-by: Jean-Francois Dagenais <jeff.dagenais@...il.com>
Signed-off-by: Thomas Gleixner <tglx@...utronix.de>
Cc: Tomoya MORINAGA <tomoya-linux@....okisemi.com>
Cc: Grant Likely <grant.likely@...retlab.ca>
Cc: alexander.stein@...tec-electronic.com
Cc: qi.wang@...el.com
Cc: yong.y.wang@...el.com
Cc: joel.clark@...el.com
Cc: kok.howg.ewe@...el.com
Cc: toshiharu-linux@....okisemi.com
Link: http://lkml.kernel.org/r/alpine.LFD.2.02.1204252332070.2542@ionos
---
drivers/gpio/gpio-pch.c | 57 +++++++++++++++++++++++-------------------------
1 file changed, 28 insertions(+), 29 deletions(-)
Index: linux-2.6/drivers/gpio/gpio-pch.c
===================================================================
--- linux-2.6.orig/drivers/gpio/gpio-pch.c
+++ linux-2.6/drivers/gpio/gpio-pch.c
@@ -230,16 +230,12 @@ static void pch_gpio_setup(struct pch_gp
static int pch_irq_type(struct irq_data *d, unsigned int type)
{
- u32 im;
- u32 __iomem *im_reg;
- u32 ien;
- u32 im_pos;
- int ch;
- unsigned long flags;
- u32 val;
- int irq = d->irq;
struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
struct pch_gpio *chip = gc->private;
+ u32 im, im_pos, val;
+ u32 __iomem *im_reg;
+ unsigned long flags;
+ int ch, irq = d->irq;
ch = irq - chip->irq_base;
if (irq <= chip->irq_base + 7) {
@@ -270,30 +266,22 @@ static int pch_irq_type(struct irq_data
case IRQ_TYPE_LEVEL_LOW:
val = PCH_LEVEL_L;
break;
- case IRQ_TYPE_PROBE:
- goto end;
default:
- dev_warn(chip->dev, "%s: unknown type(%dd)",
- __func__, type);
- goto end;
+ goto unlock;
}
/* Set interrupt mode */
im = ioread32(im_reg) & ~(PCH_IM_MASK << (im_pos * 4));
iowrite32(im | (val << (im_pos * 4)), im_reg);
- /* iclr */
- iowrite32(BIT(ch), &chip->reg->iclr);
+ /* And the handler */
+ if (type & (IRQ_TYPE_LEVEL_LOW | IRQ_TYPE_LEVEL_HIGH))
+ __irq_set_handler_locked(d->irq, handle_level_irq);
+ else if (type & (IRQ_TYPE_EDGE_FALLING | IRQ_TYPE_EDGE_RISING))
+ __irq_set_handler_locked(d->irq, handle_edge_irq);
- /* IMASKCLR */
- iowrite32(BIT(ch), &chip->reg->imaskclr);
-
- /* Enable interrupt */
- ien = ioread32(&chip->reg->ien);
- iowrite32(ien | BIT(ch), &chip->reg->ien);
-end:
+unlock:
spin_unlock_irqrestore(&chip->spinlock, flags);
-
return 0;
}
@@ -313,18 +301,24 @@ static void pch_irq_mask(struct irq_data
iowrite32(1 << (d->irq - chip->irq_base), &chip->reg->imask);
}
+static void pch_irq_ack(struct irq_data *d)
+{
+ struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
+ struct pch_gpio *chip = gc->private;
+
+ iowrite32(1 << (d->irq - chip->irq_base), &chip->reg->iclr);
+}
+
static irqreturn_t pch_gpio_handler(int irq, void *dev_id)
{
struct pch_gpio *chip = dev_id;
u32 reg_val = ioread32(&chip->reg->istatus);
- int i;
- int ret = IRQ_NONE;
+ int i, ret = IRQ_NONE;
for (i = 0; i < gpio_pins[chip->ioh]; i++) {
if (reg_val & BIT(i)) {
dev_dbg(chip->dev, "%s:[%d]:irq=%d status=0x%x\n",
__func__, i, irq, reg_val);
- iowrite32(BIT(i), &chip->reg->iclr);
generic_handle_irq(chip->irq_base + i);
ret = IRQ_HANDLED;
}
@@ -343,6 +337,7 @@ static __devinit void pch_gpio_alloc_gen
gc->private = chip;
ct = gc->chip_types;
+ ct->chip.irq_ack = pch_irq_ack;
ct->chip.irq_mask = pch_irq_mask;
ct->chip.irq_unmask = pch_irq_unmask;
ct->chip.irq_set_type = pch_irq_type;
@@ -357,6 +352,7 @@ static int __devinit pch_gpio_probe(stru
s32 ret;
struct pch_gpio *chip;
int irq_base;
+ u32 msk;
chip = kzalloc(sizeof(*chip), GFP_KERNEL);
if (chip == NULL)
@@ -408,8 +404,13 @@ static int __devinit pch_gpio_probe(stru
}
chip->irq_base = irq_base;
+ /* Mask all interrupts, but enable them */
+ msk = (1 << gpio_pins[chip->ioh]) - 1;
+ iowrite32(msk, &chip->reg->imask);
+ iowrite32(msk, &chip->reg->ien);
+
ret = request_irq(pdev->irq, pch_gpio_handler,
- IRQF_SHARED, KBUILD_MODNAME, chip);
+ IRQF_SHARED, KBUILD_MODNAME, chip);
if (ret != 0) {
dev_err(&pdev->dev,
"%s request_irq failed\n", __func__);
@@ -418,8 +419,6 @@ static int __devinit pch_gpio_probe(stru
pch_gpio_alloc_generic_chip(chip, irq_base, gpio_pins[chip->ioh]);
- /* Initialize interrupt ien register */
- iowrite32(0, &chip->reg->ien);
end:
return 0;
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists