All of lore.kernel.org
 help / color / mirror / Atom feed
From: Avi Kivity <avi@redhat.com>
To: Alexander Graf <agraf@suse.de>
Cc: kvm@vger.kernel.org, joro@8bytes.org, anthony@codemonkey.ws
Subject: Re: [PATCH 12/12] Accelerate nested SVM by emulating parts of GIF=0 v6
Date: Fri, 21 Nov 2008 19:07:05 +0200	[thread overview]
Message-ID: <4926EAB9.7060102@redhat.com> (raw)
In-Reply-To: <3D59C2E9-27A0-4492-88E2-0A01C1A1A334@suse.de>

Alexander Graf wrote:
>>
>> Neat trick.  Have you looked at svm.c to see if we can cut out 
>> extraneous instructions (or move them out of the gif=0 area)?  Could 
>> mean big savings.
>
> Well for now I only emulate pre-vmrun and vmsave, because the emulator 
> breaks on lldt and set dr6 and ltr and ...
>

Yeah.  We could move some of these out, and emulate the rest (ltr I 
think is needed).

>>
>> Move to the regular x86 emulator, so if we extend it with debug flag 
>> support, privilege checking, etc, we get that for svm as well.
>
> Do you seriously want to have a callback to svm specific code in the 
> x86 emulator? 

That's what kvm_x86_ops is for.  We already emulate vendor specific 
instructions (vmcall and vmmcall).

> I was thinking of doing that, but this way looked way cleaner to me. I 
> would actually rather use prefixes as hint on pattern matching.
> So e.g. if we find a clgi instruction with some random prefix (or 
> somehow marked in other ways) we could just run a pattern that does 
> what the entry code would do. This way no emulation would be needed.

Smells like dynamic translation to me.  I don't see how you could do 
that generically -- it might work for one version of kvm, but not for 
other hypervisors.

>>> static int nested_svm_vmexit_real(struct vcpu_svm *svm, void *arg1,
>>>                   void *arg2, void *opaque)
>>> {
>>> @@ -1551,6 +1600,9 @@ static int nested_svm_vmexit(struct vcpu_svm 
>>> *svm)
>>>     kvm_mmu_reset_context(&svm->vcpu);
>>>     kvm_mmu_load(&svm->vcpu);
>>> +    /* KVM calls vmsave after vmrun, so let's run it now if we can */
>>> +    nested_svm_emulate(svm, NULL);
>>> +
>>>
>>
>> Will also call stgi eventually, so it may make sense to loop here too.
>
> See above - I did that, it broke, I tried to debug it for ~2 days and 
> just figured I'll leave it as is for now. 

Certainly, we can leave it for later.

> The current version gives _huge_ savings already.

Can you quantify?

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.


  reply	other threads:[~2008-11-21 17:07 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-11-21 15:14 [PATCH 00/12] Add support for nested SVM (kernel) v6 Alexander Graf
2008-11-21 15:14 ` [PATCH 01/12] Clean up VINTR setting v6 Alexander Graf
2008-11-21 15:14   ` [PATCH 02/12] Move EFER and MSR constants to generic x86 code v6 Alexander Graf
2008-11-21 15:14     ` [PATCH 03/12] Add helper functions for nested SVM v6 Alexander Graf
2008-11-21 15:14       ` [PATCH 04/12] Implement GIF, clgi and stgi v6 Alexander Graf
2008-11-21 15:14         ` [PATCH 05/12] Implement hsave v6 Alexander Graf
2008-11-21 15:14           ` [PATCH 06/12] Add VMLOAD and VMSAVE handlers v6 Alexander Graf
2008-11-21 15:14             ` [PATCH 07/12] Add VMRUN handler v6 Alexander Graf
2008-11-21 15:14               ` [PATCH 08/12] Add VMEXIT handler and intercepts v6 Alexander Graf
2008-11-21 15:14                 ` [PATCH 09/12] Allow read access to MSR_VM_VR v6 Alexander Graf
2008-11-21 15:14                   ` [PATCH 10/12] Allow setting the SVME bit v6 Alexander Graf
2008-11-21 15:14                     ` [PATCH 11/12] Only allow setting of EFER_SVME when CPUID SVM is set v6 Alexander Graf
2008-11-21 15:14                       ` [PATCH 12/12] Accelerate nested SVM by emulating parts of GIF=0 v6 Alexander Graf
2008-11-21 16:53                         ` Avi Kivity
2008-11-21 16:58                           ` Alexander Graf
2008-11-21 17:07                             ` Avi Kivity [this message]
2008-11-21 17:14                               ` Alexander Graf
2008-11-21 17:18                                 ` Avi Kivity
2008-11-21 17:35                                   ` Alexander Graf
2008-11-21 15:23               ` [PATCH 07/12] Add VMRUN handler v6 Muli Ben-Yehuda
2008-11-21 15:26                 ` Alexander Graf
2008-11-21 15:35                   ` Alexander Graf
2008-11-23  8:06                     ` Muli Ben-Yehuda
2008-11-23 13:48                       ` Alexander Graf
2008-11-23 15:09                         ` Muli Ben-Yehuda

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=4926EAB9.7060102@redhat.com \
    --to=avi@redhat.com \
    --cc=agraf@suse.de \
    --cc=anthony@codemonkey.ws \
    --cc=joro@8bytes.org \
    --cc=kvm@vger.kernel.org \
    /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.