Re: [PATCH] KVM: arm64: Only enable S1PIE and S1POE for a guest with TCR2 and ATS1A

[Marc Zyngier <maz@kernel.org>]

On Thu, 15 Oct 2026 16:30:41 +0100,
Fuad Tabba <fuad.tabba@linux.dev> wrote:
> 
> The world switch skips PIR_EL1, PIRE0_EL1 and POR_EL1 for a guest
> without TCR2, but KVM doesn't trap them for a guest with S1PIE or
> S1POE. Userspace can hide TCR2 while leaving either visible, and such a
> guest then accesses registers that are neither trapped nor switched.
> 
> Under VHE, that leaks values between guests. POR_EL1, which the host
> kernel doesn't use, leaks between guests in every mode. On nVHE and
> hVHE, pKVM included, the host's own EL1 registers are saved and restored
> according to the running guest's features, so the host kernel resumes
> with its stage-1 permissions taken from the guest's PIR_EL1 and
> PIRE0_EL1.
> 
> As per the architecture, S1PIE and S1POE imply TCR2 and ATS1A. Only
> enable them for a guest that has both. A guest that has S1PIE or S1POE
> without TCR2 or ATS1A still sees the feature in its ID registers, but
> its accesses to that feature's registers UNDEF.
> 
> Fixes: 70ed7238297fb ("KVM: arm64: Sanitise ID_AA64MMFR3_EL1")
> Fixes: d4a89e5aee23e ("KVM: arm64: Expose S1PIE to guests")
> Reported-by: Mark Brown <broonie@kernel.org>
> Link: https://lore.kernel.org/r/20260930-arm64-gcs-v21-2-3556644cd927@kernel.org
> Suggested-by: Marc Zyngier <maz@kernel.org>
> Link: https://lore.kernel.org/r/86fqyn2che.wl-maz@kernel.org
> Cc: stable@vger.kernel.org
> Signed-off-by: Fuad Tabba <fuad.tabba@linux.dev>
> ---
> I'm sending this on its own because it affects the host on nVHE and in
> pKVM, so that it can go to kvmarm/fixes and stable rather than land
> with the GCS series. It's the data-driven fix Marc asked for on Mark's
> v21 02/15 [1].
> 
> Following Mark's point on Lorenzo's draft [2], only feat_s1pie() and
> feat_s1poe() test TCR2 and ATS1A, so kvm_has_s1pie() and the world
> switch are unchanged. I've left the guest's ID registers alone, since
> updating them is the larger rework Mark described.
> 
> I've got a reproducer showing the host's PIR_EL1 overwritten on nVHE
> and in pKVM, and can share it privately.
> 
> Based on Linux 7.3-rc3 (fd73f4a665989; also applies to kvmarm/fixes
> 71cc2c67fb8f8).
> 
> [1] https://lore.kernel.org/r/86fqyn2che.wl-maz@kernel.org
> [2] https://lore.kernel.org/r/ae24e80c-1b34-4827-9c93-e80b987e9cd7@sirena.org.uk
> 
>  arch/arm64/kvm/config.c | 23 +++++++++++++++++++----
>  1 file changed, 19 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm64/kvm/config.c b/arch/arm64/kvm/config.c
> index 1053676551aff..5e5277c721d52 100644
> --- a/arch/arm64/kvm/config.c
> +++ b/arch/arm64/kvm/config.c
> @@ -280,6 +280,21 @@ static bool feat_anerr(struct kvm *kvm)
>  		kvm_has_feat(kvm, ID_AA64MMFR3_EL1, SNERR, FEAT_ANERR));
>  }
>  
> +/* S1PIE and S1POE imply TCR2 and ATS1A, but userspace can set them independently */
> +static bool feat_s1pie(struct kvm *kvm)
> +{
> +	return (kvm_has_feat(kvm, FEAT_S1PIE) &&
> +		kvm_has_feat(kvm, FEAT_TCR2) &&
> +		kvm_has_feat(kvm, FEAT_ATS1A));
> +}
> +
> +static bool feat_s1poe(struct kvm *kvm)
> +{
> +	return (kvm_has_feat(kvm, FEAT_S1POE) &&
> +		kvm_has_feat(kvm, FEAT_TCR2) &&
> +		kvm_has_feat(kvm, FEAT_ATS1A));

Where's what the relevant bits of the JSON say:

(FEAT_S1POE ==> FEAT_TCR2)
(FEAT_S1POE ==> FEAT_ATS1A)
(FEAT_S1POE ==> FEAT_HPDS)

So this is missing an additional check on FEAT_HPDS, which is advertised
as:

(FEAT_AA64EL1 ==> (FEAT_HPDS <=> (UInt(ID_AA64MMFR1_EL1.HPDS) >= 1)))

Thanks,

	M.

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