Re: [kvm-ppc-devel] RFC: MMIO endianness flag

[Hollis Blanchard <hollisb-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>]

On Thu, 2008-01-10 at 17:28 +0200, Avi Kivity wrote:
> I'll apply that patch (with a #ifdef CONFIG_PPC so other archs don't
> use it by mistake).

I don't think that's the right ifdef. For example, I believe IA64 can
run in BE mode and so will have the same issue, and there are certainly
other architectures (less relevant to the current code) that definitely
are in the same situation.

We need to plumb this through to the libkvm users anyways. Take a look
at the patch below and tell me if you think it's not the right approach.
x86 simply won't consider 'is_bigendian'. I spent a lot of time on this,
and it's by far the cleanest solution I could come up with.



diff --git a/libkvm/config-i386.mak b/libkvm/config-i386.mak
--- a/libkvm/config-i386.mak
+++ b/libkvm/config-i386.mak
@@ -2,5 +2,6 @@ LIBDIR := /lib
 LIBDIR := /lib
 CFLAGS += -m32
 CFLAGS += -D__i386__
+CFLAGS += -DARCH_MMIO_ENDIAN_LITTLE
 
 libkvm-$(ARCH)-objs := libkvm-x86.o
diff --git a/libkvm/config-ia64.mak b/libkvm/config-ia64.mak
--- a/libkvm/config-ia64.mak
+++ b/libkvm/config-ia64.mak
@@ -1,5 +1,6 @@
 
 LIBDIR := /lib
 CFLAGS += -D__ia64__
+CFLAGS += -DARCH_MMIO_ENDIAN_LITTLE -DARCH_MMIO_ENDIAN_BIG
 
 libkvm-$(ARCH)-objs := libkvm-ia64.o
diff --git a/libkvm/config-powerpc.mak b/libkvm/config-powerpc.mak
--- a/libkvm/config-powerpc.mak
+++ b/libkvm/config-powerpc.mak
@@ -2,5 +2,6 @@ LIBDIR := /lib
 LIBDIR := /lib
 CFLAGS += -m32
 CFLAGS += -D__powerpc__
+CFLAGS += -DARCH_MMIO_ENDIAN_BIG -DARCH_MMIO_ENDIAN_LITTLE
 
 libkvm-$(ARCH)-objs := libkvm-powerpc.o
diff --git a/libkvm/config-x86_64.mak b/libkvm/config-x86_64.mak
--- a/libkvm/config-x86_64.mak
+++ b/libkvm/config-x86_64.mak
@@ -2,5 +2,6 @@ LIBDIR := /lib64
 LIBDIR := /lib64
 CFLAGS += -m64
 CFLAGS += -D__x86_64__
+CFLAGS += -DARCH_MMIO_ENDIAN_LITTLE
 
 libkvm-$(ARCH)-objs := libkvm-x86.o
diff --git a/libkvm/libkvm.c b/libkvm/libkvm.c
--- a/libkvm/libkvm.c
+++ b/libkvm/libkvm.c
@@ -774,21 +774,56 @@ int kvm_set_sregs(kvm_context_t kvm, int
     return ioctl(kvm->vcpu_fd[vcpu], KVM_SET_SREGS, sregs);
 }
 
+#ifdef ARCH_MMIO_ENDIAN_BIG
+static int handle_mmio_bigendian(kvm_context_t kvm, struct kvm_run *kvm_run)
+{
+	if (kvm_run->mmio.is_write)
+		return kvm->callbacks->mmio_write_be(kvm->opaque,
+		                                     kvm_run->mmio.phys_addr,
+						     kvm_run->mmio.data,
+						     kvm_run->mmio.len);
+	else
+		return kvm->callbacks->mmio_read_be(kvm->opaque,
+		                                    kvm_run->mmio.phys_addr,
+						    kvm_run->mmio.data,
+						    kvm_run->mmio.len);
+}
+#endif
+
+#ifdef ARCH_MMIO_ENDIAN_LITTLE
+static int handle_mmio_littleendian(kvm_context_t kvm, struct kvm_run *kvm_run)
+{
+	if (kvm_run->mmio.is_write)
+		return kvm->callbacks->mmio_write_le(kvm->opaque,
+		                                     kvm_run->mmio.phys_addr,
+						     kvm_run->mmio.data,
+						     kvm_run->mmio.len);
+	else
+		return kvm->callbacks->mmio_read_le(kvm->opaque,
+		                                    kvm_run->mmio.phys_addr,
+						    kvm_run->mmio.data,
+						    kvm_run->mmio.len);
+}
+#endif
+
 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;
 
 	/* 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)
-		return kvm->callbacks->mmio_write(kvm->opaque, addr, data,
-					kvm_run->mmio.len);
+#if defined(ARCH_MMIO_ENDIAN_BIG) && defined(ARCH_MMIO_ENDIAN_LITTLE)
+	if (kvm_run->mmio.is_bigendian)
+		return handle_mmio_bigendian(kvm, kvm_run);
 	else
-		return kvm->callbacks->mmio_read(kvm->opaque, addr, data,
-					kvm_run->mmio.len);
+		return handle_mmio_littleendian(kvm, kvm_run);
+#elif ARCH_MMIO_ENDIAN_LITTLE
+	return handle_mmio_littleendian(kvm, kvm_run);
+#elif ARCH_MMIO_ENDIAN_BIG
+	return handle_mmio_bigendian(kvm, kvm_run);
+#endif
 }
 
 int handle_io_window(kvm_context_t kvm)
diff --git a/libkvm/libkvm.h b/libkvm/libkvm.h
--- a/libkvm/libkvm.h
+++ b/libkvm/libkvm.h
@@ -46,11 +46,11 @@ struct kvm_callbacks {
 	/// For 32bit IO writes from the guest (Usually when executing 'outl')
     int (*outl)(void *opaque, uint16_t addr, uint32_t data);
 	/// generic memory reads to unmapped memory (For MMIO devices)
-    int (*mmio_read)(void *opaque, uint64_t addr, uint8_t *data,
-					int len);
+    int (*mmio_read_be)(void *opaque, uint64_t addr, uint8_t *data, int len);
+    int (*mmio_read_le)(void *opaque, uint64_t addr, uint8_t *data, int len);
 	/// generic memory writes to unmapped memory (For MMIO devices)
-    int (*mmio_write)(void *opaque, uint64_t addr, uint8_t *data,
-					int len);
+    int (*mmio_write_be)(void *opaque, uint64_t addr, uint8_t *data, int len);
+    int (*mmio_write_le)(void *opaque, uint64_t addr, uint8_t *data, int len);
     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
--- a/qemu/qemu-kvm.c
+++ b/qemu/qemu-kvm.c
@@ -21,6 +21,7 @@ int kvm_irqchip = 1;
 #include <libkvm.h>
 #include <pthread.h>
 #include <sys/utsname.h>
+#include <endian.h>
 
 extern void perror(const char *s);
 
@@ -533,8 +534,13 @@ static struct kvm_callbacks qemu_kvm_ops
     .outb  = kvm_outb,
     .outw  = kvm_outw,
     .outl  = kvm_outl,
-    .mmio_read = kvm_mmio_read,
-    .mmio_write = kvm_mmio_write,
+#if __BYTE_ORDER == __LITTLE_ENDIAN
+    .mmio_read_le = kvm_mmio_read,
+    .mmio_write_le = kvm_mmio_write,
+#else
+    .mmio_read_be = kvm_mmio_read,
+    .mmio_write_be = kvm_mmio_write,
+#endif
     .halt  = kvm_halt,
     .shutdown = kvm_shutdown,
     .io_window = kvm_io_window,
diff --git a/user/main-ppc.c b/user/main-ppc.c
--- a/user/main-ppc.c
+++ b/user/main-ppc.c
@@ -92,14 +92,14 @@ static int test_pre_kvm_run(void *opaque
 	return 0;
 }
 
-static int test_mem_read(void *opaque, uint64_t addr, uint8_t *data, int len)
+static int test_mem_read_be(void *opaque, uint64_t addr, uint8_t *data, int len)
 {
 	printf("%s: addr %"PRIx64" len %d\n", __func__, addr, len);
 	memset(data, 0, len);
 	return 0;
 }
 
-static int test_mem_write(void *opaque, uint64_t addr, uint8_t *data, int len)
+static int test_mem_write_be(void *opaque, uint64_t addr, uint8_t *data, int len)
 {
 	printf("%s: addr %"PRIx64" len %d data %"PRIx64"\n",
 	       __func__, addr, len, *(uint64_t *)data);
@@ -120,8 +120,8 @@ static int test_dcr_write(kvm_context_t 
 }
 
 static struct kvm_callbacks test_callbacks = {
-	.mmio_read   = test_mem_read,
-	.mmio_write  = test_mem_write,
+	.mmio_read_be = test_mem_read_be,
+	.mmio_write_be = test_mem_write_be,
 	.debug       = test_debug,
 	.halt        = test_halt,
 	.io_window = test_io_window,
diff --git a/user/main.c b/user/main.c
--- a/user/main.c
+++ b/user/main.c
@@ -389,8 +389,8 @@ static struct kvm_callbacks test_callbac
 	.outb        = test_outb,
 	.outw        = test_outw,
 	.outl        = test_outl,
-	.mmio_read   = test_mem_read,
-	.mmio_write  = test_mem_write,
+	.mmio_read_le = test_mem_read,
+	.mmio_write_le = test_mem_write,
 	.debug       = test_debug,
 	.halt        = test_halt,
 	.io_window = test_io_window,


-- 
Hollis Blanchard
IBM Linux Technology Center


-------------------------------------------------------------------------
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace