All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] KVM: arm64: Only enable S1PIE and S1POE with the features they imply
@ 2026-10-18  8:09 Fuad Tabba
  2026-10-18  8:29 ` sashiko-bot
  2026-10-18 17:37 ` Marc Zyngier
  0 siblings, 2 replies; 5+ messages in thread
From: Fuad Tabba @ 2026-10-18  8:09 UTC (permalink / raw)
  To: Marc Zyngier, Oliver Upton, kvmarm, linux-arm-kernel
  Cc: Joey Gouly, Steffen Eiden, Suzuki K Poulose, Zenghui Yu,
	Catalin Marinas, Will Deacon, Mark Rutland, Mark Brown,
	Lorenzo Stoakes, Fuad Tabba, linux-kernel

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, and
S1POE also implies HPDS. Only enable each for a guest that has the
features it implies. A guest that has S1PIE or S1POE without them 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>
---
Changes in v2:
- Also require FEAT_HPDS for S1POE (Marc)
- Link to v1: https://lore.kernel.org/r/20261015153041.3770032-1-fuad.tabba@linux.dev

Based on Linux 7.3-rc3 (fd73f4a665989).

 arch/arm64/kvm/config.c | 28 ++++++++++++++++++++++++----
 1 file changed, 24 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/kvm/config.c b/arch/arm64/kvm/config.c
index 1053676551aff..6d2650427c5fc 100644
--- a/arch/arm64/kvm/config.c
+++ b/arch/arm64/kvm/config.c
@@ -205,6 +205,7 @@ struct reg_feat_map_desc {
 #define FEAT_MEC		ID_AA64MMFR3_EL1, MEC, IMP
 #define FEAT_HAFT		ID_AA64MMFR1_EL1, HAFDBS, HAFT
 #define FEAT_HDBSS		ID_AA64MMFR1_EL1, HAFDBS, HDBSS
+#define FEAT_HPDS		ID_AA64MMFR1_EL1, HPDS, IMP
 #define FEAT_HPDS2		ID_AA64MMFR1_EL1, HPDS, HPDS2
 #define FEAT_BTI		ID_AA64PFR1_EL1, BT, IMP
 #define FEAT_ExS		ID_AA64MMFR0_EL1, EXS, IMP
@@ -280,6 +281,25 @@ static bool feat_anerr(struct kvm *kvm)
 		kvm_has_feat(kvm, ID_AA64MMFR3_EL1, SNERR, FEAT_ANERR));
 }
 
+/*
+ * S1PIE and S1POE imply TCR2 and ATS1A, and S1POE also implies HPDS, 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) &&
+		kvm_has_feat(kvm, FEAT_HPDS));
+}
+
 static bool feat_sme_smps(struct kvm *kvm)
 {
 	/*
@@ -393,10 +413,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 +494,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,
-- 
2.39.5


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] KVM: arm64: Only enable S1PIE and S1POE with the features they imply
  2026-10-18  8:09 [PATCH v2] KVM: arm64: Only enable S1PIE and S1POE with the features they imply Fuad Tabba
@ 2026-10-18  8:29 ` sashiko-bot
  2026-10-18 15:15   ` Fuad Tabba
  2026-10-18 17:37 ` Marc Zyngier
  1 sibling, 1 reply; 5+ messages in thread
From: sashiko-bot @ 2026-10-18  8:29 UTC (permalink / raw)
  To: Fuad Tabba, kvmarm; +Cc: Oliver Upton, Marc Zyngier

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] KVM: arm64: incomplete S1PIE/S1POE implied-feature checks in kvm_has_s1p*()
--

commit 1d4d0fe97a6aa6ec7a4e6a93ff7c048e9f3bf806
Author: Fuad Tabba <fuad.tabba@linux.dev>

KVM: arm64: Only enable S1PIE and S1POE with the features they imply

This commit restricts the fine-grained trap enablement of S1PIE and S1POE
registers in hfgrtr_feat_map and hfgwtr_feat_map to guests that also have
the architecturally implied features (TCR2 and ATS1A, plus HPDS for S1POE).

Link: https://lore.kernel.org/r/20260930-arm64-gcs-v21-2-3556644cd927@kernel.org
Link: https://lore.kernel.org/r/86fqyn2che.wl-maz@kernel.org

> diff --git a/arch/arm64/kvm/config.c b/arch/arm64/kvm/config.c
> index e93919cadf860..5cb2b759b68d6 100644
> --- a/arch/arm64/kvm/config.c
> +++ b/arch/arm64/kvm/config.c
> @@ -205,6 +205,7 @@ struct reg_feat_map_desc {
>  #define FEAT_MEC		ID_AA64MMFR3_EL1, MEC, IMP
>  #define FEAT_HAFT		ID_AA64MMFR1_EL1, HAFDBS, HAFT
>  #define FEAT_HDBSS		ID_AA64MMFR1_EL1, HAFDBS, HDBSS
> +#define FEAT_HPDS		ID_AA64MMFR1_EL1, HPDS, IMP
>  #define FEAT_HPDS2		ID_AA64MMFR1_EL1, HPDS, HPDS2
>  #define FEAT_BTI		ID_AA64PFR1_EL1, BT, IMP
>  #define FEAT_ExS		ID_AA64MMFR0_EL1, EXS, IMP
> @@ -280,6 +281,25 @@ static bool feat_anerr(struct kvm *kvm)
>  		kvm_has_feat(kvm, ID_AA64MMFR3_EL1, SNERR, FEAT_ANERR));
>  }
>  
> +/*
> + * S1PIE and S1POE imply TCR2 and ATS1A, and S1POE also implies HPDS, 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) &&
> +		kvm_has_feat(kvm, FEAT_HPDS));
> +}
> +

[Severity: High]
Should these implied-feature checks also be folded into kvm_has_s1pie() and
kvm_has_s1poe() in arch/arm64/include/asm/kvm_host.h, as well as the other
S1PIE and S1POE checks in KVM?

Right now, feat_s1pie() and feat_s1poe() are only wired into
hfgrtr_feat_map[] and hfgwtr_feat_map[], which populate
kvm->arch.fgu[HFGRTR_GROUP] for the EL1/EL0 registers, while
kvm_has_s1pie() and kvm_has_s1poe() still only check ID_AA64MMFR3_EL1:

arch/arm64/include/asm/kvm_host.h:
    #define kvm_has_s1pie(k)				\
    	(kvm_has_feat((k), ID_AA64MMFR3_EL1, S1PIE, IMP))

    #define kvm_has_s1poe(k)				\
    	(system_supports_poe() &&			\
    	 kvm_has_feat((k), ID_AA64MMFR3_EL1, S1POE, IMP))

In a nested virtualization guest running at vEL2 when TCR2 is hidden,
accesses to PIR_EL2, PIRE0_EL2, and POR_EL2 trap via HCR_EL2.NV rather than
fine-grained traps. Because encoding_to_fgt[] in triage_sysreg_trap() only
maps the EL1/EL0 encodings, kvm_handle_sys_reg() -> perform_access() falls
back to sysreg_hidden() -> s1pie_el2_visibility() / s1poe_el2_visibility():

arch/arm64/kvm/sys_regs.c:
    static unsigned int s1poe_visibility(const struct kvm_vcpu *vcpu,
    				     const struct sys_reg_desc *rd)
    {
    	if (kvm_has_s1poe(vcpu->kvm))
    		return 0;

    	return REG_HIDDEN;
    }

Because kvm_has_s1pie() and kvm_has_s1poe() return true even when TCR2 is
hidden, perform_access() does not inject an UNDEF. Instead, on VHE,
access_rw() -> vcpu_read_sys_reg() / vcpu_write_sys_reg() ->
locate_register() maps PIR_EL2, PIRE0_EL2, and POR_EL2 to the hardware
SYS_PIR_EL12, SYS_PIRE0_EL12, and SYS_POR_EL12 registers. However,
__sysreg_save_vel2_state() and __sysreg_restore_vel2_state() in
arch/arm64/kvm/hyp/vhe/sysreg-sr.c skip saving and restoring them when
!ctxt_has_tcrx():

    if (ctxt_has_tcrx(&vcpu->arch.ctxt)) {
    	__vcpu_assign_sys_reg(vcpu, TCR2_EL2, read_sysreg_el1(SYS_TCR2));

    	if (ctxt_has_s1pie(&vcpu->arch.ctxt)) {
    		__vcpu_assign_sys_reg(vcpu, PIRE0_EL2, read_sysreg_el1(SYS_PIRE0));
    		__vcpu_assign_sys_reg(vcpu, PIR_EL2, read_sysreg_el1(SYS_PIR));
    	}

    	if (ctxt_has_s1poe(&vcpu->arch.ctxt))
    		__vcpu_assign_sys_reg(vcpu, POR_EL2, read_sysreg_el1(SYS_POR));
    }

Won't a nested guest with S1PIE or S1POE and without TCR2 still leak and
corrupt the physical PIR_EL1, PIRE0_EL1, and POR_EL1 registers across
guests on VHE via PIR_EL2, PIRE0_EL2, and POR_EL2?

Similarly, if userspace enables TCR2 alongside S1PIE or S1POE while hiding
ATS1A or HPDS, leaving kvm_has_s1pie() and kvm_has_s1poe() unchanged means:

- s1pie_visibility() and s1poe_visibility() keep the registers visible to
  userspace and vEL2, and ctxt_has_s1pie() / ctxt_has_s1poe() still context
  switch them.
- s1pie_enabled() and compute_s1poe() in arch/arm64/kvm/at.c still treat
  S1PIE and S1POE as enabled during software AT walks.
- tcr2_el2_feat_map[] in arch/arm64/kvm/config.c still uses FEAT_S1POE and
  FEAT_S1PIE for TCR2_EL2_POE | TCR2_EL2_E0POE and TCR2_EL2_PIE instead of
  feat_s1poe and feat_s1pie.
- __activate_cptr_traps_vhe() in arch/arm64/kvm/hyp/include/hyp/switch.h
  still checks raw kvm_has_feat(vcpu->kvm, ID_AA64MMFR3_EL1, S1POE, IMP)
  when propagating CPACR_EL1_E0POE.

>  static bool feat_sme_smps(struct kvm *kvm)
>  {
>  	/*
> @@ -393,10 +413,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 +494,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,

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261018080921.3942517-1-fuad.tabba@linux.dev?part=1

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] KVM: arm64: Only enable S1PIE and S1POE with the features they imply
  2026-10-18  8:29 ` sashiko-bot
@ 2026-10-18 15:15   ` Fuad Tabba
  0 siblings, 0 replies; 5+ messages in thread
From: Fuad Tabba @ 2026-10-18 15:15 UTC (permalink / raw)
  To: sashiko-reviews; +Cc: kvmarm, Oliver Upton, Marc Zyngier

Sashiko, I see you're on Argon now... way to go!

On Sun, 18 Oct 2026 09:29:22 +0100, sashiko-bot@kernel.org wrote:
[...]
> Won't a nested guest with S1PIE or S1POE and without TCR2 still leak and
> corrupt the physical PIR_EL1, PIRE0_EL1, and POR_EL1 registers across
> guests on VHE via PIR_EL2, PIRE0_EL2, and POR_EL2?

Yes, these trap through HCR_EL2.NV and no FGT bit covers them, so the
FGU misses them. v3 will UNDEF them along with their EL1 counterparts.

> Similarly, if userspace enables TCR2 alongside S1PIE or S1POE while hiding
> ATS1A or HPDS, leaving kvm_has_s1pie() and kvm_has_s1poe() unchanged means:

With TCR2 present, the world switch saves and restores all of these
registers, so nothing leaks. The rest is KVM's handling of an ID
register combination the architecture doesn't allow, which this stable
fix leaves alone.

Cheers,
/fuad

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] KVM: arm64: Only enable S1PIE and S1POE with the features they imply
  2026-10-18  8:09 [PATCH v2] KVM: arm64: Only enable S1PIE and S1POE with the features they imply Fuad Tabba
  2026-10-18  8:29 ` sashiko-bot
@ 2026-10-18 17:37 ` Marc Zyngier
  2026-10-18 18:52   ` Fuad Tabba
  1 sibling, 1 reply; 5+ messages in thread
From: Marc Zyngier @ 2026-10-18 17:37 UTC (permalink / raw)
  To: Fuad Tabba
  Cc: Oliver Upton, kvmarm, linux-arm-kernel, Joey Gouly, Steffen Eiden,
	Suzuki K Poulose, Zenghui Yu, Catalin Marinas, Will Deacon,
	Mark Rutland, Mark Brown, Lorenzo Stoakes, Fuad Tabba,
	linux-kernel

On Sun, 18 Oct 2026 09:09:20 +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, and
> S1POE also implies HPDS. Only enable each for a guest that has the
> features it implies. A guest that has S1PIE or S1POE without them 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>
> ---
> Changes in v2:
> - Also require FEAT_HPDS for S1POE (Marc)
> - Link to v1: https://lore.kernel.org/r/20261015153041.3770032-1-fuad.tabba@linux.dev
> 
> Based on Linux 7.3-rc3 (fd73f4a665989).
> 
>  arch/arm64/kvm/config.c | 28 ++++++++++++++++++++++++----
>  1 file changed, 24 insertions(+), 4 deletions(-)

I'm afraid that this only addresses part of the problem. Ultimately,
the idregs themselves require some sanitisation because KVM uses this
as the source of truth. I was pointing this in my reply to Broonie,
and this is (probably) what sashiko is blabbing on about.

I hacked something that could be used as the bases for such
sanitisation, although it is quite crude and doesn't cover all of the
possible requirements, see below. I've only boot-tested it in a nested
guest, and I'm not even sure it actually works (my test box doesn't
have these features).

	M.

From 6632cbcebbf4b384f209e988deab0924f03dfff8 Mon Sep 17 00:00:00 2001
From: Marc Zyngier <maz@kernel.org>
Date: Sun, 18 Oct 2026 12:07:09 +0100
Subject: [PATCH] KVM: arm64: Sanitise ID_AA64MMFR3_EL1.S1P[IO]E

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 arch/arm64/include/asm/kvm_host.h |  1 +
 arch/arm64/kvm/config.c           | 59 +++++++++++++++++++++++++++++++
 arch/arm64/kvm/sys_regs.c         |  2 ++
 3 files changed, 62 insertions(+)

diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 6be90f8e19813..ab68ddca234c4 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -1742,6 +1742,7 @@ static inline bool kvm_arch_has_irq_bypass(void)
 	return true;
 }
 
+void kvm_idregs_enforce_dependencies(struct kvm *kvm);
 void compute_fgu(struct kvm *kvm, enum fgt_group_id fgt);
 struct resx get_reg_fixed_bits(struct kvm *kvm, enum vcpu_sysreg reg);
 void check_feature_map(void);
diff --git a/arch/arm64/kvm/config.c b/arch/arm64/kvm/config.c
index 6d2650427c5fc..f037a288c9d46 100644
--- a/arch/arm64/kvm/config.c
+++ b/arch/arm64/kvm/config.c
@@ -59,6 +59,7 @@ struct reg_feat_map_desc {
 	const struct reg_bits_to_feat_map feat_map;
 	const struct reg_bits_to_feat_map *bit_feat_map;
 	const unsigned int		  bit_feat_map_sz;
+	const unsigned int		  idreg_idx;
 };
 
 #define __NEEDS_FEAT_3(m, f, w, id, fld, lim)		\
@@ -130,6 +131,21 @@ struct reg_feat_map_desc {
 		.bit_feat_map_sz	= ARRAY_SIZE(m),		\
 	}
 
+/*
+ * Declare the dependency between the features advertised in an id
+ * register, and the set of other features they depend on. This
+ * generates a struct reg_feat_map_desc.
+ */
+#define DECLARE_IDREG_FEAT_MAP(n, r, m, f)				\
+	struct reg_feat_map_desc n = {					\
+		.name			= #r,				\
+		.feat_map		= NEEDS_FEAT(~(r##_RES0 |	\
+						       r##_RES1), f),	\
+		.bit_feat_map		= m,				\
+		.bit_feat_map_sz	= ARRAY_SIZE(m),		\
+		.idreg_idx		= IDREG_IDX(SYS_##r),		\
+	}
+
 #define FEAT_SPE		ID_AA64DFR0_EL1, PMSVer, IMP
 #define FEAT_BRBE		ID_AA64DFR0_EL1, BRBE, IMP
 #define FEAT_TRC_SR		ID_AA64DFR0_EL1, TraceVer, IMP
@@ -1319,6 +1335,14 @@ static const struct reg_bits_to_feat_map vtcr_el2_feat_map[] = {
 static const DECLARE_FEAT_MAP(vtcr_el2_desc, VTCR_EL2,
 			      vtcr_el2_feat_map, FEAT_AA64EL2);
 
+static const struct reg_bits_to_feat_map id_aa64mmfr3_el1_feat_map[] = {
+	NEEDS_FEAT(ID_AA64MMFR3_EL1_S1PIE, feat_s1pie),
+	NEEDS_FEAT(ID_AA64MMFR3_EL1_S1POE, feat_s1poe),
+};
+
+static const DECLARE_IDREG_FEAT_MAP(id_aa64mmfr3_el1_desc, ID_AA64MMFR3_EL1,
+				    id_aa64mmfr3_el1_feat_map, FEAT_AA64EL1);
+
 static const struct reg_bits_to_feat_map ich_hfgrtr_feat_map[] = {
 	NEEDS_FEAT(ICH_HFGRTR_EL2_ICC_APR_EL1 |
 		   ICH_HFGRTR_EL2_ICC_IDRn_EL1 |
@@ -1428,6 +1452,41 @@ void __init check_feature_map(void)
 	check_reg_desc(&ich_hfgitr_desc);
 }
 
+static const struct reg_feat_map_desc *idreg_feat_maps[] = {
+	&id_aa64mmfr3_el1_desc,
+};
+
+static bool idreg_deps(struct kvm *kvm,
+		       const struct reg_feat_map_desc *r)
+{
+	struct resx resx = {};
+	u64 val;
+
+	for (int i = 0; i < r->bit_feat_map_sz; i++) {
+		if ((r->bit_feat_map[i].flags & CALL_FUNC) &&
+		    !r->bit_feat_map[i].match(kvm))
+			resx.res0 |= reg_feat_map_bits(&r->bit_feat_map[i]);
+	}
+
+	val = kvm->arch.id_regs[r->idreg_idx];
+	kvm->arch.id_regs[r->idreg_idx] &= ~resx.res0;
+	kvm->arch.id_regs[r->idreg_idx] |= resx.res1;
+
+	return val != kvm->arch.id_regs[r->idreg_idx];
+}
+
+void kvm_idregs_enforce_dependencies(struct kvm *kvm)
+{
+	bool again;
+
+	do {
+		again = false;
+
+		for (int i = 0; i < ARRAY_SIZE(idreg_feat_maps); i++)
+			again |= idreg_deps(kvm, idreg_feat_maps[i]);
+	} while (again);
+}
+
 static bool idreg_feat_match(struct kvm *kvm, const struct reg_bits_to_feat_map *map)
 {
 	u64 regval = kvm->arch.id_regs[map->regidx];
diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index 4f18ed2793c33..0687b78cd7c2b 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -5872,6 +5872,8 @@ static int kvm_vm_finalize_sys_regs(struct kvm *kvm)
 	if (kvm_id_regs_final(kvm))
 		return 0;
 
+	kvm_idregs_enforce_dependencies(kvm);
+
 	/*
 	 * This hacks into the ID registers, so only perform it when the
 	 * first vcpu runs, or the kvm_set_vm_id_reg() helper will scream.
-- 
2.47.3


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


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] KVM: arm64: Only enable S1PIE and S1POE with the features they imply
  2026-10-18 17:37 ` Marc Zyngier
@ 2026-10-18 18:52   ` Fuad Tabba
  0 siblings, 0 replies; 5+ messages in thread
From: Fuad Tabba @ 2026-10-18 18:52 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: Oliver Upton, kvmarm, linux-arm-kernel, Joey Gouly, Steffen Eiden,
	Suzuki K Poulose, Zenghui Yu, Catalin Marinas, Will Deacon,
	Mark Rutland, Mark Brown, Lorenzo Stoakes, linux-kernel

Hi Marc,

On Sun, 18 Oct 2026 18:37:37 +0100, Marc Zyngier <maz@kernel.org> wrote:
[...]
> I'm afraid that this only addresses part of the problem. Ultimately,
> the idregs themselves require some sanitisation because KVM uses this
> as the source of truth. I was pointing this in my reply to Broonie,
> and this is (probably) what sashiko is blabbing on about.

It seems so. The trap tables were only one of the places that read
S1PIE and S1POE. Sashiko found another: PIR_EL2, PIRE0_EL2 and POR_EL2
accessed from vEL2 have no FGT bit, so accesses to them reached the
hardware registers.

> I hacked something that could be used as the bases for such
> sanitisation, although it is quite crude and doesn't cover all of the
> possible requirements, see below. I've only boot-tested it in a nested
> guest, and I'm not even sure it actually works (my test box doesn't
> have these features).

I'll send your patch as v3, rebased on -rc3 with you as the author,
once I've tested it. I'm dropping v2's trap table changes, since
they're redundant once the ID registers are sanitised. I made two
changes to your patch. The sanitisation now runs before
kvm_init_nv_sysregs(), so the nested RESx masks are computed from the
sanitised values. I dropped the res1 half, which isn't set by
anything.

Cheers,
/fuad

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-10-18 18:53 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-18  8:09 [PATCH v2] KVM: arm64: Only enable S1PIE and S1POE with the features they imply Fuad Tabba
2026-10-18  8:29 ` sashiko-bot
2026-10-18 15:15   ` Fuad Tabba
2026-10-18 17:37 ` Marc Zyngier
2026-10-18 18:52   ` Fuad Tabba

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.