From: Sean Christopherson <seanjc@google.com>
To: Like Xu <like.xu.linux@gmail.com>
Cc: Peter Zijlstra <peterz@infradead.org>,
Kan Liang <kan.liang@linux.intel.com>,
Adrian Hunter <adrian.hunter@intel.com>,
Andi Kleen <ak@linux.intel.com>,
Jim Mattson <jmattson@google.com>,
Paolo Bonzini <pbonzini@redhat.com>,
linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org,
x86@kernel.org, kvm@vger.kernel.org
Subject: Re: [PATCH RFC 1/3] KVM + perf: Rename *_intel_pt_intr() for generic usage
Date: Mon, 3 Oct 2022 19:39:09 +0000 [thread overview]
Message-ID: <Yzs6XTxOp7wxgmJO@google.com> (raw)
In-Reply-To: <20220926142938.89608-2-likexu@tencent.com>
On Mon, Sep 26, 2022, Like Xu wrote:
> From: Like Xu <likexu@tencent.com>
>
> The perf_guest_info_callbacks is common to KVM, while intel_pt is not,
> not even common to x86. In the VMX context, it makes sense to hook
> up the intel_pt specific hook, and given the uniqueness of this usage,
> calling the generic callback in the explicit location of the perf context
> is not functionally broken.
But it's extremely misleading. If I were a developer writing the perf hooks for
a different architecture, I would expect perf_handle_guest_intr() to be called on
_every_ perf interrupt that occurred in the guest.
Genericizing the hook also complicates wiring up the hook and consuming the interrupt
type. E.g. patch 3 is buggy; it wires up the VMX handler if and only if PT is in
PT_MODE_HOST_GUEST, and then takes a dependency on that buggy behavior by not
checking if Intel PT is supported in the now-generic vmx_handle_guest_intr().
This also doesn't really clean up the API from a non-x86 perspective, it just doesn't
make it any worse, i.e. other architectures are still exposed to an x86-specific hook.
Unless we anticipate ARM or RISC-V (which IIRC is gaining PMU support "soon") needing
to hook into "special" perf interrupts, it might be better to figure out a way to make
the hooks themselves more extensible for per-arch behavior. E.g similar to
kvm_vcpu and kvm_vcpu_arch, add an embedded arch (or vice versa) struct in
perf_guest_info_callbacks plus a perf-internal arch hook to update static calls,
and use that to wire up handle_intel_pt_int for x86. It'll require more work up
front, but in theory it will require less maintenance in the long run.
> Rename a bunch of intel_pt_intr() functions to the generic guest_intr().
> No functional change intended.
This changelog never says _why_. Looking forward, the reason for the rename is
to piggyback the hook for BTS.
next prev parent reply other threads:[~2022-10-03 19:39 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-09-26 14:29 [PATCH RFC 0/3] KVM: x86/pmu: Add Intel Guest Branch Trace Store Support Like Xu
2022-09-26 14:29 ` [PATCH RFC 1/3] KVM + perf: Rename *_intel_pt_intr() for generic usage Like Xu
2022-10-03 19:39 ` Sean Christopherson [this message]
2022-09-26 14:29 ` [PATCH RFC 2/3] KVM + perf: Passing vector into generic perf_handle_guest_intr() Like Xu
2022-09-26 14:29 ` [PATCH RFC 3/3] KVM: x86/pmu: Add Intel Guest Branch Trace Store Support Like Xu
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=Yzs6XTxOp7wxgmJO@google.com \
--to=seanjc@google.com \
--cc=adrian.hunter@intel.com \
--cc=ak@linux.intel.com \
--cc=jmattson@google.com \
--cc=kan.liang@linux.intel.com \
--cc=kvm@vger.kernel.org \
--cc=like.xu.linux@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-perf-users@vger.kernel.org \
--cc=pbonzini@redhat.com \
--cc=peterz@infradead.org \
--cc=x86@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.