On Sun, Jul 6, 2008 at 4:34 PM, Avi Kivity <[EMAIL PROTECTED]> wrote:
> Mohammed Gamal wrote:
>>
>> On Sun, Jul 6, 2008 at 10:51 AM, Avi Kivity <[EMAIL PROTECTED]> 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 <[EMAIL PROTECTED]>
>>>>
>>>> ---
>>>>
>>>> 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.
>
The instruction pointer would haven been written back anyway as we do
go to the done label anyway. But I am convinced with your agrument.
>> 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.
Yes it does, I'll send the updated patch right now.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
