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

Sascha Bischoff posted 1 patch 3 weeks ago
arch/arm64/tools/sysreg      |  4 +++-
drivers/irqchip/irq-gic-v5.c | 33 +++++++++++++++++++++++++++++----
2 files changed, 32 insertions(+), 5 deletions(-)
[PATCH] irqchip/gic-v5: Bring up and preserve the IRI link
Posted by Sascha Bischoff 3 weeks ago
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);
 }
-- 
2.34.1
Re: [PATCH] irqchip/gic-v5: Bring up and preserve the IRI link
Posted by Sascha Bischoff 2 weeks, 4 days ago
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