kvm: Flush coalesced MMIO buffer periodly

PATCH In review Sheng Yang 2 participants 5 messages Jan 21, 2010 – Jan 25, 2010 KVM

Share & tools

Sheng Yang

author

2010-01-21 09:37 UTC #0 lore raw

PATCH 1/1 kvm: Flush coalesced MMIO buffer periodly

The default action of coalesced 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.

Per Avi's suggestion, I add a periodly flushing coalesced MMIO buffer in QEmu IO thread. By this way, We don't need vcpu explicit exit to QEmu to handle this issue. Current synchronize rate is 1/25s.

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

---
 qemu-kvm.c |   47 +++++++++++++++++++++++++++++++++++++++++++++--
 qemu-kvm.h |    2 ++
 2 files changed, 47 insertions(+), 2 deletions(-)

qemu-kvm.c +45 -2

index 599c3d6..38f890c 100644
@@ -463,6 +463,12 @@ static void kvm_create_vcpu(CPUState *env, int id)
         goto err_fd;
     }
 
+#ifdef KVM_CAP_COALESCED_MMIO
+    if (kvm_state->coalesced_mmio && !kvm_state->coalesced_mmio_ring)
+        kvm_state->coalesced_mmio_ring = (void *) env->kvm_run +
+		kvm_state->coalesced_mmio * PAGE_SIZE;
+#endif
+
     return;
   err_fd:
     close(env->kvm_fd);

@@ -927,8 +933,7 @@ int kvm_run(CPUState *env)
 
 #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;
+        struct kvm_coalesced_mmio_ring *ring = kvm_state->coalesced_mmio_ring;
         while (ring->first != ring->last) {
             cpu_physical_memory_rw(ring->coalesced_mmio[ring->first].phys_addr,
                            &ring->coalesced_mmio[ring->first].data[0],

@@ -2073,6 +2078,29 @@ static void io_thread_wakeup(void *opaque)
     }
 }
 
+#ifdef KVM_CAP_COALESCED_MMIO
+
+/* flush interval is 1/25 second */
+#define KVM_COALESCED_MMIO_FLUSH_INTERVAL    40000000LL
+
+static void flush_coalesced_mmio_buffer(void *opaque)
+{
+    if (kvm_state->coalesced_mmio_ring) {
+        struct kvm_coalesced_mmio_ring *ring =
+            kvm_state->coalesced_mmio_ring;
+        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;
+        }
+    }
+    qemu_mod_timer(kvm_state->coalesced_mmio_timer,
+           qemu_get_clock(host_clock) + KVM_COALESCED_MMIO_FLUSH_INTERVAL);
+}
+#endif
+
 int kvm_main_loop(void)
 {
     int fds[2];

@@ -2117,6 +2145,15 @@ int kvm_main_loop(void)
     io_thread_sigfd = sigfd;
     cpu_single_env = NULL;
 
+#ifdef KVM_CAP_COALESCED_MMIO
+    if (kvm_state->coalesced_mmio) {
+        kvm_state->coalesced_mmio_timer =
+            qemu_new_timer(host_clock, flush_coalesced_mmio_buffer, NULL);
+        qemu_mod_timer(kvm_state->coalesced_mmio_timer,
+            qemu_get_clock(host_clock) + KVM_COALESCED_MMIO_FLUSH_INTERVAL);
+    }
+#endif
+
     while (1) {
         main_loop_wait(1000);
         if (qemu_shutdown_requested()) {

@@ -2135,6 +2172,12 @@ int kvm_main_loop(void)
         }
     }
 
+#ifdef KVM_CAP_COALESCED_MMIO
+    if (kvm_state->coalesced_mmio) {
+        qemu_del_timer(kvm_state->coalesced_mmio_timer);
+        qemu_free_timer(kvm_state->coalesced_mmio_timer);
+    }
+#endif
     pause_all_threads();
     pthread_mutex_unlock(&qemu_mutex);

qemu-kvm.h +2 -0

index 6b3e5a1..17f9d1b 100644
@@ -1144,6 +1144,8 @@ typedef struct KVMState {
     int fd;
     int vmfd;
     int coalesced_mmio;
+    struct kvm_coalesced_mmio_ring *coalesced_mmio_ring;
+    struct QEMUTimer *coalesced_mmio_timer;
     int broken_set_mem_region;
     int migration_log;
     int vcpu_events;

Signature · 1 line

1.5.4.5

Avi Kivity

2010-01-21 10:19 UTC #1 lore raw

Quoted 9 lines · Sheng Yang

The default action of coalesced 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.

Quoted 9 lines

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.
Per Avi's suggestion, I add a periodly flushing coalesced MMIO buffer in QEmu IO thread. By this way, We don't need vcpu explicit exit to QEmu to handle this issue. Current synchronize rate is 1/25s.

I'm not sure that a new timer is needed. If the only problem case is the display, maybe we can flush coalesced mmio from the vga refresh timer. That ensures that we flash exactly when needed, and don't have extra timers.

Signature · 1 line

error compiling committee.c: too many arguments to function

Sheng Yang

author

2010-01-22 02:23 UTC #2 lore raw

PATCH 1/1 kvm: Flush coalesced MMIO buffer periodly

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

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.

Per Avi's suggestion, I hooked a flushing for coalesced MMIO buffer in VGA update handler. By this way, We don't need vcpu explicit exit to QEmu to handle this issue.

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

---

Like this?

 qemu-kvm.c |   26 ++++++++++++++++++++++++--
 qemu-kvm.h |    6 ++++++
 vl.c       |    2 ++
 3 files changed, 32 insertions(+), 2 deletions(-)

qemu-kvm.c +24 -2

index 599c3d6..a9b5107 100644
@@ -463,6 +463,12 @@ static void kvm_create_vcpu(CPUState *env, int id)
         goto err_fd;
     }
 
+#ifdef KVM_CAP_COALESCED_MMIO
+    if (kvm_state->coalesced_mmio && !kvm_state->coalesced_mmio_ring)
+        kvm_state->coalesced_mmio_ring = (void *) env->kvm_run +
+		kvm_state->coalesced_mmio * PAGE_SIZE;
+#endif
+
     return;
   err_fd:
     close(env->kvm_fd);

@@ -927,8 +933,7 @@ int kvm_run(CPUState *env)
 
 #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;
+        struct kvm_coalesced_mmio_ring *ring = kvm_state->coalesced_mmio_ring;
         while (ring->first != ring->last) {
             cpu_physical_memory_rw(ring->coalesced_mmio[ring->first].phys_addr,
                            &ring->coalesced_mmio[ring->first].data[0],

@@ -2073,6 +2078,23 @@ static void io_thread_wakeup(void *opaque)
     }
 }
 
+#ifdef KVM_CAP_COALESCED_MMIO
+void kvm_flush_coalesced_mmio_buffer(void)
+{
+    if (kvm_state->coalesced_mmio_ring) {
+        struct kvm_coalesced_mmio_ring *ring =
+            kvm_state->coalesced_mmio_ring;
+        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
+
 int kvm_main_loop(void)
 {
     int fds[2];

qemu-kvm.h +6 -0

index 6b3e5a1..8188ff6 100644
@@ -1125,6 +1125,11 @@ static inline int kvm_set_migration_log(int enable)
     return kvm_physical_memory_set_dirty_tracking(enable);
 }
 
+#ifdef KVM_CAP_COALESCED_MMIO
+void kvm_flush_coalesced_mmio_buffer(void);
+#else
+void kvm_flush_coalesced_mmio_buffer(void) {}
+#endif
 
 int kvm_irqchip_in_kernel(void);
 #ifdef CONFIG_KVM

@@ -1144,6 +1149,7 @@ typedef struct KVMState {
     int fd;
     int vmfd;
     int coalesced_mmio;
+    struct kvm_coalesced_mmio_ring *coalesced_mmio_ring;
     int broken_set_mem_region;
     int migration_log;
     int vcpu_events;

vl.c +2 -0

index 9edea10..64902f2 100644
@@ -3235,6 +3235,7 @@ static void gui_update(void *opaque)
             interval = dcl->gui_timer_interval;
         dcl = dcl->next;
     }
+    kvm_flush_coalesced_mmio_buffer();
     qemu_mod_timer(ds->gui_timer, interval + qemu_get_clock(rt_clock));
 }

@@ -3242,6 +3243,7 @@ static void nographic_update(void *opaque)
 {
     uint64_t interval = GUI_REFRESH_INTERVAL;
 
+    kvm_flush_coalesced_mmio_buffer();
     qemu_mod_timer(nographic_timer, interval + qemu_get_clock(rt_clock));
 }

Signature · 1 line

1.5.4.5

Avi Kivity

2010-01-24 07:36 UTC #3 lore raw

Quoted 37 lines · Sheng Yang

The default action of coalesced 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.
Per Avi's suggestion, I hooked a flushing for coalesced MMIO buffer in VGA update handler. By this way, We don't need vcpu explicit exit to QEmu to handle this issue.
Signed-off-by Sheng Yang<[email protected]>
---
Like this?
  qemu-kvm.c |   26 ++++++++++++++++++++++++--
  qemu-kvm.h |    6 ++++++
  vl.c       |    2 ++
  3 files changed, 32 insertions(+), 2 deletions(-)
qemu-kvm.c +7 -0
+#ifdef KVM_CAP_COALESCED_MMIO
+void kvm_flush_coalesced_mmio_buffer(void)
+{
+    if (kvm_state->coalesced_mmio_ring) {
+        struct kvm_coalesced_mmio_ring *ring =
+            kvm_state->coalesced_mmio_ring;
+        while (ring->first != ring->last) {

qemu-kvm.c static void io_thread_wakeup(void *opaque) · line 2097

Quoted 10 lines

qemu-kvm.c +10 -0

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

qemu has a coalesced mmio api, (qemu_register_colaesced_mmio), so please follow it (stubs in exec.c, and implementation in kvm-specific files). Please send the patch against the uq/master branch (which follow upstream).

vl.c static void gui_update(void *opaque) · line 3241

Quoted 12 lines

vl.c +1 -0

index 9edea10..64902f2 100644
@@ -3235,6 +3235,7 @@ static void gui_update(void *opaque)
              interval = dcl->gui_timer_interval;
          dcl = dcl->next;
      }
+    kvm_flush_coalesced_mmio_buffer();
      qemu_mod_timer(ds->gui_timer, interval + qemu_get_clock(rt_clock));
  }

Better to do that before the call to dpy_refresh().

vl.c static void nographic_update(void *opaque) · line 3249

Quoted 9 lines

vl.c +1 -0

@@ -3242,6 +3243,7 @@ static void nographic_update(void *opaque)
  {
      uint64_t interval = GUI_REFRESH_INTERVAL;
 
+    kvm_flush_coalesced_mmio_buffer();
      qemu_mod_timer(nographic_timer, interval + qemu_get_clock(rt_clock));
  }

Any need to do it here?

(why does nographic_update use a timer?)

Signature · 1 line

Do not meddle in the internals of kernels, for they are subtle and quick to panic.

Sheng Yang

author

2010-01-25 07:45 UTC #4 lore raw

Quoted 44 lines · On Sunday 24 January 2010 15:35:58 Avi Kivity wrote:

Quoted 43 lines · Sheng Yang
The default action of coalesced 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.
Per Avi's suggestion, I hooked a flushing for coalesced MMIO buffer in VGA update handler. By this way, We don't need vcpu explicit exit to QEmu to handle this issue.
Signed-off-by Sheng Yang<[email protected]>
---
Like this?
  qemu-kvm.c |   26 ++++++++++++++++++++++++--
  qemu-kvm.h |    6 ++++++
  vl.c       |    2 ++
  3 files changed, 32 insertions(+), 2 deletions(-)
qemu-kvm.c +8 -0
+#ifdef KVM_CAP_COALESCED_MMIO
+void kvm_flush_coalesced_mmio_buffer(void)
+{
+    if (kvm_state->coalesced_mmio_ring) {
+        struct kvm_coalesced_mmio_ring *ring =
+            kvm_state->coalesced_mmio_ring;
+        while (ring->first != ring->last) {
+           
cpu_physical_memory_rw(ring->coalesced_mmio[ring->first].phys_addr, +&ring->coalesced_mmio[ring->first].data[0],
qemu-kvm.c +4 -0
+                           ring->coalesced_mmio[ring->first].len, 1);
+            smp_wmb();
+            ring->first = (ring->first + 1) % KVM_COALESCED_MMIO_MAX;
+        }

Quoted 8 lines

qemu-kvm.c static void io_thread_wakeup(void *opaque) · line 2097
Quoted 4 lines
qemu-kvm.c +4 -0
+    }
+}
+#endif
+
qemu has a coalesced mmio api, (qemu_register_colaesced_mmio), so please follow it (stubs in exec.c, and implementation in kvm-specific files). Please send the patch against the uq/master branch (which follow upstream).

You means put it in the kvm-all.c?(see the latest patch) Um, seems I have to import "libkvm.h" in with a #ifndef KVM_UPSTREAM, due to smp_wmb() and PAGE_SIZE macro. I am not sure about which is the proper way to get it work... (the relationship between qemu-kvm and upstream qemu's kvm often confuse me. I supposed kvm-all.c should belong to upstream kvm, and qemu-kvm.* and libkvm should belong to qemu-kvm?)

Another issue is, upstream QEmu compile error with "TARGET_PHYS_ADDR_BITS redefined". I would like to wait for that to be fixed.

Quoted 14 lines

vl.c static void gui_update(void *opaque) · line 3240

Quoted 12 lines

vl.c +1 -0

index 9edea10..64902f2 100644
@@ -3235,6 +3235,7 @@ static void gui_update(void *opaque)
              interval = dcl->gui_timer_interval;
          dcl = dcl->next;
      }
+    kvm_flush_coalesced_mmio_buffer();
      qemu_mod_timer(ds->gui_timer, interval + qemu_get_clock(rt_clock));
  }

Better to do that before the call to dpy_refresh().

OK.

Quoted 5 lines

vl.c +0 -0

@@ -3242,6 +3243,7 @@ static void nographic_update(void *opaque)
  {
      uint64_t interval = GUI_REFRESH_INTERVAL;

Quoted 8 lines

vl.c static void nographic_update(void *opaque) · line 3248
Quoted 3 lines
vl.c +1 -0
+    kvm_flush_coalesced_mmio_buffer();
      qemu_mod_timer(nographic_timer, interval +
 qemu_get_clock(rt_clock)); }
Any need to do it here?
(why does nographic_update use a timer?)

VNC would need nographic_update().

Signature · 2 lines

regards
Yang, Sheng