Re: [PATCH 0/5] Split the emulator: decode & execute

Laurent Vivier (Bull)
2007-09-17T19:08:01Z

Avi Kivity wrote:
> Laurent Vivier wrote:
>> Avi Kivity wrote:
...
>>> Interesting. I don't see how an instruction fetch can fail on
>>> uniprocessor. Can you give details of the failure?
>>>
>>> Instruction fetches can fail on SMP so a fix is certainly needed.
>>>
>> OK, I spoke too fast.
>>
>> x86_decode_insn() fails because it is not able to decode:
>>
>> 0xffffffff8110b7ef <__copy_user_nocache+47>: movnti %r11,(%rdi)
>> or
>> 0xffffffff8110b7ef <__copy_user_nocache+47>: 0x4c 0x0f 0xc3
0x1f
>>
>> 0x4c is decoded as a REX prefix.
>> 0x0f is decoded as a Two-byte opcode
>> but 0xc3 is unknown in twobyte_table, so we exit because of an
unrecognized
>> opcode ("Cannot emulate").
>>
>>
>
> Not being able to emulate is sometimes legitimate. In the case of
> writing to a write-protected guest page table, we simply
> un-write-protect it and go back to the guest (which should now execute
> the instruction natively).
>
> Perhaps the logic that deals with this (the call to
> kvm_mmu_unprotect_page_virt() in emulate_instruction()) was broken by
> your changes.
>

In fact this case is managed in the error cases of
emulate_instruction(). My first patch removes this management for
instruction decoding because I supposed it cannot generate such errors.
So what I proposed in my last email seems to be the good solution :

emulate_instruction()
...
r = x86_decode_insn(&vcpu->emulate_ctxt, &emulate_ops);
if (r == 0)
r = x86_emulate_insn(&vcpu->emulate_ctxt, &emulate_ops);
...
if (r) {
if (kvm_mmu_unprotect_page_virt(vcpu, cr2))
return EMULATE_DONE;
if (!vcpu->mmio_needed) {
kvm_report_emulation_failure(vcpu, "mmio");
return EMULATE_FAIL;
}
return EMULATE_DO_MMIO;
}
...

Regards,
Laurent