[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [Xen-devel] Re: [Patch 2/4] Refining Xsave/Xrestore support - Version 2
>>> On 29.10.10 at 03:25, Haitao Shan <maillists.shan@xxxxxxxxx> wrote: >@@ -376,7 +392,10 @@ int vcpu_initialise(struct vcpu *v) > > spin_lock_init(&v->arch.shadow_ldt_lock); > >- return (is_pv_32on64_vcpu(v) ? setup_compat_l4(v) : 0); >+ if ( (rc = (is_pv_32on64_vcpu(v) ? setup_compat_l4(v) : 0)) != 0 ) >+ xfree(v->arch.xsave_area); This surely is ugly to read. Why can't this be simply if ( is_pv_32on64_vcpu(v) ) rc = setup_compat_l4(v); if ( rc ) xfree(v->arch.xsave_area); with rc zero-initialized at the top of the function. >... >+ case 0xd1: /* XSETBV */ >+ { >+ u64 new_xfeature = regs->eax | ((u64)regs->edx << 32); You need (u32)regs->eax here. >+ if ( !guest_kernel_mode(v, regs) >+ /* Currently only XCR0 is implemented */ >+ || regs->ecx != XCR_XFEATURE_ENABLED_MASK >+ /* bit 0 of XCR0 must be set */ >+ || !(new_xfeature & XSTATE_FP) >+ /* Reserved bit must not be set */ >+ || (new_xfeature & ~xfeature_mask) ) >+ goto fail; You need to check for the use of invalid prefixes here (as otherwise we risk mis-emulating future meanings of prefixed versions of this opcode). Further, some of the failures here really need to report #UD (instead of #GP) to the guest. To minimize future changes I'd also suggest starting with a switch ( (u32)regs->ecx ) from the beginning (note that in any case you'll have to ignore the upper 32 bits). > #define real_cr4_to_pv_guest_cr4(c) \ >- ((c) & ~(X86_CR4_PGE | X86_CR4_PSE | X86_CR4_TSD | X86_CR4_OSXSAVE)) >+ ((c) & ~(X86_CR4_PGE | X86_CR4_PSE | X86_CR4_TSD)) I don't think this is correct: You don't want the guest to see the actual CR4 value, but its emulated version. And you handle things accordingly already in pv_guest_cr4_fixup(). Jan _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxx http://lists.xensource.com/xen-devel
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |