kvm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Xu, Like" <like.xu@intel.com>
To: Sean Christopherson <seanjc@google.com>,
	Andi Kleen <andi@firstfloor.org>,
	Kan Liang <kan.liang@linux.intel.com>,
	Peter Zijlstra <peterz@infradead.org>
Cc: Paolo Bonzini <pbonzini@redhat.com>,
	eranian@google.com, kvm@vger.kernel.org,
	Ingo Molnar <mingo@redhat.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Vitaly Kuznetsov <vkuznets@redhat.com>,
	Wanpeng Li <wanpengli@tencent.com>,
	Jim Mattson <jmattson@google.com>, Joerg Roedel <joro@8bytes.org>,
	wei.w.wang@intel.com, luwei.kang@intel.com,
Subject: Re: [PATCH v3 00/17] KVM: x86/pmu: Add support to enable Guest PEBS via DS
Date: Fri, 15 Jan 2021 10:02:29 +0800	[thread overview]
Message-ID: <f8a8e4e2-e0b1-8e68-81d4-044fb62045d5@intel.com> (raw)
In-Reply-To: <YACXQwBPI8OFV1T+@google.com>

Hi Sean,

Thanks for your comments !

On 2021/1/15 3:10, Sean Christopherson wrote:
> On Mon, Jan 04, 2021, Like Xu wrote:
>> 2) Slow path (part 3, patch 0012-0017)
>> This is when the host assigned physical PMC has a different index
>> from the virtual PMC (e.g. using physical PMC1 to emulate virtual PMC0)
>> In this case, KVM needs to rewrite the PEBS records to change the
>> applicable counter indexes to the virtual PMC indexes, which would
>> otherwise contain the physical counter index written by PEBS facility,
>> and switch the counter reset values to the offset corresponding to
>> the physical counter indexes in the DS data structure.
>> Large PEBS needs to be disabled by KVM rewriting the
>> pebs_interrupt_threshold filed in DS to only one record in
>> the slow path.  This is because a guest may implicitly drain PEBS buffer,
>> e.g., context switch. KVM doesn't get a chance to update the PEBS buffer.
> Are the PEBS record write, PEBS index update, and subsequent PMI atomic with
> respect to instruction execution?  If not, doesn't this approach still leave a
> window where the guest could see the wrong counter?

First, KVM would limit/rewrite guest DS pebs_interrupt_threshold to one 
record before vm-entry,
(see patch [PATCH v3 14/17] KVM: vmx/pmu: Limit pebs_interrupt_threshold in 
the guest DS area)
which means once a PEBS record is written into the guest pebs buffer,
a PEBS PMI will be generated immediately and thus vm-exit.

Second, KVM would complete the PEBS record rewriting, PEBS index update, 
and inject vPMI
before the next vm-entry (we deal with these separately in patches 15-17 
for easy review).

After the updated PEBS record(s) are (atomically?) prepared, guests will be 
notified via PMI
and there is no window for vcpu to check whether there is a PEBS record due 
to vm-exit.

> The virtualization hole is also visible if the guest is reading the PEBS records
> from a different vCPU, though I assume no sane kernel does that?

I have checked the guest PEBS driver behavior for Linux and Windows, and 
they're sane.

Theoretically, it's true for busy-poll PBES buffer readers from other vCPUs
and to fix it, making all vCPUs vm-exit is onerous for a large-size guest
and I don't think you would accept this or do we have a better idea ?

In fact, we don't think it's a hole or vulnerability because the motivation for
correcting the counter index(s) is to help guest PEBS reader understand their
PEBS records correctly and provide the same sampling accuracy as the non-cross
mapped case, instead of providing a new attack interface from guest to host.

PeterZ commented on the V1 version and insisted that the host perf allows 
the guest
counter to be assigned a cross-mapped back-end counter. In this case, the 
slow path
patches (13-17) are introduced to ensure that from the guest counter 
the PEBS records are also correct. We do not want these records to be 
invalid and
ignored, which would undermine the accuracy of PEBS.

In the practical use, the slow patch rarely happens and
we're glad to see if the fast patch could be upstream
and the cross-mapped case is teamprily disabled
until we're on the same page for the cross mapped case.

In actual use, slow path rarely occur. As a first step,
we propose to upstream the quick patches (patch 01-12) with your help.
The guest PEBS would been disabled temporarily when guest PEBS counters
are cross-mapped until we figure out a satisfactory cross-mapping solution.


>> The physical PMC index will confuse the guest. The difficulty comes
>> when multiple events get rescheduled inside the guest. Hence disabling
>> large PEBS in this case might be an easy and safe way to keep it corrects
>> as an initial step here.

  reply	other threads:[~2021-01-15  2:03 UTC|newest]

Thread overview: 59+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-04 13:15 [PATCH v3 00/17] KVM: x86/pmu: Add support to enable Guest PEBS via DS Like Xu
2021-01-04 13:15 ` [PATCH v3 01/17] KVM: x86/pmu: Set MSR_IA32_MISC_ENABLE_EMON bit when vPMU is enabled Like Xu
2021-01-04 13:15 ` [PATCH v3 02/17] KVM: x86/pmu: Use IA32_PERF_CAPABILITIES to adjust features visibility Like Xu
2021-01-04 13:15 ` [PATCH v3 03/17] KVM: x86/pmu: Introduce the ctrl_mask value for fixed counter Like Xu
2021-01-13 18:06   ` Peter Zijlstra
2021-01-14  1:58     ` Xu, Like
2021-01-04 13:15 ` [PATCH v3 04/17] perf: x86/ds: Handle guest PEBS overflow PMI and inject it to guest Like Xu
2021-01-13 18:22   ` Peter Zijlstra
2021-01-13 18:27     ` Peter Zijlstra
2021-01-14  3:39     ` Xu, Like
2021-01-15 12:01       ` Peter Zijlstra
2021-01-15 14:30         ` Xu, Like
2021-01-15 14:44           ` Peter Zijlstra
2021-01-15 15:12             ` Xu, Like
2021-01-25  8:26             ` Like Xu
2021-01-25 11:47               ` Peter Zijlstra
2021-02-02  6:31                 ` Xu, Like
2021-01-14 18:55   ` Sean Christopherson
2021-01-15  2:49     ` Xu, Like
2021-01-15 17:42       ` Sean Christopherson
2021-01-22  5:30         ` Like Xu
2021-01-04 13:15 ` [PATCH v3 05/17] KVM: x86/pmu: Reprogram guest PEBS event to emulate guest PEBS counter Like Xu
2021-01-15 11:33   ` Peter Zijlstra
2021-01-15 13:53     ` Xu, Like
2021-01-04 13:15 ` [PATCH v3 06/17] KVM: x86/pmu: Add IA32_PEBS_ENABLE MSR emulation for extended PEBS Like Xu
2021-01-05 21:11   ` Sean Christopherson
2021-01-07 12:38     ` Xu, Like
2021-01-15 14:46   ` Peter Zijlstra
2021-01-15 15:29     ` Xu, Like
2021-01-04 13:15 ` [PATCH v3 07/17] KVM: x86/pmu: Add IA32_DS_AREA MSR emulation to manage guest DS buffer Like Xu
2021-01-05 21:16   ` Sean Christopherson
2021-01-08  3:05     ` Xu, Like
2021-01-04 13:15 ` [PATCH v3 08/17] KVM: x86/pmu: Add PEBS_DATA_CFG MSR emulation to support adaptive PEBS Like Xu
2021-01-04 13:15 ` [PATCH v3 09/17] KVM: x86: Set PEBS_UNAVAIL in IA32_MISC_ENABLE when PEBS is enabled Like Xu
2021-01-04 13:15 ` [PATCH v3 10/17] KVM: x86/pmu: Expose CPUIDs feature bits PDCM, DS, DTES64 Like Xu
2021-01-04 13:15 ` [PATCH v3 11/17] KVM: x86/pmu: Adjust precise_ip to emulate Ice Lake guest PDIR counter Like Xu
2021-01-04 13:15 ` [PATCH v3 12/17] KVM: x86/pmu: Disable guest PEBS when counters are cross-mapped Like Xu
2021-01-04 13:15 ` [PATCH v3 13/17] KVM: x86/pmu: Add hook to emulate pebs for cross-mapped counters Like Xu
2021-01-04 13:15 ` [PATCH v3 14/17] KVM: vmx/pmu: Limit pebs_interrupt_threshold in the guest DS area Like Xu
2021-01-04 13:15 ` [PATCH v3 15/17] KVM: vmx/pmu: Rewrite applicable_counters field in guest PEBS records Like Xu
2021-01-04 13:15 ` [PATCH v3 16/17] KVM: x86/pmu: Save guest pebs reset values when pebs is configured Like Xu
2021-01-04 13:15 ` [PATCH v3 17/17] KVM: x86/pmu: Adjust guest pebs reset values for crpss-mapped counters Like Xu
2021-01-14 19:10 ` [PATCH v3 00/17] KVM: x86/pmu: Add support to enable Guest PEBS via DS Sean Christopherson
2021-01-15  2:02   ` Xu, Like [this message]
2021-01-15 17:57     ` Sean Christopherson
2021-01-15 18:27       ` Andi Kleen
2021-01-15 18:51         ` Sean Christopherson
2021-01-15 19:11           ` Andi Kleen
2021-01-22  9:56           ` Peter Zijlstra
2021-01-25  8:08             ` Like Xu
2021-01-25 11:13               ` Peter Zijlstra
2021-01-25 12:07                 ` Xu, Like
2021-01-25 12:18                   ` Peter Zijlstra
2021-01-25 12:53                     ` Xu, Like
     [not found] <EEC2A80E7137D84ABF791B01D40FA9A601EC200E@DGGEMM506-MBX.china.huawei.com>
2021-01-25  2:41 ` Like Xu
2021-01-25 14:47   ` Liuxiangdong (Aven, Cloud Infrastructure Service Product Dept.)
2021-01-26  7:08     ` Xu, Like
2021-01-29  2:52       ` Liuxiangdong (Aven, Cloud Infrastructure Service Product Dept.)
2021-02-01  8:43         ` Xu, Like

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:

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

  git send-email \
    --in-reply-to=f8a8e4e2-e0b1-8e68-81d4-044fb62045d5@intel.com \
    --to=like.xu@intel.com \
    --cc=andi@firstfloor.org \
    --cc=eranian@google.com \
    --cc=jmattson@google.com \
    --cc=joro@8bytes.org \
    --cc=kan.liang@linux.intel.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luwei.kang@intel.com \
    --cc=mingo@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=peterz@infradead.org \
    --cc=seanjc@google.com \
    --cc=tglx@linutronix.de \
    --cc=vkuznets@redhat.com \
    --cc=wanpengli@tencent.com \
    --cc=wei.w.wang@intel.com \


* 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).