On Tue, 19 Jul 2022 12:07:03 -0700 Dave Jiang <dave.jiang@xxxxxxxxx> wrote: > On 7/17/2022 10:30 PM, Davidlohr Bueso wrote: > > On Fri, 15 Jul 2022, Dave Jiang wrote: > > > >> The original implementation to flush all cache after unlocking the > >> nvdimm > >> resides in drivers/acpi/nfit/intel.c. This is a temporary stop gap until > >> nvdimm with security operations arrives on other archs. With support CXL > >> pmem supporting security operations, specifically "unlock" dimm, the > >> need > >> for an arch supported helper function to invalidate all CPU cache for > >> nvdimm has arrived. Remove original implementation from acpi/nfit and > >> add > >> cross arch support for this operation. > >> > >> Add CONFIG_ARCH_HAS_NVDIMM_INVAL_CACHE Kconfig and allow x86_64 to > >> opt in > >> and provide the support via wbinvd_on_all_cpus() call. > > > > So the 8.2.9.5.5 bits will also need wbinvd - and I guess arm64 will need > > its own semantics (iirc there was a flush all call in the past). Cc'ing > > Jonathan as well. > > > > Anyway, I think this call should not be defined in any place other > > than core > > kernel headers, and not in pat/nvdimm. I was trying to make it fit in > > smp.h, > > for example, but conviniently we might be able to hijack > > flush_cache_all() > > for our purposes as of course neither x86-64 arm64 uses it :) > > > > And I see this as safe (wrt not adding a big hammer on unaware > > drivers) as > > the 32bit archs that define the call are mostly contained thin their > > arch/, > > and the few in drivers/ are still specific to those archs. > > > > Maybe something like the below. > > Ok. I'll replace my version with yours. Careful with flush_cache_all(). The stub version in include/asm-generic/cacheflush.h has a comment above it that would need updating at very least (I think). Note there 'was' a flush_cache_all() for ARM64, but: https://patchwork.kernel.org/project/linux-arm-kernel/patch/1429521875-16893-1-git-send-email-mark.rutland@xxxxxxx/ Also, I'm far from sure it will be the right choice on all CXL supporting architectures. +CC linux-arch, linux-arm and Arnd. > > > > > > Thanks, > > Davidlohr > > > > ------8<---------------------------------------- > > Subject: [PATCH] arch/x86: define flush_cache_all as global wbinvd > > > > With CXL security features, global CPU cache flushing nvdimm > > requirements are no longer specific to that subsystem, even > > beyond the scope of security_ops. CXL will need such semantics > > for features not necessarily limited to persistent memory. > > > > So use the flush_cache_all() for the wbinvd across all > > CPUs on x86. arm64, which is another platform to have CXL > > support can also define its own semantics here. > > > > Signed-off-by: Davidlohr Bueso <dave@xxxxxxxxxxxx> > > --- > > arch/x86/Kconfig | 1 - > > arch/x86/include/asm/cacheflush.h | 5 +++++ > > arch/x86/mm/pat/set_memory.c | 8 -------- > > drivers/acpi/nfit/intel.c | 11 ++++++----- > > drivers/cxl/security.c | 5 +++-- > > include/linux/libnvdimm.h | 9 --------- > > 6 files changed, 14 insertions(+), 25 deletions(-) > > > > diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig > > index 8dbe89eba639..be0b95e51df6 100644 > > --- a/arch/x86/Kconfig > > +++ b/arch/x86/Kconfig > > @@ -83,7 +83,6 @@ config X86 > > select ARCH_HAS_MEMBARRIER_SYNC_CORE > > select ARCH_HAS_NON_OVERLAPPING_ADDRESS_SPACE > > select ARCH_HAS_PMEM_API if X86_64 > > - select ARCH_HAS_NVDIMM_INVAL_CACHE if X86_64 > > select ARCH_HAS_PTE_DEVMAP if X86_64 > > select ARCH_HAS_PTE_SPECIAL > > select ARCH_HAS_UACCESS_FLUSHCACHE if X86_64 > > diff --git a/arch/x86/include/asm/cacheflush.h > > b/arch/x86/include/asm/cacheflush.h > > index b192d917a6d0..05c79021665d 100644 > > --- a/arch/x86/include/asm/cacheflush.h > > +++ b/arch/x86/include/asm/cacheflush.h > > @@ -10,4 +10,9 @@ > > > > void clflush_cache_range(void *addr, unsigned int size); > > > > +#define flush_cache_all() \ > > +do { \ > > + wbinvd_on_all_cpus(); \ > > +} while (0) > > + > > #endif /* _ASM_X86_CACHEFLUSH_H */ > > diff --git a/arch/x86/mm/pat/set_memory.c b/arch/x86/mm/pat/set_memory.c > > index e4cd1286deef..1abd5438f126 100644 > > --- a/arch/x86/mm/pat/set_memory.c > > +++ b/arch/x86/mm/pat/set_memory.c > > @@ -330,14 +330,6 @@ void arch_invalidate_pmem(void *addr, size_t size) > > EXPORT_SYMBOL_GPL(arch_invalidate_pmem); > > #endif > > > > -#ifdef CONFIG_ARCH_HAS_NVDIMM_INVAL_CACHE > > -void arch_invalidate_nvdimm_cache(void) > > -{ > > - wbinvd_on_all_cpus(); > > -} > > -EXPORT_SYMBOL_GPL(arch_invalidate_nvdimm_cache); > > -#endif > > - > > static void __cpa_flush_all(void *arg) > > { > > unsigned long cache = (unsigned long)arg; > > diff --git a/drivers/acpi/nfit/intel.c b/drivers/acpi/nfit/intel.c > > index 242d2e9203e9..1b0ecb4d67e6 100644 > > --- a/drivers/acpi/nfit/intel.c > > +++ b/drivers/acpi/nfit/intel.c > > @@ -1,6 +1,7 @@ > > // SPDX-License-Identifier: GPL-2.0 > > /* Copyright(c) 2018 Intel Corporation. All rights reserved. */ > > #include <linux/libnvdimm.h> > > +#include <linux/cacheflush.h> > > #include <linux/ndctl.h> > > #include <linux/acpi.h> > > #include <asm/smp.h> > > @@ -226,7 +227,7 @@ static int __maybe_unused > > intel_security_unlock(struct nvdimm *nvdimm, > > } > > > > /* DIMM unlocked, invalidate all CPU caches before we read it */ > > - arch_invalidate_nvdimm_cache(); > > + flush_cache_all(); > > > > return 0; > > } > > @@ -296,7 +297,7 @@ static int __maybe_unused > > intel_security_erase(struct nvdimm *nvdimm, > > return -ENOTTY; > > > > /* flush all cache before we erase DIMM */ > > - arch_invalidate_nvdimm_cache(); > > + flush_cache_all(); > > memcpy(nd_cmd.cmd.passphrase, key->data, > > sizeof(nd_cmd.cmd.passphrase)); > > rc = nvdimm_ctl(nvdimm, ND_CMD_CALL, &nd_cmd, sizeof(nd_cmd), NULL); > > @@ -316,7 +317,7 @@ static int __maybe_unused > > intel_security_erase(struct nvdimm *nvdimm, > > } > > > > /* DIMM erased, invalidate all CPU caches before we read it */ > > - arch_invalidate_nvdimm_cache(); > > + flush_cache_all(); > > return 0; > > } > > > > @@ -353,7 +354,7 @@ static int __maybe_unused > > intel_security_query_overwrite(struct nvdimm *nvdimm) > > } > > > > /* flush all cache before we make the nvdimms available */ > > - arch_invalidate_nvdimm_cache(); > > + flush_cache_all(); > > return 0; > > } > > > > @@ -379,7 +380,7 @@ static int __maybe_unused > > intel_security_overwrite(struct nvdimm *nvdimm, > > return -ENOTTY; > > > > /* flush all cache before we erase DIMM */ > > - arch_invalidate_nvdimm_cache(); > > + flush_cache_all(); > > memcpy(nd_cmd.cmd.passphrase, nkey->data, > > sizeof(nd_cmd.cmd.passphrase)); > > rc = nvdimm_ctl(nvdimm, ND_CMD_CALL, &nd_cmd, sizeof(nd_cmd), NULL); > > diff --git a/drivers/cxl/security.c b/drivers/cxl/security.c > > index 3dc04b50afaf..e2977872bf2f 100644 > > --- a/drivers/cxl/security.c > > +++ b/drivers/cxl/security.c > > @@ -6,6 +6,7 @@ > > #include <linux/ndctl.h> > > #include <linux/async.h> > > #include <linux/slab.h> > > +#include <linux/cacheflush.h> > > #include "cxlmem.h" > > #include "cxl.h" > > > > @@ -137,7 +138,7 @@ static int cxl_pmem_security_unlock(struct nvdimm > > *nvdimm, > > return rc; > > > > /* DIMM unlocked, invalidate all CPU caches before we read it */ > > - arch_invalidate_nvdimm_cache(); > > + flush_cache_all(); > > return 0; > > } > > > > @@ -165,7 +166,7 @@ static int > > cxl_pmem_security_passphrase_erase(struct nvdimm *nvdimm, > > return rc; > > > > /* DIMM erased, invalidate all CPU caches before we read it */ > > - arch_invalidate_nvdimm_cache(); > > + flush_cache_all(); > > return 0; > > } > > > > diff --git a/include/linux/libnvdimm.h b/include/linux/libnvdimm.h > > index 07e4e7572089..0769afb73380 100644 > > --- a/include/linux/libnvdimm.h > > +++ b/include/linux/libnvdimm.h > > @@ -309,13 +309,4 @@ static inline void arch_invalidate_pmem(void > > *addr, size_t size) > > { > > } > > #endif > > - > > -#ifdef CONFIG_ARCH_HAS_NVDIMM_INVAL_CACHE > > -void arch_invalidate_nvdimm_cache(void); > > -#else > > -static inline void arch_invalidate_nvdimm_cache(void) > > -{ > > -} > > -#endif > > - > > #endif /* __LIBNVDIMM_H__ */ > > -- > > 2.36.1 > > >
