linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Sean Christopherson <sean.j.christopherson@intel.com>
To: Vitaly Kuznetsov <vkuznets@redhat.com>
Cc: Wanpeng Li <wanpengli@tencent.com>,
	Jim Mattson <jmattson@google.com>, Joerg Roedel <joro@8bytes.org>,
	kvm@vger.kernel.org, linux-kernel@vger.kernel.org,
	Paolo Bonzini <pbonzini@redhat.com>
Subject: Re: [PATCH] KVM: x86/mmu: Avoid retpoline on ->page_fault() with TDP
Date: Fri, 7 Feb 2020 07:55:39 -0800	[thread overview]
Message-ID: <20200207155539.GC2401@linux.intel.com> (raw)
In-Reply-To: <878sleg2z7.fsf@vitty.brq.redhat.com>

On Fri, Feb 07, 2020 at 10:29:16AM +0100, Vitaly Kuznetsov wrote:
> Sean Christopherson <sean.j.christopherson@intel.com> writes:
> 
> > Wrap calls to ->page_fault() with a small shim to directly invoke the
> > TDP fault handler when the kernel is using retpolines and TDP is being
> > used.  Denote the TDP fault handler by nullifying mmu->page_fault, and
> > annotate the TDP path as likely to coerce the compiler into preferring
> > the TDP path.
> >
> > Rename tdp_page_fault() to kvm_tdp_page_fault() as it's exposed outside
> > of mmu.c to allow inlining the shim.
> >
> > Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
> > ---
> 
> Out of pure curiosity, if we do something like
> 
> if (vcpu->arch.mmu->page_fault == tdp_page_fault)
>     tdp_page_fault(...)
> else if (vcpu->arch.mmu->page_fault == nonpaging_page_fault)
>    nonpaging_page_fault(...)
> ...
> 
> we also defeat the retpoline, right?

Yep.

> Should we use this technique ... everywhere? :-)

It becomes a matter of weighing the maintenance cost and robustness against
the performance benefits.  For the TDP case, amost no one (that cares about
performance) uses shadow paging, the change is very explicit, tiny and
isolated, and TDP page fault are a hot path, e.g. when booting the VM.
I.e. low maintenance overhead, still robust, and IMO worth the shenanigans.

The changes to VMX's VM-Exit handlers follow similar thinking: snipe off
the exit handlers that are performance critical, but use a low maintenance
implementation for the majority of handlers.

There have been multiple attempts to add infrastructure to solve the
maintenance and robustness problems[*], but AFAIK none of them have made
their way upstream.

[*] https://lwn.net/Articles/774743/

  reply	other threads:[~2020-02-07 15:55 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-06 22:14 [PATCH] KVM: x86/mmu: Avoid retpoline on ->page_fault() with TDP Sean Christopherson
2020-02-07  9:29 ` Vitaly Kuznetsov
2020-02-07 15:55   ` Sean Christopherson [this message]
2020-02-07 16:15     ` Vitaly Kuznetsov
2020-02-12 11:55     ` Paolo Bonzini
2020-02-12 16:22       ` Sean Christopherson

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=20200207155539.GC2401@linux.intel.com \
    --to=sean.j.christopherson@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=vkuznets@redhat.com \
    --cc=wanpengli@tencent.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).