linux-kernel - Re: [PATCH V1 07/15] spmi: pmic-arb: clear the latched status of the interrupt

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:   Tue, 06 Jun 2017 16:25:30 +0530
From:   kgunda@...eaurora.org
To:     Stephen Boyd <sboyd@...eaurora.org>
Cc:     Abhijeet Dharmapurikar <adharmap@...eaurora.org>,
        Christophe JAILLET <christophe.jaillet@...adoo.fr>,
        Subbaraman Narayanamurthy <subbaram@...eaurora.org>,
        David Collins <collinsd@...eaurora.org>,
        linux-kernel@...r.kernel.org, linux-arm-msm@...r.kernel.org,
        adharmap@...cinc.com, aghayal@....qualcomm.com,
        linux-arm-msm-owner@...r.kernel.org
Subject: Re: [PATCH V1 07/15] spmi: pmic-arb: clear the latched status of the
 interrupt

On 2017-06-01 03:33, Stephen Boyd wrote:
> On 05/30, Kiran Gunda wrote:
>> From: Abhijeet Dharmapurikar <adharmap@...eaurora.org>
>> 
>> PMIC interrupts each have an internal latched status bit which is
>> not visible from any register.  This status bit is set as soon as
>> the conditions specified in the interrupt type and polarity
>> registers are met even if the interrupt is not enabled.  When it
>> is set, nothing else changes within the PMIC and no interrupt
>> notification packets are sent.  If the internal latched status
>> bit is set when an interrupt is enabled, then the value is
>> immediately propagated into the interrupt latched status register
>> and an interrupt notification packet is sent out from the PMIC
>> over SPMI.
>> 
>> This PMIC hardware behavior can lead to a situation where the
>> handler for a level triggered interrupt is called immediately
>> after enable_irq() is called even though the interrupt physically
>> triggered while it was disabled within the genirq framework.
>> This situation takes place if the the interrupt fires twice after
> 
> Double 'the'
> 
>> calling disable_irq().  The first time it fires, the level flow
>> handler will mask and disregard it.  Unfortunately, the second
>> time it fires, the internal latched status bit is set within the
>> PMIC and no further notification is received.
> 
> because the interrupt has been disabled.
> 
>> When enable_irq()
>> is called later, the interrupt is unmasked (enabled in the PMIC)
>> which results in the PMIC immediately sending an interrupt
>> notification packet out over SPMI.  This breaks the semantics
>> of level triggered interrupts within the genirq framework since
>> they should be completely ignored while disabled.
> 
> Ok. I wonder why the hardware latches interrupts at all.
> 
>> 
>> The PMIC internal latched status behavior also affects how
>> interrupts are treated during suspend.  While entering suspend,
>> all interrupts not specified as wakeup mode are masked.  Upon
>> resume, these interrupts are unmasked.  Thus if any of the
>> non-wakeup PMIC interrupts fired while the system was suspended,
>> then the PMIC will send interrupt notification packets out via
>> SPMI as soon as they are unmasked during resume.  This behavior
>> violates genirq semantics as well since non-wakeup interrupts
>> should be completely ignored during suspend.
>> 
>> Modify the qpnpint_irq_unmask() function so that the interrupt
>> latched status clear register is written immediately before the
>> interrupt enable register.  This clears the internal latched
>> status bit of the interrupt so that it cannot trigger spuriously
>> immediately upon being enabled.
>> 
>> Also, while resuming an irq, an unmask could be called even if it
>> was not previously masked.  So, before writing these registers,
>> check if the interrupt is already enabled within the PMIC. If it
>> is, then no further register writes are required.  This
>> condition check ensures that a valid latched status register bit
>> is not cleared until it is properly handled.
>> 
>> Signed-off-by: Abhijeet Dharmapurikar <adharmap@...eaurora.org>
>> Signed-off-by: Kiran Gunda <kgunda@...eaurora.org>
> 
> Reviewed-by: Stephen Boyd <sboyd@...eaurora.org>
Thanks for reviewing and acking this.