HVM PCI Passthrough (non-IOMMU)

RFC PATCH In review Guy Zana 7 participants 17 messages May 31, 2007 – Jun 6, 2007

lore

Share & tools

lore

Guy Zana

author

2007-05-31 23:04 UTC #0 lore raw

RFC PATCH 1/6 HVM PCI Passthrough (non-IOMMU)

conf.patch:

- Compile in x86_32 (disable PAE)
- Additional compilation switches (found in Config.mk):
mapping_1to1 - enables the 1:1 layout that allows DMA passthrough - enables non-IOMMU pass-through

Signed-off-by Guy Zana <[email protected]>

Config.mk +6 -2

@@ -1,7 +1,11 @@
 # -*- mode: Makefile; -*-
 
 # A debug build of Xen and tools?
-debug ?= n
+debug ?= n 
+
+# PCI pass-through flags 
+mapping_1to1 ?= y
+passthrough ?= y
 
 XEN_COMPILE_ARCH    ?= $(shell uname -m | sed -e s/i.86/x86_32/ \
                          -e s/ppc/powerpc/ -e s/i86pc/x86_32/)

@@ -9,7 +13,7 @@ XEN_OS              ?= $(shell uname -s)
 XEN_OS              ?= $(shell uname -s)
 
 ifeq ($(XEN_TARGET_ARCH),x86_32)
-XEN_TARGET_X86_PAE  ?= y
+XEN_TARGET_X86_PAE  ?= n 
 endif
 
 CONFIG_$(XEN_OS) := y

tools/Rules.mk +8 -0

@@ -13,6 +13,14 @@ X11_LDPATH = -L/usr/X11R6/$(LIBDIR)
 X11_LDPATH = -L/usr/X11R6/$(LIBDIR)
 
 CFLAGS += -D__XEN_TOOLS__
+
+ifeq ($(passthrough),y)
+    CFLAGS += -DCONFIG_NEO
+endif
+
+ifeq ($(mapping_1to1),y)
+    CFLAGS += -DCONFIG_1TO1
+endif
 
 # Enable implicit LFS support *and* explicit LFS names.
 CFLAGS  += $(shell getconf LFS_CFLAGS)

xen/Rules.mk +10 -6

@@ -50,12 +50,16 @@ ALL_OBJS-$(ACM_SECURITY) += $(BASEDIR)/a
 ALL_OBJS-$(ACM_SECURITY) += $(BASEDIR)/acm/built_in.o
 ALL_OBJS-y               += $(BASEDIR)/arch/$(TARGET_ARCH)/built_in.o
 
-CFLAGS-y               += -g -D__XEN__
-CFLAGS-$(ACM_SECURITY) += -DACM_SECURITY
-CFLAGS-$(verbose)      += -DVERBOSE
-CFLAGS-$(crash_debug)  += -DCRASH_DEBUG
-CFLAGS-$(perfc)        += -DPERF_COUNTERS
-CFLAGS-$(perfc_arrays) += -DPERF_ARRAYS
+CFLAGS-y                        += -g -D__XEN__
+# NEO stuff
+CFLAGS-$(passthrough)           += -DCONFIG_NEO
+CFLAGS-$(mapping_1to1)          += -DCONFIG_1TO1
+
+CFLAGS-$(ACM_SECURITY)          += -DACM_SECURITY
+CFLAGS-$(verbose)               += -DVERBOSE
+CFLAGS-$(crash_debug)           += -DCRASH_DEBUG
+CFLAGS-$(perfc)                 += -DPERF_COUNTERS
+CFLAGS-$(perfc_arrays)          += -DPERF_ARRAYS
 
 ifneq ($(max_phys_cpus),)
 CFLAGS-y               += -DMAX_PHYS_CPUS=$(max_phys_cpus)

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

Muli Ben-Yehuda

2007-06-04 18:25 UTC #1 lore raw

Quoted 12 lines · Guy Zana

conf.patch:
- Compile in x86_32 (disable PAE)
- Additional compilation switches (found in Config.mk):
mapping_1to1 - enables the 1:1 layout that allows DMA passthrough - enables non-IOMMU pass-through
Signed-off-by Guy Zana <[email protected]>

Config.mk line 4

Quoted 5 lines

Config.mk +1 -1

@@ -1,7 +1,11 @@
 # -*- mode: Makefile; -*-
 
 # A debug build of Xen and tools?
-debug ?= n
+debug ?= n

Whitespace damage.

Config.mk line 8

Quoted 4 lines

Config.mk +4 -0

+
+# PCI pass-through flags 
+mapping_1to1 ?= y
+passthrough ?= y

Does one make sense without the other?

Config.mk XEN_OS ?= $(shell uname -s) · line 16

Quoted 8 lines

Config.mk +1 -1

 XEN_COMPILE_ARCH    ?= $(shell uname -m | sed -e s/i.86/x86_32/ \
                          -e s/ppc/powerpc/ -e s/i86pc/x86_32/)

@@ -9,7 +13,7 @@ XEN_OS              ?= $(shell uname -s)
 XEN_OS              ?= $(shell uname -s)
 
 ifeq ($(XEN_TARGET_ARCH),x86_32)
-XEN_TARGET_X86_PAE  ?= y
+XEN_TARGET_X86_PAE  ?= n

Is there a dependency between 1-1 / passthrough and !PAE, or is this just for convenience?

tools/Rules.mk X11_LDPATH = -L/usr/X11R6/$(LIBDIR) · line 19

Quoted 13 lines

Config.mk +0 -0

 endif
 
 CONFIG_$(XEN_OS) := y

tools/Rules.mk +4 -0

@@ -13,6 +13,14 @@ X11_LDPATH = -L/usr/X11R6/$(LIBDIR)
 X11_LDPATH = -L/usr/X11R6/$(LIBDIR)
 
 CFLAGS += -D__XEN_TOOLS__
+
+ifeq ($(passthrough),y)
+    CFLAGS += -DCONFIG_NEO
+endif

You probably want a better name than CONFIG_NEO, much to the disappointment of matrix fans everywhere.

Quoted 19 lines

tools/Rules.mk +4 -0

+
+ifeq ($(mapping_1to1),y)
+    CFLAGS += -DCONFIG_1TO1
+endif
 
 # Enable implicit LFS support *and* explicit LFS names.
 CFLAGS  += $(shell getconf LFS_CFLAGS)

xen/Rules.mk +0 -6

@@ -50,12 +50,16 @@ ALL_OBJS-$(ACM_SECURITY) += $(BASEDIR)/a
 ALL_OBJS-$(ACM_SECURITY) += $(BASEDIR)/acm/built_in.o
 ALL_OBJS-y               += $(BASEDIR)/arch/$(TARGET_ARCH)/built_in.o
 
-CFLAGS-y               += -g -D__XEN__
-CFLAGS-$(ACM_SECURITY) += -DACM_SECURITY
-CFLAGS-$(verbose)      += -DVERBOSE
-CFLAGS-$(crash_debug)  += -DCRASH_DEBUG
-CFLAGS-$(perfc)        += -DPERF_COUNTERS
-CFLAGS-$(perfc_arrays) += -DPERF_ARRAYS

xen/Rules.mk ALL_OBJS-$(ACM_SECURITY) += $(BASEDIR)/a · line 62

Quoted 10 lines

xen/Rules.mk +10 -0

+CFLAGS-y                        += -g -D__XEN__
+# NEO stuff
+CFLAGS-$(passthrough)           += -DCONFIG_NEO
+CFLAGS-$(mapping_1to1)          += -DCONFIG_1TO1
+
+CFLAGS-$(ACM_SECURITY)          += -DACM_SECURITY
+CFLAGS-$(verbose)               += -DVERBOSE
+CFLAGS-$(crash_debug)           += -DCRASH_DEBUG
+CFLAGS-$(perfc)                 += -DPERF_COUNTERS
+CFLAGS-$(perfc_arrays)          += -DPERF_ARRAYS

This should be changed to only touch the new bits, makes it easier to review.

Cheers,
Muli

Guy Zana

author

2007-06-04 21:43 UTC #2 lore raw

Quoted 10 lines

-----Original Message-----
From: Muli Ben-Yehuda [mailto:[email protected]]
Sent: Monday, June 04, 2007 9:26 PM
To: Guy Zana
Cc: [email protected]
Subject: Re: [Xen-devel] [RFC][PATCH 1/6] HVM PCI Passthrough (non-IOMMU)

Quoted 6 lines

Config.mk line 8
Quoted 4 lines
Config.mk +4 -0
+
+# PCI pass-through flags
+mapping_1to1 ?= y
+passthrough ?= y
Does one make sense without the other?

Actually yes, you'll need the 1:1 layout only when using devices that needs DMA. Just the passthorugh switch could be turned on for other devices...

Quoted 7 lines


 XEN_COMPILE_ARCH    ?= $(shell uname -m | sed -e s/i.86/x86_32/ \
                          -e s/ppc/powerpc/ -e s/i86pc/x86_32/)

Config.mk +0 -0

@@ -9,7 +13,7 @@ XEN_OS              ?= $(shell uname -s)
 XEN_OS              ?= $(shell uname -s)

Quoted 6 lines

Config.mk XEN_OS ?= $(shell uname -s) · line 16
Quoted 3 lines
Config.mk +1 -1
 ifeq ($(XEN_TARGET_ARCH),x86_32)
-XEN_TARGET_X86_PAE  ?= y
+XEN_TARGET_X86_PAE  ?= n
Is there a dependency between 1-1 / passthrough and !PAE, or is this just for convenience?

There is a dependency, not that it's that a problem to support PAE, we just didn't implement it yet.
We decided to support x86_64 and remove the x86_32 / PAE support, as we see it, there is just no need to support x86_32 / PAE anymore...

Quoted 10 lines

tools/Rules.mk X11_LDPATH = -L/usr/X11R6/$(LIBDIR)
Quoted 7 lines
Config.mk +0 -0
 endif
 
 CONFIG_$(XEN_OS) := y
tools/Rules.mk +0 -0
@@ -13,6 +13,14 @@ X11_LDPATH = -L/usr/X11R6/$(LIBDIR)  
X11_LDPATH =
Quoted 2 lines
tools/Rules.mk +0 -1
-L/usr/X11R6/$(LIBDIR)
 

Quoted 8 lines

tools/Rules.mk X11_LDPATH = -L/usr/X11R6/$(LIBDIR) · line 19
Quoted 5 lines
tools/Rules.mk +4 -0
 CFLAGS += -D__XEN_TOOLS__
+
+ifeq ($(passthrough),y)
+    CFLAGS += -DCONFIG_NEO
+endif
You probably want a better name than CONFIG_NEO, much to the disappointment of matrix fans everywhere.

Hehe :)
It'll be removed completely.
The 1:1 enabled layout will be controlled by a boot parameter.

Thanks,
Guy.

Muli Ben-Yehuda

2007-06-05 06:37 UTC #3 lore raw

Quoted 17 lines · Guy Zana

Quoted 10 lines
-----Original Message-----
From: Muli Ben-Yehuda [mailto:[email protected]]
Sent: Monday, June 04, 2007 9:26 PM
To: Guy Zana
Cc: [email protected]
Subject: Re: [Xen-devel] [RFC][PATCH 1/6] HVM PCI Passthrough (non-IOMMU)
Quoted 6 lines
Config.mk line 8
Quoted 4 lines
Config.mk +4 -0
+
+# PCI pass-through flags
+mapping_1to1 ?= y
+passthrough ?= y
Does one make sense without the other?

Quoted 3 lines

Actually yes, you'll need the 1:1 layout only when using devices that needs DMA. Just the passthorugh switch could be turned on for other devices...

I think we take it as a given that passthrough (whether through 1:1 or IOMMU support) will be compiled into Xen, so no need for a separate flag.

Quoted 12 lines

Quoted 11 lines
Config.mk XEN_OS ?= $(shell uname -s) · line 16
Quoted 8 lines
 XEN_COMPILE_ARCH    ?= $(shell uname -m | sed -e s/i.86/x86_32/ \
                          -e s/ppc/powerpc/ -e s/i86pc/x86_32/)
Config.mk +1 -1
@@ -9,7 +13,7 @@ XEN_OS              ?= $(shell uname -s)
 XEN_OS              ?= $(shell uname -s)
 
 ifeq ($(XEN_TARGET_ARCH),x86_32)
-XEN_TARGET_X86_PAE  ?= y
+XEN_TARGET_X86_PAE  ?= n
Is there a dependency between 1-1 / passthrough and !PAE, or is this just for convenience?

Quoted 4 lines

There is a dependency, not that it's that a problem to support PAE, we just didn't implement it yet. We decided to support x86_64 and remove the x86_32 / PAE support, as we see it, there is just no need to support x86_32 / PAE anymore...

Ok, just for reference, if there's a dependency in the future, it should be expressed that way (only turn on PAE if 1:1 is on, in this case).

Quoted 18 lines

Quoted 9 lines
tools/Rules.mk X11_LDPATH = -L/usr/X11R6/$(LIBDIR)
Quoted 6 lines
Config.mk +0 -0
 endif
 
 CONFIG_$(XEN_OS) := y
tools/Rules.mk +0 -0
@@ -13,6 +13,14 @@ X11_LDPATH = -L/usr/X11R6/$(LIBDIR)  
X11_LDPATH =
Quoted 2 lines
tools/Rules.mk +0 -1
-L/usr/X11R6/$(LIBDIR)
 
Quoted 8 lines
tools/Rules.mk X11_LDPATH = -L/usr/X11R6/$(LIBDIR) · line 19
Quoted 5 lines
tools/Rules.mk +4 -0
 CFLAGS += -D__XEN_TOOLS__
+
+ifeq ($(passthrough),y)
+    CFLAGS += -DCONFIG_NEO
+endif
You probably want a better name than CONFIG_NEO, much to the disappointment of matrix fans everywhere.

Quoted 3 lines

Hehe :)
It'll be removed completely.
The 1:1 enabled layout will be controlled by a boot parameter.

Ok, good.

Cheers,
Muli

Ben Guthro

2007-06-06 16:00 UTC #4 lore raw

PATCH 1/1 Prevent usb driver crashes in Windows

In reply to Guy Zana

qemu-word-tearing.patch:
Use atomic updates to read/write usb controller data. This can be done because:
a) word copies on x86 are atomic
b) The USB spec requires word alignment

This will need to be enhanced once USB 1.2 is supported.

Signed-off-by Steve Ofsthun <[email protected]>

tools/ioemu/target-i386-dm/exec-dm.c +24 -2

diff -r 86fa3e4277f6 tools/ioemu/target-i386-dm/exec-dm.c
@@ -434,6 +434,28 @@ extern unsigned long *logdirty_bitmap;
 extern unsigned long *logdirty_bitmap;
 extern unsigned long logdirty_bitmap_size;
 
+/*
+ * Replace the standard byte memcpy with a int memcpy for appropriately sized
+ * memory copy operations.  Some users (USB-UHCI) can not tolerate the possible
+ * word tearing that can result from a guest concurrently writing a memory
+ * structure while the qemu device model is modifying the same location.
+ * Forcing a int sized read/write prevents the guest from seeing a partially
+ * written int sized atom.
+ */
+void memcpy32(void *dst, void *src, size_t n)
+{
+    if ((n % sizeof(int)) != 0) {
+        memcpy(dst, src, n);
+        return;
+    }
+    n /= sizeof(int);
+    while(n--) {
+        *(int *)dst = *(int *)src;
+        dst += sizeof(int);
+        src += sizeof(int);
+    }
+}
+
 void cpu_physical_memory_rw(target_phys_addr_t addr, uint8_t *buf, 
                             int len, int is_write)
 {

@@ -470,7 +492,7 @@ void cpu_physical_memory_rw(target_phys_
                 }
             } else if ((ptr = phys_ram_addr(addr)) != NULL) {
                 /* Writing to RAM */
-                memcpy(ptr, buf, l);
+                memcpy32(ptr, buf, l);
                 if (logdirty_bitmap != NULL) {
                     /* Record that we have dirtied this frame */
                     unsigned long pfn = addr >> TARGET_PAGE_BITS;

@@ -506,7 +528,7 @@ void cpu_physical_memory_rw(target_phys_
                 }
             } else if ((ptr = phys_ram_addr(addr)) != NULL) {
                 /* Reading from RAM */
-                memcpy(buf, ptr, l);
+                memcpy32(buf, ptr, l);
             } else {
                 /* Neither RAM nor known MMIO space */
                 memset(buf, 0xff, len);

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

Ben Guthro

2007-06-06 16:05 UTC #5 lore raw

PATCH 1/1 Fix vnc port offset in xenstore

vnc-port-offset.patch:
The new version of qemu uses 5900 instead of 0 as the port.
There is no need to offset it by 5900 when writing out the console/vnc-port entry.

Signed-off-by Ben Guthro <[email protected]>

tools/ioemu/patches/xenstore-write-vnc-port +1 -1

diff -r a5cca7226e8a tools/ioemu/patches/xenstore-write-vnc-port
@@ -23,7 +23,7 @@ Index: ioemu/xenstore.c
 +    if (pasprintf(&buf, "%s/console/vnc-port", path) == -1)
 +        goto out;
 +
-+    if (pasprintf(&portstr, "%d", 5900 + display) == -1)
++    if (pasprintf(&portstr, "%d", display) == -1)
 +        goto out;
 +
 +    if (xs_write(xsh, XBT_NULL, buf, portstr, strlen(portstr)) == 0)

tools/ioemu/xenstore.c +1 -1

diff -r a5cca7226e8a tools/ioemu/xenstore.c
@@ -367,7 +367,7 @@ void xenstore_write_vncport(int display)
     if (pasprintf(&buf, "%s/console/vnc-port", path) == -1)
         goto out;
 
-    if (pasprintf(&portstr, "%d", 5900 + display) == -1)
+    if (pasprintf(&portstr, "%d", display) == -1)
         goto out;
 
     if (xs_write(xsh, XBT_NULL, buf, portstr, strlen(portstr)) == 0)

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

Ben Guthro

2007-06-06 16:08 UTC #6 lore raw

PATCH 1/1 Fix 64 bit PV-on-HVM driver builds

xenbus-drivers-64bit-build-fix.patch:
allow unmodified drivers to build under 64 bits

Signed-off-by David Lively <[email protected]>

linux-2.6-xen-sparse/include/asm-i386/mach-xen/asm/hypervisor.h +3 -1

diff -r d4a0706d6747 linux-2.6-xen-sparse/include/asm-i386/mach-xen/asm/hypervisor.h
@@ -45,13 +45,15 @@
 #include <xen/interface/nmi.h>
 #include <asm/ptrace.h>
 #include <asm/page.h>
-#if defined(__i386__)
+
+#if !defined(CONFIG_XEN) || defined(__i386__)
 #  ifdef CONFIG_X86_PAE
 #   include <asm-generic/pgtable-nopud.h>
 #  else
 #   include <asm-generic/pgtable-nopmd.h>
 #  endif
 #endif
+
 
 extern shared_info_t *HYPERVISOR_shared_info;

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

Ben Guthro

2007-06-06 16:12 UTC #7 lore raw

PATCH 1/1 Allow mkbuildtree to override XEN and XL

mkbuildtree.patch
Previous versions of mkbuildtree allowed XL to be overridden. With
the introduction of the XEN variable, this functonality was removed. This patch gives the flexibility to override XL, and XEN on the command line.

Signed-off-by Ben Guthro <[email protected]>

unmodified_drivers/linux-2.6/mkbuildtree +11 -2

diff -r d4a0706d6747 unmodified_drivers/linux-2.6/mkbuildtree
@@ -10,8 +10,17 @@ fi
 
 C=$PWD
 
-XEN=$C/../../xen
-XL=$C/../../linux-2.6-xen-sparse
+if [ -n "$XEN" -a -d "$XEN" ]; then
+	XEN=$(cd $XEN && pwd)
+else
+	XEN=$C/../../xen
+fi
+
+if [ -n "$XL" -a -d "$XL" ]; then
+	XL=$(cd $XL && pwd)
+else
+	XL=$C/../../linux-2.6-xen-sparse
+fi
 
 for d in $(find ${XL}/drivers/xen/ -maxdepth 1 -type d | sed -e 1d); do
     if ! echo $d | egrep -q back; then

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

Jan Beulich

2007-06-06 16:17 UTC #8 lore raw

Re: [PATCH] [HVM] Fix 64 bit PV-on-HVM driver builds

In reply to Ben Guthro

Quoted 9 lines

Quoted 1 line
Quoted 1 line
Ben Guthro <[email protected]> 06.06.07 18:08 >>>
xenbus-drivers-64bit-build-fix.patch:
allow unmodified drivers to build under 64 bits
Signed-off-by David Lively <[email protected]>
linux-2.6-xen-sparse/include/asm-i386/mach-xen/asm/hypervisor.h +2 -1
-#if defined(__i386__)
+
+#if !defined(CONFIG_XEN) || defined(__i386__)

Quoted 6 lines

# ifdef CONFIG_X86_PAE
# include <asm-generic/pgtable-nopud.h>
# else
# include <asm-generic/pgtable-nopmd.h>
# endif
#endif

How can this be correct on x86-64?

Jan

Ben Guthro

2007-06-06 16:26 UTC #9 lore raw

PATCH 1/1 remove .depend file on clean build

qemu-build-dependency.patch:
Ensure .depend files are cleaned up properly for a "make clean"

Signed-off-by Boris Ostrovsky <[email protected]>

tools/ioemu/Makefile.target +1 -1

diff -r 6a486f395f92 tools/ioemu/Makefile.target
@@ -609,7 +609,7 @@ endif
 	$(CC) $(CPPFLAGS) -c -o $@ $<
 
 clean:
-	rm -f *.o  *.a *~ $(PROGS) gen-op.h opc.h op.h nwfpe/*.o slirp/*.o fpu/*.o
+	rm -f *.o  *.a *~ $(PROGS) gen-op.h opc.h op.h nwfpe/*.o slirp/*.o fpu/*.o .depend
 
 distclean: clean
 	rm -rf config.mak config.h

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

Keir Fraser

2007-06-06 16:38 UTC #10 lore raw

Re: [PATCH] [HVM] Fix 64 bit PV-on-HVM driver builds

In reply to Ben Guthro

This makes no sense. x86/64 drivers build fine for me as is.

-- Keir

Quoted 31 lines · Ben Guthro

xenbus-drivers-64bit-build-fix.patch:
allow unmodified drivers to build under 64 bits
Signed-off-by David Lively <[email protected]>
diff -r d4a0706d6747 linux-2.6-xen-sparse/include/asm-i386/mach-xen/asm/hypervisor.h --- a/linux-2.6-xen-sparse/include/asm-i386/mach-xen/asm/hypervisor.h Tue May 15 15:14:02 2007 +0100
+++ b/linux-2.6-xen-sparse/include/asm-i386/mach-xen/asm/hypervisor.h Thu May 17 15:01:35 2007 -0400
linux-2.6-xen-sparse/include/asm-i386/mach-xen/asm/hypervisor.h +3 -1
@@ -45,13 +45,15 @@
 #include <xen/interface/nmi.h>
 #include <asm/ptrace.h>
 #include <asm/page.h>
-#if defined(__i386__)
+
+#if !defined(CONFIG_XEN) || defined(__i386__)
 #  ifdef CONFIG_X86_PAE
 #   include <asm-generic/pgtable-nopud.h>
 #  else
 #   include <asm-generic/pgtable-nopmd.h>
 #  endif
 #endif
+
 
 extern shared_info_t *HYPERVISOR_shared_info;
 
_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

Dave Lively

2007-06-06 18:42 UTC #11 lore raw

Re: [PATCH] [HVM] Fix 64 bit PV-on-HVM driver builds

After further investigation, I see that the PV-on-HVM driver build was broken only for x86_64 linux kernels PRIOR TO 2.6.11 (i.e., without a pud_t typedef). And my original patch breaks the build with kernels

Quoted 1 line

= 2.6.11. I had mistakenly thought it was a matter of CONFIG_XEN or

not. The attached new version of this patch should work with all (2.6) kernels. I now test LINUX_VERSION_CODE instead of CONFIG_XEN.

I've also separated this x86_64 case from the i386 case. We end up including the same file (unmodified_drivers/linux-2.6/compat-include/asm-generic/pgtable-nopud.h) in either case, but it was pretty confusing the way I had it mixed in with the i386 case.

Tested building against 2.6.5, 2.6.9, and 2.6.18 guest kernels.

Dave

Quoted 34 lines · Keir Fraser

This makes no sense. x86/64 drivers build fine for me as is.
-- Keir
Quoted 27 lines · Ben Guthro
xenbus-drivers-64bit-build-fix.patch:
allow unmodified drivers to build under 64 bits
Signed-off-by David Lively <[email protected]>
diff -r d4a0706d6747 linux-2.6-xen-sparse/include/asm-i386/mach-xen/asm/hypervisor.h --- a/linux-2.6-xen-sparse/include/asm-i386/mach-xen/asm/hypervisor.h Tue May 15 15:14:02 2007 +0100
+++ b/linux-2.6-xen-sparse/include/asm-i386/mach-xen/asm/hypervisor.h Thu May 17 15:01:35 2007 -0400
(quoted diff) +3 -1
@@ -45,13 +45,15 @@
 #include <xen/interface/nmi.h>
 #include <asm/ptrace.h>
 #include <asm/page.h>
-#if defined(__i386__)
+
+#if !defined(CONFIG_XEN) || defined(__i386__)
 #  ifdef CONFIG_X86_PAE
 #   include <asm-generic/pgtable-nopud.h>
 #  else
 #   include <asm-generic/pgtable-nopmd.h>
 #  endif
 #endif
+
 
 extern shared_info_t *HYPERVISOR_shared_info;
 

Quoted 11 lines

Quoted 4 lines
_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel
_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

linux-2.6-xen-sparse/include/asm-i386/mach-xen/asm/hypervisor.h +2 -0

diff -r bd3d6b4c52ec linux-2.6-xen-sparse/include/asm-i386/mach-xen/asm/hypervisor.h
@@ -51,6 +51,8 @@
 #  else
 #   include <asm-generic/pgtable-nopmd.h>
 #  endif
+#elif defined(__x86_64__) && LINUX_VERSION_CODE < KERNEL_VERSION(2,6,11)
+#  include <asm-generic/pgtable-nopmd.h>
 #endif
 
 extern shared_info_t *HYPERVISOR_shared_info;

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

Keir Fraser

2007-06-06 18:57 UTC #12 lore raw

Re: [PATCH] [HVM] Fix 64 bit PV-on-HVM driver builds

Quoted 5 lines · Dave Lively

I've also separated this x86_64 case from the i386 case. We end up including the same file (unmodified_drivers/linux-2.6/compat-include/asm-generic/pgtable-nopud.h) in either case, but it was pretty confusing the way I had it mixed in with the i386 case.

In the patch it's pgtable-nopmd.h. But pgtable-nopud.h makes more sense (x86/64 Linux has always had proper pmds). So is the patch incorrect?

Also, this could be placed inside asm-x86_64/mach-xen/asm/hypervisor.h, couldn't it?

-- Keir

David Lively

2007-06-06 19:20 UTC #13 lore raw

Re: [PATCH] [HVM] Fix 64 bit PV-on-HVM driver builds

Quoted 4 lines · Keir Fraser

In the patch it's pgtable-nopmd.h. But pgtable-nopud.h makes more sense (x86/64 Linux has always had proper pmds). So is the patch incorrect?

Oops. You're absolutely right. Both define pud_t, but nopud.h is definitely the correct one. Thanks for catching this ....

Quoted 7 lines

Also, this could be placed inside asm-x86_64/mach-xen/asm/hypervisor.h, couldn't it?
-- Keir

I tried that. But since the x86_64 hypervisor.h includes the i386 hypervisor.h, I have to put the conditional include of pgtable-nopud.h either before or after the include of i386 hypervisor.h, neither of which compiles on pre-2.6.11 kernels.

Dave

Keir Fraser

2007-06-06 16:40 UTC #14 lore raw

Re: [PATCH] [HVM] Prevent usb driver crashes in Windows

In reply to Ben Guthro

What precisely is the race? Qemu-dm is single threaded. Memcpy() probably ends up doing int copies anyway if things are appropriately aligned (although we might not want to rely on that).

-- Keir

Quoted 65 lines · Ben Guthro

qemu-word-tearing.patch:
Use atomic updates to read/write usb controller data. This can be done because:
a) word copies on x86 are atomic
b) The USB spec requires word alignment
This will need to be enhanced once USB 1.2 is supported.
Signed-off-by Steve Ofsthun <[email protected]>
diff -r 86fa3e4277f6 tools/ioemu/target-i386-dm/exec-dm.c
tools/ioemu/target-i386-dm/exec-dm.c Mon Jun 04 11:29:03 2007 -0400 renamed from tools/ioemu/target-i386-dm/exec-dm.c Mon Jun 04 11:26:30 2007 -0400 +3 -0
@@ -434,6 +434,28 @@ extern unsigned long *logdirty_bitmap;
 extern unsigned long *logdirty_bitmap;
 extern unsigned long logdirty_bitmap_size;
 
+/*
+ * Replace the standard byte memcpy with a int memcpy for appropriately sized
+ * memory copy operations.  Some users (USB-UHCI) can not tolerate the
possible
+ * word tearing that can result from a guest concurrently writing a memory
+ * structure while the qemu device model is modifying the same location.
+ * Forcing a int sized read/write prevents the guest from seeing a partially
+ * written int sized atom.
+ */
tools/ioemu/target-i386-dm/exec-dm.c Mon Jun 04 11:29:03 2007 -0400 renamed from tools/ioemu/target-i386-dm/exec-dm.c Mon Jun 04 11:26:30 2007 -0400 +16 -2
+void memcpy32(void *dst, void *src, size_t n)
+{
+    if ((n % sizeof(int)) != 0) {
+        memcpy(dst, src, n);
+        return;
+    }
+    n /= sizeof(int);
+    while(n--) {
+        *(int *)dst = *(int *)src;
+        dst += sizeof(int);
+        src += sizeof(int);
+    }
+}
+
 void cpu_physical_memory_rw(target_phys_addr_t addr, uint8_t *buf,
                             int len, int is_write)
 {
@@ -470,7 +492,7 @@ void cpu_physical_memory_rw(target_phys_
                 }
             } else if ((ptr = phys_ram_addr(addr)) != NULL) {
                 /* Writing to RAM */
-                memcpy(ptr, buf, l);
+                memcpy32(ptr, buf, l);
                 if (logdirty_bitmap != NULL) {
                     /* Record that we have dirtied this frame */
                     unsigned long pfn = addr >> TARGET_PAGE_BITS;
@@ -506,7 +528,7 @@ void cpu_physical_memory_rw(target_phys_
                 }
             } else if ((ptr = phys_ram_addr(addr)) != NULL) {
                 /* Reading from RAM */
-                memcpy(buf, ptr, l);
+                memcpy32(buf, ptr, l);
             } else {
                 /* Neither RAM nor known MMIO space */
                 memset(buf, 0xff, len);
_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

Steve Ofsthun

2007-06-06 17:03 UTC #15 lore raw

Re: [PATCH] [HVM] Prevent usb driver crashes in Windows

Quoted 3 lines · Keir Fraser

What precisely is the race? Qemu-dm is single threaded. Memcpy() probably ends up doing int copies anyway if things are appropriately aligned (although we might not want to rely on that).

The race is between a domU guest cpu running guest driver code and a dom0 cpu running the timer based USB emulation code in QEMU.

A Windows domU driver is accessing and modifying the USB controller data simultaneously with the USB controller emulation in QEMU. The controller data consists of linked lists/queues that a real hardware USB controller would access/modify atomically. The USB emulation in QEMU was updating queue/list entries with cpu_physical_memory_rw() which ultimately calls memcpy. This allowed the windows USB subsystem to see partial updates to physical addresses in these lists/queues that were not stored there by Windows. Windows responded to these transient "data corruptions" with a BSOD. We generally see this when stacking many Windows guests that are using a usb mouse/tablet pointing device.

The attached patch simply uses word copies when alignment and size are appropriate.

Steve

Quoted 76 lines

-- Keir
Quoted 61 lines · Ben Guthro
qemu-word-tearing.patch:
Use atomic updates to read/write usb controller data. This can be done because:
a) word copies on x86 are atomic
b) The USB spec requires word alignment
This will need to be enhanced once USB 1.2 is supported.
Signed-off-by Steve Ofsthun <[email protected]>
diff -r 86fa3e4277f6 tools/ioemu/target-i386-dm/exec-dm.c
tools/ioemu/target-i386-dm/exec-dm.c Mon Jun 04 11:29:03 2007 -0400 renamed from tools/ioemu/target-i386-dm/exec-dm.c Mon Jun 04 11:26:30 2007 -0400 +3 -0
@@ -434,6 +434,28 @@ extern unsigned long *logdirty_bitmap;
 extern unsigned long *logdirty_bitmap;
 extern unsigned long logdirty_bitmap_size;
 
+/*
+ * Replace the standard byte memcpy with a int memcpy for appropriately sized
+ * memory copy operations.  Some users (USB-UHCI) can not tolerate the
possible
+ * word tearing that can result from a guest concurrently writing a memory
+ * structure while the qemu device model is modifying the same location.
+ * Forcing a int sized read/write prevents the guest from seeing a partially
+ * written int sized atom.
+ */
tools/ioemu/target-i386-dm/exec-dm.c Mon Jun 04 11:29:03 2007 -0400 renamed from tools/ioemu/target-i386-dm/exec-dm.c Mon Jun 04 11:26:30 2007 -0400 +16 -2
+void memcpy32(void *dst, void *src, size_t n)
+{
+    if ((n % sizeof(int)) != 0) {
+        memcpy(dst, src, n);
+        return;
+    }
+    n /= sizeof(int);
+    while(n--) {
+        *(int *)dst = *(int *)src;
+        dst += sizeof(int);
+        src += sizeof(int);
+    }
+}
+
 void cpu_physical_memory_rw(target_phys_addr_t addr, uint8_t *buf,
                             int len, int is_write)
 {
@@ -470,7 +492,7 @@ void cpu_physical_memory_rw(target_phys_
                 }
             } else if ((ptr = phys_ram_addr(addr)) != NULL) {
                 /* Writing to RAM */
-                memcpy(ptr, buf, l);
+                memcpy32(ptr, buf, l);
                 if (logdirty_bitmap != NULL) {
                     /* Record that we have dirtied this frame */
                     unsigned long pfn = addr >> TARGET_PAGE_BITS;
@@ -506,7 +528,7 @@ void cpu_physical_memory_rw(target_phys_
                 }
             } else if ((ptr = phys_ram_addr(addr)) != NULL) {
                 /* Reading from RAM */
-                memcpy(buf, ptr, l);
+                memcpy32(buf, ptr, l);
             } else {
                 /* Neither RAM nor known MMIO space */
                 memset(buf, 0xff, len);
Quoted 4 lines
_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel
_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

Signature · 1 line

Steve Ofsthun - Virtual Iron Software, Inc.

Keir Fraser

2007-06-06 18:22 UTC #16 lore raw

Re: [PATCH] [HVM] Prevent usb driver crashes in Windows

Quoted 6 lines · Steve Ofsthun

Quoted 3 lines
What precisely is the race? Qemu-dm is single threaded. Memcpy() probably ends up doing int copies anyway if things are appropriately aligned (although we might not want to rely on that).
The race is between a domU guest cpu running guest driver code and a dom0 cpu running the timer based USB emulation code in QEMU.

I see. Thanks!

-- Keir