Re: [PATCH RESEND 2/3] irqchip/apple-aic: Only access IPI sysregs when use_fast_ipi is true
From: Thomas Gleixner <hidden>
Date: 2024-08-29 13:08:59
Also in:
asahi, linux-devicetree, lkml
On Thu, Aug 29 2024 at 19:02, Nick Chan wrote:
Starting from the A11 (T8015) SoC, Apple introuced system registers for fast IPI and UNCORE PMC control. These sysregs do not exist on earlier A7-A10 SoCs and trying to access them results in an instant crash. Restrict sysreg access within the AIC driver to configurations where use_fast_ipi is true to allow AIC to function properly on A7-A10 SoCs.
use_fast_ipi is an implementation detail and does not mean anything here. It's sufficient to say: Only access system registers on SoCs which provide them. Hmm?
While at it, remove the IPI-always-ack path on aic_handle_fiq.
It's not while at it. It's part of handling this correctly.
If we are able to reach there, we are on an IPI-capable system and should be using one of the IPI-capable compatibles, anyway.
'we' can't reach that code ever. https://www.kernel.org/doc/html/latest/process/maintainer-tip.html#changelog
Signed-off-by: Konrad Dybcio <redacted> Signed-off-by: Nick Chan <redacted>
This Signed-off-by chain is invalid. If Konrad authored the patch then you need to have a 'From: Konrad ...' line at the top of the change log. If you worked together on this then this needs a Co-developed-by tag. See Documentation/process/...
quoted hunk ↗ jump to hunk
- if (read_sysreg_s(SYS_IMP_APL_IPI_SR_EL1) & IPI_SR_PENDING) { - if (static_branch_likely(&use_fast_ipi)) { - aic_handle_ipi(regs); - } else { - pr_err_ratelimited("Fast IPI fired. Acking.\n"); - write_sysreg_s(IPI_SR_PENDING, SYS_IMP_APL_IPI_SR_EL1); - } + if (static_branch_likely(&use_fast_ipi) && + (read_sysreg_s(SYS_IMP_APL_IPI_SR_EL1) & IPI_SR_PENDING)) { + aic_handle_ipi(regs); } if (TIMER_FIRING(read_sysreg(cntp_ctl_el0)))@@ -574,8 +571,9 @@ static void __exception_irq_entry aic_handle_fiq(struct pt_regs *regs) AIC_FIQ_HWIRQ(irq)); } - if (FIELD_GET(UPMCR0_IMODE, read_sysreg_s(SYS_IMP_APL_UPMCR0_EL1)) == UPMCR0_IMODE_FIQ && - (read_sysreg_s(SYS_IMP_APL_UPMSR_EL1) & UPMSR_IACT)) { + if (static_branch_likely(&use_fast_ipi) && + (FIELD_GET(UPMCR0_IMODE, read_sysreg_s(SYS_IMP_APL_UPMCR0_EL1)) == UPMCR0_IMODE_FIQ) && + (read_sysreg_s(SYS_IMP_APL_UPMSR_EL1) & UPMSR_IACT)) { /* Same story with uncore PMCs */ pr_err_ratelimited("Uncore PMC FIQ fired. Masking.\n"); sysreg_clear_set_s(SYS_IMP_APL_UPMCR0_EL1, UPMCR0_IMODE,@@ -811,7 +809,8 @@ static int aic_init_cpu(unsigned int cpu) /* Mask all hard-wired per-CPU IRQ/FIQ sources */ /* Pending Fast IPI FIQs */ - write_sysreg_s(IPI_SR_PENDING, SYS_IMP_APL_IPI_SR_EL1); + if (static_branch_likely(&use_fast_ipi)) + write_sysreg_s(IPI_SR_PENDING, SYS_IMP_APL_IPI_SR_EL1); /* Timer FIQs */ sysreg_clear_set(cntp_ctl_el0, 0, ARCH_TIMER_CTRL_IT_MASK);@@ -832,8 +831,9 @@ static int aic_init_cpu(unsigned int cpu) FIELD_PREP(PMCR0_IMODE, PMCR0_IMODE_OFF)); /* Uncore PMC FIQ */ - sysreg_clear_set_s(SYS_IMP_APL_UPMCR0_EL1, UPMCR0_IMODE, - FIELD_PREP(UPMCR0_IMODE, UPMCR0_IMODE_OFF)); + if (static_branch_likely(&use_fast_ipi)) + sysreg_clear_set_s(SYS_IMP_APL_UPMCR0_EL1, UPMCR0_IMODE, + FIELD_PREP(UPMCR0_IMODE, UPMCR0_IMODE_OFF));
Please see the bracket rules in the tip maintainers doc.
Thanks,
tglx