Re: [PATCH] irqchip/gic-v5: Bring up and preserve the IRI link

[Sascha Bischoff <Sascha.Bischoff@arm.com>]

On Fri, 2026-09-04 at 18:41 +0100, Sascha Bischoff wrote:
> ICC_CR0_EL1 controls the link between the CPU interface and the IRI.
> The driver currently writes absolute values when enabling and
> disabling the interface. When EL3 is not present, this clears LINK
> and
> can disconnect the link. Moreover, in this situation the kernel
> itself
> might need to bring the link online.
> 
> Define the missing LINK and LINK_IDLE fields. Use read-modify-write
> operations when changing EN so that the existing register state is
> preserved.
> 
> During CPU interface initialisation, set LINK if it is clear and poll
> LINK_IDLE before proceeding to enable interrupt
> delivery. Propagate a link timeout through the CPU hotplug startup
> callback.
> 
> Fixes: 7ec80fb3f025 ("irqchip/gic-v5: Add GICv5 PPI support")
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Link:
> https://lore.kernel.org/r/20260807121703.D4B7A1F00A3A@smtp.kernel.org
> Signed-off-by: Sascha Bischoff <sascha.bischoff@arm.com>
> ---
>  arch/arm64/tools/sysreg      |  4 +++-
>  drivers/irqchip/irq-gic-v5.c | 33 +++++++++++++++++++++++++++++----
>  2 files changed, 32 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/arm64/tools/sysreg b/arch/arm64/tools/sysreg
> index 94bf065c8ac71..e2d37ee221b84 100644
> --- a/arch/arm64/tools/sysreg
> +++ b/arch/arm64/tools/sysreg
> @@ -3761,7 +3761,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
>  
> diff --git a/drivers/irqchip/irq-gic-v5.c b/drivers/irqchip/irq-gic-
> v5.c
> index ac2d423b17239..4669e40356eaf 100644
> --- a/drivers/irqchip/irq-gic-v5.c
> +++ b/drivers/irqchip/irq-gic-v5.c
> @@ -8,6 +8,7 @@
>  #include <linux/acpi_iort.h>
>  #include <linux/cpuhotplug.h>
>  #include <linux/idr.h>
> +#include <linux/iopoll.h>
>  #include <linux/irqdomain.h>
>  #include <linux/slab.h>
>  #include <linux/wordpart.h>
> @@ -974,14 +975,16 @@ static void gicv5_cpu_disable_interrupts(void)
>  {
>  	u64 cr0;
>  
> -	cr0 = FIELD_PREP(ICC_CR0_EL1_EN, 0);
> +	cr0 = read_sysreg_s(SYS_ICC_CR0_EL1);
> +	cr0 &= ~ICC_CR0_EL1_EN_MASK;
>  	write_sysreg_s(cr0, SYS_ICC_CR0_EL1);
>  	isb();
>  }
>  
> -static void gicv5_cpu_enable_interrupts(void)
> +static int gicv5_cpu_enable_interrupts(void)
>  {
>  	u64 cr0, pcr;
> +	int ret;
>  
>  	write_sysreg_s(0, SYS_ICC_PPI_ENABLER0_EL1);
>  	write_sysreg_s(0, SYS_ICC_PPI_ENABLER1_EL1);
> @@ -991,19 +994,41 @@ static void gicv5_cpu_enable_interrupts(void)
>  	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);
> +	cr0 = read_sysreg_s(SYS_ICC_CR0_EL1);
> +	if (!(cr0 & ICC_CR0_EL1_LINK_MASK)) {
> +		cr0 |= ICC_CR0_EL1_LINK_MASK;
> +		write_sysreg_s(cr0, SYS_ICC_CR0_EL1);
> +	}
> +
> +	ret = read_poll_timeout_atomic(read_sysreg_s, cr0,
> +				       cr0 &
> ICC_CR0_EL1_LINK_IDLE_MASK,
> +				       1, 10 * USEC_PER_MSEC, false,
> +				       SYS_ICC_CR0_EL1);
> +	if (ret) {
> +		pr_err_ratelimited("CPU interface link timeout\n");
> +		return ret;
> +	}
> +
> +	cr0 = read_sysreg_s(SYS_ICC_CR0_EL1);
> +	cr0 |= ICC_CR0_EL1_EN_MASK;
>  	write_sysreg_s(cr0, SYS_ICC_CR0_EL1);
> +
> +	return 0;
>  }
>  
>  static int base_ipi_virq;
>  
>  static int gicv5_starting_cpu(unsigned int cpu)
>  {
> +	int ret;
> +
>  	if (WARN(!gicv5_cpuif_has_gcie(),
>  		 "GICv5 system components present but CPU does not
> have FEAT_GCIE"))
>  		return -ENODEV;
>  
> -	gicv5_cpu_enable_interrupts();
> +	ret = gicv5_cpu_enable_interrupts();
> +	if (ret)
> +		return ret;
>  
>  	return gicv5_irs_register_cpu(cpu);
>  }

After discussing this offline with Lorenzo and Marc, I've decided to
drop the setting of the LINK if it is inactive. This is not a case we
expect to see, so it makes little sense to add dead code to handle the
case.

Instead, I've re-spun this to just preserve the other bits in the
register when writing the EN bit (so, doing a read-modify-write).

v2 is here:
https://lore.kernel.org/lkml/20260907164945.714545-1-sascha.bischoff@arm.com

Thanks,
Sascha