arm64 pmem support

arch/arm64/Kconfig +0 -0

@@ -960,6 +960,17 @@ config ARM64_UAO
 	  regular load/store instructions if the cpu does not implement the
 	  feature.
 

arch/arm64/mm/cache.S +14 -0

@@ -172,6 +172,20 @@ ENDPIPROC(__clean_dcache_area_poc)
 ENDPROC(__dma_clean_area)
 
 /*
+ *	__clean_dcache_area_pop(kaddr, size)
+ *
+ * 	Ensure that any D-cache lines for the interval [kaddr, kaddr+size)
+ * 	are cleaned to the PoP.
+ *
+ *	- kaddr   - kernel address
+ *	- size    - size in question
+ */
+ENTRY(__clean_dcache_area_pop)
+	dcache_by_line_op cvap, sy, x0, x1, x2, x3
+	ret
+ENDPIPROC(__clean_dcache_area_pop)
+
+/*
  *	__dma_flush_area(start, size)
  *
  *	clean & invalidate D / U line

arch/arm64/mm/pageattr.c +0 -0

index a682a0a2a0fa..a461a00ceb3e 100644
@@ -183,3 +183,21 @@ bool kernel_page_present(struct page *page)
 }
 #endif /* CONFIG_HIBERNATION */
 #endif /* CONFIG_DEBUG_PAGEALLOC */

Two minor comments below.

Quoted 6 lines · Robin Murphy

arch/arm64/Kconfig +0 -0

@@ -960,6 +960,17 @@ config ARM64_UAO
 	  regular load/store instructions if the cpu does not implement the
 	  feature.
 

arch/arm64/Kconfig config ARM64_UAO · line 972

Quoted 10 lines

arch/arm64/Kconfig +10 -0

+config ARM64_PMEM
+	bool "Enable support for persistent memory"
+	select ARCH_HAS_PMEM_API
+	help
+	  Say Y to enable support for the persistent memory API based on the
+	  ARMv8.2 DCPoP feature.
+
+	  The feature is detected at runtime, and the kernel will use DC CVAC
+	  operations if DC CVAP is not supported (following the behaviour of
+	  DC CVAP itself if the system does not define a point of persistence).

Any reason not to have this default y?

Quoted 36 lines

arch/arm64/mm/cache.S +14 -0

@@ -172,6 +172,20 @@ ENDPIPROC(__clean_dcache_area_poc)
 ENDPROC(__dma_clean_area)
 
 /*
+ *	__clean_dcache_area_pop(kaddr, size)
+ *
+ * 	Ensure that any D-cache lines for the interval [kaddr, kaddr+size)
+ * 	are cleaned to the PoP.
+ *
+ *	- kaddr   - kernel address
+ *	- size    - size in question
+ */
+ENTRY(__clean_dcache_area_pop)
+	dcache_by_line_op cvap, sy, x0, x1, x2, x3
+	ret
+ENDPIPROC(__clean_dcache_area_pop)
+
+/*
  *	__dma_flush_area(start, size)
  *
  *	clean & invalidate D / U line

arch/arm64/mm/pageattr.c +5 -0

index a682a0a2a0fa..a461a00ceb3e 100644
@@ -183,3 +183,21 @@ bool kernel_page_present(struct page *page)
 }
 #endif /* CONFIG_HIBERNATION */
 #endif /* CONFIG_DEBUG_PAGEALLOC */
+
+#ifdef CONFIG_ARCH_HAS_PMEM_API
+#include <asm/cacheflush.h>
+
+static inline void arch_wb_cache_pmem(void *addr, size_t size)

arch/arm64/mm/pageattr.c bool kernel_page_present(struct page *page) · line 195

Quoted 5 lines

arch/arm64/mm/pageattr.c +5 -0

+{
+	/* Ensure order against any prior non-cacheable writes */
+	dmb(sy);
+	__clean_dcache_area_pop(addr, size);
+}

Could we keep the dmb() in the actual __clean_dcache_area_pop() implementation?

I can do the changes myself if you don't have any objections.

Quoted 9 lines · Catalin Marinas

Two minor comments below.

Quoted 6 lines · Robin Murphy

arch/arm64/Kconfig +0 -0

@@ -960,6 +960,17 @@ config ARM64_UAO
       regular load/store instructions if the cpu does not implement the
       feature.
 

Quoted 12 lines

arch/arm64/Kconfig config ARM64_UAO · line 972

Quoted 10 lines

arch/arm64/Kconfig +10 -0

+config ARM64_PMEM
+    bool "Enable support for persistent memory"
+    select ARCH_HAS_PMEM_API
+    help
+      Say Y to enable support for the persistent memory API based on the
+      ARMv8.2 DCPoP feature.
+
+      The feature is detected at runtime, and the kernel will use DC CVAC
+      operations if DC CVAP is not supported (following the behaviour of
+      DC CVAP itself if the system does not define a point of persistence).

Any reason not to have this default y?

Mostly because it's untested, and not actually useful without some way of describing persistent memory regions to the kernel (I'm currently trying to make sense of what exactly ARCH_HAS_MMIO_FLUSH is supposed to mean in order to enable ACPI NFIT support).

Quoted 23 lines · Robin Murphy

Quoted 9 lines · Catalin Marinas

Two minor comments below.

Quoted 6 lines · Robin Murphy

arch/arm64/Kconfig +0 -0

@@ -960,6 +960,17 @@ config ARM64_UAO
       regular load/store instructions if the cpu does not implement the
       feature.
 

Quoted 12 lines

arch/arm64/Kconfig config ARM64_UAO · line 972

Quoted 10 lines

arch/arm64/Kconfig +10 -0

+config ARM64_PMEM
+    bool "Enable support for persistent memory"
+    select ARCH_HAS_PMEM_API
+    help
+      Say Y to enable support for the persistent memory API based on the
+      ARMv8.2 DCPoP feature.
+
+      The feature is detected at runtime, and the kernel will use DC CVAC
+      operations if DC CVAP is not supported (following the behaviour of
+      DC CVAP itself if the system does not define a point of persistence).

Any reason not to have this default y?

Quoted 4 lines

Mostly because it's untested, and not actually useful without some way of describing persistent memory regions to the kernel (I'm currently trying to make sense of what exactly ARCH_HAS_MMIO_FLUSH is supposed to mean in order to enable ACPI NFIT support).

This is related to block-aperture support described by the NFIT where a sliding-memory-mapped window can be programmed to access different ranges of the NVDIMM. Before the window is programmed to a new DIMM-address we need to flush any dirty data through the current window setting to media. See the call to mmio_flush_range() in acpi_nfit_blk_single_io(). I think it's ok to omit ARCH_HAS_MMIO_FLUSH support, and add a configuration option to compile out the block-aperture support.

Quoted 29 lines · Dan Williams

Quoted 23 lines · Robin Murphy

Quoted 9 lines · Catalin Marinas

Two minor comments below.

Quoted 6 lines · Robin Murphy

arch/arm64/Kconfig +0 -0

@@ -960,6 +960,17 @@ config ARM64_UAO
       regular load/store instructions if the cpu does not implement the
       feature.
 

Quoted 12 lines

arch/arm64/Kconfig config ARM64_UAO · line 972

Quoted 10 lines

arch/arm64/Kconfig +10 -0

+config ARM64_PMEM
+    bool "Enable support for persistent memory"
+    select ARCH_HAS_PMEM_API
+    help
+      Say Y to enable support for the persistent memory API based on the
+      ARMv8.2 DCPoP feature.
+
+      The feature is detected at runtime, and the kernel will use DC CVAC
+      operations if DC CVAP is not supported (following the behaviour of
+      DC CVAP itself if the system does not define a point of persistence).

Any reason not to have this default y?

Quoted 4 lines

Mostly because it's untested, and not actually useful without some way of describing persistent memory regions to the kernel (I'm currently trying to make sense of what exactly ARCH_HAS_MMIO_FLUSH is supposed to mean in order to enable ACPI NFIT support).

Quoted 8 lines

This is related to block-aperture support described by the NFIT where a sliding-memory-mapped window can be programmed to access different ranges of the NVDIMM. Before the window is programmed to a new DIMM-address we need to flush any dirty data through the current window setting to media. See the call to mmio_flush_range() in acpi_nfit_blk_single_io(). I think it's ok to omit ARCH_HAS_MMIO_FLUSH support, and add a configuration option to compile out the block-aperture support.

Oh, I have every intention of implementing it one way or another if necessary - it's not difficult, it's just been a question of working through the NFIT code to figure out the subtleties of translation to arm64 ;)

If mmio_flush_range() is for true MMIO (i.e. __iomem) mappings, then arm64 should only need a barrier, rather than actual cache operations. If on the other hand it's misleadingly named and only actually used on MEMREMAP_WB mappings (as I'm staring to think it might be), then I can't help thinking it could simply go away in favour of arch_wb_pmem(), since that now seems to have those same semantics and intent, plus a much more appropriate name.

Two minor comments below.

Quoted 6 lines · Robin Murphy

arch/arm64/Kconfig +0 -0

@@ -960,6 +960,17 @@ config ARM64_UAO
 	  regular load/store instructions if the cpu does not implement the
 	  feature.
 

arch/arm64/Kconfig config ARM64_UAO · line 972

Quoted 10 lines

arch/arm64/Kconfig +10 -0

+config ARM64_PMEM
+	bool "Enable support for persistent memory"
+	select ARCH_HAS_PMEM_API
+	help
+	  Say Y to enable support for the persistent memory API based on the
+	  ARMv8.2 DCPoP feature.
+
+	  The feature is detected at runtime, and the kernel will use DC CVAC
+	  operations if DC CVAP is not supported (following the behaviour of
+	  DC CVAP itself if the system does not define a point of persistence).

Any reason not to have this default y?

Quoted 31 lines

arch/arm64/mm/cache.S +14 -0

@@ -172,6 +172,20 @@ ENDPIPROC(__clean_dcache_area_poc)
 ENDPROC(__dma_clean_area)
 
 /*
+ *	__clean_dcache_area_pop(kaddr, size)
+ *
+ * 	Ensure that any D-cache lines for the interval [kaddr, kaddr+size)
+ * 	are cleaned to the PoP.
+ *
+ *	- kaddr   - kernel address
+ *	- size    - size in question
+ */
+ENTRY(__clean_dcache_area_pop)
+	dcache_by_line_op cvap, sy, x0, x1, x2, x3
+	ret
+ENDPIPROC(__clean_dcache_area_pop)
+
+/*
  *	__dma_flush_area(start, size)
  *
  *	clean & invalidate D / U line

arch/arm64/mm/pageattr.c +0 -0

index a682a0a2a0fa..a461a00ceb3e 100644
@@ -183,3 +183,21 @@ bool kernel_page_present(struct page *page)
 }
 #endif /* CONFIG_HIBERNATION */
 #endif /* CONFIG_DEBUG_PAGEALLOC */

arch/arm64/mm/pageattr.c bool kernel_page_present(struct page *page) · line 195

Quoted 10 lines

arch/arm64/mm/pageattr.c +10 -0

+
+#ifdef CONFIG_ARCH_HAS_PMEM_API
+#include <asm/cacheflush.h>
+
+static inline void arch_wb_cache_pmem(void *addr, size_t size)
+{
+	/* Ensure order against any prior non-cacheable writes */
+	dmb(sy);
+	__clean_dcache_area_pop(addr, size);
+}

Could we keep the dmb() in the actual __clean_dcache_area_pop() implementation?

I can do the changes myself if you don't have any objections.

Implement the set of copy functions with guarantees of a clean cache upon completion necessary to support the pmem driver.

Signed-off-by Robin Murphy <[email protected]>

---
 arch/arm64/Kconfig                  |  1 +
 arch/arm64/include/asm/string.h     |  4 ++++
 arch/arm64/include/asm/uaccess.h    | 12 ++++++++++
 arch/arm64/lib/Makefile             |  2 ++
 arch/arm64/lib/uaccess_flushcache.c | 47 +++++++++++++++++++++++++++++++++++++
 5 files changed, 66 insertions(+)
 create mode 100644 arch/arm64/lib/uaccess_flushcache.c

arch/arm64/lib/uaccess_flushcache.c new file +32 -0

new file mode 100644
index 000000000000..b6ceafdb8b72
@@ -0,0 +1,47 @@
+/*
+ * Copyright (C) 2017 ARM Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <linux/uaccess.h>
+#include <asm/barrier.h>
+#include <asm/cacheflush.h>
+
+void memcpy_flushcache(void *dst, const void *src, size_t cnt)
+{
+	/*
+	 * We assume this should not be called with @dst pointing to
+	 * non-cacheable memory, such that we don't need an explicit
+	 * barrier to order the cache maintenance against the memcpy.
+	 */
+	memcpy(dst, src, cnt);
+	__clean_dcache_area_pop(dst, cnt);
+}
+EXPORT_SYMBOL_GPL(memcpy_flushcache);
+

Implement the set of copy functions with guarantees of a clean cache upon completion necessary to support the pmem driver.

Signed-off-by Robin Murphy <[email protected]>

---
 arch/arm64/Kconfig                  |  1 +
 arch/arm64/include/asm/string.h     |  4 ++++
 arch/arm64/include/asm/uaccess.h    | 12 ++++++++++
 arch/arm64/lib/Makefile             |  2 ++
 arch/arm64/lib/uaccess_flushcache.c | 47 +++++++++++++++++++++++++++++++++++++
 5 files changed, 66 insertions(+)
 create mode 100644 arch/arm64/lib/uaccess_flushcache.c

Quoted 4 lines · Robin Murphy

Implement the set of copy functions with guarantees of a clean cache upon completion necessary to support the pmem driver.

Signed-off-by Robin Murphy <[email protected]>

Quoted 8 lines

---
 arch/arm64/Kconfig                  |  1 +
 arch/arm64/include/asm/string.h     |  4 ++++
 arch/arm64/include/asm/uaccess.h    | 12 ++++++++++
 arch/arm64/lib/Makefile             |  2 ++
 arch/arm64/lib/uaccess_flushcache.c | 47 +++++++++++++++++++++++++++++++++++++
 5 files changed, 66 insertions(+)
 create mode 100644 arch/arm64/lib/uaccess_flushcache.c

It looks like Catalin applied part of this patch but forgot to add arch/arm64/lib/uaccess_flushcache.c

With the latest updates to the pmem API, the arch code contribution becomes very straightforward to wire up - I think there's about as much code here to just cope with the existence of our new instruction as there is to actually make use of it. I don't have access to any NVDIMMs nor suitable hardware to put them in, so this is written purely to spec - the extent of testing has been the feature detection on a v8.2 Fast Model vs. v8.0 systems.

Patch #1 could go in as a fix ahead of the rest; it just needs to come before patch #5 to prevent that blowing up the build.