Re: [PATCH 0/3] KVM: arm64: nv: Add EL2 PMU event filtering support

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

Hey,

On Sun, Aug 25, 2024 at 09:16:33AM +0100, Marc Zyngier wrote:
> On Sat, 24 Aug 2024 01:13:59 +0100,
> Oliver Upton <oliver.upton@linux.dev> wrote:
> > 
> > We allow a nested VM to be configured with a vPMU, although it isn't
> > entirely functional. We do not currently respect the EL2 event filter
> > configuration from the guest hypervisor, which really needs to be
> > applied to EL1 when in a hyp context.
> > 
> > Series to do just that. Tested on Neoverse-V2 which fortunately
> > implements PMUv3 and NV, unlike the fruity stuff I'm typically using...
> > 
> > Oliver Upton (3):
> >   KVM: arm64: Add helpers to determine if PMC counts at a given EL
> >   KVM: arm64: nv: Honor NSH filter when in hyp context
> >   KVM: arm64: nv: Reprogram PMU events affected by nested transition
> > 
> >  arch/arm64/kvm/emulate-nested.c |  4 ++
> >  arch/arm64/kvm/pmu-emul.c       | 77 ++++++++++++++++++++++++++++-----
> >  include/kvm/arm_pmu.h           |  3 ++
> >  3 files changed, 72 insertions(+), 12 deletions(-)
> > 
> > 
> > base-commit: 7c626ce4bae1ac14f60076d00eafe71af30450ba
> 
> I had a quick look, and definitely like the way it is shaping up (it
> does the job without fuss, and NV isn't about performance anyway).
> 
> However, I think it needs a bit more work so that MDCR_EL2.HPMN is
> correctly handled. This shouldn't be hard to do, and if FEAT_FGT is
> supported in the guest, then we could even expose FEAT_HPMN0 (though I
> haven't worked out yet why FGT is a dependency).

Right, I was hoping to spoon-feed some more PMU patches going forward,
since we already allow the NV+PMU combination and can treat it as a 'bugfix'.
But I'm happy to throw some more cycles at the problem.

I think there are some gaps in the way the other coarse-grained PMU
traps are handled (TPM, TPMCR), since it looks like they also apply to
Host EL0 based on the pseudocode. I don't want to have a separate
mechanism for describing these 'InHost' traps, so I'm gonna try and
address this like so:

 - Indicate if CGTs apply to host EL0 in the trap_bits descriptor

 - Steal a trap_config bit to indicate if a sysreg can trap while
   InHost, and:

 - Mark a trap_config as InHost if any of the associated CGTs are
   InHost when inserting in the xarray

That way I can preserve the early return for is_hyp_ctxt() for all but a
few trap configs.

Thoughts?

-- 
Thanks,
Oliver