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 78CBAC433F5 for ; Thu, 14 Apr 2022 10:06:58 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=RgYDU9E8sCWUPwXGyd3yhWG5JFeRe1SsalXTFMaq1BI=; b=e8SWekDffb6j5+ hPznspCKvH1nAkWh94/uOV9Z4ZSt8RD0zxB39jQ6MaTLO7VBJLVcYKItbXZmkkg8LdM20jo+rHyaD hyKd8E/fRzhBhrNc0fcouHwqPXB13cldcwJIb9HwZ7k00Z2oGf5wXJsv8Tw65++lOgxKxNEgbDQth k0OFrM2VLna7Hdd4VXEkqUuieO9PhUf0kv/ndtkABD+S4yb2iYGByAYTqeQ61Y4jtQ7ZavtQqTl/a mZXS1gsjGvXvn9++/jGoAR0hNiG09UE4bAlKgYeMCPLCQ98DPc6/vMq2q3Pl/nUIuJDrog04p3+mB 7CbDOu3LEQXoNG7Vtjcg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1newMH-0052lQ-UQ; Thu, 14 Apr 2022 10:05:50 +0000 Received: from dfw.source.kernel.org ([2604:1380:4641:c500::1]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1newMD-0052iZ-9i for linux-arm-kernel@lists.infradead.org; Thu, 14 Apr 2022 10:05:47 +0000 Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 715AB61D4D; Thu, 14 Apr 2022 10:05:42 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 86612C385A5; Thu, 14 Apr 2022 10:05:39 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1649930741; bh=Dmla2dPaUtWMTVxmNVoyiq1mZIf/bZdj49HOCheIL5g=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=HgQc04JQWUYRZZuWRorf2UFDSTE8CLHFAtlO4puAc+Q3y4L+aCZSmbUMDs0XKmFNn qh/tGovkPUIPvH1T+AzLn9SYhMb92IpB+ZfMo1pVqGkFiMdy0KPXxL6zEpz7HEaRK/ S3rQ+H6cm3PWi6Z0gckzFTY4pbpCj2rTbtvzPg4qKjWspYJDZ5dtfcCA/yT7wjL6nc cm5u1O+pck4AzObjG3Zd0xmTkvZTb5UUmjiHJBHQm4+m+5J8h92mMsT2Eq49zRXqwt o5Uj9haFXd1fIsRoxsT12YQUS8t98KrKzB0c8x7tfbUfdxIbvX/5t1uJZXwOg2/Iso KryjHogmPHsnQ== Date: Thu, 14 Apr 2022 11:05:35 +0100 From: Will Deacon To: Steven Price Cc: Muchun Song , catalin.marinas@arm.com, akpm@linux-foundation.org, anshuman.khandual@arm.com, lengxujun2007@126.com, arnd@arndb.de, smuchun@gmail.com, duanxiongchun@bytedance.com, quic_qiancai@quicinc.com, aneesh.kumar@linux.ibm.com, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v2] arm64: mm: fix pmd_leaf() Message-ID: <20220414100535.GB2298@willie-the-truck> References: <20220411122653.40284-1-songmuchun@bytedance.com> <20220413101929.GA1229@willie-the-truck> <64d4288e-7776-a3fd-5ee4-70486dfd0394@arm.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <64d4288e-7776-a3fd-5ee4-70486dfd0394@arm.com> User-Agent: Mutt/1.10.1 (2018-07-13) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220414_030545_442263_A91C6573 X-CRM114-Status: GOOD ( 38.63 ) 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: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Wed, Apr 13, 2022 at 11:39:49AM +0100, Steven Price wrote: > On 13/04/2022 11:19, Will Deacon wrote: > > On Mon, Apr 11, 2022 at 08:26:53PM +0800, Muchun Song wrote: > >> The pmd_leaf() is used to test a leaf mapped PMD, however, it misses > >> the PROT_NONE mapped PMD on arm64. Fix it. A real world issue [1] > >> caused by this was reported by Qian Cai. > >> > >> Link: https://patchwork.kernel.org/comment/24798260/ [1] > >> Fixes: 8aa82df3c123 ("arm64: mm: add p?d_leaf() definitions") > >> Reported-by: Qian Cai > >> Signed-off-by: Muchun Song > >> --- > >> v2: > >> - Replace pmd_present() with pmd_val() since we expect pmd_leaf() works > >> well on non-present pmd case. > >> > >> arch/arm64/include/asm/pgtable.h | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h > >> index ad9b221963d4..00cdd2d895d3 100644 > >> --- a/arch/arm64/include/asm/pgtable.h > >> +++ b/arch/arm64/include/asm/pgtable.h > >> @@ -551,7 +551,7 @@ extern pgprot_t phys_mem_access_prot(struct file *file, unsigned long pfn, > >> PMD_TYPE_TABLE) > >> #define pmd_sect(pmd) ((pmd_val(pmd) & PMD_TYPE_MASK) == \ > >> PMD_TYPE_SECT) > >> -#define pmd_leaf(pmd) pmd_sect(pmd) > >> +#define pmd_leaf(pmd) (pmd_val(pmd) && !(pmd_val(pmd) & PMD_TABLE_BIT)) > >> #define pmd_bad(pmd) (!pmd_table(pmd)) > > > > I'm still trying to get my head around the desired semantics here. > > > > If we want to fix the original report, then we need to take PROT_NONE > > entries into account. The easiest way to do that is, as you originally > > suggested, by using pmd_present(): > > > > #define pmd_leaf(pmd) (pmd_present(pmd) && !pmd_table(pmd)) > > > > But now you seem to be saying that !pmd_present() entries should also be > > considered as pmd_leaf() -- is there a real need for that? > > > > If so, then I think this simply becomes: > > > > #define pmd_leaf(pmd) (!pmd_table(pmd)) > > > > which is, amusingly, identical to pmd_bad(). > > > > The documentation/comment that Steven referred to also desperately needs > > clarifying as it currently states: > > > > "Only meaningful when called on a valid entry." > > > > whatever that means. > > The intention at the time is that this had the same meaning as > pmd_huge() (when CONFIG_HUGETLB_PAGE is defined), which would then match > this patch. This is referred in the comment, albeit in a rather weak way: > > > * This differs from p?d_huge() by the fact that they are always available (if > > * the architecture supports large pages at the appropriate level) even > > * if CONFIG_HUGETLB_PAGE is not defined. > > However, the real issue here is that the definition of pmd_leaf() isn't > clear. I know what the original uses of it needed but since then it's > been used in other areas, and I'm afraid my 'documentation' isn't > precise enough to actually be useful. > > At the time I wrote that comment I think I meant "valid" in the AArch64 > sense (i.e. the LSB of the entry). PROT_NONE isn't 'valid' by that > definition (and I hadn't considered it). But of course that definition > of 'valid' is pretty meaningless in the cross-architecture case. arm64 'valid' + PROT_NONE is roughly what 'present' means. So we could say that this only works for present entries, but then Muchun's latest patch wants to work with !present which is why I tried to work this through. Will _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel