From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by smtp.subspace.kernel.org (Postfix) with ESMTP id 360C41DBB3A for ; Tue, 27 Jan 2026 14:03:47 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=217.140.110.172 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1769522629; cv=none; b=SiDXHN5sZalYvkVI+6o2w05ESkE4EiT54sTj2BGZQQnvAk9zmP7ZVDxTchYSqdYcd3Tvw65p8TkCG5T5rfTzBMlBYo1pDwxP9SpjA/YZVTix8/3Mv6AZTT2SAd/DrtrLKwsiLuzjeA3fz2bsphAo2TY6+wAHrUgRfXUZoVPd+bc= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1769522629; c=relaxed/simple; bh=yT0UtRa/YyCVBgiyxYyflOFF8jYTF6DW82Qeu3cbGCM=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=YR9N8zxsPFt6+KOOFO7j2Qp+2jD2DfUcssohTInYbwSi+VjV9rqRO6IPBQC9AjGeqxgiCfqySEAj5uBZJFIWPqG3ZmvMpNEsAdYDwHXR+G6ToeacAqW2PxBtxwOGLZEl2sqaQdD4Uev4vUXHcBEWS05Jlmni+ii5QbfuWPPweCA= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=arm.com; spf=pass smtp.mailfrom=arm.com; arc=none smtp.client-ip=217.140.110.172 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=arm.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=arm.com 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 E5F3C1595; Tue, 27 Jan 2026 06:03:40 -0800 (PST) Received: from [10.1.37.210] (XHFQ2J9959.cambridge.arm.com [10.1.37.210]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 738653F73F; Tue, 27 Jan 2026 06:03:45 -0800 (PST) Message-ID: <8184baab-8774-4a73-8cee-d8d3e22553c0@arm.com> Date: Tue, 27 Jan 2026 14:03:43 +0000 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v2 12/13] arm64: mm: Wrap flush_tlb_page() around ___flush_tlb_range() Content-Language: en-GB To: Jonathan Cameron Cc: Will Deacon , Ard Biesheuvel , Catalin Marinas , Mark Rutland , Linus Torvalds , Oliver Upton , Marc Zyngier , Dev Jain , Linu Cherian , linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org References: <20260119172202.1681510-1-ryan.roberts@arm.com> <20260119172202.1681510-13-ryan.roberts@arm.com> <20260127125933.00006101@huawei.com> From: Ryan Roberts In-Reply-To: <20260127125933.00006101@huawei.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit On 27/01/2026 12:59, Jonathan Cameron wrote: > On Mon, 19 Jan 2026 17:21:59 +0000 > Ryan Roberts wrote: > >> Flushing a page from the tlb is just a special case of flushing a range. >> So let's rework flush_tlb_page() so that it simply wraps >> ___flush_tlb_range(). While at it, let's also update the API to take the >> same flags that we use when flushing a range. This allows us to delete >> all the ugly "_nosync", "_local" and "_nonotify" variants. >> >> Thanks to constant folding, all of the complex looping and tlbi-by-range >> options get eliminated so that the generated code for flush_tlb_page() >> looks very similar to the previous version. >> >> Reviewed-by: Linu Cherian >> Signed-off-by: Ryan Roberts > > So this does include the use of the > > Case TLBF_NOBROADCAST from previous patch, but only whilst (I think) > slightly changing behavior. > > Gah. I'm regretting looking at this series. The original code is really hard to > read :) Rather you than me to fix it! > >> static inline void flush_tlb_kernel_range(unsigned long start, unsigned long end) >> { >> const unsigned long stride = PAGE_SIZE; >> diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c >> index be9dab2c7d6a..f91aa686f142 100644 >> --- a/arch/arm64/mm/fault.c >> +++ b/arch/arm64/mm/fault.c >> @@ -239,7 +239,7 @@ int __ptep_set_access_flags(struct vm_area_struct *vma, >> * flush_tlb_fix_spurious_fault(). >> */ >> if (dirty) >> - local_flush_tlb_page(vma, address); >> + __flush_tlb_page(vma, address, TLBF_NOBROADCAST); > > Ultimately I think this previously did __tlbi(vale1) and now does __tlbi(vae1) > Original call was to __local_flush_tlb_page_notify_nosync() No not quite; the new code is still doing __tlbi(vale1). The trick is that the __flush_tlb_page() wrapper unconditionally adds TLBF_NOWALKCACHE to the flags. Since this API is operating on a *page* it is implicit that we should only be evicting a leaf entry (as per the old implementation). You'll see I've also updated the documentation to make that clear in tlbflush.h. Now that you have raised it, I can see how it might be confusing though, since __flush_tlb_page() does not explicitly have TLBF_NOWALKCACHE. We could require all __flush_tlb_page() callers to explicitly pass TLBF_NOWALKCACHE if you think that helps? It would still be implicit for flush_tlb_page() (the generic kernel API) though. > > I'd like to see that sort of change called out and explained in the patch description. > It's a broader scoped flush so not a bug, but still a functional change. As I say, the emitted code is the same. It's my new API that's the problem here... Thanks, Ryan > >> return 1; >> } >> >