>> +int mce_severity(struct mce *m, int tolerant, char **msg, bool is_excp)
>
> You're adding a function argument which is carrying redundant info which
> is already present in *m...
>
>> {
>> + enum exception excp = (is_excp ? EXCP_CONTEXT : NO_EXCP);
>
> ... and so this should be:
>
> excp = ((m->mcg_status & MCG_STATUS_MCIP) ? EXCP_CONTEXT : NO_EXCP);
That only works if you trust that MCG_STATUS.MCIP is correctly set to indicate
whether
we are in MCE or CMCI context. The current code doesn't do that - we check for,
and flag
it as a fatal error if we find ourselves in the MCE handler with MCIP==0. If
you add the
code you suggest, then it completely neuters the severity check:
MCESEV(
PANIC, "MCIP not set in MCA handler",
MCGMASK(MCG_STATUS_MCIP, 0)
),
I'm also a bit worried about the check for DEFERRED errors in
the severity table. That isn't conditional on an:
if (intel) do_onething(); else /*amd/ do_anotherthing();
So if we can misinterpret some bits on an Intel cpu as if
we had a deferred error.
Overall, this might have seemed like a good idea to begin with,
but we are piling more complexity into mce_severity() [a routine
which everyone agrees is already tough to understand].
It doesn't even buy us some simple code in the polling path.
We still have to do more checks on MCi_STATUS.MCACOD above
and beyond what we get back from mce_severity()
Boris: Do you still want to keep pushing this way? Or should
we look back fondly at version 1 of this patch?
-Tony
