Re: [PATCH v3 0/4] KVM: arm64: Improve PMU support on heterogeneous systems

From: Marc Zyngier <maz@kernel.org>
To: Alexandru Elisei <alexandru.elisei@arm.com>
Cc: james.morse@arm.com, suzuki.poulose@arm.com, will@kernel.org,
	mark.rutland@arm.com, linux-arm-kernel@lists.infradead.org,
	kvmarm@lists.cs.columbia.edu, tglx@linutronix.de,
	mingo@redhat.com, peter.maydell@linaro.org
Subject: Re: [PATCH v3 0/4] KVM: arm64: Improve PMU support on heterogeneous systems
Date: Thu, 30 Dec 2021 20:01:10 +0000	[thread overview]
Message-ID: <87bl0xzwu1.wl-maz@kernel.org> (raw)
In-Reply-To: <20211213152309.158462-1-alexandru.elisei@arm.com>

Alex,

On Mon, 13 Dec 2021 15:23:05 +0000,
Alexandru Elisei <alexandru.elisei@arm.com> wrote:
> 
> (CC'ing Peter Maydell in case this might be of interest to qemu)
> 
> The series can be found on a branch at [1], and the kvmtool support at [2].
> The kvmtool patches are also on the mailing list [3] and haven't changed
> since v1.
> 
> Detailed explanation of the issue and symptoms that the patches attempt to
> correct can be found in the cover letter for v1 [4].
> 
> A summary of the problem is that on heterogeneous systems KVM will always
> use the same PMU for creating the VCPU events for *all* VCPUs regardless of
> the physical CPU on which the VCPU is running, leading to events suddenly
> stopping and resuming in the guest as the VCPU thread gets migrated across
> different CPUs.
> 
> This series proposes to fix this behaviour by allowing the user to specify
> which physical PMU is used when creating the VCPU events needed for guest
> PMU emulation. When the PMU is set, KVM will refuse to the VCPU on a
> physical which is not part of the supported CPUs for the specified PMU. The
> restriction is that all VCPUs must use the same PMU to avoid emulating an
> asymmetric platform.
> 
> The default behaviour stays the same - without userspace setting the PMU,
> events will stop counting if the VCPU is scheduled on the wrong CPU.
> 
> Tested with a hacked version of kvmtool that does the PMU initialization
> from the VCPU thread as opposed to from the main thread. Tested on
> rockpro64 by testing what happens when all VCPUs having the same PMU, one
> random VCPU having a different PMU than the other VCPUs and one random VCPU
> not having the PMU set (each test was run 1,000 times on the little cores
> and 1,000 times on the big cores).
> 
> Also tested on an Altra by testing all VCPUs having the same PMU, all VCPUs
> not having a PMU set, and one random VCPU not having the PMU set; the VM
> had 64 threads in each of the tests and each test was run 10,000 times.

Came back to this series, and found more problems. On top of the
remarks I had earlier (the per-CPU data structures that really should
per VM, the disappearing attribute size), what happens when event
filters are already registered and that you set a specific PMU?

I took the matter in my own hands (the joy of being in quarantine) and
wrote whatever fixes I thought were necessary[1].

Please have a look.

	M.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git/log/?h=kvm-arm64/pmu-bl

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel