On 06/01/2010 08:12 PM, Marcelo Tosatti wrote:
On Mon, May 31, 2010 at 07:54:11PM +0800, Sheng Yang wrote:
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 99ae513..8649627 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -36,6 +36,8 @@
#include<asm/vmx.h>
#include<asm/virtext.h>
#include<asm/mce.h>
+#include<asm/i387.h>
+#include<asm/xcr.h>
#include "trace.h"
@@ -3354,6 +3356,16 @@ static int handle_wbinvd(struct kvm_vcpu *vcpu)
return 1;
}
+static int handle_xsetbv(struct kvm_vcpu *vcpu)
+{
+ u64 new_bv = kvm_read_edx_eax(vcpu);
+ u32 index = kvm_register_read(vcpu, VCPU_REGS_RCX);
+
+ kvm_set_xcr(vcpu, index, new_bv);
+ skip_emulated_instruction(vcpu);
Should only skip_emulated_instruction if no exception was injected.
Good catch. So, unit testing failure cases _is_ important.
+int __kvm_set_xcr(struct kvm_vcpu *vcpu, u32 index, u64 xcr)
+{
+ u64 xcr0;
+
+ /* Only support XCR_XFEATURE_ENABLED_MASK(xcr0) now */
+ if (index != XCR_XFEATURE_ENABLED_MASK)
+ return 1;
+ xcr0 = xcr;
+ if (kvm_x86_ops->get_cpl(vcpu) != 0)
+ return 1;
+ if (!(xcr0& XSTATE_FP))
+ return 1;
+ if ((xcr0& XSTATE_YMM)&& !(xcr0& XSTATE_SSE))
+ return 1;
+ if (xcr0& ~host_xcr0)
+ return 1;
+ vcpu->arch.xcr0 = xcr0;
+ xsetbv(XCR_XFEATURE_ENABLED_MASK, vcpu->arch.xcr0);
Won't this happen on guest entry, since vcpu->guest_xcr0_loaded == 0?
In fact we don't know what vcpu->guest_xcr0_loaded is. Best to unload
it explicitly (and drop the xsetbv() call).
+ vcpu->guest_xcr0_loaded = 1;
+ }
+}
+
+static void kvm_put_guest_xcr0(struct kvm_vcpu *vcpu)
+{
+ if (kvm_read_cr4_bits(vcpu, X86_CR4_OSXSAVE)&&
+ vcpu->guest_xcr0_loaded) {
+ xsetbv(XCR_XFEATURE_ENABLED_MASK, host_xcr0);
+ vcpu->guest_xcr0_loaded = 0;
+ }
+}
What if you load guest's XCR0, then guest clears CR4.OSXSAVE? (restore
of host_xcr0 should be conditional on guest_xcr0_loaded only).
Good catch again.
@@ -5140,6 +5250,8 @@ void kvm_load_guest_fpu(struct kvm_vcpu *vcpu)
void kvm_put_guest_fpu(struct kvm_vcpu *vcpu)
{
+ kvm_put_guest_xcr0(vcpu);
+
Should be in kvm_arch_vcpu_put?
That's called unconditionally from kvm_arch_vcpu_put(), so equivalent.
--
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
