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

[Laurent Vivier <Laurent.Vivier-6ktuUTfB/bM@public.gmane.org>]

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

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2005.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/