[PATCH] powernv/hmi: Use the "unknown" checkstop type as a fallback

[PATCH] powernv/hmi: Use the "unknown" checkstop type as a fallback

[Russell Currey]

The HMI code knows about three types of errors: CORE, NX and UNKNOWN.
If OPAL were to add a new type, it would not be handled at all since
there is no fallback case.  Instead of explicitly checking for UNKNOWN,
treat any checkstop type without a handler as unknown.

Signed-off-by: Russell Currey <ruscur@russell.cc>
---
 arch/powerpc/platforms/powernv/opal-hmi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/platforms/powernv/opal-hmi.c b/arch/powerpc/platforms/powernv/opal-hmi.c
index d000f4e..bff4dd1 100644
--- a/arch/powerpc/platforms/powernv/opal-hmi.c
+++ b/arch/powerpc/platforms/powernv/opal-hmi.c
@@ -157,7 +157,7 @@ static void print_checkstop_reason(const char *level,
 	case CHECKSTOP_TYPE_NX:
 		print_nx_checkstop_reason(level, hmi_evt);
 		break;
-	case CHECKSTOP_TYPE_UNKNOWN:
+	default:
 		printk("%s	Unknown Malfunction Alert.\n", level);
 		break;
 	}
-- 
2.7.3

Re: [PATCH] powernv/hmi: Use the "unknown" checkstop type as a fallback

[Daniel Axtens]

Russell Currey <ruscur@russell.cc> writes:

> The HMI code knows about three types of errors: CORE, NX and UNKNOWN.
> If OPAL were to add a new type, it would not be handled at all since
> there is no fallback case.  Instead of explicitly checking for UNKNOWN,
> treat any checkstop type without a handler as unknown.
>

Just checked the surrounding function; you're quite right.
Reviewed-by: Daniel Axtens <dja@axtens.net>

> Signed-off-by: Russell Currey <ruscur@russell.cc>
> ---
>  arch/powerpc/platforms/powernv/opal-hmi.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/platforms/powernv/opal-hmi.c b/arch/powerpc/platforms/powernv/opal-hmi.c
> index d000f4e..bff4dd1 100644
> --- a/arch/powerpc/platforms/powernv/opal-hmi.c
> +++ b/arch/powerpc/platforms/powernv/opal-hmi.c
> @@ -157,7 +157,7 @@ static void print_checkstop_reason(const char *level,
>  	case CHECKSTOP_TYPE_NX:
>  		print_nx_checkstop_reason(level, hmi_evt);
>  		break;
> -	case CHECKSTOP_TYPE_UNKNOWN:
> +	default:
>  		printk("%s	Unknown Malfunction Alert.\n", level);
>  		break;
>  	}
> -- 
> 2.7.3
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH] powernv/hmi: Use the "unknown" checkstop type as a fallback

[Andrew Donnellan]

On 15/03/16 14:26, Russell Currey wrote:
> The HMI code knows about three types of errors: CORE, NX and UNKNOWN.
> If OPAL were to add a new type, it would not be handled at all since
> there is no fallback case.  Instead of explicitly checking for UNKNOWN,
> treat any checkstop type without a handler as unknown.
>
> Signed-off-by: Russell Currey <ruscur@russell.cc>

Indeed it looks like there isn't a fallback case.

Would it be useful to print xstop_type in the unknown case?

Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>

-- 
Andrew Donnellan              Software Engineer, OzLabs
andrew.donnellan@au1.ibm.com  Australia Development Lab, Canberra
+61 2 6201 8874 (work)        IBM Australia Limited

Re: [PATCH] powernv/hmi: Use the "unknown" checkstop type as a fallback

[Russell Currey]

On Tue, 2016-03-15 at 14:56 +1100, Andrew Donnellan wrote:
> On 15/03/16 14:26, Russell Currey wrote:
> > 
> > The HMI code knows about three types of errors: CORE, NX and UNKNOWN.
> > If OPAL were to add a new type, it would not be handled at all since
> > there is no fallback case.  Instead of explicitly checking for UNKNOWN,
> > treat any checkstop type without a handler as unknown.
> > 
> > Signed-off-by: Russell Currey <ruscur@russell.cc>
> Indeed it looks like there isn't a fallback case.
> 
> Would it be useful to print xstop_type in the unknown case?

I don't think so - if there's a new checkstop type, specific handling for
it should be implemented in the kernel, and if you're getting unknown
checkstops that are bringing down your machine you should be looking at the
OPAL firmware log anyway, which would contain details if there was
something "new".

> 
> Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
> 

Re: [PATCH] powernv/hmi: Use the "unknown" checkstop type as a fallback

[Andrew Donnellan]

On 15/03/16 16:15, Russell Currey wrote:
>> Would it be useful to print xstop_type in the unknown case?
>
> I don't think so - if there's a new checkstop type, specific handling for
> it should be implemented in the kernel, and if you're getting unknown
> checkstops that are bringing down your machine you should be looking at the
> OPAL firmware log anyway, which would contain details if there was
> something "new".

That's fair, would primarily help debugging in the rather corner case of 
new skiboot, old kernel, and no access to the OPAL log...

-- 
Andrew Donnellan              Software Engineer, OzLabs
andrew.donnellan@au1.ibm.com  Australia Development Lab, Canberra
+61 2 6201 8874 (work)        IBM Australia Limited

Re: [PATCH] powernv/hmi: Use the "unknown" checkstop type as a fallback

[Russell Currey]

On Tue, 2016-03-15 at 16:36 +1100, Andrew Donnellan wrote:
> On 15/03/16 16:15, Russell Currey wrote:
> > 
> > > 
> > > Would it be useful to print xstop_type in the unknown case?
> > I don't think so - if there's a new checkstop type, specific handling
> > for
> > it should be implemented in the kernel, and if you're getting unknown
> > checkstops that are bringing down your machine you should be looking at
> > the
> > OPAL firmware log anyway, which would contain details if there was
> > something "new".
> That's fair, would primarily help debugging in the rather corner case of 
> new skiboot, old kernel, and no access to the OPAL log...
> 
I think if you don't have access to the OPAL log, finding out what number
OPAL sent isn't going to help you.

Re: [PATCH] powernv/hmi: Use the "unknown" checkstop type as a fallback

[Michael Ellerman]

On Tue, 2016-03-15 at 16:15 +1100, Russell Currey wrote:
> On Tue, 2016-03-15 at 14:56 +1100, Andrew Donnellan wrote:
> > On 15/03/16 14:26, Russell Currey wrote:
> > >
> > > The HMI code knows about three types of errors: CORE, NX and UNKNOWN.
> > > If OPAL were to add a new type, it would not be handled at all since
> > > there is no fallback case.  Instead of explicitly checking for UNKNOWN,
> > > treat any checkstop type without a handler as unknown.
> > >
> > > Signed-off-by: Russell Currey <ruscur@russell.cc>
> > Indeed it looks like there isn't a fallback case.
> >
> > Would it be useful to print xstop_type in the unknown case?
>
> I don't think so - if there's a new checkstop type, specific handling for
> it should be implemented in the kernel, and if you're getting unknown
> checkstops that are bringing down your machine you should be looking at the
> OPAL firmware log anyway, which would contain details if there was
> something "new".

That's probably true.

But it's trivial to print it out, so I'd rather we did.

Putting it in a local would be nice, rather than saying
hmi_evt->u.xstop_error.xstop_type twice.

cheers