Re: [PATCH v13 021/113] KVM: TDX: Make pmu_intel.c ignore guest TD case

From: Like Xu
Date: Mon May 29 2023 - 10:19:33 EST

On 28/5/2023 4:26 pm, Isaku Yamahata wrote:
On Wed, Apr 19, 2023 at 04:21:21PM +0800,
Like Xu <like.xu.linux@xxxxxxxxx> wrote:

On 2/4/2023 4:50 pm, Zhi Wang wrote:
Hi Like:

Would you mind to take a look on this patch? It would be nice to have
a r-b also from you. :)

On Sun, 12 Mar 2023 10:55:45 -0700
isaku.yamahata@xxxxxxxxx wrote:

From: Isaku Yamahata <isaku.yamahata@xxxxxxxxx>

Because TDX KVM doesn't support PMU yet (it's future work of TDX KVM
support as another patch series) and pmu_intel.c touches vmx specific

It would be nice to have pmu support for tdx-guest from the very beginning.

It's supported in the public github repo.
As this patch series has 100+ patches, I don't want to bloat this patch more.

I presume we are talking about 873e2391e729...63761adbf5aa for TD pmu:

A quick glance brought me at least these comments:

(1) how does intel_pmu_save/restore() handle the enabled host LBR/PEBS ?
(2) guest PMI injection may be malicious and could the current guest pmu driver handle it ?
(3) how do we handle the case when host counters can be enabled before TDENTER
for debuggable TD and support the case like "perf-kvm for both guest and host" ?

My point is actually, changes to perf/core should be CC to the perf reviewers
as early as possible to prevent key player from killing the direction.

If you guys are more open on the tdx development model, I'd like to help on
those features.

This mainling list is the place.

Yeah, plus the PUCK and LPC KVM micro-conference.

structure in vcpu initialization, as workaround add dummy structure to
struct vcpu_tdx and pmu_intel.c can ignore TDX case.

If the target is not to provide a workaround, how about other variants:
- struct lbr_desc lbr_desc;
- pebs ds_buffer;

We also need tdx selftest to verify the unavailability of these features.
Also, it would be great to have TDX's "System Profiling Mode" featue back in
the specification.

Detailed TD (plus debuggable) PMU selftest would clearly speed up the review process.

I don't think it's productive. Once merging this patch series, we can move on
to TDX PMU support (or whatever still missing feature) as second (or later)

I totally agree that TD PMU (plus debuggable) support should not be the first part of the
code to be merged in, but the related discussion should be kicked off on day 0.
As a scenario for users using Intel TDX, it's expected to support "System Profiling Mode",
which was first introduced in 344425-001US but disappeared since version 003.