Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 8 Aug 2019 06:26:35 +0000 (UTC)
From:      Alan Cox <alc@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r350741 - in head/sys/arm64: arm64 include
Message-ID:  <201908080626.x786QZh9081445@repo.freebsd.org>

next in thread | raw e-mail | index | archive | help
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



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201908080626.x786QZh9081445>