Re: [PATCH 1/5] KVM: arm64: Allow ICC_SRE_EL2 accesses on a GICv5 host

[Marc Zyngier <maz@kernel.org>]

On Thu, 28 Aug 2025 11:59:42 +0100,
Sascha Bischoff <Sascha.Bischoff@arm.com> wrote:
> 
> The bet0 release of the GICv5 specification didn't include the
> ICC_SRE_EL2 register as part of FEAT_GCIE_LEGACY. This was an
> oversight, and support for this register has been added as of the bet1
> release of the specification.
> 
> Remove the guarding in the vGICv3 code that skipped the ICC_SRE_EL2
> accesses for a GICv5 host. As a result of this change, it now becomes
> possible to use nested virtualisation on a GICv5 host when running
> legacy GICv3-based VMs.
> 
> Signed-off-by: Sascha Bischoff <sascha.bischoff@arm.com>
> ---
>  arch/arm64/kvm/hyp/vgic-v3-sr.c | 27 +++++++--------------------
>  1 file changed, 7 insertions(+), 20 deletions(-)
> 
> diff --git a/arch/arm64/kvm/hyp/vgic-v3-sr.c b/arch/arm64/kvm/hyp/vgic-v3-sr.c
> index d81275790e69..7dbfd35a63a8 100644
> --- a/arch/arm64/kvm/hyp/vgic-v3-sr.c
> +++ b/arch/arm64/kvm/hyp/vgic-v3-sr.c
> @@ -296,19 +296,12 @@ void __vgic_v3_activate_traps(struct vgic_v3_cpu_if *cpu_if)
>  	}
>  
>  	/*
> -	 * GICv5 BET0 FEAT_GCIE_LEGACY doesn't include ICC_SRE_EL2. This is due
> -	 * to be relaxed in a future spec release, at which point this in
> -	 * condition can be dropped.
> +	 * Prevent the guest from touching the ICC_SRE_EL1 system
> +	 * register. Note that this may not have any effect, as
> +	 * ICC_SRE_EL2.Enable being RAO/WI is a valid implementation.
>  	 */
> -	if (!cpus_have_final_cap(ARM64_HAS_GICV5_CPUIF)) {
> -		/*
> -		 * Prevent the guest from touching the ICC_SRE_EL1 system
> -		 * register. Note that this may not have any effect, as
> -		 * ICC_SRE_EL2.Enable being RAO/WI is a valid implementation.
> -		 */
> -		write_gicreg(read_gicreg(ICC_SRE_EL2) & ~ICC_SRE_EL2_ENABLE,
> -			     ICC_SRE_EL2);
> -	}
> +	write_gicreg(read_gicreg(ICC_SRE_EL2) & ~ICC_SRE_EL2_ENABLE,
> +		     ICC_SRE_EL2);

At some point, it would be great to elide this on systems where
GICv2-on-v3 doesn't exist, as there is no way for the guest to disable
the system register view. This would avoid a couple of pointless traps
on each entry-exit for a nested guest.

>  
>  	/*
>  	 * If we need to trap system registers, we must write
> @@ -329,14 +322,8 @@ void __vgic_v3_deactivate_traps(struct vgic_v3_cpu_if *cpu_if)
>  		cpu_if->vgic_vmcr = read_gicreg(ICH_VMCR_EL2);
>  	}
>  
> -	/*
> -	 * Can be dropped in the future when GICv5 spec is relaxed. See comment
> -	 * above.
> -	 */
> -	if (!cpus_have_final_cap(ARM64_HAS_GICV5_CPUIF)) {
> -		val = read_gicreg(ICC_SRE_EL2);
> -		write_gicreg(val | ICC_SRE_EL2_ENABLE, ICC_SRE_EL2);
> -	}
> +	val = read_gicreg(ICC_SRE_EL2);
> +	write_gicreg(val | ICC_SRE_EL2_ENABLE, ICC_SRE_EL2);

Same here. That's two back-to-back traps for values that cannot
realistically change on non-v2-compat systems (i.e. relatively modern
machines).

No need to respin for that, but I may end-up posting a follow-up to
clean this up.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.