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 7A48DC27C53 for ; Wed, 19 Jun 2024 19:05:04 +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-Type:In-Reply-To: MIME-Version:References:Message-ID:Subject:Cc:To:From:Date:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=MCt/ec5n0jXCIKya1SxULKfdQ16O1Xui3yy1433Obec=; b=cuE714CKbmJSxXr1L5aYQ04ieQ UqLuAz5JMrTU7sAm6WGpeIVxFe1tInDT3SmJRJZaeIP5shiNkHrE2oSszlf0yMtx63otHedOyy8yq H8F57aQPI/7ZJ8o8Gmu6c7SWqabnKkQsBRv9u4AUaLWgbDQtTUAMoRyNYwnsVKACC5BCvbTSRL10l OsaYVym7jSD7I2lkPXIojOuoYBUGZ9c0oJRnhfnZhRNhw+SxUUMSj0nB2SE/axj517OJ/mfOTvEgq HnaTgZjEHzcehgBxSa7F2of27TB6vs6bpCco4x3pbWFpD1r092NZr1okIvPXBFzAq1eMmKdPeeLe6 dd71z2YA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1sK0c1-00000002Tcj-1YPj; Wed, 19 Jun 2024 19:04:53 +0000 Received: from us-smtp-delivery-124.mimecast.com ([170.10.129.124]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1sK0by-00000002Tc5-2U5q for linux-arm-kernel@lists.infradead.org; Wed, 19 Jun 2024 19:04:52 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1718823887; 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=MCt/ec5n0jXCIKya1SxULKfdQ16O1Xui3yy1433Obec=; b=F2/9A5YjjNUEqCVrlKwu+PS7lswXYB3mYsg1rfoxZo0N/WKzhBCAEtJ9Gyu/IAYzBWZAcU t+or+vFsNXc3OplWbeyKzwoIk2R0CtfvmLPXN0A9cmxkoF7gRZFt9uWfIxRKgCK42kgP7H oV5uN+WZLp5y03aOwCV+c3y9WQRfMjA= Received: from mail-qv1-f70.google.com (mail-qv1-f70.google.com [209.85.219.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-551-T4cLViliMgeQgh-DY02Dyw-1; Wed, 19 Jun 2024 15:04:46 -0400 X-MC-Unique: T4cLViliMgeQgh-DY02Dyw-1 Received: by mail-qv1-f70.google.com with SMTP id 6a1803df08f44-6b50790e82bso418106d6.2 for ; Wed, 19 Jun 2024 12:04:46 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1718823885; x=1719428685; 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=MCt/ec5n0jXCIKya1SxULKfdQ16O1Xui3yy1433Obec=; b=E91nJ7EfyLApaJu/QCBlgNs96w302R7QLDUkwxEKy3+vOTJV9ReV447BcG+a07O7aR pbZFHBGbplThoXZfnNCWWt3HLvc6bQQJ2sPRzuE8Q+FZoACvR8qhbTdvnppZsjHZjNip CQ1rmxp8XKgcaNB6JD8cTxU84sDXnFPVla21oUNKzSzi2hh0HiB1+24IXwRl60l0pOFA SQ239QIJ9qAMndd2BLFsSMtkQCmGEERSaNw5aMXsyCI4GHvNqZriQ6v6RUFRRVjkMCxF Dz3aabKf3i0Tgq+kj7UHKFFImnQIGTlJ1/IxY01iP+/DO9ArNHUFIPQrXX9SnL4QM+5y 2TPA== X-Forwarded-Encrypted: i=1; AJvYcCXbofliTi6HFXbmyRbOm4Cqw+bZ+4LXG89+LM+jDU+BIWdn6TShJQvEfleHAsGMYoc5NCh2kA43bM7XlOGsocVbcetXq2Lh8jl7r36XtvW53zhszxc= X-Gm-Message-State: AOJu0Ywlgory8gQWufqcLIPWzXCO3MBAu+wwPaRcORsMi5MYE0H62sVB Zb3s544g+nLQG3MhGRxY71oCf0zk3qdfPjbUFayWTCf3HQZVKvo2kFripS4WnDjjaJQ749C9nje Yodmy0rXr+/Ftu1CR06NtlrbFLxwW0E95MuV7zViWr0mADWbON3lgcSkVn5i+/dnvZqxUidau X-Received: by 2002:a05:620a:3956:b0:79b:b8ef:15ea with SMTP id af79cd13be357-79bb8ef1bc1mr218243185a.0.1718823885259; Wed, 19 Jun 2024 12:04:45 -0700 (PDT) X-Google-Smtp-Source: AGHT+IGCeFMZ7zbjhTYtGhWFGlaVISx3v4MKfTBkiAt+40UZLf1NQOhEkfPnOIcwFncVz4u7qVQEWg== X-Received: by 2002:a05:620a:3956:b0:79b:b8ef:15ea with SMTP id af79cd13be357-79bb8ef1bc1mr218235285a.0.1718823883440; Wed, 19 Jun 2024 12:04:43 -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 af79cd13be357-798abc0ce27sm628140785a.90.2024.06.19.12.04.42 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 19 Jun 2024 12:04:42 -0700 (PDT) Date: Wed, 19 Jun 2024 15:04:41 -0400 From: Peter Xu To: Ryan Roberts Cc: Catalin Marinas , Will Deacon , Mark Rutland , linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH v1] arm64: mm: Permit PTE SW bits to change in live mappings Message-ID: References: <20240619121859.4153966-1-ryan.roberts@arm.com> <3a42e195-9392-442f-aba7-fdd2c186b98f@arm.com> MIME-Version: 1.0 In-Reply-To: <3a42e195-9392-442f-aba7-fdd2c186b98f@arm.com> X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Disposition: inline X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240619_120450_810138_B0E084DF X-CRM114-Status: GOOD ( 35.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 Wed, Jun 19, 2024 at 04:58:32PM +0100, Ryan Roberts wrote: > The code in question is: > > if (userfaultfd_pte_wp(vma, ptep_get(vmf->pte))) { > if (!userfaultfd_wp_async(vma)) { > pte_unmap_unlock(vmf->pte, vmf->ptl); > return handle_userfault(vmf, VM_UFFD_WP); > } > > /* > * Nothing needed (cache flush, TLB invalidations, > * etc.) because we're only removing the uffd-wp bit, > * which is completely invisible to the user. > */ > pte = pte_clear_uffd_wp(ptep_get(vmf->pte)); > > set_pte_at(vma->vm_mm, vmf->address, vmf->pte, pte); > /* > * Update this to be prepared for following up CoW > * handling > */ > vmf->orig_pte = pte; > } > > Perhaps we should consider a change to the following style as a cleanup? > > old_pte = ptep_modify_prot_start(vma, addr, pte); > ptent = pte_clear_uffd_wp(old_pte); > ptep_modify_prot_commit(vma, addr, pte, old_pte, ptent); You're probably right that at least the access bit seems racy to be set here, so we may have risk of losing that when a race happened against HW. Dirty bit shouldn't be a concern in this case due to missing W bit, iiuc. IMO it's a matter of whether we'd like to "make access bit 100% accurate" when the race happened, while paying that off with an always slower generic path. Looks cleaner indeed but maybe not very beneficial in reality. > > Regardless, this patch is still a correct and valuable change; arm64 arch > doesn't care if SW bits are modified in valid mappings so we shouldn't be > checking for it. Agreed. Let's keep this discussion separate from the original patch if that already fixes stuff. > > > > >> > >> /* creating or taking down mappings is always safe */ > >> if (!pte_valid(__pte(old)) || !pte_valid(__pte(new))) > >> -- > >> 2.43.0 > >> > > > > When looking at this function I found this and caught my attention too: > > > > /* live contiguous mappings may not be manipulated at all */ > > if ((old | new) & PTE_CONT) > > return false; > > > > I'm now wondering how cont-ptes work with uffd-wp now for arm64, from > > either hugetlb or mTHP pov. This check may be relevant here as a start. > > When transitioning a block of ptes between cont and non-cont, we transition the > block through invalid with tlb invalidation. See contpte_convert(). > > > > > The other thing is since x86 doesn't have cont-ptes yet, uffd-wp didn't > > consider that, and there may be things overlooked at least from my side. > > E.g., consider wr-protect one cont-pte huge pages on hugetlb: > > > > static inline pte_t huge_pte_mkuffd_wp(pte_t pte) > > { > > return huge_pte_wrprotect(pte_mkuffd_wp(pte)); > > } > > > > I think it means so far it won't touch the rest cont-ptes but the 1st. Not > > sure whether it'll work if write happens on the rest. > > I'm not completely sure I follow your point. I think this should work correctly. > The arm64 huge_pte code knows what size (and level) the huge pte is and spreads > the passed in pte across all the HW ptes. What I was considering is about wr-protect a 64K cont-pte entry in arm64: UFFDIO_WRITEPROTECT -> hugetlb_change_protection() -> huge_pte_mkuffd_wp() What I'm expecting is huge_pte_mkuffd_wp() would wr-protect all ptes, but looks not right now. I'm not sure if the HW is able to identify "the whole 64K is wr-protected" in this case, rather than "only the 1st pte is wr-protected", as IIUC current "pte" points to only the 1st pte entry. Thanks, -- Peter Xu