kvm-17 release

Hi!

With respect to the Core Duo, this is the best release I've seen so far:

- FreeBSD 6.1: works.
- NetBSD 3.1: works.
- OpenSuse 10.2 (sans gfxmenu): works.
- Qemu-Puppy 2.01-3: works.
- Qemu-Puppy 2.13-1: works.
- Solaris 10.1: works.
- XP Pro: works.

Unfortunately, Knoppix (both 5.0 and 5.1.1) has stopped working now. I didn't try 5.2 yet.

Quoted 1 line · Michael Riepe

With respect to the Core Duo, this is the best release I've seen so far:

Actually, the above line can be preserved, if we move the previous line down by 1: Was it a simple race?

Quoted 7 lines

Actually, the above line can be preserved, if we move the previous line down by 1: Was it a simple race?

kvm_main.c renamed from kvm_main-17.c +0 -1

[..]
-		vcpu->mmio_read_completed = 1;
  		emulate_instruction(vcpu, kvm_run, vcpu->mmio_fault_cr2, 0);

kvm_main.c line 1578

Quoted 1 line

kvm_main.c renamed from kvm_main-17.c +1 -0

+		vcpu->mmio_read_completed = 1;

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. [..]
Or did I get something wrong?

Hi!

Quoted 8 lines · Carsten Emde

Actually, the above line can be preserved, if we move the previous line down by 1: Was it a simple race?

kvm_main.c line 1581

Quoted 9 lines

kvm_main.c renamed from kvm_main-17.c +1 -1

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

Review

Quoted 7 lines · Carsten Emde

I then reverted to kvm-16 and repeated all upgrade changes until the problem occurred. The culprit was found to be
[..]

(quoted diff) +2 -0

+#ifdef THIS_PREVENTS_FC6_FROM_BOOTING
          emulate_instruction(vcpu, kvm_run, vcpu->mmio_fault_cr2, 0);
+#endif
[..]

Actually, the above line can be preserved, if we move the previous line down by 1: Was it a simple race?

Quoted 3 lines · Avi Kivity

Quoted 1 line · Michael Riepe

With respect to the Core Duo, this is the best release I've seen so far:

Unfortunately, kvm-17 was less successful here. FC6 did not boot any longer after I upgraded from kvm-16 to kvm-17, so I was rather surprised when I read the above message.

This is the test system:
CPU: Intel(R) Core(TM)2 CPU 6600 @ 2.40GHz, Chipset: Intel 975X

Quoted 4 lines

Quoted 1 line

Quoted 1 line

With respect to the Core Duo, this is the best release I've seen so far:

Unfortunately, kvm-17 was less successful here. FC6 did not boot any longer after I upgraded from kvm-16 to kvm-17, so I was rather surprised when I read the above message. [..]

But it was the first release that worked with both Net- and FreeBSD as well as Solaris.

Probably the same problem that killed Knoppix in -17 (-16 worked fine). [..] Oh by the way: Knoppix 5.2 behaves the same way as 5.1.1. With -17, it hangs before the penguin logo appears on the framebuffer.

Quoted 2 lines

This is the test system:
CPU: Intel(R) Core(TM)2 CPU 6600 @ 2.40GHz, Chipset: Intel 975X

There's quite a difference between Core Duo and Core 2 Duo.

Did the host and/or guest run in 64-bit mode, by the way?

Hi!

Quoted 6 lines · Carsten Emde

Quoted 3 lines · Avi Kivity

Michael Riepe wrote:

Quoted 6 lines

Quoted 2 lines

Quoted 2 lines

With respect to the Core Duo, this is the best release I've seen so far:

Unfortunately, kvm-17 was less successful here. FC6 did not boot any longer after I upgraded from kvm-16 to kvm-17, so I was rather surprised when I read the above message.

Probably the same problem that killed Knoppix in -17 (-16 worked fine).

Quoted 3 lines · Avi Kivity

Quoted 2 lines · Michael Riepe

With respect to the Core Duo, this is the best release I've seen so far:

Unfortunately, kvm-17 was less successful here. FC6 did not boot any longer after I upgraded from kvm-16 to kvm-17, so I was rather surprised when I read the above message.

This is the test system:
CPU: Intel(R) Core(TM)2 CPU 6600 @ 2.40GHz, Chipset: Intel 975X

The exact problem was that FC6 stopped booting before under kvm-16 it would display the line
IP route cache hash table entries: 4096 (order: 2, 16384 bytes)

I then reverted to kvm-16 and repeated all upgrade changes until the problem occurred. The culprit was found to be

Hi!

With respect to the Core Duo, this is the best release I've seen so far:

- FreeBSD 6.1: works.
- NetBSD 3.1: works.
- OpenSuse 10.2 (sans gfxmenu): works.
- Qemu-Puppy 2.01-3: works.
- Qemu-Puppy 2.13-1: works.
- Solaris 10.1: works.
- XP Pro: works.

Unfortunately, Knoppix (both 5.0 and 5.1.1) has stopped working now. I didn't try 5.2 yet.

(using Kernel 2.6.20)