Re: [PATCH v5 4/7] KVM: arm64: Fix missing traps of guest accesses to the MPAM registers

[Oliver Upton <oliver.upton@linux.dev>]

On Thu, Oct 17, 2024 at 11:58:49AM +0100, Joey Gouly wrote:
> On Wed, Oct 16, 2024 at 05:10:17PM -0700, Oliver Upton wrote:
> > Hi Joey,
> > 
> > On Tue, Oct 15, 2024 at 02:39:20PM +0100, Joey Gouly wrote:
> > > +static inline void  __activate_traps_mpam(struct kvm_vcpu *vcpu)
> > > +{
> > > +	u64 r = MPAM2_EL2_TRAPMPAM0EL1 | MPAM2_EL2_TRAPMPAM1EL1;
> > > +
> > > +	if (!cpus_support_mpam())
> > > +		return;
> > > +
> > > +	/* trap guest access to MPAMIDR_EL1 */
> > > +	if (mpam_cpus_have_mpam_hcr()) {
> > > +		write_sysreg_s(MPAMHCR_EL2_TRAP_MPAMIDR_EL1, SYS_MPAMHCR_EL2);
> > > +	} else {
> > > +		/* From v1.1 TIDR can trap MPAMIDR, set it unconditionally */
> > > +		r |= MPAM2_EL2_TIDR;
> > > +	}
> > > +
> > > +	write_sysreg_s(r, SYS_MPAM2_EL2);
> > > +}
> > > +
> > > +static inline void __deactivate_traps_mpam(void)
> > > +{
> > > +	if (!cpus_support_mpam())
> > > +		return;
> > > +
> > > +	write_sysreg_s(0, SYS_MPAM2_EL2);
> > > +
> > > +	if (mpam_cpus_have_mpam_hcr())
> > > +		write_sysreg_s(MPAMHCR_HOST_FLAGS, SYS_MPAMHCR_EL2);
> > > +}
> > 
> > TBH, I think our trap configuration should *not* be conditioned on
> > CONFIG_ARM64_MPAM. Otherwise we're silently allowing the guest to change
> > things under the nose of KVM/host kernel, assuming an unkind firmware
> > that left the EL2 trap configuration in a permissive state.
> > 
> > WDYT about detecting the feature && enforcing traps regardless of the Kconfig?
> 
> I had actually thought about the same thing. I spoke with James and he agrees,
> so I will look into removing that.
> 
> I will probably end up removing the Kconfig entirely, it can be added back in
> later, when actual support for MPAM is added.

Sounds good, thanks Joey!

If we go down this route, I'm guessing we can also skip the boot
time EL2 setup portion of it (for now). That'd constrain the fossilized
EL3 issue to *just* failures to run KVM VMs as opposed to kernels not
booting at all.

-- 
Thanks,
Oliver