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 <konrad.dybcio@xxxxxxxxxxxxxx> > Signed-off-by: Nick Chan <towinchenmi@xxxxxxxxx> 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/... > > - 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
