Re: [PATCH v1 1/3] KVM: arm64: Hide S1POE from guests when not supported by the host

Hi Marc,

On Thu, 12 Feb 2026 at 09:29, Marc Zyngier [off-list ref] wrote:

Hi Fuad,

On Thu, 12 Feb 2026 09:02:50 +0000,
Fuad Tabba [off-list ref] wrote:

quoted

When CONFIG_ARM64_POE is disabled, KVM does not save/restore POR_EL1.
However, ID_AA64MMFR3_EL1 sanitisation currently exposes the feature to
guests whenever the hardware supports it, ignoring the host kernel
configuration.

This is the umpteenth time we get caught by this. PAN was the latest
instance until this one. Maybe an approach would be to have a default
override when a config option is not enabled, so that KVM is
consistent with the rest of the kernel?

I spoke to Will about this, and one thing he'll look into is whether
this value in `struct arm64_ftr_reg` can be made consistent with the
cpu configuration itself (in cpufeature.c itself) . This would avoid
the problem altogether if possible. The question is whether the kernel
needs to somehow know that a certain feature exists even if it's
disabled in the config...

If he thinks it's not doable at that level, I'll look into
alternatives to make it correct by construction.

quoted

If a guest detects this feature and attempts to use it, the host will
fail to context-switch POR_EL1, potentially leading to state corruption.

Fix this by masking ID_AA64MMFR3_EL1.S1POE and preventing KVM from
advertising the feature when the host does not support it, i.e.,
system_supports_poe() is false.

Fixes: 70ed7238297f ("KVM: arm64: Sanitise ID_AA64MMFR3_EL1")
Signed-off-by: Fuad Tabba <redacted>
---
 arch/arm64/include/asm/kvm_host.h | 3 ++-
 arch/arm64/kvm/sys_regs.c         | 3 +++
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index ac7f970c7883..7af72ca749a6 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h

@@ -1592,7 +1592,8 @@ void kvm_set_vm_id_reg(struct kvm *kvm, u32 reg, u64 val);
      (kvm_has_feat((k), ID_AA64MMFR3_EL1, S1PIE, IMP))

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

Why do we need to further key this on system_supports_poe()? I can see
this is a potential optimisation, but I don't think this is part of
the minimal fix.

I did this to be consistent with similar checks, e.g., kvm_has_fpmr().
I can remove it or put it in a separate patch, whichever you prefer.

quoted

 #define kvm_has_ras(k)                                       \
      (kvm_has_feat((k), ID_AA64PFR0_EL1, RAS, IMP))

diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index 88a57ca36d96..237e8bd1cf29 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c

@@ -1816,6 +1816,9 @@ static u64 __kvm_read_sanitised_id_reg(const struct kvm_vcpu *vcpu,
                     ID_AA64MMFR3_EL1_SCTLRX |
                     ID_AA64MMFR3_EL1_S1POE |
                     ID_AA64MMFR3_EL1_S1PIE;
+
+             if (!system_supports_poe())
+                     val &= ~ID_AA64MMFR3_EL1_S1POE;

How about S1PIE? It seems to have a similar problem, in the sense that
it has extra state. But I guess because we don't put it behind a
config option, we context-switch it anyway and all is good?

Like you said, S1PIE doesn't have a config option. I thought if I were
to add one to S1PIE, then why not add one to the other features that
don't have config options... It is conceivable that a config option
might be added in the future, but like you noted above, I think we
need a deeper fix that nips this problem in the bud.

Let's see if Will thinks that we can fix this at the `arm64`
cpufeature layer, rather than somewhere in KVM.

Cheers,
/fuad

Thanks,

        M.

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