thr3ads.net - Xen devel - [Xen-devel] Missing checks for xc_map_foreign

If this information is useful, please help other people find it:
Share via:

Markus Armbruster

2007-Aug-31 14:06 UTC

[Xen-devel] Missing checks for xc_map_foreign_range() failure

privcmd_ioctl() case IOCTL_PRIVCMD_MMAPBATCH has two different ways to
indicate failure:

1. The usual way: do nothing and return error code.

2. An unusual way: set the most significant four bits of those
   elements of the mfn array argument that could not be mapped, and
   return 0.

   When the array isn''t writable, the error information is simply
   discarded.

xc_map_foreign_range() behaves exactly the same.  Its second failure
mode isn''t documented anywhere I can see.  Doc bug.

Some users of xc_map_foreign_range() search the array for failed mfns.
Others don''t.  All check the return value.

There might be uses for the second failure mode of
xc_map_foreign_range() which justify this awkward API.
qemu_remap_bucket() looks like such a case.

Other, more naive uses would profit from a less awkward convenience
function that either succeeds completely or fails completely.  As far
as I can see, these uses all neglect to check for the second failure
mode so far.  Potential crash bugs.

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

Keir Fraser

2007-Aug-31 14:11 UTC

head link

Re: [Xen-devel] Missing checks for xc_map_foreign_range() failure

On 31/8/07 15:06, "Markus Armbruster" <armbru@redhat.com> wrote:
> Other, more naive uses would profit from a less awkward convenience
> function that either succeeds completely or fails completely.  As far
> as I can see, these uses all neglect to check for the second failure
> mode so far.  Potential crash bugs.
Yes, that makes sense. Are you proposing to make a patch?

 -- Keir



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

Markus Armbruster

2007-Aug-31 15:03 UTC

head link

Re: [Xen-devel] Missing checks for xc_map_foreign_range() failure

Keir Fraser <Keir.Fraser@cl.cam.ac.uk> writes:
> On 31/8/07 15:06, "Markus Armbruster" <armbru@redhat.com>
wrote:
>
>> Other, more naive uses would profit from a less awkward convenience
>> function that either succeeds completely or fails completely.  As far
>> as I can see, these uses all neglect to check for the second failure
>> mode so far.  Potential crash bugs.
>
> Yes, that makes sense. Are you proposing to make a patch?
Happy to do that :)

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

Markus Armbruster

2007-Sep-05 16:36 UTC

head link

[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@redhat.com>

---
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@lists.xensource.com
http://lists.xensource.com/xen-devel

Tian, Kevin

2007-Sep-06 07:44 UTC

head link

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

Just curious why this can''t be fixed in kernel driver? Is there case
that
caller is tolerable with partial mapping?

Also why do you need an extra allocation and copy to arr?

Thanks,
Kevin
>From: Markus Armbruster
>Sent: 2007年9月6日 0:37
>
>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@redhat.com>
>
>---
>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@lists.xensource.com
>http://lists.xensource.com/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Keir Fraser

2007-Sep-06 08:47 UTC

head link

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

On 6/9/07 08:44, "Tian, Kevin" <kevin.tian@intel.com> wrote:
> Just curious why this can''t be fixed in kernel driver? Is there
case that
> caller is tolerable with partial mapping?
We would need an extra command in the kernel driver, as the current
partial-success semantics are used appropriately by some callers. Doing it
in libxc avoids requiring a kernel upgrade to fix this issue.

 -- Keir



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

Tian, Kevin

2007-Sep-06 08:49 UTC

head link

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

>From: Keir Fraser [mailto:Keir.Fraser@cl.cam.ac.uk]
>Sent: 2007年9月6日 16:47
>
>On 6/9/07 08:44, "Tian, Kevin" <kevin.tian@intel.com> wrote:
>
>> Just curious why this can''t be fixed in kernel driver? Is
there case that
>> caller is tolerable with partial mapping?
>
>We would need an extra command in the kernel driver, as the current
>partial-success semantics are used appropriately by some callers. Doing
>it
>in libxc avoids requiring a kernel upgrade to fix this issue.
>
> -- Keir
Interesting... any example I can learn? Yes, compatibility is one concern.

Thanks,
Kevin

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

Markus Armbruster

2007-Sep-06 11:20 UTC

head link

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

"Tian, Kevin" <kevin.tian@intel.com> writes:
>>From: Keir Fraser [mailto:Keir.Fraser@cl.cam.ac.uk]
>>Sent: 2007年9月6日 16:47
>>
>>On 6/9/07 08:44, "Tian, Kevin" <kevin.tian@intel.com>
wrote:
>>
>>> Just curious why this can''t be fixed in kernel driver? Is
there case that
>>> caller is tolerable with partial mapping?
>>
>>We would need an extra command in the kernel driver, as the current
>>partial-success semantics are used appropriately by some callers. Doing
>>it
>>in libxc avoids requiring a kernel upgrade to fix this issue.
>>
>> -- Keir
>
> Interesting... any example I can learn? Yes, compatibility is one concern.
>
> Thanks,
> Kevin
Have a look at qemu_remap_bucket() in tools/ioemu/vl.c.

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

Markus Armbruster

2007-Sep-06 11:24 UTC

head link

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

"Tian, Kevin" <kevin.tian@intel.com> writes:

[...]> Also why do you need an extra allocation and copy to arr?
To avoid clobbering the arr argument, which I feel would be a rather
bad API.

[...]

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

Isaku Yamahata

2007-Sep-10 02:03 UTC

head link

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

On Wed, Sep 05, 2007 at 06:36:54PM +0200, Markus Armbruster
wrote:> 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:
...> * main() in tools/ioemu/vl.c, in __ia64__ code
> 
>   Doesn''t test for mapping failure.  Likely candidate for
>   xc_map_foreign_pages().
This can''t be replaced because some pages, e.g. VGA area [0xA0000,
0xC0000),
can''t be mapped.

> * 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.
This can be replaced with xc_map_foreign_pages()
This change will go through xen-ia64-devel.hg.
I''ll send the patch to xen-ia64-devel.

> * 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().
This can be replaced with xc_map_foreign_pages().

-- 
yamahata

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

Markus Armbruster

2007-Sep-10 14:13 UTC

head link

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

Isaku Yamahata <yamahata@valinux.co.jp> writes:
> On Wed, Sep 05, 2007 at 06:36:54PM +0200, Markus Armbruster wrote:
>> 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:
> ...
>> * main() in tools/ioemu/vl.c, in __ia64__ code
>> 
>>   Doesn''t test for mapping failure.  Likely candidate for
>>   xc_map_foreign_pages().
>
> This can''t be replaced because some pages, e.g. VGA area [0xA0000,
0xC0000),
> can''t be mapped.
Is it okay not to check for success?
>> * 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.
>
> This can be replaced with xc_map_foreign_pages()
> This change will go through xen-ia64-devel.hg.
> I''ll send the patch to xen-ia64-devel.
Thanks!
>> * 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().
>
> This can be replaced with xc_map_foreign_pages().
Obvious patch for that:

diff -r 9071521d4864 tools/libxc/xc_core_x86.c
--- a/tools/libxc/xc_core_x86.c	Fri Sep 07 11:39:10 2007 +0100
+++ b/tools/libxc/xc_core_x86.c	Mon Sep 10 15:26:26 2007 +0200
@@ -86,7 +86,7 @@ xc_core_arch_map_p2m(int xc_handle, xc_d
     }
 
     live_p2m_frame_list -        xc_map_foreign_batch(xc_handle, dom,
PROT_READ,
+        xc_map_foreign_pages(xc_handle, dom, PROT_READ,
                              live_p2m_frame_list_list,
                              P2M_FLL_ENTRIES);
 
@@ -96,7 +96,7 @@ xc_core_arch_map_p2m(int xc_handle, xc_d
         goto out;
     }
 
-    *live_p2m = xc_map_foreign_batch(xc_handle, dom, PROT_READ,
+    *live_p2m = xc_map_foreign_pages(xc_handle, dom, PROT_READ,
                                     live_p2m_frame_list,
                                     P2M_FL_ENTRIES);
 

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

tgh

2008-Jan-24 02:17 UTC

head link

Re: [Xen-devel] Missing checks for xc_map_foreign_range() failure

hi
what is the function of the xc_map_foreign_range(), what does it work 
for ? could you give me someexplanation or example about it , i 
encounter it in the code of xc_domain_save(), and confused about the 
function of it,could you help me

Thanks in advance




Markus Armbruster 写道:> Keir Fraser <Keir.Fraser@cl.cam.ac.uk> writes:
>
>   
>> On 31/8/07 15:06, "Markus Armbruster"
<armbru@redhat.com> wrote:
>>
>>     
>>> Other, more naive uses would profit from a less awkward convenience
>>> function that either succeeds completely or fails completely.  As
far
>>> as I can see, these uses all neglect to check for the second
failure
>>> mode so far.  Potential crash bugs.
>>>       
>> Yes, that makes sense. Are you proposing to make a patch?
>>     
>
> Happy to do that :)
>
> _______________________________________________
> 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

Markus Armbruster

2008-Jan-24 09:52 UTC

head link

Re: [Xen-devel] Missing checks for xc_map_foreign_range() failure

tgh <wwwwww4187@sina.com.cn> writes:
> hi
> what is the function of the xc_map_foreign_range(), what does it work
> for ? could you give me someexplanation or example about it , i
> encounter it in the code of xc_domain_save(), and confused about the
> function of it,could you help me
>
> Thanks in advance
xc_map_foreign_{range,pages,batch}() let you map another domain''s
memory, similar to mmap().

xc_map_foreign_range() is the simplest of them: it maps a range of
memory starting at a mfn.  You typically map one page that way,
i.e. you pass PAGE_SIZE as size argument.

xc_domain_save() uses it to map the domain''s shared info page. 
That''s
a well-known page, and its mfn can be found in the domain''s info
obtained from xc_domain_getinfo().

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

Xen devel - Aug 2007 - Missing checks for xc_map_foreign_range() failure

[Xen-devel] Missing checks for xc_map_foreign_range() failure

Re: [Xen-devel] Missing checks for xc_map_foreign_range() failure

Re: [Xen-devel] Missing checks for xc_map_foreign_range() failure

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

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

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

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

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

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

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

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

Re: [Xen-devel] Missing checks for xc_map_foreign_range() failure

Re: [Xen-devel] Missing checks for xc_map_foreign_range() failure