2019-08-07 14:58:13

by Santosh Sivaraj

[permalink] [raw]
Subject: [PATCH v8 0/7] powerpc: implement machine check safe memcpy

During a memcpy from a pmem device, if a machine check exception is
generated we end up in a panic. In case of fsdax read, this should
only result in a -EIO. Avoid MCE by implementing memcpy_mcsafe.

Before this patch series:

```
bash-4.4# mount -o dax /dev/pmem0 /mnt/pmem/
[ 7621.714094] Disabling lock debugging due to kernel taint
[ 7621.714099] MCE: CPU0: machine check (Severe) Host UE Load/Store [Not recovered]
[ 7621.714104] MCE: CPU0: NIP: [c000000000088978] memcpy_power7+0x418/0x7e0
[ 7621.714107] MCE: CPU0: Hardware error
[ 7621.714112] opal: Hardware platform error: Unrecoverable Machine Check exception
[ 7621.714118] CPU: 0 PID: 1368 Comm: mount Tainted: G M 5.2.0-rc5-00239-g241e39004581
#50
[ 7621.714123] NIP: c000000000088978 LR: c0000000008e16f8 CTR: 00000000000001de
[ 7621.714129] REGS: c0000000fffbfd70 TRAP: 0200 Tainted: G M
(5.2.0-rc5-00239-g241e39004581)
[ 7621.714131] MSR: 9000000002209033 <SF,HV,VEC,EE,ME,IR,DR,RI,LE> CR: 24428840 XER: 00040000
[ 7621.714160] CFAR: c0000000000889a8 DAR: deadbeefdeadbeef DSISR: 00008000 IRQMASK: 0
[ 7621.714171] GPR00: 000000000e000000 c0000000f0b8b1e0 c0000000012cf100 c0000000ed8e1100
[ 7621.714186] GPR04: c000020000001100 0000000000010000 0000000000000200 03fffffff1272000
[ 7621.714201] GPR08: 0000000080000000 0000000000000010 0000000000000020 0000000000000030
[ 7621.714216] GPR12: 0000000000000040 00007fffb8c6d390 0000000000000050 0000000000000060
[ 7621.714232] GPR16: 0000000000000070 0000000000000000 0000000000000001 c0000000f0b8b960
[ 7621.714247] GPR20: 0000000000000001 c0000000f0b8b940 0000000000000001 0000000000010000
[ 7621.714262] GPR24: c000000001382560 c00c0000003b6380 c00c0000003b6380 0000000000010000
[ 7621.714277] GPR28: 0000000000000000 0000000000010000 c000020000000000 0000000000010000
[ 7621.714294] NIP [c000000000088978] memcpy_power7+0x418/0x7e0
[ 7621.714298] LR [c0000000008e16f8] pmem_do_bvec+0xf8/0x430
... <snip> ...
```

After this patch series:

```
bash-4.4# mount -o dax /dev/pmem0 /mnt/pmem/
[25302.883978] Buffer I/O error on dev pmem0, logical block 0, async page read
[25303.020816] EXT4-fs (pmem0): DAX enabled. Warning: EXPERIMENTAL, use at your own risk
[25303.021236] EXT4-fs (pmem0): Can't read superblock on 2nd try
[25303.152515] EXT4-fs (pmem0): DAX enabled. Warning: EXPERIMENTAL, use at your own risk
[25303.284031] EXT4-fs (pmem0): DAX enabled. Warning: EXPERIMENTAL, use at your own risk
[25304.084100] UDF-fs: bad mount option "dax" or missing value
mount: /mnt/pmem: wrong fs type, bad option, bad superblock on /dev/pmem0, missing codepage or helper
program, or other error.
```

MCE is injected on a pmem address using mambo. The last patch which adds a
nop is only for testing on mambo, where r13 is not restored upon hitting
vector 200.

The memcpy code can be optimised by adding VMX optimizations and GAS macros
can be used to enable code reusablity, which I will send as another series.

---
Change-log:
v8:
* While ignoring UE events, return was used instead of continue.
* Checkpatch fixups for commit log

v7:
* Move schedule_work to be called from irq_work.

v6:
* Don't return pfn, all callees are expecting physical address anyway [nick]
* Patch re-ordering: move exception table patch before memcpy_mcsafe patch [nick]
* Reword commit log for search_exception_tables patch [nick]

v5:
* Don't use search_exception_tables since it searches for module exception tables
also [Nicholas]
* Fix commit message for patch 2 [Nicholas]

v4:
* Squash return remaining bytes patch to memcpy_mcsafe implemtation patch [christophe]
* Access ok should be checked for copy_to_user_mcsafe() [christophe]

v3:
* Drop patch which enables DR/IR for external modules
* Drop notifier call chain, we don't want to do that in real mode
* Return remaining bytes from memcpy_mcsafe correctly
* We no longer restore r13 for simulator tests, rather use a nop at
vector 0x200 [workaround for simulator; not to be merged]

v2:
* Don't set RI bit explicitly [mahesh]
* Re-ordered series to get r13 workaround as the last patch
--
Balbir Singh (2):
powerpc/mce: Fix MCE handling for huge pages
powerpc/memcpy: Add memcpy_mcsafe for pmem

Reza Arbab (1):
powerpc/mce: Make machine_check_ue_event() static

Santosh Sivaraj (4):
powerpc/mce: Schedule work from irq_work
extable: Add function to search only kernel exception table
powerpc/mce: Handle UE event for memcpy_mcsafe
powerpc: add machine check safe copy_to_user

arch/powerpc/Kconfig | 1 +
arch/powerpc/include/asm/mce.h | 6 +-
arch/powerpc/include/asm/string.h | 2 +
arch/powerpc/include/asm/uaccess.h | 14 ++
arch/powerpc/kernel/mce.c | 26 ++-
arch/powerpc/kernel/mce_power.c | 65 ++++---
arch/powerpc/lib/Makefile | 2 +-
arch/powerpc/lib/memcpy_mcsafe_64.S | 242 +++++++++++++++++++++++++++
arch/powerpc/platforms/pseries/ras.c | 9 +-
include/linux/extable.h | 2 +
kernel/extable.c | 11 +-
11 files changed, 337 insertions(+), 43 deletions(-)
create mode 100644 arch/powerpc/lib/memcpy_mcsafe_64.S

--
2.20.1


2019-08-07 14:58:14

by Santosh Sivaraj

[permalink] [raw]
Subject: [PATCH v8 1/7] powerpc/mce: Schedule work from irq_work

schedule_work() cannot be called from MCE exception context as MCE can
interrupt even in interrupt disabled context.

fixes: 733e4a4c ("powerpc/mce: hookup memory_failure for UE errors")
Signed-off-by: Santosh Sivaraj <[email protected]>
---
arch/powerpc/kernel/mce.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/kernel/mce.c b/arch/powerpc/kernel/mce.c
index b18df633eae9..0ab6fa7cbbbb 100644
--- a/arch/powerpc/kernel/mce.c
+++ b/arch/powerpc/kernel/mce.c
@@ -144,7 +144,6 @@ void save_mce_event(struct pt_regs *regs, long handled,
if (phys_addr != ULONG_MAX) {
mce->u.ue_error.physical_address_provided = true;
mce->u.ue_error.physical_address = phys_addr;
- machine_check_ue_event(mce);
}
}
return;
@@ -275,8 +274,7 @@ static void machine_process_ue_event(struct work_struct *work)
}
}
/*
- * process pending MCE event from the mce event queue. This function will be
- * called during syscall exit.
+ * process pending MCE event from the mce event queue.
*/
static void machine_check_process_queued_event(struct irq_work *work)
{
@@ -292,6 +290,10 @@ static void machine_check_process_queued_event(struct irq_work *work)
while (__this_cpu_read(mce_queue_count) > 0) {
index = __this_cpu_read(mce_queue_count) - 1;
evt = this_cpu_ptr(&mce_event_queue[index]);
+
+ if (evt->error_type == MCE_ERROR_TYPE_UE)
+ machine_check_ue_event(evt);
+
machine_check_print_event_info(evt, false, false);
__this_cpu_dec(mce_queue_count);
}
--
2.20.1

2019-08-07 14:58:21

by Santosh Sivaraj

[permalink] [raw]
Subject: [PATCH v8 2/7] powerpc/mce: Make machine_check_ue_event() static

From: Reza Arbab <[email protected]>

The function doesn't get used outside this file, so make it static.

Signed-off-by: Reza Arbab <[email protected]>
Signed-off-by: Santosh Sivaraj <[email protected]>
Reviewed-by: Nicholas Piggin <[email protected]>
---
arch/powerpc/kernel/mce.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/kernel/mce.c b/arch/powerpc/kernel/mce.c
index 0ab6fa7cbbbb..8c0b471658a7 100644
--- a/arch/powerpc/kernel/mce.c
+++ b/arch/powerpc/kernel/mce.c
@@ -33,7 +33,7 @@ static DEFINE_PER_CPU(struct machine_check_event[MAX_MC_EVT],
mce_ue_event_queue);

static void machine_check_process_queued_event(struct irq_work *work);
-void machine_check_ue_event(struct machine_check_event *evt);
+static void machine_check_ue_event(struct machine_check_event *evt);
static void machine_process_ue_event(struct work_struct *work);

static struct irq_work mce_event_process_work = {
@@ -202,7 +202,7 @@ void release_mce_event(void)
/*
* Queue up the MCE event which then can be handled later.
*/
-void machine_check_ue_event(struct machine_check_event *evt)
+static void machine_check_ue_event(struct machine_check_event *evt)
{
int index;

--
2.20.1

2019-08-07 14:58:28

by Santosh Sivaraj

[permalink] [raw]
Subject: [PATCH v8 3/7] powerpc/mce: Fix MCE handling for huge pages

From: Balbir Singh <[email protected]>

The current code would fail on huge pages addresses, since the shift would
be incorrect. Use the correct page shift value returned by
__find_linux_pte() to get the correct physical address. The code is more
generic and can handle both regular and compound pages.

Fixes: ba41e1e1ccb9 ("powerpc/mce: Hookup derror (load/store) UE errors")
Signed-off-by: Balbir Singh <[email protected]>
[[email protected]: Fixup pseries_do_memory_failure()]
Signed-off-by: Reza Arbab <[email protected]>
Co-developed-by: Santosh Sivaraj <[email protected]>
Signed-off-by: Santosh Sivaraj <[email protected]>
---
arch/powerpc/include/asm/mce.h | 2 +-
arch/powerpc/kernel/mce_power.c | 50 ++++++++++++++--------------
arch/powerpc/platforms/pseries/ras.c | 9 ++---
3 files changed, 29 insertions(+), 32 deletions(-)

diff --git a/arch/powerpc/include/asm/mce.h b/arch/powerpc/include/asm/mce.h
index a4c6a74ad2fb..f3a6036b6bc0 100644
--- a/arch/powerpc/include/asm/mce.h
+++ b/arch/powerpc/include/asm/mce.h
@@ -209,7 +209,7 @@ extern void release_mce_event(void);
extern void machine_check_queue_event(void);
extern void machine_check_print_event_info(struct machine_check_event *evt,
bool user_mode, bool in_guest);
-unsigned long addr_to_pfn(struct pt_regs *regs, unsigned long addr);
+unsigned long addr_to_phys(struct pt_regs *regs, unsigned long addr);
#ifdef CONFIG_PPC_BOOK3S_64
void flush_and_reload_slb(void);
#endif /* CONFIG_PPC_BOOK3S_64 */
diff --git a/arch/powerpc/kernel/mce_power.c b/arch/powerpc/kernel/mce_power.c
index a814d2dfb5b0..bed38a8e2e50 100644
--- a/arch/powerpc/kernel/mce_power.c
+++ b/arch/powerpc/kernel/mce_power.c
@@ -20,13 +20,14 @@
#include <asm/exception-64s.h>

/*
- * Convert an address related to an mm to a PFN. NOTE: we are in real
- * mode, we could potentially race with page table updates.
+ * Convert an address related to an mm to a physical address.
+ * NOTE: we are in real mode, we could potentially race with page table updates.
*/
-unsigned long addr_to_pfn(struct pt_regs *regs, unsigned long addr)
+unsigned long addr_to_phys(struct pt_regs *regs, unsigned long addr)
{
- pte_t *ptep;
- unsigned long flags;
+ pte_t *ptep, pte;
+ unsigned int shift;
+ unsigned long flags, phys_addr;
struct mm_struct *mm;

if (user_mode(regs))
@@ -35,14 +36,21 @@ unsigned long addr_to_pfn(struct pt_regs *regs, unsigned long addr)
mm = &init_mm;

local_irq_save(flags);
- if (mm == current->mm)
- ptep = find_current_mm_pte(mm->pgd, addr, NULL, NULL);
- else
- ptep = find_init_mm_pte(addr, NULL);
+ ptep = __find_linux_pte(mm->pgd, addr, NULL, &shift);
local_irq_restore(flags);
+
if (!ptep || pte_special(*ptep))
return ULONG_MAX;
- return pte_pfn(*ptep);
+
+ pte = *ptep;
+ if (shift > PAGE_SHIFT) {
+ unsigned long rpnmask = (1ul << shift) - PAGE_SIZE;
+
+ pte = __pte(pte_val(pte) | (addr & rpnmask));
+ }
+ phys_addr = pte_pfn(pte) << PAGE_SHIFT;
+
+ return phys_addr;
}

/* flush SLBs and reload */
@@ -354,18 +362,16 @@ static int mce_find_instr_ea_and_pfn(struct pt_regs *regs, uint64_t *addr,
* faults
*/
int instr;
- unsigned long pfn, instr_addr;
+ unsigned long instr_addr;
struct instruction_op op;
struct pt_regs tmp = *regs;

- pfn = addr_to_pfn(regs, regs->nip);
- if (pfn != ULONG_MAX) {
- instr_addr = (pfn << PAGE_SHIFT) + (regs->nip & ~PAGE_MASK);
+ instr_addr = addr_to_phys(regs, regs->nip) + (regs->nip & ~PAGE_MASK);
+ if (instr_addr != ULONG_MAX) {
instr = *(unsigned int *)(instr_addr);
if (!analyse_instr(&op, &tmp, instr)) {
- pfn = addr_to_pfn(regs, op.ea);
*addr = op.ea;
- *phys_addr = (pfn << PAGE_SHIFT);
+ *phys_addr = addr_to_phys(regs, op.ea);
return 0;
}
/*
@@ -440,15 +446,9 @@ static int mce_handle_ierror(struct pt_regs *regs,
*addr = regs->nip;
if (mce_err->sync_error &&
table[i].error_type == MCE_ERROR_TYPE_UE) {
- unsigned long pfn;
-
- if (get_paca()->in_mce < MAX_MCE_DEPTH) {
- pfn = addr_to_pfn(regs, regs->nip);
- if (pfn != ULONG_MAX) {
- *phys_addr =
- (pfn << PAGE_SHIFT);
- }
- }
+ if (get_paca()->in_mce < MAX_MCE_DEPTH)
+ *phys_addr = addr_to_phys(regs,
+ regs->nip);
}
}
return handled;
diff --git a/arch/powerpc/platforms/pseries/ras.c b/arch/powerpc/platforms/pseries/ras.c
index f16fdd0f71f7..5743f6353638 100644
--- a/arch/powerpc/platforms/pseries/ras.c
+++ b/arch/powerpc/platforms/pseries/ras.c
@@ -739,13 +739,10 @@ static void pseries_do_memory_failure(struct pt_regs *regs,
if (mce_log->sub_err_type & UE_LOGICAL_ADDR_PROVIDED) {
paddr = be64_to_cpu(mce_log->logical_address);
} else if (mce_log->sub_err_type & UE_EFFECTIVE_ADDR_PROVIDED) {
- unsigned long pfn;
-
- pfn = addr_to_pfn(regs,
- be64_to_cpu(mce_log->effective_address));
- if (pfn == ULONG_MAX)
+ paddr = addr_to_phys(regs,
+ be64_to_cpu(mce_log->effective_address));
+ if (paddr == ULONG_MAX)
return;
- paddr = pfn << PAGE_SHIFT;
} else {
return;
}
--
2.20.1

2019-08-07 14:58:51

by Santosh Sivaraj

[permalink] [raw]
Subject: [PATCH v8 5/7] powerpc/memcpy: Add memcpy_mcsafe for pmem

From: Balbir Singh <[email protected]>

The pmem infrastructure uses memcpy_mcsafe in the pmem layer so as to
convert machine check exceptions into a return value on failure in case
a machine check exception is encountered during the memcpy. The return
value is the number of bytes remaining to be copied.

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]>
[[email protected]: Added symbol export]
Co-developed-by: Santosh Sivaraj <[email protected]>
Signed-off-by: Santosh Sivaraj <[email protected]>
---
arch/powerpc/include/asm/string.h | 2 +
arch/powerpc/lib/Makefile | 2 +-
arch/powerpc/lib/memcpy_mcsafe_64.S | 242 ++++++++++++++++++++++++++++
3 files changed, 245 insertions(+), 1 deletion(-)
create mode 100644 arch/powerpc/lib/memcpy_mcsafe_64.S

diff --git a/arch/powerpc/include/asm/string.h b/arch/powerpc/include/asm/string.h
index 9bf6dffb4090..b72692702f35 100644
--- a/arch/powerpc/include/asm/string.h
+++ b/arch/powerpc/include/asm/string.h
@@ -53,7 +53,9 @@ void *__memmove(void *to, const void *from, __kernel_size_t n);
#ifndef CONFIG_KASAN
#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);
diff --git a/arch/powerpc/lib/Makefile b/arch/powerpc/lib/Makefile
index eebc782d89a5..fa6b1b657b43 100644
--- a/arch/powerpc/lib/Makefile
+++ b/arch/powerpc/lib/Makefile
@@ -39,7 +39,7 @@ obj-$(CONFIG_PPC_BOOK3S_64) += copyuser_power7.o copypage_power7.o \
memcpy_power7.o

obj64-y += copypage_64.o copyuser_64.o mem_64.o hweight_64.o \
- memcpy_64.o pmem.o
+ memcpy_64.o pmem.o memcpy_mcsafe_64.o

obj64-$(CONFIG_SMP) += locks.o
obj64-$(CONFIG_ALTIVEC) += vmx-helper.o
diff --git a/arch/powerpc/lib/memcpy_mcsafe_64.S b/arch/powerpc/lib/memcpy_mcsafe_64.S
new file mode 100644
index 000000000000..949976dc115d
--- /dev/null
+++ b/arch/powerpc/lib/memcpy_mcsafe_64.S
@@ -0,0 +1,242 @@
+/* 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>
+#include <asm/export.h>
+
+ .macro err1
+100:
+ EX_TABLE(100b,.Ldo_err1)
+ .endm
+
+ .macro err2
+200:
+ EX_TABLE(200b,.Ldo_err2)
+ .endm
+
+ .macro err3
+300: EX_TABLE(300b,.Ldone)
+ .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:
+ /* Do a byte by byte copy to get the exact remaining size */
+ mtctr r7
+46:
+err3; lbz r0,0(r4)
+ addi r4,r4,1
+err3; stb r0,0(r3)
+ addi r3,r3,1
+ bdnz 46b
+ li r3,0
+ blr
+
+.Ldone:
+ mfctr r3
+ blr
+
+
+_GLOBAL(memcpy_mcsafe)
+ mr r7,r5
+ 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
+ subi r7,r7,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
+ subi r7,r7,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
+ subi r7,r7,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 r8,16(r4)
+err2; ld r9,24(r4)
+err2; ld r10,32(r4)
+err2; ld r11,40(r4)
+err2; ld r12,48(r4)
+err2; ld r14,56(r4)
+err2; ld r15,64(r4)
+err2; ld r16,72(r4)
+err2; ld r17,80(r4)
+err2; ld r18,88(r4)
+err2; ld r19,96(r4)
+err2; ld r20,104(r4)
+err2; ld r21,112(r4)
+err2; ld r22,120(r4)
+ addi r4,r4,128
+err2; std r0,0(r3)
+err2; std r6,8(r3)
+err2; std r8,16(r3)
+err2; std r9,24(r3)
+err2; std r10,32(r3)
+err2; std r11,40(r3)
+err2; std r12,48(r3)
+err2; std r14,56(r3)
+err2; std r15,64(r3)
+err2; std r16,72(r3)
+err2; std r17,80(r3)
+err2; std r18,88(r3)
+err2; std r19,96(r3)
+err2; std r20,104(r3)
+err2; std r21,112(r3)
+err2; std r22,120(r3)
+ addi r3,r3,128
+ subi r7,r7,128
+ bdnz 4b
+
+ clrldi r5,r5,(64-7)
+
+ /* Up to 127B to go */
+5: srdi r6,r5,4
+ mtocrf 0x01,r6
+
+6: bf cr7*4+1,7f
+err2; ld r0,0(r4)
+err2; ld r6,8(r4)
+err2; ld r8,16(r4)
+err2; ld r9,24(r4)
+err2; ld r10,32(r4)
+err2; ld r11,40(r4)
+err2; ld r12,48(r4)
+err2; ld r14,56(r4)
+ addi r4,r4,64
+err2; std r0,0(r3)
+err2; std r6,8(r3)
+err2; std r8,16(r3)
+err2; std r9,24(r3)
+err2; std r10,32(r3)
+err2; std r11,40(r3)
+err2; std r12,48(r3)
+err2; std r14,56(r3)
+ addi r3,r3,64
+ subi r7,r7,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 63B to go */
+ bf cr7*4+2,8f
+err1; ld r0,0(r4)
+err1; ld r6,8(r4)
+err1; ld r8,16(r4)
+err1; ld r9,24(r4)
+ addi r4,r4,32
+err1; std r0,0(r3)
+err1; std r6,8(r3)
+err1; std r8,16(r3)
+err1; std r9,24(r3)
+ addi r3,r3,32
+ subi r7,r7,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
+ subi r7,r7,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
+ subi r7,r7,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
+ subi r7,r7,4
+
+13: bf cr7*4+2,14f
+err1; lhz r0,0(r4)
+ addi r4,r4,2
+err1; sth r0,0(r3)
+ addi r3,r3,2
+ subi r7,r7,2
+
+14: bf cr7*4+3,15f
+err1; lbz r0,0(r4)
+err1; stb r0,0(r3)
+
+15: li r3,0
+ blr
+
+EXPORT_SYMBOL_GPL(memcpy_mcsafe);
--
2.20.1

2019-08-07 14:58:54

by Santosh Sivaraj

[permalink] [raw]
Subject: [PATCH v8 6/7] powerpc/mce: Handle UE event for memcpy_mcsafe

If we take a UE on one of the instructions with a fixup entry, set nip
to continue execution at the fixup entry. Stop processing the event
further or print it.

Co-developed-by: Reza Arbab <[email protected]>
Signed-off-by: Reza Arbab <[email protected]>
Cc: Mahesh Salgaonkar <[email protected]>
Signed-off-by: Santosh Sivaraj <[email protected]>
---
arch/powerpc/include/asm/mce.h | 4 +++-
arch/powerpc/kernel/mce.c | 18 ++++++++++++++++--
arch/powerpc/kernel/mce_power.c | 15 +++++++++++++--
3 files changed, 32 insertions(+), 5 deletions(-)

diff --git a/arch/powerpc/include/asm/mce.h b/arch/powerpc/include/asm/mce.h
index f3a6036b6bc0..e1931c8c2743 100644
--- a/arch/powerpc/include/asm/mce.h
+++ b/arch/powerpc/include/asm/mce.h
@@ -122,7 +122,8 @@ struct machine_check_event {
enum MCE_UeErrorType ue_error_type:8;
u8 effective_address_provided;
u8 physical_address_provided;
- u8 reserved_1[5];
+ u8 ignore_event;
+ u8 reserved_1[4];
u64 effective_address;
u64 physical_address;
u8 reserved_2[8];
@@ -193,6 +194,7 @@ struct mce_error_info {
enum MCE_Initiator initiator:8;
enum MCE_ErrorClass error_class:8;
bool sync_error;
+ bool ignore_event;
};

#define MAX_MC_EVT 100
diff --git a/arch/powerpc/kernel/mce.c b/arch/powerpc/kernel/mce.c
index 8c0b471658a7..a80f5d6ef166 100644
--- a/arch/powerpc/kernel/mce.c
+++ b/arch/powerpc/kernel/mce.c
@@ -33,7 +33,6 @@ static DEFINE_PER_CPU(struct machine_check_event[MAX_MC_EVT],
mce_ue_event_queue);

static void machine_check_process_queued_event(struct irq_work *work);
-static void machine_check_ue_event(struct machine_check_event *evt);
static void machine_process_ue_event(struct work_struct *work);

static struct irq_work mce_event_process_work = {
@@ -144,6 +143,7 @@ void save_mce_event(struct pt_regs *regs, long handled,
if (phys_addr != ULONG_MAX) {
mce->u.ue_error.physical_address_provided = true;
mce->u.ue_error.physical_address = phys_addr;
+ mce->u.ue_error.ignore_event = mce_err->ignore_event;
}
}
return;
@@ -256,8 +256,17 @@ static void machine_process_ue_event(struct work_struct *work)
/*
* This should probably queued elsewhere, but
* oh! well
+ *
+ * Don't report this machine check because the caller has a
+ * asked us to ignore the event, it has a fixup handler which
+ * will do the appropriate error handling and reporting.
*/
if (evt->error_type == MCE_ERROR_TYPE_UE) {
+ if (evt->u.ue_error.ignore_event) {
+ __this_cpu_dec(mce_ue_count);
+ continue;
+ }
+
if (evt->u.ue_error.physical_address_provided) {
unsigned long pfn;

@@ -291,8 +300,13 @@ static void machine_check_process_queued_event(struct irq_work *work)
index = __this_cpu_read(mce_queue_count) - 1;
evt = this_cpu_ptr(&mce_event_queue[index]);

- if (evt->error_type == MCE_ERROR_TYPE_UE)
+ if (evt->error_type == MCE_ERROR_TYPE_UE) {
machine_check_ue_event(evt);
+ if (evt->u.ue_error.ignore_event) {
+ __this_cpu_dec(mce_queue_count);
+ continue;
+ }
+ }

machine_check_print_event_info(evt, false, false);
__this_cpu_dec(mce_queue_count);
diff --git a/arch/powerpc/kernel/mce_power.c b/arch/powerpc/kernel/mce_power.c
index bed38a8e2e50..36ca45bbb273 100644
--- a/arch/powerpc/kernel/mce_power.c
+++ b/arch/powerpc/kernel/mce_power.c
@@ -11,6 +11,7 @@

#include <linux/types.h>
#include <linux/ptrace.h>
+#include <linux/extable.h>
#include <asm/mmu.h>
#include <asm/mce.h>
#include <asm/machdep.h>
@@ -18,6 +19,7 @@
#include <asm/pte-walk.h>
#include <asm/sstep.h>
#include <asm/exception-64s.h>
+#include <asm/extable.h>

/*
* Convert an address related to an mm to a physical address.
@@ -558,9 +560,18 @@ static int mce_handle_derror(struct pt_regs *regs,
return 0;
}

-static long mce_handle_ue_error(struct pt_regs *regs)
+static long mce_handle_ue_error(struct pt_regs *regs,
+ struct mce_error_info *mce_err)
{
long handled = 0;
+ const struct exception_table_entry *entry;
+
+ entry = search_kernel_exception_table(regs->nip);
+ if (entry) {
+ mce_err->ignore_event = true;
+ regs->nip = extable_fixup(entry);
+ return 1;
+ }

/*
* On specific SCOM read via MMIO we may get a machine check
@@ -593,7 +604,7 @@ static long mce_handle_error(struct pt_regs *regs,
&phys_addr);

if (!handled && mce_err.error_type == MCE_ERROR_TYPE_UE)
- handled = mce_handle_ue_error(regs);
+ handled = mce_handle_ue_error(regs, &mce_err);

save_mce_event(regs, handled, &mce_err, regs->nip, addr, phys_addr);

--
2.20.1

2019-08-07 14:59:26

by Santosh Sivaraj

[permalink] [raw]
Subject: [PATCH v8 7/7] powerpc: add machine check safe copy_to_user

Use memcpy_mcsafe() implementation to define copy_to_user_mcsafe()

Signed-off-by: Santosh Sivaraj <[email protected]>
---
arch/powerpc/Kconfig | 1 +
arch/powerpc/include/asm/uaccess.h | 14 ++++++++++++++
2 files changed, 15 insertions(+)

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 77f6ebf97113..4316e36095a2 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -137,6 +137,7 @@ config PPC
select ARCH_HAS_STRICT_KERNEL_RWX if ((PPC_BOOK3S_64 || PPC32) && !RELOCATABLE && !HIBERNATION)
select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST
select ARCH_HAS_UACCESS_FLUSHCACHE if PPC64
+ select ARCH_HAS_UACCESS_MCSAFE if PPC64
select ARCH_HAS_UBSAN_SANITIZE_ALL
select ARCH_HAVE_NMI_SAFE_CMPXCHG
select ARCH_KEEP_MEMBLOCK
diff --git a/arch/powerpc/include/asm/uaccess.h b/arch/powerpc/include/asm/uaccess.h
index 8b03eb44e876..15002b51ff18 100644
--- a/arch/powerpc/include/asm/uaccess.h
+++ b/arch/powerpc/include/asm/uaccess.h
@@ -387,6 +387,20 @@ static inline unsigned long raw_copy_to_user(void __user *to,
return ret;
}

+static __always_inline unsigned long __must_check
+copy_to_user_mcsafe(void __user *to, const void *from, unsigned long n)
+{
+ if (likely(check_copy_size(from, n, true))) {
+ if (access_ok(to, n)) {
+ allow_write_to_user(to, n);
+ n = memcpy_mcsafe((void *)to, from, n);
+ prevent_write_to_user(to, n);
+ }
+ }
+
+ return n;
+}
+
extern unsigned long __clear_user(void __user *addr, unsigned long size);

static inline unsigned long clear_user(void __user *addr, unsigned long size)
--
2.20.1

2019-08-07 15:11:36

by Santosh Sivaraj

[permalink] [raw]
Subject: [PATCH v8 4/7] extable: Add function to search only kernel exception table

Certain architecture specific operating modes (e.g., in powerpc machine
check handler that is unable to access vmalloc memory), the
search_exception_tables cannot be called because it also searches the
module exception tables if entry is not found in the kernel exception
table.

Cc: Thomas Gleixner <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Nicholas Piggin <[email protected]>
Signed-off-by: Santosh Sivaraj <[email protected]>
Reviewed-by: Nicholas Piggin <[email protected]>
---
include/linux/extable.h | 2 ++
kernel/extable.c | 11 +++++++++--
2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/include/linux/extable.h b/include/linux/extable.h
index 41c5b3a25f67..81ecfaa83ad3 100644
--- a/include/linux/extable.h
+++ b/include/linux/extable.h
@@ -19,6 +19,8 @@ void trim_init_extable(struct module *m);

/* Given an address, look for it in the exception tables */
const struct exception_table_entry *search_exception_tables(unsigned long add);
+const struct exception_table_entry *
+search_kernel_exception_table(unsigned long addr);

#ifdef CONFIG_MODULES
/* For extable.c to search modules' exception tables. */
diff --git a/kernel/extable.c b/kernel/extable.c
index e23cce6e6092..f6c9406eec7d 100644
--- a/kernel/extable.c
+++ b/kernel/extable.c
@@ -40,13 +40,20 @@ void __init sort_main_extable(void)
}
}

+/* Given an address, look for it in the kernel exception table */
+const
+struct exception_table_entry *search_kernel_exception_table(unsigned long addr)
+{
+ return search_extable(__start___ex_table,
+ __stop___ex_table - __start___ex_table, addr);
+}
+
/* Given an address, look for it in the exception tables. */
const struct exception_table_entry *search_exception_tables(unsigned long addr)
{
const struct exception_table_entry *e;

- e = search_extable(__start___ex_table,
- __stop___ex_table - __start___ex_table, addr);
+ e = search_kernel_exception_table(addr);
if (!e)
e = search_module_extables(addr);
return e;
--
2.20.1

Subject: Re: [PATCH v8 1/7] powerpc/mce: Schedule work from irq_work

On 8/7/19 8:26 PM, Santosh Sivaraj wrote:
> schedule_work() cannot be called from MCE exception context as MCE can
> interrupt even in interrupt disabled context.
>
> fixes: 733e4a4c ("powerpc/mce: hookup memory_failure for UE errors")
> Signed-off-by: Santosh Sivaraj <[email protected]>
> ---
> arch/powerpc/kernel/mce.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/arch/powerpc/kernel/mce.c b/arch/powerpc/kernel/mce.c
> index b18df633eae9..0ab6fa7cbbbb 100644
> --- a/arch/powerpc/kernel/mce.c
> +++ b/arch/powerpc/kernel/mce.c
> @@ -144,7 +144,6 @@ void save_mce_event(struct pt_regs *regs, long handled,
> if (phys_addr != ULONG_MAX) {
> mce->u.ue_error.physical_address_provided = true;
> mce->u.ue_error.physical_address = phys_addr;
> - machine_check_ue_event(mce);
> }
> }
> return;
> @@ -275,8 +274,7 @@ static void machine_process_ue_event(struct work_struct *work)
> }
> }
> /*
> - * process pending MCE event from the mce event queue. This function will be
> - * called during syscall exit.
> + * process pending MCE event from the mce event queue.
> */
> static void machine_check_process_queued_event(struct irq_work *work)
> {
> @@ -292,6 +290,10 @@ static void machine_check_process_queued_event(struct irq_work *work)
> while (__this_cpu_read(mce_queue_count) > 0) {
> index = __this_cpu_read(mce_queue_count) - 1;
> evt = this_cpu_ptr(&mce_event_queue[index]);
> +
> + if (evt->error_type == MCE_ERROR_TYPE_UE)
> + machine_check_ue_event(evt);

This will work only for the event that are queued by mce handler, others
will get ignored. I think you should introduce a separate irq work queue
for schedule_work().

Thanks,
-Mahesh.

Subject: Re: [PATCH v8 3/7] powerpc/mce: Fix MCE handling for huge pages

On 8/7/19 8:26 PM, Santosh Sivaraj wrote:
> From: Balbir Singh <[email protected]>
>
> The current code would fail on huge pages addresses, since the shift would
> be incorrect. Use the correct page shift value returned by
> __find_linux_pte() to get the correct physical address. The code is more
> generic and can handle both regular and compound pages.
>
> Fixes: ba41e1e1ccb9 ("powerpc/mce: Hookup derror (load/store) UE errors")
> Signed-off-by: Balbir Singh <[email protected]>
> [[email protected]: Fixup pseries_do_memory_failure()]
> Signed-off-by: Reza Arbab <[email protected]>
> Co-developed-by: Santosh Sivaraj <[email protected]>
> Signed-off-by: Santosh Sivaraj <[email protected]>
> ---
> arch/powerpc/include/asm/mce.h | 2 +-
> arch/powerpc/kernel/mce_power.c | 50 ++++++++++++++--------------
> arch/powerpc/platforms/pseries/ras.c | 9 ++---
> 3 files changed, 29 insertions(+), 32 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/mce.h b/arch/powerpc/include/asm/mce.h
> index a4c6a74ad2fb..f3a6036b6bc0 100644
> --- a/arch/powerpc/include/asm/mce.h
> +++ b/arch/powerpc/include/asm/mce.h
> @@ -209,7 +209,7 @@ extern void release_mce_event(void);
> extern void machine_check_queue_event(void);
> extern void machine_check_print_event_info(struct machine_check_event *evt,
> bool user_mode, bool in_guest);
> -unsigned long addr_to_pfn(struct pt_regs *regs, unsigned long addr);
> +unsigned long addr_to_phys(struct pt_regs *regs, unsigned long addr);
> #ifdef CONFIG_PPC_BOOK3S_64
> void flush_and_reload_slb(void);
> #endif /* CONFIG_PPC_BOOK3S_64 */
> diff --git a/arch/powerpc/kernel/mce_power.c b/arch/powerpc/kernel/mce_power.c
> index a814d2dfb5b0..bed38a8e2e50 100644
> --- a/arch/powerpc/kernel/mce_power.c
> +++ b/arch/powerpc/kernel/mce_power.c
> @@ -20,13 +20,14 @@
> #include <asm/exception-64s.h>
>
> /*
> - * Convert an address related to an mm to a PFN. NOTE: we are in real
> - * mode, we could potentially race with page table updates.
> + * Convert an address related to an mm to a physical address.
> + * NOTE: we are in real mode, we could potentially race with page table updates.
> */
> -unsigned long addr_to_pfn(struct pt_regs *regs, unsigned long addr)
> +unsigned long addr_to_phys(struct pt_regs *regs, unsigned long addr)
> {
> - pte_t *ptep;
> - unsigned long flags;
> + pte_t *ptep, pte;
> + unsigned int shift;
> + unsigned long flags, phys_addr;
> struct mm_struct *mm;
>
> if (user_mode(regs))
> @@ -35,14 +36,21 @@ unsigned long addr_to_pfn(struct pt_regs *regs, unsigned long addr)
> mm = &init_mm;
>
> local_irq_save(flags);
> - if (mm == current->mm)
> - ptep = find_current_mm_pte(mm->pgd, addr, NULL, NULL);
> - else
> - ptep = find_init_mm_pte(addr, NULL);
> + ptep = __find_linux_pte(mm->pgd, addr, NULL, &shift);
> local_irq_restore(flags);
> +
> if (!ptep || pte_special(*ptep))
> return ULONG_MAX;
> - return pte_pfn(*ptep);
> +
> + pte = *ptep;
> + if (shift > PAGE_SHIFT) {
> + unsigned long rpnmask = (1ul << shift) - PAGE_SIZE;
> +
> + pte = __pte(pte_val(pte) | (addr & rpnmask));
> + }
> + phys_addr = pte_pfn(pte) << PAGE_SHIFT;
> +
> + return phys_addr;
> }
>
> /* flush SLBs and reload */
> @@ -354,18 +362,16 @@ static int mce_find_instr_ea_and_pfn(struct pt_regs *regs, uint64_t *addr,

Now that we have addr_to_phys() can we change this function name as well
to mce_find_instr_ea_and_phys() ?

Tested-by: Mahesh Salgaonkar <[email protected]>

This should go to stable tree. Can you move this patch to 2nd position ?

Thanks,
-Mahesh.

2019-08-10 09:35:09

by Michael Ellerman

[permalink] [raw]
Subject: Re: [PATCH v8 7/7] powerpc: add machine check safe copy_to_user

Santosh Sivaraj <[email protected]> writes:
> Use memcpy_mcsafe() implementation to define copy_to_user_mcsafe()
>
> Signed-off-by: Santosh Sivaraj <[email protected]>
> ---
> arch/powerpc/Kconfig | 1 +
> arch/powerpc/include/asm/uaccess.h | 14 ++++++++++++++
> 2 files changed, 15 insertions(+)
>
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index 77f6ebf97113..4316e36095a2 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -137,6 +137,7 @@ config PPC
> select ARCH_HAS_STRICT_KERNEL_RWX if ((PPC_BOOK3S_64 || PPC32) && !RELOCATABLE && !HIBERNATION)
> select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST
> select ARCH_HAS_UACCESS_FLUSHCACHE if PPC64
> + select ARCH_HAS_UACCESS_MCSAFE if PPC64
> select ARCH_HAS_UBSAN_SANITIZE_ALL
> select ARCH_HAVE_NMI_SAFE_CMPXCHG
> select ARCH_KEEP_MEMBLOCK
> diff --git a/arch/powerpc/include/asm/uaccess.h b/arch/powerpc/include/asm/uaccess.h
> index 8b03eb44e876..15002b51ff18 100644
> --- a/arch/powerpc/include/asm/uaccess.h
> +++ b/arch/powerpc/include/asm/uaccess.h
> @@ -387,6 +387,20 @@ static inline unsigned long raw_copy_to_user(void __user *to,
> return ret;
> }
>
> +static __always_inline unsigned long __must_check
> +copy_to_user_mcsafe(void __user *to, const void *from, unsigned long n)
> +{
> + if (likely(check_copy_size(from, n, true))) {
> + if (access_ok(to, n)) {
> + allow_write_to_user(to, n);
> + n = memcpy_mcsafe((void *)to, from, n);
> + prevent_write_to_user(to, n);
> + }
> + }
> +
> + return n;
> +}

This looks OK to me.

It would be nice though if copy_to_user_mcsafe() followed the pattern of
the other copy_to_user() etc. routines where the arch code is only
responsible for the actual arch details, and all the checks are done in
the generic code. That would be a good cleanup to do after this has gone
in, as the 2nd implementation of the API.

cheers

2019-08-11 01:33:35

by Santosh Sivaraj

[permalink] [raw]
Subject: Re: [PATCH v8 3/7] powerpc/mce: Fix MCE handling for huge pages

Mahesh Jagannath Salgaonkar <[email protected]> writes:

> On 8/7/19 8:26 PM, Santosh Sivaraj wrote:
>> From: Balbir Singh <[email protected]>
>>
>> The current code would fail on huge pages addresses, since the shift would
>> be incorrect. Use the correct page shift value returned by
>> __find_linux_pte() to get the correct physical address. The code is more
>> generic and can handle both regular and compound pages.
>>
>> Fixes: ba41e1e1ccb9 ("powerpc/mce: Hookup derror (load/store) UE errors")
>> Signed-off-by: Balbir Singh <[email protected]>
>> [[email protected]: Fixup pseries_do_memory_failure()]
>> Signed-off-by: Reza Arbab <[email protected]>
>> Co-developed-by: Santosh Sivaraj <[email protected]>
>> Signed-off-by: Santosh Sivaraj <[email protected]>
>> ---
>> arch/powerpc/include/asm/mce.h | 2 +-
>> arch/powerpc/kernel/mce_power.c | 50 ++++++++++++++--------------
>> arch/powerpc/platforms/pseries/ras.c | 9 ++---
>> 3 files changed, 29 insertions(+), 32 deletions(-)
>>
>> diff --git a/arch/powerpc/include/asm/mce.h b/arch/powerpc/include/asm/mce.h
>> index a4c6a74ad2fb..f3a6036b6bc0 100644
>> --- a/arch/powerpc/include/asm/mce.h
>> +++ b/arch/powerpc/include/asm/mce.h
>> @@ -209,7 +209,7 @@ extern void release_mce_event(void);
>> extern void machine_check_queue_event(void);
>> extern void machine_check_print_event_info(struct machine_check_event *evt,
>> bool user_mode, bool in_guest);
>> -unsigned long addr_to_pfn(struct pt_regs *regs, unsigned long addr);
>> +unsigned long addr_to_phys(struct pt_regs *regs, unsigned long addr);
>> #ifdef CONFIG_PPC_BOOK3S_64
>> void flush_and_reload_slb(void);
>> #endif /* CONFIG_PPC_BOOK3S_64 */
>> diff --git a/arch/powerpc/kernel/mce_power.c b/arch/powerpc/kernel/mce_power.c
>> index a814d2dfb5b0..bed38a8e2e50 100644
>> --- a/arch/powerpc/kernel/mce_power.c
>> +++ b/arch/powerpc/kernel/mce_power.c
>> @@ -20,13 +20,14 @@
>> #include <asm/exception-64s.h>
>>
>> /*
>> - * Convert an address related to an mm to a PFN. NOTE: we are in real
>> - * mode, we could potentially race with page table updates.
>> + * Convert an address related to an mm to a physical address.
>> + * NOTE: we are in real mode, we could potentially race with page table updates.
>> */
>> -unsigned long addr_to_pfn(struct pt_regs *regs, unsigned long addr)
>> +unsigned long addr_to_phys(struct pt_regs *regs, unsigned long addr)
>> {
>> - pte_t *ptep;
>> - unsigned long flags;
>> + pte_t *ptep, pte;
>> + unsigned int shift;
>> + unsigned long flags, phys_addr;
>> struct mm_struct *mm;
>>
>> if (user_mode(regs))
>> @@ -35,14 +36,21 @@ unsigned long addr_to_pfn(struct pt_regs *regs, unsigned long addr)
>> mm = &init_mm;
>>
>> local_irq_save(flags);
>> - if (mm == current->mm)
>> - ptep = find_current_mm_pte(mm->pgd, addr, NULL, NULL);
>> - else
>> - ptep = find_init_mm_pte(addr, NULL);
>> + ptep = __find_linux_pte(mm->pgd, addr, NULL, &shift);
>> local_irq_restore(flags);
>> +
>> if (!ptep || pte_special(*ptep))
>> return ULONG_MAX;
>> - return pte_pfn(*ptep);
>> +
>> + pte = *ptep;
>> + if (shift > PAGE_SHIFT) {
>> + unsigned long rpnmask = (1ul << shift) - PAGE_SIZE;
>> +
>> + pte = __pte(pte_val(pte) | (addr & rpnmask));
>> + }
>> + phys_addr = pte_pfn(pte) << PAGE_SHIFT;
>> +
>> + return phys_addr;
>> }
>>
>> /* flush SLBs and reload */
>> @@ -354,18 +362,16 @@ static int mce_find_instr_ea_and_pfn(struct pt_regs *regs, uint64_t *addr,
>
> Now that we have addr_to_phys() can we change this function name as well
> to mce_find_instr_ea_and_phys() ?

Makes sense, will avoid confusions.

>
> Tested-by: Mahesh Salgaonkar <[email protected]>
>
> This should go to stable tree. Can you move this patch to 2nd position ?

Thanks for testing. Sure. Will reorder and mark stable as well.

Thanks,
Santosh
>
> Thanks,
> -Mahesh.
>

2019-08-11 01:36:19

by Santosh Sivaraj

[permalink] [raw]
Subject: Re: [PATCH v8 7/7] powerpc: add machine check safe copy_to_user

Michael Ellerman <[email protected]> writes:

> Santosh Sivaraj <[email protected]> writes:
>> Use memcpy_mcsafe() implementation to define copy_to_user_mcsafe()
>>
>> Signed-off-by: Santosh Sivaraj <[email protected]>
>> ---
>> arch/powerpc/Kconfig | 1 +
>> arch/powerpc/include/asm/uaccess.h | 14 ++++++++++++++
>> 2 files changed, 15 insertions(+)
>>
>> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
>> index 77f6ebf97113..4316e36095a2 100644
>> --- a/arch/powerpc/Kconfig
>> +++ b/arch/powerpc/Kconfig
>> @@ -137,6 +137,7 @@ config PPC
>> select ARCH_HAS_STRICT_KERNEL_RWX if ((PPC_BOOK3S_64 || PPC32) && !RELOCATABLE && !HIBERNATION)
>> select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST
>> select ARCH_HAS_UACCESS_FLUSHCACHE if PPC64
>> + select ARCH_HAS_UACCESS_MCSAFE if PPC64
>> select ARCH_HAS_UBSAN_SANITIZE_ALL
>> select ARCH_HAVE_NMI_SAFE_CMPXCHG
>> select ARCH_KEEP_MEMBLOCK
>> diff --git a/arch/powerpc/include/asm/uaccess.h b/arch/powerpc/include/asm/uaccess.h
>> index 8b03eb44e876..15002b51ff18 100644
>> --- a/arch/powerpc/include/asm/uaccess.h
>> +++ b/arch/powerpc/include/asm/uaccess.h
>> @@ -387,6 +387,20 @@ static inline unsigned long raw_copy_to_user(void __user *to,
>> return ret;
>> }
>>
>> +static __always_inline unsigned long __must_check
>> +copy_to_user_mcsafe(void __user *to, const void *from, unsigned long n)
>> +{
>> + if (likely(check_copy_size(from, n, true))) {
>> + if (access_ok(to, n)) {
>> + allow_write_to_user(to, n);
>> + n = memcpy_mcsafe((void *)to, from, n);
>> + prevent_write_to_user(to, n);
>> + }
>> + }
>> +
>> + return n;
>> +}
>
> This looks OK to me.
>
> It would be nice though if copy_to_user_mcsafe() followed the pattern of
> the other copy_to_user() etc. routines where the arch code is only
> responsible for the actual arch details, and all the checks are done in
> the generic code. That would be a good cleanup to do after this has gone
> in, as the 2nd implementation of the API.

Sure, will do that.

>
> cheers