Re: [PATCH v3] KVM: arm64: Trap guest MPAM accesses whenever MPAM is implemented

[Will Deacon <will@kernel.org>]

On Tue, Sep 15, 2026 at 07:51:00PM +0100, Fuad Tabba wrote:
> On Sun, 13 Sept 2026 at 11:00, Marc Zyngier <maz@kernel.org> wrote:
> [...]
> > > diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
> [...]
> > >  static void cpu_hyp_init_context(void)
> > >  {
> > > + u64 pfr0 = __read_sysreg_by_encoding(SYS_ID_AA64PFR0_EL1);
> > > + u64 pfr1 = __read_sysreg_by_encoding(SYS_ID_AA64PFR1_EL1);
> > > +
> >
> > Why not directly read_sysreg(id_aa64pfr0_el1) and co?
> > __read_sysreg_by_encoding() is useful when the encoding comes from a
> > variable, but it looks odd in the case of a literal sysreg.
> 
> It applies the arm64.nompam override, which is what
> finalise_el2_state() tests. With the override set, EL2 setup leaves
> the MPAM traps alone, and a raw read_sysreg() would still see MPAM in
> the ID registers, so KVM would write MPAM2_EL2 on exactly the firmware
> the option exists for. I'll add a comment.
> 
> [...]
> > > diff --git a/arch/arm64/kvm/hyp/include/hyp/switch.h b/arch/arm64/kvm/hyp/include/hyp/switch.h
> [...]
> > > @@ -298,14 +298,17 @@ static inline void  __activate_traps_mpam(struct kvm_vcpu *vcpu)
> [...]
> > >   /* trap guest access to MPAMIDR_EL1 */
> > > - if (system_supports_mpam_hcr()) {
> > > + if (read_sysreg_s(SYS_MPAMIDR_EL1) & MPAMIDR_EL1_HAS_HCR) {
> >
> > This is going to suck under NV. The host hypervisor is of course going
> > to set MPAMHCR_EL2.TRAP_MPAMIDR_EL1, and we're in for a recursive trap
> > on the hottest possible path in KVM. Which is silly as the actual
> > write to MPAMHCR_EL2 is free (it lands in NVMem[]).
> >
> > This really should be replaced by a flag called HAS_MPAM_HCR, just
> > like you have HAS_MPAM.
> 
> I'll probe MPAMIDR_EL1.HAS_HCR once in cpu_hyp_init_context(), next to
> HAS_MPAM, and test the flag in both trap functions instead.
> 
> I'll hold off on v4 until Will has had a chance to comment as well.

I'm planning to generalise the logic you added recently in
gmid_el1_accessible() so that __cpuinfo_store_cpu() stores the values
in 'struct cpuinfo_arm64' with the overrides already applied. I don't
think that directly impacts this patch, but it should make the general
shape of things easier to reason about. I also need it for the parallel
hotplug work.

If you fancy helping with that, please let me know.

Will