[PATCH V5] powerpc/85xx: Add machine check handler to fix PCIe erratum on mpc85xx

[Jia Hongtao-B38951]

> -----Original Message-----
> From: Wood Scott-B07421
> Sent: Thursday, April 11, 2013 5:52 AM
> To: Jia Hongtao-B38951
> Cc: linuxppc-dev at lists.ozlabs.org; galak at kernel.crashing.org; Wood Scott-
> B07421; Li Yang-R58472; Jia Hongtao-B38951
> Subject: Re: [PATCH V5] powerpc/85xx: Add machine check handler to fix
> PCIe erratum on mpc85xx
> 
> On 04/08/2013 03:26:54 AM, Jia Hongtao wrote:
> > @@ -826,6 +829,124 @@ u64 fsl_pci_immrbar_base(struct pci_controller
> > *hose)
> >  	return 0;
> >  }
> >
> > +#ifdef CONFIG_E500
> > +
> > +#define OP_LWZ  32
> > +#define OP_LWZU 33
> > +#define OP_LBZ  34
> > +#define OP_LBZU 35
> > +#define OP_LHZ  40
> > +#define OP_LHZU 41
> > +#define OP_LHA  42
> > +#define OP_LHAU 43
> 
> Can you move these to asm/ppc-opcode.h (or possibly
> asm/ppc-disassemble.h if we don't want to mix the two methods of
> describing instructions)?

Yes, mix the two methods is not appropriate.
asm/ppc-disassemble.h is a nice choice.

> 
> > +static int mcheck_handle_load(struct pt_regs *regs, u32 inst)
> > +{
> > +	unsigned int rd, ra, d;
> > +
> > +	rd = get_rt(inst);
> > +	ra = get_ra(inst);
> > +	d = get_d(inst);
> > +
> > +	switch (get_op(inst)) {
> > +	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;
> > +
> > +	case OP_LHA:
> > +		regs->gpr[rd] = 0xffff;
> > +		break;
> > +
> > +	case OP_LHAU:
> > +		regs->gpr[rd] = 0xffff;
> > +		regs->gpr[ra] += (s16)d;
> > +		break;
> 
> The X and (especially for PCI) BRX versions are important -- probably
> more so than the U versions.  I doubt we need the A variant.

Then I will add X and BRX variant and remove A variant.

> 
> If you do support the A variant, why are you not sign-extending the
> value?

Just curious, sign-extending the value means fill rd with 0xffffffff?

> 
> Is this erratum present on any 64-bit chips?

No. This erratum only happened in e500 core chips.

> 
> -Scott

Thanks.
-Hongtao