Re: [PATCH 3/8] ACPI, x86: Extended error log driver for x86 platform

[Chen Gong]

On Mon, Oct 14, 2013 at 12:26:35PM +0200, Borislav Petkov wrote:
> Date: Mon, 14 Oct 2013 12:26:35 +0200
> From: Borislav Petkov <bp@xxxxxxxxx>
> To: Chen Gong <gong.chen@xxxxxxxxxxxxxxx>
> Cc: tony.luck@xxxxxxxxx, linux-kernel@xxxxxxxxxxxxxxx,
>  linux-acpi@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH 3/8] ACPI, x86: Extended error log driver for x86
>  platform
> User-Agent: Mutt/1.5.21 (2010-09-15)
> 
> On Sun, Oct 13, 2013 at 11:16:33PM -0400, Chen Gong wrote:
> > But this driver can be loaded as a module. If this module is unloaded,
> > extlog_print is gone. I can't keep such a pointer internally.
> 
> Sure you can - you define a weak extlog_print() function in a
> compilation unit which is always builtin. Maybe mce.c or so.
> 
Oh, yes. Let me do it.

> > This macro is great and I'd loved to use it. But it looks like a
> > litttle bit weird to let eMCA depends on a header file like edac.h.
> > Meanwhile, I found in drivers/video/sis/init.c:3323 we have a very
> > similar macro for this purpose. So how about writing a separate patch
> > to clean it up first?
> 
> Actually, you're right. Those macros are much more generic and
> could be exposed to the general public by putting them, say into
> include/include/bitops.h, for example?
> 
> Btw, the sis one generates unsigneds (4 byte on x86) while the edac one
> 8 byte ULLs. So you could call them
> 
> GENMASK
> and
> GENMASK_ULL
> 
> How does that sound?

This kind of mask is often unsigned. So how about following mode:

GENMASKL / GENMASKQ
or
GENMASK_L / GENMASK_Q

> 
> > Because I think in theory "CPU < 0" is impossible. When it hits such
> > situation, it should be a very serious H/W or firmware bug. At least,
> > It think it should be a WARN_ON.
> 
> Yes, I think a WARN_ON is much better than the heavy hammer. We can
> always turn it into a FW_BUG later if it really starts to trigger
> anywhere...
> 
Agree.

> Thanks.
> 
> -- 
> Regards/Gruss,
>     Boris.
> 
> Sent from a fat crate under my desk. Formatting is fine.
> --

Attachment: signature.asc
Description: Digital signature