Re: [PATCH] KVM: arm64: Don't access ICC_SRE_EL2 if GICv3 doesn't support v2 compatibility

[Oliver Upton <oliver.upton@linux.dev>]

On Wed, Sep 17, 2025 at 05:19:35PM +0100, Marc Zyngier wrote:
> We currently access ICC_SRE_EL2 at each load/put on VHE, and on each
> entry/exit on nVHE. Both are quite onerous on NV, as this register
> always traps.
> 
> We do this to make sure the EL1 guest doesn't flip between v2 and v3
> behind our back. But all modern implementations have dropped v2,
> and this is just overhead.
> 
> At the same time, the GICv5 spec has been fixed to allow access to
> ICC_SRE_EL2 in legacy mode. Use this opportunity to replace the
> GICv5 checks for v2 compat checks, with an ad-hoc static key.
> 
> Co-developed-by: Sascha Bischoff <sascha.bischoff@arm.com>
> Signed-off-by: Sascha Bischoff <sascha.bischoff@arm.com>
> Signed-off-by: Marc Zyngier <maz@kernel.org>

Reviewed-by: Oliver Upton <oliver.upton@linux.dev>

Thanks,
Oliver

> ---
>  arch/arm64/kernel/image-vars.h  |  3 +++
>  arch/arm64/kvm/hyp/vgic-v3-sr.c | 25 +++++++++----------------
>  arch/arm64/kvm/vgic/vgic-v3.c   |  8 ++++++++
>  include/kvm/arm_vgic.h          |  1 +
>  4 files changed, 21 insertions(+), 16 deletions(-)
> 
> diff --git a/arch/arm64/kernel/image-vars.h b/arch/arm64/kernel/image-vars.h
> index 714b0b5ec5ac4..5369763606e71 100644
> --- a/arch/arm64/kernel/image-vars.h
> +++ b/arch/arm64/kernel/image-vars.h
> @@ -105,6 +105,9 @@ KVM_NVHE_ALIAS(__hyp_stub_vectors);
>  KVM_NVHE_ALIAS(vgic_v2_cpuif_trap);
>  KVM_NVHE_ALIAS(vgic_v3_cpuif_trap);
>  
> +/* Static key indicating whether GICv3 has GICv2 compatibility */
> +KVM_NVHE_ALIAS(vgic_v3_has_v2_compat);
> +
>  /* Static key which is set if CNTVOFF_EL2 is unusable */
>  KVM_NVHE_ALIAS(broken_cntvoff_key);
>  
> diff --git a/arch/arm64/kvm/hyp/vgic-v3-sr.c b/arch/arm64/kvm/hyp/vgic-v3-sr.c
> index d81275790e69b..acd909b7f2257 100644
> --- a/arch/arm64/kvm/hyp/vgic-v3-sr.c
> +++ b/arch/arm64/kvm/hyp/vgic-v3-sr.c
> @@ -295,12 +295,8 @@ 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.
> -	 */
> -	if (!cpus_have_final_cap(ARM64_HAS_GICV5_CPUIF)) {
> +	/* Only disable SRE if the host implements the GICv2 interface */
> +	if (static_branch_unlikely(&vgic_v3_has_v2_compat)) {
>  		/*
>  		 * Prevent the guest from touching the ICC_SRE_EL1 system
>  		 * register. Note that this may not have any effect, as
> @@ -329,19 +325,16 @@ 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)) {
> +	/* Only restore SRE if the host implements the GICv2 interface */
> +	if (static_branch_unlikely(&vgic_v3_has_v2_compat)) {
>  		val = read_gicreg(ICC_SRE_EL2);
>  		write_gicreg(val | ICC_SRE_EL2_ENABLE, ICC_SRE_EL2);
> -	}
>  
> -	if (!cpu_if->vgic_sre) {
> -		/* Make sure ENABLE is set at EL2 before setting SRE at EL1 */
> -		isb();
> -		write_gicreg(1, ICC_SRE_EL1);
> +		if (!cpu_if->vgic_sre) {
> +			/* Make sure ENABLE is set at EL2 before setting SRE at EL1 */
> +			isb();
> +			write_gicreg(1, ICC_SRE_EL1);
> +		}
>  	}
>  
>  	/*
> diff --git a/arch/arm64/kvm/vgic/vgic-v3.c b/arch/arm64/kvm/vgic/vgic-v3.c
> index b9ad7c42c5b01..f1c153106c563 100644
> --- a/arch/arm64/kvm/vgic/vgic-v3.c
> +++ b/arch/arm64/kvm/vgic/vgic-v3.c
> @@ -588,6 +588,7 @@ int vgic_v3_map_resources(struct kvm *kvm)
>  }
>  
>  DEFINE_STATIC_KEY_FALSE(vgic_v3_cpuif_trap);
> +DEFINE_STATIC_KEY_FALSE(vgic_v3_has_v2_compat);
>  
>  static int __init early_group0_trap_cfg(char *buf)
>  {
> @@ -697,6 +698,13 @@ int vgic_v3_probe(const struct gic_kvm_info *info)
>  	if (kvm_vgic_global_state.vcpu_base == 0)
>  		kvm_info("disabling GICv2 emulation\n");
>  
> +	/*
> +	 * Flip the static branch if the HW supports v2, even if we're
> +	 * not using it (such as in protected mode).
> +	 */
> +	if (has_v2)
> +		static_branch_enable(&vgic_v3_has_v2_compat);
> +
>  	if (cpus_have_final_cap(ARM64_WORKAROUND_CAVIUM_30115)) {
>  		group0_trap = true;
>  		group1_trap = true;
> diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
> index 404883c7af6e8..9a6340d9c91e5 100644
> --- a/include/kvm/arm_vgic.h
> +++ b/include/kvm/arm_vgic.h
> @@ -375,6 +375,7 @@ struct vgic_cpu {
>  
>  extern struct static_key_false vgic_v2_cpuif_trap;
>  extern struct static_key_false vgic_v3_cpuif_trap;
> +extern struct static_key_false vgic_v3_has_v2_compat;
>  
>  int kvm_set_legacy_vgic_v2_addr(struct kvm *kvm, struct kvm_arm_device_addr *dev_addr);
>  void kvm_vgic_early_init(struct kvm *kvm);
> -- 
> 2.39.2
>