Re: [PATCH] x86: svm: make wbinvd faster
2015-03-09 20:28-0300, Marcelo Tosatti: On Fri, Feb 27, 2015 at 06:19:18PM -0600, Joel Schopp wrote: From: David Kaplan david.kap...@amd.com No need to re-decode WBINVD since we know what it is from the intercept. Signed-off-by: David Kaplan david.kap...@amd.com [extracted from larger unlrelated patch, forward ported, tested] Signed-off-by: Joel Schopp joel.sch...@amd.com Can't you disable the intercept if need_emulate_wbinvd(vcpu) == false? I don't think we want to: it should be faster to intercept and ignore than to invalidate all caches. The exit doesn't affect other physical cores and costs just about 10(?) L3 cache misses. -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] x86: svm: make wbinvd faster
On Tue, Mar 10, 2015 at 12:01:31PM +0100, Radim Krčmář wrote: 2015-03-09 20:28-0300, Marcelo Tosatti: On Fri, Feb 27, 2015 at 06:19:18PM -0600, Joel Schopp wrote: From: David Kaplan david.kap...@amd.com No need to re-decode WBINVD since we know what it is from the intercept. Signed-off-by: David Kaplan david.kap...@amd.com [extracted from larger unlrelated patch, forward ported, tested] Signed-off-by: Joel Schopp joel.sch...@amd.com Can't you disable the intercept if need_emulate_wbinvd(vcpu) == false? I don't think we want to: it should be faster to intercept and ignore than to invalidate all caches. The exit doesn't affect other physical cores and costs just about 10(?) L3 cache misses. Yes, right. -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] x86: svm: make wbinvd faster
On Fri, Feb 27, 2015 at 06:19:18PM -0600, Joel Schopp wrote:
From: David Kaplan david.kap...@amd.com
No need to re-decode WBINVD since we know what it is from the intercept.
Signed-off-by: David Kaplan david.kap...@amd.com
[extracted from larger unlrelated patch, forward ported, tested]
Signed-off-by: Joel Schopp joel.sch...@amd.com
Can't you disable the intercept if need_emulate_wbinvd(vcpu) == false?
---
arch/x86/kvm/svm.c | 10 +-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index d319e0c..86ecd21 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -2776,6 +2776,14 @@ static int skinit_interception(struct vcpu_svm *svm)
return 1;
}
+static int wbinvd_interception(struct vcpu_svm *svm)
+{
+ kvm_emulate_wbinvd(svm-vcpu);
+ skip_emulated_instruction(svm-vcpu);
+ return 1;
+}
+
+
static int xsetbv_interception(struct vcpu_svm *svm)
{
u64 new_bv = kvm_read_edx_eax(svm-vcpu);
@@ -3376,7 +3384,7 @@ static int (*const svm_exit_handlers[])(struct vcpu_svm
*svm) = {
[SVM_EXIT_STGI] = stgi_interception,
[SVM_EXIT_CLGI] = clgi_interception,
[SVM_EXIT_SKINIT] = skinit_interception,
- [SVM_EXIT_WBINVD] = emulate_on_interception,
+ [SVM_EXIT_WBINVD] = wbinvd_interception,
[SVM_EXIT_MONITOR] = monitor_interception,
[SVM_EXIT_MWAIT]= mwait_interception,
[SVM_EXIT_XSETBV] = xsetbv_interception,
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] x86: svm: make wbinvd faster
2015-03-01 21:29-0500, Bandan Das:
Joel Schopp joel.sch...@amd.com writes:
From: David Kaplan david.kap...@amd.com
No need to re-decode WBINVD since we know what it is from the intercept.
Signed-off-by: David Kaplan david.kap...@amd.com
[extracted from larger unlrelated patch, forward ported, tested]
Signed-off-by: Joel Schopp joel.sch...@amd.com
---
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
+static int wbinvd_interception(struct vcpu_svm *svm)
+{
+ kvm_emulate_wbinvd(svm-vcpu);
+ skip_emulated_instruction(svm-vcpu);
+ return 1;
+}
+
+
Can't we merge this to kvm_emulate_wbinvd, and just call that function
directly for both vmx and svm ?
kvm_emulate_wbinvd() lives in x86.c and skip_emulated_instruction() is
from svm.c/vmx.c: so we'd have to create a new x86 op and change the
emulator code as well ... it's probably better like this.
static int xsetbv_interception(struct vcpu_svm *svm)
{
u64 new_bv = kvm_read_edx_eax(svm-vcpu);
@@ -3376,7 +3384,7 @@ static int (*const svm_exit_handlers[])(struct
vcpu_svm *svm) = {
[SVM_EXIT_STGI] = stgi_interception,
[SVM_EXIT_CLGI] = clgi_interception,
[SVM_EXIT_SKINIT] = skinit_interception,
- [SVM_EXIT_WBINVD] = emulate_on_interception,
So, this means x86_emulate_insn() in emulate.c has no callers left for the
wbinvd case ? vmx calls kvm_emulate_wbinvd directly too..
I think that invalid state emulation might still hit wbinvd.
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] x86: svm: make wbinvd faster
2015-02-27 18:19-0600, Joel Schopp:
From: David Kaplan david.kap...@amd.com
No need to re-decode WBINVD since we know what it is from the intercept.
Signed-off-by: David Kaplan david.kap...@amd.com
[extracted from larger unlrelated patch, forward ported, tested]
Signed-off-by: Joel Schopp joel.sch...@amd.com
---
Reviewed-by: Radim Krčmář rkrc...@redhat.com
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
+static int wbinvd_interception(struct vcpu_svm *svm)
+{
+ kvm_emulate_wbinvd(svm-vcpu);
+ skip_emulated_instruction(svm-vcpu);
+ return 1;
+}
+
+
(One line is optimal.)
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] x86: svm: make wbinvd faster
Radim Krčmář rkrc...@redhat.com writes:
2015-03-01 21:29-0500, Bandan Das:
Joel Schopp joel.sch...@amd.com writes:
From: David Kaplan david.kap...@amd.com
No need to re-decode WBINVD since we know what it is from the intercept.
Signed-off-by: David Kaplan david.kap...@amd.com
[extracted from larger unlrelated patch, forward ported, tested]
Signed-off-by: Joel Schopp joel.sch...@amd.com
---
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
+static int wbinvd_interception(struct vcpu_svm *svm)
+{
+ kvm_emulate_wbinvd(svm-vcpu);
+ skip_emulated_instruction(svm-vcpu);
+ return 1;
+}
+
+
Can't we merge this to kvm_emulate_wbinvd, and just call that function
directly for both vmx and svm ?
kvm_emulate_wbinvd() lives in x86.c and skip_emulated_instruction() is
from svm.c/vmx.c: so we'd have to create a new x86 op and change the
emulator code as well ... it's probably better like this.
There's already one - kvm_x86_ops-skip_emulated_instruction
static int xsetbv_interception(struct vcpu_svm *svm)
{
u64 new_bv = kvm_read_edx_eax(svm-vcpu);
@@ -3376,7 +3384,7 @@ static int (*const svm_exit_handlers[])(struct
vcpu_svm *svm) = {
[SVM_EXIT_STGI] = stgi_interception,
[SVM_EXIT_CLGI] = clgi_interception,
[SVM_EXIT_SKINIT] = skinit_interception,
- [SVM_EXIT_WBINVD] = emulate_on_interception,
So, this means x86_emulate_insn() in emulate.c has no callers left for the
wbinvd case ? vmx calls kvm_emulate_wbinvd directly too..
I think that invalid state emulation might still hit wbinvd.
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] x86: svm: make wbinvd faster
2015-03-02 10:25-0500, Bandan Das:
Radim Krčmář rkrc...@redhat.com writes:
2015-03-01 21:29-0500, Bandan Das:
Joel Schopp joel.sch...@amd.com writes:
+static int wbinvd_interception(struct vcpu_svm *svm)
+{
+kvm_emulate_wbinvd(svm-vcpu);
+skip_emulated_instruction(svm-vcpu);
+return 1;
+}
Can't we merge this to kvm_emulate_wbinvd, and just call that function
directly for both vmx and svm ?
kvm_emulate_wbinvd() lives in x86.c and skip_emulated_instruction() is
from svm.c/vmx.c: so we'd have to create a new x86 op and change the
emulator code as well ... it's probably better like this.
There's already one - kvm_x86_ops-skip_emulated_instruction
My bad, its usage is inconsistent and I only looked at two close
interceptions where it was used ... kvm_emulate_cpuid() calls
kvm_x86_ops-skip_emulated_instruction(), while kvm_emulate_halt() and
kvm_emulate_hypercall() need an external skip.
We do skip the instruction with kvm_emulate(), so automatically
skipping the instruction on kvm_emulate_*() makes sense:
1. rename kvm_emulate_halt() and kvm_emulate_wbinvd() to accommodate
callers that don't want to skip
2. introduce kvm_emulate_{halt,wbinvd}() and move the skip to to
kvm_emulate_{halt,wbinvd,hypercall}()
The alternative is to remove kvm_x86_ops-skip_emulated_instruction():
1. remove skip from kvm_emulate_cpuid() and modify callers
2. move kvm_complete_insn_gp to a header file and use
skip_emulated_instruction directly
3. remove unused kvm_x86_ops-skip_emulated_instruction()
Which one do you prefer?
Thanks.
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] x86: svm: make wbinvd faster
On 03/02/2015 10:03 AM, Radim Krčmář wrote:
2015-03-02 10:25-0500, Bandan Das:
Radim Krčmář rkrc...@redhat.com writes:
2015-03-01 21:29-0500, Bandan Das:
Joel Schopp joel.sch...@amd.com writes:
+static int wbinvd_interception(struct vcpu_svm *svm)
+{
+ kvm_emulate_wbinvd(svm-vcpu);
+ skip_emulated_instruction(svm-vcpu);
+ return 1;
+}
Can't we merge this to kvm_emulate_wbinvd, and just call that function
directly for both vmx and svm ?
kvm_emulate_wbinvd() lives in x86.c and skip_emulated_instruction() is
from svm.c/vmx.c: so we'd have to create a new x86 op and change the
emulator code as well ... it's probably better like this.
There's already one - kvm_x86_ops-skip_emulated_instruction
My bad, its usage is inconsistent and I only looked at two close
interceptions where it was used ... kvm_emulate_cpuid() calls
kvm_x86_ops-skip_emulated_instruction(), while kvm_emulate_halt() and
kvm_emulate_hypercall() need an external skip.
We do skip the instruction with kvm_emulate(), so automatically
skipping the instruction on kvm_emulate_*() makes sense:
1. rename kvm_emulate_halt() and kvm_emulate_wbinvd() to accommodate
callers that don't want to skip
2. introduce kvm_emulate_{halt,wbinvd}() and move the skip to to
kvm_emulate_{halt,wbinvd,hypercall}()
The alternative is to remove kvm_x86_ops-skip_emulated_instruction():
1. remove skip from kvm_emulate_cpuid() and modify callers
2. move kvm_complete_insn_gp to a header file and use
skip_emulated_instruction directly
3. remove unused kvm_x86_ops-skip_emulated_instruction()
Which one do you prefer?
I prefer renaming them, ie kvm_emulate_wbinvd_noskip(), and making the
existing ones, ie kvm_emulate_wbinvd() call the noskip verion and add a
skip similar to how wbinvd_interception above does. I can send out a
patch later today with that rework.
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] x86: svm: make wbinvd faster
Joel Schopp joel.sch...@amd.com writes:
From: David Kaplan david.kap...@amd.com
No need to re-decode WBINVD since we know what it is from the intercept.
Signed-off-by: David Kaplan david.kap...@amd.com
[extracted from larger unlrelated patch, forward ported, tested]
Signed-off-by: Joel Schopp joel.sch...@amd.com
---
arch/x86/kvm/svm.c | 10 +-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index d319e0c..86ecd21 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -2776,6 +2776,14 @@ static int skinit_interception(struct vcpu_svm *svm)
return 1;
}
+static int wbinvd_interception(struct vcpu_svm *svm)
+{
+ kvm_emulate_wbinvd(svm-vcpu);
+ skip_emulated_instruction(svm-vcpu);
+ return 1;
+}
+
+
Can't we merge this to kvm_emulate_wbinvd, and just call that function
directly for both vmx and svm ?
static int xsetbv_interception(struct vcpu_svm *svm)
{
u64 new_bv = kvm_read_edx_eax(svm-vcpu);
@@ -3376,7 +3384,7 @@ static int (*const svm_exit_handlers[])(struct vcpu_svm
*svm) = {
[SVM_EXIT_STGI] = stgi_interception,
[SVM_EXIT_CLGI] = clgi_interception,
[SVM_EXIT_SKINIT] = skinit_interception,
- [SVM_EXIT_WBINVD] = emulate_on_interception,
So, this means x86_emulate_insn() in emulate.c has no callers left for the
wbinvd case ? vmx calls kvm_emulate_wbinvd directly too..
Bandan
+ [SVM_EXIT_WBINVD] = wbinvd_interception,
[SVM_EXIT_MONITOR] = monitor_interception,
[SVM_EXIT_MWAIT]= mwait_interception,
[SVM_EXIT_XSETBV] = xsetbv_interception,
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
