Re: [ANNOUNCE] kvm-17 release

[Michael Riepe <michael-0QoEqw4nQxo@public.gmane.org>]

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?

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?

-- 
Michael "Tired" Riepe <michael-0QoEqw4nQxo@public.gmane.org>
X-Tired: Each morning I get up I die a little

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