* KVM: x86: fix maintenance of guest/host xcr0 state
@ 2013-04-16 2:30 Marcelo Tosatti
2013-04-16 7:22 ` Paolo Bonzini
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Marcelo Tosatti @ 2013-04-16 2:30 UTC (permalink / raw)
To: kvm; +Cc: Gleb Natapov, Paolo Bonzini, Ulrich Obergfell
** Untested **.
Emulation of xcr0 writes zero guest_xcr0_loaded variable so that
subsequent VM-entry reloads CPU's xcr0 with guests xcr0 value.
However, this is incorrect because guest_xcr0_loaded variable is
read to decide whether to reload hosts xcr0.
In case the vcpu thread is scheduled out after the guest_xcr0_loaded = 0
assignment, and scheduler decides to preload FPU:
switch_to
{
__switch_to
__math_state_restore
restore_fpu_checking
fpu_restore_checking
if (use_xsave())
fpu_xrstor_checking
xrstor64 with CPU's xcr0 == guests xcr0
Fix by properly restoring hosts xcr0 during emulation of xcr0 writes.
Analyzed-by: Ulrich Obergfell <uobergfe@redhat.com>
Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 999d124..222926a 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -555,6 +555,25 @@ void kvm_lmsw(struct kvm_vcpu *vcpu, unsigned long msw)
}
EXPORT_SYMBOL_GPL(kvm_lmsw);
+static void kvm_load_guest_xcr0(struct kvm_vcpu *vcpu)
+{
+ if (kvm_read_cr4_bits(vcpu, X86_CR4_OSXSAVE) &&
+ !vcpu->guest_xcr0_loaded) {
+ /* kvm_set_xcr() also depends on this */
+ xsetbv(XCR_XFEATURE_ENABLED_MASK, vcpu->arch.xcr0);
+ vcpu->guest_xcr0_loaded = 1;
+ }
+}
+
+static void kvm_put_guest_xcr0(struct kvm_vcpu *vcpu)
+{
+ if (vcpu->guest_xcr0_loaded) {
+ if (vcpu->arch.xcr0 != host_xcr0)
+ xsetbv(XCR_XFEATURE_ENABLED_MASK, host_xcr0);
+ vcpu->guest_xcr0_loaded = 0;
+ }
+}
+
int __kvm_set_xcr(struct kvm_vcpu *vcpu, u32 index, u64 xcr)
{
u64 xcr0;
@@ -571,8 +590,8 @@ int __kvm_set_xcr(struct kvm_vcpu *vcpu, u32 index, u64 xcr)
return 1;
if (xcr0 & ~host_xcr0)
return 1;
+ kvm_put_guest_xcr0(vcpu);
vcpu->arch.xcr0 = xcr0;
- vcpu->guest_xcr0_loaded = 0;
return 0;
}
@@ -5600,25 +5619,6 @@ static void inject_pending_event(struct kvm_vcpu *vcpu)
}
}
-static void kvm_load_guest_xcr0(struct kvm_vcpu *vcpu)
-{
- if (kvm_read_cr4_bits(vcpu, X86_CR4_OSXSAVE) &&
- !vcpu->guest_xcr0_loaded) {
- /* kvm_set_xcr() also depends on this */
- xsetbv(XCR_XFEATURE_ENABLED_MASK, vcpu->arch.xcr0);
- vcpu->guest_xcr0_loaded = 1;
- }
-}
-
-static void kvm_put_guest_xcr0(struct kvm_vcpu *vcpu)
-{
- if (vcpu->guest_xcr0_loaded) {
- if (vcpu->arch.xcr0 != host_xcr0)
- xsetbv(XCR_XFEATURE_ENABLED_MASK, host_xcr0);
- vcpu->guest_xcr0_loaded = 0;
- }
-}
-
static void process_nmi(struct kvm_vcpu *vcpu)
{
unsigned limit = 2;
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: KVM: x86: fix maintenance of guest/host xcr0 state
2013-04-16 2:30 KVM: x86: fix maintenance of guest/host xcr0 state Marcelo Tosatti
@ 2013-04-16 7:22 ` Paolo Bonzini
2013-04-17 13:16 ` Gleb Natapov
2013-05-08 10:16 ` Gleb Natapov
2 siblings, 0 replies; 4+ messages in thread
From: Paolo Bonzini @ 2013-04-16 7:22 UTC (permalink / raw)
To: Marcelo Tosatti; +Cc: kvm, Gleb Natapov, Ulrich Obergfell
Il 16/04/2013 04:30, Marcelo Tosatti ha scritto:
>
> ** Untested **.
>
> Emulation of xcr0 writes zero guest_xcr0_loaded variable so that
> subsequent VM-entry reloads CPU's xcr0 with guests xcr0 value.
>
> However, this is incorrect because guest_xcr0_loaded variable is
> read to decide whether to reload hosts xcr0.
>
> In case the vcpu thread is scheduled out after the guest_xcr0_loaded = 0
> assignment, and scheduler decides to preload FPU:
>
> switch_to
> {
> __switch_to
> __math_state_restore
> restore_fpu_checking
> fpu_restore_checking
> if (use_xsave())
> fpu_xrstor_checking
> xrstor64 with CPU's xcr0 == guests xcr0
>
> Fix by properly restoring hosts xcr0 during emulation of xcr0 writes.
>
> Analyzed-by: Ulrich Obergfell <uobergfe@redhat.com>
> Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
>
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 999d124..222926a 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -555,6 +555,25 @@ void kvm_lmsw(struct kvm_vcpu *vcpu, unsigned long msw)
> }
> EXPORT_SYMBOL_GPL(kvm_lmsw);
>
> +static void kvm_load_guest_xcr0(struct kvm_vcpu *vcpu)
> +{
> + if (kvm_read_cr4_bits(vcpu, X86_CR4_OSXSAVE) &&
> + !vcpu->guest_xcr0_loaded) {
> + /* kvm_set_xcr() also depends on this */
> + xsetbv(XCR_XFEATURE_ENABLED_MASK, vcpu->arch.xcr0);
> + vcpu->guest_xcr0_loaded = 1;
> + }
> +}
> +
> +static void kvm_put_guest_xcr0(struct kvm_vcpu *vcpu)
> +{
> + if (vcpu->guest_xcr0_loaded) {
> + if (vcpu->arch.xcr0 != host_xcr0)
> + xsetbv(XCR_XFEATURE_ENABLED_MASK, host_xcr0);
> + vcpu->guest_xcr0_loaded = 0;
> + }
> +}
> +
> int __kvm_set_xcr(struct kvm_vcpu *vcpu, u32 index, u64 xcr)
> {
> u64 xcr0;
This is just code movement...
> @@ -571,8 +590,8 @@ int __kvm_set_xcr(struct kvm_vcpu *vcpu, u32 index, u64 xcr)
> return 1;
> if (xcr0 & ~host_xcr0)
> return 1;
> + kvm_put_guest_xcr0(vcpu);
> vcpu->arch.xcr0 = xcr0;
> - vcpu->guest_xcr0_loaded = 0;
> return 0;
> }
>
... and this is the bulk of the fix: never set guest_xcr0_loaded, always
go through kvm_load/put_guest_xcr0.
Pending test,
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Paolo
> @@ -5600,25 +5619,6 @@ static void inject_pending_event(struct kvm_vcpu *vcpu)
> }
> }
>
> -static void kvm_load_guest_xcr0(struct kvm_vcpu *vcpu)
> -{
> - if (kvm_read_cr4_bits(vcpu, X86_CR4_OSXSAVE) &&
> - !vcpu->guest_xcr0_loaded) {
> - /* kvm_set_xcr() also depends on this */
> - xsetbv(XCR_XFEATURE_ENABLED_MASK, vcpu->arch.xcr0);
> - vcpu->guest_xcr0_loaded = 1;
> - }
> -}
> -
> -static void kvm_put_guest_xcr0(struct kvm_vcpu *vcpu)
> -{
> - if (vcpu->guest_xcr0_loaded) {
> - if (vcpu->arch.xcr0 != host_xcr0)
> - xsetbv(XCR_XFEATURE_ENABLED_MASK, host_xcr0);
> - vcpu->guest_xcr0_loaded = 0;
> - }
> -}
> -
> static void process_nmi(struct kvm_vcpu *vcpu)
> {
> unsigned limit = 2;
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: KVM: x86: fix maintenance of guest/host xcr0 state
2013-04-16 2:30 KVM: x86: fix maintenance of guest/host xcr0 state Marcelo Tosatti
2013-04-16 7:22 ` Paolo Bonzini
@ 2013-04-17 13:16 ` Gleb Natapov
2013-05-08 10:16 ` Gleb Natapov
2 siblings, 0 replies; 4+ messages in thread
From: Gleb Natapov @ 2013-04-17 13:16 UTC (permalink / raw)
To: Marcelo Tosatti; +Cc: kvm, Paolo Bonzini, Ulrich Obergfell
On Mon, Apr 15, 2013 at 11:30:13PM -0300, Marcelo Tosatti wrote:
>
> ** Untested **.
>
> Emulation of xcr0 writes zero guest_xcr0_loaded variable so that
> subsequent VM-entry reloads CPU's xcr0 with guests xcr0 value.
>
> However, this is incorrect because guest_xcr0_loaded variable is
> read to decide whether to reload hosts xcr0.
>
> In case the vcpu thread is scheduled out after the guest_xcr0_loaded = 0
> assignment, and scheduler decides to preload FPU:
>
> switch_to
> {
> __switch_to
> __math_state_restore
> restore_fpu_checking
> fpu_restore_checking
> if (use_xsave())
> fpu_xrstor_checking
> xrstor64 with CPU's xcr0 == guests xcr0
>
> Fix by properly restoring hosts xcr0 during emulation of xcr0 writes.
>
> Analyzed-by: Ulrich Obergfell <uobergfe@redhat.com>
> Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
>
Reviewed-by: Gleb Natapov <gleb@redhat.com>
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 999d124..222926a 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -555,6 +555,25 @@ void kvm_lmsw(struct kvm_vcpu *vcpu, unsigned long msw)
> }
> EXPORT_SYMBOL_GPL(kvm_lmsw);
>
> +static void kvm_load_guest_xcr0(struct kvm_vcpu *vcpu)
> +{
> + if (kvm_read_cr4_bits(vcpu, X86_CR4_OSXSAVE) &&
> + !vcpu->guest_xcr0_loaded) {
> + /* kvm_set_xcr() also depends on this */
> + xsetbv(XCR_XFEATURE_ENABLED_MASK, vcpu->arch.xcr0);
> + vcpu->guest_xcr0_loaded = 1;
> + }
> +}
> +
> +static void kvm_put_guest_xcr0(struct kvm_vcpu *vcpu)
> +{
> + if (vcpu->guest_xcr0_loaded) {
> + if (vcpu->arch.xcr0 != host_xcr0)
> + xsetbv(XCR_XFEATURE_ENABLED_MASK, host_xcr0);
> + vcpu->guest_xcr0_loaded = 0;
> + }
> +}
> +
> int __kvm_set_xcr(struct kvm_vcpu *vcpu, u32 index, u64 xcr)
> {
> u64 xcr0;
> @@ -571,8 +590,8 @@ int __kvm_set_xcr(struct kvm_vcpu *vcpu, u32 index, u64 xcr)
> return 1;
> if (xcr0 & ~host_xcr0)
> return 1;
> + kvm_put_guest_xcr0(vcpu);
> vcpu->arch.xcr0 = xcr0;
> - vcpu->guest_xcr0_loaded = 0;
> return 0;
> }
>
> @@ -5600,25 +5619,6 @@ static void inject_pending_event(struct kvm_vcpu *vcpu)
> }
> }
>
> -static void kvm_load_guest_xcr0(struct kvm_vcpu *vcpu)
> -{
> - if (kvm_read_cr4_bits(vcpu, X86_CR4_OSXSAVE) &&
> - !vcpu->guest_xcr0_loaded) {
> - /* kvm_set_xcr() also depends on this */
> - xsetbv(XCR_XFEATURE_ENABLED_MASK, vcpu->arch.xcr0);
> - vcpu->guest_xcr0_loaded = 1;
> - }
> -}
> -
> -static void kvm_put_guest_xcr0(struct kvm_vcpu *vcpu)
> -{
> - if (vcpu->guest_xcr0_loaded) {
> - if (vcpu->arch.xcr0 != host_xcr0)
> - xsetbv(XCR_XFEATURE_ENABLED_MASK, host_xcr0);
> - vcpu->guest_xcr0_loaded = 0;
> - }
> -}
> -
> static void process_nmi(struct kvm_vcpu *vcpu)
> {
> unsigned limit = 2;
--
Gleb.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: KVM: x86: fix maintenance of guest/host xcr0 state
2013-04-16 2:30 KVM: x86: fix maintenance of guest/host xcr0 state Marcelo Tosatti
2013-04-16 7:22 ` Paolo Bonzini
2013-04-17 13:16 ` Gleb Natapov
@ 2013-05-08 10:16 ` Gleb Natapov
2 siblings, 0 replies; 4+ messages in thread
From: Gleb Natapov @ 2013-05-08 10:16 UTC (permalink / raw)
To: Marcelo Tosatti; +Cc: kvm, Paolo Bonzini, Ulrich Obergfell
On Mon, Apr 15, 2013 at 11:30:13PM -0300, Marcelo Tosatti wrote:
>
> ** Untested **.
>
> Emulation of xcr0 writes zero guest_xcr0_loaded variable so that
> subsequent VM-entry reloads CPU's xcr0 with guests xcr0 value.
>
> However, this is incorrect because guest_xcr0_loaded variable is
> read to decide whether to reload hosts xcr0.
>
> In case the vcpu thread is scheduled out after the guest_xcr0_loaded = 0
> assignment, and scheduler decides to preload FPU:
>
> switch_to
> {
> __switch_to
> __math_state_restore
> restore_fpu_checking
> fpu_restore_checking
> if (use_xsave())
> fpu_xrstor_checking
> xrstor64 with CPU's xcr0 == guests xcr0
>
> Fix by properly restoring hosts xcr0 during emulation of xcr0 writes.
>
> Analyzed-by: Ulrich Obergfell <uobergfe@redhat.com>
> Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
>
Applied, thanks.
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 999d124..222926a 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -555,6 +555,25 @@ void kvm_lmsw(struct kvm_vcpu *vcpu, unsigned long msw)
> }
> EXPORT_SYMBOL_GPL(kvm_lmsw);
>
> +static void kvm_load_guest_xcr0(struct kvm_vcpu *vcpu)
> +{
> + if (kvm_read_cr4_bits(vcpu, X86_CR4_OSXSAVE) &&
> + !vcpu->guest_xcr0_loaded) {
> + /* kvm_set_xcr() also depends on this */
> + xsetbv(XCR_XFEATURE_ENABLED_MASK, vcpu->arch.xcr0);
> + vcpu->guest_xcr0_loaded = 1;
> + }
> +}
> +
> +static void kvm_put_guest_xcr0(struct kvm_vcpu *vcpu)
> +{
> + if (vcpu->guest_xcr0_loaded) {
> + if (vcpu->arch.xcr0 != host_xcr0)
> + xsetbv(XCR_XFEATURE_ENABLED_MASK, host_xcr0);
> + vcpu->guest_xcr0_loaded = 0;
> + }
> +}
> +
> int __kvm_set_xcr(struct kvm_vcpu *vcpu, u32 index, u64 xcr)
> {
> u64 xcr0;
> @@ -571,8 +590,8 @@ int __kvm_set_xcr(struct kvm_vcpu *vcpu, u32 index, u64 xcr)
> return 1;
> if (xcr0 & ~host_xcr0)
> return 1;
> + kvm_put_guest_xcr0(vcpu);
> vcpu->arch.xcr0 = xcr0;
> - vcpu->guest_xcr0_loaded = 0;
> return 0;
> }
>
> @@ -5600,25 +5619,6 @@ static void inject_pending_event(struct kvm_vcpu *vcpu)
> }
> }
>
> -static void kvm_load_guest_xcr0(struct kvm_vcpu *vcpu)
> -{
> - if (kvm_read_cr4_bits(vcpu, X86_CR4_OSXSAVE) &&
> - !vcpu->guest_xcr0_loaded) {
> - /* kvm_set_xcr() also depends on this */
> - xsetbv(XCR_XFEATURE_ENABLED_MASK, vcpu->arch.xcr0);
> - vcpu->guest_xcr0_loaded = 1;
> - }
> -}
> -
> -static void kvm_put_guest_xcr0(struct kvm_vcpu *vcpu)
> -{
> - if (vcpu->guest_xcr0_loaded) {
> - if (vcpu->arch.xcr0 != host_xcr0)
> - xsetbv(XCR_XFEATURE_ENABLED_MASK, host_xcr0);
> - vcpu->guest_xcr0_loaded = 0;
> - }
> -}
> -
> static void process_nmi(struct kvm_vcpu *vcpu)
> {
> unsigned limit = 2;
--
Gleb.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2013-05-08 10:16 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-04-16 2:30 KVM: x86: fix maintenance of guest/host xcr0 state Marcelo Tosatti
2013-04-16 7:22 ` Paolo Bonzini
2013-04-17 13:16 ` Gleb Natapov
2013-05-08 10:16 ` Gleb Natapov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox