All of lore.kernel.org
 help / color / mirror / Atom feed
From: Marc Zyngier <maz@kernel.org>
To: Alexandru Elisei <alexandru.elisei@arm.com>
Cc: mingo@redhat.com, tglx@linutronix.de, will@kernel.org,
	kvmarm@lists.cs.columbia.edu,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v3 0/4] KVM: arm64: Improve PMU support on heterogeneous systems
Date: Thu, 06 Jan 2022 18:21:27 +0000	[thread overview]
Message-ID: <871r1kvi6w.wl-maz@kernel.org> (raw)
In-Reply-To: <Ydbbivoqo3vUxB4j@monolith.localdoman>

On Thu, 06 Jan 2022 12:07:38 +0000,
Alexandru Elisei <alexandru.elisei@arm.com> wrote:
> 
> Hi Marc,
> 
> On Thu, Dec 30, 2021 at 08:01:10PM +0000, Marc Zyngier wrote:
> > 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?
> 
> This is a good point. When I looked at how the PMU event filter works, I
> saw that KVM doesn't attempt to check that the events are actually
> implemented on the PMU, but somehow skipped over the fact that the PMU
> affects the total number of events available.

That, but also the meaning of the events. Switching PMU after
programmed event filters is really odd, as you don't know what you are
filtering anymore (unless you stick to purely architected events).

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

WARNING: multiple messages have this Message-ID (diff)
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, 06 Jan 2022 18:21:27 +0000	[thread overview]
Message-ID: <871r1kvi6w.wl-maz@kernel.org> (raw)
In-Reply-To: <Ydbbivoqo3vUxB4j@monolith.localdoman>

On Thu, 06 Jan 2022 12:07:38 +0000,
Alexandru Elisei <alexandru.elisei@arm.com> wrote:
> 
> Hi Marc,
> 
> On Thu, Dec 30, 2021 at 08:01:10PM +0000, Marc Zyngier wrote:
> > 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?
> 
> This is a good point. When I looked at how the PMU event filter works, I
> saw that KVM doesn't attempt to check that the events are actually
> implemented on the PMU, but somehow skipped over the fact that the PMU
> affects the total number of events available.

That, but also the meaning of the events. Switching PMU after
programmed event filters is really odd, as you don't know what you are
filtering anymore (unless you stick to purely architected events).

Thanks,

	M.

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

  reply	other threads:[~2022-01-06 18:21 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-12-13 15:23 [PATCH v3 0/4] KVM: arm64: Improve PMU support on heterogeneous systems Alexandru Elisei
2021-12-13 15:23 ` Alexandru Elisei
2021-12-13 15:23 ` [PATCH v3 1/4] perf: Fix wrong name in comment for struct perf_cpu_context Alexandru Elisei
2021-12-13 15:23   ` Alexandru Elisei
2021-12-13 15:23 ` [PATCH v3 2/4] KVM: arm64: Keep a list of probed PMUs Alexandru Elisei
2021-12-13 15:23   ` Alexandru Elisei
2021-12-14  7:23   ` Reiji Watanabe
2021-12-14  7:23     ` Reiji Watanabe
2021-12-14 12:30   ` Marc Zyngier
2021-12-14 12:30     ` Marc Zyngier
2022-01-06 11:46     ` Alexandru Elisei
2022-01-06 11:46       ` Alexandru Elisei
2021-12-13 15:23 ` [PATCH v3 3/4] KVM: arm64: Add KVM_ARM_VCPU_PMU_V3_SET_PMU attribute Alexandru Elisei
2021-12-13 15:23   ` Alexandru Elisei
2021-12-14 12:28   ` Marc Zyngier
2021-12-14 12:28     ` Marc Zyngier
2022-01-06 11:54     ` Alexandru Elisei
2022-01-06 11:54       ` Alexandru Elisei
2022-01-06 18:16       ` Marc Zyngier
2022-01-06 18:16         ` Marc Zyngier
2022-01-07 11:08         ` Alexandru Elisei
2022-01-07 11:08           ` Alexandru Elisei
2022-01-07 14:35           ` Marc Zyngier
2022-01-07 14:35             ` Marc Zyngier
2021-12-13 15:23 ` [PATCH v3 4/4] KVM: arm64: Refuse to run VCPU if the PMU doesn't match the physical CPU Alexandru Elisei
2021-12-13 15:23   ` Alexandru Elisei
2021-12-30 20:01 ` [PATCH v3 0/4] KVM: arm64: Improve PMU support on heterogeneous systems Marc Zyngier
2021-12-30 20:01   ` Marc Zyngier
2022-01-06 12:07   ` Alexandru Elisei
2022-01-06 12:07     ` Alexandru Elisei
2022-01-06 18:21     ` Marc Zyngier [this message]
2022-01-06 18:21       ` Marc Zyngier

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=871r1kvi6w.wl-maz@kernel.org \
    --to=maz@kernel.org \
    --cc=alexandru.elisei@arm.com \
    --cc=kvmarm@lists.cs.columbia.edu \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=mingo@redhat.com \
    --cc=tglx@linutronix.de \
    --cc=will@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.