| Message ID | 165791937063.2491387.15277418618265930924.stgit@djiang5-desk3.ch.intel.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Introduce security commands for CXL pmem device | expand |
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. 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@stgolabs.net> --- 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
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. > > 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@stgolabs.net> > --- > 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 >
On Tue, 19 Jul 2022 12:07:03 -0700 Dave Jiang <dave.jiang@intel.com> 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@arm.com/ 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@stgolabs.net> > > --- > > 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 > > >
On 8/3/2022 10:37 AM, Jonathan Cameron wrote: > On Tue, 19 Jul 2022 12:07:03 -0700 > Dave Jiang <dave.jiang@intel.com> 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@arm.com/ flush_and_invalidate_cache_all() instead given it calls wbinvd on x86? I think other archs, at least ARM, those are separate instructions aren't they? > > 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@stgolabs.net> >>> --- >>> 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 >>>
On Tue, Aug 09, 2022 at 02:47:06PM -0700, Dave Jiang wrote: > > On 8/3/2022 10:37 AM, Jonathan Cameron wrote: > > On Tue, 19 Jul 2022 12:07:03 -0700 > > Dave Jiang <dave.jiang@intel.com> 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@arm.com/ > > > flush_and_invalidate_cache_all() instead given it calls wbinvd on x86? I > think other archs, at least ARM, those are separate instructions aren't > they? On arm and arm64 there is no way to perform maintenance on *all* caches; it has to be done in cacheline increments by address. It's not realistic to do that for the entire address space, so we need to know the relevant address ranges (as per the commit referenced above). So we probably need to think a bit harder about the geenric interface, since "all" isn't possible to implement. :/ Thanks, Mark. > > > > > 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@stgolabs.net> > > > > --- > > > > 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
On 8/10/2022 10:15 AM, Mark Rutland wrote: > On Tue, Aug 09, 2022 at 02:47:06PM -0700, Dave Jiang wrote: >> >> On 8/3/2022 10:37 AM, Jonathan Cameron wrote: >>> On Tue, 19 Jul 2022 12:07:03 -0700 >>> Dave Jiang <dave.jiang@intel.com> 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@arm.com/ >> >> >> flush_and_invalidate_cache_all() instead given it calls wbinvd on x86? I >> think other archs, at least ARM, those are separate instructions aren't >> they? > > On arm and arm64 there is no way to perform maintenance on *all* caches; it has > to be done in cacheline increments by address. It's not realistic to do that > for the entire address space, so we need to know the relevant address ranges > (as per the commit referenced above). > > So we probably need to think a bit harder about the geenric interface, since > "all" isn't possible to implement. :/ Can you not do flushing by set and way on each cache, probably working outwards from L1? Eliot Moss
On Wed, Aug 10, 2022 at 10:31:12AM -0400, Eliot Moss wrote: > On 8/10/2022 10:15 AM, Mark Rutland wrote: > > On Tue, Aug 09, 2022 at 02:47:06PM -0700, Dave Jiang wrote: > > > > > > On 8/3/2022 10:37 AM, Jonathan Cameron wrote: > > > > On Tue, 19 Jul 2022 12:07:03 -0700 > > > > Dave Jiang <dave.jiang@intel.com> 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@arm.com/ > > > > > > > > > flush_and_invalidate_cache_all() instead given it calls wbinvd on x86? I > > > think other archs, at least ARM, those are separate instructions aren't > > > they? > > > > On arm and arm64 there is no way to perform maintenance on *all* caches; it has > > to be done in cacheline increments by address. It's not realistic to do that > > for the entire address space, so we need to know the relevant address ranges > > (as per the commit referenced above). > > > > So we probably need to think a bit harder about the geenric interface, since > > "all" isn't possible to implement. :/ > > Can you not do flushing by set and way on each cache, > probably working outwards from L1? Unfortunately, for a number of reasons, that doeesn't work. For better or worse, the *only* way which is guaranteed to work is to do this by address. If you look at the latest ARM ARM (ARM DDI 0487H.a): https://developer.arm.com/documentation/ddi0487/ha/ ... on page D4-4754, in the block "Example code for cache maintenance instructions", there's note with a treatise on this. The gist is that: * Set/Way ops are only guaranteed to affect the caches local to the CPU issuing them, and are not guaranteed to affect caches owned by other CPUs. * Set/Way ops are not guaranteed to affect system-level caches, which are fairly popular these days (whereas VA ops are required to affect those). * Set/Way ops race with the natural behaviour of caches (so e.g. a line could bounce between layers of cache, or between caches in the system, and avoid being operated upon). So unless you're on a single CPU system, with translation disabled, and you *know* that there are no system-level caches, you can't rely upon Set/Way ops to do anything useful. They're really there for firmware to use for IMPLEMENTATION DEFINED power-up and power-down sequences, and aren'y useful to portable code. Thanks, Mark.
On 8/10/2022 2:09 PM, Mark Rutland wrote: > On Wed, Aug 10, 2022 at 10:31:12AM -0400, Eliot Moss wrote: >> On 8/10/2022 10:15 AM, Mark Rutland wrote: >>> On Tue, Aug 09, 2022 at 02:47:06PM -0700, Dave Jiang wrote: >>>> >>>> On 8/3/2022 10:37 AM, Jonathan Cameron wrote: >>>>> On Tue, 19 Jul 2022 12:07:03 -0700 >>>>> Dave Jiang <dave.jiang@intel.com> 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@arm.com/ >>>> >>>> >>>> flush_and_invalidate_cache_all() instead given it calls wbinvd on x86? I >>>> think other archs, at least ARM, those are separate instructions aren't >>>> they? >>> >>> On arm and arm64 there is no way to perform maintenance on *all* caches; it has >>> to be done in cacheline increments by address. It's not realistic to do that >>> for the entire address space, so we need to know the relevant address ranges >>> (as per the commit referenced above). >>> >>> So we probably need to think a bit harder about the geenric interface, since >>> "all" isn't possible to implement. :/ >> >> Can you not do flushing by set and way on each cache, >> probably working outwards from L1? > > Unfortunately, for a number of reasons, that doeesn't work. For better or > worse, the *only* way which is guaranteed to work is to do this by address. > > If you look at the latest ARM ARM (ARM DDI 0487H.a): > > https://developer.arm.com/documentation/ddi0487/ha/ > > ... on page D4-4754, in the block "Example code for cache maintenance > instructions", there's note with a treatise on this. > > The gist is that: > > * Set/Way ops are only guaranteed to affect the caches local to the CPU > issuing them, and are not guaranteed to affect caches owned by other CPUs. > > * Set/Way ops are not guaranteed to affect system-level caches, which are > fairly popular these days (whereas VA ops are required to affect those). > > * Set/Way ops race with the natural behaviour of caches (so e.g. a line could > bounce between layers of cache, or between caches in the system, and avoid > being operated upon). > > So unless you're on a single CPU system, with translation disabled, and you > *know* that there are no system-level caches, you can't rely upon Set/Way ops > to do anything useful. > > They're really there for firmware to use for IMPLEMENTATION DEFINED power-up > and power-down sequences, and aren'y useful to portable code. Thanks for the explanation. Really does seem that ARM could use the equivalent on wbnoinvd/wbinvd/invd. Regards - Eliot
Mark Rutland wrote: > On Tue, Aug 09, 2022 at 02:47:06PM -0700, Dave Jiang wrote: > > > > On 8/3/2022 10:37 AM, Jonathan Cameron wrote: > > > On Tue, 19 Jul 2022 12:07:03 -0700 > > > Dave Jiang <dave.jiang@intel.com> 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@arm.com/ > > > > > > flush_and_invalidate_cache_all() instead given it calls wbinvd on x86? I > > think other archs, at least ARM, those are separate instructions aren't > > they? > > On arm and arm64 there is no way to perform maintenance on *all* caches; it has > to be done in cacheline increments by address. It's not realistic to do that > for the entire address space, so we need to know the relevant address ranges > (as per the commit referenced above). > > So we probably need to think a bit harder about the geenric interface, since > "all" isn't possible to implement. :/ > I expect the interface would not be in the "flush_cache_" namespace since those functions are explicitly for virtually tagged caches that need maintenance on TLB operations that change the VA to PA association. In this case the cache needs maintenance because the data at the PA changes. That also means that putting it in the "nvdimm_" namespace is also wrong because there are provisions in the CXL spec where volatile memory ranges can also change contents at a given PA, for example caches might need to be invalidated if software resets the device, but not the platform. Something like: region_cache_flush(resource_size_t base, resource_size_t n, bool nowait) ...where internally that function can decide if it can rely on an instruction like wbinvd, use set / way based flushing (if set / way maintenance can be made to work which sounds like no for arm64), or map into VA space and loop. If it needs to fall back to that VA-based loop it might be the case that the caller would want to just fail the security op rather than suffer the loop latency.
On Wed, 10 Aug 2022, Dan Williams wrote: >I expect the interface would not be in the "flush_cache_" namespace >since those functions are explicitly for virtually tagged caches that >need maintenance on TLB operations that change the VA to PA association. >In this case the cache needs maintenance because the data at the PA >changes. That also means that putting it in the "nvdimm_" namespace is >also wrong because there are provisions in the CXL spec where volatile >memory ranges can also change contents at a given PA, for example caches >might need to be invalidated if software resets the device, but not the >platform. > >Something like: > > region_cache_flush(resource_size_t base, resource_size_t n, bool nowait) > >...where internally that function can decide if it can rely on an >instruction like wbinvd, use set / way based flushing (if set / way >maintenance can be made to work which sounds like no for arm64), or map >into VA space and loop. If it needs to fall back to that VA-based loop >it might be the case that the caller would want to just fail the >security op rather than suffer the loop latency. Yep, I was actually prototyping something similar, but want to still reuse cacheflush.h machinery and just introduce cache_flush_region() or whatever name, which returns any error. So all the logic would just be per-arch, where x86 will do the wbinv and return 0, and arm64 can just do -EINVAL until VA-based is no longer the only way. Thanks, Davidlohr
Davidlohr Bueso wrote: > On Wed, 10 Aug 2022, Dan Williams wrote: > > >I expect the interface would not be in the "flush_cache_" namespace > >since those functions are explicitly for virtually tagged caches that > >need maintenance on TLB operations that change the VA to PA association. > >In this case the cache needs maintenance because the data at the PA > >changes. That also means that putting it in the "nvdimm_" namespace is > >also wrong because there are provisions in the CXL spec where volatile > >memory ranges can also change contents at a given PA, for example caches > >might need to be invalidated if software resets the device, but not the > >platform. > > > >Something like: > > > > region_cache_flush(resource_size_t base, resource_size_t n, bool nowait) > > > >...where internally that function can decide if it can rely on an > >instruction like wbinvd, use set / way based flushing (if set / way > >maintenance can be made to work which sounds like no for arm64), or map > >into VA space and loop. If it needs to fall back to that VA-based loop > >it might be the case that the caller would want to just fail the > >security op rather than suffer the loop latency. > > Yep, I was actually prototyping something similar, but want to still > reuse cacheflush.h machinery and just introduce cache_flush_region() > or whatever name, which returns any error. So all the logic would > just be per-arch, where x86 will do the wbinv and return 0, and arm64 > can just do -EINVAL until VA-based is no longer the only way. cache_flush_region() works for me, but I wonder if there should be a cache_flush_region_capable() call to shut off dependent code early rather than discovering it at runtime? For example, even archs like x86, that have wbinvd, have scenarios where wbinvd is prohibited, or painful. TDX, and virtualization in general, comes to mind.
On Wed, 10 Aug 2022, Dan Williams wrote: >Davidlohr Bueso wrote: >> On Wed, 10 Aug 2022, Dan Williams wrote: >> >> >I expect the interface would not be in the "flush_cache_" namespace >> >since those functions are explicitly for virtually tagged caches that >> >need maintenance on TLB operations that change the VA to PA association. >> >In this case the cache needs maintenance because the data at the PA >> >changes. That also means that putting it in the "nvdimm_" namespace is >> >also wrong because there are provisions in the CXL spec where volatile >> >memory ranges can also change contents at a given PA, for example caches >> >might need to be invalidated if software resets the device, but not the >> >platform. >> > >> >Something like: >> > >> > region_cache_flush(resource_size_t base, resource_size_t n, bool nowait) >> > >> >...where internally that function can decide if it can rely on an >> >instruction like wbinvd, use set / way based flushing (if set / way >> >maintenance can be made to work which sounds like no for arm64), or map >> >into VA space and loop. If it needs to fall back to that VA-based loop >> >it might be the case that the caller would want to just fail the >> >security op rather than suffer the loop latency. >> >> Yep, I was actually prototyping something similar, but want to still >> reuse cacheflush.h machinery and just introduce cache_flush_region() >> or whatever name, which returns any error. So all the logic would >> just be per-arch, where x86 will do the wbinv and return 0, and arm64 >> can just do -EINVAL until VA-based is no longer the only way. > >cache_flush_region() works for me, but I wonder if there should be a >cache_flush_region_capable() call to shut off dependent code early >rather than discovering it at runtime? For example, even archs like x86, >that have wbinvd, have scenarios where wbinvd is prohibited, or painful. >TDX, and virtualization in general, comes to mind. Yeah I'm no fan of wbinv, but in these cases (cxl/nvdimm), at least from the performance angle, I am not worried: the user is explicity doing a security/cleaning specific op, probably decomisioning, so it's rare and should not expect better. Thanks, Davidlohr
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig index be0b95e51df6..8dbe89eba639 100644 --- a/arch/x86/Kconfig +++ b/arch/x86/Kconfig @@ -83,6 +83,7 @@ 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/mm/pat/set_memory.c b/arch/x86/mm/pat/set_memory.c index 1abd5438f126..e4cd1286deef 100644 --- a/arch/x86/mm/pat/set_memory.c +++ b/arch/x86/mm/pat/set_memory.c @@ -330,6 +330,14 @@ 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 8dd792a55730..242d2e9203e9 100644 --- a/drivers/acpi/nfit/intel.c +++ b/drivers/acpi/nfit/intel.c @@ -190,8 +190,6 @@ static int intel_security_change_key(struct nvdimm *nvdimm, } } -static void nvdimm_invalidate_cache(void); - static int __maybe_unused intel_security_unlock(struct nvdimm *nvdimm, const struct nvdimm_key_data *key_data) { @@ -228,7 +226,7 @@ static int __maybe_unused intel_security_unlock(struct nvdimm *nvdimm, } /* DIMM unlocked, invalidate all CPU caches before we read it */ - nvdimm_invalidate_cache(); + arch_invalidate_nvdimm_cache(); return 0; } @@ -298,7 +296,7 @@ static int __maybe_unused intel_security_erase(struct nvdimm *nvdimm, return -ENOTTY; /* flush all cache before we erase DIMM */ - nvdimm_invalidate_cache(); + arch_invalidate_nvdimm_cache(); 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); @@ -318,7 +316,7 @@ static int __maybe_unused intel_security_erase(struct nvdimm *nvdimm, } /* DIMM erased, invalidate all CPU caches before we read it */ - nvdimm_invalidate_cache(); + arch_invalidate_nvdimm_cache(); return 0; } @@ -355,7 +353,7 @@ static int __maybe_unused intel_security_query_overwrite(struct nvdimm *nvdimm) } /* flush all cache before we make the nvdimms available */ - nvdimm_invalidate_cache(); + arch_invalidate_nvdimm_cache(); return 0; } @@ -381,7 +379,7 @@ static int __maybe_unused intel_security_overwrite(struct nvdimm *nvdimm, return -ENOTTY; /* flush all cache before we erase DIMM */ - nvdimm_invalidate_cache(); + arch_invalidate_nvdimm_cache(); 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); @@ -401,22 +399,6 @@ static int __maybe_unused intel_security_overwrite(struct nvdimm *nvdimm, } } -/* - * TODO: define a cross arch wbinvd equivalent when/if - * NVDIMM_FAMILY_INTEL command support arrives on another arch. - */ -#ifdef CONFIG_X86 -static void nvdimm_invalidate_cache(void) -{ - wbinvd_on_all_cpus(); -} -#else -static void nvdimm_invalidate_cache(void) -{ - WARN_ON_ONCE("cache invalidation required after unlock\n"); -} -#endif - static const struct nvdimm_security_ops __intel_security_ops = { .get_flags = intel_security_flags, .freeze = intel_security_freeze, diff --git a/include/linux/libnvdimm.h b/include/linux/libnvdimm.h index 0d61e07b6827..455d54ec3c86 100644 --- a/include/linux/libnvdimm.h +++ b/include/linux/libnvdimm.h @@ -308,4 +308,12 @@ 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__ */ diff --git a/lib/Kconfig b/lib/Kconfig index eaaad4d85bf2..d4bc48eea635 100644 --- a/lib/Kconfig +++ b/lib/Kconfig @@ -652,6 +652,9 @@ config ARCH_NO_SG_CHAIN config ARCH_HAS_PMEM_API bool +config ARCH_HAS_NVDIMM_INVAL_CACHE + bool + config MEMREGION bool
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. Signed-off-by: Dave Jiang <dave.jiang@intel.com> --- arch/x86/Kconfig | 1 + arch/x86/mm/pat/set_memory.c | 8 ++++++++ drivers/acpi/nfit/intel.c | 28 +++++----------------------- include/linux/libnvdimm.h | 8 ++++++++ lib/Kconfig | 3 +++ 5 files changed, 25 insertions(+), 23 deletions(-)