| 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: Tue, 9 Aug 2016 14:20:48 +0100
From: Jon Hunter <jonathanh@...dia.com>
To: John Stultz <john.stultz@...aro.org>
CC: Marc Zyngier <marc.zyngier@....com>,
Thomas Gleixner <tglx@...utronix.de>,
lkml <linux-kernel@...r.kernel.org>,
Bjorn Andersson <bjorn.andersson@...aro.org>
Subject: Re: [Regression] "irqdomain: Don't set type when mapping an IRQ"
breaks nexus7 gpio buttons
On 09/08/16 05:25, John Stultz wrote:
...
> So actually no. We usually call irqd_set_trigger_type() but something
> still doesn't work.
>
> Interestingly, just adding irq_set_irq_type(virq, type); to the top of
> that block (leaving the rest of the code) also works.
Interesting. By saving the trigger type during the mapping, we defer
setting the interrupt type to when the interrupt is requested. So this
would imply that the interrupt is not being setup as expected when its
requested.
>> What is odd, is that the above sequence is only executed if a irq
>> mapping exists and so really, AFAICT this should not happen. Ie. the irq
>> descriptor should have been allocated for the mapping to exist. We
>> should probably warn if this happens.
>>
>> Without reverting the above, can you add a print to show the
>> domain->name, hwirq and virq information if !irq_data? That will confirm
>> the domain for us.
>
> So I put some printk info in (in either case since I'm never seeing
> the !irq_data case happen):
>
> [ 1.514217] JDB: virq: 93 hwirq: 74 domain name: msmgpio
> [ 1.838342] JDB: virq: 25 hwirq: 6 domain name: msmgpio
>
> Which is odd, looking at:
>
> shell@flo:/ $ cat /proc/interrupts
> CPU0 CPU1 CPU2 CPU3
> 16: 1159 1138 1332 1574 GIC-0 18 Edge
> gp_timer
> 25: 0 0 0 0 msmgpio 6 Edge
> ekth3500
> 111: 6 0 0 0 GIC-0 51 Edge
> qcom_rpm_ack
> 112: 0 0 0 0 GIC-0 53 Edge
> qcom_rpm_err
> 113: 0 0 0 0 GIC-0 54 Edge
> qcom_rpm_wakeup
> 114: 48 0 0 0 GIC-0 132 Edge
> msm_otg, ci_hdrc_msm
> 115: 796 0 0 0 GIC-0 130 Level bam_dma
> 116: 0 0 0 0 GIC-0 128 Level bam_dma
> 117: 0 0 0 0 GIC-0 127 Level bam_dma
> 118: 2627 0 0 0 GIC-0 136 Level
> mmci-pl18x (cmd)
> 119: 54 0 0 0 GIC-0 226 Level i2c_qup
> 120: 21 0 0 0 GIC-0 183 Level i2c_qup
> 122: 0 0 0 0 GIC-0 189 Level i2c_qup
> 123: 202 0 0 0 GIC-0 190 Level
> msm_serial0
> 124: 0 0 0 0 GIC-0 70 Edge smsm
> 125: 0 0 0 0 GIC-0 121 Edge smsm
> 126: 0 0 0 0 GIC-0 236 Edge smsm
> 127: 0 0 0 0 GIC-0 169 Edge smsm
> 131: 0 0 0 0 pm8xxx 195 Edge
> Volume Up
> 165: 0 0 0 0 pm8xxx 229 Edge
> Volume Down
> 184: 0 0 0 0 pm8xxx 39 Edge
> pm8xxx_rtc_alarm
> 185: 0 0 0 0 pm8xxx 50 Edge
> pmic8xxx_pwrkey_release
> 186: 0 0 0 0 pm8xxx 51 Edge
> pmic8xxx_pwrkey_press
> IPI0: 0 1 1 1 CPU wakeup interrupts
> IPI1: 0 0 0 0 Timer broadcast interrupts
> IPI2: 944 539 1015 529 Rescheduling interrupts
> IPI3: 1 4 6 4 Function call interrupts
> IPI4: 0 0 0 0 CPU stop interrupts
> IPI5: 0 0 0 0 IRQ work interrupts
> IPI6: 0 0 0 0 completion interrupts
> Err: 0
>
> Since 25 maps to the ekth3500 (touch panel, which is still working
> fine), but 93/74 doesn't seem to map to anything, and the problematic
> irqs are the volume keys 195/229 and power keys 50/51.
So looking at the DT source, I believe that hwirq 74 (virq 93) is the
problem. This is the parent interrupt from the pm8xxx to the apq8064
if it is not requested then the type is not set. It seems that for
parent interrupts these are not typically requested, but enabled when
an irqchip is chained.
To confirm and for testing purposes I am curious if this works ...
if (irq_get_trigger_type(virq) == IRQ_TYPE_NONE) {
irq_data = irq_get_irq_data(virq);
if (!irq_data)
return 0;
- irqd_set_trigger_type(irq_data, type);
+ if (hwirq == 74)
+ irq_set_irq_type(virq, type);
+ else
+ irqd_set_trigger_type(irq_data, type);
return virq;
}
If that works, then does the following also work (without the above) ...
diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
index b4c1bc7c9ca2..e111b72e3162 100644
--- a/kernel/irq/chip.c
+++ b/kernel/irq/chip.c
@@ -824,6 +824,7 @@ __irq_do_set_handler(struct irq_desc *desc, irq_flow_handler_t handle,
irq_settings_set_norequest(desc);
irq_settings_set_nothread(desc);
desc->action = &chained_action;
+ __irq_set_trigger(desc, irqd_get_trigger_type(&desc->irq_data));
irq_startup(desc, true);
}
}
It looks like there is a path for parent interrupts where the type
is not getting set. If the above works then we can discuss with Thomas
and Marc on the correct fix.
Cheers
Jon
--
nvpublic
Powered by blists - more mailing lists