All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sean Christopherson <sean.j.christopherson@intel.com>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: "Radim Krčmář" <rkrcmar@redhat.com>,
	"Joerg Roedel" <joro@8bytes.org>,
	kvm@vger.kernel.org
Subject: Re: [PATCH 2/3] KVM: x86: Prevent use of kvm_register_{read,write}() with known GPRs
Date: Tue, 30 Apr 2019 13:44:40 -0700	[thread overview]
Message-ID: <20190430204439.GC4523@linux.intel.com> (raw)
In-Reply-To: <a1dc6d40-feba-4b14-bc23-b25e23bb3c2d@redhat.com>

On Tue, Apr 30, 2019 at 10:34:52PM +0200, Paolo Bonzini wrote:
> On 30/04/19 19:36, Sean Christopherson wrote:
> > ... to prevent future code from using the unoptimized generic accessors
> > when hardcoding access to always-available GPRs.
> 
> This cannot be done in general, because builtin_constant_p could be used
> through layers of inlining.  For example you could have a function that
> takes an enum kvm_reg argument and _its_ caller passes a constant in
> there.  Of course we may just say that we don't have such a case now (do
> we?) and so it's unlikely to happen in the future as well.

Another potential hiccup, and probably more likely, is that
the register save loops in enter_smm_save_state_{32,64} get unrolled by
the compiler.

My thought was (is?) that the only time KVM should ever use the
generic accessors is when the register is truly unknown, i.e. comes
from emulating an instruction with ModRM (or equivalent).

I thought about manually unrolling enter_smm_save_state_{32,64} so as
to avoid the caching logic, but that seemed like overkill at the time.
I didn't consider the compiler unrolling the loop and exploding.  I'll
see if I can come up with anything clever for the SMM flow and expand
the changelog if I end up with a version that is "guaranteed" to not
run afould of compiler tricks.

P.S. Do you want me to send a patch with the cleanup parts that I unwisely
smushed into this patch?  Or have you already taken care of that?

  reply	other threads:[~2019-04-30 20:44 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-04-30 17:36 [PATCH 0/3] KVM: x86: Drop "caching" of always-available GPRs Sean Christopherson
2019-04-30 17:36 ` [PATCH 1/3] KVM: x86: Omit caching logic for " Sean Christopherson
2019-04-30 17:36 ` [PATCH 2/3] KVM: x86: Prevent use of kvm_register_{read,write}() with known GPRs Sean Christopherson
2019-04-30 20:34   ` Paolo Bonzini
2019-04-30 20:44     ` Sean Christopherson [this message]
2019-04-30 17:36 ` [PATCH 3/3] KVM: VMX: Use accessors for GPRs outside of dedicated caching logic Sean Christopherson
2019-04-30 20:03 ` [PATCH 0/3] KVM: x86: Drop "caching" of always-available GPRs Paolo Bonzini
2019-04-30 20:19   ` 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=20190430204439.GC4523@linux.intel.com \
    --to=sean.j.christopherson@intel.com \
    --cc=joro@8bytes.org \
    --cc=kvm@vger.kernel.org \
    --cc=pbonzini@redhat.com \
    --cc=rkrcmar@redhat.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.