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: Fri, 9 Oct 2015 16:35:03 -0700 From: Duc Dang <dhdang@....com> To: Thomas Gleixner <tglx@...utronix.de> Cc: Marc Zyngier <marc.zyngier@....com>, Jason Cooper <jason@...edaemon.net>, linux-arm <linux-arm-kernel@...ts.infradead.org>, Linux Kernel Mailing List <linux-kernel@...r.kernel.org>, patches <patches@....com> Subject: Re: arm/arm64: GICv2 driver does not have irq_disable implemented On Fri, Oct 9, 2015 at 3:21 PM, Duc Dang <dhdang@....com> wrote: > On Fri, Oct 9, 2015 at 2:52 PM, Thomas Gleixner <tglx@...utronix.de> wrote: >> On Fri, 9 Oct 2015, Duc Dang wrote: >>> On Fri, Oct 9, 2015 at 10:52 AM, Thomas Gleixner <tglx@...utronix.de> wrote: >>> > On Fri, 9 Oct 2015, Duc Dang wrote: >>> >> In APM ARM64 X-Gene Enet controller driver, we use disable_irq_nosync to >>> >> disable interrupt before calling __napi_schedule to schedule packet handler >>> >> to process the Tx/Rx packets. >>> > >>> > Which is wrong to begin with. Disable the interrupt at the device >>> > level not at the interrupt line level. >>> > >>> We could not disable the interrupt at Enet controller level due to the >>> controller limitation. As you said, using disable_irq_nosync is wrong >>> but it looks like that the only option that we have. >> >> Oh well. >> >>> Do you have any suggestion about different approach that we could try? >> >> Try the patch below and add >> >> irq_set_status_flags(irq, IRQ_DISABLE_UNLAZY); >> >> to your driver before requesting the interrupt. Unset it when freeing >> the interrupt. > > Thanks, Thomas! > > We will try and let you know soon. Hi Thomas, We use irq_set_status_flags(irq, IRQ_DISABLE_UNLAZY) in our X-Gene Ethernet driver along with your patch and interrupt count works as expected. Are you going to commit your patch soon? We will post the patch for our X-Gene Ethernet driver after your patch is available. Thanks, > >> >> Thanks, >> >> tglx >> >> 8<-------------- >> >> Subject: genirq: Add flag to force mask in disable_irq[_nosync]() >> From: Thomas Gleixner <tglx@...utronix.de> >> Date: Fri, 09 Oct 2015 23:28:58 +0200 >> >> If an irq chip does not implement the irq_disable callback, then we >> use a lazy approach for disabling the interrupt. That means that the >> interrupt is marked disabled, but the interrupt line is not >> immediately masked in the interrupt chip. It only becomes masked if >> the interrupt is raised while it's marked disabled. We use this to avoid >> possibly expensive mask/unmask operations for common case operations. >> >> Unfortunately there are devices which do not allow the interrupt to be >> disabled easily at the device level. They are forced to use >> disable_irq_nosync(). This can result in taking each interrupt twice. >> >> Instead of enforcing the non lazy mode on all interrupts of a irq >> chip, provide a settings flag, which can be set by the driver for that >> particular interrupt line. >> >> Signed-off-by: Thomas Gleixner <tglx@...utronix.de> >> --- >> include/linux/irq.h | 4 +++- >> kernel/irq/chip.c | 9 +++++++++ >> kernel/irq/settings.h | 7 +++++++ >> 3 files changed, 19 insertions(+), 1 deletion(-) >> >> Index: tip/include/linux/irq.h >> =================================================================== >> --- tip.orig/include/linux/irq.h >> +++ tip/include/linux/irq.h >> @@ -72,6 +72,7 @@ enum irqchip_irq_state; >> * IRQ_IS_POLLED - Always polled by another interrupt. Exclude >> * it from the spurious interrupt detection >> * mechanism and from core side polling. >> + * IRQ_DISABLE_UNLAZY - Disable lazy irq disable >> */ >> enum { >> IRQ_TYPE_NONE = 0x00000000, >> @@ -97,13 +98,14 @@ enum { >> IRQ_NOTHREAD = (1 << 16), >> IRQ_PER_CPU_DEVID = (1 << 17), >> IRQ_IS_POLLED = (1 << 18), >> + IRQ_DISABLE_UNLAZY = (1 << 19), >> }; >> >> #define IRQF_MODIFY_MASK \ >> (IRQ_TYPE_SENSE_MASK | IRQ_NOPROBE | IRQ_NOREQUEST | \ >> IRQ_NOAUTOEN | IRQ_MOVE_PCNTXT | IRQ_LEVEL | IRQ_NO_BALANCING | \ >> IRQ_PER_CPU | IRQ_NESTED_THREAD | IRQ_NOTHREAD | IRQ_PER_CPU_DEVID | \ >> - IRQ_IS_POLLED) >> + IRQ_IS_POLLED | IRQ_DISABLE_UNLAZY) >> >> #define IRQ_NO_BALANCING_MASK (IRQ_PER_CPU | IRQ_NO_BALANCING) >> >> Index: tip/kernel/irq/chip.c >> =================================================================== >> --- tip.orig/kernel/irq/chip.c >> +++ tip/kernel/irq/chip.c >> @@ -241,6 +241,13 @@ void irq_enable(struct irq_desc *desc) >> * disabled. If an interrupt happens, then the interrupt flow >> * handler masks the line at the hardware level and marks it >> * pending. >> + * >> + * If the interrupt chip does not implement the irq_disable callback, >> + * a driver can disable the lazy approach for a particular irq line by >> + * calling 'irq_set_status_flags(irq, IRQ_DISABLE_UNLAZY)'. This can be >> + * used for devices which cannot disable the interrupt at the device >> + * level under certain circumstances and have to use >> + * disable_irq[_nosync] instead. >> */ >> void irq_disable(struct irq_desc *desc) >> { >> @@ -248,6 +255,8 @@ void irq_disable(struct irq_desc *desc) >> if (desc->irq_data.chip->irq_disable) { >> desc->irq_data.chip->irq_disable(&desc->irq_data); >> irq_state_set_masked(desc); >> + } else if (irq_settings_disable_unlazy(desc)) { >> + mask_irq(desc); >> } >> } >> >> Index: tip/kernel/irq/settings.h >> =================================================================== >> --- tip.orig/kernel/irq/settings.h >> +++ tip/kernel/irq/settings.h >> @@ -15,6 +15,7 @@ enum { >> _IRQ_NESTED_THREAD = IRQ_NESTED_THREAD, >> _IRQ_PER_CPU_DEVID = IRQ_PER_CPU_DEVID, >> _IRQ_IS_POLLED = IRQ_IS_POLLED, >> + _IRQ_DISABLE_UNLAZY = IRQ_DISABLE_UNLAZY, >> _IRQF_MODIFY_MASK = IRQF_MODIFY_MASK, >> }; >> >> @@ -28,6 +29,7 @@ enum { >> #define IRQ_NESTED_THREAD GOT_YOU_MORON >> #define IRQ_PER_CPU_DEVID GOT_YOU_MORON >> #define IRQ_IS_POLLED GOT_YOU_MORON >> +#define IRQ_DISABLE_UNLAZY GOT_YOU_MORON >> #undef IRQF_MODIFY_MASK >> #define IRQF_MODIFY_MASK GOT_YOU_MORON >> >> @@ -154,3 +156,8 @@ static inline bool irq_settings_is_polle >> { >> return desc->status_use_accessors & _IRQ_IS_POLLED; >> } >> + >> +static inline bool irq_settings_disable_unlazy(struct irq_desc *desc) >> +{ >> + return desc->status_use_accessors & _IRQ_DISABLE_UNLAZY; >> +} > > Regards, > Duc Dang. Duc Dang. -- 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