Author: alc
Date: Thu Aug  8 06:26:34 2019
New Revision: 350741
URL: https://svnweb.freebsd.org/changeset/base/350741

Log:
  Ordinarily, during a superpage promotion or demotion within a pmap, the
  pmap's lock ensures that other operations on the pmap don't observe the
  old mapping being broken before the new mapping is established.  However,
  pmap_kextract() doesn't acquire the kernel pmap's lock, so it may observe
  the broken mapping.  And, if it does, it returns an incorrect result.
  
  This revision implements a lock-free solution to this problem in
  pmap_update_entry() and pmap_kextract() because pmap_kextract() can't
  acquire the kernel pmap's lock.
  
  Reported by:  andrew, greg_unrelenting.technology
  Reviewed by:  andrew, markj
  Tested by:    greg_unrelenting.technology
  X-MFC with:   r350579
  Differential Revision:        https://reviews.freebsd.org/D21169

Modified:
  head/sys/arm64/arm64/pmap.c
  head/sys/arm64/include/pte.h

Modified: head/sys/arm64/arm64/pmap.c
==============================================================================
--- head/sys/arm64/arm64/pmap.c Thu Aug  8 04:29:46 2019        (r350740)
+++ head/sys/arm64/arm64/pmap.c Thu Aug  8 06:26:34 2019        (r350741)
@@ -1128,40 +1128,35 @@ vm_paddr_t
 pmap_kextract(vm_offset_t va)
 {
        pt_entry_t *pte, tpte;
-       vm_paddr_t pa;
-       int lvl;
 
-       if (va >= DMAP_MIN_ADDRESS && va < DMAP_MAX_ADDRESS) {
-               pa = DMAP_TO_PHYS(va);
-       } else {
-               pa = 0;
-               pte = pmap_pte(kernel_pmap, va, &lvl);
-               if (pte != NULL) {
-                       tpte = pmap_load(pte);
-                       pa = tpte & ~ATTR_MASK;
-                       switch(lvl) {
-                       case 1:
-                               KASSERT((tpte & ATTR_DESCR_MASK) == L1_BLOCK,
-                                   ("pmap_kextract: Invalid L1 pte found: %lx",
-                                   tpte & ATTR_DESCR_MASK));
-                               pa |= (va & L1_OFFSET);
-                               break;
-                       case 2:
-                               KASSERT((tpte & ATTR_DESCR_MASK) == L2_BLOCK,
-                                   ("pmap_kextract: Invalid L2 pte found: %lx",
-                                   tpte & ATTR_DESCR_MASK));
-                               pa |= (va & L2_OFFSET);
-                               break;
-                       case 3:
-                               KASSERT((tpte & ATTR_DESCR_MASK) == L3_PAGE,
-                                   ("pmap_kextract: Invalid L3 pte found: %lx",
-                                   tpte & ATTR_DESCR_MASK));
-                               pa |= (va & L3_OFFSET);
-                               break;
-                       }
-               }
-       }
-       return (pa);
+       if (va >= DMAP_MIN_ADDRESS && va < DMAP_MAX_ADDRESS)
+               return (DMAP_TO_PHYS(va));
+       pte = pmap_l1(kernel_pmap, va);
+       if (pte == NULL)
+               return (0);
+
+       /*
+        * A concurrent pmap_update_entry() will clear the entry's valid bit
+        * but leave the rest of the entry unchanged.  Therefore, we treat a
+        * non-zero entry as being valid, and we ignore the valid bit when
+        * determining whether the entry maps a block, page, or table.
+        */
+       tpte = pmap_load(pte);
+       if (tpte == 0)
+               return (0);
+       if ((tpte & ATTR_DESCR_TYPE_MASK) == ATTR_DESCR_TYPE_BLOCK)
+               return ((tpte & ~ATTR_MASK) | (va & L1_OFFSET));
+       pte = pmap_l1_to_l2(&tpte, va);
+       tpte = pmap_load(pte);
+       if (tpte == 0)
+               return (0);
+       if ((tpte & ATTR_DESCR_TYPE_MASK) == ATTR_DESCR_TYPE_BLOCK)
+               return ((tpte & ~ATTR_MASK) | (va & L2_OFFSET));
+       pte = pmap_l2_to_l3(&tpte, va);
+       tpte = pmap_load(pte);
+       if (tpte == 0)
+               return (0);
+       return ((tpte & ~ATTR_MASK) | (va & L3_OFFSET));
 }
 
 /***************************************************
@@ -3021,8 +3016,12 @@ pmap_update_entry(pmap_t pmap, pd_entry_t *pte, pd_ent
        intr = intr_disable();
        critical_enter();
 
-       /* Clear the old mapping */
-       pmap_clear(pte);
+       /*
+        * Clear the old mapping's valid bit, but leave the rest of the entry
+        * unchanged, so that a lockless, concurrent pmap_kextract() can still
+        * lookup the physical address.
+        */
+       pmap_clear_bits(pte, ATTR_DESCR_VALID);
        pmap_invalidate_range_nopin(pmap, va, va + size);
 
        /* Create the new mapping */

Modified: head/sys/arm64/include/pte.h
==============================================================================
--- head/sys/arm64/include/pte.h        Thu Aug  8 04:29:46 2019        
(r350740)
+++ head/sys/arm64/include/pte.h        Thu Aug  8 06:26:34 2019        
(r350741)
@@ -71,7 +71,12 @@ typedef      uint64_t        pt_entry_t;             /* page 
table entry */
 
 #define        ATTR_DEFAULT    (ATTR_AF | ATTR_SH(ATTR_SH_IS))
 
-#define        ATTR_DESCR_MASK 3
+#define        ATTR_DESCR_MASK         3
+#define        ATTR_DESCR_VALID        1
+#define        ATTR_DESCR_TYPE_MASK    2
+#define        ATTR_DESCR_TYPE_TABLE   2
+#define        ATTR_DESCR_TYPE_PAGE    2
+#define        ATTR_DESCR_TYPE_BLOCK   0
 
 /* Level 0 table, 512GiB per entry */
 #define        L0_SHIFT        39
_______________________________________________
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Reply via email to