[PATCH] x86, mce: Always fall into mcelog path, regardless of what notifiers returned

[PATCH] x86, mce: Always fall into mcelog path, regardless of what notifiers returned

[Tony Luck]

This has been broken for a while - but we should not allow code
registered on the x86_mce_decoder_chain to bypass sending machine
check logs to /dev/mcelog.

Signed-off-by: Tony Luck <tony.luck@intel.com>
---

Yes, we've had this conversation before and I wimped out and didn't
push the issue. But userspace is broken. People are complaining to
me that errors don't show up in /var/log/mcelog even though they
started the mcelog(8) daemon.

 arch/x86/kernel/cpu/mcheck/mce.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/arch/x86/kernel/cpu/mcheck/mce.c b/arch/x86/kernel/cpu/mcheck/mce.c
index d2c611699cd9..f439c429c133 100644
--- a/arch/x86/kernel/cpu/mcheck/mce.c
+++ b/arch/x86/kernel/cpu/mcheck/mce.c
@@ -150,14 +150,11 @@ static struct mce_log mcelog = {
 void mce_log(struct mce *mce)
 {
 	unsigned next, entry;
-	int ret = 0;
 
 	/* Emit the trace record: */
 	trace_mce_record(mce);
 
-	ret = atomic_notifier_call_chain(&x86_mce_decoder_chain, 0, mce);
-	if (ret == NOTIFY_STOP)
-		return;
+	atomic_notifier_call_chain(&x86_mce_decoder_chain, 0, mce);
 
 	mce->finished = 0;
 	wmb();
-- 
2.1.0

Re: [PATCH] x86, mce: Always fall into mcelog path, regardless of what notifiers returned

[Borislav Petkov]

On Fri, Jan 30, 2015 at 09:29:47AM -0800, Tony Luck wrote:
> This has been broken for a while - but we should not allow code
> registered on the x86_mce_decoder_chain to bypass sending machine
> check logs to /dev/mcelog.
> 
> Signed-off-by: Tony Luck <tony.luck@intel.com>
> ---
> 
> Yes, we've had this conversation before and I wimped out and didn't
> push the issue. But userspace is broken. People are complaining to
> me that errors don't show up in /var/log/mcelog even though they
> started the mcelog(8) daemon.

We need to fix this differently - on AMD, mcelog is not needed as the
decoding is done in the kernel.

We could use mce_register_decode_chain() for callers to state what they
want.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--

[PATCHv2] x86, mce: Kernel does full decoding for AMD, others still need /dev/mcelog reports

[Tony Luck]

We should only believe a NOTIFY_STOP on an AMD system. For others
we need to provide the error record out to a user level decoder
via /dev/mcelog.

Signed-off-by: Tony Luck <tony.luck@intel.com>
---
v2: Boris says: "on AMD, mcelog is not needed"

 arch/x86/kernel/cpu/mcheck/mce.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kernel/cpu/mcheck/mce.c b/arch/x86/kernel/cpu/mcheck/mce.c
index d2c611699cd9..ea1bc1a05c07 100644
--- a/arch/x86/kernel/cpu/mcheck/mce.c
+++ b/arch/x86/kernel/cpu/mcheck/mce.c
@@ -151,12 +151,13 @@ void mce_log(struct mce *mce)
 {
 	unsigned next, entry;
 	int ret = 0;
+	struct cpuinfo_x86 *c = &boot_cpu_data;
 
 	/* Emit the trace record: */
 	trace_mce_record(mce);
 
 	ret = atomic_notifier_call_chain(&x86_mce_decoder_chain, 0, mce);
-	if (ret == NOTIFY_STOP)
+	if (c->x86_vendor == X86_VENDOR_AMD && ret == NOTIFY_STOP)
 		return;
 
 	mce->finished = 0;
-- 
2.1.0

Re: [PATCHv2] x86, mce: Kernel does full decoding for AMD, others still need /dev/mcelog reports

[Borislav Petkov]

On Fri, Jan 30, 2015 at 10:16:48AM -0800, Tony Luck wrote:
> We should only believe a NOTIFY_STOP on an AMD system. For others
> we need to provide the error record out to a user level decoder
> via /dev/mcelog.
> 
> Signed-off-by: Tony Luck <tony.luck@intel.com>
> ---
> v2: Boris says: "on AMD, mcelog is not needed"
> 
>  arch/x86/kernel/cpu/mcheck/mce.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kernel/cpu/mcheck/mce.c b/arch/x86/kernel/cpu/mcheck/mce.c
> index d2c611699cd9..ea1bc1a05c07 100644
> --- a/arch/x86/kernel/cpu/mcheck/mce.c
> +++ b/arch/x86/kernel/cpu/mcheck/mce.c
> @@ -151,12 +151,13 @@ void mce_log(struct mce *mce)
>  {
>  	unsigned next, entry;
>  	int ret = 0;
> +	struct cpuinfo_x86 *c = &boot_cpu_data;
>  
>  	/* Emit the trace record: */
>  	trace_mce_record(mce);
>  
>  	ret = atomic_notifier_call_chain(&x86_mce_decoder_chain, 0, mce);
> -	if (ret == NOTIFY_STOP)
> +	if (c->x86_vendor == X86_VENDOR_AMD && ret == NOTIFY_STOP)

Yeah, ok, we do other vendor checks in mce.c already.

Acked-by: Borislav Petkov <bp@suse.de>

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--