[PATCH 0/7] kvm-userspace: Batch writes to MMIO

[PATCH 0/7] kvm-userspace: Batch writes to MMIO

[Laurent Vivier]

When kernel has to send MMIO writes to userspace, it stores them
in memory until it has to pass the hand to userspace for another
reason. This avoids to have too many context switches on operations
that can wait.

			WITHOUT			WITH
			PATCH			PATCH

iperf (e1000)		169 MB/s		185,5 MB/s	+9,7%
   host_state_reload	(626594)		(391825)	-37%

	[9,7% is a more realistic value than my previous benchmark]

boot XP
host_state_reload	764677			516059		-32%

VGA text scroll	
host_state_reload	13280568 (6:15)		3608362 (4:42)	-73% (-25%)

This is the userspace part of the MMIO batching functionality.

[PATCH 1/7] libkvm: delayed MMIO support (common part)

	This patch introduces in libkvm the common part of MMIO batching 
	interface.

[PATCH 2/7] libkvm: delayed MMIO support (x86 part)

	This patch enables MMIO batching for x86 architecture.

[PATCH 3/7] libkvm: delayed MMIO support (powerpc part)

	This patch enables MMIO batching for powerpc architecture.
	WARNING: this has not been tested.

[PATCH 4/7] libkvm: delayed MMIO support (ia64 part)

	This patch enables MMIO batching for ia64 architecture.
	WARNING: this has not been tested.

[PATCH 5/7] qemu: delayed MMIO support (core)

	This patch introduces in qemu the MMIO batching.

[PATCH 6/7] qemu: delayed MMIO support (VGA)

	This patch defines delayed MMIO zones for VGA cards.

[PATCH 7/7] qemu: delayed MMIO support (e1000)

	This patch defines delayed MMIO zones for e1000 ethernet card.

Signed-off-by: Laurent Vivier <Laurent.Vivier@bull.net>

[PATCH 1/7] libkvm: delayed MMIO support (common part)

[Laurent Vivier]

This patch introduces in libkvm the common part of MMIO batching interface.

It checks the MMIO batching availability with ioctl(KVM_CHECK_EXTENSION).
If KVM_CAP_DELAYED_MMIO is available, it processes the MMIO ring buffer
at the return of ioctl(KVM_RUN).
It defines kvm_register_delayed_mmio() to register a delayed MMIO zone, and 
kvm_unregister_delayed_mmio() to unregister it.

Signed-off-by: Laurent Vivier <Laurent.Vivier@bull.net>
---
 libkvm/kvm-common.h |    2 +
 libkvm/libkvm.c     |   73 +++++++++++++++++++++++++++++++++++++++++++++++++++
 libkvm/libkvm.h     |    2 +
 3 files changed, 77 insertions(+), 0 deletions(-)

diff --git a/libkvm/kvm-common.h b/libkvm/kvm-common.h
index a3549e2..f975d56 100644
--- a/libkvm/kvm-common.h
+++ b/libkvm/kvm-common.h
@@ -49,6 +49,8 @@ struct kvm_context {
 	int no_pit_creation;
 	/// in-kernel pit status
 	int pit_in_kernel;
+	/// in-kernel delayed mmio
+	int delayed_mmio;
 };
 
 void init_slots(void);
diff --git a/libkvm/libkvm.c b/libkvm/libkvm.c
index d1e95a4..718f4c6 100644
--- a/libkvm/libkvm.c
+++ b/libkvm/libkvm.c
@@ -879,6 +879,20 @@ again:
 
 	post_kvm_run(kvm, vcpu);
 
+#if defined(KVM_CAP_DELAYED_MMIO)
+	if (kvm->delayed_mmio) {
+	        struct kvm_batch *batch = (void *)run +
+					  kvm->delayed_mmio * PAGE_SIZE;
+		while (batch->first != batch->last) {
+			kvm->callbacks->mmio_write(kvm->opaque,
+					    batch->mmio[batch->first].phys_addr,
+					   &batch->mmio[batch->first].data[0],
+					    batch->mmio[batch->first].len);
+			batch->first = (batch->first + 1) % KVM_MAX_BATCH;
+		}
+	}
+#endif
+
 	if (r == -1) {
 		r = handle_io_window(kvm);
 		goto more;
@@ -983,3 +997,62 @@ int kvm_pit_in_kernel(kvm_context_t kvm)
 {
 	return kvm->pit_in_kernel;
 }
+
+int kvm_init_delayed_mmio(kvm_context_t kvm)
+{
+	int r = 0;
+	kvm->delayed_mmio = 0;
+#ifdef KVM_CAP_DELAYED_MMIO
+	r = ioctl(kvm->fd, KVM_CHECK_EXTENSION, KVM_CAP_DELAYED_MMIO);
+	if (r > 0) {
+		kvm->delayed_mmio = r;
+		return 0;
+	}
+#endif
+	return r;
+}
+
+int kvm_register_delayed_mmio(kvm_context_t kvm, uint64_t addr, uint32_t size)
+{
+#ifdef KVM_CAP_DELAYED_MMIO
+	struct kvm_delayed_mmio_zone zone;
+	int r;
+
+	if (kvm->delayed_mmio) {
+
+		zone.addr = addr;
+		zone.size = size;
+
+		r = ioctl(kvm->vm_fd, KVM_REGISTER_DELAYED_MMIO, &zone);
+		if (r == -1) {
+			perror("kvm_register_delayed_mmio_zone");
+			return -errno;
+		}
+		return 0;
+	}
+#endif
+	return -1;
+}
+
+int kvm_unregister_delayed_mmio(kvm_context_t kvm, uint64_t addr, uint32_t size)
+{
+#ifdef KVM_CAP_DELAYED_MMIO
+	struct kvm_delayed_mmio_zone zone;
+	int r;
+
+	if (kvm->delayed_mmio) {
+
+		zone.addr = addr;
+		zone.size = size;
+
+		r = ioctl(kvm->vm_fd, KVM_UNREGISTER_DELAYED_MMIO, &zone);
+		if (r == -1) {
+			perror("kvm_unregister_delayed_mmio_zone");
+			return -errno;
+		}
+		return 0;
+	}
+#endif
+	return -1;
+}
+
diff --git a/libkvm/libkvm.h b/libkvm/libkvm.h
index 63183b8..dfb615a 100644
--- a/libkvm/libkvm.h
+++ b/libkvm/libkvm.h
@@ -457,6 +457,8 @@ int kvm_get_dirty_pages_range(kvm_context_t kvm, unsigned long phys_addr,
 			      unsigned long end_addr, void *buf, void*opaque,
 			      int (*cb)(unsigned long start, unsigned long len,
 					void*bitmap, void *opaque));
+int kvm_register_delayed_mmio(kvm_context_t kvm, uint64_t addr, uint32_t size);
+int kvm_unregister_delayed_mmio(kvm_context_t kvm, uint64_t addr, uint32_t size);
 
 /*!
  * \brief Create a memory alias
-- 
1.5.2.4

[PATCH 2/7] libkvm: delayed MMIO support (x86 part)

[Laurent Vivier]

This patch enables MMIO batching for x86 architecture.

Signed-off-by: Laurent Vivier <Laurent.Vivier@bull.net>
---
 libkvm/libkvm-x86.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/libkvm/libkvm-x86.c b/libkvm/libkvm-x86.c
index d46fdcc..6762b91 100644
--- a/libkvm/libkvm-x86.c
+++ b/libkvm/libkvm-x86.c
@@ -179,6 +179,10 @@ int kvm_arch_create(kvm_context_t kvm, unsigned long phys_mem_bytes,
 	if (r < 0)
 		return r;
 
+	r = kvm_init_delayed_mmio(kvm);
+	if (r < 0)
+		return r;
+
 	return 0;
 }
 
-- 
1.5.2.4

[PATCH 3/7] libkvm: delayed MMIO support (powerpc part)

[Laurent Vivier]

This patch enables MMIO batching for powerpc architecture.

WARNING: this has not been tested.

Signed-off-by: Laurent Vivier <Laurent.Vivier@bull.net>
---
 libkvm/libkvm-powerpc.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/libkvm/libkvm-powerpc.c b/libkvm/libkvm-powerpc.c
index 53a2478..0f342d7 100644
--- a/libkvm/libkvm-powerpc.c
+++ b/libkvm/libkvm-powerpc.c
@@ -93,6 +93,12 @@ void kvm_show_regs(kvm_context_t kvm, int vcpu)
 int kvm_arch_create(kvm_context_t kvm, unsigned long phys_mem_bytes,
 			 void **vm_mem)
 {
+	int r;
+
+	r = kvm_init_delayed_mmio(kvm);
+	if (r < 0)
+		return r;
+
 	return 0;
 }
 
-- 
1.5.2.4

[PATCH 4/7] libkvm: delayed MMIO support (ia64 part)

[Laurent Vivier]

This patch enables MMIO batching for ia64 architecture.

WARNING: this has not been tested.

Signed-off-by: Laurent Vivier <Laurent.Vivier@bull.net>
---
 libkvm/libkvm-ia64.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/libkvm/libkvm-ia64.c b/libkvm/libkvm-ia64.c
index d7477d0..b9ea600 100644
--- a/libkvm/libkvm-ia64.c
+++ b/libkvm/libkvm-ia64.c
@@ -67,6 +67,12 @@ void *kvm_create_kernel_phys_mem(kvm_context_t kvm, unsigned long phys_start,
 int kvm_arch_create(kvm_context_t kvm, unsigned long phys_mem_bytes,
 			void **vm_mem)
 {
+	int r;
+
+	r = kvm_init_delayed_mmio(kvm);
+	if (r < 0)
+		return r;
+
 	return 0;
 }
 
-- 
1.5.2.4

[PATCH 5/7] qemu: delayed MMIO support (core)

[Laurent Vivier]

This patch introduces in qemu the MMIO batching.

It defines qemu_kvm_register_delayed_mmio() and
qemu_kvm_unregister_delayed_mmio() to register and unregister delayed MMIO
zone.
It calls qemu_kvm_unregister_delayed_mmio() on pci_update_mappings().

Signed-off-by: Laurent Vivier <Laurent.Vivier@bull.net>
---
 qemu/hw/pci.c   |    4 ++++
 qemu/qemu-kvm.c |   10 ++++++++++
 qemu/qemu-kvm.h |    2 ++
 3 files changed, 16 insertions(+), 0 deletions(-)

diff --git a/qemu/hw/pci.c b/qemu/hw/pci.c
index a23a466..70809fb 100644
--- a/qemu/hw/pci.c
+++ b/qemu/hw/pci.c
@@ -26,6 +26,7 @@
 #include "console.h"
 #include "net.h"
 #include "pc.h"
+#include "qemu-kvm.h"
 
 //#define DEBUG_PCI
 
@@ -323,6 +324,9 @@ static void pci_update_mappings(PCIDevice *d)
                         cpu_register_physical_memory(pci_to_cpu_addr(r->addr),
                                                      r->size,
                                                      IO_MEM_UNASSIGNED);
+                        if (kvm_enabled())
+                                qemu_kvm_unregister_delayed_mmio(r->addr,
+                                                                 r->size);
                     }
                 }
                 r->addr = new_addr;
diff --git a/qemu/qemu-kvm.c b/qemu/qemu-kvm.c
index 02776bc..484a713 100644
--- a/qemu/qemu-kvm.c
+++ b/qemu/qemu-kvm.c
@@ -928,3 +928,13 @@ void kvm_mutex_lock(void)
     pthread_mutex_lock(&qemu_mutex);
     cpu_single_env = NULL;
 }
+
+int qemu_kvm_register_delayed_mmio(target_phys_addr_t addr, unsigned int size)
+{
+    return kvm_register_delayed_mmio(kvm_context, addr, size);
+}
+
+int qemu_kvm_unregister_delayed_mmio(target_phys_addr_t addr, unsigned int size)
+{
+    return kvm_unregister_delayed_mmio(kvm_context, addr, size);
+}
diff --git a/qemu/qemu-kvm.h b/qemu/qemu-kvm.h
index 7412e20..a69142c 100644
--- a/qemu/qemu-kvm.h
+++ b/qemu/qemu-kvm.h
@@ -76,6 +76,8 @@ int handle_tpr_access(void *opaque, int vcpu,
 void kvm_tpr_vcpu_start(CPUState *env);
 
 int qemu_kvm_get_dirty_pages(unsigned long phys_addr, void *buf);
+int qemu_kvm_register_delayed_mmio(target_phys_addr_t addr, unsigned int size);
+int qemu_kvm_unregister_delayed_mmio(target_phys_addr_t addr, unsigned int size);
 
 void qemu_kvm_system_reset_request(void);
 
-- 
1.5.2.4

[PATCH 6/7] qemu: delayed MMIO support (VGA)

[Laurent Vivier]

This patch defines delayed MMIO zones for VGA cards.

Signed-off-by: Laurent Vivier <Laurent.Vivier@bull.net>
---
 qemu/hw/cirrus_vga.c |    2 ++
 qemu/hw/vga.c        |    4 ++++
 2 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/qemu/hw/cirrus_vga.c b/qemu/hw/cirrus_vga.c
index 2c4aeec..9eb6929 100644
--- a/qemu/hw/cirrus_vga.c
+++ b/qemu/hw/cirrus_vga.c
@@ -3291,6 +3291,8 @@ static void cirrus_init_common(CirrusVGAState * s, int device_id, int is_pci)
                                            cirrus_vga_mem_write, s);
     cpu_register_physical_memory(isa_mem_base + 0x000a0000, 0x20000,
                                  vga_io_memory);
+    if (kvm_enabled())
+        qemu_kvm_register_delayed_mmio(isa_mem_base + 0x000a0000, 0x20000);
 
     s->sr[0x06] = 0x0f;
     if (device_id == CIRRUS_ID_CLGD5446) {
diff --git a/qemu/hw/vga.c b/qemu/hw/vga.c
index 3a49573..69afbd9 100644
--- a/qemu/hw/vga.c
+++ b/qemu/hw/vga.c
@@ -2257,6 +2257,8 @@ void vga_init(VGAState *s)
     vga_io_memory = cpu_register_io_memory(0, vga_mem_read, vga_mem_write, s);
     cpu_register_physical_memory(isa_mem_base + 0x000a0000, 0x20000,
                                  vga_io_memory);
+    if (kvm_enabled())
+    	qemu_kvm_register_delayed_mmio(isa_mem_base + 0x000a0000, 0x20000);
 }
 
 /* Memory mapped interface */
@@ -2332,6 +2334,8 @@ static void vga_mm_init(VGAState *s, target_phys_addr_t vram_base,
     cpu_register_physical_memory(ctrl_base, 0x100000, s_ioport_ctrl);
     s->bank_offset = 0;
     cpu_register_physical_memory(vram_base + 0x000a0000, 0x20000, vga_io_memory);
+    if (kvm_enabled())
+        qemu_kvm_register_delayed_mmio(vram_base + 0x000a0000, 0x20000);
 }
 
 int isa_vga_init(DisplayState *ds, uint8_t *vga_ram_base,
-- 
1.5.2.4

[PATCH 7/7] qemu: delayed MMIO support (e1000)

[Laurent Vivier]

This patch defines delayed MMIO zones for e1000 ethernet card.

Signed-off-by: Laurent Vivier <Laurent.Vivier@bull.net>
---
 qemu/hw/e1000.c |   17 +++++++++++++++++
 1 files changed, 17 insertions(+), 0 deletions(-)

diff --git a/qemu/hw/e1000.c b/qemu/hw/e1000.c
index 01f8983..97f46f6 100644
--- a/qemu/hw/e1000.c
+++ b/qemu/hw/e1000.c
@@ -26,6 +26,7 @@
 #include "hw.h"
 #include "pci.h"
 #include "net.h"
+#include "qemu-kvm.h"
 
 #include "e1000_hw.h"
 
@@ -938,6 +939,22 @@ e1000_mmio_map(PCIDevice *pci_dev, int region_num,
 
     d->mmio_base = addr;
     cpu_register_physical_memory(addr, PNPMMIO_SIZE, d->mmio_index);
+
+    if (kvm_enabled()) {
+        /* delayed zones are:
+         * 0x00000 -> 0x000C0        exclude: E1000_ICR,4
+         * 0x000C4 -> 0x00400        exclude: E1000_TCTL,4
+         * 0x00404 -> 0x03818        exclude: E1000_TDT,4
+         * 0x0381C -> 0x20000        PNPMMIO_SIZE
+         */
+        qemu_kvm_register_delayed_mmio(addr, E1000_ICR);
+        qemu_kvm_register_delayed_mmio(addr + E1000_ICR + 4,
+                                       E1000_TCTL - (E1000_ICR + 4));
+        qemu_kvm_register_delayed_mmio(addr + E1000_TCTL + 4,
+                                       E1000_TDT - (E1000_TCTL + 4));
+        qemu_kvm_register_delayed_mmio(addr + E1000_TDT + 4,
+                                       PNPMMIO_SIZE - (E1000_TDT + 4));
+    }
 }
 
 static int
-- 
1.5.2.4

Re: [PATCH 1/7] libkvm: delayed MMIO support (common part)

[Avi Kivity]

Laurent Vivier wrote:
> This patch introduces in libkvm the common part of MMIO batching interface.
>
> It checks the MMIO batching availability with ioctl(KVM_CHECK_EXTENSION).
> If KVM_CAP_DELAYED_MMIO is available, it processes the MMIO ring buffer
> at the return of ioctl(KVM_RUN).
> It defines kvm_register_delayed_mmio() to register a delayed MMIO zone, and 
> kvm_unregister_delayed_mmio() to unregister it.
>
>  
>  void init_slots(void);
> diff --git a/libkvm/libkvm.c b/libkvm/libkvm.c
> index d1e95a4..718f4c6 100644
> --- a/libkvm/libkvm.c
> +++ b/libkvm/libkvm.c
> @@ -879,6 +879,20 @@ again:
>  
>  	post_kvm_run(kvm, vcpu);
>  
> +#if defined(KVM_CAP_DELAYED_MMIO)
> +	if (kvm->delayed_mmio) {
> +	        struct kvm_batch *batch = (void *)run +
> +					  kvm->delayed_mmio * PAGE_SIZE;
> +		while (batch->first != batch->last) {
> +			kvm->callbacks->mmio_write(kvm->opaque,
> +					    batch->mmio[batch->first].phys_addr,
> +					   &batch->mmio[batch->first].data[0],
> +					    batch->mmio[batch->first].len);
> +			batch->first = (batch->first + 1) % KVM_MAX_BATCH;
>   

There needs to be a write barrier after this (smp_wmb()), to avoid 
compiler and cpu reordering.  Especially important for non-x86.  Also 
need a read barrier before reading batch->first.

Or maybe not, likely the pthread_mutex_lock() acts as the correct 
barrier.  But we do need the equivalent barriers in the kernel code 
(missed it on that review).

> +		}
> +	}
> +#endif
> +
>  	if (r == -1) {
>  		r = handle_io_window(kvm);
>  		goto more;
>   

> +
> +int kvm_register_delayed_mmio(kvm_context_t kvm, uint64_t addr, uint32_t size)
> +{
> +#ifdef KVM_CAP_DELAYED_MMIO
> +	struct kvm_delayed_mmio_zone zone;
> +	int r;
> +
> +	if (kvm->delayed_mmio) {
> +
> +		zone.addr = addr;
> +		zone.size = size;
> +
> +		r = ioctl(kvm->vm_fd, KVM_REGISTER_DELAYED_MMIO, &zone);
> +		if (r == -1) {
> +			perror("kvm_register_delayed_mmio_zone");
> +			return -errno;
> +		}
> +		return 0;
> +	}
> +#endif
> +	return -1;
>   

-ENOSYS, or something.  -1 is -ENOPERM.

> +}
> +
> +int kvm_unregister_delayed_mmio(kvm_context_t kvm, uint64_t addr, uint32_t size)
> +{
> +#ifdef KVM_CAP_DELAYED_MMIO
> +	struct kvm_delayed_mmio_zone zone;
> +	int r;
> +
> +	if (kvm->delayed_mmio) {
> +
> +		zone.addr = addr;
> +		zone.size = size;
> +
> +		r = ioctl(kvm->vm_fd, KVM_UNREGISTER_DELAYED_MMIO, &zone);
> +		if (r == -1) {
> +			perror("kvm_unregister_delayed_mmio_zone");
> +			return -errno;
> +		}
> +		return 0;
> +	}
> +#endif
> +	return -1;
>   

ditto.

-- 
error compiling committee.c: too many arguments to function

Re: [PATCH 6/7] qemu: delayed MMIO support (VGA)

[Avi Kivity]

Laurent Vivier wrote:
> This patch defines delayed MMIO zones for VGA cards.
>
> Signed-off-by: Laurent Vivier <Laurent.Vivier@bull.net>
> ---
>  qemu/hw/cirrus_vga.c |    2 ++
>  qemu/hw/vga.c        |    4 ++++
>  2 files changed, 6 insertions(+), 0 deletions(-)
>
> diff --git a/qemu/hw/cirrus_vga.c b/qemu/hw/cirrus_vga.c
> index 2c4aeec..9eb6929 100644
> --- a/qemu/hw/cirrus_vga.c
> +++ b/qemu/hw/cirrus_vga.c
> @@ -3291,6 +3291,8 @@ static void cirrus_init_common(CirrusVGAState * s, int device_id, int is_pci)
>                                             cirrus_vga_mem_write, s);
>      cpu_register_physical_memory(isa_mem_base + 0x000a0000, 0x20000,
>                                   vga_io_memory);
> +    if (kvm_enabled())
> +        qemu_kvm_register_delayed_mmio(isa_mem_base + 0x000a0000, 0x20000);
>  
>   

Cirrus has some mmio registers that cause actions (bitblt), which could 
be mapped at this address.  But I guess that since all the actions are 
synchronous anyway, no harm is done in delaying them.

-- 
error compiling committee.c: too many arguments to function

Re: [PATCH 7/7] qemu: delayed MMIO support (e1000)

[Avi Kivity]

Laurent Vivier wrote:
> This patch defines delayed MMIO zones for e1000 ethernet card.
>
> Signed-off-by: Laurent Vivier <Laurent.Vivier@bull.net>
> ---
>  qemu/hw/e1000.c |   17 +++++++++++++++++
>  1 files changed, 17 insertions(+), 0 deletions(-)
>
> diff --git a/qemu/hw/e1000.c b/qemu/hw/e1000.c
> index 01f8983..97f46f6 100644
> --- a/qemu/hw/e1000.c
> +++ b/qemu/hw/e1000.c
> @@ -26,6 +26,7 @@
>  #include "hw.h"
>  #include "pci.h"
>  #include "net.h"
> +#include "qemu-kvm.h"
>  
>  #include "e1000_hw.h"
>  
> @@ -938,6 +939,22 @@ e1000_mmio_map(PCIDevice *pci_dev, int region_num,
>  
>      d->mmio_base = addr;
>      cpu_register_physical_memory(addr, PNPMMIO_SIZE, d->mmio_index);
> +
> +    if (kvm_enabled()) {
> +        /* delayed zones are:
> +         * 0x00000 -> 0x000C0        exclude: E1000_ICR,4
> +         * 0x000C4 -> 0x00400        exclude: E1000_TCTL,4
> +         * 0x00404 -> 0x03818        exclude: E1000_TDT,4
> +         * 0x0381C -> 0x20000        PNPMMIO_SIZE
> +         */
> +        qemu_kvm_register_delayed_mmio(addr, E1000_ICR);
> +        qemu_kvm_register_delayed_mmio(addr + E1000_ICR + 4,
> +                                       E1000_TCTL - (E1000_ICR + 4));
> +        qemu_kvm_register_delayed_mmio(addr + E1000_TCTL + 4,
> +                                       E1000_TDT - (E1000_TCTL + 4));
> +        qemu_kvm_register_delayed_mmio(addr + E1000_TDT + 4,
> +                                       PNPMMIO_SIZE - (E1000_TDT + 4));
> +    }
>  }
>  

What about the calls to set_ics()?  They can cause interrupts to be 
deasserted AFAICS.  So IMC (and maybe a few other registers) need to be 
excluded as well.

Instead of hardcoding this, how about a

uint32_t delayed_mmi_excluded_regs[] = {
   E1000_ICR, E1000_TCTL, ...
};

The code can sort them and generate the ranges automatically.

(alternatively, a boolmap:

bool delayed_mmi_excluded_regs[] = {
  [E1000_ICR]: true, ...
};
)

-- 
error compiling committee.c: too many arguments to function

Re: [PATCH 1/7] libkvm: delayed MMIO support (common part)

[Laurent Vivier]

Le lundi 26 mai 2008 à 16:55 +0300, Avi Kivity a écrit :
> Laurent Vivier wrote:
> > This patch introduces in libkvm the common part of MMIO batching interface.
> >
> > It checks the MMIO batching availability with ioctl(KVM_CHECK_EXTENSION).
> > If KVM_CAP_DELAYED_MMIO is available, it processes the MMIO ring buffer
> > at the return of ioctl(KVM_RUN).
> > It defines kvm_register_delayed_mmio() to register a delayed MMIO zone, and 
> > kvm_unregister_delayed_mmio() to unregister it.
> >
> >  
> >  void init_slots(void);
> > diff --git a/libkvm/libkvm.c b/libkvm/libkvm.c
> > index d1e95a4..718f4c6 100644
> > --- a/libkvm/libkvm.c
> > +++ b/libkvm/libkvm.c
> > @@ -879,6 +879,20 @@ again:
> >  
> >  	post_kvm_run(kvm, vcpu);
> >  
> > +#if defined(KVM_CAP_DELAYED_MMIO)
> > +	if (kvm->delayed_mmio) {
> > +	        struct kvm_batch *batch = (void *)run +
> > +					  kvm->delayed_mmio * PAGE_SIZE;
> > +		while (batch->first != batch->last) {
> > +			kvm->callbacks->mmio_write(kvm->opaque,
> > +					    batch->mmio[batch->first].phys_addr,
> > +					   &batch->mmio[batch->first].data[0],
> > +					    batch->mmio[batch->first].len);
> > +			batch->first = (batch->first + 1) % KVM_MAX_BATCH;
> >   
> 
> There needs to be a write barrier after this (smp_wmb()), to avoid 
> compiler and cpu reordering.  Especially important for non-x86.  Also 
> need a read barrier before reading batch->first.
> 
> Or maybe not, likely the pthread_mutex_lock() acts as the correct 
> barrier.  But we do need the equivalent barriers in the kernel code 
> (missed it on that review).

OK

> > +		}
> > +	}
> > +#endif
> > +
> >  	if (r == -1) {
> >  		r = handle_io_window(kvm);
> >  		goto more;
> >   
> 
> > +
> > +int kvm_register_delayed_mmio(kvm_context_t kvm, uint64_t addr, uint32_t size)
> > +{
> > +#ifdef KVM_CAP_DELAYED_MMIO
> > +	struct kvm_delayed_mmio_zone zone;
> > +	int r;
> > +
> > +	if (kvm->delayed_mmio) {
> > +
> > +		zone.addr = addr;
> > +		zone.size = size;
> > +
> > +		r = ioctl(kvm->vm_fd, KVM_REGISTER_DELAYED_MMIO, &zone);
> > +		if (r == -1) {
> > +			perror("kvm_register_delayed_mmio_zone");
> > +			return -errno;
> > +		}
> > +		return 0;
> > +	}
> > +#endif
> > +	return -1;
> >   
> 
> -ENOSYS, or something.  -1 is -ENOPERM.

OK

> 
> > +}
> > +
> > +int kvm_unregister_delayed_mmio(kvm_context_t kvm, uint64_t addr, uint32_t size)
> > +{
> > +#ifdef KVM_CAP_DELAYED_MMIO
> > +	struct kvm_delayed_mmio_zone zone;
> > +	int r;
> > +
> > +	if (kvm->delayed_mmio) {
> > +
> > +		zone.addr = addr;
> > +		zone.size = size;
> > +
> > +		r = ioctl(kvm->vm_fd, KVM_UNREGISTER_DELAYED_MMIO, &zone);
> > +		if (r == -1) {
> > +			perror("kvm_unregister_delayed_mmio_zone");
> > +			return -errno;
> > +		}
> > +		return 0;
> > +	}
> > +#endif
> > +	return -1;
> >   
> 
> ditto.

OK
-- 
------------- Laurent.Vivier@bull.net ---------------
"The best way to predict the future is to invent it."
- Alan Kay

Re: [PATCH 7/7] qemu: delayed MMIO support (e1000)

[Laurent Vivier]

Le lundi 26 mai 2008 à 17:05 +0300, Avi Kivity a écrit :
> Laurent Vivier wrote:
> > This patch defines delayed MMIO zones for e1000 ethernet card.
> >
> > Signed-off-by: Laurent Vivier <Laurent.Vivier@bull.net>
> > ---
> >  qemu/hw/e1000.c |   17 +++++++++++++++++
> >  1 files changed, 17 insertions(+), 0 deletions(-)
> >
> > diff --git a/qemu/hw/e1000.c b/qemu/hw/e1000.c
> > index 01f8983..97f46f6 100644
> > --- a/qemu/hw/e1000.c
> > +++ b/qemu/hw/e1000.c
> > @@ -26,6 +26,7 @@
> >  #include "hw.h"
> >  #include "pci.h"
> >  #include "net.h"
> > +#include "qemu-kvm.h"
> >  
> >  #include "e1000_hw.h"
> >  
> > @@ -938,6 +939,22 @@ e1000_mmio_map(PCIDevice *pci_dev, int region_num,
> >  
> >      d->mmio_base = addr;
> >      cpu_register_physical_memory(addr, PNPMMIO_SIZE, d->mmio_index);
> > +
> > +    if (kvm_enabled()) {
> > +        /* delayed zones are:
> > +         * 0x00000 -> 0x000C0        exclude: E1000_ICR,4
> > +         * 0x000C4 -> 0x00400        exclude: E1000_TCTL,4
> > +         * 0x00404 -> 0x03818        exclude: E1000_TDT,4
> > +         * 0x0381C -> 0x20000        PNPMMIO_SIZE
> > +         */
> > +        qemu_kvm_register_delayed_mmio(addr, E1000_ICR);
> > +        qemu_kvm_register_delayed_mmio(addr + E1000_ICR + 4,
> > +                                       E1000_TCTL - (E1000_ICR + 4));
> > +        qemu_kvm_register_delayed_mmio(addr + E1000_TCTL + 4,
> > +                                       E1000_TDT - (E1000_TCTL + 4));
> > +        qemu_kvm_register_delayed_mmio(addr + E1000_TDT + 4,
> > +                                       PNPMMIO_SIZE - (E1000_TDT + 4));
> > +    }
> >  }
> >  
> 
> What about the calls to set_ics()?  They can cause interrupts to be 
> deasserted AFAICS.  So IMC (and maybe a few other registers) need to be 
> excluded as well.

Well, I don't know how I missed them...

> Instead of hardcoding this, how about a
> 
> uint32_t delayed_mmi_excluded_regs[] = {
>    E1000_ICR, E1000_TCTL, ...
> };
> 
> The code can sort them and generate the ranges automatically.
> 
> (alternatively, a boolmap:
> 
> bool delayed_mmi_excluded_regs[] = {
>   [E1000_ICR]: true, ...
> };
> )

I'll try...

Regards,
Laurent
-- 
------------- Laurent.Vivier@bull.net ---------------
"The best way to predict the future is to invent it."
- Alan Kay