[PATCH] RFC: simplify kvm-userspace to qemu-kvm callback structure

[PATCH] RFC: simplify kvm-userspace to qemu-kvm callback structure

[Christian Ehrhardt]

[-- Attachment #1: Type: text/plain, Size: 7652 bytes --]

Background:
In our ppc code for the demo we only needed a call to cpu_physical_memory_rw to handle all kind of mmio we needed. Looking at all the callback pointers for read/write mmio in kvm_callbacks I wondered if this can be simplified with cpu_physical_memory_rw for x86 too. So I tested it today and it works fine on with kvm-svm on my opteron.
The only code that did not just redirect to another function was a workaround for a Redhat 7.1 issue, so I merged it in the central call making it easier to find and maintain (was split before) anyway.
If everyone agrees with it I will create a new patch also affecting the other implementations of this interface e.g. user/main.c and a rebased version of this one.
But maybe there was a reason to do it that split way with all the callback pointers that was not obvious to me, so please comment.

P.S. up to now I did not find such a single function in qemu to handle the normal I/O (non mm), but maybe there is more potential to slimline that callback stuff.

---

Subject: [PATCH] simplify kvm-userspace to qemu-kvm callback structure

From: Christian Ehrhardt <ehrhardt-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>

Merging the kvm callback elements write[bwlq] and read[bwlq] into a single call
simplifying that interface.
Additionally this patch fixes a small typo and combines two workarounds for the
same issue in code the patch touches anyway.

Signed-off-by: Christian Ehrhardt <ehrhardt-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
---

[diffstat]
 libkvm/libkvm.c |   40 ++++--------------------------------
 libkvm/libkvm.h |   19 ++---------------
 qemu/qemu-kvm.c |   62 +++-----------------------------------------------------
 3 files changed, 12 insertions(+), 109 deletions(-)

[diff (also attached)]
diff --git a/libkvm/libkvm.c b/libkvm/libkvm.c
index 573a9ab..057706c 100644
--- a/libkvm/libkvm.c
+++ b/libkvm/libkvm.c
@@ -771,44 +771,14 @@ static int handle_mmio(kvm_context_t kvm, struct kvm_run *kvm_run)
 {
 	unsigned long addr = kvm_run->mmio.phys_addr;
 	void *data = kvm_run->mmio.data;
-	int r = -1;
 
-	/* hack: Red Hat 7.1 generates these wierd accesses. */
-	if (addr == 0xa0000 && kvm_run->mmio.len == 3)
+	/* hack: Red Hat 7.1 generates these weird accesses. */
+	if ((addr > 0xa0000-4 && addr <= 0xa0000) && kvm_run->mmio.len == 3)
 	    return 0;
 
-	if (kvm_run->mmio.is_write) {
-		switch (kvm_run->mmio.len) {
-		case 1:
-			r = kvm->callbacks->writeb(kvm->opaque, addr, *(uint8_t *)data);
-			break;
-		case 2:
-			r = kvm->callbacks->writew(kvm->opaque, addr, *(uint16_t *)data);
-			break;
-		case 4:
-			r = kvm->callbacks->writel(kvm->opaque, addr, *(uint32_t *)data);
-			break;
-		case 8:
-			r = kvm->callbacks->writeq(kvm->opaque, addr, *(uint64_t *)data);
-			break;
-		}
-	} else {
-		switch (kvm_run->mmio.len) {
-		case 1:
-			r = kvm->callbacks->readb(kvm->opaque, addr, (uint8_t *)data);
-			break;
-		case 2:
-			r = kvm->callbacks->readw(kvm->opaque, addr, (uint16_t *)data);
-			break;
-		case 4:
-			r = kvm->callbacks->readl(kvm->opaque, addr, (uint32_t *)data);
-			break;
-		case 8:
-			r = kvm->callbacks->readq(kvm->opaque, addr, (uint64_t *)data);
-			break;
-		}
-	}
-	return r;
+	return kvm->callbacks->mmio_rw(kvm->opaque, addr, data,
+					kvm_run->mmio.len,
+					kvm_run->mmio.is_write);
 }
 
 int handle_io_window(kvm_context_t kvm)
diff --git a/libkvm/libkvm.h b/libkvm/libkvm.h
index ff260f4..d44a364 100644
--- a/libkvm/libkvm.h
+++ b/libkvm/libkvm.h
@@ -45,22 +45,9 @@ struct kvm_callbacks {
     int (*outw)(void *opaque, uint16_t addr, uint16_t data);
 	/// For 32bit IO writes from the guest (Usually when executing 'outl')
     int (*outl)(void *opaque, uint16_t addr, uint32_t data);
-	/// For 8bit memory reads from unmapped memory (For MMIO devices)
-    int (*readb)(void *opaque, uint64_t addr, uint8_t *data);
-	/// For 16bit memory reads from unmapped memory (For MMIO devices)
-    int (*readw)(void *opaque, uint64_t addr, uint16_t *data);
-	/// For 32bit memory reads from unmapped memory (For MMIO devices)
-    int (*readl)(void *opaque, uint64_t addr, uint32_t *data);
-	/// For 64bit memory reads from unmapped memory (For MMIO devices)
-    int (*readq)(void *opaque, uint64_t addr, uint64_t *data);
-	/// For 8bit memory writes to unmapped memory (For MMIO devices)
-    int (*writeb)(void *opaque, uint64_t addr, uint8_t data);
-	/// For 16bit memory writes to unmapped memory (For MMIO devices)
-    int (*writew)(void *opaque, uint64_t addr, uint16_t data);
-	/// For 32bit memory writes to unmapped memory (For MMIO devices)
-    int (*writel)(void *opaque, uint64_t addr, uint32_t data);
-	/// For 64bit memory writes to unmapped memory (For MMIO devices)
-    int (*writeq)(void *opaque, uint64_t addr, uint64_t data);
+	/// generic memory writes to unmapped memory (For MMIO devices)
+    int (*mmio_rw)(void *opaque, uint64_t addr, uint8_t *data,
+					int len, int is_write);
     int (*debug)(void *opaque, int vcpu);
 	/*!
 	 * \brief Called when the VCPU issues an 'hlt' instruction.
diff --git a/qemu/qemu-kvm.c b/qemu/qemu-kvm.c
index 3aeba39..2b61a42 100644
--- a/qemu/qemu-kvm.c
+++ b/qemu/qemu-kvm.c
@@ -477,58 +477,11 @@ static int kvm_outl(void *opaque, uint16_t addr, uint32_t data)
     return 0;
 }
 
-static int kvm_readb(void *opaque, uint64_t addr, uint8_t *data)
+static int kvm_mmio_rw(void *opaque, uint64_t addr,
+					uint8_t *data, int len, int is_write)
 {
-    *data = ldub_phys(addr);
-    return 0;
-}
- 
-static int kvm_readw(void *opaque, uint64_t addr, uint16_t *data)
-{
-    *data = lduw_phys(addr);
-    return 0;
-}
-
-static int kvm_readl(void *opaque, uint64_t addr, uint32_t *data)
-{
-    /* hack: Red Hat 7.1 generates some wierd accesses. */
-    if (addr > 0xa0000 - 4 && addr < 0xa0000) {
-	*data = 0;
+	cpu_physical_memory_rw(addr,data,len,is_write);
 	return 0;
-    }
-
-    *data = ldl_phys(addr);
-    return 0;
-}
-
-static int kvm_readq(void *opaque, uint64_t addr, uint64_t *data)
-{
-    *data = ldq_phys(addr);
-    return 0;
-}
-
-static int kvm_writeb(void *opaque, uint64_t addr, uint8_t data)
-{
-    stb_phys(addr, data);
-    return 0;
-}
-
-static int kvm_writew(void *opaque, uint64_t addr, uint16_t data)
-{
-    stw_phys(addr, data);
-    return 0;
-}
-
-static int kvm_writel(void *opaque, uint64_t addr, uint32_t data)
-{
-    stl_phys(addr, data);
-    return 0;
-}
-
-static int kvm_writeq(void *opaque, uint64_t addr, uint64_t data)
-{
-    stq_phys(addr, data);
-    return 0;
 }
 
 static int kvm_io_window(void *opaque)
@@ -556,14 +509,7 @@ static struct kvm_callbacks qemu_kvm_ops = {
     .outb  = kvm_outb,
     .outw  = kvm_outw,
     .outl  = kvm_outl,
-    .readb = kvm_readb,
-    .readw = kvm_readw,
-    .readl = kvm_readl,
-    .readq = kvm_readq,
-    .writeb = kvm_writeb,
-    .writew = kvm_writew,
-    .writel = kvm_writel,
-    .writeq = kvm_writeq,
+    .mmio_rw = kvm_mmio_rw,
     .halt  = kvm_halt,
     .shutdown = kvm_shutdown,
     .io_window = kvm_io_window,



-- 

Grüsse / regards, 
Christian Ehrhardt

IBM Linux Technology Center, Open Virtualization
+49 7031/16-3385
Ehrhardt-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org
Ehrhardt-tA70FqPdS9bQT0dZR+AlfA@public.gmane.org

IBM Deutschland Entwicklung GmbH
Vorsitzender des Aufsichtsrats: Johann Weihen 
Geschäftsführung: Herbert Kircher 
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294

[-- Attachment #2: simplify_kvm_callback --]
[-- Type: text/plain, Size: 5237 bytes --]

diff --git a/libkvm/libkvm.c b/libkvm/libkvm.c
index 573a9ab..057706c 100644
--- a/libkvm/libkvm.c
+++ b/libkvm/libkvm.c
@@ -771,44 +771,14 @@ static int handle_mmio(kvm_context_t kvm, struct kvm_run *kvm_run)
 {
 	unsigned long addr = kvm_run->mmio.phys_addr;
 	void *data = kvm_run->mmio.data;
-	int r = -1;
 
-	/* hack: Red Hat 7.1 generates these wierd accesses. */
-	if (addr == 0xa0000 && kvm_run->mmio.len == 3)
+	/* hack: Red Hat 7.1 generates these weird accesses. */
+	if ((addr > 0xa0000-4 && addr <= 0xa0000) && kvm_run->mmio.len == 3)
 	    return 0;
 
-	if (kvm_run->mmio.is_write) {
-		switch (kvm_run->mmio.len) {
-		case 1:
-			r = kvm->callbacks->writeb(kvm->opaque, addr, *(uint8_t *)data);
-			break;
-		case 2:
-			r = kvm->callbacks->writew(kvm->opaque, addr, *(uint16_t *)data);
-			break;
-		case 4:
-			r = kvm->callbacks->writel(kvm->opaque, addr, *(uint32_t *)data);
-			break;
-		case 8:
-			r = kvm->callbacks->writeq(kvm->opaque, addr, *(uint64_t *)data);
-			break;
-		}
-	} else {
-		switch (kvm_run->mmio.len) {
-		case 1:
-			r = kvm->callbacks->readb(kvm->opaque, addr, (uint8_t *)data);
-			break;
-		case 2:
-			r = kvm->callbacks->readw(kvm->opaque, addr, (uint16_t *)data);
-			break;
-		case 4:
-			r = kvm->callbacks->readl(kvm->opaque, addr, (uint32_t *)data);
-			break;
-		case 8:
-			r = kvm->callbacks->readq(kvm->opaque, addr, (uint64_t *)data);
-			break;
-		}
-	}
-	return r;
+	return kvm->callbacks->mmio_rw(kvm->opaque, addr, data,
+					kvm_run->mmio.len,
+					kvm_run->mmio.is_write);
 }
 
 int handle_io_window(kvm_context_t kvm)
diff --git a/libkvm/libkvm.h b/libkvm/libkvm.h
index ff260f4..d44a364 100644
--- a/libkvm/libkvm.h
+++ b/libkvm/libkvm.h
@@ -45,22 +45,9 @@ struct kvm_callbacks {
     int (*outw)(void *opaque, uint16_t addr, uint16_t data);
 	/// For 32bit IO writes from the guest (Usually when executing 'outl')
     int (*outl)(void *opaque, uint16_t addr, uint32_t data);
-	/// For 8bit memory reads from unmapped memory (For MMIO devices)
-    int (*readb)(void *opaque, uint64_t addr, uint8_t *data);
-	/// For 16bit memory reads from unmapped memory (For MMIO devices)
-    int (*readw)(void *opaque, uint64_t addr, uint16_t *data);
-	/// For 32bit memory reads from unmapped memory (For MMIO devices)
-    int (*readl)(void *opaque, uint64_t addr, uint32_t *data);
-	/// For 64bit memory reads from unmapped memory (For MMIO devices)
-    int (*readq)(void *opaque, uint64_t addr, uint64_t *data);
-	/// For 8bit memory writes to unmapped memory (For MMIO devices)
-    int (*writeb)(void *opaque, uint64_t addr, uint8_t data);
-	/// For 16bit memory writes to unmapped memory (For MMIO devices)
-    int (*writew)(void *opaque, uint64_t addr, uint16_t data);
-	/// For 32bit memory writes to unmapped memory (For MMIO devices)
-    int (*writel)(void *opaque, uint64_t addr, uint32_t data);
-	/// For 64bit memory writes to unmapped memory (For MMIO devices)
-    int (*writeq)(void *opaque, uint64_t addr, uint64_t data);
+	/// generic memory writes to unmapped memory (For MMIO devices)
+    int (*mmio_rw)(void *opaque, uint64_t addr, uint8_t *data,
+					int len, int is_write);
     int (*debug)(void *opaque, int vcpu);
 	/*!
 	 * \brief Called when the VCPU issues an 'hlt' instruction.
diff --git a/qemu/qemu-kvm.c b/qemu/qemu-kvm.c
index 3aeba39..2b61a42 100644
--- a/qemu/qemu-kvm.c
+++ b/qemu/qemu-kvm.c
@@ -477,58 +477,11 @@ static int kvm_outl(void *opaque, uint16_t addr, uint32_t data)
     return 0;
 }
 
-static int kvm_readb(void *opaque, uint64_t addr, uint8_t *data)
+static int kvm_mmio_rw(void *opaque, uint64_t addr,
+					uint8_t *data, int len, int is_write)
 {
-    *data = ldub_phys(addr);
-    return 0;
-}
- 
-static int kvm_readw(void *opaque, uint64_t addr, uint16_t *data)
-{
-    *data = lduw_phys(addr);
-    return 0;
-}
-
-static int kvm_readl(void *opaque, uint64_t addr, uint32_t *data)
-{
-    /* hack: Red Hat 7.1 generates some wierd accesses. */
-    if (addr > 0xa0000 - 4 && addr < 0xa0000) {
-	*data = 0;
+	cpu_physical_memory_rw(addr,data,len,is_write);
 	return 0;
-    }
-
-    *data = ldl_phys(addr);
-    return 0;
-}
-
-static int kvm_readq(void *opaque, uint64_t addr, uint64_t *data)
-{
-    *data = ldq_phys(addr);
-    return 0;
-}
-
-static int kvm_writeb(void *opaque, uint64_t addr, uint8_t data)
-{
-    stb_phys(addr, data);
-    return 0;
-}
-
-static int kvm_writew(void *opaque, uint64_t addr, uint16_t data)
-{
-    stw_phys(addr, data);
-    return 0;
-}
-
-static int kvm_writel(void *opaque, uint64_t addr, uint32_t data)
-{
-    stl_phys(addr, data);
-    return 0;
-}
-
-static int kvm_writeq(void *opaque, uint64_t addr, uint64_t data)
-{
-    stq_phys(addr, data);
-    return 0;
 }
 
 static int kvm_io_window(void *opaque)
@@ -556,14 +509,7 @@ static struct kvm_callbacks qemu_kvm_ops = {
     .outb  = kvm_outb,
     .outw  = kvm_outw,
     .outl  = kvm_outl,
-    .readb = kvm_readb,
-    .readw = kvm_readw,
-    .readl = kvm_readl,
-    .readq = kvm_readq,
-    .writeb = kvm_writeb,
-    .writew = kvm_writew,
-    .writel = kvm_writel,
-    .writeq = kvm_writeq,
+    .mmio_rw = kvm_mmio_rw,
     .halt  = kvm_halt,
     .shutdown = kvm_shutdown,
     .io_window = kvm_io_window,

[-- Attachment #3: Type: text/plain, Size: 309 bytes --]

-------------------------------------------------------------------------
SF.Net email is sponsored by: The Future of Linux Business White Paper
from Novell.  From the desktop to the data center, Linux is going
mainstream.  Let it simplify your IT future.
http://altfarm.mediaplex.com/ad/ck/8857-50307-18918-4

[-- Attachment #4: Type: text/plain, Size: 186 bytes --]

_______________________________________________
kvm-devel mailing list
kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
https://lists.sourceforge.net/lists/listinfo/kvm-devel

Re: [PATCH] RFC: simplify kvm-userspace to qemu-kvm callback structure

[Avi Kivity]

Christian Ehrhardt wrote:
> Background:
> In our ppc code for the demo we only needed a call to 
> cpu_physical_memory_rw to handle all kind of mmio we needed. Looking 
> at all the callback pointers for read/write mmio in kvm_callbacks I 
> wondered if this can be simplified with cpu_physical_memory_rw for x86 
> too. So I tested it today and it works fine on with kvm-svm on my 
> opteron.
> The only code that did not just redirect to another function was a 
> workaround for a Redhat 7.1 issue, so I merged it in the central call 
> making it easier to find and maintain (was split before) anyway.
> If everyone agrees with it I will create a new patch also affecting 
> the other implementations of this interface e.g. user/main.c and a 
> rebased version of this one.
> But maybe there was a reason to do it that split way with all the 
> callback pointers that was not obvious to me, so please comment.

It may be worthwhile to unify all the reads into a single read, but read 
and write are fundamentally different.  The fact that qemu implements it 
in one function is a detail; libkvm aims to satisfy more than just qemu.


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


-------------------------------------------------------------------------
SF.Net email is sponsored by: The Future of Linux Business White Paper
from Novell.  From the desktop to the data center, Linux is going
mainstream.  Let it simplify your IT future.
http://altfarm.mediaplex.com/ad/ck/8857-50307-18918-4

[PATCH] [0/1] simplify kvm-userspace to qemu-kvm callback structure

[Christian Ehrhardt]

Well no response might mean that there is nothing to be said against this
simplification ;-)
Because of that I completed the patch (main.c as well) and corrected some
whitespace issues I had before.

Christian Ehrhardt wrote:
> Background:
> In our ppc code for the demo we only needed a call to 
> cpu_physical_memory_rw to handle all kind of mmio we needed. Looking at 
> all the callback pointers for read/write mmio in kvm_callbacks I 
> wondered if this can be simplified with cpu_physical_memory_rw for x86 
> too. So I tested it today and it works fine on with kvm-svm on my opteron.
> The only code that did not just redirect to another function was a 
> workaround for a Redhat 7.1 issue, so I merged it in the central call 
> making it easier to find and maintain (was split before) anyway.
> If everyone agrees with it I will create a new patch also affecting the 
> other implementations of this interface e.g. user/main.c and a rebased 
> version of this one.
> But maybe there was a reason to do it that split way with all the 
> callback pointers that was not obvious to me, so please comment.
> [...]

-- 

Grüsse / regards, 
Christian Ehrhardt

IBM Linux Technology Center, Open Virtualization
+49 7031/16-3385
Ehrhardt-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org
Ehrhardt-tA70FqPdS9bQT0dZR+AlfA@public.gmane.org

IBM Deutschland Entwicklung GmbH
Vorsitzender des Aufsichtsrats: Johann Weihen 
Geschäftsführung: Herbert Kircher 
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294

-------------------------------------------------------------------------
SF.Net email is sponsored by:
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://sourceforge.net/services/buy/index.php

[PATCH] [1/1] simplify kvm-userspace to qemu-kvm callback structure

[Christian Ehrhardt]

[-- Attachment #1: Type: text/plain, Size: 1288 bytes --]

Subject: [PATCH] simplify read[bwlq]/write[bwlq] callback structure

From: Christian Ehrhardt <ehrhardt-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>

Merging the read[bwlq]/write[bwlq] callback callback pointers to one 
mmio_rw function simplifies the callback interface (a lot of lines
in libkvm.c and main.c) and eventually uses the qemu copy frontend
function cpu_physical_memory_rw.

Signed-off-by: Christian Ehrhardt <ehrhardt-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
---

[diffstat]
 libkvm/libkvm.c |   40 ++++--------------------------------
 libkvm/libkvm.h |   19 ++---------------
 qemu/qemu-kvm.c |   62 +++-----------------------------------------------------
 user/main.c     |   52 ++++++----------------------------------------
 4 files changed, 19 insertions(+), 154 deletions(-)

[diff]
*Attached*

-- 

Grüsse / regards, 
Christian Ehrhardt

IBM Linux Technology Center, Open Virtualization
+49 7031/16-3385
Ehrhardt-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org
Ehrhardt-tA70FqPdS9bQT0dZR+AlfA@public.gmane.org

IBM Deutschland Entwicklung GmbH
Vorsitzender des Aufsichtsrats: Johann Weihen 
Geschäftsführung: Herbert Kircher 
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294

[-- Attachment #2: simplify_kvm_callback --]
[-- Type: text/plain, Size: 7136 bytes --]

diff --git a/libkvm/libkvm.c b/libkvm/libkvm.c
index 573a9ab..057706c 100644
--- a/libkvm/libkvm.c
+++ b/libkvm/libkvm.c
@@ -771,44 +771,14 @@ static int handle_mmio(kvm_context_t kvm, struct kvm_run *kvm_run)
 {
 	unsigned long addr = kvm_run->mmio.phys_addr;
 	void *data = kvm_run->mmio.data;
-	int r = -1;
 
-	/* hack: Red Hat 7.1 generates these wierd accesses. */
-	if (addr == 0xa0000 && kvm_run->mmio.len == 3)
+	/* hack: Red Hat 7.1 generates these weird accesses. */
+	if ((addr > 0xa0000-4 && addr <= 0xa0000) && kvm_run->mmio.len == 3)
 	    return 0;
 
-	if (kvm_run->mmio.is_write) {
-		switch (kvm_run->mmio.len) {
-		case 1:
-			r = kvm->callbacks->writeb(kvm->opaque, addr, *(uint8_t *)data);
-			break;
-		case 2:
-			r = kvm->callbacks->writew(kvm->opaque, addr, *(uint16_t *)data);
-			break;
-		case 4:
-			r = kvm->callbacks->writel(kvm->opaque, addr, *(uint32_t *)data);
-			break;
-		case 8:
-			r = kvm->callbacks->writeq(kvm->opaque, addr, *(uint64_t *)data);
-			break;
-		}
-	} else {
-		switch (kvm_run->mmio.len) {
-		case 1:
-			r = kvm->callbacks->readb(kvm->opaque, addr, (uint8_t *)data);
-			break;
-		case 2:
-			r = kvm->callbacks->readw(kvm->opaque, addr, (uint16_t *)data);
-			break;
-		case 4:
-			r = kvm->callbacks->readl(kvm->opaque, addr, (uint32_t *)data);
-			break;
-		case 8:
-			r = kvm->callbacks->readq(kvm->opaque, addr, (uint64_t *)data);
-			break;
-		}
-	}
-	return r;
+	return kvm->callbacks->mmio_rw(kvm->opaque, addr, data,
+					kvm_run->mmio.len,
+					kvm_run->mmio.is_write);
 }
 
 int handle_io_window(kvm_context_t kvm)
diff --git a/libkvm/libkvm.h b/libkvm/libkvm.h
index ff260f4..d44a364 100644
--- a/libkvm/libkvm.h
+++ b/libkvm/libkvm.h
@@ -45,22 +45,9 @@ struct kvm_callbacks {
     int (*outw)(void *opaque, uint16_t addr, uint16_t data);
 	/// For 32bit IO writes from the guest (Usually when executing 'outl')
     int (*outl)(void *opaque, uint16_t addr, uint32_t data);
-	/// For 8bit memory reads from unmapped memory (For MMIO devices)
-    int (*readb)(void *opaque, uint64_t addr, uint8_t *data);
-	/// For 16bit memory reads from unmapped memory (For MMIO devices)
-    int (*readw)(void *opaque, uint64_t addr, uint16_t *data);
-	/// For 32bit memory reads from unmapped memory (For MMIO devices)
-    int (*readl)(void *opaque, uint64_t addr, uint32_t *data);
-	/// For 64bit memory reads from unmapped memory (For MMIO devices)
-    int (*readq)(void *opaque, uint64_t addr, uint64_t *data);
-	/// For 8bit memory writes to unmapped memory (For MMIO devices)
-    int (*writeb)(void *opaque, uint64_t addr, uint8_t data);
-	/// For 16bit memory writes to unmapped memory (For MMIO devices)
-    int (*writew)(void *opaque, uint64_t addr, uint16_t data);
-	/// For 32bit memory writes to unmapped memory (For MMIO devices)
-    int (*writel)(void *opaque, uint64_t addr, uint32_t data);
-	/// For 64bit memory writes to unmapped memory (For MMIO devices)
-    int (*writeq)(void *opaque, uint64_t addr, uint64_t data);
+	/// generic memory writes to unmapped memory (For MMIO devices)
+    int (*mmio_rw)(void *opaque, uint64_t addr, uint8_t *data,
+					int len, int is_write);
     int (*debug)(void *opaque, int vcpu);
 	/*!
 	 * \brief Called when the VCPU issues an 'hlt' instruction.
diff --git a/qemu/qemu-kvm.c b/qemu/qemu-kvm.c
index 3aeba39..b28d832 100644
--- a/qemu/qemu-kvm.c
+++ b/qemu/qemu-kvm.c
@@ -477,58 +477,11 @@ static int kvm_outl(void *opaque, uint16_t addr, uint32_t data)
     return 0;
 }
 
-static int kvm_readb(void *opaque, uint64_t addr, uint8_t *data)
+static int kvm_mmio_rw(void *opaque, uint64_t addr,
+					uint8_t *data, int len, int is_write)
 {
-    *data = ldub_phys(addr);
-    return 0;
-}
- 
-static int kvm_readw(void *opaque, uint64_t addr, uint16_t *data)
-{
-    *data = lduw_phys(addr);
-    return 0;
-}
-
-static int kvm_readl(void *opaque, uint64_t addr, uint32_t *data)
-{
-    /* hack: Red Hat 7.1 generates some wierd accesses. */
-    if (addr > 0xa0000 - 4 && addr < 0xa0000) {
-	*data = 0;
+	cpu_physical_memory_rw(addr, data, len, is_write);
 	return 0;
-    }
-
-    *data = ldl_phys(addr);
-    return 0;
-}
-
-static int kvm_readq(void *opaque, uint64_t addr, uint64_t *data)
-{
-    *data = ldq_phys(addr);
-    return 0;
-}
-
-static int kvm_writeb(void *opaque, uint64_t addr, uint8_t data)
-{
-    stb_phys(addr, data);
-    return 0;
-}
-
-static int kvm_writew(void *opaque, uint64_t addr, uint16_t data)
-{
-    stw_phys(addr, data);
-    return 0;
-}
-
-static int kvm_writel(void *opaque, uint64_t addr, uint32_t data)
-{
-    stl_phys(addr, data);
-    return 0;
-}
-
-static int kvm_writeq(void *opaque, uint64_t addr, uint64_t data)
-{
-    stq_phys(addr, data);
-    return 0;
 }
 
 static int kvm_io_window(void *opaque)
@@ -556,14 +509,7 @@ static struct kvm_callbacks qemu_kvm_ops = {
     .outb  = kvm_outb,
     .outw  = kvm_outw,
     .outl  = kvm_outl,
-    .readb = kvm_readb,
-    .readw = kvm_readw,
-    .readl = kvm_readl,
-    .readq = kvm_readq,
-    .writeb = kvm_writeb,
-    .writew = kvm_writew,
-    .writel = kvm_writel,
-    .writeq = kvm_writeq,
+    .mmio_rw = kvm_mmio_rw,
     .halt  = kvm_halt,
     .shutdown = kvm_shutdown,
     .io_window = kvm_io_window,
diff --git a/user/main.c b/user/main.c
index 213b019..c408bbe 100644
--- a/user/main.c
+++ b/user/main.c
@@ -382,44 +382,13 @@ static int test_mem_write(uint64_t addr, void *data, unsigned len)
 	return 0;
 }
 
-static int test_readb(void *opaque, uint64_t addr, uint8_t *data)
+static int test_mmio_rw(void *opaque, uint64_t addr, uint8_t *data, int len,
+			int is_write)
 {
-	return test_mem_read(addr, data, 1);
-}
-
-static int test_readw(void *opaque, uint64_t addr, uint16_t *data)
-{
-	return test_mem_read(addr, data, 2);
-}
-
-static int test_readl(void *opaque, uint64_t addr, uint32_t *data)
-{
-	return test_mem_read(addr, data, 4);
-
-}
-static int test_readq(void *opaque, uint64_t addr, uint64_t *data)
-{
-	return test_mem_read(addr, data, 8);
-}
-
-static int test_writeb(void *opaque, uint64_t addr, uint8_t data)
-{
-	return test_mem_write(addr, &data, 1);
-}
-
-static int test_writew(void *opaque, uint64_t addr, uint16_t data)
-{
-	return test_mem_write(addr, &data, 2);
-}
-
-static int test_writel(void *opaque, uint64_t addr, uint32_t data)
-{
-	return test_mem_write(addr, &data, 4);
-}
-
-static int test_writeq(void *opaque, uint64_t addr, uint64_t data)
-{
-	return test_mem_write(addr, &data, 8);
+	if (is_write)
+		return test_mem_write(addr, data, len);
+	else
+		return test_mem_read(addr, data, len);
 }
 
 static struct kvm_callbacks test_callbacks = {
@@ -429,14 +398,7 @@ static struct kvm_callbacks test_callbacks = {
 	.outb        = test_outb,
 	.outw        = test_outw,
 	.outl        = test_outl,
-	.readb       = test_readb,
-	.readw       = test_readw,
-	.readl       = test_readl,
-	.readq       = test_readq,
-	.writeb      = test_writeb,
-	.writew      = test_writew,
-	.writel      = test_writel,
-	.writeq      = test_writeq,
+	.mmio_rw     = test_mmio_rw,
 	.debug       = test_debug,
 	.halt        = test_halt,
 	.io_window = test_io_window,

[-- Attachment #3: Type: text/plain, Size: 277 bytes --]

-------------------------------------------------------------------------
SF.Net email is sponsored by:
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://sourceforge.net/services/buy/index.php

[-- Attachment #4: Type: text/plain, Size: 186 bytes --]

_______________________________________________
kvm-devel mailing list
kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
https://lists.sourceforge.net/lists/listinfo/kvm-devel

Re: [PATCH] [0/1] simplify kvm-userspace to qemu-kvm callback structure

[Avi Kivity]

Christian Ehrhardt wrote:
> Well no response might mean that there is nothing to be said against this
> simplification ;-)
>   

I did respond, see 
http://article.gmane.org/gmane.comp.emulators.kvm.devel/10467.

-- 
Any sufficiently difficult bug is indistinguishable from a feature.


-------------------------------------------------------------------------
SF.Net email is sponsored by:
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://sourceforge.net/services/buy/index.php

Re: [PATCH] RFC: simplify kvm-userspace to qemu-kvm callback structure

[Christian Ehrhardt]

Avi Kivity wrote:
> Christian Ehrhardt wrote:
>> Background:
>> In our ppc code for the demo we only needed a call to 
>> cpu_physical_memory_rw to handle all kind of mmio we needed. Looking 
>> at all the callback pointers for read/write mmio in kvm_callbacks I 
>> wondered if this can be simplified with cpu_physical_memory_rw for x86 
>> too. So I tested it today and it works fine on with kvm-svm on my 
>> opteron.
>> The only code that did not just redirect to another function was a 
>> workaround for a Redhat 7.1 issue, so I merged it in the central call 
>> making it easier to find and maintain (was split before) anyway.
>> If everyone agrees with it I will create a new patch also affecting 
>> the other implementations of this interface e.g. user/main.c and a 
>> rebased version of this one.
>> But maybe there was a reason to do it that split way with all the 
>> callback pointers that was not obvious to me, so please comment.
> 
> It may be worthwhile to unify all the reads into a single read, but read 
> and write are fundamentally different.  The fact that qemu implements it 
> in one function is a detail; libkvm aims to satisfy more than just qemu.
> 

That's fair - I thought too qemu-centric while the interface should be 
generic.
I changed the simplification patch to have separate read/write calls,
but not one per len. That still reduce the different callbacks for mmio
from eight to two pointers.
To have it clean I put the updated patch in a separate mail.

P.S. Sorry for overlooking your response and asking again.

-- 

Grüsse / regards, 
Christian Ehrhardt

IBM Linux Technology Center, Open Virtualization
+49 7031/16-3385
Ehrhardt-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org
Ehrhardt-tA70FqPdS9bQT0dZR+AlfA@public.gmane.org

IBM Deutschland Entwicklung GmbH
Vorsitzender des Aufsichtsrats: Johann Weihen 
Geschäftsführung: Herbert Kircher 
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294

-------------------------------------------------------------------------
SF.Net email is sponsored by:
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://sourceforge.net/services/buy/index.php