Re: [PATCH v3 06/40] arm64/sysreg: Update ICC_CR0_EL1 with LINK and LINK_IDLE fields

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Writes to control-plane system registers (e.g., ICC_CR0_EL1, ICC_PCR_EL1, ICC_PPI_ENABLERx_EL1) are not followed by an immediate isb() instruction.
--

commit a711c6a81167d568ab1f95533a38518adf17a484
Author: Sascha Bischoff <Sascha.Bischoff@arm.com>

arm64/sysreg: Update ICC_CR0_EL1 with LINK and LINK_IDLE fields

This commit updates the sysreg generator definition for ICC_CR0_EL1 to
include the LINK and LINK_IDLE fields, which have been added to the
architecture.

> diff --git a/arch/arm64/tools/sysreg b/arch/arm64/tools/sysreg
> index 6c3ff14e561e6..57ab09404267c 100644
> --- a/arch/arm64/tools/sysreg
> +++ b/arch/arm64/tools/sysreg
> @@ -3736,7 +3736,9 @@ Sysreg	ICC_CR0_EL1	3	1	12	0	1
>  Res0	63:39
>  Field	38	PID
>  Field	37:32	IPPT
> -Res0	31:1
> +Res0	31:3
> +Field	2	LINK_IDLE
> +Field	1	LINK
>  Field	0	EN
>  EndSysreg

[Severity: High]
This isn't a bug introduced by this patch, but since it updates the definition
of ICC_CR0_EL1, I noticed that writes to control-plane system registers (e.g.,
ICC_CR0_EL1, ICC_PCR_EL1, ICC_PPI_ENABLERx_EL1) are missing immediate isb()
synchronization in several places.

For example, in drivers/irqchip/irq-gic-v5.c:gicv5_cpu_disable_interrupts():

	cr0 = FIELD_PREP(ICC_CR0_EL1_EN, 0);
	write_sysreg_s(cr0, SYS_ICC_CR0_EL1);
}

And in drivers/irqchip/irq-gic-v5.c:gicv5_cpu_enable_interrupts():

	pcr = FIELD_PREP(ICC_PCR_EL1_PRIORITY, GICV5_IRQ_PRI_MI);
	write_sysreg_s(pcr, SYS_ICC_PCR_EL1);

	cr0 = FIELD_PREP(ICC_CR0_EL1_EN, 1);
	write_sysreg_s(cr0, SYS_ICC_CR0_EL1);

There are also instances where multiple writes are batched without an immediate
isb() in between, such as in gicv5_cpu_enable_interrupts():

	write_sysreg_s(0, SYS_ICC_PPI_ENABLER0_EL1);
	write_sysreg_s(0, SYS_ICC_PPI_ENABLER1_EL1);

	gicv5_ppi_priority_init();

And in drivers/irqchip/irq-gic-v5.c:gicv5_ppi_priority_init():

	write_sysreg_s(REPEAT_BYTE(GICV5_IRQ_PRI_MI), SYS_ICC_PPI_PRIORITYR0_EL1);
	write_sysreg_s(REPEAT_BYTE(GICV5_IRQ_PRI_MI), SYS_ICC_PPI_PRIORITYR1_EL1);

The exact same missing isb() patterns appear in the selftest implementations
in tools/testing/selftests/kvm/include/arm64/gic_v5.h.

Could this leave the CPU operating under an architecturally undefined pipeline
state? According to architectural rules, every write to a control-plane system
register (including all ICC_*_EL1 registers except ICC_PMR_EL1) must be
followed by an isb() as the very next instruction.

Batching writes or executing any instruction between the write and the isb()
might lead to unpredictable behavior, spurious traps, or kernel panics.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260703154811.3355680-1-sascha.bischoff@arm.com?part=6