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

[Joey Gouly <joey.gouly@arm.com>]

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)
	- 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.

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

Thanks,
Joey

> 
> > +}
> > +
> > +static inline void __deactivate_traps_mpam(void)
> > +{
> > +	if (!cpus_support_mpam())
> > +		return;
> > +
> > +	write_sysreg_s(0, SYS_MPAM2_EL2);
> 
> Same thing.
> 
> > +
> > +	if (mpam_cpus_have_mpam_hcr())
> > +		write_sysreg_s(MPAMHCR_HOST_FLAGS, SYS_MPAMHCR_EL2);
> > +}
> > +
> 
> Thanks,
> 
> 	M.
> 
> -- 
> Without deviation from the norm, progress is not possible.