From mboxrd@z Thu Jan 1 00:00:00 1970 From: Christian Ehrhardt Subject: [PATCH] RFC: simplify kvm-userspace to qemu-kvm callback structure Date: Wed, 05 Dec 2007 15:52:04 +0100 Message-ID: <4756BB14.1080708@linux.vnet.ibm.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="------------020709070503010509050807" To: "kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org" , Hollis Blanchard , kvm-ppc-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org, Jerone Young Return-path: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: kvm-devel-bounces-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org Errors-To: kvm-devel-bounces-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org List-Id: kvm.vger.kernel.org This is a multi-part message in MIME format. --------------020709070503010509050807 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: quoted-printable 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 poi= nters for read/write mmio in kvm_callbacks I wondered if this can be simp= lified 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 workar= ound 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 o= ther implementations of this interface e.g. user/main.c and a rebased ver= sion of this one. But maybe there was a reason to do it that split way with all the callbac= k 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 th= e 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 Merging the kvm callback elements write[bwlq] and read[bwlq] into a singl= e call simplifying that interface. Additionally this patch fixes a small typo and combines two workarounds f= or the same issue in code the patch touches anyway. Signed-off-by: Christian Ehrhardt --- [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 kv= m_run *kvm_run) { unsigned long addr =3D kvm_run->mmio.phys_addr; void *data =3D kvm_run->mmio.data; - int r =3D -1; =20 - /* hack: Red Hat 7.1 generates these wierd accesses. */ - if (addr =3D=3D 0xa0000 && kvm_run->mmio.len =3D=3D 3) + /* hack: Red Hat 7.1 generates these weird accesses. */ + if ((addr > 0xa0000-4 && addr <=3D 0xa0000) && kvm_run->mmio.len =3D=3D= 3) return 0; =20 - if (kvm_run->mmio.is_write) { - switch (kvm_run->mmio.len) { - case 1: - r =3D kvm->callbacks->writeb(kvm->opaque, addr, *(uint8_t *)data); - break; - case 2: - r =3D kvm->callbacks->writew(kvm->opaque, addr, *(uint16_t *)data); - break; - case 4: - r =3D kvm->callbacks->writel(kvm->opaque, addr, *(uint32_t *)data); - break; - case 8: - r =3D kvm->callbacks->writeq(kvm->opaque, addr, *(uint64_t *)data); - break; - } - } else { - switch (kvm_run->mmio.len) { - case 1: - r =3D kvm->callbacks->readb(kvm->opaque, addr, (uint8_t *)data); - break; - case 2: - r =3D kvm->callbacks->readw(kvm->opaque, addr, (uint16_t *)data); - break; - case 4: - r =3D kvm->callbacks->readl(kvm->opaque, addr, (uint32_t *)data); - break; - case 8: - r =3D 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); } =20 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, ui= nt32_t data) return 0; } =20 -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 =3D ldub_phys(addr); - return 0; -} -=20 -static int kvm_readw(void *opaque, uint64_t addr, uint16_t *data) -{ - *data =3D 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 =3D 0; + cpu_physical_memory_rw(addr,data,len,is_write); return 0; - } - - *data =3D ldl_phys(addr); - return 0; -} - -static int kvm_readq(void *opaque, uint64_t addr, uint64_t *data) -{ - *data =3D 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; } =20 static int kvm_io_window(void *opaque) @@ -556,14 +509,7 @@ static struct kvm_callbacks qemu_kvm_ops =3D { .outb =3D kvm_outb, .outw =3D kvm_outw, .outl =3D kvm_outl, - .readb =3D kvm_readb, - .readw =3D kvm_readw, - .readl =3D kvm_readl, - .readq =3D kvm_readq, - .writeb =3D kvm_writeb, - .writew =3D kvm_writew, - .writel =3D kvm_writel, - .writeq =3D kvm_writeq, + .mmio_rw =3D kvm_mmio_rw, .halt =3D kvm_halt, .shutdown =3D kvm_shutdown, .io_window =3D kvm_io_window, --=20 Gr=FCsse / regards,=20 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=20 Gesch=E4ftsf=FChrung: Herbert Kircher=20 Sitz der Gesellschaft: B=F6blingen Registergericht: Amtsgericht Stuttgart, HRB 243294 --------------020709070503010509050807 Content-Type: text/plain; name="simplify_kvm_callback" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="simplify_kvm_callback" 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, --------------020709070503010509050807 Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline ------------------------------------------------------------------------- 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 --------------020709070503010509050807 Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ kvm-devel mailing list kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org https://lists.sourceforge.net/lists/listinfo/kvm-devel --------------020709070503010509050807--