All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Radim Krčmář" <rkrcmar@redhat.com>
To: Bandan Das <bsd@redhat.com>
Cc: Joel Schopp <joel.schopp@amd.com>, Gleb Natapov <gleb@kernel.org>,
	Paolo Bonzini <pbonzini@redhat.com>,
	kvm@vger.kernel.org, David Kaplan <David.Kaplan@amd.com>,
	Joerg Roedel <joro@8bytes.org>,
	Marcelo Tosatti <mtosatti@redhat.com>,
	linux-kernel@vger.kernel.org, Borislav Petkov <bp@alien8.de>
Subject: Re: [PATCH] x86: svm: make wbinvd faster
Date: Mon, 2 Mar 2015 17:03:20 +0100	[thread overview]
Message-ID: <20150302160319.GA25123@potion.brq.redhat.com> (raw)
In-Reply-To: <jpgmw3v2zgx.fsf@redhat.com>

2015-03-02 10:25-0500, Bandan Das:
> Radim Krčmář <rkrcmar@redhat.com> writes:
> > 2015-03-01 21:29-0500, Bandan Das:
> >> Joel Schopp <joel.schopp@amd.com> writes:
> >> > +static int wbinvd_interception(struct vcpu_svm *svm)
> >> > +{
> >> > +	kvm_emulate_wbinvd(&svm->vcpu);
> >> > +	skip_emulated_instruction(&svm->vcpu);
> >> > +	return 1;
> >> > +}
> >> Can't we merge this to kvm_emulate_wbinvd, and just call that function
> >> directly for both vmx and svm ?
> >
> > kvm_emulate_wbinvd() lives in x86.c and skip_emulated_instruction() is
> > from svm.c/vmx.c:  so we'd have to create a new x86 op and change the
> > emulator code as well ... it's probably better like this.
> 
> There's already one - kvm_x86_ops->skip_emulated_instruction

My bad, its usage is inconsistent and I only looked at two close
interceptions where it was used ... kvm_emulate_cpuid() calls
kvm_x86_ops->skip_emulated_instruction(), while kvm_emulate_halt() and
kvm_emulate_hypercall() need an external skip.

We do "skip" the instruction with kvm_emulate(), so automatically
skipping the instruction on kvm_emulate_*() makes sense:
 1. rename kvm_emulate_halt() and kvm_emulate_wbinvd() to accommodate
    callers that don't want to skip
 2. introduce kvm_emulate_{halt,wbinvd}() and move the skip to to
    kvm_emulate_{halt,wbinvd,hypercall}()

The alternative is to remove kvm_x86_ops->skip_emulated_instruction():
 1. remove skip from kvm_emulate_cpuid() and modify callers
 2. move kvm_complete_insn_gp to a header file and use
    skip_emulated_instruction directly
 3. remove unused kvm_x86_ops->skip_emulated_instruction()

Which one do you prefer?

Thanks.

  reply	other threads:[~2015-03-02 16:03 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-02-28  0:19 [PATCH] x86: svm: make wbinvd faster Joel Schopp
2015-03-02  2:29 ` Bandan Das
2015-03-02 13:59   ` Radim Krčmář
2015-03-02 15:25     ` Bandan Das
2015-03-02 16:03       ` Radim Krčmář [this message]
2015-03-02 16:58         ` Joel Schopp
2015-03-02 14:09 ` Radim Krčmář
2015-03-09 23:28 ` Marcelo Tosatti
2015-03-10 11:01   ` Radim Krčmář
2015-03-10 22:37     ` Marcelo Tosatti

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=20150302160319.GA25123@potion.brq.redhat.com \
    --to=rkrcmar@redhat.com \
    --cc=David.Kaplan@amd.com \
    --cc=bp@alien8.de \
    --cc=bsd@redhat.com \
    --cc=gleb@kernel.org \
    --cc=joel.schopp@amd.com \
    --cc=joro@8bytes.org \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mtosatti@redhat.com \
    --cc=pbonzini@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.