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

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

[Fuad Tabba]

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));
+}
+
 static bool feat_sme_smps(struct kvm *kvm)
 {
 	/*
@@ -393,10 +408,10 @@ static const struct reg_bits_to_feat_map hfgrtr_feat_map[] = {
 	NEEDS_FEAT(HFGRTR_EL2_nS2POR_EL1, FEAT_S2POE),
 	NEEDS_FEAT(HFGRTR_EL2_nPOR_EL1		|
 		   HFGRTR_EL2_nPOR_EL0,
-		   FEAT_S1POE),
+		   feat_s1poe),
 	NEEDS_FEAT(HFGRTR_EL2_nPIR_EL1		|
 		   HFGRTR_EL2_nPIRE0_EL1,
-		   FEAT_S1PIE),
+		   feat_s1pie),
 	NEEDS_FEAT(HFGRTR_EL2_nRCWMASK_EL1, FEAT_THE),
 	NEEDS_FEAT(HFGRTR_EL2_nTPIDR2_EL0	|
 		   HFGRTR_EL2_nSMPRI_EL1,
@@ -474,10 +489,10 @@ static const struct reg_bits_to_feat_map hfgwtr_feat_map[] = {
 	NEEDS_FEAT(HFGWTR_EL2_nS2POR_EL1, FEAT_S2POE),
 	NEEDS_FEAT(HFGWTR_EL2_nPOR_EL1		|
 		   HFGWTR_EL2_nPOR_EL0,
-		   FEAT_S1POE),
+		   feat_s1poe),
 	NEEDS_FEAT(HFGWTR_EL2_nPIR_EL1		|
 		   HFGWTR_EL2_nPIRE0_EL1,
-		   FEAT_S1PIE),
+		   feat_s1pie),
 	NEEDS_FEAT(HFGWTR_EL2_nRCWMASK_EL1, FEAT_THE),
 	NEEDS_FEAT(HFGWTR_EL2_nTPIDR2_EL0	|
 		   HFGWTR_EL2_nSMPRI_EL1,

base-commit: fd73f4a6659897191fa0d40695fe370925dd3780
-- 
2.39.5

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

[Marc Zyngier]

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.