Re: [PATCH kvm/queue v2 2/3] perf: x86/core: Add interface to query perfmon_event_map[] directly

From: Dave Hansen <dave.hansen@intel.com>
To: Jim Mattson <jmattson@google.com>
Cc: Peter Zijlstra <peterz@infradead.org>,
	Like Xu <like.xu.linux@gmail.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	Sean Christopherson <seanjc@google.com>,
	Vitaly Kuznetsov <vkuznets@redhat.com>,
	Wanpeng Li <wanpengli@tencent.com>,
	Joerg Roedel <joro@8bytes.org>,
	kvm@vger.kernel.org, linux-kernel@vger.kernel.org,
	Like Xu <likexu@tencent.com>,
	Stephane Eranian <eranian@google.com>,
	David Dunn <daviddunn@google.com>
Subject: Re: [PATCH kvm/queue v2 2/3] perf: x86/core: Add interface to query perfmon_event_map[] directly
Date: Wed, 9 Feb 2022 10:57:01 -0800	[thread overview]
Message-ID: <67a731dd-53ba-0eb8-377f-9707e5c9be1b@intel.com> (raw)
In-Reply-To: <CALMp9eS=1U7T39L-vL_cTXTNN2Li8epjtAPoP_+Hwefe9d+teQ@mail.gmail.com>

On 2/9/22 10:47, Jim Mattson wrote:
> On Wed, Feb 9, 2022 at 7:41 AM Dave Hansen <dave.hansen@intel.com> wrote:
>>
>> On 2/9/22 05:21, Peter Zijlstra wrote:
>>> On Wed, Feb 02, 2022 at 02:35:45PM -0800, Jim Mattson wrote:
>>>> 3) TDX is going to pull the rug out from under us anyway. When the TDX
>>>> module usurps control of the PMU, any active host counters are going
>>>> to stop counting. We are going to need a way of telling the host perf
>>>> subsystem what's happening, or other host perf clients are going to
>>>> get bogus data.
>>> That's not acceptible behaviour. I'm all for unilaterally killing any
>>> guest that does this.
>>
>> I'm not sure where the "bogus data" comes or to what that refers
>> specifically.  But, the host does have some level of control:
> 
> I was referring to gaps in the collection of data that the host perf
> subsystem doesn't know about if ATTRIBUTES.PERFMON is set for a TDX
> guest. This can potentially be a problem if someone is trying to
> measure events per unit of time.

Ahh, that makes sense.

Does SGX cause problem for these people?  It can create some of the same
collection gaps:

	performance monitoring activities are suppressed when entering
	an opt-out (of performance monitoring) enclave.

>>> The host VMM controls whether a guest TD can use the performance
>>> monitoring ISA using the TD’s ATTRIBUTES.PERFMON bit...
>>
>> So, worst-case, we don't need to threaten to kill guests.  The host can
>> just deny access in the first place.
>>
>> I'm not too picky about what the PMU does, but the TDX behavior didn't
>> seem *that* onerous to me.  The gory details are all in "On-TD
>> Performance Monitoring" here:
>>
>>> https://www.intel.com/content/dam/develop/external/us/en/documents/tdx-module-1.0-public-spec-v0.931.pdf
>>
>> My read on it is that TDX host _can_ cede the PMU to TDX guests if it
>> wants.  I assume the context-switching model Jim mentioned is along the
>> lines of what TDX is already doing on host<->guest transitions.
> 
> Right. If ATTRIBUTES.PERFMON is set, then "perfmon state is
> context-switched by the Intel TDX module across TD entry and exit
> transitions." Furthermore, the VMM has no access to guest perfmon
> state.
> 
> If you're saying that setting this bit is unacceptable, then perhaps
> the TDX folks need to redesign their in-guest PMU support.

It's fine with *me*, but I'm not too picky about the PMU.  But, it
sounded like Peter was pretty concerned about it.

In any case, if we (Linux folks) need a change, it's *possible* because
most of this policy is implemented in software in the TDX module.  It
would just be painful for the folks who came up with the existing mechanism.