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 kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id B8644C8303C for ; Fri, 11 Jul 2025 11:53:35 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 56EAC6B009B; Fri, 11 Jul 2025 07:53:35 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 51F346B009E; Fri, 11 Jul 2025 07:53:35 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 45C1D6B00A3; Fri, 11 Jul 2025 07:53:35 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id 32F4F6B009B for ; Fri, 11 Jul 2025 07:53:35 -0400 (EDT) Received: from smtpin24.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id D215380A02 for ; Fri, 11 Jul 2025 11:53:34 +0000 (UTC) X-FDA: 83651823948.24.E75B7A3 Received: from frasgout.his.huawei.com (frasgout.his.huawei.com [185.176.79.56]) by imf26.hostedemail.com (Postfix) with ESMTP id DC51614000A for ; Fri, 11 Jul 2025 11:53:32 +0000 (UTC) Authentication-Results: imf26.hostedemail.com; dkim=none; dmarc=pass (policy=quarantine) header.from=huawei.com; spf=pass (imf26.hostedemail.com: domain of jonathan.cameron@huawei.com designates 185.176.79.56 as permitted sender) smtp.mailfrom=jonathan.cameron@huawei.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1752234813; a=rsa-sha256; cv=none; b=fslWu3F3/P6n9bJ7i8IX0a0+lRc9QbpUsY+6V3J8eQnynMDtA3hGIN3zJ+GCVO0TSBCmnW Cx2NYLS/uknf9qtHVmtaGAGUgXYFzz7IhWi1Af083ruh7a+5EV2Add189BNjctmk/EJ3hR n4Ylou0ls8QT1x6PefY0In+iH7QmCHI= ARC-Authentication-Results: i=1; imf26.hostedemail.com; dkim=none; dmarc=pass (policy=quarantine) header.from=huawei.com; spf=pass (imf26.hostedemail.com: domain of jonathan.cameron@huawei.com designates 185.176.79.56 as permitted sender) smtp.mailfrom=jonathan.cameron@huawei.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1752234813; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=q0a74CRDfle7fzEwfUGNPCxbgsBDZp3A3culNU1YvnU=; b=xWXiyPIWCQTgVkWQJFSeo0bqvkhuYdof+dJ71C2XhRXvcG3nQBRx0I/A2I1oYomv3Xxujk C+pPnCjKeYaUxFdndQ5fvZ8y9gr82QCjAcK1qpd4iiyvPuKdbSAHDyFpryd2ybdmUROLpv z3XRUYRMrmPKtjd0dWuJxAtYxYkgxvo= Received: from mail.maildlp.com (unknown [172.18.186.31]) by frasgout.his.huawei.com (SkyGuard) with ESMTP id 4bdqlT12Ykz6L53x; Fri, 11 Jul 2025 19:50:13 +0800 (CST) Received: from frapeml500008.china.huawei.com (unknown [7.182.85.71]) by mail.maildlp.com (Postfix) with ESMTPS id 1F3E01400D3; Fri, 11 Jul 2025 19:53:31 +0800 (CST) Received: from localhost (10.203.177.66) by frapeml500008.china.huawei.com (7.182.85.71) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.2507.39; Fri, 11 Jul 2025 13:53:30 +0200 Date: Fri, 11 Jul 2025 12:53:28 +0100 From: Jonathan Cameron To: "H. Peter Anvin" CC: , Catalin Marinas , , , , , , , , Will Deacon , Davidlohr Bueso , "Yicong Yang" , , Yushan Wang , Lorenzo Pieralisi , "Mark Rutland" , Dave Hansen , Thomas Gleixner , Ingo Molnar , Borislav Petkov , , Andy Lutomirski , Peter Zijlstra Subject: Re: [PATCH v2 2/8] generic: Support ARCH_HAS_CPU_CACHE_INVALIDATE_MEMREGION Message-ID: <20250711125328.00002334@huawei.com> In-Reply-To: References: <20250624154805.66985-1-Jonathan.Cameron@huawei.com> <20250624154805.66985-3-Jonathan.Cameron@huawei.com> <686f565121ea5_1d3d100ee@dwillia2-xfh.jf.intel.com.notmuch> X-Mailer: Claws Mail 4.3.0 (GTK 3.24.42; x86_64-w64-mingw32) MIME-Version: 1.0 Content-Type: text/plain; charset="US-ASCII" Content-Transfer-Encoding: 7bit X-Originating-IP: [10.203.177.66] X-ClientProxiedBy: lhrpeml500001.china.huawei.com (7.191.163.213) To frapeml500008.china.huawei.com (7.182.85.71) X-Rspamd-Queue-Id: DC51614000A X-Stat-Signature: oci6ihzkiw6oajrqf4bitpgwjgkejysd X-Rspam-User: X-Rspamd-Server: rspam10 X-HE-Tag: 1752234812-227499 X-HE-Meta: U2FsdGVkX18UIURU7fthv4FNv/5EpiXI9rfCt+T7VpL/Og2K+Flw8vzSGW1MHI/grHrl2JvfkjxahhSwxmdrbtLHepCggitYZcHYrIXMFjWv1+MATDJH46tzI9QzsmPg98BXqcaAWlCmtHesMaKDnGjnGZs2WHSECIviTdzojjE3MbZM4/8wAd2KrDm3Isuzv/09+D1/BUPiAYdqD5EymQxWqJ+/cCFIU2YWKSsN07i1GEZGITXHD43vOaNpAgp6rmZS+AuCTDUS1MJ+oHLilAdM+7Cmmp551LoYBUiAwxUBuU33XfVL0Ok551cVZ1WmVVB6nI4YppIswvgkeCy8bKkPC378oLmpAk9FquLo/7LCKdv8jJ7K0pDJMN/5plujIyg2b/R26NmvDSxtXrsGVU6HaIlYLyenD0LpGAK1INPDUh5EpQQYxD4638jjfiRcVm2+wc17acxVYu7y0yV70W+bLxLMMF0nZ3I/u44KQg0QRKiE3ha0dZEs+8z8QW0QfnaOHlh0bpqt5BDPgaaXuq6CxbdxAPrS2strrm5w1rEFBc56XDNHh5FpH6OJVp+XD1/64/fi8PoPYSpClh2AR/FgiDbxuU0nwPmiWAM2uokvjFsJtnoOr4pqWC2YNjfs2eHUMq6W8eA5R2zZZDPK6m2ULljiENZU62Lw3cRS86d5WFRJ3WFsp/SgDGI2ENdYfWE+KCVjbLLBy7WJw4/lrCIhaHDmN5vH/0Vjf7OwE0ahSm6Y01Uf31YkEWKVJrhxqqKBgvm/dNwkW2M5h3AVDWOefREpWFQlrOmhtDttMEtmNsi52Du+W7YL9t06jgfS2fwSGa2Y6jg2yhxbLs0rq7C7NKjlSDVrZAs2GRsaa28gcA6txAVMfazgaPYaN5bsLT1SneDmFxiNi2AcN84p6jree1/CxR26A2YWgHFq4C0VQBg9MEi+5/UvsVOOcEGY1DwGMmPyESDpiz0kZxb Lm0dNbDt eFm/RS0exaqD0Rp6TltVjhxrfjIZRg7NCwks2TZOrWRbaD5GtCszjPPm1Z7clONx3j9GSTR7YGecfod6boACuHY4o4WLkQr4/a6BY5/ZC1hM6rGXh8OdNSaU7CE2eAcqTdU9Jz6MFjRFwYj1JPnUqxIeHSwOjeY9g/0tvprlDtKjtXJMMr+M0zcZk2JO0Csr6SzZ7LZicZYh4N99tXzCQZrrwIRpotrlHcSpifeUR8YmY81sRE7Q0Pxsv43mo9eJaDE20nUj0S7GUa3x3RTeHQeoOhxXScmv8aSdopFATyxaMV5pXFQkxQNHMuaULFPHFPw1xNDCvmcxcBDCOa3iIcFdA3wFqqNDGtv1SkssdTdaOkEMLKnupA+YicUYL2WgEh8NE2Os35kh+U45u7vMHxlJUdq0rjTLm88cGs9PaDvZT+vHWAETeNKstSw== X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: On Wed, 09 Jul 2025 23:01:50 -0700 "H. Peter Anvin" wrote: > On July 9, 2025 10:57:37 PM PDT, dan.j.williams@intel.com wrote: > >Jonathan Cameron wrote: > >> From: Yicong Yang > >> > >> ARCH_HAS_CPU_CACHE_INVALIDATE_MEMREGION provides the mechanism for > >> invalidate certain memory regions in a cache-incoherent manner. > >> Currently is used by NVIDMM adn CXL memory. This is mainly done > >> by the system component and is implementation define per spec. > >> Provides a method for the platforms register their own invalidate > >> method and implement ARCH_HAS_CPU_CACHE_INVALIDATE_MEMREGION. > > > >Please run spell-check on changelogs. > > > >> > >> Architectures can opt in for this support via > >> CONFIG_GENERIC_CPU_CACHE_INVALIDATE_MEMREGION. > >> > >> Signed-off-by: Yicong Yang > >> Signed-off-by: Jonathan Cameron > >> --- > >> drivers/base/Kconfig | 3 +++ > >> drivers/base/Makefile | 1 + > >> drivers/base/cache.c | 46 ++++++++++++++++++++++++++++++++ > > > >I do not understand what any of this has to do with drivers/base/. > > > >See existing cache management memcpy infrastructure in lib/Kconfig. > > > >> include/asm-generic/cacheflush.h | 12 +++++++++ > >> 4 files changed, 62 insertions(+) > >> > >> diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig > >> index 064eb52ff7e2..cc6df87a0a96 100644 > >> --- a/drivers/base/Kconfig > >> +++ b/drivers/base/Kconfig > >> @@ -181,6 +181,9 @@ config SYS_HYPERVISOR > >> bool > >> default n > >> > >> +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..0fbfa4300b98 100644 > >> --- a/drivers/base/Makefile > >> +++ b/drivers/base/Makefile > >> @@ -26,6 +26,7 @@ obj-$(CONFIG_DEV_COREDUMP) += devcoredump.o > >> obj-$(CONFIG_GENERIC_MSI_IRQ) += platform-msi.o > >> obj-$(CONFIG_GENERIC_ARCH_TOPOLOGY) += arch_topology.o > >> obj-$(CONFIG_GENERIC_ARCH_NUMA) += arch_numa.o > >> +obj-$(CONFIG_GENERIC_CPU_CACHE_INVALIDATE_MEMREGION) += cache.o > >> obj-$(CONFIG_ACPI) += physical_location.o > >> > >> obj-y += test/ > >> diff --git a/drivers/base/cache.c b/drivers/base/cache.c > >> new file mode 100644 > >> index 000000000000..8d351657bbef > >> --- /dev/null > >> +++ b/drivers/base/cache.c > >> @@ -0,0 +1,46 @@ > >> +// SPDX-License-Identifier: GPL-2.0 > >> +/* > >> + * 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_flush_method *method) > >> +{ > >> + guard(spinlock_irqsave)(&scfm_lock); > >> + if (scfm_data || !method || !method->invalidate_memregion) > >> + return; > >> + > >> + scfm_data = method; > > > >The lock looks unnecessary here, this is just atomic_cmpxchg(). > > > >> +} > >> +EXPORT_SYMBOL_GPL(generic_set_sys_cache_flush_method); > >> + > >> +void generic_clr_sys_cache_flush_method(const struct system_cache_flush_method *method) > >> +{ > >> + guard(spinlock_irqsave)(&scfm_lock); > >> + if (scfm_data && scfm_data == method) > >> + scfm_data = NULL; > > > >Same here, but really what is missing is a description of the locking > >requirements of cpu_cache_invalidate_memregion(). > > > > > >> +} > >> + > >> +int cpu_cache_invalidate_memregion(int res_desc, phys_addr_t start, size_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. > > > >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. > > > >> +} > >> +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. > > > >More concerning is this diverges from the original intent of this > >function which was to disable physical address space manipulation from > >virtual environments. > > > >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. > > > >> +} > >> +EXPORT_SYMBOL_NS_GPL(cpu_cache_has_invalidate_memregion, "DEVMEM"); > >> diff --git a/include/asm-generic/cacheflush.h b/include/asm-generic/cacheflush.h > >> index 7ee8a179d103..87e64295561e 100644 > >> --- a/include/asm-generic/cacheflush.h > >> +++ b/include/asm-generic/cacheflush.h > >> @@ -124,4 +124,16 @@ static inline void flush_cache_vunmap(unsigned long start, unsigned long end) > >> } while (0) > >> #endif > >> > >> +#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. Why does this need a indirect function call at all? > > Yes, blocking interrupts is much like the problem with WBINVD. > > More or less, once user space is running, this isn't acceptable. It's a bug that I missed in dragging this from a very different implementation. Will fix for v3. J >