Re: [PATCH v2] powerpc: Handle MCE on POWER9 with only DSISR bit 33 set

[Michael Neuling <mikey@neuling.org>]

On Thu, 2017-09-21 at 18:18 +1000, Nicholas Piggin wrote:
> On Thu, 21 Sep 2017 12:04:34 +1000
> Michael Neuling <mikey@neuling.org> wrote:
>=20
> > On POWER9 DD2.1 and below, it's possible to get Machine Check
> > Exception (MCE) where only DSISR bit 33 is set. This will result in
> > the linux MCE handler seeing an unknown event, which triggers linux to
> > crash.
> >=20
> > We change this by detecting unknown events in the MCE handler and
> > marking them as handled so that we no longer crash. We do this only on
> > chip revisions known to have this problem.
> >=20
> > MCE that occurs like this is spurious, so we don't need to do anything
> > in terms of servicing it. If there is something that needs to be
> > serviced, the CPU will raise the MCE again with the correct DSISR so
> > that it can be serviced properly.
> >=20
> > Signed-off-by: Michael Neuling <mikey@neuling.org>
> > ---
> > v2 update commit message based on Balbir's comments
> > ---
> > =C2=A0arch/powerpc/kernel/mce_power.c | 15 +++++++++++++++
> > =C2=A01 file changed, 15 insertions(+)
> >=20
> > diff --git a/arch/powerpc/kernel/mce_power.c
> > b/arch/powerpc/kernel/mce_power.c
> > index b76ca198e0..72ec667136 100644
> > --- a/arch/powerpc/kernel/mce_power.c
> > +++ b/arch/powerpc/kernel/mce_power.c
> > @@ -595,6 +595,7 @@ static long mce_handle_error(struct pt_regs *regs,
> > =C2=A0	uint64_t addr;
> > =C2=A0	uint64_t srr1 =3D regs->msr;
> > =C2=A0	long handled;
> > +	unsigned long pvr;
> > =C2=A0
> > =C2=A0	if (SRR1_MC_LOADSTORE(srr1))
> > =C2=A0		handled =3D mce_handle_derror(regs, dtable, &mce_err, &addr);
> > @@ -604,6 +605,20 @@ static long mce_handle_error(struct pt_regs *regs,
> > =C2=A0	if (!handled && mce_err.error_type =3D=3D MCE_ERROR_TYPE_UE)
> > =C2=A0		handled =3D mce_handle_ue_error(regs);
> > =C2=A0
> > +	/*
> > +	=C2=A0* On POWER9 DD2.1 and below, it's possible to get machine
> > +	=C2=A0* check where only DSISR bit 33 is set. This will result in
> > +	=C2=A0* the MCE handler seeing an unknown event and us crashing.
> > +	=C2=A0* Change this to mark as handled on these revisions.
> > +	=C2=A0*/
> > +	pvr =3D mfspr(SPRN_PVR);
> > +	if (((PVR_VER(pvr) =3D=3D PVR_POWER9) &&
> > +	=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0(PVR_CFG(pvr) =3D=3D 2) &&
> > +	=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0(PVR_MIN(pvr) <=3D 1)) || cpu_has_featu=
re(CPU_FTR_POWER9_DD1))
> > +		/* DD2.1 and below */
> > +		if (mce_err.error_type =3D=3D MCE_ERROR_TYPE_UNKNOWN)
> > +		=C2=A0=C2=A0=C2=A0=C2=A0handled =3D 1;
>=20
> I might be missing something, but can you just do
>=20
> =C2=A0 if (regs->dsisr =3D=3D 0x40000000)
> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0return 1;
>=20
> In __machine_check_early_realmode_p9() ?

You're right, thanks.

Mikey