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 84F81C4345F for ; Fri, 26 Apr 2024 13:54: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=BI3MvCiSpdZVVqplPAUt8PY4bM6G9NOYh1ykL+0Lazc=; b=xlmLrCaizmWbH0 zpCZ9b5RVx+mhk69/2SbaHHeuGmpytczHmwsn5xPEU+5Pwfm4IAVY6YdQonEmnzoWm/0cTEgy5Agu 94SnVbBZccqaXwsZxl6p1eut+/uWGN/uqB0AXA+0iS+oQDOGaYavXviNbDieH6Rfs+CDE+u1ZfOHQ SSGjQtlomr5bDVKBO7g/Jy/ZV4b8cnq1wn/oK9V/4mkH8okFg8IV5Gys4yEgEswcNB68MXN6xSC0w nxKOxYaudyCboEeRdEqdWsKHy1bBuywpLeMq/MoIBHSszWed8fuPtn/3XbivTCaHelHSLczrJx81e oUHjlKj8ogwBYujm27Hg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1s0M2I-0000000CjBw-1mR4; Fri, 26 Apr 2024 13:54:46 +0000 Received: from us-smtp-delivery-124.mimecast.com ([170.10.133.124]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1s0M2G-0000000CjAL-0SKR for linux-arm-kernel@lists.infradead.org; Fri, 26 Apr 2024 13:54:45 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1714139681; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=WXzg55UNGF+ubjZE2+OFVJ9vlVX3fwUMCwPAWdRnKtI=; b=OBEadv5jD/1PDopgNT6agE75GVBlI4lg8lvMPhmgLjytVJV2VG4W8+gc9eAi6mXo6hs/Hp vCoA+HxhNndUDo8Rwmv5QrbYHDCMsLkMOCFy4muOViXTf5mgFrpHEKiC0xmoVmLGbRCa5F t9r1DOLssz9VIFoGDgZLpc8PC/tL/GY= Received: from mail-qt1-f199.google.com (mail-qt1-f199.google.com [209.85.160.199]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-60-B3q3fOB1Nl2aZQn0Rz7gZA-1; Fri, 26 Apr 2024 09:54:37 -0400 X-MC-Unique: B3q3fOB1Nl2aZQn0Rz7gZA-1 Received: by mail-qt1-f199.google.com with SMTP id d75a77b69052e-437972269edso2852291cf.3 for ; Fri, 26 Apr 2024 06:54:37 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1714139677; x=1714744477; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=WXzg55UNGF+ubjZE2+OFVJ9vlVX3fwUMCwPAWdRnKtI=; b=SPlCj2+7WzmIfMHD4ieuDT/+Zsowg+QfBMH/6BK4K4uqilBm4rYJTAqQMgr7tkJZGL oDzIJkX/skUDSTqbYBDb12WWXdwTXKniRkgObrXkSLVENq8VuKB24fMIf4L4wmv7VDOm hTkAGIfnd2rbWQpg0UkkEdBco9ZQ5jEyVAJN7vPYNDR/dbzGcGue/sqJIBKO7rJKLWbM WBOa47JS3vzqOqISmoZmc8Lw0MV6HW+sAt/U87qIpcwgYmTMU9lUoE/3YzFlQx3lbNZ6 cEVojb4dFhOz3uxQhaGzPupRaJ0fwyArnZSjT0NfVqCW6yR4y6h5cOZ7rKB6RamCc3+F XMTw== X-Forwarded-Encrypted: i=1; AJvYcCVcdyHWxXKKQ8marwhS02mDE3/19b3nSk3ZlJds4dY/zGMOsjhaU07yyuaxUorcil4ytLBkMZIl3yPaEIZEzh9B/aLepq9D9YSo7Qiobo8fLdu5Wjg= X-Gm-Message-State: AOJu0YxAxADg3nIsQMjRV5Q/4oFGhL0fog7+n4dEZj+G7YJvKEjOTWLO 3p5fVzw5Ql6vzoeHSe6sumAK6yVa31axKaa6oD1AAyWc2aVdmL2tUtaXP0NQ9QumLip7hkV8ceB 6TNWHLzhfBf2KfD6d+gQiNRsVvr/1i7uy9LWUvYQbxxZgrbA2UZ8JuihAaAYVy8tRbBpCb/Fq X-Received: by 2002:a05:620a:1aaa:b0:790:98be:4cc9 with SMTP id bl42-20020a05620a1aaa00b0079098be4cc9mr2966483qkb.6.1714139676505; Fri, 26 Apr 2024 06:54:36 -0700 (PDT) X-Google-Smtp-Source: AGHT+IHVmEjzcMxKjM7Nh6FWEAWEijr1jAB0CV7Rmv1BMhzxskt4GL/bzsE7DJkfbgEV3HMrnwlhvA== X-Received: by 2002:a05:620a:1aaa:b0:790:98be:4cc9 with SMTP id bl42-20020a05620a1aaa00b0079098be4cc9mr2966447qkb.6.1714139675859; Fri, 26 Apr 2024 06:54:35 -0700 (PDT) Received: from x1n (pool-99-254-121-117.cpe.net.cable.rogers.com. [99.254.121.117]) by smtp.gmail.com with ESMTPSA id m21-20020ae9e015000000b007908bd7a309sm2541679qkk.111.2024.04.26.06.54.34 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 26 Apr 2024 06:54:35 -0700 (PDT) Date: Fri, 26 Apr 2024 09:54:33 -0400 From: Peter Xu To: Ryan Roberts Cc: Muhammad Usama Anjum , Catalin Marinas , Will Deacon , Joey Gouly , Ard Biesheuvel , Mark Rutland , Anshuman Khandual , David Hildenbrand , Mike Rapoport , Shivansh Vij , linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v1 2/2] arm64/mm: Add uffd write-protect support Message-ID: References: <20240424111017.3160195-1-ryan.roberts@arm.com> <20240424111017.3160195-3-ryan.roberts@arm.com> MIME-Version: 1.0 In-Reply-To: X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Disposition: inline X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240426_065444_357876_62504658 X-CRM114-Status: GOOD ( 37.67 ) 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 Fri, Apr 26, 2024 at 02:17:41PM +0100, Ryan Roberts wrote: > + Muhammad Usama Anjum > > Hi Peter, Muhammad, > > > On 24/04/2024 12:57, Peter Xu wrote: > > Hi, Ryan, > > > > On Wed, Apr 24, 2024 at 12:10:17PM +0100, Ryan Roberts wrote: > >> Let's use the newly-free PTE SW bit (58) to add support for uffd-wp. > >> > >> The standard handlers are implemented for set/test/clear for both pte > >> and pmd. Additionally we must also track the uffd-wp state as a pte swp > >> bit, so use a free swap entry pte bit (3). > >> > >> Signed-off-by: Ryan Roberts > > > > Looks all sane here from userfault perspective, just one comment below. > > > >> --- > >> arch/arm64/Kconfig | 1 + > >> arch/arm64/include/asm/pgtable-prot.h | 8 ++++ > >> arch/arm64/include/asm/pgtable.h | 55 +++++++++++++++++++++++++++ > >> 3 files changed, 64 insertions(+) > >> > >> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig > >> index 7b11c98b3e84..763e221f2169 100644 > >> --- a/arch/arm64/Kconfig > >> +++ b/arch/arm64/Kconfig > >> @@ -255,6 +255,7 @@ config ARM64 > >> select SYSCTL_EXCEPTION_TRACE > >> select THREAD_INFO_IN_TASK > >> select HAVE_ARCH_USERFAULTFD_MINOR if USERFAULTFD > >> + select HAVE_ARCH_USERFAULTFD_WP if USERFAULTFD > >> select TRACE_IRQFLAGS_SUPPORT > >> select TRACE_IRQFLAGS_NMI_SUPPORT > >> select HAVE_SOFTIRQ_ON_OWN_STACK > >> diff --git a/arch/arm64/include/asm/pgtable-prot.h b/arch/arm64/include/asm/pgtable-prot.h > >> index ef952d69fd04..f1e1f6306e03 100644 > >> --- a/arch/arm64/include/asm/pgtable-prot.h > >> +++ b/arch/arm64/include/asm/pgtable-prot.h > >> @@ -20,6 +20,14 @@ > >> #define PTE_DEVMAP (_AT(pteval_t, 1) << 57) > >> #define PTE_PROT_NONE (PTE_UXN) /* Reuse PTE_UXN; only when !PTE_VALID */ > >> > >> +#ifdef CONFIG_HAVE_ARCH_USERFAULTFD_WP > >> +#define PTE_UFFD_WP (_AT(pteval_t, 1) << 58) /* uffd-wp tracking */ > >> +#define PTE_SWP_UFFD_WP (_AT(pteval_t, 1) << 3) /* only for swp ptes */ > > I've just noticed code in task_mmu.c: > > static int pagemap_scan_pmd_entry(pmd_t *pmd, unsigned long start, > unsigned long end, struct mm_walk *walk) > { > ... > > if (!p->arg.category_anyof_mask && !p->arg.category_inverted && > p->arg.category_mask == PAGE_IS_WRITTEN && > p->arg.return_mask == PAGE_IS_WRITTEN) { > for (addr = start; addr < end; pte++, addr += PAGE_SIZE) { > unsigned long next = addr + PAGE_SIZE; > > if (pte_uffd_wp(ptep_get(pte))) <<<<<< > continue; > > ... > } > } > } > > As far as I can see, you don't know that the pte is present when you do this. So > does this imply that the UFFD-WP bit is expected to be in the same position for > both present ptes and swap ptes? I had assumed pte_uffd_wp() was for present > ptes and pte_swp_uffd_wp() was for swap ptes. > > As you can see, the way I've implemented this for arm64 the bit is in a > different position for these 2 cases. I've just done a slightly different > implementation that changes the first patch in this series quite a bit and a > bunch of pagemap_ioctl mm kselftests are now failing. I think this is the root > cause, but haven't proven it definitively yet. > > I'm inclined towords thinking the above is a bug and should be fixed so that I > can store the bit in different places. What do you think? Yep I agree. Even on x86_64 they should be defined differently. It looks like some sheer luck the test constantly pass on x86 even if it checked the wrong one. Worth checking all the relevant paths in the pagemap code to make sure it's checked, e.g. I also see one fast path above this chunk of code which looks like to have the same issue. Thanks, -- Peter Xu _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel