Re: [PATCH] x86 emulator: Add IRET instruction

[Paolo Bonzini <pbonzini@redhat.com>]

On 07/28/2010 03:30 PM, Paolo Bonzini wrote:
> On 07/28/2010 11:38 AM, Mohammed Gamal wrote:
>> + unsigned long mask = EFLG_CF | EFLG_PF | EFLG_AF | EFLG_ZF | EFLG_SF
>> | EFLG_TF |
>> + EFLG_IF | EFLG_DF | EFLG_OF | EFLG_IOPL | EFLG_NT | EFLG_RF |
>> + EFLG_AC | EFLG_ID | (1 << 1); /* Last one is the reserved bit */
>> + unsigned long vm86_mask = EFLG_VM | EFLG_VIF | EFLG_VIP;
>> ...
>> + if (c->op_bytes == 4)
>> + ctxt->eflags = ((temp_eflags & mask) | (ctxt->eflags & vm86_mask));
>> + else if (c->op_bytes == 2) {
>> + ctxt->eflags &= ~0xffff;
>> + ctxt->eflags |= temp_eflags;
>> + }
>
> I think that's still not it. You can set reserved bits for c->op_bytes
> == 2, and you can clear bit 1 for both 16- and 32-bit IRET.
>
> IOW you need something like this:
>
> mask = ...; /* without (1 << 1); */
> ctxt_mask = (1 << 1) | EFLG_VM | EFLG_VIF | EFLG_VIP;
>
> if (c->op_bytes == 2) {
> mask &= 0xffff;
> ctxt_mask |= ~0xffff;
> }
>
> ctxt->eflags = (temp_eflags & mask) | (ctxt->eflags & ctxt_mask);

Sorry, I replied to v3 of the patch while reviewing v2.

Looks good indeed.

Thanks!

Paolo