lkml.org 
[lkml]   [2021]   [Dec]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v2] spmi: spmi-pmic-arb: fix irq_set_type race condition
From
Date
Quoting David Collins (2021-11-17 19:47:19)
> The qpnpint_irq_set_type() callback function configures the type
> (edge vs level) and polarity (high, low, or both) of a particular
> PMIC interrupt within a given peripheral. To do this, it reads
> the three consecutive IRQ configuration registers, modifies the
> specified IRQ bit within the register values, and finally writes
> the three modified register values back to the PMIC. While a
> spinlock is used to provide mutual exclusion on the SPMI bus
> during the register read and write calls, there is no locking
> around the overall read, modify, write sequence. This opens up
> the possibility of a race condition if two tasks set the type of
> a PMIC IRQ within the same peripheral simultaneously.
>
> When the race condition is encountered, both tasks will read the
> old value of the registers and IRQ bits set by one of the tasks
> will be dropped upon the register write of the other task. This
> then leads to PMIC IRQs being enabled with an incorrect type and
> polarity configured. Such misconfiguration can lead to an IRQ
> storm that overwhelms the system and causes it to crash.
>
> This race condition and IRQ storm have been observed when using
> a pair of pm8941-pwrkey devices to handle PMK8350 pwrkey and
> resin interrupts. The independent devices probe asynchronously
> in parallel and can simultaneously request and configure PMIC
> IRQs in the same PMIC peripheral.
>
> For a good case, the IRQ configuration calls end up serialized
> due to timing deltas and the register read/write sequence looks
> like this:
>
> 1. pwrkey probe: SPMI read(0x1311): 0x00, 0x00, 0x00
> 2. pwrkey probe: SPMI write(0x1311): 0x80, 0x80, 0x80
> 3. resin probe: SPMI read(0x1311): 0x80, 0x80, 0x80
> 4. resin probe: SPMI write(0x1311): 0xC0, 0xC0, 0xC0
>
> The final register states after both devices have requested and
> enabled their respective IRQs is thus:
>
> 0x1311: 0xC0
> 0x1312: 0xC0
> 0x1313: 0xC0
> 0x1314: 0x00
> 0x1315: 0xC0
>
> For a bad case, the IRQ configuration calls end up occurring
> simultaneously and the race condition is encountered. The
> register read/write sequence then looks like this:
>
> 1. pwrkey probe: SPMI read(0x1311): 0x00, 0x00, 0x00
> 2. resin probe: SPMI read(0x1311): 0x00, 0x00, 0x00
> 3. pwrkey probe: SPMI write(0x1311): 0x80, 0x80, 0x80
> 4. resin probe: SPMI write(0x1311): 0x40, 0x40, 0x40
>
> In this case, the final register states after both devices have
> requested and enabled their respective IRQs is thus:
>
> 0x1311: 0x40
> 0x1312: 0x40
> 0x1313: 0x40
> 0x1314: 0x00
> 0x1315: 0xC0
>
> This corresponds to the resin IRQ being configured for both
> rising and falling edges, as expected. However, the pwrkey IRQ
> is misconfigured as level type with both polarity high and low
> set to disabled. The PMIC IRQ triggering hardware treats this
> particular register configuration as if level low triggering is
> enabled.
>
> The raw pwrkey IRQ signal is low when the power key is not being
> pressed. Thus, the pwrkey IRQ begins firing continuously in an
> IRQ storm.
>
> Fix the race condition by holding the spmi-pmic-arb spinlock for
> the duration of the read, modify, write sequence performed in the
> qpnpint_irq_set_type() function. Split the pmic_arb_read_cmd()
> and pmic_arb_write_cmd() functions each into three parts so that
> hardware register IO is decoupled from spinlock locking. This
> allows a new function pmic_arb_masked_write() to be added which
> locks the spinlock and then calls register IO functions to
> perform SPMI read and write commands in a single atomic
> operation.
>
> Signed-off-by: David Collins <quic_collinsd@quicinc.com>
> ---

Applied to spmi-next

\
 
 \ /
  Last update: 2021-12-16 08:15    [W:0.074 / U:2.172 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site