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

[Marc Zyngier <maz@kernel.org>]

On Thu, 17 Oct 2024 14:06:38 +0100,
Joey Gouly <joey.gouly@arm.com> wrote:
> 
> Hi Marc,
> 
> Thanks for looking!
> 
> On Thu, Oct 17, 2024 at 12:54:09PM +0100, Marc Zyngier wrote:
> > On Tue, 15 Oct 2024 14:39:20 +0100,
> > Joey Gouly <joey.gouly@arm.com> wrote:
> > > 
> > > @@ -204,6 +205,35 @@ static inline void __deactivate_traps_hfgxtr(struct kvm_vcpu *vcpu)
> > >  		__deactivate_fgt(hctxt, vcpu, kvm, HAFGRTR_EL2);
> > >  }
> > >  
> > > +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);
> > 
> > Please use the write_sysreg_el2() accessor, so that VHE under NV has
> > an easier time, should we ever get there.
> 
> Are you suggesting this, to avoid a trap (and instead get the register-memory
> redirection)?
> 
> The two reasons I could see to keep it as-is:
> 	- The name changes from MPAM2_EL2 to MPAM1_EL1 (1 and 2 suffixes after
> 	  MPAM)

Arghhh.

> 	- It's setting trap behaviour, which is an EL2 concept (and are RES0
> 	  bits in the MPAM1_EL1 reg). Other trap registers such as HCRX_EL2 use
> 	  that register directly, however they don't have an _EL1 counterpart so have to.

Arghhh again.

So this is yet another register that doesn't match FEAT_NV2p1, by
having RES0 fields in its EL2-equivalent-EL1 register, and we can't
even use MPAM1_EL1 for NV.

> I'm fine to change it, just want to clarify before I do.

Nah, don't bother. This thing can't be rescued.

Thanks for the gory details!

	M.

-- 
Without deviation from the norm, progress is not possible.