Skip to contentSkip to conversation

libxc: mmap fixes for BSD

PATCH In review Christoph Egger 2 participants 4 messages Jun 10, 2008 – Jul 23, 2008

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 libxc: mmap fixes for BSD

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.

Signature · 12 lines
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
tools/libxc/xc_dom_boot.c +1 -1
diff -r 57b8c74c35ef tools/libxc/xc_dom_boot.c
@@ -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;
tools/libxc/xc_domain_save.c +1 -1
diff -r 57b8c74c35ef tools/libxc/xc_domain_save.c
@@ -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;
tools/libxc/xc_hvm_build.c +1 -1
diff -r 57b8c74c35ef tools/libxc/xc_hvm_build.c
@@ -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;
 

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

#1 lore raw
Quoted 9 lines · Christoph Egger

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

#2 lore raw
Quoted 17 lines · On Tuesday 10 June 2008 16:21:56 Keir Fraser wrote:
Quoted 9 lines · Christoph Egger

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()?

Quoted 3 lines

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

Signature · 12 lines
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
#3 lore raw
In reply to Keir Fraser
Quoted 17 lines · On Tuesday 10 June 2008 16:21:56 Keir Fraser wrote:
Quoted 9 lines · Christoph Egger

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()?

Quoted 5 lines

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

Signature · 12 lines
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
tools/libxc/xc_dom_boot.c +8 -20
diff -r f86941c1b523 tools/libxc/xc_dom_boot.c
@@ -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;
 }
 
tools/libxc/xc_domain_save.c +22 -24

79 lines not on this page · raw message

tools/libxc/xc_hvm_build.c +6 -18
diff -r f86941c1b523 tools/libxc/xc_hvm_build.c
@@ -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);
 
tools/libxc/xc_linux.c +29 -4
diff -r f86941c1b523 tools/libxc/xc_linux.c
@@ -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)
tools/libxc/xc_minios.c +11 -0
diff -r f86941c1b523 tools/libxc/xc_minios.c
@@ -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)
 {
tools/libxc/xc_netbsd.c +32 -13

61 lines not on this page · raw message

tools/libxc/xc_private.h +3 -2
diff -r f86941c1b523 tools/libxc/xc_private.h
@@ -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);
tools/libxc/xc_solaris.c +27 -4
diff -r f86941c1b523 tools/libxc/xc_solaris.c
@@ -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)
 {

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel