On Mon, May 29, 2023 at 10:19:16PM +0800, Like Xu <like.xu.linux@xxxxxxxxx> wrote: > 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. > > https://github.com/intel/tdx/tree/kvm-upstream-workaround > > 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 ? It's not handled yet. We need to save/restore those MSRs. > (2) guest PMI injection may be malicious and could the current guest pmu > driver handle it ? This isn't specific to PMI. Malicious VMM can inject any interrupt to the guest at any time. Guest should be prepared for 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" ? On TDEXIT, those are disabled. VMM has to restore MSRs and enable it again. There is a window where events can be missed. > 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. Sure, agreed. > > > > > 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. The existing KVM PMU selftest can be utilized. Or do you have something else in mind? -- Isaku Yamahata <isaku.yamahata@xxxxxxxxx>
