From owner-svn-src-head@freebsd.org Thu Aug 8 06:26:35 2019 Return-Path: Delivered-To: svn-src-head@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id D50C3C4A9A; Thu, 8 Aug 2019 06:26:35 +0000 (UTC) (envelope-from alc@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) server-signature RSA-PSS (4096 bits) client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 463z0q5FSRz3PpF; Thu, 8 Aug 2019 06:26:35 +0000 (UTC) (envelope-from alc@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 91F4420F30; Thu, 8 Aug 2019 06:26:35 +0000 (UTC) (envelope-from alc@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id x786QZdL081447; Thu, 8 Aug 2019 06:26:35 GMT (envelope-from alc@FreeBSD.org) Received: (from alc@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id x786QZh9081445; Thu, 8 Aug 2019 06:26:35 GMT (envelope-from alc@FreeBSD.org) Message-Id: <201908080626.x786QZh9081445@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: alc set sender to alc@FreeBSD.org using -f From: Alan Cox Date: Thu, 8 Aug 2019 06:26:35 +0000 (UTC) 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 X-SVN-Group: head X-SVN-Commit-Author: alc X-SVN-Commit-Paths: in head/sys/arm64: arm64 include X-SVN-Commit-Revision: 350741 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 08 Aug 2019 06:26:35 -0000 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