[Xen-devel] 32-on-64: pvfb issue

[Gerd Hoffmann <kraxel@xxxxxxx>]

  Hi,

There is a problem with the virtual framebuffer: The page directory in
the shared page (xenfb_page->pd[]) is unsigned long and thus has
different sizes on 32bit and 64bit.  The alignment is different too. And
on top of that the frontend driver doesn't clear the shared page, which
makes it difficult to autodetect the bitsize.  I've tried nevertheless,
patch (untested!) attached for comments.  In the long run this code is
supposed to be replaced by grant tables anyway, so it is probably okay
to live with the hack for the time being.  We could of course also fix
the struct if we can afford breaking the interface.  As it is quite new
and probably (hmm, does fc6 ship it?) not widely used yet that might be
an option.

cheers,
  Gerd

--- build-32-unstable-13401/tools/xenfb/xenfb.c-13401   2007-01-18 
11:35:53.000000000 +0100
+++ build-32-unstable-13401/tools/xenfb/xenfb.c 2007-01-18 12:30:44.000000000 
+0100
@@ -329,12 +329,59 @@
        struct xenfb_page *page = xenfb->fb.page;
        int n_fbmfns;
        int n_fbdirs;
+       unsigned long pgmfns[2];
        unsigned long *fbmfns;
+       uint32_t *ptr32;
+       uint64_t *ptr64;
+       int i,mode;
+
+/*
+ * bimodal guess work.
+ * page->depth is at a 64bit border, thus we get for page->pg[] ...
+ *
+ *  i386    | depth | 3b pad |  pg0  |  pg1  |  pg2  |  pg3  |  pg4  |
+ *  x86_64  | depth | 7b pad         |      pg0      |      pg1      |
+ *
+ * frontend does *not* clear the page.  pg0 and pg1 are initialized.
+ * at the moment pg0 is used only (800x600x32 needs less than 512 pages,
+ * which fits into one page directory even with 64bit), pg1 is initialized
+ * to zero.  We also assume the machine has less than 16 TB main memory,
+ * thus 32bit are enougth for the page frame numbers.  By the time this
+ * assumtion is rendered invalid the code should have been converted to
+ * grant tables anyway.
+ */
+#if defined(__i386__)
+       mode = 32;
+       ptr32 = (void*)page->pd;
+       ptr64 = ((void*)ptr32) + 4;
+       if (ptr32[1] > 0 &&
+           ptr32[2] == 0 && 
+           ptr32[3] == 0 &&
+           ptr32[4] == 0)
+               mode = 64;
+#elif defined(__x86_64__)
+       mode = 64;
+       ptr64 = (void*)page->pd;
+       ptr32 = ((void*)ptr64) - 4;
+       if (ptr32[0] > 0 &&
+           ptr32[1] == 0)
+               mode = 32;
+#else
+# error unknown arch
+#endif
 
        n_fbmfns = (xenfb->fb_len + (XC_PAGE_SIZE - 1)) / XC_PAGE_SIZE;
-       n_fbdirs = n_fbmfns * sizeof(unsigned long);
+       n_fbdirs = n_fbmfns * mode / 4;
        n_fbdirs = (n_fbdirs + (XC_PAGE_SIZE - 1)) / XC_PAGE_SIZE;
 
+       if (32 == mode) {
+               for (i = 0; i < n_fbdirs; i++)
+                       pgmfns[i] = ptr32[i];
+       } else {
+               for (i = 0; i < n_fbdirs; i++)
+                       pgmfns[i] = ptr64[i];
+       }
+
        /*
         * Bug alert: xc_map_foreign_batch() can fail partly and
         * return a non-null value.  This is a design flaw.  When it
@@ -342,18 +389,31 @@
         * access.
         */
        fbmfns = xc_map_foreign_batch(xenfb->xc, domid,
-                       PROT_READ, page->pd, n_fbdirs);
+                       PROT_READ, pgmfns, n_fbdirs);
        if (fbmfns == NULL)
                return -1;
 
+       if (32 == mode) {
+               ptr32 = (void*)fbmfns;
+               fbmfns = malloc(n_fbmfns * sizeof(*fbmfns));
+               for (i = 0; i < n_fbmfns; i++)
+                       fbmfns[i] = ptr32[i];
+               munmap(ptr32, n_fbdirs * XC_PAGE_SIZE);
+       } else {
+               ptr64 = (void*)fbmfns;
+               fbmfns = malloc(n_fbmfns * sizeof(*fbmfns));
+               for (i = 0; i < n_fbmfns; i++)
+                       fbmfns[i] = ptr64[i];
+               munmap(ptr64, n_fbdirs * XC_PAGE_SIZE);
+       }
+
        xenfb->pub.pixels = xc_map_foreign_batch(xenfb->xc, domid,
                                PROT_READ | PROT_WRITE, fbmfns, n_fbmfns);
-       if (xenfb->pub.pixels == NULL) {
-               munmap(fbmfns, n_fbdirs * XC_PAGE_SIZE);
+       if (xenfb->pub.pixels == NULL)
                return -1;
-       }
 
-       return munmap(fbmfns, n_fbdirs * XC_PAGE_SIZE);
+       free(fbmfns);
+       return 0;
 }
 
 static int xenfb_bind(struct xenfb_device *dev)

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