All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kevin Wolf <kwolf@redhat.com>
To: Avi Kivity <avi@redhat.com>
Cc: kvm@vger.kernel.org, gleb@redhat.com, joerg.roedel@amd.com,
	yoshikawa.takuya@oss.ntt.co.jp, mtosatti@redhat.com
Subject: Re: [PATCH v3 4/4] KVM: x86 emulator: Allow PM/VM86 switch during task switch
Date: Mon, 06 Feb 2012 11:54:19 +0100	[thread overview]
Message-ID: <4F2FB15B.8000508@redhat.com> (raw)
In-Reply-To: <4F2FAC2A.8000101@redhat.com>

Am 06.02.2012 11:32, schrieb Avi Kivity:
> On 02/03/2012 08:29 PM, Kevin Wolf wrote:
>> Task switches can switch between Protected Mode and VM86. The current
>> mode must be updated during the task switch emulation so that the new
>> segment selectors are interpreted correctly.
>>
>> In order to let privilege checks succeed, rflags needs to be updated in
>> the vcpu struct as this causes a CPL update.
>>
>> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
>> index 4124a7e..38d5ef3 100644
>> --- a/arch/x86/kvm/svm.c
>> +++ b/arch/x86/kvm/svm.c
>> @@ -1286,6 +1286,7 @@ static unsigned long svm_get_rflags(struct kvm_vcpu *vcpu)
>>  static void svm_set_rflags(struct kvm_vcpu *vcpu, unsigned long rflags)
>>  {
>>  	to_svm(vcpu)->vmcb->save.rflags = rflags;
>> +	svm_update_cpl(vcpu);
>>  }
> 
> This can trigger the (cs&3)!=0 && (cr0&1) != 0 problem, right after we
> enter protected mode.
> 
> What this code does is slave the cpl to other x86 state (copying vmx),
> whereas in reality it is separate state (with independent existence from
> the mov cr0 instruction until the next far jump...).

Yes, as discussed in v2, eventually we'll want to call set_cpl() rather
than set_rflags() during the task switch (I left a TODO comment there).
Then the not quite correct update in svm_set_rflags can be removed again.

> We can fix this with
> 
>     if ((save.rflags ^ rflags) & EFLAGS_VM)
>         svm_update_cpl(vcpu)
> 
> but that leaves us with an unupdated cpl after live migration (well, it
> will update after loading segment registers).  Both are bad, but I think
> my version is less bad - live migration is broken anyway in this window,
> since we don't save the cpl.

Right, it just leaves broken what is already broken. If the rest of the
series is okay now, I'll resend with a check if rflags.VM has changed.

Kevin

      reply	other threads:[~2012-02-06 10:50 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-02-03 18:29 [PATCH v3 0/4] Fix task switches into/out of VM86 Kevin Wolf
2012-02-03 18:29 ` [PATCH v3 1/4] KVM: x86 emulator: Fix task switch privilege checks Kevin Wolf
2012-02-03 18:29 ` [PATCH v3 2/4] KVM: x86 emulator: VM86 segments must have DPL 3 Kevin Wolf
2012-02-03 18:29 ` [PATCH v3 3/4] KVM: SVM: Fix CPL updates Kevin Wolf
2012-02-05 11:16   ` Gleb Natapov
2012-02-06  9:18     ` Kevin Wolf
2012-02-06  9:57       ` Gleb Natapov
2012-02-06 10:40         ` Kevin Wolf
2012-02-03 18:29 ` [PATCH v3 4/4] KVM: x86 emulator: Allow PM/VM86 switch during task switch Kevin Wolf
2012-02-06 10:32   ` Avi Kivity
2012-02-06 10:54     ` Kevin Wolf [this message]

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=4F2FB15B.8000508@redhat.com \
    --to=kwolf@redhat.com \
    --cc=avi@redhat.com \
    --cc=gleb@redhat.com \
    --cc=joerg.roedel@amd.com \
    --cc=kvm@vger.kernel.org \
    --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.