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

Laurent Vivier
2007-09-18T07:40:42Z

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