Re: [PATCH][TOOLS] libxc: mmap fixes for BSD

[Christoph Egger <Christoph.Egger@amd.com>]

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

On Tuesday 10 June 2008 16:21:56 Keir Fraser wrote:
> On 10/6/08 15:09, "Christoph Egger" <Christoph.Egger@amd.com> wrote:
> > On BSD, mmap()ing files works on on-disk files but not on
> > pseudo filesystems like kernfs or procfs.
> > Therefore, attached patch mmap()s anonymous memory.
> >
> > Linux equivalents are sysfs and procfs to above BSD's pseudo-filesystems.
> > On Linux, mmap is implemented for sysfs but it is questionable to me
> > how write operations work since (most) files in sysfs have
> > static content. So this patch may fix some problems
> > with a Linux Dom0, too.
>
> We can hide special setup in mmap(/proc/xen/privcmd) that is necessary for
> later foreign mapping operations. Perhaps xc_map_foreign_ranges() interface
> could be changed so that it does the mmap(), then fills in the .va field in
> the array of entries that it is passed, and finally return the address it
> got from mmap()?
>
> This would make xc_map_foreign_ranges() similar to our other xc_map*()
> functions, which all hide the mmap() invocation inside their
> implementation.
>
>  -- Keir

Here is the patch. This also cleans up and fixes memory leaks in the
error path.

Christoph

-- 
AMD Saxony, Dresden, Germany
Operating System Research Center

Legal Information:
AMD Saxony Limited Liability Company & Co. KG
Sitz (Geschäftsanschrift):
   Wilschdorfer Landstr. 101, 01109 Dresden, Deutschland
Registergericht Dresden: HRA 4896
vertretungsberechtigter Komplementär:
   AMD Saxony LLC (Sitz Wilmington, Delaware, USA)
Geschäftsführer der AMD Saxony LLC:
   Dr. Hans-R. Deppe, Thomas McCoy

[-- Attachment #2: libxc_mmap.diff --]
[-- Type: text/x-diff, Size: 11911 bytes --]

diff -r f86941c1b523 tools/libxc/xc_dom_boot.c
--- a/tools/libxc/xc_dom_boot.c	Tue Jul 22 16:03:45 2008 +0100
+++ b/tools/libxc/xc_dom_boot.c	Wed Jul 23 16:55:27 2008 +0200
@@ -153,7 +153,7 @@ void *xc_dom_boot_domU_map(struct xc_dom
     int page_shift = XC_DOM_PAGE_SHIFT(dom);
     privcmd_mmap_entry_t *entries;
     void *ptr;
-    int i, rc;
+    int i;
     int err;
 
     entries = xc_dom_malloc(dom, count * sizeof(privcmd_mmap_entry_t));
@@ -165,9 +165,13 @@ void *xc_dom_boot_domU_map(struct xc_dom
         return NULL;
     }
 
-    ptr = mmap(NULL, count << page_shift, PROT_READ | PROT_WRITE,
-               MAP_SHARED, dom->guest_xc, 0);
-    if ( ptr == MAP_FAILED )
+    for ( i = 0; i < count; i++ )
+        entries[i].mfn = xc_dom_p2m_host(dom, pfn + i);
+
+    ptr = xc_map_foreign_ranges(dom->guest_xc, dom->guest_domid,
+                count << page_shift, PROT_READ | PROT_WRITE, 1 << page_shift,
+                entries, count);
+    if ( ptr == NULL )
     {
         err = errno;
         xc_dom_panic(XC_INTERNAL_ERROR,
@@ -177,22 +181,6 @@ void *xc_dom_boot_domU_map(struct xc_dom
         return NULL;
     }
 
-    for ( i = 0; i < count; i++ )
-    {
-        entries[i].va = (uintptr_t) ptr + (i << page_shift);
-        entries[i].mfn = xc_dom_p2m_host(dom, pfn + i);
-        entries[i].npages = 1;
-    }
-
-    rc = xc_map_foreign_ranges(dom->guest_xc, dom->guest_domid,
-                               entries, count);
-    if ( rc < 0 )
-    {
-        xc_dom_panic(XC_INTERNAL_ERROR,
-                     "%s: failed to mmap domU pages 0x%" PRIpfn "+0x%" PRIpfn
-                     " [xenctl, rc=%d]\n", __FUNCTION__, pfn, count, rc);
-        return NULL;
-    }
     return ptr;
 }
 
diff -r f86941c1b523 tools/libxc/xc_domain_save.c
--- a/tools/libxc/xc_domain_save.c	Tue Jul 22 16:03:45 2008 +0100
+++ b/tools/libxc/xc_domain_save.c	Wed Jul 23 16:55:27 2008 +0200
@@ -568,16 +568,19 @@ static xen_pfn_t *xc_map_m2p(int xc_hand
     unsigned long m2p_chunks, m2p_size;
     xen_pfn_t *m2p;
     xen_pfn_t *extent_start;
-    int i, rc;
+    int i;
 
+    m2p = NULL;
     m2p_size   = M2P_SIZE(max_mfn);
     m2p_chunks = M2P_CHUNKS(max_mfn);
 
     xmml.max_extents = m2p_chunks;
-    if ( !(extent_start = malloc(m2p_chunks * sizeof(xen_pfn_t))) )
+
+    extent_start = calloc(m2p_chunks, sizeof(xen_pfn_t));
+    if ( !extent_start )
     {
         ERROR("failed to allocate space for m2p mfns");
-        return NULL;
+        goto err0;
     }
     set_xen_guest_handle(xmml.extent_start, extent_start);
 
@@ -585,41 +588,36 @@ static xen_pfn_t *xc_map_m2p(int xc_hand
          (xmml.nr_extents != m2p_chunks) )
     {
         ERROR("xc_get_m2p_mfns");
-        return NULL;
+        goto err1;
     }
 
-    if ( (m2p = mmap(NULL, m2p_size, prot,
-                     MAP_SHARED, xc_handle, 0)) == MAP_FAILED )
-    {
-        ERROR("failed to mmap m2p");
-        return NULL;
-    }
-
-    if ( !(entries = malloc(m2p_chunks * sizeof(privcmd_mmap_entry_t))) )
+    entries = calloc(m2p_chunks, sizeof(privcmd_mmap_entry_t));
+    if (entries == NULL)
     {
         ERROR("failed to allocate space for mmap entries");
-        return NULL;
+        goto err1;
     }
 
     for ( i = 0; i < m2p_chunks; i++ )
+        entries[i].mfn = extent_start[i];
+
+    m2p = xc_map_foreign_ranges(xc_handle, DOMID_XEN,
+			m2p_size, prot, M2P_CHUNK_SIZE,
+			entries, m2p_chunks);
+    if (m2p == NULL)
     {
-        entries[i].va = (unsigned long)(((void *)m2p) + (i * M2P_CHUNK_SIZE));
-        entries[i].mfn = extent_start[i];
-        entries[i].npages = M2P_CHUNK_SIZE >> PAGE_SHIFT;
-    }
-
-    if ( (rc = xc_map_foreign_ranges(xc_handle, DOMID_XEN,
-                                     entries, m2p_chunks)) < 0 )
-    {
-        ERROR("xc_mmap_foreign_ranges failed (rc = %d)", rc);
-        return NULL;
+        ERROR("xc_mmap_foreign_ranges failed");
+        goto err2;
     }
 
     m2p_mfn0 = entries[0].mfn;
 
+err2:
+    free(entries);
+err1:
     free(extent_start);
-    free(entries);
 
+err0:
     return m2p;
 }
 
diff -r f86941c1b523 tools/libxc/xc_hvm_build.c
--- a/tools/libxc/xc_hvm_build.c	Tue Jul 22 16:03:45 2008 +0100
+++ b/tools/libxc/xc_hvm_build.c	Wed Jul 23 16:55:27 2008 +0200
@@ -115,27 +115,21 @@ static int loadelfimage(
     struct elf_binary *elf, int xch, uint32_t dom, unsigned long *parray)
 {
     privcmd_mmap_entry_t *entries = NULL;
-    int pages = (elf->pend - elf->pstart + PAGE_SIZE - 1) >> PAGE_SHIFT;
+    size_t pages = (elf->pend - elf->pstart + PAGE_SIZE - 1) >> PAGE_SHIFT;
     int i, rc = -1;
 
     /* Map address space for initial elf image. */
-    entries = malloc(pages * sizeof(privcmd_mmap_entry_t));
+    entries = calloc(pages, sizeof(privcmd_mmap_entry_t));
     if ( entries == NULL )
-        goto err;
-    elf->dest = mmap(NULL, pages << PAGE_SHIFT, PROT_READ | PROT_WRITE,
-                     MAP_SHARED, xch, 0);
-    if ( elf->dest == MAP_FAILED )
         goto err;
 
     for ( i = 0; i < pages; i++ )
-    {
-        entries[i].va = (uintptr_t)elf->dest + (i << PAGE_SHIFT);
         entries[i].mfn = parray[(elf->pstart >> PAGE_SHIFT) + i];
-        entries[i].npages = 1;
-    }
 
-    rc = xc_map_foreign_ranges(xch, dom, entries, pages);
-    if ( rc < 0 )
+    elf->dest = xc_map_foreign_ranges(xch, dom, pages << PAGE_SHIFT,
+			PROT_READ | PROT_WRITE, 1 << PAGE_SHIFT,
+			entries, pages);
+    if (elf->dest == NULL)
         goto err;
 
     /* Load the initial elf image. */
@@ -143,12 +137,6 @@ static int loadelfimage(
     rc = 0;
 
  err:
-    if ( elf->dest )
-    {
-        munmap(elf->dest, pages << PAGE_SHIFT);
-        elf->dest = NULL;
-    }
-
     if ( entries )
         free(entries);
 
diff -r f86941c1b523 tools/libxc/xc_linux.c
--- a/tools/libxc/xc_linux.c	Tue Jul 22 16:03:45 2008 +0100
+++ b/tools/libxc/xc_linux.c	Wed Jul 23 16:55:27 2008 +0200
@@ -118,16 +118,41 @@ void *xc_map_foreign_range(int xc_handle
     return addr;
 }
 
-int xc_map_foreign_ranges(int xc_handle, uint32_t dom,
-                          privcmd_mmap_entry_t *entries, int nr)
+void *xc_map_foreign_ranges(int xc_handle, uint32_t dom,
+                            size_t size, int prot, size_t chunksize,
+                            privcmd_mmap_entry_t entries[], int nentries)
 {
     privcmd_mmap_t ioctlx;
 
-    ioctlx.num   = nr;
+    int i, rc;
+    void *addr;
+
+    addr = mmap(NULL, size, prot, MAP_SHARED, xc_handle, 0);
+    if (addr == MAP_FAILED)
+        goto mmap_failed;
+
+    for (i = 0; i < nentries; i++) {
+        entries[i].va = (uintptr_t)addr + (i * chunksize);
+        entries[i].npages = chunksize >> PAGE_SHIFT;
+    }
+
+    ioctlx.num   = nentries;
     ioctlx.dom   = dom;
     ioctlx.entry = entries;
 
-    return ioctl(xc_handle, IOCTL_PRIVCMD_MMAP, &ioctlx);
+    rc = ioctl(xc_handle, IOCTL_PRIVCMD_MMAP, &ioctlx);
+    if (rc)
+        goto ioctl_failed;
+
+    return addr;
+
+ioctl_failed:
+    rc = munmap(addr, size);
+    if (rc == -1)
+        ERROR("%s: error in error path\n", __FUNCTION__);
+
+mmap_failed:
+    return NULL;
 }
 
 static int do_privcmd(int xc_handle, unsigned int cmd, unsigned long data)
diff -r f86941c1b523 tools/libxc/xc_minios.c
--- a/tools/libxc/xc_minios.c	Tue Jul 22 16:03:45 2008 +0100
+++ b/tools/libxc/xc_minios.c	Wed Jul 23 16:55:27 2008 +0200
@@ -76,6 +76,16 @@ void *xc_map_foreign_range(int xc_handle
     return map_frames_ex(&mfn, size / getpagesize(), 0, 1, 1, dom, 0, pt_prot);
 }
 
+void *xc_map_foreign_ranges(int xc_handle, uint32_t dom,
+                            size_t size, int prot, size_t chunksize,
+                            privcmd_mmap_entry_t entries[], int nentries)
+{
+    ERROR("%s: implement me\n");
+    return NULL;
+}
+
+
+#if 0
 int xc_map_foreign_ranges(int xc_handle, uint32_t dom,
                           privcmd_mmap_entry_t *entries, int nr)
 {
@@ -86,6 +96,7 @@ int xc_map_foreign_ranges(int xc_handle,
     }
     return 0;
 }
+#endif
 
 int do_xen_hypercall(int xc_handle, privcmd_hypercall_t *hypercall)
 {
diff -r f86941c1b523 tools/libxc/xc_netbsd.c
--- a/tools/libxc/xc_netbsd.c	Tue Jul 22 16:03:45 2008 +0100
+++ b/tools/libxc/xc_netbsd.c	Wed Jul 23 16:55:27 2008 +0200
@@ -11,7 +11,6 @@
 
 #include "xc_private.h"
 
-#include <xen/memory.h>
 #include <xen/sys/evtchn.h>
 #include <unistd.h>
 #include <fcntl.h>
@@ -114,22 +113,42 @@ void *xc_map_foreign_range(int xc_handle
     return addr;
 }
 
-int xc_map_foreign_ranges(int xc_handle, uint32_t dom,
-                          privcmd_mmap_entry_t *entries, int nr)
+void *xc_map_foreign_ranges(int xc_handle, uint32_t dom,
+                            size_t size, int prot, size_t chunksize,
+                            privcmd_mmap_entry_t entries[], int nentries)
 {
-    privcmd_mmap_t ioctlx;
-    int err;
+	privcmd_mmap_t ioctlx;
+	int i, rc;
+	void *addr;
 
-    ioctlx.num   = nr;
-    ioctlx.dom   = dom;
-    ioctlx.entry = entries;
+	addr = mmap(NULL, size, prot, MAP_ANON | MAP_SHARED, -1, 0);
+	if (addr == MAP_FAILED)
+		goto mmap_failed;
 
-    err = ioctl(xc_handle, IOCTL_PRIVCMD_MMAP, &ioctlx);
-    if (err == 0)
-	return 0;
-    else
-	return -errno;
+	for (i = 0; i < nentries; i++) {
+		entries[i].va = (uintptr_t)addr + (i * chunksize);
+		entries[i].npages = chunksize >> PAGE_SHIFT;
+	}
+
+	ioctlx.num   = nentries;
+	ioctlx.dom   = dom;
+	ioctlx.entry = entries;
+
+	rc = ioctl(xc_handle, IOCTL_PRIVCMD_MMAP, &ioctlx);
+	if (rc)
+		goto ioctl_failed;
+
+	return addr;
+
+ioctl_failed:
+	rc = munmap(addr, size);
+	if (rc == -1)
+		ERROR("%s: error in error path\n", __FUNCTION__);
+
+mmap_failed:
+	return NULL;
 }
+
 
 static int do_privcmd(int xc_handle, unsigned int cmd, unsigned long data)
 {
diff -r f86941c1b523 tools/libxc/xc_private.h
--- a/tools/libxc/xc_private.h	Tue Jul 22 16:03:45 2008 +0100
+++ b/tools/libxc/xc_private.h	Wed Jul 23 16:55:27 2008 +0200
@@ -184,8 +184,9 @@ static inline int do_sysctl(int xc_handl
     return ret;
 }
 
-int xc_map_foreign_ranges(int xc_handle, uint32_t dom,
-                          privcmd_mmap_entry_t *entries, int nr);
+void *xc_map_foreign_ranges(int xc_handle, uint32_t dom,
+                            size_t size, int prot, size_t chunksize,
+                            privcmd_mmap_entry_t entries[], int nentries);
 
 void *map_domain_va_core(unsigned long domfd, int cpu, void *guest_va,
                          vcpu_guest_context_any_t *ctxt);
diff -r f86941c1b523 tools/libxc/xc_solaris.c
--- a/tools/libxc/xc_solaris.c	Tue Jul 22 16:03:45 2008 +0100
+++ b/tools/libxc/xc_solaris.c	Wed Jul 23 16:55:27 2008 +0200
@@ -109,17 +109,40 @@ void *xc_map_foreign_range(int xc_handle
     return addr;
 }
 
-int xc_map_foreign_ranges(int xc_handle, uint32_t dom,
-                          privcmd_mmap_entry_t *entries, int nr)
+void *xc_map_foreign_ranges(int xc_handle, uint32_t dom,
+                            size_t size, int prot, size_t chunksize,
+                            privcmd_mmap_entry_t entries[], int nentries)
 {
     privcmd_mmap_t ioctlx;
+    int i, rc;
+    void *addr;
 
-    ioctlx.num   = nr;
+    addr = mmap(NULL, size, prot, MAP_SHARED, xc_handle, 0);
+    if (addr == MAP_FAILED)
+        goto mmap_failed;
+
+    for (i = 0; i < nentries; i++) {
+        entries[i].va = (uintptr_t)addr + (i * chunksize);
+        entries[i].npages = chunksize >> PAGE_SHIFT;
+    }
+
+    ioctlx.num   = nentries;
     ioctlx.dom   = dom;
     ioctlx.entry = entries;
 
-    return ioctl(xc_handle, IOCTL_PRIVCMD_MMAP, &ioctlx);
+    rc = ioctl(xc_handle, IOCTL_PRIVCMD_MMAP, &ioctlx);
+    if (rc)
+        goto ioctl_failed;
+
+ioctl_failed:
+    rc = munmap(addr, size);
+    if (rc == -1)
+        ERROR("%s: error in error path\n", __FUNCTION__);
+
+mmap_failed:
+    return NULL;
 }
+
 
 static int do_privcmd(int xc_handle, unsigned int cmd, unsigned long data)
 {

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

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