linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Sean Christopherson <sean.j.christopherson@intel.com>
To: Xiaoyao Li <xiaoyao.li@intel.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>,
	Vitaly Kuznetsov <vkuznets@redhat.com>,
	Wanpeng Li <wanpengli@tencent.com>,
	Jim Mattson <jmattson@google.com>, Joerg Roedel <joro@8bytes.org>,
	kvm@vger.kernel.org, linux-kernel@vger.kernel.org,
	Jingqi Liu <jingqi.liu@intel.com>, Tao Xu <tao3.xu@intel.com>
Subject: Re: [PATCH] KVM: VMX: Stop context switching MSR_IA32_UMWAIT_CONTROL
Date: Mon, 22 Jun 2020 18:31:11 -0700	[thread overview]
Message-ID: <20200623013111.GE6151@linux.intel.com> (raw)
In-Reply-To: <7e840f1c-d8e2-3374-5009-f2ab41a87386@intel.com>

On Tue, Jun 23, 2020 at 09:21:28AM +0800, Xiaoyao Li wrote:
> On 6/23/2020 8:51 AM, Sean Christopherson wrote:
> >Remove support for context switching between the guest's and host's
> >desired UMWAIT_CONTROL.  Propagating the guest's value to hardware isn't
> >required for correct functionality, e.g. KVM intercepts reads and writes
> >to the MSR, and the latency effects of the settings controlled by the
> >MSR are not architecturally visible.
> >
> >As a general rule, KVM should not allow the guest to control power
> >management settings unless explicitly enabled by userspace, e.g. see
> >KVM_CAP_X86_DISABLE_EXITS.  E.g. Intel's SDM explicitly states that C0.2
> >can improve the performance of SMT siblings.  A devious guest could
> >disable C0.2 so as to improve the performance of their workloads at the
> >detriment to workloads running in the host or on other VMs.
> >
> >Wholesale removal of UMWAIT_CONTROL context switching also fixes a race
> >condition where updates from the host may cause KVM to enter the guest
> >with the incorrect value.  Because updates are are propagated to all
> >CPUs via IPI (SMP function callback), the value in hardware may be
> >stale with respect to the cached value and KVM could enter the guest
> >with the wrong value in hardware.  As above, the guest can't observe the
> >bad value, but it's a weird and confusing wart in the implementation.
> >
> >Removal also fixes the unnecessary usage of VMX's atomic load/store MSR
> >lists.  Using the lists is only necessary for MSRs that are required for
> >correct functionality immediately upon VM-Enter/VM-Exit, e.g. EFER on
> >old hardware, or for MSRs that need to-the-uop precision, e.g. perf
> >related MSRs.  For UMWAIT_CONTROL, the effects are only visible in the
> >kernel via TPAUSE/delay(), and KVM doesn't do any form of delay in
> >vcpu_vmx_run().
> 
> >Using the atomic lists is undesirable as they are more
> >expensive than direct RDMSR/WRMSR.
> 
> Do you mean the extra handling of atomic list facility in kvm? Or just mean
> vm-exit/-entry MSR-load/save in VMX hardware is expensive than direct
> RDMSR/WRMSR instruction?

Both.  The KVM handling is the bigger cost, e.g. requires two VMWRITEs to
update the list counts, on top of the list processing.  The actual ucode
cost is also somewhat expensive if adding an MSR to the list causes the
load/store lists to be activated, e.g. on top of the memory accesses for
the list, VM-Enter ucode needs to do its consistency checks.

Expensive is obviously relative, but as far as the lists are concerned it's
an easy penalty to avoid.

      reply	other threads:[~2020-06-23  1:31 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-23  0:51 [PATCH] KVM: VMX: Stop context switching MSR_IA32_UMWAIT_CONTROL Sean Christopherson
2020-06-23  0:55 ` Paolo Bonzini
2020-06-23  1:21 ` Xiaoyao Li
2020-06-23  1:31   ` Sean Christopherson [this message]

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=20200623013111.GE6151@linux.intel.com \
    --to=sean.j.christopherson@intel.com \
    --cc=jingqi.liu@intel.com \
    --cc=jmattson@google.com \
    --cc=joro@8bytes.org \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pbonzini@redhat.com \
    --cc=tao3.xu@intel.com \
    --cc=vkuznets@redhat.com \
    --cc=wanpengli@tencent.com \
    --cc=xiaoyao.li@intel.com \
    /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 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).