| Message ID | 1458012409-32448-1-git-send-email-ruscur@russell.cc (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
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
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>
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> >
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...
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.
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
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; }
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(-)