[Xen-devel] Re: [Patch 1/3] Refining Xsave/Xrestore support - Version 3

["Jan Beulich" <JBeulich@xxxxxxxxxx>]

>>> On 02.11.10 at 04:47, Haitao Shan <maillists.shan@xxxxxxxxx> wrote:
>@@ -1796,7 +1796,10 @@ static int emulate_privileged_op(struct 
> 
>     /* REX prefix. */
>     if ( rex & 8 ) /* REX.W */
>+    {
>+        opsize_prefix = 0; /* 66H is ignored according to SDM 2A */

This I'm sure isn't correct: The 0x66 prefix is being ignored as an
operand size override here, but REX.W has no interaction with
0x66 when the latter is used as an opcode extension selector.

Specifically in the case of xsetbv the manual clearly states

#UD     If CPUID.01H:ECX.XSAVE[bit 26] = 0.
        If CR4.OSXSAVE[bit 18] = 0.
        If the LOCK prefix is used.
        If 66H, F3H or F2H prefix is used.

>@@ -2051,13 +2054,48 @@ static int emulate_privileged_op(struct 
>         goto fail;
>     switch ( opcode )
>     {
>-    case 0x1: /* RDTSCP */
>-        if ( (v->arch.guest_context.ctrlreg[4] & X86_CR4_TSD) &&
>-             !guest_kernel_mode(v, regs) )
>+    case 0x1: /* RDTSCP and XSETBV */
>+        switch ( insn_fetch(u8, code_base, eip, code_limit) )
>+        {
>+        case 0xf9: /* RDTSCP */
>+            if ( (v->arch.guest_context.ctrlreg[4] & X86_CR4_TSD) &&
>+                 !guest_kernel_mode(v, regs) )
>+                goto fail;
>+            pv_soft_rdtsc(v, regs, 1);
>+            break;
>+        case 0xd1: /* XSETBV */
>+        {
>+            u64 new_xfeature = (u32)regs->eax | ((u64)regs->edx << 32);
>+
>+            if ( lock || rep_prefix || opsize_prefix
>+                 || !(v->arch.guest_context.ctrlreg[4] & X86_CR4_OSXSAVE) )
>+            {
>+                do_guest_trap(TRAP_invalid_op, regs, 0);
>+                break;

I think you need to "goto skip" or "return EXCRET_fault_fixed" here,
to avoid executing instruction_done().

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel