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

[Reiji Watanabe <reijiw@google.com>]

Hi Alex,

On Mon, Dec 6, 2021 at 9:02 AM 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 brief 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.

Just to confirm, this series provides an API for userspace to request
KVM to detect a wrong affinity setting due to a userspace bug so that
userspace can get an error at KVM_RUN instead of leading to events
suddenly stopping, correct ?


> The default behaviour stays the same - without userspace setting the PMU,
> events will stop counting if the VCPU is scheduled on the wrong CPU.

Can't we fix the default behavior (in addition to the current fix) ?
(Do we need to maintain the default behavior ??)
IMHO I feel it is better to prevent userspace from configuring PMU
for guests on such heterogeneous systems rather than leading to
events suddenly stopping even as the default behavior.

Thanks,
Reiji


>
> Changes since v1:
>
> - Rebased on top of v5.16-rc4
>
> - Implemented review comments: protect iterating through the list of PMUs
>   with a mutex, documentation changes, initialize vcpu-arch.supported_cpus
>   to cpu_possible_mask, changed vcpu->arch.cpu_not_supported to a VCPU
>   flag, set exit reason to KVM_EXIT_FAIL_ENTRY and populate fail_entry when
>   the VCPU is run on a CPU not in the PMU's supported cpumask. Many thanks
>   for the review!
>
> [1] https://gitlab.arm.com/linux-arm/linux-ae/-/tree/pmu-big-little-fix-v2
> [2] https://gitlab.arm.com/linux-arm/kvmtool-ae/-/tree/pmu-big-little-fix-v1
> [3] https://www.spinics.net/lists/arm-kernel/msg933584.html
> [4] https://www.spinics.net/lists/arm-kernel/msg933579.html
>
> Alexandru Elisei (4):
>   perf: Fix wrong name in comment for struct perf_cpu_context
>   KVM: arm64: Keep a list of probed PMUs
>   KVM: arm64: Add KVM_ARM_VCPU_PMU_V3_SET_PMU attribute
>   KVM: arm64: Refuse to run VCPU if the PMU doesn't match the physical
>     CPU
>
>  Documentation/virt/kvm/devices/vcpu.rst | 29 +++++++++++
>  arch/arm64/include/asm/kvm_host.h       | 12 +++++
>  arch/arm64/include/uapi/asm/kvm.h       |  4 ++
>  arch/arm64/kvm/arm.c                    | 19 ++++++++
>  arch/arm64/kvm/pmu-emul.c               | 64 +++++++++++++++++++++++--
>  include/kvm/arm_pmu.h                   |  6 +++
>  include/linux/perf_event.h              |  2 +-
>  tools/arch/arm64/include/uapi/asm/kvm.h |  1 +
>  8 files changed, 132 insertions(+), 5 deletions(-)
>
> --
> 2.34.1
>
> _______________________________________________
> kvmarm mailing list
> kvmarm@lists.cs.columbia.edu
> https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm