[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Xen-devel] [PATCH] xc_map_foreign_pages(), a convenient alternative to xc_map_foreign_batch()



xc_map_foreign_batch() can succeed partially.  It is awkward to use
when you're only interested in complete success.  Provide new
xc_map_foreign_pages() convenience function for that kind of use.
Also convert two obvious calls to use it.

Signed-off-by: Markus Armbruster <armbru@xxxxxxxxxx>

---
The patch converts only those uses of xc_map_foreign_batch() to
xc_map_foreign_pages() where I'm 100% sure it's safe.  For the others,
I'd rather have a second opinion from somebody familiar with the code.

Discussion of uses:

* qemu_remap_bucket() in tools/ioemu/vl.c

  Tests for mapping failure, although in a slightly sloppy way:

      !(pfns[i + --j] & 0xF0000000UL)

  This is true when any of the four bits is set.  But mapping failed
  only when all of them are set, so maybe it should do this instead:

      (pfns[i + --j] & 0xF0000000UL) != 0xF0000000UL

* main() in tools/ioemu/vl.c, in __ia64__ code

  Doesn't test for mapping failure.  Likely candidate for
  xc_map_foreign_pages().

* set_vram_mapping() in tools/ioemu/hw/cirrus_vga.c

  Doesn't test for mapping failure.  Patch converts this one to
  xc_map_foreign_pages().

* copy_from_GFW_to_nvram() in tools/libxc/ia64/xc_ia64_hvm_build.c

  Doesn't test for mapping failure.  Likely candidate for
  xc_map_foreign_pages(), but I have no way to test it.

* xc_core_arch_map_p2m() in tools/libxc/xc_core_x86.c

  This maps a two level page directory: first a a page of pfns for the
  page directory (live_p2m_frame_list_list), using that pages of pfns
  for the pages themselves (live_p2m_frame_list), and using that the
  actual pages.

  Doesn't test for mapping failure.  Likely candidate for
  xc_map_foreign_pages().

* xc_domain_restore() in tools/libxc/xc_domain_restore.c

  Three uses, none of them tests for mapping failure.

  I don't understand this code.

  The first use of xc_map_foreign_batch() appears to deliberately pass
  invalid PFNs.

* map_and_save_p2m_table() in tools/libxc/xc_domain_save.c

  Similar to xc_core_arch_map_p2m().

* xc_domain_save() in tools/libxc/xc_domain_save.c

  Doesn't test for mapping failure.  Likely candidate for
  xc_map_foreign_pages().

* xc_domain_resume_any() in tools/libxc/xc_resume.c

  Similar to xc_core_arch_map_p2m().

* xenfb_map_fb() in tools/xenfb/xenfb.c

  Another map through a two level page directory, roughly similar to
  xc_core_arch_map_p2m().

  Doesn't test for mapping failure.  Patch converts this one to
  xc_map_foreign_pages().


diff -r 256160ff19b7 tools/ioemu/hw/cirrus_vga.c
--- a/tools/ioemu/hw/cirrus_vga.c       Thu Aug 16 13:27:59 2007 +0100
+++ b/tools/ioemu/hw/cirrus_vga.c       Wed Sep 05 17:38:50 2007 +0200
@@ -2561,7 +2561,7 @@ static void *set_vram_mapping(unsigned l
 
     set_mm_mapping(xc_handle, domid, nr_extents, 0, extent_start);
 
-    vram_pointer = xc_map_foreign_batch(xc_handle, domid,
+    vram_pointer = xc_map_foreign_pages(xc_handle, domid,
                                         PROT_READ|PROT_WRITE,
                                         extent_start, nr_extents);
     if (vram_pointer == NULL) {
diff -r 256160ff19b7 tools/libxc/xc_misc.c
--- a/tools/libxc/xc_misc.c     Thu Aug 16 13:27:59 2007 +0100
+++ b/tools/libxc/xc_misc.c     Wed Sep 05 18:31:46 2007 +0200
@@ -224,6 +224,39 @@ int xc_hvm_set_pci_link_route(
     unlock_pages(&arg, sizeof(arg));
 
     return rc;
+}
+
+void *xc_map_foreign_pages(int xc_handle, uint32_t dom, int prot,
+                           const xen_pfn_t *arr, int num)
+{
+    xen_pfn_t *pfn;
+    void *res;
+    int i;
+
+    pfn = malloc(num * sizeof(*pfn));
+    if (!pfn)
+        return NULL;
+    memcpy(pfn, arr, num * sizeof(*pfn));
+
+    res = xc_map_foreign_batch(xc_handle, dom, prot, pfn, num);
+    if (res) {
+        for (i = 0; i < num; i++) {
+            if ((pfn[i] & 0xF0000000UL) == 0xF0000000UL) {
+                /*
+                 * xc_map_foreign_batch() doesn't give us an error
+                 * code, so we have to make one up.  May not be the
+                 * appropriate one.
+                 */
+                errno = EINVAL;
+                munmap(res, num * PAGE_SIZE);
+                res = NULL;
+                break;
+            }
+        }
+    }
+
+    free(pfn);
+    return res;
 }
 
 /*
diff -r 256160ff19b7 tools/libxc/xenctrl.h
--- a/tools/libxc/xenctrl.h     Thu Aug 16 13:27:59 2007 +0100
+++ b/tools/libxc/xenctrl.h     Mon Sep 03 09:18:50 2007 +0200
@@ -646,6 +646,14 @@ void *xc_map_foreign_range(int xc_handle
                             int size, int prot,
                             unsigned long mfn );
 
+void *xc_map_foreign_pages(int xc_handle, uint32_t dom, int prot,
+                           const xen_pfn_t *arr, int num );
+
+/**
+ * Like xc_map_foreign_pages(), except it can succeeed partially.
+ * When a page cannot be mapped, its PFN in @arr is or'ed with
+ * 0xF0000000 to indicate the error.
+ */
 void *xc_map_foreign_batch(int xc_handle, uint32_t dom, int prot,
                            xen_pfn_t *arr, int num );
 
diff -r 256160ff19b7 tools/xenfb/xenfb.c
--- a/tools/xenfb/xenfb.c       Thu Aug 16 13:27:59 2007 +0100
+++ b/tools/xenfb/xenfb.c       Wed Sep 05 17:39:05 2007 +0200
@@ -398,21 +398,15 @@ static int xenfb_map_fb(struct xenfb_pri
        if (!pgmfns || !fbmfns)
                goto out;
 
-       /*
-        * Bug alert: xc_map_foreign_batch() can fail partly and
-        * return a non-null value.  This is a design flaw.  When it
-        * happens, we happily continue here, and later crash on
-        * access.
-        */
        xenfb_copy_mfns(mode, n_fbdirs, pgmfns, pd);
-       map = xc_map_foreign_batch(xenfb->xc, domid,
+       map = xc_map_foreign_pages(xenfb->xc, domid,
                                   PROT_READ, pgmfns, n_fbdirs);
        if (map == NULL)
                goto out;
        xenfb_copy_mfns(mode, n_fbmfns, fbmfns, map);
        munmap(map, n_fbdirs * XC_PAGE_SIZE);
 
-       xenfb->pub.pixels = xc_map_foreign_batch(xenfb->xc, domid,
+       xenfb->pub.pixels = xc_map_foreign_pages(xenfb->xc, domid,
                                PROT_READ | PROT_WRITE, fbmfns, n_fbmfns);
        if (xenfb->pub.pixels == NULL)
                goto out;

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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.