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 23FB0C83F1A for ; Thu, 10 Jul 2025 07:15:24 +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-Transfer-Encoding: Content-Type:MIME-Version:Message-ID:References:In-Reply-To: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=FwvuqdG3hYjFpKWdCMq6tprnniSoRmedFOTBUf2WREU=; b=q4dGJ9CNgVZj1pskNS+ljut2ne qyEwLoqkskSFIK8W9EZ3btSfT5J2gEvF9DnpUKq2CkpTXriz6podXjdRnClXTCjAJvpo8jgsk+UTW ufGQcv/5xlJYaMVdg1Aj5rXbG8FO482o1oNqN/dZk5WNlqXHeyG124wZSKuhEUfFS6/tsLO/XhaHj n14nvlPb3ZKK3wmqo5rj1K92AyTksT6Urbjq9RTZB5wNHMdOww5oa8oWjKUFBxkF/tsiVltrd+4Wj b0e4n71Lg71Woi4lpedyNN/CxLqcEuupP51q4k0uzTHUR3605SPtql0e8fuQ3etr69CpqY8DpxO34 mltuocNA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.98.2 #2 (Red Hat Linux)) id 1uZlUz-0000000AyIv-3LmZ; Thu, 10 Jul 2025 07:15:17 +0000 Received: from terminus.zytor.com ([2607:7c80:54:3::136] helo=mail.zytor.com) by bombadil.infradead.org with esmtps (Exim 4.98.2 #2 (Red Hat Linux)) id 1uZkMj-0000000Amnp-2aDg for linux-arm-kernel@lists.infradead.org; Thu, 10 Jul 2025 06:02:43 +0000 Received: from [127.0.0.1] (c-76-133-66-138.hsd1.ca.comcast.net [76.133.66.138]) (authenticated bits=0) by mail.zytor.com (8.18.1/8.17.1) with ESMTPSA id 56A61oVv420818 (version=TLSv1.3 cipher=TLS_AES_128_GCM_SHA256 bits=128 verify=NO); Wed, 9 Jul 2025 23:01:51 -0700 DKIM-Filter: OpenDKIM Filter v2.11.0 mail.zytor.com 56A61oVv420818 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=zytor.com; s=2025062101; t=1752127312; bh=FwvuqdG3hYjFpKWdCMq6tprnniSoRmedFOTBUf2WREU=; h=Date:From:To:CC:Subject:In-Reply-To:References:From; b=cOydII2KcDOQTmWFVW0q5/KLzpEOqFpnyHLLxS1JoRZ9+vwP74kNGSFenusS+FFcP AEfl38tkTa9xogtA+khkrfI6ZWbQzsmHE0ZPh+WeUwrDPxEI/R/Coyn8N2zABaBbtD M6xldWlsVv1wyMc8O1oSS9m8xb0YYaolroX+DZWm1m1XY368ArgAvAcDGe6lKada1G ae563v8hzpdM5IpThABUh4dEge45JPjPxbFEu1oU1ASebBU41KVM9H7Vm+u9vRV7zM 42f9PNFNaiuU6dalJAnxPIEb3r1r4zxs4wAdA+dIKxB405GoBTdW4/BJMwzT6Vkmpk qpTCeEG5Byz5A== Date: Wed, 09 Jul 2025 23:01:50 -0700 From: "H. Peter Anvin" To: dan.j.williams@intel.com, Jonathan Cameron , Catalin Marinas , james.morse@arm.com, linux-cxl@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-acpi@vger.kernel.org, linux-arch@vger.kernel.org, linux-mm@kvack.org, gregkh@linuxfoundation.org, Will Deacon , Dan Williams , Davidlohr Bueso CC: Yicong Yang , linuxarm@huawei.com, Yushan Wang , Lorenzo Pieralisi , Mark Rutland , Dave Hansen , Thomas Gleixner , Ingo Molnar , Borislav Petkov , x86@kernel.org, Andy Lutomirski , Peter Zijlstra Subject: =?US-ASCII?Q?Re=3A_=5BPATCH_v2_2/8=5D_generic=3A_Support_A?= =?US-ASCII?Q?RCH=5FHAS=5FCPU=5FCACHE=5FINVALIDATE=5FMEMREGION?= User-Agent: K-9 Mail for Android In-Reply-To: <686f565121ea5_1d3d100ee@dwillia2-xfh.jf.intel.com.notmuch> References: <20250624154805.66985-1-Jonathan.Cameron@huawei.com> <20250624154805.66985-3-Jonathan.Cameron@huawei.com> <686f565121ea5_1d3d100ee@dwillia2-xfh.jf.intel.com.notmuch> Message-ID: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20250709_230241_649784_63F29FFB X-CRM114-Status: GOOD ( 26.05 ) 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 July 9, 2025 10:57:37 PM PDT, dan=2Ej=2Ewilliams@intel=2Ecom wrote: >Jonathan Cameron wrote: >> From: Yicong Yang >>=20 >> ARCH_HAS_CPU_CACHE_INVALIDATE_MEMREGION provides the mechanism for >> invalidate certain memory regions in a cache-incoherent manner=2E >> Currently is used by NVIDMM adn CXL memory=2E This is mainly done >> by the system component and is implementation define per spec=2E >> Provides a method for the platforms register their own invalidate >> method and implement ARCH_HAS_CPU_CACHE_INVALIDATE_MEMREGION=2E > >Please run spell-check on changelogs=2E > >>=20 >> Architectures can opt in for this support via >> CONFIG_GENERIC_CPU_CACHE_INVALIDATE_MEMREGION=2E >>=20 >> Signed-off-by: Yicong Yang >> Signed-off-by: Jonathan Cameron >> --- >> drivers/base/Kconfig | 3 +++ >> drivers/base/Makefile | 1 + >> drivers/base/cache=2Ec | 46 ++++++++++++++++++++++++++++++= ++ > >I do not understand what any of this has to do with drivers/base/=2E > >See existing cache management memcpy infrastructure in lib/Kconfig=2E > >> include/asm-generic/cacheflush=2Eh | 12 +++++++++ >> 4 files changed, 62 insertions(+) >>=20 >> diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig >> index 064eb52ff7e2=2E=2Ecc6df87a0a96 100644 >> --- a/drivers/base/Kconfig >> +++ b/drivers/base/Kconfig >> @@ -181,6 +181,9 @@ config SYS_HYPERVISOR >> bool >> default n >> =20 >> +config GENERIC_CPU_CACHE_INVALIDATE_MEMREGION >> + bool >> + >> config GENERIC_CPU_DEVICES >> bool >> default n >> diff --git a/drivers/base/Makefile b/drivers/base/Makefile >> index 8074a10183dc=2E=2E0fbfa4300b98 100644 >> --- a/drivers/base/Makefile >> +++ b/drivers/base/Makefile >> @@ -26,6 +26,7 @@ obj-$(CONFIG_DEV_COREDUMP) +=3D devcoredump=2Eo >> obj-$(CONFIG_GENERIC_MSI_IRQ) +=3D platform-msi=2Eo >> obj-$(CONFIG_GENERIC_ARCH_TOPOLOGY) +=3D arch_topology=2Eo >> obj-$(CONFIG_GENERIC_ARCH_NUMA) +=3D arch_numa=2Eo >> +obj-$(CONFIG_GENERIC_CPU_CACHE_INVALIDATE_MEMREGION) +=3D cache=2Eo >> obj-$(CONFIG_ACPI) +=3D physical_location=2Eo >> =20 >> obj-y +=3D test/ >> diff --git a/drivers/base/cache=2Ec b/drivers/base/cache=2Ec >> new file mode 100644 >> index 000000000000=2E=2E8d351657bbef >> --- /dev/null >> +++ b/drivers/base/cache=2Ec >> @@ -0,0 +1,46 @@ >> +// SPDX-License-Identifier: GPL-2=2E0 >> +/* >> + * Generic support for CPU Cache Invalidate Memregion >> + */ >> + >> +#include >> +#include >> +#include >> + >> + >> +static const struct system_cache_flush_method *scfm_data; >> +DEFINE_SPINLOCK(scfm_lock); >> + >> +void generic_set_sys_cache_flush_method(const struct system_cache_flus= h_method *method) >> +{ >> + guard(spinlock_irqsave)(&scfm_lock); >> + if (scfm_data || !method || !method->invalidate_memregion) >> + return; >> + >> + scfm_data =3D method; > >The lock looks unnecessary here, this is just atomic_cmpxchg()=2E > >> +} >> +EXPORT_SYMBOL_GPL(generic_set_sys_cache_flush_method); >> + >> +void generic_clr_sys_cache_flush_method(const struct system_cache_flus= h_method *method) >> +{ >> + guard(spinlock_irqsave)(&scfm_lock); >> + if (scfm_data && scfm_data =3D=3D method) >> + scfm_data =3D NULL; > >Same here, but really what is missing is a description of the locking >requirements of cpu_cache_invalidate_memregion()=2E > > >> +} >> + >> +int cpu_cache_invalidate_memregion(int res_desc, phys_addr_t start, si= ze_t len) >> +{ >> + guard(spinlock_irqsave)(&scfm_lock); >> + if (!scfm_data) >> + return -EOPNOTSUPP; >> + >> + return scfm_data->invalidate_memregion(res_desc, start, len); > >Is it really the case that you need to disable interrupts during cache >operations? For potentially flushing 10s to 100s of gigabytes, is it >really the case that all archs can support holding interrupts off for >that event? > >A read lock (rcu or rwsem) seems sufficient to maintain registration >until the invalidate operation completes=2E > >If an arch does need to disable interrupts while it manages caches that >does not feel like something that should be enforced for everyone at >this top-level entry point=2E > >> +} >> +EXPORT_SYMBOL_NS_GPL(cpu_cache_invalidate_memregion, "DEVMEM"); >> + >> +bool cpu_cache_has_invalidate_memregion(void) >> +{ >> + guard(spinlock_irqsave)(&scfm_lock); >> + return !!scfm_data; > >Lock seems pointless here=2E > >More concerning is this diverges from the original intent of this >function which was to disable physical address space manipulation from >virtual environments=2E > >Now, different archs may have reason to diverge here but the fact that >the API requirements are non-obvious points at a minimum to missing >documentation if not missing cross-arch consensus=2E > >> +} >> +EXPORT_SYMBOL_NS_GPL(cpu_cache_has_invalidate_memregion, "DEVMEM"); >> diff --git a/include/asm-generic/cacheflush=2Eh b/include/asm-generic/c= acheflush=2Eh >> index 7ee8a179d103=2E=2E87e64295561e 100644 >> --- a/include/asm-generic/cacheflush=2Eh >> +++ b/include/asm-generic/cacheflush=2Eh >> @@ -124,4 +124,16 @@ static inline void flush_cache_vunmap(unsigned lon= g start, unsigned long end) >> } while (0) >> #endif >> =20 >> +#ifdef CONFIG_GENERIC_CPU_CACHE_INVALIDATE_MEMREGION >> + >> +struct system_cache_flush_method { >> + int (*invalidate_memregion)(int res_desc, >> + phys_addr_t start, size_t len); >> +}; > >The whole point of ARCH_HAS facilities is to resolve symbols like this >at compile time=2E Why does this need a indirect function call at all? Yes, blocking interrupts is much like the problem with WBINVD=2E More or less, once user space is running, this isn't acceptable=2E