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>