Re: [PATCH v2] KVM: arm64: Only enable S1PIE and S1POE with the features they imply
From: Marc Zyngier
Date: Mon Oct 19 2026 - 03:36:21 EST
On Sun, 18 Oct 2026 19:52:35 +0100,
Fuad Tabba <fuad.tabba@xxxxxxxxx> wrote:
>
> Hi Marc,
>
> On Sun, 18 Oct 2026 18:37:37 +0100, Marc Zyngier <maz@xxxxxxxxxx> 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,
Please at least add yourself as a co-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.
Isn't that already the case? At least with kvmarm-7.4,
kvm_vm_finalize_sys_regs() is called before kvm_init_nv_sysregs().
> I dropped the res1 half, which isn't set by anything.
I'd like to keep this, as it will eventually get used, just like the
recompute loop is a bit useless for now, but will become useful in the
future as dependencies get recomputed.
The way I imagine this to work is that each sanitised field entry will
point to a subtable containing the encoding of the field and the a
function that checks for the dependency. That's not required for now,
as S1P[IO]E is only a boolean, but I expect some features to
eventually need something more fine-grained.
Thanks,
M.
--
Without deviation from the norm, progress is not possible.