This patch implements the xsetbv intercept to the AMD part
of KVM. This makes AVX usable in a save way for the guest on
AVX capable AMD hardware.
The patch is tested by using AVX in the guest and host in
parallel and checking for data corruption. I also used the
KVM xsave unit-tests and they all pass.
Signed-off-by: Joerg Roedel <[email protected]>
---
arch/x86/include/asm/svm.h | 2 ++
arch/x86/kvm/svm.c | 16 ++++++++++++++++
2 files changed, 18 insertions(+), 0 deletions(-)
diff --git a/arch/x86/include/asm/svm.h b/arch/x86/include/asm/svm.h
index 11dbca7..7f3a304 100644
--- a/arch/x86/include/asm/svm.h
+++ b/arch/x86/include/asm/svm.h
@@ -47,6 +47,7 @@ enum {
INTERCEPT_MONITOR,
INTERCEPT_MWAIT,
INTERCEPT_MWAIT_COND,
+ INTERCEPT_XSETBV,
};
@@ -326,6 +327,7 @@ struct __attribute__ ((__packed__)) vmcb {
#define SVM_EXIT_MONITOR 0x08a
#define SVM_EXIT_MWAIT 0x08b
#define SVM_EXIT_MWAIT_COND 0x08c
+#define SVM_EXIT_XSETBV 0x08d
#define SVM_EXIT_NPF 0x400
#define SVM_EXIT_ERR -1
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index c00ea90..9cd0c14 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -904,6 +904,7 @@ static void init_vmcb(struct vcpu_svm *svm)
set_intercept(svm, INTERCEPT_WBINVD);
set_intercept(svm, INTERCEPT_MONITOR);
set_intercept(svm, INTERCEPT_MWAIT);
+ set_intercept(svm, INTERCEPT_XSETBV);
control->iopm_base_pa = iopm_base;
control->msrpm_base_pa = __pa(svm->msrpm);
@@ -2493,6 +2494,19 @@ static int skinit_interception(struct vcpu_svm *svm)
return 1;
}
+static int xsetbv_interception(struct vcpu_svm *svm)
+{
+ u64 new_bv = kvm_read_edx_eax(&svm->vcpu);
+ u32 index = kvm_register_read(&svm->vcpu, VCPU_REGS_RCX);
+
+ if (kvm_set_xcr(&svm->vcpu, index, new_bv) == 0) {
+ svm->next_rip = kvm_rip_read(&svm->vcpu) + 3;
+ skip_emulated_instruction(&svm->vcpu);
+ }
+
+ return 1;
+}
+
static int invalid_op_interception(struct vcpu_svm *svm)
{
kvm_queue_exception(&svm->vcpu, UD_VECTOR);
@@ -2916,6 +2930,7 @@ static int (*svm_exit_handlers[])(struct vcpu_svm *svm) = {
[SVM_EXIT_WBINVD] = emulate_on_interception,
[SVM_EXIT_MONITOR] = invalid_op_interception,
[SVM_EXIT_MWAIT] = invalid_op_interception,
+ [SVM_EXIT_XSETBV] = xsetbv_interception,
[SVM_EXIT_NPF] = pf_interception,
};
@@ -3628,6 +3643,7 @@ static const struct trace_print_flags svm_exit_reasons_str[] = {
{ SVM_EXIT_WBINVD, "wbinvd" },
{ SVM_EXIT_MONITOR, "monitor" },
{ SVM_EXIT_MWAIT, "mwait" },
+ { SVM_EXIT_XSETBV, "xsetbv" },
{ SVM_EXIT_NPF, "npf" },
{ -1, NULL }
};
--
1.7.1
On 12/03/2010 06:42 PM, Joerg Roedel wrote:
> This patch implements the xsetbv intercept to the AMD part
> of KVM. This makes AVX usable in a save way for the guest on
> AVX capable AMD hardware.
> The patch is tested by using AVX in the guest and host in
> parallel and checking for data corruption. I also used the
> KVM xsave unit-tests and they all pass.
>
That is really strange. You didn't need to do anything to get cpuid.avx
recognized. So running an older kvm on newer hardware will happily
expose avx even though it's not supported.
We screwed up - we should have made cpuid.avx dependent on vendor support.
The patch itself looks fine, I just want to understand this point first.
--
error compiling committee.c: too many arguments to function
On Mon, Dec 06, 2010 at 11:10:46AM -0500, Avi Kivity wrote:
> On 12/03/2010 06:42 PM, Joerg Roedel wrote:
> > This patch implements the xsetbv intercept to the AMD part
> > of KVM. This makes AVX usable in a save way for the guest on
> > AVX capable AMD hardware.
> > The patch is tested by using AVX in the guest and host in
> > parallel and checking for data corruption. I also used the
> > KVM xsave unit-tests and they all pass.
> >
>
> That is really strange. You didn't need to do anything to get cpuid.avx
> recognized. So running an older kvm on newer hardware will happily
> expose avx even though it's not supported.
>
> We screwed up - we should have made cpuid.avx dependent on vendor support.
Hmm, right. The set_supported_cpuid arch-callback should basically
disable xsave on AMD for all KVM versions which do not handle the xsetbv
intercept.
Joerg
--
AMD Operating System Research Center
Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach
General Managers: Alberto Bozzo, Andrew Bowd
Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632
On 12/06/2010 07:48 PM, Roedel, Joerg wrote:
> On Mon, Dec 06, 2010 at 11:10:46AM -0500, Avi Kivity wrote:
> > On 12/03/2010 06:42 PM, Joerg Roedel wrote:
> > > This patch implements the xsetbv intercept to the AMD part
> > > of KVM. This makes AVX usable in a save way for the guest on
> > > AVX capable AMD hardware.
> > > The patch is tested by using AVX in the guest and host in
> > > parallel and checking for data corruption. I also used the
> > > KVM xsave unit-tests and they all pass.
> > >
> >
> > That is really strange. You didn't need to do anything to get cpuid.avx
> > recognized. So running an older kvm on newer hardware will happily
> > expose avx even though it's not supported.
> >
> > We screwed up - we should have made cpuid.avx dependent on vendor support.
>
> Hmm, right. The set_supported_cpuid arch-callback should basically
> disable xsave on AMD for all KVM versions which do not handle the xsetbv
> intercept.
>
Please post a patch to do that, and update this patch to undo the
change. We'll backport the first patch to -stable so that people
running older kernels don't get a nasty surprise when they upgrade their
hardware (and use -cpu host).
One more thing to watch out for. We also need to see if there aren't
more mistakes like that out there.
--
error compiling committee.c: too many arguments to function
On Tue, Dec 07, 2010 at 04:34:33AM -0500, Avi Kivity wrote:
> On 12/06/2010 07:48 PM, Roedel, Joerg wrote:
> > On Mon, Dec 06, 2010 at 11:10:46AM -0500, Avi Kivity wrote:
> > > On 12/03/2010 06:42 PM, Joerg Roedel wrote:
> > > > This patch implements the xsetbv intercept to the AMD part
> > > > of KVM. This makes AVX usable in a save way for the guest on
> > > > AVX capable AMD hardware.
> > > > The patch is tested by using AVX in the guest and host in
> > > > parallel and checking for data corruption. I also used the
> > > > KVM xsave unit-tests and they all pass.
> > > >
> > >
> > > That is really strange. You didn't need to do anything to get cpuid.avx
> > > recognized. So running an older kvm on newer hardware will happily
> > > expose avx even though it's not supported.
> > >
> > > We screwed up - we should have made cpuid.avx dependent on vendor support.
> >
> > Hmm, right. The set_supported_cpuid arch-callback should basically
> > disable xsave on AMD for all KVM versions which do not handle the xsetbv
> > intercept.
> >
>
> Please post a patch to do that, and update this patch to undo the
> change. We'll backport the first patch to -stable so that people
> running older kernels don't get a nasty surprise when they upgrade their
> hardware (and use -cpu host).
Ok, I'll post them soon.
--
AMD Operating System Research Center
Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach
General Managers: Alberto Bozzo, Andrew Bowd
Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632
This patch implements the xsetbv intercept to the AMD part
of KVM. This makes AVX usable in a save way for the guest on
AVX capable AMD hardware.
The patch is tested by using AVX in the guest and host in
parallel and checking for data corruption. I also used the
KVM xsave unit-tests and they all pass.
Signed-off-by: Joerg Roedel <[email protected]>
---
arch/x86/include/asm/svm.h | 2 ++
arch/x86/kvm/svm.c | 20 ++++++++++++++++----
2 files changed, 18 insertions(+), 4 deletions(-)
diff --git a/arch/x86/include/asm/svm.h b/arch/x86/include/asm/svm.h
index 11dbca7..7f3a304 100644
--- a/arch/x86/include/asm/svm.h
+++ b/arch/x86/include/asm/svm.h
@@ -47,6 +47,7 @@ enum {
INTERCEPT_MONITOR,
INTERCEPT_MWAIT,
INTERCEPT_MWAIT_COND,
+ INTERCEPT_XSETBV,
};
@@ -326,6 +327,7 @@ struct __attribute__ ((__packed__)) vmcb {
#define SVM_EXIT_MONITOR 0x08a
#define SVM_EXIT_MWAIT 0x08b
#define SVM_EXIT_MWAIT_COND 0x08c
+#define SVM_EXIT_XSETBV 0x08d
#define SVM_EXIT_NPF 0x400
#define SVM_EXIT_ERR -1
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index 8aeda9f..4a5e584 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -904,6 +904,7 @@ static void init_vmcb(struct vcpu_svm *svm)
set_intercept(svm, INTERCEPT_WBINVD);
set_intercept(svm, INTERCEPT_MONITOR);
set_intercept(svm, INTERCEPT_MWAIT);
+ set_intercept(svm, INTERCEPT_XSETBV);
control->iopm_base_pa = iopm_base;
control->msrpm_base_pa = __pa(svm->msrpm);
@@ -2494,6 +2495,19 @@ static int skinit_interception(struct vcpu_svm *svm)
return 1;
}
+static int xsetbv_interception(struct vcpu_svm *svm)
+{
+ u64 new_bv = kvm_read_edx_eax(&svm->vcpu);
+ u32 index = kvm_register_read(&svm->vcpu, VCPU_REGS_RCX);
+
+ if (kvm_set_xcr(&svm->vcpu, index, new_bv) == 0) {
+ svm->next_rip = kvm_rip_read(&svm->vcpu) + 3;
+ skip_emulated_instruction(&svm->vcpu);
+ }
+
+ return 1;
+}
+
static int invalid_op_interception(struct vcpu_svm *svm)
{
kvm_queue_exception(&svm->vcpu, UD_VECTOR);
@@ -2917,6 +2931,7 @@ static int (*svm_exit_handlers[])(struct vcpu_svm *svm) = {
[SVM_EXIT_WBINVD] = emulate_on_interception,
[SVM_EXIT_MONITOR] = invalid_op_interception,
[SVM_EXIT_MWAIT] = invalid_op_interception,
+ [SVM_EXIT_XSETBV] = xsetbv_interception,
[SVM_EXIT_NPF] = pf_interception,
};
@@ -3556,10 +3571,6 @@ static void svm_cpuid_update(struct kvm_vcpu *vcpu)
static void svm_set_supported_cpuid(u32 func, struct kvm_cpuid_entry2 *entry)
{
switch (func) {
- case 0x00000001:
- /* Mask out xsave bit as long as it is not supported by SVM */
- entry->ecx &= ~(bit(X86_FEATURE_XSAVE));
- break;
case 0x80000001:
if (nested)
entry->ecx |= (1 << 2); /* Set SVM bit */
@@ -3633,6 +3644,7 @@ static const struct trace_print_flags svm_exit_reasons_str[] = {
{ SVM_EXIT_WBINVD, "wbinvd" },
{ SVM_EXIT_MONITOR, "monitor" },
{ SVM_EXIT_MWAIT, "mwait" },
+ { SVM_EXIT_XSETBV, "xsetbv" },
{ SVM_EXIT_NPF, "npf" },
{ -1, NULL }
};
--
1.7.1
To support xsave properly for the guest the SVM module need
software support for it. As long as this is not present do
not report the xsave as supported feature in cpuid.
As a side-effect this patch moves the bit() helper function
into the x86.h file so that it can be used in svm.c too.
Cc: [email protected]
Signed-off-by: Joerg Roedel <[email protected]>
---
arch/x86/kvm/svm.c | 4 ++++
arch/x86/kvm/vmx.c | 5 -----
arch/x86/kvm/x86.c | 5 -----
arch/x86/kvm/x86.h | 5 +++++
4 files changed, 9 insertions(+), 10 deletions(-)
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index ae943bb..8aeda9f 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -3556,6 +3556,10 @@ static void svm_cpuid_update(struct kvm_vcpu *vcpu)
static void svm_set_supported_cpuid(u32 func, struct kvm_cpuid_entry2 *entry)
{
switch (func) {
+ case 0x00000001:
+ /* Mask out xsave bit as long as it is not supported by SVM */
+ entry->ecx &= ~(bit(X86_FEATURE_XSAVE));
+ break;
case 0x80000001:
if (nested)
entry->ecx |= (1 << 2); /* Set SVM bit */
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 72cfdb7..f3693ca 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -4247,11 +4247,6 @@ static int vmx_get_lpage_level(void)
return PT_PDPE_LEVEL;
}
-static inline u32 bit(int bitno)
-{
- return 1 << (bitno & 31);
-}
-
static void vmx_cpuid_update(struct kvm_vcpu *vcpu)
{
struct kvm_cpuid_entry2 *best;
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index ed373ba..a85a989 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -163,11 +163,6 @@ static inline void kvm_async_pf_hash_reset(struct kvm_vcpu *vcpu)
vcpu->arch.apf.gfns[i] = ~0;
}
-static inline u32 bit(int bitno)
-{
- return 1 << (bitno & 31);
-}
-
static void kvm_on_user_return(struct user_return_notifier *urn)
{
unsigned slot;
diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h
index 2cea414..c600da8 100644
--- a/arch/x86/kvm/x86.h
+++ b/arch/x86/kvm/x86.h
@@ -70,6 +70,11 @@ static inline int is_paging(struct kvm_vcpu *vcpu)
return kvm_read_cr0_bits(vcpu, X86_CR0_PG);
}
+static inline u32 bit(int bitno)
+{
+ return 1 << (bitno & 31);
+}
+
void kvm_before_handle_nmi(struct kvm_vcpu *vcpu);
void kvm_after_handle_nmi(struct kvm_vcpu *vcpu);
int kvm_inject_realmode_interrupt(struct kvm_vcpu *vcpu, int irq);
--
1.7.1
On 12/07/2010 06:15 PM, Joerg Roedel wrote:
> This patch implements the xsetbv intercept to the AMD part
> of KVM. This makes AVX usable in a save way for the guest on
> AVX capable AMD hardware.
> The patch is tested by using AVX in the guest and host in
> parallel and checking for data corruption. I also used the
> KVM xsave unit-tests and they all pass.
>
Applied both, thanks.
--
error compiling committee.c: too many arguments to function