WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

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

To: Keir Fraser <Keir.Fraser@xxxxxxxxxxxx>
Subject: [Xen-devel] [PATCH] xc_map_foreign_pages(), a convenient alternative to xc_map_foreign_batch()
From: Markus Armbruster <armbru@xxxxxxxxxx>
Date: Wed, 05 Sep 2007 18:36:54 +0200
Cc: xen-devel@xxxxxxxxxxxxxxxxxxx
Delivery-date: Wed, 05 Sep 2007 09:37:24 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxx
In-reply-to: <C2FDE025.1517A%Keir.Fraser@xxxxxxxxxxxx> (Keir Fraser's message of "Fri\, 31 Aug 2007 15\:11\:33 +0100")
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <C2FDE025.1517A%Keir.Fraser@xxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: Gnus/5.11 (Gnus v5.11) Emacs/22.1 (gnu/linux)
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