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