* [PATCH] kvm: svm: Only propagate next_rip when guest supports it
@ 2015-10-09 9:51 Joerg Roedel
2015-10-09 11:15 ` Paolo Bonzini
0 siblings, 1 reply; 4+ messages in thread
From: Joerg Roedel @ 2015-10-09 9:51 UTC (permalink / raw)
To: Paolo Bonzini, Gleb Natapov
Cc: kvm, linux-kernel, Joerg Roedel, Bandan Das, Dirk Mueller
From: Joerg Roedel <jroedel@suse.de>
Currently we always write the next_rip of the shadow vmcb to
the guests vmcb when we emulate a vmexit. This could confuse
the guest when its cpuid indicated no support for the
next_rip feature.
Fix this by only propagating next_rip if the guest actually
supports it.
Cc: Bandan Das <bsd@redhat.com>
Cc: Dirk Mueller <dmueller@suse.com>
Tested-by: Dirk Mueller <dmueller@suse.com>
Signed-off-by: Joerg Roedel <jroedel@suse.de>
---
arch/x86/kvm/cpuid.h | 21 +++++++++++++++++++++
arch/x86/kvm/svm.c | 4 +++-
2 files changed, 24 insertions(+), 1 deletion(-)
diff --git a/arch/x86/kvm/cpuid.h b/arch/x86/kvm/cpuid.h
index dd05b9c..effca1f 100644
--- a/arch/x86/kvm/cpuid.h
+++ b/arch/x86/kvm/cpuid.h
@@ -133,4 +133,25 @@ static inline bool guest_cpuid_has_mpx(struct kvm_vcpu *vcpu)
best = kvm_find_cpuid_entry(vcpu, 7, 0);
return best && (best->ebx & bit(X86_FEATURE_MPX));
}
+
+/*
+ * NRIPS is provided through cpuidfn 0x8000000a.edx bit 3
+ */
+#define BIT_NRIPS 3
+
+static inline bool guest_cpuid_has_nrips(struct kvm_vcpu *vcpu)
+{
+ struct kvm_cpuid_entry2 *best;
+
+ best = kvm_find_cpuid_entry(vcpu, 0x8000000a, 0);
+
+ /*
+ * NRIPS is a scattered cpuid feature, so we can't use
+ * X86_FEATURE_NRIPS here (X86_FEATURE_NRIPS would be bit
+ * position 8, not 3).
+ */
+ return best && (best->edx & bit(BIT_NRIPS));
+}
+#undef BIT_NRIPS
+
#endif
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index 2f9ed1f..4084b33 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -2365,7 +2365,9 @@ static int nested_svm_vmexit(struct vcpu_svm *svm)
nested_vmcb->control.exit_info_2 = vmcb->control.exit_info_2;
nested_vmcb->control.exit_int_info = vmcb->control.exit_int_info;
nested_vmcb->control.exit_int_info_err = vmcb->control.exit_int_info_err;
- nested_vmcb->control.next_rip = vmcb->control.next_rip;
+
+ if (guest_cpuid_has_nrips(&svm->vcpu))
+ nested_vmcb->control.next_rip = vmcb->control.next_rip;
/*
* If we emulate a VMRUN/#VMEXIT in the same host #vmexit cycle we have
--
1.9.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] kvm: svm: Only propagate next_rip when guest supports it
2015-10-09 9:51 [PATCH] kvm: svm: Only propagate next_rip when guest supports it Joerg Roedel
@ 2015-10-09 11:15 ` Paolo Bonzini
2015-10-14 13:10 ` [PATCH v2] " Joerg Roedel
0 siblings, 1 reply; 4+ messages in thread
From: Paolo Bonzini @ 2015-10-09 11:15 UTC (permalink / raw)
To: Joerg Roedel, Gleb Natapov
Cc: kvm, linux-kernel, Joerg Roedel, Bandan Das, Dirk Mueller
On 09/10/2015 11:51, Joerg Roedel wrote:
> From: Joerg Roedel <jroedel@suse.de>
>
> Currently we always write the next_rip of the shadow vmcb to
> the guests vmcb when we emulate a vmexit. This could confuse
> the guest when its cpuid indicated no support for the
> next_rip feature.
>
> Fix this by only propagating next_rip if the guest actually
> supports it.
>
> Cc: Bandan Das <bsd@redhat.com>
> Cc: Dirk Mueller <dmueller@suse.com>
> Tested-by: Dirk Mueller <dmueller@suse.com>
> Signed-off-by: Joerg Roedel <jroedel@suse.de>
> ---
> arch/x86/kvm/cpuid.h | 21 +++++++++++++++++++++
> arch/x86/kvm/svm.c | 4 +++-
> 2 files changed, 24 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/kvm/cpuid.h b/arch/x86/kvm/cpuid.h
> index dd05b9c..effca1f 100644
> --- a/arch/x86/kvm/cpuid.h
> +++ b/arch/x86/kvm/cpuid.h
> @@ -133,4 +133,25 @@ static inline bool guest_cpuid_has_mpx(struct kvm_vcpu *vcpu)
> best = kvm_find_cpuid_entry(vcpu, 7, 0);
> return best && (best->ebx & bit(X86_FEATURE_MPX));
> }
> +
> +/*
> + * NRIPS is provided through cpuidfn 0x8000000a.edx bit 3
> + */
> +#define BIT_NRIPS 3
> +
> +static inline bool guest_cpuid_has_nrips(struct kvm_vcpu *vcpu)
> +{
> + struct kvm_cpuid_entry2 *best;
> +
> + best = kvm_find_cpuid_entry(vcpu, 0x8000000a, 0);
> +
> + /*
> + * NRIPS is a scattered cpuid feature, so we can't use
> + * X86_FEATURE_NRIPS here (X86_FEATURE_NRIPS would be bit
> + * position 8, not 3).
> + */
> + return best && (best->edx & bit(BIT_NRIPS));
> +}
> +#undef BIT_NRIPS
> +
> #endif
> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
> index 2f9ed1f..4084b33 100644
> --- a/arch/x86/kvm/svm.c
> +++ b/arch/x86/kvm/svm.c
> @@ -2365,7 +2365,9 @@ static int nested_svm_vmexit(struct vcpu_svm *svm)
> nested_vmcb->control.exit_info_2 = vmcb->control.exit_info_2;
> nested_vmcb->control.exit_int_info = vmcb->control.exit_int_info;
> nested_vmcb->control.exit_int_info_err = vmcb->control.exit_int_info_err;
> - nested_vmcb->control.next_rip = vmcb->control.next_rip;
> +
> + if (guest_cpuid_has_nrips(&svm->vcpu))
This could be a bit expensive to do on every vmexit. Can you benchmark
it with kvm-unit-tests, or just cache the result in struct vcpu_svm?
Thanks,
Paolo
> + nested_vmcb->control.next_rip = vmcb->control.next_rip;
>
> /*
> * If we emulate a VMRUN/#VMEXIT in the same host #vmexit cycle we have
>
^ permalink raw reply [flat|nested] 4+ messages in thread* [PATCH v2] kvm: svm: Only propagate next_rip when guest supports it
2015-10-09 11:15 ` Paolo Bonzini
@ 2015-10-14 13:10 ` Joerg Roedel
2015-10-14 15:11 ` Paolo Bonzini
0 siblings, 1 reply; 4+ messages in thread
From: Joerg Roedel @ 2015-10-14 13:10 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Gleb Natapov, kvm, linux-kernel, Joerg Roedel, Bandan Das,
Dirk Mueller
On Fri, Oct 09, 2015 at 01:15:11PM +0200, Paolo Bonzini wrote:
> This could be a bit expensive to do on every vmexit. Can you benchmark
> it with kvm-unit-tests, or just cache the result in struct vcpu_svm?
Yes, caching it is certainly a good idea. I updated the patch:
>From 94ee662c527683c26ea5fa98a5a8f2c798c58470 Mon Sep 17 00:00:00 2001
From: Joerg Roedel <jroedel@suse.de>
Date: Wed, 7 Oct 2015 13:38:19 +0200
Subject: [PATCH] kvm: svm: Only propagate next_rip when guest supports it
Currently we always write the next_rip of the shadow vmcb to
the guests vmcb when we emulate a vmexit. This could confuse
the guest when its cpuid indicated no support for the
next_rip feature.
Fix this by only propagating next_rip if the guest actually
supports it.
Cc: Bandan Das <bsd@redhat.com>
Cc: Dirk Mueller <dmueller@suse.com>
Tested-By: Dirk Mueller <dmueller@suse.com>
Signed-off-by: Joerg Roedel <jroedel@suse.de>
---
arch/x86/kvm/cpuid.h | 21 +++++++++++++++++++++
arch/x86/kvm/svm.c | 11 ++++++++++-
2 files changed, 31 insertions(+), 1 deletion(-)
diff --git a/arch/x86/kvm/cpuid.h b/arch/x86/kvm/cpuid.h
index dd05b9c..effca1f 100644
--- a/arch/x86/kvm/cpuid.h
+++ b/arch/x86/kvm/cpuid.h
@@ -133,4 +133,25 @@ static inline bool guest_cpuid_has_mpx(struct kvm_vcpu *vcpu)
best = kvm_find_cpuid_entry(vcpu, 7, 0);
return best && (best->ebx & bit(X86_FEATURE_MPX));
}
+
+/*
+ * NRIPS is provided through cpuidfn 0x8000000a.edx bit 3
+ */
+#define BIT_NRIPS 3
+
+static inline bool guest_cpuid_has_nrips(struct kvm_vcpu *vcpu)
+{
+ struct kvm_cpuid_entry2 *best;
+
+ best = kvm_find_cpuid_entry(vcpu, 0x8000000a, 0);
+
+ /*
+ * NRIPS is a scattered cpuid feature, so we can't use
+ * X86_FEATURE_NRIPS here (X86_FEATURE_NRIPS would be bit
+ * position 8, not 3).
+ */
+ return best && (best->edx & bit(BIT_NRIPS));
+}
+#undef BIT_NRIPS
+
#endif
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index 2f9ed1f..e9e3294 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -159,6 +159,9 @@ struct vcpu_svm {
u32 apf_reason;
u64 tsc_ratio;
+
+ /* cached guest cpuid flags for faster access */
+ bool nrips_enabled : 1;
};
static DEFINE_PER_CPU(u64, current_tsc_ratio);
@@ -2365,7 +2368,9 @@ static int nested_svm_vmexit(struct vcpu_svm *svm)
nested_vmcb->control.exit_info_2 = vmcb->control.exit_info_2;
nested_vmcb->control.exit_int_info = vmcb->control.exit_int_info;
nested_vmcb->control.exit_int_info_err = vmcb->control.exit_int_info_err;
- nested_vmcb->control.next_rip = vmcb->control.next_rip;
+
+ if (svm->nrips_enabled)
+ nested_vmcb->control.next_rip = vmcb->control.next_rip;
/*
* If we emulate a VMRUN/#VMEXIT in the same host #vmexit cycle we have
@@ -4098,6 +4103,10 @@ static u64 svm_get_mt_mask(struct kvm_vcpu *vcpu, gfn_t gfn, bool is_mmio)
static void svm_cpuid_update(struct kvm_vcpu *vcpu)
{
+ struct vcpu_svm *svm = to_svm(vcpu);
+
+ /* Update nrips enabled cache */
+ svm->nrips_enabled = !!guest_cpuid_has_nrips(&svm->vcpu);
}
static void svm_set_supported_cpuid(u32 func, struct kvm_cpuid_entry2 *entry)
--
1.8.4.5
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH v2] kvm: svm: Only propagate next_rip when guest supports it
2015-10-14 13:10 ` [PATCH v2] " Joerg Roedel
@ 2015-10-14 15:11 ` Paolo Bonzini
0 siblings, 0 replies; 4+ messages in thread
From: Paolo Bonzini @ 2015-10-14 15:11 UTC (permalink / raw)
To: Joerg Roedel
Cc: Gleb Natapov, kvm, linux-kernel, Joerg Roedel, Bandan Das,
Dirk Mueller
On 14/10/2015 15:10, Joerg Roedel wrote:
> From 94ee662c527683c26ea5fa98a5a8f2c798c58470 Mon Sep 17 00:00:00 2001
> From: Joerg Roedel <jroedel@suse.de>
> Date: Wed, 7 Oct 2015 13:38:19 +0200
> Subject: [PATCH] kvm: svm: Only propagate next_rip when guest supports it
>
> Currently we always write the next_rip of the shadow vmcb to
> the guests vmcb when we emulate a vmexit. This could confuse
> the guest when its cpuid indicated no support for the
> next_rip feature.
>
> Fix this by only propagating next_rip if the guest actually
> supports it.
>
> Cc: Bandan Das <bsd@redhat.com>
> Cc: Dirk Mueller <dmueller@suse.com>
> Tested-By: Dirk Mueller <dmueller@suse.com>
> Signed-off-by: Joerg Roedel <jroedel@suse.de>
> ---
> arch/x86/kvm/cpuid.h | 21 +++++++++++++++++++++
> arch/x86/kvm/svm.c | 11 ++++++++++-
> 2 files changed, 31 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/kvm/cpuid.h b/arch/x86/kvm/cpuid.h
> index dd05b9c..effca1f 100644
> --- a/arch/x86/kvm/cpuid.h
> +++ b/arch/x86/kvm/cpuid.h
> @@ -133,4 +133,25 @@ static inline bool guest_cpuid_has_mpx(struct kvm_vcpu *vcpu)
> best = kvm_find_cpuid_entry(vcpu, 7, 0);
> return best && (best->ebx & bit(X86_FEATURE_MPX));
> }
> +
> +/*
> + * NRIPS is provided through cpuidfn 0x8000000a.edx bit 3
> + */
> +#define BIT_NRIPS 3
> +
> +static inline bool guest_cpuid_has_nrips(struct kvm_vcpu *vcpu)
> +{
> + struct kvm_cpuid_entry2 *best;
> +
> + best = kvm_find_cpuid_entry(vcpu, 0x8000000a, 0);
> +
> + /*
> + * NRIPS is a scattered cpuid feature, so we can't use
> + * X86_FEATURE_NRIPS here (X86_FEATURE_NRIPS would be bit
> + * position 8, not 3).
> + */
> + return best && (best->edx & bit(BIT_NRIPS));
> +}
> +#undef BIT_NRIPS
> +
> #endif
> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
> index 2f9ed1f..e9e3294 100644
> --- a/arch/x86/kvm/svm.c
> +++ b/arch/x86/kvm/svm.c
> @@ -159,6 +159,9 @@ struct vcpu_svm {
> u32 apf_reason;
>
> u64 tsc_ratio;
> +
> + /* cached guest cpuid flags for faster access */
> + bool nrips_enabled : 1;
> };
>
> static DEFINE_PER_CPU(u64, current_tsc_ratio);
> @@ -2365,7 +2368,9 @@ static int nested_svm_vmexit(struct vcpu_svm *svm)
> nested_vmcb->control.exit_info_2 = vmcb->control.exit_info_2;
> nested_vmcb->control.exit_int_info = vmcb->control.exit_int_info;
> nested_vmcb->control.exit_int_info_err = vmcb->control.exit_int_info_err;
> - nested_vmcb->control.next_rip = vmcb->control.next_rip;
> +
> + if (svm->nrips_enabled)
> + nested_vmcb->control.next_rip = vmcb->control.next_rip;
>
> /*
> * If we emulate a VMRUN/#VMEXIT in the same host #vmexit cycle we have
> @@ -4098,6 +4103,10 @@ static u64 svm_get_mt_mask(struct kvm_vcpu *vcpu, gfn_t gfn, bool is_mmio)
>
> static void svm_cpuid_update(struct kvm_vcpu *vcpu)
> {
> + struct vcpu_svm *svm = to_svm(vcpu);
> +
> + /* Update nrips enabled cache */
> + svm->nrips_enabled = !!guest_cpuid_has_nrips(&svm->vcpu);
> }
>
> static void svm_set_supported_cpuid(u32 func, struct kvm_cpuid_entry2 *entry)
>
Applied to kvm/queue, thanks.
Paolo
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2015-10-14 15:11 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-10-09 9:51 [PATCH] kvm: svm: Only propagate next_rip when guest supports it Joerg Roedel
2015-10-09 11:15 ` Paolo Bonzini
2015-10-14 13:10 ` [PATCH v2] " Joerg Roedel
2015-10-14 15:11 ` Paolo Bonzini
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).