From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id B3472C3DA49 for ; Fri, 26 Jul 2024 11:27:40 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:Content-Transfer-Encoding: Content-Type:In-Reply-To:References:Cc:To:Subject:From:MIME-Version:Date: Message-ID:Reply-To:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=YajhKetmfQ2+coI5861yiPrsD7aw4Ou3m1WyjGz5y80=; b=Dwq7geLY8Pn2OqJriHqfz2rAZB GNOXfUz5eDENaf+3xePfuRypwn/3HRg9CZo22r2ryBu/sqpsjt4BM0MEQ+5VWjYaXCVeIBGO4GRtJ etR1uJjfcHjDiwHbdWa9hACVwYuwWhEdzsamXLE8f8sLSYLvFrm6nPMMEkc3MK8NQqI48ltlrhfYf NinKF4/KQI/I7WSVtsweWO0QxiLaekGvRs2Wp3sBoWxMV5eZtQahB1qJ7QdIcJTaiTXZ+yyAQuBtA 31Mp3HkSfNmUouSk9PdguOOBAsqfccY2mbRi0QVvjmRS6W+nrwDwspkyl3MrdNq4MipANWwFauRJu RjZf77lw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1sXJ6g-00000003iBZ-0wXm; Fri, 26 Jul 2024 11:27:30 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1sXJ6H-00000003i5X-0tjs for linux-arm-kernel@lists.infradead.org; Fri, 26 Jul 2024 11:27:06 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 066801007; Fri, 26 Jul 2024 04:27:30 -0700 (PDT) Received: from [10.163.53.239] (unknown [10.163.53.239]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id A562A3F73F; Fri, 26 Jul 2024 04:27:02 -0700 (PDT) Message-ID: Date: Fri, 26 Jul 2024 16:56:59 +0530 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird From: Anshuman Khandual Subject: Re: [PATCH] arm64/mm: Avoid direct referencing page table enties in map_range() To: Ryan Roberts , linux-arm-kernel@lists.infradead.org Cc: Catalin Marinas , Will Deacon , linux-kernel@vger.kernel.org References: <20240725091052.314750-1-anshuman.khandual@arm.com> <3e82687a-0183-42f3-b32c-6d99dbd4fe49@arm.com> Content-Language: en-US In-Reply-To: <3e82687a-0183-42f3-b32c-6d99dbd4fe49@arm.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240726_042705_392883_0FF62799 X-CRM114-Status: GOOD ( 19.49 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On 7/25/24 16:06, Ryan Roberts wrote: > On 25/07/2024 10:10, Anshuman Khandual wrote: >> Like else where in arm64 platform, use WRITE_ONCE() in map_range() while >> creating page table entries. This avoids referencing page table entries >> directly. > > I could be wrong, but I don't think this code is ever operating on live map_range() is called on these page tables but sequentially during boot. primary_entry() create_init_idmap() map_range(...init_idmap_pg_dir...) primary_switch() early_map_kernel() map_fdt() map_range(...init_idmap_pg_dir...) remap_idmap_for_lpa2() create_init_idmap() map_range(...init_pg_dir...) create_init_idmap() map_range(...init_idmap_pg_dir...) map_kernel() map_segment() map_range(...init_pg_dir...) paging_init() create_idmap() __pi_map_range(...idmap_pg_dir...) > pgtables? So there is never a potential to race with the HW walker and therefore > no need to guarrantee copy atomicity? As long as the correct barriers are placed Unless there is possibility of concurrent HW walk through these page tables, WRITE_ONCE() based atomic is not required here ? I thought arm64 platform decided some time earlier (but don't remember when) to use READ_ONCE()-WRITE_ONCE() for all page table entry, direct references for read or write accesses - possibly for some increased safety ? > at the point where you load the pgdir into the TTBRx there should be no problem? Those barriers are already placed as required. > > If my assertion is correct, I don't think there is any need for this change. > > Thanks, > Ryan > >> >> Cc: Catalin Marinas >> Cc: Will Deacon >> Cc: Ryan Roberts >> Cc: linux-arm-kernel@lists.infradead.org >> Cc: linux-kernel@vger.kernel.org >> Signed-off-by: Anshuman Khandual >> --- >> arch/arm64/kernel/pi/map_range.c | 6 +++--- >> 1 file changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/arch/arm64/kernel/pi/map_range.c b/arch/arm64/kernel/pi/map_range.c >> index 5410b2cac590..b93b70cdfb62 100644 >> --- a/arch/arm64/kernel/pi/map_range.c >> +++ b/arch/arm64/kernel/pi/map_range.c >> @@ -56,8 +56,8 @@ void __init map_range(u64 *pte, u64 start, u64 end, u64 pa, pgprot_t prot, >> * table mapping if necessary and recurse. >> */ >> if (pte_none(*tbl)) { >> - *tbl = __pte(__phys_to_pte_val(*pte) | >> - PMD_TYPE_TABLE | PMD_TABLE_UXN); >> + WRITE_ONCE(*tbl, __pte(__phys_to_pte_val(*pte) | >> + PMD_TYPE_TABLE | PMD_TABLE_UXN)); >> *pte += PTRS_PER_PTE * sizeof(pte_t); >> } >> map_range(pte, start, next, pa, prot, level + 1, >> @@ -79,7 +79,7 @@ void __init map_range(u64 *pte, u64 start, u64 end, u64 pa, pgprot_t prot, >> protval &= ~PTE_CONT; >> >> /* Put down a block or page mapping */ >> - *tbl = __pte(__phys_to_pte_val(pa) | protval); >> + WRITE_ONCE(*tbl, __pte(__phys_to_pte_val(pa) | protval)); >> } >> pa += next - start; >> start = next; >