From: Christian Ehrhardt <ehrhardt-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
To: "kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org"
<kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org>,
Hollis Blanchard
<hollisb-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>,
kvm-ppc-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org,
Jerone Young <jyoung5-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
Subject: [PATCH] RFC: simplify kvm-userspace to qemu-kvm callback structure
Date: Wed, 05 Dec 2007 15:52:04 +0100 [thread overview]
Message-ID: <4756BB14.1080708@linux.vnet.ibm.com> (raw)
[-- 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
next reply other threads:[~2007-12-05 14:52 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2007-12-05 14:52 Christian Ehrhardt [this message]
[not found] ` <4756BB14.1080708-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
2007-12-06 14:50 ` [PATCH] RFC: simplify kvm-userspace to qemu-kvm callback structure Avi Kivity
[not found] ` <47580C1B.4090708-atKUWr5tajBWk0Htik3J/w@public.gmane.org>
2007-12-11 13:01 ` Christian Ehrhardt
2007-12-07 11:38 ` [PATCH] [0/1] " Christian Ehrhardt
[not found] ` <475930A6.60301-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
2007-12-07 11:40 ` Avi Kivity
2007-12-07 11:38 ` [PATCH] [1/1] " Christian Ehrhardt
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4756BB14.1080708@linux.vnet.ibm.com \
--to=ehrhardt-23vcf4htsmix0ybbhkvfkdbpr1lh4cv8@public.gmane.org \
--cc=hollisb-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org \
--cc=jyoung5-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org \
--cc=kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org \
--cc=kvm-ppc-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox