Re: [PATCH v3 01/40] irqchip/gic-v5: Allow KVM setup without a maintenance IRQ

[sashiko-bot@kernel.org]

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

New issues:
- [Medium] Leaked KVM `gic_kvm_info` allocation on error path when GICv5 legacy is supported but maintenance IRQ is absent.

Pre-existing issues:
- [High] Writes to GICv5 control-plane system registers (ICC_*_EL1) lack immediate `isb()` synchronization.
--

commit 77500f7dacbe8c6ac1528bc594d810bf95cef563
Author: Sascha Bischoff <Sascha.Bischoff@arm.com>
Subject: irqchip/gic-v5: Allow KVM setup without a maintenance IRQ

This commit modifies the GICv5 driver to allow KVM setup even if the
maintenance interrupt is absent, provided that legacy GICv3 support is
not required. It sets a flag to inform KVM that the maintenance interrupt
is not needed for native GICv5 operation.

> diff --git a/drivers/irqchip/irq-gic-v5.c b/drivers/irqchip/irq-gic-v5.c
> index e9d1795235a66..c04523a438cee 100644
> --- a/drivers/irqchip/irq-gic-v5.c
> +++ b/drivers/irqchip/irq-gic-v5.c
> @@ -1143,10 +1143,18 @@ static void __init gic_of_setup_kvm_info(struct device_node *node)
>  	/* GIC Virtual CPU interface maintenance interrupt */
>  	gic_v5_kvm_info.no_maint_irq_mask = false;
>  	gic_v5_kvm_info.maint_irq = irq_of_parse_and_map(node, 0);
> -	if (!gic_v5_kvm_info.maint_irq) {
> -		pr_warn("cannot find GICv5 virtual CPU interface maintenance interrupt\n");
> -		return;
> -	}
> +
> +	/*
> +	 * We require an MI if we have legacy support, but don't, otherwise.
> +	 * Given that there's an existing flag to convey that an MI isn't
> +	 * needed, we (ab)use it to tell KVM that the MI isn't needed if we
> +	 * don't support legacy.
> +	 *
> +	 * The check for ARM64_HAS_GICV5_LEGACY explicitly doesn't use
> +	 * cpus_have_final_cap() here as we run too early.
> +	 */
> +	if (!cpus_have_cap(ARM64_HAS_GICV5_LEGACY) && !gic_v5_kvm_info.maint_irq)
> +		gic_v5_kvm_info.no_maint_irq_mask = true;
>  
>  	vgic_set_kvm_info(&gic_v5_kvm_info);

[Severity: Medium]
Does this introduce a memory leak of the gic_kvm_info allocation?

If ARM64_HAS_GICV5_LEGACY is supported and maint_irq is 0, this bypasses
the early return and proceeds to vgic_set_kvm_info(), which dynamically
allocates gic_kvm_info.

Later, kvm_vgic_hyp_init() processes this configuration:

arch/arm64/kvm/vgic/vgic-init.c:kvm_vgic_hyp_init() {
    ...
	if (has_mask && !gic_kvm_info->maint_irq) {
		kvm_err("No vgic maintenance irq\n");
		return -ENXIO;
	}
    ...
}

Will this early return bypass the kfree(gic_kvm_info) at the end of the
function and leak the memory?


[Severity: High]
This is a pre-existing issue, but do the writes to GICv5 control-plane system
registers lack immediate isb() synchronization?

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 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();

ARM64 architectural invariants require writes to control-plane system
registers to be followed immediately by an isb() as the very next
instruction. Missing this can cause the CPU to operate under a stale
configuration for several cycles.

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