Re: [PATCH RFC 10/15] x86: add an arch helper function to invalidate all cache for nvdimm

[Davidlohr Bueso <dave@stgolabs.net>]

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