KVM: Ensure the exit frequency to QEmu for coalesced MMIO

The default action of calesced MMIO is, cache the writing in buffer, until:
1. The buffer is full.
2. Or the exit to QEmu due to other reasons.

But this would result in a very late writing in some condition.
1. The each time write to MMIO content is small.
2. The writing interval is big.
3. No need for input or accessing other devices frequently.

This issue was observed in a experimental embbed system. The test image
simply print "test" every 1 seconds. The output in QEmu meets expectation, but the output in KVM is delayed for seconds.

To solve this issue, a timeout is added, to ensure userspace exit freqency
is high enough(mostly target for video now) to write the buffered MMIO data. Current the maximum exit interval is 1/25s(so 25 times exit to userspace per second at least, pretty low compared to normal environment), and reused KVM_EXIT_IRQ_WINDOW_OPEN as exit reason, for it would doing nothing at all in userspace handler.

Signed-off-by Sheng Yang <[email protected]>

---
 arch/x86/include/asm/kvm_host.h |    2 ++
 arch/x86/kvm/x86.c              |   31 +++++++++++++++++++++++++++++++
 2 files changed, 33 insertions(+), 0 deletions(-)

(quoted diff) +0 -0

 b/arch/x86/include/asm/kvm_host.h index a1f0b5d..eb8bb20 100644

arch/x86/include/asm/kvm_host.h +2 -0

@@ -365,6 +365,8 @@ struct kvm_vcpu_arch {
 	unsigned long singlestep_rip;
 	/* fields used by HYPER-V emulation */
 	u64 hv_vapic;
+
+	ktime_t latest_userspace_exit_time;
 };
 
 struct kvm_mem_alias {

arch/x86/kvm/x86.c +28 -0

index 56a90a6..0b05f11 100644
@@ -4339,6 +4339,20 @@ out:
 	return r;
 }
 
+#ifdef CONFIG_KVM_MMIO
+
+#define KVM_USERSPACE_MMIO_MAX_INTERVAL (NSEC_PER_SEC / 25)
+static bool mmio_need_exit_to_userspace(struct kvm_vcpu *vcpu)
+{
+	ktime_t gap, now = ktime_get();
+
+	gap = ktime_sub(now, vcpu->arch.latest_userspace_exit_time);
+	if (ktime_to_ns(gap) > KVM_USERSPACE_MMIO_MAX_INTERVAL)
+		return true;
+
+	return false;
+}
+#endif
 
 static int __vcpu_run(struct kvm_vcpu *vcpu)
 {

@@ -4404,6 +4418,10 @@ static int __vcpu_run(struct kvm_vcpu *vcpu)
 			kvm_resched(vcpu);
 			vcpu->srcu_idx = srcu_read_lock(&kvm->srcu);
 		}
+#ifdef CONFIG_KVM_MMIO
+		if (mmio_need_exit_to_userspace(vcpu))
+			r = 0;
+#endif
 	}
 
 	srcu_read_unlock(&kvm->srcu, vcpu->srcu_idx);

@@ -4463,6 +4481,16 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu,
 struct kvm_run *kvm_run)
 
 	r = __vcpu_run(vcpu);
 
+#ifdef CONFIG_KVM_MMIO
+	if (mmio_need_exit_to_userspace(vcpu)) {
+		/* Use KVM_EXIT_IRQ_WINDOW_OPEN because userspace would do
+		 * nothing to handle it */
+		kvm_run->exit_reason = KVM_EXIT_IRQ_WINDOW_OPEN;
+		r = 0;
+	}
+	vcpu->arch.latest_userspace_exit_time = ktime_get();
+#endif
+
 out:
 	if (vcpu->sigset_active)

arch/x86/kvm/x86.c +3 -0

 		sigprocmask(SIG_SETMASK, &sigsaved, NULL);

@@ -5455,6 +5483,9 @@ int kvm_arch_vcpu_init(struct kvm_vcpu *vcpu)
 		goto fail_mmu_destroy;
 	}
 	vcpu->arch.mcg_cap = KVM_MAX_MCE_BANKS;
+#ifdef CONFIG_KVM_MMIO
+	vcpu->arch.latest_userspace_exit_time = ktime_get();
+#endif
 
 	return 0;
 

The default action of calesced MMIO is, cache the writing in buffer, until:
1. The buffer is full.
2. Or the exit to QEmu due to other reasons.

But this would result in a very late writing in some condition.
1. The each time write to MMIO content is small.
2. The writing interval is big.
3. No need for input or accessing other devices frequently.

This issue was observed in a experimental embbed system. The test image simply print "test" every 1 seconds. The output in QEmu meets expectation, but the output in KVM is delayed for seconds.

To solve this issue, a timeout is added, to ensure userspace exit freqency is high enough(mostly target for video now) to write the buffered MMIO data. Current the maximum exit interval is 1/25s(so 25 times exit to userspace per second at least, pretty low compared to normal environment), and reused KVM_EXIT_IRQ_WINDOW_OPEN as exit reason, for it would doing nothing at all in userspace handler.

Quoted 13 lines · Sheng Yang

The default action of calesced MMIO is, cache the writing in buffer,
until: 1. The buffer is full.
2. Or the exit to QEmu due to other reasons.

But this would result in a very late writing in some condition.
1. The each time write to MMIO content is small.
2. The writing interval is big.
3. No need for input or accessing other devices frequently.

This issue was observed in a experimental embbed system. The test image simply print "test" every 1 seconds. The output in QEmu meets expectation, but the output in KVM is delayed for seconds.

Quoted 6 lines

To solve this issue, a timeout is added, to ensure userspace exit freqency is high enough(mostly target for video now) to write the buffered MMIO data. Current the maximum exit interval is 1/25s(so 25 times exit to userspace per second at least, pretty low compared to normal environment), and reused KVM_EXIT_IRQ_WINDOW_OPEN as exit reason, for it would doing nothing at all in userspace handler.

This can be done from userspace, by scheduling a SIGALRM in the desired frequency. This way, userspace has control over the update frequency (for example, if the SDL window is minimized or VNC is disconnected, we don't need to poll).

I think we can even do this from the I/O thread, without stopping a vcpu, since the colaesced mmio page is not tied to a vcpu but is a vm property.

#if defined(KVM_CAP_COALESCED_MMIO)
   if (kvm_state->coalesced_mmio) {
       struct kvm_coalesced_mmio_ring *ring =
           (void *) run + kvm_state->coalesced_mmio * PAGE_SIZE;
       while (ring->first != ring->last) {
           cpu_physical_memory_rw(ring->coalesced_mmio[ring->first].phys_addr,
                          &ring->coalesced_mmio[ring->first].data[0],
                          ring->coalesced_mmio[ring->first].len, 1);
           smp_wmb();
           ring->first = (ring->first + 1) % KVM_COALESCED_MMIO_MAX;
       }
   }

#endif

Quoted 5 lines

I think we can even do this from the I/O thread, without stopping a vcpu, since the colaesced mmio page is not tied to a vcpu but is a vm property.

This one sounds better. But I've taken a look at the current userspace code:

Quoted 12 lines

#if defined(KVM_CAP_COALESCED_MMIO)
    if (kvm_state->coalesced_mmio) {
        struct kvm_coalesced_mmio_ring *ring =
            (void *) run + kvm_state->coalesced_mmio * PAGE_SIZE;
        while (ring->first != ring->last) {
            cpu_physical_memory_rw(ring->coalesced_mmio[ring->first].phys_addr,
                           &ring->coalesced_mmio[ring->first].data[0],
                           ring->coalesced_mmio[ring->first].len, 1);
            smp_wmb();
            ring->first = (ring->first + 1) % KVM_COALESCED_MMIO_MAX;
        }
    }

Quoted 2 lines

#endif

No protection for ring->first and ring->last? Seems it can writing the same element pointed by ring->first twice, then skip one element at (ring->first + 1)...

Quoted 5 lines · Sheng Yang

Quoted 3 lines

I think we can even do this from the I/O thread, without stopping a vcpu, since the colaesced mmio page is not tied to a vcpu but is a vm property.

This one sounds better. But I've taken a look at the current userspace

Quoted 14 lines

Quoted 13 lines

#if defined(KVM_CAP_COALESCED_MMIO)
    if (kvm_state->coalesced_mmio) {
        struct kvm_coalesced_mmio_ring *ring =
            (void *) run + kvm_state->coalesced_mmio * PAGE_SIZE;
        while (ring->first != ring->last) {

cpu_physical_memory_rw(ring->coalesced_mmio[ring->first].phys_addr, &ring->coalesced_mmio[ring->first].data[0],

ring->coalesced_mmio[ring->first].len, 1); smp_wmb();
            ring->first = (ring->first + 1) % KVM_COALESCED_MMIO_MAX;
        }
    }

#endif

Quoted 3 lines

No protection for ring->first and ring->last? Seems it can writing the same element pointed by ring->first twice, then skip one element at (ring->first + 1)...

ring->first is owned by userspace, while ring->last is owned by the kernel, so no protection is necessary except for the memory barrier. Can you elaborate on how it would fail?

Quoted 5 lines

Quoted 5 lines

No protection for ring->first and ring->last? Seems it can writing the same element pointed by ring->first twice, then skip one element at (ring->first + 1)...

Quoted 5 lines

ring->first is owned by userspace, while ring->last is owned by the kernel, so no protection is necessary except for the memory barrier. Can you elaborate on how it would fail?

This piece of code can only be executed on one thread/vcpu at same time? I think different vcpus accessing/modifying ring->first at the same time would cause problem.

But for a separate iothread which handle all userspace accessing, it should be fine.