Re: [PATCH] x86 emulator: Update EIP even with instructions with no writeback

[Avi Kivity <avi@qumranet.com>]

Mohammed Gamal wrote:
> On Sun, Jul 6, 2008 at 10:51 AM, Avi Kivity <avi@qumranet.com> wrote:
>   
>> Mohammed Gamal wrote:
>>     
>>> This patch resolves the problem encountered with HLT emulation with
>>> FreeDOS's HIMEM XMS Driver.
>>> HLT is the only instruction that goes to the done label unconditionally,
>>> causing the EIP value not to be updated which leads to the guest looping
>>> forever on the same instruction.
>>>
>>> Signed-off-by: Mohammed Gamal <m.gamal005@gmail.com>
>>>
>>> ---
>>>
>>>  arch/x86/kvm/x86_emulate.c |    4 +++-
>>>  1 files changed, 3 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/arch/x86/kvm/x86_emulate.c b/arch/x86/kvm/x86_emulate.c
>>> index dd4efe1..04d7f02 100644
>>> --- a/arch/x86/kvm/x86_emulate.c
>>> +++ b/arch/x86/kvm/x86_emulate.c
>>> @@ -1769,13 +1769,15 @@ writeback:
>>>          /* Commit shadow register state. */
>>>        memcpy(ctxt->vcpu->arch.regs, c->regs, sizeof c->regs);
>>> -       kvm_rip_write(ctxt->vcpu, c->eip);
>>>   done:
>>>        if (rc == X86EMUL_UNHANDLEABLE) {
>>>                c->eip = saved_eip;
>>>                return -1;
>>>        }
>>> +       else
>>> +               kvm_rip_write(ctxt->vcpu, c->eip);
>>> +
>>>        return 0;
>>>       
>> Why not change hlt to writeback like all other instructions?
>>
>>     
>
> IIRC hlt doesn't do writebacks. So, instead of changing hlt to go for
> a bogus writeback, I thought it'd be more logical that since we're
> going to the done label anyway we check first if the instruction is
> unhandleable, in which case we write the saved EIP, otherwise we
> update the EIP value.
>   

It's not bogus, you have to write back the instruction pointer at 
least.  It also helps having less code paths.

> Anyway, here is a patch that changes hlt to writeback.
>   

Does it solve the problem?  If so, please provide an updated changelog 
entry and a signoff.

-- 
error compiling committee.c: too many arguments to function