Re: [PATCH v1 1/3] KVM: arm64: Hide S1POE from guests when not supported by the host
From: Marc Zyngier <maz@kernel.org>
Date: 2026-02-12 15:35:46
Also in:
kvm, kvmarm, stable
Subsystem:
arm64 port (aarch64 architecture), the rest · Maintainers:
Catalin Marinas, Will Deacon, Linus Torvalds
On Thu, 12 Feb 2026 09:41:22 +0000, Fuad Tabba [off-list ref] wrote:
Hi Marc, On Thu, 12 Feb 2026 at 09:29, Marc Zyngier [off-list ref] wrote:quoted
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.
What I currently have for that is rather ugly:
diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
index 72f39cecce93a..3bde0ad5ea972 100644
--- a/arch/arm64/include/asm/cpufeature.h
+++ b/arch/arm64/include/asm/cpufeature.h@@ -971,6 +971,7 @@ struct arm64_ftr_reg *get_arm64_ftr_reg(u32 sys_id); extern struct arm64_ftr_override id_aa64mmfr0_override; extern struct arm64_ftr_override id_aa64mmfr1_override; extern struct arm64_ftr_override id_aa64mmfr2_override; +extern struct arm64_ftr_override id_aa64mmfr3_override; extern struct arm64_ftr_override id_aa64pfr0_override; extern struct arm64_ftr_override id_aa64pfr1_override; extern struct arm64_ftr_override id_aa64zfr0_override;
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index 1a7eec542675b..32069da9651bf 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c@@ -778,6 +778,7 @@ static const struct arm64_ftr_bits ftr_raz[] = { struct arm64_ftr_override __read_mostly id_aa64mmfr0_override; struct arm64_ftr_override __read_mostly id_aa64mmfr1_override; struct arm64_ftr_override __read_mostly id_aa64mmfr2_override; +struct arm64_ftr_override __read_mostly id_aa64mmfr3_override; struct arm64_ftr_override __read_mostly id_aa64pfr0_override; struct arm64_ftr_override __read_mostly id_aa64pfr1_override; struct arm64_ftr_override __read_mostly id_aa64zfr0_override;
@@ -850,7 +851,8 @@ static const struct __ftr_reg_entry { &id_aa64mmfr1_override), ARM64_FTR_REG_OVERRIDE(SYS_ID_AA64MMFR2_EL1, ftr_id_aa64mmfr2, &id_aa64mmfr2_override), - ARM64_FTR_REG(SYS_ID_AA64MMFR3_EL1, ftr_id_aa64mmfr3), + ARM64_FTR_REG_OVERRIDE(SYS_ID_AA64MMFR3_EL1, ftr_id_aa64mmfr3, + &id_aa64mmfr3_override), ARM64_FTR_REG(SYS_ID_AA64MMFR4_EL1, ftr_id_aa64mmfr4), /* Op1 = 0, CRn = 10, CRm = 4 */
diff --git a/arch/arm64/kernel/image-vars.h b/arch/arm64/kernel/image-vars.h
index 85bc629270bd9..202e165a4680c 100644
--- a/arch/arm64/kernel/image-vars.h
+++ b/arch/arm64/kernel/image-vars.h@@ -51,6 +51,7 @@ PI_EXPORT_SYM(id_aa64isar2_override); PI_EXPORT_SYM(id_aa64mmfr0_override); PI_EXPORT_SYM(id_aa64mmfr1_override); PI_EXPORT_SYM(id_aa64mmfr2_override); +PI_EXPORT_SYM(id_aa64mmfr3_override); PI_EXPORT_SYM(id_aa64pfr0_override); PI_EXPORT_SYM(id_aa64pfr1_override); PI_EXPORT_SYM(id_aa64smfr0_override);
diff --git a/arch/arm64/kernel/pi/idreg-override.c b/arch/arm64/kernel/pi/idreg-override.c
index e5ea280452c3b..b8dbe02e53171 100644
--- a/arch/arm64/kernel/pi/idreg-override.c
+++ b/arch/arm64/kernel/pi/idreg-override.c@@ -24,10 +24,12 @@ static u64 __boot_status __initdata; typedef bool filter_t(u64 val); +typedef void cfg_override_t(struct arm64_ftr_override *); struct ftr_set_desc { char name[FTR_DESC_NAME_LEN]; PREL64(struct arm64_ftr_override, override); + PREL64(cfg_override_t, cfg_override); struct { char name[FTR_DESC_FIELD_LEN]; u8 shift;
@@ -106,6 +108,22 @@ static const struct ftr_set_desc mmfr2 __prel64_initconst = { }, }; +static void __init cfg_mmfr3_override(struct arm64_ftr_override *override) +{ +#ifndef CONFIG_ARM64_POE + override->mask |= ID_AA64MMFR3_EL1_S1POE_MASK; +#endif +} + +static const struct ftr_set_desc mmfr3 __prel64_initconst = { + .name = "id_aa64mmfr3", + .override = &id_aa64mmfr3_override, + .cfg_override = cfg_mmfr3_override, + .fields = { + {} + }, +}; + static bool __init pfr0_sve_filter(u64 val) { /*
@@ -221,6 +239,7 @@ PREL64(const struct ftr_set_desc, reg) regs[] __prel64_initconst = { { &mmfr0 }, { &mmfr1 }, { &mmfr2 }, + { &mmfr3 }, { &pfr0 }, { &pfr1 }, { &isar1 },
@@ -398,14 +417,19 @@ void __init init_feature_override(u64 boot_status, const void *fdt, { struct arm64_ftr_override *override; const struct ftr_set_desc *reg; + cfg_override_t *cfg_override; int i; for (i = 0; i < ARRAY_SIZE(regs); i++) { reg = prel64_pointer(regs[i].reg); override = prel64_pointer(reg->override); + cfg_override = prel64_pointer(reg->cfg_override); override->val = 0; override->mask = 0; + + if (cfg_override) + cfg_override(override); } __boot_status = boot_status;
which works, but is not super friendly. Looking at the arm64_ftr_reg structure, this could work if FTR_VISIBLE_IF_IS_ENABLED() didn't simply put "HIDDEN" when the feature is not present, but forced things to be disabled altogether. The problem is that "HIDDEN" means not shown to userspace, and that we have plenty of HIDDEN features that must make it into KVM. I'll have a think. M. -- Without deviation from the norm, progress is not possible.