linux-kernel - Re: [PATCH 1/5] KVM: arm64: Allow ICC_SRE

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <86a53jee3k.wl-maz@kernel.org>
Date: Thu, 28 Aug 2025 15:05:19 +0100
From: Marc Zyngier <maz@...nel.org>
To: Sascha Bischoff <Sascha.Bischoff@....com>
Cc: "linux-arm-kernel@...ts.infradead.org"
	<linux-arm-kernel@...ts.infradead.org>,
	"kvmarm@...ts.linux.dev"
	<kvmarm@...ts.linux.dev>,
	"linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>,
	"kvm@...r.kernel.org" <kvm@...r.kernel.org>,
	nd <nd@....com>,
	"oliver.upton@...ux.dev" <oliver.upton@...ux.dev>,
	Joey Gouly
	<Joey.Gouly@....com>,
	Suzuki Poulose <Suzuki.Poulose@....com>,
	"yuzenghui@...wei.com" <yuzenghui@...wei.com>,
	"will@...nel.org"
	<will@...nel.org>,
	"tglx@...utronix.de" <tglx@...utronix.de>,
	"lpieralisi@...nel.org" <lpieralisi@...nel.org>,
	Timothy Hayes
	<Timothy.Hayes@....com>
Subject: Re: [PATCH 1/5] KVM: arm64: Allow ICC_SRE_EL2 accesses on a GICv5 host

On Thu, 28 Aug 2025 11:59:42 +0100,
Sascha Bischoff <Sascha.Bischoff@....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@....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.