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

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

[-- Attachment #1.1: Type: text/plain, Size: 2518 bytes --]

Avi Kivity wrote:
> Laurent Vivier wrote:
>> 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 ?
>>   
> 
> Decode can actually have fetch faults in smp (due to the instruction
> lengthening during decode, or due to the page tables changing with
> npt/ept).
> 
> I think these are the only two errors possible for decode: can't decode
> and can't fetch.
> 
>> Should we remove it from after the emulate stage ?
>>
>>   
> 
> Instruction execution shouldn't cause decode failures, so yes, that
> error shouldn't be emitted there.
> 
> But we can defer these fine tunings until later. Let's merge something
> that works first.

Agree, I think it is better to merge something close to the original behavior
before improving it.

I try to post patches today.

Laurent
-- 
------------- Laurent.Vivier-6ktuUTfB/bM@public.gmane.org  --------------
          "Software is hard" - Donald Knuth


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

[-- Attachment #2: Type: text/plain, Size: 228 bytes --]

-------------------------------------------------------------------------
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/

[-- Attachment #3: Type: text/plain, Size: 186 bytes --]

_______________________________________________
kvm-devel mailing list
kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
https://lists.sourceforge.net/lists/listinfo/kvm-devel