Re: [patch] 2.4.21-ia64-030702 arch/ia64/kernel/mca.c

[Bjorn Helgaas <bjorn.helgaas@hp.com>]

On Saturday 19 July 2003 12:25 am, Keith Owens wrote:
> Patches to arch/ia64/kernel/mca.c against 2.4.21-ia64-030702.
>
> Generic:
>   Issue a message to identify INIT/MCA records printed on the next boot.

ia64_log_print() already prints a note on the next boot if the log
contains saved records.  That note could be expanded, but the
logic around the new return values seems too complicated to me.

>   gp passed to SAL_VECTOR_OS_MCA should be a physical address.

I applied the bits to convert the GP passed to SAL_VECTOR_OS_MCA
to physical.  Actually, I copied the use of ia64_tpa() from 2.5, which
I believe should be safe since that's only done when mappings should
be valid.

>   Enable console logging for INIT/MCA messages.

I guess this refers to the SGI-specific addition to ia64_log_print().
I'm hoping to decrease, not increase, the error record decoding
code in the kernel.  Particularly for INIT, this is code that is
(1) platform-specific, and (2) only used after a non-recoverable
error.  It seems better to move such code to an off-line environment.

>   OEM data can be variable sized, do not use sizeof().

I don't quite understand this one.  Don't these:

> -				      (int)sizeof(sal_log_plat_specific_err_info_t) - 1,
> +				      ((char*)psei->oem_data - (char*)psei),

compute the same value?  I see that OEM data can be of variable
size (and we use "u8 oem_data[1]" as a placeholder), but isn't this
expression just computing the size of the fixed part of it?

>   Convert #ifdef CONFIG_SGI_SN2 to ia64_platform_is("sn2") to allow the
>   building of generic kernels.

I don't see any conversion -- just the addition of several ia64_platform_is()
checks, which I object to, especially when they change the behavior of
things that are not obviously platform-dependent.  For example, I think
we should come up with a strategy for breaking SAL locks and clearing INIT
logs that everybody can agree on.  Error handling is confusing enough
without making it platform-dependent.

Bjorn