[PATCH v1 1/3] KVM: arm64: Hide S1POE from guests when not supported by the host
Fuad Tabba
tabba at google.com
Thu Feb 12 10:53:05 PST 2026
/fuad
On Thu, 12 Feb 2026 at 15:35, Marc Zyngier <maz at kernel.org> wrote:
>
> On Thu, 12 Feb 2026 09:41:22 +0000,
> Fuad Tabba <tabba at google.com> wrote:
> >
> > Hi Marc,
> >
> > On Thu, 12 Feb 2026 at 09:29, Marc Zyngier <maz at kernel.org> wrote:
> > >
> > > Hi Fuad,
> > >
> > > On Thu, 12 Feb 2026 09:02:50 +0000,
> > > Fuad Tabba <tabba at google.com> wrote:
> > > >
> > > > 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.
Ouch! Yeah, no easy solutions here. For now, as a fix to be able to
backport, would you like me to respin this without the change to
kvm_has_s1poe(), or with that change as a separate patch?
Cheers,
/fuad
> 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.
More information about the linux-arm-kernel
mailing list