linux-kernel - Re: [V6,3/9] irqdomain: Don't set type when mapping an IRQ

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 17 Aug 2017 14:34:37 +0100
From:   Marc Zyngier <marc.zyngier@....com>
To:     jeffy <jeffy.chen@...k-chips.com>,
        Jon Hunter <jonathanh@...dia.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Jason Cooper <jason@...edaemon.net>
Cc:     Rob Herring <robh+dt@...nel.org>, Pawel Moll <pawel.moll@....com>,
        Mark Rutland <mark.rutland@....com>,
        Ian Campbell <ijc+devicetree@...lion.org.uk>,
        Kumar Gala <galak@...eaurora.org>,
        Stephen Warren <swarren@...dotorg.org>,
        Thierry Reding <thierry.reding@...il.com>,
        Kevin Hilman <khilman@...nel.org>,
        Geert Uytterhoeven <geert@...ux-m68k.org>,
        Grygorii Strashko <grygorii.strashko@...com>,
        Lars-Peter Clausen <lars@...afoo.de>,
        Linus Walleij <linus.walleij@...aro.org>,
        linux-tegra@...r.kernel.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org, briannorris@...omium.com
Subject: Re: [V6,3/9] irqdomain: Don't set type when mapping an IRQ

On 17/08/17 13:46, jeffy wrote:
> Hi guys,
> 
> I hit an problem when testing a level triggered irq with:
> 
> irq = platform_get_irq_byname(pdev, "wake");
> ...<-- irq trigger type is correct
> irq_set_status_flags(irq, IRQ_NOAUTOEN);
> ...<-- irq trigger type become zero
> request_threaded_irq(irq, ...)
> 
> the trigger type lost(irqd_get_trigger_type returns zero) after 
> irq_set_status_flags.
> 
> 
> it looks like irq_set_status_flags would try to use 
> irq_settings_is_level to get level trigger type, which would get zero 
> since we removed set type here...could you help to check this, thanks :)

Hmm. That's quite unexpected. Can you please try this?

diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
index a3cc37c0c85e..3675c6004f2a 100644
--- a/kernel/irq/chip.c
+++ b/kernel/irq/chip.c
@@ -1000,7 +1000,7 @@ EXPORT_SYMBOL_GPL(irq_set_chip_and_handler_name);
 
 void irq_modify_status(unsigned int irq, unsigned long clr, unsigned long set)
 {
-	unsigned long flags;
+	unsigned long flags, trigger, tmp;
 	struct irq_desc *desc = irq_get_desc_lock(irq, &flags, 0);
 
 	if (!desc)
@@ -1014,6 +1014,8 @@ void irq_modify_status(unsigned int irq, unsigned long clr, unsigned long set)
 
 	irq_settings_clr_and_set(desc, clr, set);
 
+	trigger = irqd_get_trigger_type(&desc->irq_data);
+
 	irqd_clear(&desc->irq_data, IRQD_NO_BALANCING | IRQD_PER_CPU |
 		   IRQD_TRIGGER_MASK | IRQD_LEVEL | IRQD_MOVE_PCNTXT);
 	if (irq_settings_has_no_balance_set(desc))
@@ -1025,7 +1027,11 @@ void irq_modify_status(unsigned int irq, unsigned long clr, unsigned long set)
 	if (irq_settings_is_level(desc))
 		irqd_set(&desc->irq_data, IRQD_LEVEL);
 
-	irqd_set(&desc->irq_data, irq_settings_get_trigger_mask(desc));
+	tmp = irq_settings_get_trigger_mask(desc);
+	if (tmp != IRQ_TYPE_NONE)
+		trigger = tmp;
+
+	irqd_set(&desc->irq_data, trigger);
 
 	irq_put_desc_unlock(desc, flags);
 }

Basically, save the trigger info before clearing it. If the interrupt
has already been configured, use that. Otherwise restore the previous
trigger settings. But I'm slightly confused as to which field is used
for what...

	M.
-- 
Jazz is not dead. It just smells funny...