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

Sascha Bischoff posted 5 patches 1 month ago
[PATCH 1/5] KVM: arm64: Allow ICC_SRE_EL2 accesses on a GICv5 host
Posted by Sascha Bischoff 1 month ago
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);
 
 	/*
 	 * 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);
 
 	if (!cpu_if->vgic_sre) {
 		/* Make sure ENABLE is set at EL2 before setting SRE at EL1 */
-- 
2.34.1
Re: [PATCH 1/5] KVM: arm64: Allow ICC_SRE_EL2 accesses on a GICv5 host
Posted by Marc Zyngier 2 weeks, 1 day ago
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>

I just remembered my promise from almost 3 weeks ago, and just posted
this:

https://lore.kernel.org/r/20250917161935.1630908-1-maz@kernel.org

which kills two birds with one stone. I'll take it as a prefix to this
series.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.
Re: [PATCH 1/5] KVM: arm64: Allow ICC_SRE_EL2 accesses on a GICv5 host
Posted by Marc Zyngier 1 month ago
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.