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

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

[Guy Zana]

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

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 <guy@neocleus.com>

[-- Attachment #2: conf.patch --]
[-- Type: application/octet-stream, Size: 2104 bytes --]

--- a/Config.mk	Sat May 05 13:48:05 2007 +0100
+++ b/Config.mk	Thu May 31 21:05:07 2007 +0300
@@ -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
--- a/tools/Rules.mk	Sat May 05 13:48:05 2007 +0100
+++ b/tools/Rules.mk	Thu May 31 21:04:53 2007 +0300
@@ -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)
--- a/xen/Rules.mk	Sat May 05 13:48:05 2007 +0100
+++ b/xen/Rules.mk	Thu May 31 21:04:53 2007 +0300
@@ -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)

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

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [RFC][PATCH 1/6] HVM PCI Passthrough (non-IOMMU)

[Muli Ben-Yehuda]

On Thu, May 31, 2007 at 07:04:40PM -0400, Guy Zana wrote:
> 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 <guy@neocleus.com>
> 
> --- a/Config.mk	Sat May 05 13:48:05 2007 +0100
> +++ b/Config.mk	Thu May 31 21:05:07 2007 +0300
> @@ -1,7 +1,11 @@
>  # -*- mode: Makefile; -*-
>  
>  # A debug build of Xen and tools?
> -debug ?= n
> +debug ?= n 

Whitespace damage.

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

Does one make sense without the other?


>  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?

>  endif
>  
>  CONFIG_$(XEN_OS) := y
> --- a/tools/Rules.mk	Sat May 05 13:48:05 2007 +0100
> +++ b/tools/Rules.mk	Thu May 31 21:04:53 2007 +0300
> @@ -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.

> +
> +ifeq ($(mapping_1to1),y)
> +    CFLAGS += -DCONFIG_1TO1
> +endif
>  
>  # Enable implicit LFS support *and* explicit LFS names.
>  CFLAGS  += $(shell getconf LFS_CFLAGS)
> --- a/xen/Rules.mk	Sat May 05 13:48:05 2007 +0100
> +++ b/xen/Rules.mk	Thu May 31 21:04:53 2007 +0300
> @@ -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

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

Cheers,
Muli

RE: [RFC][PATCH 1/6] HVM PCI Passthrough (non-IOMMU)

[Guy Zana]

 

> -----Original Message-----
> From: Muli Ben-Yehuda [mailto:muli@il.ibm.com] 
> Sent: Monday, June 04, 2007 9:26 PM
> To: Guy Zana
> Cc: xen-devel@lists.xensource.com
> Subject: Re: [Xen-devel] [RFC][PATCH 1/6] HVM PCI Passthrough 
> (non-IOMMU)
> 
> > +
> > +# 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...

> 
> 
> >  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?

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...

> 
> >  endif
> >  
> >  CONFIG_$(XEN_OS) := y
> > --- a/tools/Rules.mk	Sat May 05 13:48:05 2007 +0100
> > +++ b/tools/Rules.mk	Thu May 31 21:04:53 2007 +0300
> > @@ -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.

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

Thanks,
Guy.

Re: [RFC][PATCH 1/6] HVM PCI Passthrough (non-IOMMU)

[Muli Ben-Yehuda]

On Mon, Jun 04, 2007 at 05:43:02PM -0400, Guy Zana wrote:
>  
> 
> > -----Original Message-----
> > From: Muli Ben-Yehuda [mailto:muli@il.ibm.com] 
> > Sent: Monday, June 04, 2007 9:26 PM
> > To: Guy Zana
> > Cc: xen-devel@lists.xensource.com
> > Subject: Re: [Xen-devel] [RFC][PATCH 1/6] HVM PCI Passthrough 
> > (non-IOMMU)
> > 
> > > +
> > > +# 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...

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.

> > >  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?
> 
> 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).

> > >  endif
> > >  
> > >  CONFIG_$(XEN_OS) := y
> > > --- a/tools/Rules.mk	Sat May 05 13:48:05 2007 +0100
> > > +++ b/tools/Rules.mk	Thu May 31 21:04:53 2007 +0300
> > > @@ -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.
> 
> Hehe :)
> It'll be removed completely.
> The 1:1 enabled layout will be controlled by a boot parameter.

Ok, good.

Cheers,
Muli

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

[Ben Guthro]

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

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 <sofsthun@virtualiron.com>


[-- Attachment #2: qemu-word-tearing.patch --]
[-- Type: text/x-patch, Size: 2008 bytes --]

diff -r 86fa3e4277f6 tools/ioemu/target-i386-dm/exec-dm.c
--- a/tools/ioemu/target-i386-dm/exec-dm.c	Mon Jun 04 11:26:30 2007 -0400
+++ b/tools/ioemu/target-i386-dm/exec-dm.c	Mon Jun 04 11:29:03 2007 -0400
@@ -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); 

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

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

[PATCH] Fix vnc port offset in xenstore

[Ben Guthro]

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

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 <bguthro@virtualiron.com>

[-- Attachment #2: vnc-port-offset.patch --]
[-- Type: text/x-patch, Size: 1066 bytes --]

diff -r a5cca7226e8a tools/ioemu/patches/xenstore-write-vnc-port
--- a/tools/ioemu/patches/xenstore-write-vnc-port	Tue Jun 05 13:32:38 2007 -0400
+++ b/tools/ioemu/patches/xenstore-write-vnc-port	Wed Jun 06 10:19:00 2007 -0400
@@ -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)
diff -r a5cca7226e8a tools/ioemu/xenstore.c
--- a/tools/ioemu/xenstore.c	Tue Jun 05 13:32:38 2007 -0400
+++ b/tools/ioemu/xenstore.c	Wed Jun 06 10:17:08 2007 -0400
@@ -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)

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

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

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

[Ben Guthro]

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

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

Signed-off-by: David Lively <dlively@virtualiron.com>

[-- Attachment #2: xenbus-drivers-64bit-build-fix.patch --]
[-- Type: text/x-patch, Size: 650 bytes --]

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
@@ -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;
 

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

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

[PATCH] [HVM] Allow mkbuildtree to override XEN and XL

[Ben Guthro]

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

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 <bguthro@virtualiron.com>


[-- Attachment #2: mkbuildtree.patch --]
[-- Type: text/x-patch, Size: 613 bytes --]

diff -r d4a0706d6747 unmodified_drivers/linux-2.6/mkbuildtree
--- a/unmodified_drivers/linux-2.6/mkbuildtree	Tue May 15 15:14:02 2007 +0100
+++ b/unmodified_drivers/linux-2.6/mkbuildtree	Fri May 18 07:48:18 2007 -0400
@@ -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

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

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

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

[Jan Beulich]

>>> Ben Guthro <bguthro@virtualiron.com> 06.06.07 18:08 >>>
>xenbus-drivers-64bit-build-fix.patch:
>allow unmodified drivers to build under 64 bits
>
>Signed-off-by: David Lively <dlively@virtualiron.com>
>
>-#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

How can this be correct on x86-64?

Jan

[PATCH] [QEMU] remove .depend file on clean build

[Ben Guthro]

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

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

Signed-off-by: Boris Ostrovsky <bostrovsky@virtualiron.com>

[-- Attachment #2: qemu-build-dependency.patch --]
[-- Type: text/x-patch, Size: 458 bytes --]

diff -r 6a486f395f92 tools/ioemu/Makefile.target
--- a/tools/ioemu/Makefile.target	Tue Jun 05 07:45:18 2007 -0400
+++ b/tools/ioemu/Makefile.target	Tue Jun 05 07:46:14 2007 -0400
@@ -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

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

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

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

[Keir Fraser]

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

 -- Keir


On 6/6/07 17:08, "Ben Guthro" <bguthro@virtualiron.com> wrote:

> xenbus-drivers-64bit-build-fix.patch:
> allow unmodified drivers to build under 64 bits
> 
> Signed-off-by: David Lively <dlively@virtualiron.com>
> 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
> @@ -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
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

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

[Dave Lively]

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

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
>= 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

On 6/6/07, Keir Fraser <keir@xensource.com> wrote:
> This makes no sense. x86/64 drivers build fine for me as is.
>
>  -- Keir
>
>
> On 6/6/07 17:08, "Ben Guthro" <bguthro@virtualiron.com> wrote:
>
> > xenbus-drivers-64bit-build-fix.patch:
> > allow unmodified drivers to build under 64 bits
> >
> > Signed-off-by: David Lively <dlively@virtualiron.com>
> > 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
> > @@ -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
> > Xen-devel@lists.xensource.com
> > http://lists.xensource.com/xen-devel
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
>

[-- Attachment #2: xenbus-drivers-64bit-build-fix.patch --]
[-- Type: text/x-patch, Size: 540 bytes --]

diff -r bd3d6b4c52ec 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	Fri Jun 01 14:50:52 2007 +0100
+++ b/linux-2.6-xen-sparse/include/asm-i386/mach-xen/asm/hypervisor.h	Wed Jun 06 14:21:34 2007 -0400
@@ -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;

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

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

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

[Keir Fraser]

On 6/6/07 19:42, "Dave Lively" <dlively@virtualiron.com> wrote:

> 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

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

[David Lively]

Keir Fraser wrote:

>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 ....

>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

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

[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).

 -- Keir

On 6/6/07 17:00, "Ben Guthro" <bguthro@virtualiron.com> wrote:

> 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 <sofsthun@virtualiron.com>
> 
> diff -r 86fa3e4277f6 tools/ioemu/target-i386-dm/exec-dm.c
> --- a/tools/ioemu/target-i386-dm/exec-dm.c Mon Jun 04 11:26:30 2007 -0400
> +++ b/tools/ioemu/target-i386-dm/exec-dm.c Mon Jun 04 11:29:03 2007 -0400
> @@ -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
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

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

[Steve Ofsthun]

Keir Fraser wrote:
> 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
> 
>  -- Keir
> 
> On 6/6/07 17:00, "Ben Guthro" <bguthro@virtualiron.com> wrote:
> 
>> 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 <sofsthun@virtualiron.com>
>>
>> diff -r 86fa3e4277f6 tools/ioemu/target-i386-dm/exec-dm.c
>> --- a/tools/ioemu/target-i386-dm/exec-dm.c Mon Jun 04 11:26:30 2007 -0400
>> +++ b/tools/ioemu/target-i386-dm/exec-dm.c Mon Jun 04 11:29:03 2007 -0400
>> @@ -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
>> Xen-devel@lists.xensource.com
>> http://lists.xensource.com/xen-devel
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel


-- 
Steve Ofsthun - Virtual Iron Software, Inc.

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

[Keir Fraser]

On 6/6/07 18:03, "Steve Ofsthun" <sofsthun@virtualiron.com> wrote:

>> 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