All of lore.kernel.org
 help / color / mirror / Atom feed
From: Gleb Natapov <gleb@redhat.com>
To: Avi Kivity <avi@redhat.com>
Cc: Kevin Wolf <kwolf@redhat.com>,
	kvm@vger.kernel.org, joerg.roedel@amd.com,
	yoshikawa.takuya@oss.ntt.co.jp, mtosatti@redhat.com
Subject: Re: [PATCH v2 0/3] Fix task switches into/out of VM86
Date: Mon, 30 Jan 2012 12:50:22 +0200	[thread overview]
Message-ID: <20120130105022.GC23536@redhat.com> (raw)
In-Reply-To: <4F2674BB.2000608@redhat.com>

On Mon, Jan 30, 2012 at 12:45:15PM +0200, Avi Kivity wrote:
> On 01/30/2012 12:35 PM, Kevin Wolf wrote:
> > Am 30.01.2012 09:55, schrieb Gleb Natapov:
> > > On Mon, Jan 30, 2012 at 09:48:33AM +0100, Kevin Wolf wrote:
> > >> Am 27.01.2012 20:52, schrieb Gleb Natapov:
> > >>> On Fri, Jan 27, 2012 at 08:23:33PM +0100, Kevin Wolf wrote:
> > >>>> I believe this should work with both VMX and SVM now. Gleb, Jörg, can one of
> > >>>> you test this with SVM? I did some testing on my buggy processor and it looks
> > >>>> as good as it gets, but it would be better if you could confirm.
> > >>>>
> > >>> You forgot to set cpl to 3 in vmcb in svm_set_rflags() when vm86 is enabled, no?
> > >>
> > >> SVM updates the CPL when the segment selector for CS is loaded. From a
> > >> svm.c POV, segment selectors are updated immediately after set_rflags,
> > >> so it wouldn't really make a difference to do it twice.
> > >>
> > > It is too subtle to rely on that. The fact is that checking cpl after
> > > set_rflags provides incorrect value. This better be fixed.
> >
> > Depends on what value you consider to be correct between reloading
> > eflags and reloading cs. I think it's logical and more consistent to say
> > that CPL only changes when cs is reloaded, but you could argue that it's
> > effective with the reload of rflags. It doesn't make a difference to
> > guests, so we can decide to choose whatever we like.
> 
> It's best to make it independent (like svm, and force vmx to emulate
> this behaviour).  Real mode forces cpl to 0, vm86 forces cpl to 3,
> protected mode (and I think long mode) uses cs.rpl.
This is what vmx does, not svm. svm checks vmcb->cpl that can be
outdated during emulation.


>                                                     Making it depend on
> the mode causes subtle issues during the mode switch - if you switch
> from real mode to protected mode while cs & 3 != 0 you end up with the
> wrong cpl until the jmp instruction is executed.
> 
> >
> > Depending on what we decide on (Gleb and I disagree on this, so more
> > input would be helpful), either VMX or SVM need a cleanup. I think it
> > can be done independent from and on top of this fix.
> 
> Right.  IMO we should follow svm and make vmx be more flexible.
> 
> One way to do it is to have a new variable for vmx cpl, and reconcile
> all the places where cpl is stored (cs.rpl, ss.rpl, cr0.pe, rflags.vm)
> just before entry.  If we can't reconcile it, we have to emulate.
> 
> 
> 
> -- 
> error compiling committee.c: too many arguments to function

--
			Gleb.

  reply	other threads:[~2012-01-30 10:50 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-01-27 19:23 [PATCH v2 0/3] Fix task switches into/out of VM86 Kevin Wolf
2012-01-27 19:23 ` [PATCH v2 1/3] KVM: x86 emulator: Fix task switch privilege checks Kevin Wolf
2012-01-30 10:39   ` Avi Kivity
2012-01-30 11:12     ` Kevin Wolf
2012-01-27 19:23 ` [PATCH v2 2/3] KVM: x86 emulator: VM86 segments must have DPL 3 Kevin Wolf
2012-01-27 19:23 ` [PATCH v2 3/3] KVM: x86 emulator: Allow PM/VM86 switch during task switch Kevin Wolf
2012-01-30 10:24   ` Avi Kivity
2012-01-30 10:56     ` Gleb Natapov
2012-01-30 12:02       ` Avi Kivity
2012-01-30 12:04         ` Gleb Natapov
2012-01-30 13:24           ` Avi Kivity
2012-01-30 11:05     ` Kevin Wolf
2012-01-30 11:09       ` Gleb Natapov
2012-01-30 13:23       ` Avi Kivity
2012-01-30 14:01         ` Kevin Wolf
2012-01-30 14:32           ` Avi Kivity
2012-01-30 15:26             ` Kevin Wolf
2012-01-30 15:44               ` Avi Kivity
2012-01-30 15:55               ` Takuya Yoshikawa
2012-01-31  9:37               ` Gleb Natapov
2012-01-31 10:26                 ` Avi Kivity
2012-01-27 19:52 ` [PATCH v2 0/3] Fix task switches into/out of VM86 Gleb Natapov
2012-01-30  8:48   ` Kevin Wolf
2012-01-30  8:55     ` Gleb Natapov
2012-01-30 10:22       ` Gleb Natapov
2012-01-30 10:35       ` Kevin Wolf
2012-01-30 10:45         ` Avi Kivity
2012-01-30 10:50           ` Gleb Natapov [this message]
2012-01-30 11:59             ` Avi Kivity
2012-01-30 12:16               ` Gleb Natapov
2012-01-30 13:27                 ` Avi Kivity
2012-01-30 12:31               ` Gleb Natapov
2012-01-30 13:28                 ` Avi Kivity
2012-01-30 13:29                   ` Gleb Natapov
2012-01-30 13:39                     ` Avi Kivity
2012-01-30 10:47         ` Gleb Natapov

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=20120130105022.GC23536@redhat.com \
    --to=gleb@redhat.com \
    --cc=avi@redhat.com \
    --cc=joerg.roedel@amd.com \
    --cc=kvm@vger.kernel.org \
    --cc=kwolf@redhat.com \
    --cc=mtosatti@redhat.com \
    --cc=yoshikawa.takuya@oss.ntt.co.jp \
    /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.