tech-kern archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

UVM patch for PR port-xen/45975



Hello,
there is an issue with SMP Xen guests (see port-xen/45975), where uvm
can put a page back to free list before clearing its mappings.
This happens when uvm_km_pgremove_intrsafe() is used: is has to be called
before pmap_kremove() because it uses pmap_extract(), and (I guess for
efficiency), all pages are freed before calling pmap_kremove() for the
whole range.
This is a problem for Xen, because one of these pages can be grabbed by
another CPU for a PDP, and Xen won't allow to pin a page as PDP if it
still has R/W mappings. As a CPU can be preempted by the hypervisor,
there is potentially plenty of time for another CPU to grab the
page, initialize it and try to use it as PDP between uvm_km_pgremove_intrsafe()
and pmap_kremove().

To fix this I propose the attached patch:
- change uvm_km_pgremove_intrsafe()'s semantic to also unmap the page
  (all uvm_km_pgremove_intrsafe() use are followed by a pmap_kremove(),
  just move the call in uvm_km_pgremove_intrsafe()).
- If __PMAP_NEEDS_UNMAP_BEFORE_FREE is defined, uvm_km_pgremove_intrsafe()
  does a pmap_kremove() for each page, after pmap_extract() but before
  uvm_pagefree(). If not defined, uvm_km_pgremove_intrsafe() does
  a tail pmap_kremove() call at the end
- __PMAP_NEEDS_UNMAP_BEFORE_FREE is defined for XEN in x86/include/pmap.h

Does anyone see a problem with this ?

-- 
Manuel Bouyer <bouyer%antioche.eu.org@localhost>
     NetBSD: 26 ans d'experience feront toujours la difference
--
Index: uvm/uvm_km.c
===================================================================
RCS file: /cvsroot/src/sys/uvm/uvm_km.c,v
retrieving revision 1.119
diff -u -p -u -r1.119 uvm_km.c
--- uvm/uvm_km.c        4 Feb 2012 17:56:17 -0000       1.119
+++ uvm/uvm_km.c        17 Feb 2012 15:26:30 -0000
@@ -472,12 +472,18 @@ uvm_km_pgremove_intrsafe(struct vm_map *
                if (!pmap_extract(pmap_kernel(), start, &pa)) {
                        continue;
                }
+#ifdef __PMAP_NEEDS_UNMAP_BEFORE_FREE
+               pmap_kremove(start, PAGE_SIZE);
+#endif
                pg = PHYS_TO_VM_PAGE(pa);
                KASSERT(pg);
                KASSERT(pg->uobject == NULL && pg->uanon == NULL);
                KASSERT((pg->flags & PG_BUSY) == 0);
                uvm_pagefree(pg);
        }
+#ifndef __PMAP_NEEDS_UNMAP_BEFORE_FREE
+       pmap_kremove(start, end - start);
+#endif
 }
 
 #if defined(DEBUG)
@@ -670,7 +676,6 @@ uvm_km_free(struct vm_map *map, vaddr_t 
                 * remove it after.  See comment below about KVA visibility.
                 */
                uvm_km_pgremove_intrsafe(map, addr, addr + size);
-               pmap_kremove(addr, size);
        }
 
        /*
@@ -747,7 +752,6 @@ again:
                        } else {
                                uvm_km_pgremove_intrsafe(kernel_map, va,
                                    va + size);
-                               pmap_kremove(va, size);
                                vmem_free(kmem_va_arena, va, size);
                                return ENOMEM;
                        }
@@ -783,7 +787,6 @@ uvm_km_kmem_free(vmem_t *vm, vmem_addr_t
        }
 #endif /* PMAP_UNMAP_POOLPAGE */
        uvm_km_pgremove_intrsafe(kernel_map, addr, addr + size);
-       pmap_kremove(addr, size);
        pmap_update(pmap_kernel());
 
        vmem_free(vm, addr, size);
Index: uvm/uvm_kmguard.c
===================================================================
RCS file: /cvsroot/src/sys/uvm/uvm_kmguard.c,v
retrieving revision 1.9
diff -u -p -u -r1.9 uvm_kmguard.c
--- uvm/uvm_kmguard.c   5 Feb 2012 11:08:06 -0000       1.9
+++ uvm/uvm_kmguard.c   17 Feb 2012 15:26:30 -0000
@@ -180,7 +180,6 @@ uvm_kmguard_free(struct uvm_kmguard *kg,
         */
 
        uvm_km_pgremove_intrsafe(kernel_map, va, va + PAGE_SIZE * 2);
-       pmap_kremove(va, PAGE_SIZE * 2);
        pmap_update(pmap_kernel());
 
        /*
Index: uvm/uvm_map.c
===================================================================
RCS file: /cvsroot/src/sys/uvm/uvm_map.c,v
retrieving revision 1.312
diff -u -p -u -r1.312 uvm_map.c
--- uvm/uvm_map.c       28 Jan 2012 00:00:06 -0000      1.312
+++ uvm/uvm_map.c       17 Feb 2012 15:26:30 -0000
@@ -2246,7 +2246,6 @@ uvm_unmap_remove(struct vm_map *map, vad
                        if ((entry->flags & UVM_MAP_KMAPENT) == 0) {
                                uvm_km_pgremove_intrsafe(map, entry->start,
                                    entry->end);
-                               pmap_kremove(entry->start, len);
                        }
                } else if (UVM_ET_ISOBJ(entry) &&
                           UVM_OBJ_IS_KERN_OBJECT(entry->object.uvm_obj)) {
Index: arch/x86/include/pmap.h
===================================================================
RCS file: /cvsroot/src/sys/arch/x86/include/pmap.h,v
retrieving revision 1.49
diff -u -p -u -r1.49 pmap.h
--- arch/x86/include/pmap.h     4 Dec 2011 16:24:13 -0000       1.49
+++ arch/x86/include/pmap.h     17 Feb 2012 15:26:30 -0000
@@ -296,6 +298,14 @@ void               pmap_tlb_intr(void);
 #define PMAP_GROWKERNEL                /* turn on pmap_growkernel interface */
 #define PMAP_FORK              /* turn on pmap_fork interface */
 
+#ifdef XEN
+/*
+ * If a free vm_page is allocated for a PDP, is will be rejected
+ * by Xen if it has still some R/W mapping.
+ */
+#define __PMAP_NEEDS_UNMAP_BEFORE_FREE
+#endif
+
 /*
  * Do idle page zero'ing uncached to avoid polluting the cache.
  */


Home | Main Index | Thread Index | Old Index