diff mbox

[2/2,V7] powerpc/85xx: Add machine check handler to fix PCIe erratum on mpc85xx

Message ID 1366684776-10946-2-git-send-email-hongtao.jia@freescale.com (mailing list archive)
State Superseded
Headers show

Commit Message

Hongtao Jia April 23, 2013, 2:39 a.m. UTC 
A PCIe erratum of mpc85xx may causes a core hang when a link of PCIe
goes down. when the link goes down, Non-posted transactions issued
via the ATMU requiring completion result in an instruction stall.
At the same time a machine-check exception is generated to the core
to allow further processing by the handler. We implements the handler
which skips the instruction caused the stall.

This patch depends on patch:
powerpc/85xx: Add platform_device declaration to fsl_pci.h

Signed-off-by: Zhao Chenhui <b35336@freescale.com>
Signed-off-by: Li Yang <leoli@freescale.com>
Signed-off-by: Liu Shuo <soniccat.liu@gmail.com>
Signed-off-by: Jia Hongtao <hongtao.jia@freescale.com>
---
V6:
* Correct PCIe checking method (Using indirect_type member of pci_controller
  stucture).

V5:
* Move OP and XOP defines to a new header file: asm/ppc-disassemble.h
* Add X UX BRX variant of load instruction emulation
* Remove A variant of load instruction emulation

V4:
* Fill rd with all-Fs if the skipped instruction is load and emulate the
  instruction.
* Let KVM/QEMU deal with the exception if the machine check comes from KVM.

 arch/powerpc/kernel/cpu_setup_fsl_booke.S |   2 +-
 arch/powerpc/kernel/traps.c               |   3 +
 arch/powerpc/sysdev/fsl_pci.c             | 140 ++++++++++++++++++++++++++++++
 arch/powerpc/sysdev/fsl_pci.h             |   6 ++
 4 files changed, 150 insertions(+), 1 deletion(-)

Comments

Segher Boessenkool April 25, 2013, 3:31 p.m. UTC | #1 
> * Remove A variant of load instruction emulation

Why is this?  You handle all other simple load insns, there is
nothing special about LHA.  (I reviewed the V4 email thread, no
reason for the chance is given there).


Segher
Scott Wood April 25, 2013, 4:58 p.m. UTC | #2 
On 04/25/2013 10:31:51 AM, Segher Boessenkool wrote:
>> * Remove A variant of load instruction emulation
> 
> Why is this?  You handle all other simple load insns, there is
> nothing special about LHA.  (I reviewed the V4 email thread, no
> reason for the chance is given there).

The LHA implementation in V5 was incorrect (didn't sign-extend).

-Scott
Segher Boessenkool April 25, 2013, 5:32 p.m. UTC | #3 
>>> * Remove A variant of load instruction emulation
>> Why is this?  You handle all other simple load insns, there is
>> nothing special about LHA.  (I reviewed the V4 email thread, no
>> reason for the chance is given there).
>
> The LHA implementation in V5 was incorrect (didn't sign-extend).

The history on the current version says that it was _removed_ in V5 :-)
So an off-by-one in the history, I suppose.

It would be totally trivial to implement it correctly, seems a bit
silly to remove it?


Segher
Hongtao Jia April 27, 2013, 2:26 a.m. UTC | #4 
> -----Original Message-----
> From: Wood Scott-B07421
> Sent: Friday, April 26, 2013 12:58 AM
> To: Segher Boessenkool
> Cc: Jia Hongtao-B38951; linuxppc-dev@lists.ozlabs.org;
> galak@kernel.crashing.org; Wood Scott-B07421
> Subject: Re: [PATCH 2/2 V7] powerpc/85xx: Add machine check handler to
> fix PCIe erratum on mpc85xx
> 
> On 04/25/2013 10:31:51 AM, Segher Boessenkool wrote:
> >> * Remove A variant of load instruction emulation
> >
> > Why is this?  You handle all other simple load insns, there is nothing
> > special about LHA.  (I reviewed the V4 email thread, no reason for the
> > chance is given there).
> 
> The LHA implementation in V5 was incorrect (didn't sign-extend).
> 
> -Scott

In former email you doubt whether we need A variant or not.
Any particular reason for that?
If not should I emulate all the A ARX AU AUX and AX variant?

Thanks.
-Hongtao.
Segher Boessenkool April 27, 2013, 1:31 p.m. UTC | #5 
> In former email you doubt whether we need A variant or not.
> Any particular reason for that?
> If not should I emulate all the A ARX AU AUX and AX variant?

A/AU/AX/AUX are just normal loads, sign-extended instead of
zero-extended (so assign -1 to the register loaded).

The ARX thing is load-locked, you do not want that one.


Segher
Hongtao Jia April 28, 2013, 2:20 a.m. UTC | #6 
> -----Original Message-----
> From: Segher Boessenkool [mailto:segher@kernel.crashing.org]
> Sent: Saturday, April 27, 2013 9:32 PM
> To: Jia Hongtao-B38951
> Cc: Wood Scott-B07421; linuxppc-dev@lists.ozlabs.org;
> galak@kernel.crashing.org
> Subject: Re: [PATCH 2/2 V7] powerpc/85xx: Add machine check handler to
> fix PCIe erratum on mpc85xx
> 
> > In former email you doubt whether we need A variant or not.
> > Any particular reason for that?
> > If not should I emulate all the A ARX AU AUX and AX variant?
> 
> A/AU/AX/AUX are just normal loads, sign-extended instead of zero-extended
> (so assign -1 to the register loaded).
> 
> The ARX thing is load-locked, you do not want that one.
> 
> 
> Segher

Thanks, very helpful.
-Hongtao
Scott Wood April 29, 2013, 8:29 p.m. UTC | #7 
On 04/26/2013 09:26:26 PM, Jia Hongtao-B38951 wrote:
> > -----Original Message-----
> > From: Wood Scott-B07421
> > Sent: Friday, April 26, 2013 12:58 AM
> > To: Segher Boessenkool
> > Cc: Jia Hongtao-B38951; linuxppc-dev@lists.ozlabs.org;
> > galak@kernel.crashing.org; Wood Scott-B07421
> > Subject: Re: [PATCH 2/2 V7] powerpc/85xx: Add machine check handler  
> to
> > fix PCIe erratum on mpc85xx
> >
> > On 04/25/2013 10:31:51 AM, Segher Boessenkool wrote:
> > >> * Remove A variant of load instruction emulation
> > >
> > > Why is this?  You handle all other simple load insns, there is  
> nothing
> > > special about LHA.  (I reviewed the V4 email thread, no reason  
> for the
> > > chance is given there).
> >
> > The LHA implementation in V5 was incorrect (didn't sign-extend).
> >
> > -Scott
> 
> In former email you doubt whether we need A variant or not.
> Any particular reason for that?
> If not should I emulate all the A ARX AU AUX and AX variant?

I was just noting that the variants you left out from the earlier  
revisions (e.g. BRX) were much more likely to be used for I/O than some  
of the ones you included (e.g. "A").  Implementing all the normal  
load/store instructions would be better, if they're done correctly.

-Scott
Hongtao Jia May 2, 2013, 7 a.m. UTC | #8 
> -----Original Message-----
> From: Wood Scott-B07421
> Sent: Tuesday, April 30, 2013 4:30 AM
> To: Jia Hongtao-B38951
> Cc: Wood Scott-B07421; Segher Boessenkool; linuxppc-dev@lists.ozlabs.org;
> galak@kernel.crashing.org
> Subject: Re: [PATCH 2/2 V7] powerpc/85xx: Add machine check handler to
> fix PCIe erratum on mpc85xx
> 
> On 04/26/2013 09:26:26 PM, Jia Hongtao-B38951 wrote:
> > > -----Original Message-----
> > > From: Wood Scott-B07421
> > > Sent: Friday, April 26, 2013 12:58 AM
> > > To: Segher Boessenkool
> > > Cc: Jia Hongtao-B38951; linuxppc-dev@lists.ozlabs.org;
> > > galak@kernel.crashing.org; Wood Scott-B07421
> > > Subject: Re: [PATCH 2/2 V7] powerpc/85xx: Add machine check handler
> > to
> > > fix PCIe erratum on mpc85xx
> > >
> > > On 04/25/2013 10:31:51 AM, Segher Boessenkool wrote:
> > > >> * Remove A variant of load instruction emulation
> > > >
> > > > Why is this?  You handle all other simple load insns, there is
> > nothing
> > > > special about LHA.  (I reviewed the V4 email thread, no reason
> > for the
> > > > chance is given there).
> > >
> > > The LHA implementation in V5 was incorrect (didn't sign-extend).
> > >
> > > -Scott
> >
> > In former email you doubt whether we need A variant or not.
> > Any particular reason for that?
> > If not should I emulate all the A ARX AU AUX and AX variant?
> 
> I was just noting that the variants you left out from the earlier
> revisions (e.g. BRX) were much more likely to be used for I/O than some
> of the ones you included (e.g. "A").  Implementing all the normal
> load/store instructions would be better, if they're done correctly.
> 
> -Scott

All right.
I have submitted a new version and please have a review.
http://patchwork.ozlabs.org/patch/240238/
http://patchwork.ozlabs.org/patch/240239/

Thanks.
-Hongtao
diff mbox

Patch

diff --git a/arch/powerpc/kernel/cpu_setup_fsl_booke.S b/arch/powerpc/kernel/cpu_setup_fsl_booke.S
index 0b9af01..bfb18c7 100644
--- a/arch/powerpc/kernel/cpu_setup_fsl_booke.S
+++ b/arch/powerpc/kernel/cpu_setup_fsl_booke.S
@@ -75,7 +75,7 @@  _GLOBAL(__setup_cpu_e500v2)
 	bl	__e500_icache_setup
 	bl	__e500_dcache_setup
 	bl	__setup_e500_ivors
-#ifdef CONFIG_FSL_RIO
+#if defined(CONFIG_FSL_RIO) || defined(CONFIG_FSL_PCI)
 	/* Ensure that RFXE is set */
 	mfspr	r3,SPRN_HID1
 	oris	r3,r3,HID1_RFXE@h
diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
index 37cc40e..d15cfb5 100644
--- a/arch/powerpc/kernel/traps.c
+++ b/arch/powerpc/kernel/traps.c
@@ -60,6 +60,7 @@ 
 #include <asm/switch_to.h>
 #include <asm/tm.h>
 #include <asm/debug.h>
+#include <sysdev/fsl_pci.h>
 
 #if defined(CONFIG_DEBUGGER) || defined(CONFIG_KEXEC)
 int (*__debugger)(struct pt_regs *regs) __read_mostly;
@@ -565,6 +566,8 @@  int machine_check_e500(struct pt_regs *regs)
 	if (reason & MCSR_BUS_RBERR) {
 		if (fsl_rio_mcheck_exception(regs))
 			return 1;
+		if (fsl_pci_mcheck_exception(regs))
+			return 1;
 	}
 
 	printk("Machine check in kernel mode.\n");
diff --git a/arch/powerpc/sysdev/fsl_pci.c b/arch/powerpc/sysdev/fsl_pci.c
index 40ffe29..6bddf0f 100644
--- a/arch/powerpc/sysdev/fsl_pci.c
+++ b/arch/powerpc/sysdev/fsl_pci.c
@@ -26,11 +26,15 @@ 
 #include <linux/memblock.h>
 #include <linux/log2.h>
 #include <linux/slab.h>
+#include <linux/uaccess.h>
 
 #include <asm/io.h>
 #include <asm/prom.h>
 #include <asm/pci-bridge.h>
+#include <asm/ppc-pci.h>
 #include <asm/machdep.h>
+#include <asm/disassemble.h>
+#include <asm/ppc-opcode.h>
 #include <sysdev/fsl_soc.h>
 #include <sysdev/fsl_pci.h>
 
@@ -876,6 +880,142 @@  u64 fsl_pci_immrbar_base(struct pci_controller *hose)
 	return 0;
 }
 
+#ifdef CONFIG_E500
+static int mcheck_handle_load(struct pt_regs *regs, u32 inst)
+{
+	unsigned int rd, ra, rb, d;
+
+	rd = get_rt(inst);
+	ra = get_ra(inst);
+	rb = get_rb(inst);
+	d = get_d(inst);
+
+	switch (get_op(inst)) {
+	case 31:
+		switch (get_xop(inst)) {
+		case OP_31_XOP_LWZX:
+		case OP_31_XOP_LWBRX:
+			regs->gpr[rd] = 0xffffffff;
+			break;
+
+		case OP_31_XOP_LWZUX:
+			regs->gpr[rd] = 0xffffffff;
+			regs->gpr[ra] += regs->gpr[rb];
+			break;
+
+		case OP_31_XOP_LBZX:
+			regs->gpr[rd] = 0xff;
+			break;
+
+		case OP_31_XOP_LBZUX:
+			regs->gpr[rd] = 0xff;
+			regs->gpr[ra] += regs->gpr[rb];
+			break;
+
+		case OP_31_XOP_LHZX:
+		case OP_31_XOP_LHBRX:
+			regs->gpr[rd] = 0xffff;
+			break;
+
+		case OP_31_XOP_LHZUX:
+			regs->gpr[rd] = 0xffff;
+			regs->gpr[ra] += regs->gpr[rb];
+			break;
+
+		default:
+			return 0;
+		}
+		break;
+
+	case OP_LWZ:
+		regs->gpr[rd] = 0xffffffff;
+		break;
+
+	case OP_LWZU:
+		regs->gpr[rd] = 0xffffffff;
+		regs->gpr[ra] += (s16)d;
+		break;
+
+	case OP_LBZ:
+		regs->gpr[rd] = 0xff;
+		break;
+
+	case OP_LBZU:
+		regs->gpr[rd] = 0xff;
+		regs->gpr[ra] += (s16)d;
+		break;
+
+	case OP_LHZ:
+		regs->gpr[rd] = 0xffff;
+		break;
+
+	case OP_LHZU:
+		regs->gpr[rd] = 0xffff;
+		regs->gpr[ra] += (s16)d;
+		break;
+
+	default:
+		return 0;
+	}
+
+	return 1;
+}
+
+static int is_in_pci_mem_space(phys_addr_t addr)
+{
+	struct pci_controller *hose;
+	struct resource *res;
+	int i;
+
+	list_for_each_entry(hose, &hose_list, list_node) {
+		if (!(hose->indirect_type & PPC_INDIRECT_TYPE_EXT_REG))
+			continue;
+
+		for (i = 0; i < 3; i++) {
+			res = &hose->mem_resources[i];
+			if ((res->flags & IORESOURCE_MEM) &&
+				addr >= res->start && addr <= res->end)
+				return 1;
+		}
+	}
+	return 0;
+}
+
+int fsl_pci_mcheck_exception(struct pt_regs *regs)
+{
+	u32 inst;
+	int ret;
+	phys_addr_t addr = 0;
+
+	/* Let KVM/QEMU deal with the exception */
+	if (regs->msr & MSR_GS)
+		return 0;
+
+#ifdef CONFIG_PHYS_64BIT
+	addr = mfspr(SPRN_MCARU);
+	addr <<= 32;
+#endif
+	addr += mfspr(SPRN_MCAR);
+
+	if (is_in_pci_mem_space(addr)) {
+		if (user_mode(regs)) {
+			pagefault_disable();
+			ret = get_user(regs->nip, &inst);
+			pagefault_enable();
+		} else {
+			ret = probe_kernel_address(regs->nip, inst);
+		}
+
+		if (mcheck_handle_load(regs, inst)) {
+			regs->nip += 4;
+			return 1;
+		}
+	}
+
+	return 0;
+}
+#endif
+
 #if defined(CONFIG_FSL_SOC_BOOKE) || defined(CONFIG_PPC_86xx)
 static const struct of_device_id pci_ids[] = {
 	{ .compatible = "fsl,mpc8540-pci", },
diff --git a/arch/powerpc/sysdev/fsl_pci.h b/arch/powerpc/sysdev/fsl_pci.h
index 72b5625..defc422 100644
--- a/arch/powerpc/sysdev/fsl_pci.h
+++ b/arch/powerpc/sysdev/fsl_pci.h
@@ -126,5 +126,11 @@  static inline int mpc85xx_pci_err_probe(struct platform_device *op)
 }
 #endif
 
+#ifdef CONFIG_FSL_PCI
+extern int fsl_pci_mcheck_exception(struct pt_regs *);
+#else
+static inline int fsl_pci_mcheck_exception(struct pt_regs *regs) {return 0; }
+#endif
+
 #endif /* __POWERPC_FSL_PCI_H */
 #endif /* __KERNEL__ */