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

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

[Christoph Egger]

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


Hi!

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.

Signed-off-by: Christoph Egger <Christoph.Egger@amd.com>


-- 
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: 1454 bytes --]

diff -r 57b8c74c35ef tools/libxc/xc_dom_boot.c
--- a/tools/libxc/xc_dom_boot.c	Tue Jun 10 14:17:20 2008 +0100
+++ b/tools/libxc/xc_dom_boot.c	Tue Jun 10 15:47:37 2008 +0200
@@ -166,7 +166,7 @@ void *xc_dom_boot_domU_map(struct xc_dom
     }
 
     ptr = mmap(NULL, count << page_shift, PROT_READ | PROT_WRITE,
-               MAP_SHARED, dom->guest_xc, 0);
+               MAP_ANON | MAP_SHARED, -1, 0);
     if ( ptr == MAP_FAILED )
     {
         err = errno;
diff -r 57b8c74c35ef tools/libxc/xc_domain_save.c
--- a/tools/libxc/xc_domain_save.c	Tue Jun 10 14:17:20 2008 +0100
+++ b/tools/libxc/xc_domain_save.c	Tue Jun 10 15:47:37 2008 +0200
@@ -589,7 +589,7 @@ static xen_pfn_t *xc_map_m2p(int xc_hand
     }
 
     if ( (m2p = mmap(NULL, m2p_size, prot,
-                     MAP_SHARED, xc_handle, 0)) == MAP_FAILED )
+                     MAP_ANON | MAP_SHARED, -1, 0)) == MAP_FAILED )
     {
         ERROR("failed to mmap m2p");
         return NULL;
diff -r 57b8c74c35ef tools/libxc/xc_hvm_build.c
--- a/tools/libxc/xc_hvm_build.c	Tue Jun 10 14:17:20 2008 +0100
+++ b/tools/libxc/xc_hvm_build.c	Tue Jun 10 15:47:37 2008 +0200
@@ -123,7 +123,7 @@ static int loadelfimage(
     if ( entries == NULL )
         goto err;
     elf->dest = mmap(NULL, pages << PAGE_SHIFT, PROT_READ | PROT_WRITE,
-                     MAP_SHARED, xch, 0);
+                     MAP_ANON | MAP_SHARED, -1, 0);
     if ( elf->dest == MAP_FAILED )
         goto err;
 

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

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

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

[Keir Fraser]

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

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

[Christoph Egger]

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.

That's fine with me.

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

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

[Christoph Egger]

[-- 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