Re: [PATCH] x86 emulator: Add IRET instruction

[Paolo Bonzini <pbonzini@redhat.com>]

On 07/26/2010 11:00 AM, Avi Kivity wrote:
> On 07/26/2010 11:47 AM, Paolo Bonzini wrote:
>>> I don't think this is needed. The temp_eflags value is assigned
>>> directly to eflags if we're operand size is 16 bits. At least
>>> that's what the Intel manual says!
>>
>> That's fine, but please make sure that
>>
>> mov %sp, %bp
>>  orw $2, 4(%bp)
>>  iret
>>
>> followed at return site by
>>
>> pushf
>> popw %ax
>>
>> does not set bit 1 in %ax. That's the important point (also see how
>> emulate_popf avoids magic hex constants).
>
> Moreover, vmx will fail the next entry if this is not done. 23.3.1.4
> says:
>
>> RFLAGS. — Reserved bits 63:22 (bits 31:22 on processors that do not
>> support Intel 64 architecture), bit 15, bit 5 and bit 3 must be 0
>> in the field, and reserved bit 1 must be 1.

(I remembered one bit had to be 1, but failed to recall which one.  I 
should have looked up SAHF in the manual).  This means that my code 
actually should be

  mov %sp, %bp
  orw $8, 4(%bp)
  iret

followed by testing bit 3.

The emulate_popf approach that explicitly lists bits taken from the 
stack seems more robust.  For example, Mohammed's "if (c->op_bytes == 
4)" code leaves bit 1 cleared:

     temp_eflags = ((temp_eflags & 0x257fd5)
           | (ctxt->eflags & 0x1a0000));

(But then it is probably never used since only a 32-bit code segment in 
unreal mode would trigger it).

Paolo