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 96435C433EF for ; Thu, 14 Apr 2022 11:19:39 +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=5mGFxX60BFIKDAOGc4sZl/imEYRMNP1RoP217Z9YJZ0=; b=FN7XyJQPwCM2JO CZhNffHkwe4Ll/3C42m0+rfz39pjZ7mLjePDctaOlZgHLAwmvDg0e5BBQZCGpFsk8rIhT4ooF0VAV /ldhkDXYnkzs6viyMwuOETUlwdhSXh7lNk9JXhloYTgQrTzj/KAnkPDA9WfdtpettqahR02J/lV2+ H+H9vYkoBFaTmPWIQExSkaXkHo8CMomtIBDxpQ8W44Qt2Ya0YZDLYnxODAx5FtmImF7m/FE5pq4Rx IY/nANl3mv/28XmtwOybRYgH6eP9ySFE13IFKa+n5sAlR1askWyc65nPfpUXxbafcoIuwZ7ftogcp jk3kX6DiVnxr5805MSWg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1nexUb-005GQ0-2C; Thu, 14 Apr 2022 11:18:29 +0000 Received: from mail-pj1-x1036.google.com ([2607:f8b0:4864:20::1036]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1nexUW-005GOs-Av for linux-arm-kernel@lists.infradead.org; Thu, 14 Apr 2022 11:18:26 +0000 Received: by mail-pj1-x1036.google.com with SMTP id j8-20020a17090a060800b001cd4fb60dccso5373931pjj.2 for ; Thu, 14 Apr 2022 04:18:20 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bytedance-com.20210112.gappssmtp.com; s=20210112; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=cApjy1z/WTuYgC0OK2plrpPC0MAtVd0VXO0NPNL/K7k=; b=JRiEcfNcIxdtXaRVoJ49EaIWVLBuPwVwzbNP5U2aJD+SwsDxylVURuBvt0Qv9wNrSd pvXnJY6xxmNXRgWlzyaDe34yc3PwAlKhlHPJUEaXAE3fh2IH63usa7ptt5edKIVeD82P H2Y77Wd5pHy/jEAoSGSRAnZgvg1AOKVY96bkduwWnLjiujAS9wx806bOuQr6sFsDtO0j NxMyJ5bKj1IEBxf49Zby/yQ/lMDtghiPvMGmpjZPkPMkqRMiXqTJcUy+ID320/MmZEfI 1x3wqupplKRF5XNO/Gi0kRuCQzIOvb3LZcr+kpEKetyEWeGfXpV/05F7Fp91E3IEI4AB nPGQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=cApjy1z/WTuYgC0OK2plrpPC0MAtVd0VXO0NPNL/K7k=; b=rHa7gdYQoZw0ISHsrSzNIPZEANTAXTh6iV77eUjPfVoW1wdpzSClGIb55vP+Ob8VIF Ow0wpB4pHjV7qBKqwFAJJg2qfjWNghr0V5OA7w9xwgVH00cPf4/cJ86Ot8M/VZZoAMLx x23U4FscxyR2LjgoGzAvElCODH6NgbAQjmDjHcgCr02ko8kE4MNcBbTDy9dklnQ0OwWS CHGosI36rQyrArvW1yiyEZJohHmLru0p8kPLjA3QAJm/JW/JkCkEm4U9s5H6JaZbSEzp odYDqZsFgeGUsnIjit0Zq1RLfi37HosKbz89QPvYMsgL3oZUHvj3gMWCiC7/NGOA9SJV ZXIw== X-Gm-Message-State: AOAM532GIDFeet2z/3mWfyUA5oIG2L77VNRDYeEot7Q4q97r6BHh60EL umkknJ91jF38t/o4qS7ZET5nrQ== X-Google-Smtp-Source: ABdhPJy7RRXNPbA+lQVGxBoi5SOBxAGPWUefgr9V+P+8VYZr88w41MNltNvDvmzCwsofV1+Hh9rnFw== X-Received: by 2002:a17:90b:4a89:b0:1c7:3933:d802 with SMTP id lp9-20020a17090b4a8900b001c73933d802mr3117701pjb.75.1649935100207; Thu, 14 Apr 2022 04:18:20 -0700 (PDT) Received: from localhost ([139.177.225.229]) by smtp.gmail.com with ESMTPSA id m2-20020a17090a4d8200b001cb41f25148sm1848735pjh.17.2022.04.14.04.18.19 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 14 Apr 2022 04:18:19 -0700 (PDT) Date: Thu, 14 Apr 2022 19:18:11 +0800 From: Muchun Song To: Will Deacon Cc: Steven Price , 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: References: <20220411122653.40284-1-songmuchun@bytedance.com> <20220413101929.GA1229@willie-the-truck> <64d4288e-7776-a3fd-5ee4-70486dfd0394@arm.com> <20220414100535.GB2298@willie-the-truck> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20220414100535.GB2298@willie-the-truck> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220414_041824_638400_1CFB2BDE X-CRM114-Status: GOOD ( 42.86 ) 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 Thu, Apr 14, 2022 at 11:05:35AM +0100, Will Deacon wrote: > 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. > My bad. In the previous version, Aneesh seems want to make pmd_leaf() works for a not present page table entry, I am trying doing this in this version. Seems like I do the right thing in the previous version from your explanation. I'll use the previos version and fix pud_leaf() as well and update the documentation. Do you think this is okay? Thanks. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel