On 10/13/2021 2:02 AM, Stephen Boyd wrote:
Quoting Fenglin Wu (2021-09-16 23:32:58)
From: David Collins <collinsd@xxxxxxxxxxxxxx>
Check that the apid for an SPMI interrupt falls between the
min_apid and max_apid that can be handled by the APPS processor
before invoking the per-apid interrupt handler:
periph_interrupt().
This avoids an access violation in rare cases where the status
bit is set for an interrupt that is not owned by the APPS
processor.
Signed-off-by: David Collins <collinsd@xxxxxxxxxxxxxx>
Signed-off-by: Fenglin Wu <quic_fenglinw@xxxxxxxxxxx>
---
Fixes? BTW, a lot of these patches are irqchip specific. It would be
good to get review from irqchip maintainers. Maybe we should split the
irqchip driver off via the auxiliary bus so that irqchip maintainers can
review. Please Cc them on irqchip related patches.
IRQCHIP DRIVERS
M: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
M: Marc Zyngier <maz@xxxxxxxxxx>
Sure, copied Thomas and Marc for code review.
This is a fix to avoid the register access violation in a case that an
interrupt is fired in a PMIC module which is not owned by APPS
processor.
drivers/spmi/spmi-pmic-arb.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/spmi/spmi-pmic-arb.c b/drivers/spmi/spmi-pmic-arb.c
index 4d7ad004..c4adc06 100644
--- a/drivers/spmi/spmi-pmic-arb.c
+++ b/drivers/spmi/spmi-pmic-arb.c
@@ -535,6 +535,12 @@ static void pmic_arb_chained_irq(struct irq_desc *desc)
id = ffs(status) - 1;
status &= ~BIT(id);
apid = id + i * 32;
+ if (apid < pmic_arb->min_apid
+ || apid > pmic_arb->max_apid) {
The || goes on the line above. What about making a local variable for
first and last and then shifting by 5 in the loop?
int first = pmic_arb->min_apid;
int last = pmic_arb->max_apid;
for (i = first >> 5; i <= last >> 5; i++)
if (apid < first || apid > last)
ACK, will update it following this.
+ WARN_ONCE(true, "spurious spmi irq received for apid=%d\n",
+ apid);
Is there any way to recover from this? Or once the mapping is wrong
we're going to get interrupts that we don't know what to do with
forever?
This is a rare case that the unexpected interrupt is fired in a module
not owned by APPS process, so the interrupt itself is not expected hence
no need to recover from this but just bail out to avoid following register
access violation.
+ continue;
+ }
enable = readl_relaxed(
ver_ops->acc_enable(pmic_arb, apid));
if (enable & SPMI_PIC_ACC_ENABLE_BIT)
