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

Laurent Vivier
2007-09-17T19:59:14Z

Avi Kivity wrote:
> Laurent Vivier (Bull) wrote:
>> >
>> > 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;
>> }
>> ...
>>
>
> Yes. But pushing the kvm_mmu_unprotect_page() to immediately after the
> decode stage may be better.
>

OK, but is this the only error case we can have in the decode stage ?
Should we remove it from after the emulate stage ?

Laurent