Add support for memcpy_mcsafe

The code currently assumes PAGE_SHIFT as the shift value of the pfn, this works correctly (mostly) for user space pages, but the correct thing to do is

1. Extrace the shift value returned via the pte-walk API's

2. Use the shift value to access the instruction address.

Note, the final physical address still use PAGE_SHIFT for computation. handle_ierror() is not modified and handle_derror() is modified just for extracting the correct instruction address.

Fixes ba41e1e1ccb9 ("powerpc/mce: Hookup derror (load/store) UE errors")

Signed-off-by Balbir Singh <[email protected]>

---
 arch/powerpc/kernel/mce_power.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

arch/powerpc/kernel/mce_power.c +8 -6

index fe6fc63251fe..69c8cc1e8e4f 100644
@@ -36,7 +36,8 @@
  * Convert an address related to an mm to a PFN. NOTE: we are in real
  * mode, we could potentially race with page table updates.
  */
-static unsigned long addr_to_pfn(struct pt_regs *regs, unsigned long addr)
+static unsigned long addr_to_pfn(struct pt_regs *regs, unsigned long addr,
+		unsigned int *shift)
 {
 	pte_t *ptep;
 	unsigned long flags;

@@ -49,9 +50,9 @@ static unsigned long addr_to_pfn(struct pt_regs *regs, unsigned long addr)
 
 	local_irq_save(flags);
 	if (mm == current->mm)
-		ptep = find_current_mm_pte(mm->pgd, addr, NULL, NULL);
+		ptep = find_current_mm_pte(mm->pgd, addr, NULL, shift);
 	else
-		ptep = find_init_mm_pte(addr, NULL);
+		ptep = find_init_mm_pte(addr, shift);
 	local_irq_restore(flags);
 	if (!ptep || pte_special(*ptep))
 		return ULONG_MAX;

@@ -353,13 +354,14 @@ static int mce_find_instr_ea_and_pfn(struct pt_regs *regs, uint64_t *addr,
 	unsigned long pfn, instr_addr;
 	struct instruction_op op;
 	struct pt_regs tmp = *regs;
+	unsigned int shift;
 
-	pfn = addr_to_pfn(regs, regs->nip);
+	pfn = addr_to_pfn(regs, regs->nip, &shift);
 	if (pfn != ULONG_MAX) {
-		instr_addr = (pfn << PAGE_SHIFT) + (regs->nip & ~PAGE_MASK);
+		instr_addr = (pfn << shift) + (regs->nip & ((1 << shift) - 1));
 		instr = *(unsigned int *)(instr_addr);
 		if (!analyse_instr(&op, &tmp, instr)) {
-			pfn = addr_to_pfn(regs, op.ea);
+			pfn = addr_to_pfn(regs, op.ea, &shift);
 			*addr = op.ea;
 			*phys_addr = (pfn << PAGE_SHIFT);
 			return 0;

Quoted 3 lines · On Thu, 5 Apr 2018 09:19:41 +1000 Balbir Singh <[email protected]> wrote:

The code currently assumes PAGE_SHIFT as the shift value of the pfn, this works correctly (mostly) for user space pages, but the correct thing to do is

It would be good to actually explain the problem in the changelog. I would have thought pte_pfn returns a PAGE_SIZE based pfn value?

Quoted 2 lines

1. Extrace the shift value returned via the pte-walk API's

      ^^^ extract?

The pmem infrastructure uses memcpy_mcsafe in the pmem layer so as to convert machine check excpetions into a return value on failure in case a machine check exception is encoutered during the memcpy.

This patch largely borrows from the copyuser_power7 logic and does not add the VMX optimizations, largely to keep the patch simple. If needed those optimizations can be folded in.

Signed-off-by Balbir Singh <[email protected]>

---
 arch/powerpc/include/asm/string.h   |   2 +
 arch/powerpc/lib/Makefile           |   2 +-
 arch/powerpc/lib/memcpy_mcsafe_64.S | 212 ++++++++++++++++++++++++++++++++++++
 3 files changed, 215 insertions(+), 1 deletion(-)
 create mode 100644 arch/powerpc/lib/memcpy_mcsafe_64.S

arch/powerpc/include/asm/string.h +2 -0

index 9b8cedf618f4..b7e872a64726 100644
@@ -30,7 +30,9 @@ extern void * memcpy_flushcache(void *,const void *,__kernel_size_t);
 #ifdef CONFIG_PPC64
 #define __HAVE_ARCH_MEMSET32
 #define __HAVE_ARCH_MEMSET64
+#define __HAVE_ARCH_MEMCPY_MCSAFE
 
+extern int memcpy_mcsafe(void *dst, const void *src, __kernel_size_t sz);
 extern void *__memset16(uint16_t *, uint16_t v, __kernel_size_t);
 extern void *__memset32(uint32_t *, uint32_t v, __kernel_size_t);
 extern void *__memset64(uint64_t *, uint64_t v, __kernel_size_t);

arch/powerpc/lib/Makefile +1 -1

index 3c29c9009bbf..048afee9f518 100644
@@ -24,7 +24,7 @@ endif
 
 obj64-y	+= copypage_64.o copyuser_64.o mem_64.o hweight_64.o \
 	   copyuser_power7.o string_64.o copypage_power7.o memcpy_power7.o \
-	   memcpy_64.o memcmp_64.o pmem.o
+	   memcpy_64.o memcmp_64.o pmem.o memcpy_mcsafe_64.o
 
 obj64-$(CONFIG_SMP)	+= locks.o
 obj64-$(CONFIG_ALTIVEC)	+= vmx-helper.o

arch/powerpc/lib/memcpy_mcsafe_64.S new file +202 -0

new file mode 100644
index 000000000000..e7eaa9b6cded
@@ -0,0 +1,212 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) IBM Corporation, 2011
+ * Derived from copyuser_power7.s by Anton Blanchard <[email protected]>
+ * Author - Balbir Singh <[email protected]>
+ */
+#include <asm/ppc_asm.h>
+#include <asm/errno.h>
+
+	.macro err1
+100:
+	EX_TABLE(100b,.Ldo_err1)
+	.endm
+
+	.macro err2
+200:
+	EX_TABLE(200b,.Ldo_err2)
+	.endm
+
+.Ldo_err2:
+	ld	r22,STK_REG(R22)(r1)
+	ld	r21,STK_REG(R21)(r1)
+	ld	r20,STK_REG(R20)(r1)
+	ld	r19,STK_REG(R19)(r1)
+	ld	r18,STK_REG(R18)(r1)
+	ld	r17,STK_REG(R17)(r1)
+	ld	r16,STK_REG(R16)(r1)
+	ld	r15,STK_REG(R15)(r1)
+	ld	r14,STK_REG(R14)(r1)
+	addi	r1,r1,STACKFRAMESIZE
+.Ldo_err1:
+	li	r3,-EFAULT
+	blr
+
+
+_GLOBAL(memcpy_mcsafe)
+	cmpldi	r5,16
+	blt	.Lshort_copy
+
+.Lcopy:
+	/* Get the source 8B aligned */
+	neg	r6,r4
+	mtocrf	0x01,r6
+	clrldi	r6,r6,(64-3)
+
+	bf	cr7*4+3,1f
+err1;	lbz	r0,0(r4)
+	addi	r4,r4,1
+err1;	stb	r0,0(r3)
+	addi	r3,r3,1
+
+1:	bf	cr7*4+2,2f
+err1;	lhz	r0,0(r4)
+	addi	r4,r4,2
+err1;	sth	r0,0(r3)
+	addi	r3,r3,2
+
+2:	bf	cr7*4+1,3f
+err1;	lwz	r0,0(r4)
+	addi	r4,r4,4
+err1;	stw	r0,0(r3)
+	addi	r3,r3,4
+
+3:	sub	r5,r5,r6
+	cmpldi	r5,128
+	blt	5f
+
+	mflr	r0
+	stdu	r1,-STACKFRAMESIZE(r1)
+	std	r14,STK_REG(R14)(r1)
+	std	r15,STK_REG(R15)(r1)
+	std	r16,STK_REG(R16)(r1)
+	std	r17,STK_REG(R17)(r1)
+	std	r18,STK_REG(R18)(r1)
+	std	r19,STK_REG(R19)(r1)
+	std	r20,STK_REG(R20)(r1)
+	std	r21,STK_REG(R21)(r1)
+	std	r22,STK_REG(R22)(r1)
+	std	r0,STACKFRAMESIZE+16(r1)
+
+	srdi	r6,r5,7
+	mtctr	r6
+
+	/* Now do cacheline (128B) sized loads and stores. */
+	.align	5
+4:
+err2;	ld	r0,0(r4)
+err2;	ld	r6,8(r4)
+err2;	ld	r7,16(r4)
+err2;	ld	r8,24(r4)
+err2;	ld	r9,32(r4)
+err2;	ld	r10,40(r4)
+err2;	ld	r11,48(r4)
+err2;	ld	r12,56(r4)
+err2;	ld	r14,64(r4)
+err2;	ld	r15,72(r4)
+err2;	ld	r16,80(r4)
+err2;	ld	r17,88(r4)
+err2;	ld	r18,96(r4)
+err2;	ld	r19,104(r4)
+err2;	ld	r20,112(r4)
+err2;	ld	r21,120(r4)
+	addi	r4,r4,128
+err2;	std	r0,0(r3)
+err2;	std	r6,8(r3)
+err2;	std	r7,16(r3)
+err2;	std	r8,24(r3)
+err2;	std	r9,32(r3)
+err2;	std	r10,40(r3)
+err2;	std	r11,48(r3)
+err2;	std	r12,56(r3)
+err2;	std	r14,64(r3)
+err2;	std	r15,72(r3)
+err2;	std	r16,80(r3)
+err2;	std	r17,88(r3)
+err2;	std	r18,96(r3)
+err2;	std	r19,104(r3)
+err2;	std	r20,112(r3)
+err2;	std	r21,120(r3)
+	addi	r3,r3,128
+	bdnz	4b
+
+	clrldi	r5,r5,(64-7)
+
+	ld	r14,STK_REG(R14)(r1)
+	ld	r15,STK_REG(R15)(r1)
+	ld	r16,STK_REG(R16)(r1)
+	ld	r17,STK_REG(R17)(r1)
+	ld	r18,STK_REG(R18)(r1)
+	ld	r19,STK_REG(R19)(r1)
+	ld	r20,STK_REG(R20)(r1)
+	ld	r21,STK_REG(R21)(r1)
+	ld	r22,STK_REG(R22)(r1)
+	addi	r1,r1,STACKFRAMESIZE
+
+	/* Up to 127B to go */
+5:	srdi	r6,r5,4
+	mtocrf	0x01,r6
+
+6:	bf	cr7*4+1,7f
+err1;	ld	r0,0(r4)
+err1;	ld	r6,8(r4)
+err1;	ld	r7,16(r4)
+err1;	ld	r8,24(r4)
+err1;	ld	r9,32(r4)
+err1;	ld	r10,40(r4)
+err1;	ld	r11,48(r4)
+err1;	ld	r12,56(r4)
+	addi	r4,r4,64
+err1;	std	r0,0(r3)
+err1;	std	r6,8(r3)
+err1;	std	r7,16(r3)
+err1;	std	r8,24(r3)
+err1;	std	r9,32(r3)
+err1;	std	r10,40(r3)
+err1;	std	r11,48(r3)
+err1;	std	r12,56(r3)
+	addi	r3,r3,64
+
+	/* Up to 63B to go */
+7:	bf	cr7*4+2,8f
+err1;	ld	r0,0(r4)
+err1;	ld	r6,8(r4)
+err1;	ld	r7,16(r4)
+err1;	ld	r8,24(r4)
+	addi	r4,r4,32
+err1;	std	r0,0(r3)
+err1;	std	r6,8(r3)
+err1;	std	r7,16(r3)
+err1;	std	r8,24(r3)
+	addi	r3,r3,32
+
+	/* Up to 31B to go */
+8:	bf	cr7*4+3,9f
+err1;	ld	r0,0(r4)
+err1;	ld	r6,8(r4)
+	addi	r4,r4,16
+err1;	std	r0,0(r3)
+err1;	std	r6,8(r3)
+	addi	r3,r3,16
+
+9:	clrldi	r5,r5,(64-4)
+
+	/* Up to 15B to go */
+.Lshort_copy:
+	mtocrf	0x01,r5
+	bf	cr7*4+0,12f
+err1;	lwz	r0,0(r4)	/* Less chance of a reject with word ops */
+err1;	lwz	r6,4(r4)
+	addi	r4,r4,8
+err1;	stw	r0,0(r3)
+err1;	stw	r6,4(r3)
+	addi	r3,r3,8
+
+12:	bf	cr7*4+1,13f
+err1;	lwz	r0,0(r4)
+	addi	r4,r4,4
+err1;	stw	r0,0(r3)
+	addi	r3,r3,4
+
+13:	bf	cr7*4+2,14f
+err1;	lhz	r0,0(r4)

Quoted 5 lines · On Thu, 5 Apr 2018 09:19:42 +1000 Balbir Singh <[email protected]> wrote:

The pmem infrastructure uses memcpy_mcsafe in the pmem layer so as to convert machine check excpetions into a return value on failure in case a machine check exception is encoutered during the memcpy.

Quoted 4 lines

This patch largely borrows from the copyuser_power7 logic and does not add the VMX optimizations, largely to keep the patch simple. If needed those optimizations can be folded in.

So memcpy_mcsafe doesn't return number of bytes copied? Huh, well that makes it simple.

[ adding Matthew, Christoph, and Tony ]

Quoted 8 lines · Nicholas Piggin

Quoted 5 lines · On Thu, 5 Apr 2018 09:19:42 +1000 Balbir Singh <[email protected]> wrote:

The pmem infrastructure uses memcpy_mcsafe in the pmem layer so as to convert machine check excpetions into a return value on failure in case a machine check exception is encoutered during the memcpy.

Quoted 7 lines

Quoted 4 lines

This patch largely borrows from the copyuser_power7 logic and does not add the VMX optimizations, largely to keep the patch simple. If needed those optimizations can be folded in.

So memcpy_mcsafe doesn't return number of bytes copied? Huh, well that makes it simple.

Well, not in current kernels, but we need to add that support or remove the direct call to copy_to_iter() in fs/dax.c. I'm looking right now to add "bytes remaining" support to the x86 memcpy_mcsafe(), but for copy_to_user we also need to handle bytes remaining for write faults. That fix is hopefully something that can land in an early 4.17-rc, but it won't be ready for -rc1.

Quoted 19 lines · On Wed, 4 Apr 2018 20:00:52 -0700 Dan Williams <[email protected]> wrote:

[ adding Matthew, Christoph, and Tony ]

Quoted 8 lines · Nicholas Piggin

Quoted 5 lines · On Thu, 5 Apr 2018 09:19:42 +1000 Balbir Singh <[email protected]> wrote:

The pmem infrastructure uses memcpy_mcsafe in the pmem layer so as to convert machine check excpetions into a return value on failure in case a machine check exception is encoutered during the memcpy.

Quoted 7 lines

Quoted 4 lines

This patch largely borrows from the copyuser_power7 logic and does not add the VMX optimizations, largely to keep the patch simple. If needed those optimizations can be folded in.

So memcpy_mcsafe doesn't return number of bytes copied? Huh, well that makes it simple.

Quoted 6 lines

Well, not in current kernels, but we need to add that support or remove the direct call to copy_to_iter() in fs/dax.c. I'm looking right now to add "bytes remaining" support to the x86 memcpy_mcsafe(), but for copy_to_user we also need to handle bytes remaining for write faults. That fix is hopefully something that can land in an early 4.17-rc, but it won't be ready for -rc1.

I wonder if the powerpc implementation should just go straight to counting bytes. Backporting to this interface would be trivial, but it would just mean there's only one variant of the code to support. That's up to Balbir though.

Quoted 29 lines · On Thu, 5 Apr 2018 15:04:05 +1000 Nicholas Piggin <[email protected]> wrote:

Quoted 19 lines · On Wed, 4 Apr 2018 20:00:52 -0700 Dan Williams <[email protected]> wrote:

[ adding Matthew, Christoph, and Tony ]

Quoted 8 lines · Nicholas Piggin

Quoted 5 lines · On Thu, 5 Apr 2018 09:19:42 +1000 Balbir Singh <[email protected]> wrote:

The pmem infrastructure uses memcpy_mcsafe in the pmem layer so as to convert machine check excpetions into a return value on failure in case a machine check exception is encoutered during the memcpy.

Quoted 7 lines

Quoted 4 lines

This patch largely borrows from the copyuser_power7 logic and does not add the VMX optimizations, largely to keep the patch simple. If needed those optimizations can be folded in.

So memcpy_mcsafe doesn't return number of bytes copied? Huh, well that makes it simple.

Quoted 6 lines

Well, not in current kernels, but we need to add that support or remove the direct call to copy_to_iter() in fs/dax.c. I'm looking right now to add "bytes remaining" support to the x86 memcpy_mcsafe(), but for copy_to_user we also need to handle bytes remaining for write faults. That fix is hopefully something that can land in an early 4.17-rc, but it won't be ready for -rc1.

Quoted 5 lines

I wonder if the powerpc implementation should just go straight to counting bytes. Backporting to this interface would be trivial, but it would just mean there's only one variant of the code to support. That's up to Balbir though.

I'm thinking about it, I wonder what "bytes remaining" mean in pmem context in the context of a machine check exception. Also, do we want to be byte accurate or cache-line accurate for the bytes remaining? The former is much easier than the latter :)

I'd rather implement the existing interface and port/support the new interface as it becomes available

Quoted 38 lines · On Thu, 5 Apr 2018 15:53:07 +1000 Balbir Singh <[email protected]> wrote:

Quoted 29 lines · On Thu, 5 Apr 2018 15:04:05 +1000 Nicholas Piggin <[email protected]> wrote:

Quoted 19 lines · On Wed, 4 Apr 2018 20:00:52 -0700 Dan Williams <[email protected]> wrote:

[ adding Matthew, Christoph, and Tony ]

Quoted 8 lines · Nicholas Piggin

Quoted 5 lines · On Thu, 5 Apr 2018 09:19:42 +1000 Balbir Singh <[email protected]> wrote:

The pmem infrastructure uses memcpy_mcsafe in the pmem layer so as to convert machine check excpetions into a return value on failure in case a machine check exception is encoutered during the memcpy.

Quoted 7 lines

Quoted 4 lines

This patch largely borrows from the copyuser_power7 logic and does not add the VMX optimizations, largely to keep the patch simple. If needed those optimizations can be folded in.

So memcpy_mcsafe doesn't return number of bytes copied? Huh, well that makes it simple.

Quoted 6 lines

Well, not in current kernels, but we need to add that support or remove the direct call to copy_to_iter() in fs/dax.c. I'm looking right now to add "bytes remaining" support to the x86 memcpy_mcsafe(), but for copy_to_user we also need to handle bytes remaining for write faults. That fix is hopefully something that can land in an early 4.17-rc, but it won't be ready for -rc1.

Quoted 5 lines

I wonder if the powerpc implementation should just go straight to counting bytes. Backporting to this interface would be trivial, but it would just mean there's only one variant of the code to support. That's up to Balbir though.

Quoted 4 lines

I'm thinking about it, I wonder what "bytes remaining" mean in pmem context in the context of a machine check exception. Also, do we want to be byte accurate or cache-line accurate for the bytes remaining? The former is much easier than the latter :)

The ideal would be a linear measure of how much of your copy reached (or can reach) non-volatile storage with nothing further copied. You may have to allow for some relaxing of the semantics depending on what the architecture can support.

What's the problem with just counting bytes copied like usercopy -- why is that harder than cacheline accuracy?

Quoted 2 lines

I'd rather implement the existing interface and port/support the new interface as it becomes available

Fair enough.

On Wed, Apr 4, 2018 at 11:45 PM, Nicholas Piggin <[email protected]> wrote:

Quoted 7 lines

What's the problem with just counting bytes copied like usercopy -- why is that harder than cacheline accuracy?

Quoted 2 lines

I'd rather implement the existing interface and port/support the new interface as it becomes available

Fair enough.

I have patches already in progress to change the interface. My preference is to hold off on adding a new implementation that will need to be immediately reworked. When I say "immediate" I mean that should be able to post what I have for review within the next few days.

Whether this is all too late for 4.17 is another question...

Quoted 1 line · Dan Williams

On Wed, Apr 4, 2018 at 11:45 PM, Nicholas Piggin <[email protected]> wrote:

[,,]

Quoted 14 lines

Quoted 7 lines

What's the problem with just counting bytes copied like usercopy -- why is that harder than cacheline accuracy?

Quoted 2 lines

I'd rather implement the existing interface and port/support the new interface as it becomes available

Fair enough.

I have patches already in progress to change the interface. My preference is to hold off on adding a new implementation that will need to be immediately reworked. When I say "immediate" I mean that should be able to post what I have for review within the next few days.

Quoted 1 line

Whether this is all too late for 4.17 is another question...

Here is the x86 version of a 'bytes remaining' memcpy_mcsafe() implemenation:

    https://lists.01.org/pipermail/linux-nvdimm/2018-May/015548.html

Quoted 4 lines · On Thu, 5 Apr 2018 15:53:07 +1000 Balbir Singh <[email protected]> wrote:

I'm thinking about it, I wonder what "bytes remaining" mean in pmem context in the context of a machine check exception. Also, do we want to be byte accurate or cache-line accurate for the bytes remaining? The former is much easier than the latter :)

The ideal would be a linear measure of how much of your copy reached (or can reach) non-volatile storage with nothing further copied. You may have to allow for some relaxing of the semantics depending on what the architecture can support.

What's the problem with just counting bytes copied like usercopy -- why is that harder than cacheline accuracy?

Quoted 11 lines · Nicholas Piggin

Quoted 4 lines · On Thu, 5 Apr 2018 15:53:07 +1000 Balbir Singh <[email protected]> wrote:

I'm thinking about it, I wonder what "bytes remaining" mean in pmem context in the context of a machine check exception. Also, do we want to be byte accurate or cache-line accurate for the bytes remaining? The former is much easier than the latter :)

The ideal would be a linear measure of how much of your copy reached (or can reach) non-volatile storage with nothing further copied. You may have to allow for some relaxing of the semantics depending on what the architecture can support.

I think you've got that backwards. memcpy_mcsafe is used to copy *from* persistent memory. The idea is to catch errors when reading pmem, not writing to it.

Quoted 2 lines

What's the problem with just counting bytes copied like usercopy -- why is that harder than cacheline accuracy?

He said the former (i.e. bytes) is easier. So, I think you're on the same page. :)

Quoted 14 lines · On Thu, 05 Apr 2018 16:40:26 -0400 Jeff Moyer <[email protected]> wrote:

Quoted 11 lines · Nicholas Piggin

Quoted 4 lines · On Thu, 5 Apr 2018 15:53:07 +1000 Balbir Singh <[email protected]> wrote:

I'm thinking about it, I wonder what "bytes remaining" mean in pmem context in the context of a machine check exception. Also, do we want to be byte accurate or cache-line accurate for the bytes remaining? The former is much easier than the latter :)

The ideal would be a linear measure of how much of your copy reached (or can reach) non-volatile storage with nothing further copied. You may have to allow for some relaxing of the semantics depending on what the architecture can support.

Quoted 4 lines

I think you've got that backwards. memcpy_mcsafe is used to copy *from* persistent memory. The idea is to catch errors when reading pmem, not writing to it.

Quoted 5 lines

Quoted 2 lines

What's the problem with just counting bytes copied like usercopy -- why is that harder than cacheline accuracy?

He said the former (i.e. bytes) is easier. So, I think you're on the same page. :)

Oh well that makes a lot more sense in my mind now, thanks :)

I thought the cache-aligned might make sense, since usually we'd expect the failure to be at a cache-line level, but our copy_tofrom_user does accurate accounting