Michael Riepe wrote:
> Hi!
>
> Carsten Emde wrote:
>
>
>> Actually, the above line can be preserved, if we move the previous line
>> down by 1: Was it a simple race?
>>
>> --- kvm_main-17.c 2007-03-24 02:09:00.000000000 +0100
>> +++ kvm_main.c 2007-03-24 04:10:59.000000000 +0100
>> @@ -1574,8 +1574,8 @@
>>
>> if (kvm_run->mmio_completed) {
>> memcpy(vcpu->mmio_data, kvm_run->mmio.data, 8);
>> - vcpu->mmio_read_completed = 1;
>> emulate_instruction(vcpu, kvm_run, vcpu->mmio_fault_cr2, 0);
>> + vcpu->mmio_read_completed = 1;
>> }
>>
>> vcpu->mmio_needed = 0;
>>
>>
>
> As far as I understand, vcpu->mmio_read_completed indicates that read
> data is present in vcpu->mmio_data. If the flag isn't set,
> emulator_read_emulated() - which is called by emulate_instruction() -
> will try to read the data *again*. While that's supposed to be okay for
> ordinary memory, it might make memory mapped i/o devices fail.
>
> On the other hand, emulator_read_emulated() clears the flag to indicate
> that the emulated read has actually happened - which means that another
> read may drop us into userspace for emulation again. There is, however,
> no other place where the flag is reset, so setting it after
> emulate_instruction() may affect the *next* emulated instruction. I
> think that the flag should be cleared after the call, rather than set.
>
> Besides that, vcpu->mmio_read_completed seems to guard a call to
> do_interrupt_requests() in vmx_vcpu_run(). I guess the reason is that
> interrupts must not be processed if the current instruction has already
> been partially emulated.
>
> Or did I get something wrong?
>
No, it looks right. The code is fairly convoluted due to a control
inversion problem: the instruction emulator wants to call userspace, but
the only efficient way to do that is to return to userspace, and have
userspace call the kernel again with the flag marking the continuation.
> There's another point here that bothers me: The result of
> emulate_instruction() isn't checked. Are you sure that it never fails,
> and that it's safe to proceed to kvm_arch_ops->run(), aka vmx_vcpu_run()
> in case it's an Intel CPU?
>
>
When emulate_instruction() fails, it's a benign failure; it doesn't
update any state.
--
error compiling committee.c: too many arguments to function
-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel
