Re: [Xen-devel] [PATCH v2 2/3] x86emul: correct handling of FPU insns faulting on memory write

[Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>]

On 03/15/2017 09:31 AM, Jan Beulich wrote:
>>>> On 15.03.17 at 14:24, <boris.ostrovsky@xxxxxxxxxx> wrote:
>> On 03/15/2017 06:28 AM, Jan Beulich wrote:
>>> @@ -3716,9 +3720,9 @@ x86_emulate(
>>>          break;
>>>  
>>>      case 0x9b:  /* wait/fwait */
>>> -        fic.insn_bytes = 1;
>>>          host_and_vcpu_must_have(fpu);
>>>          get_fpu(X86EMUL_FPU_wait, &fic);
>>> +        fic.insn_bytes = 1;
>>>          asm volatile ( "fwait" ::: "memory" );
>>>          check_fpu_exn(&fic);
>>>          break;
>> Why is this needed?
> This isn't strictly needed, but desirable, due to the conditional being
> added in
>
> @@ -7916,7 +7920,7 @@ x86_emulate(
>      ctxt->regs->eflags &= ~X86_EFLAGS_RF;
>  
>   done:
> -    put_fpu(&fic, ctxt, ops);
> +    put_fpu(&fic, fic.insn_bytes > 0 && dst.type == OP_MEM, ctxt, ops);
>      put_stub(stub);
>      return rc;
>  #undef state
>
> (both host_and_vcpu_must_have() and get_fpu() may end up
> branching to "done"). Everywhere else the field is already being
> set after such basic checks.

Ah, OK.

But fic is a local variable that is not initialized (is it?) so
insn_bytes may be non-zero anyway?

-boris


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel