Re: [PATCH 1/2] KVM: arm64: Fix MDCR_EL2.HPMN reset value

[Marc Zyngier <maz@kernel.org>]

On Wed, 19 Feb 2025 19:04:12 +0000,
Oliver Upton <oliver.upton@linux.dev> wrote:
> 
> On Wed, Feb 19, 2025 at 02:03:49PM +0000, Marc Zyngier wrote:
> > On Mon, 17 Feb 2025 18:53:50 +0000, Oliver Upton <oliver.upton@linux.dev> wrote:
> > > What do you think about adding a new vCPU attribute for selecting the
> > > number of counters for a VM? We can allow non-nested VMs to use the
> > > 'old' method of writing PMCR_EL0.N and force nested VMs to use the
> > > attribute.
> > 
> > VCPU attribute? or PMU attribute? I'm really not keen on the former,
> > but the latter is probably workable, as it is VM-wide, similar to the
> > way we keep track of pmcr_n.
> 
> Well the _existing_ PMU attributes are actually vCPU attributes. I do
> agree that accessing them as a VM attribute makes more sense, but that's
> the UAPI we already have...

Gah, I remember now. Someone please take the API to the backyard...

> 
> > > We can then enforce ordering on the attribute and prevent it from being
> > > used after vCPU reset.
> > 
> > How would that work? Do you really want to mandate the PMU selection
> > (with its counter capping) to strictly occur between vcpu creation and
> > init?
> > 
> > This would, for example, break kvmtool which has these two operations
> > back-to-back, and sneaking new device-specific actions in the middle
> > is a bit unpalatable (there is a split between VM-wide and per-vcpu
> > actions).
> > 
> > Any idea?
> 
> If we want to do this the 'right' way, we should provide VM attributes
> for selecting the PMU implementation / configuring the event filter
> to complement an attribute for setting the number of event counters.
> 
> I don't want to have a mix-and-match approach where vPMU attributes are
> scattered between the vCPU and the VM since it requires a similar amount
> of gymnastics in userspace to set crap up.

I agree on not mixing vCPU and VM scoped attributes, even if that
amounts to the same thing in the back-end. But freezing the number of
counters on vcpu reset is something that should be considered very
carefully, and I fear it breaks existing models -- specially given
that this is yet another one-off.

I wonder if we should instead make use of the KVM_ARM_VCPU_FINALIZE
ioctl just like we do for SVE, passing KVM_ARM_VCPU_PMU_V3 instead.
This would make use of an existing mechanism and lock the PMU for good
(implementation, IRQ, number of counters...).

	M.

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