Message ID | 1366684776-10946-2-git-send-email-hongtao.jia@freescale.com (mailing list archive) |
---|---|
State | Superseded, archived |
Headers | show |
> * 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
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
>>> * 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
> -----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.
> 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
> -----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
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
> -----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 --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__ */