Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1513248 > unrolled thread

[RFC PATCH 0/3] x86/RAS: Dump error record to dmesg if no consumers

Back to article view | Back to linux.kernel

Contents

  [RFC PATCH 0/3] x86/RAS: Dump error record to dmesg if no consumers Borislav Petkov <bp@alien8.de> - 2016-11-01 13:20 +0100
    [PATCH] x86/MCE: Remove MCP_TIMESTAMP Borislav Petkov <bp@alien8.de> - 2016-11-05 14:20 +0100
      RE: [PATCH] x86/MCE: Remove MCP_TIMESTAMP "Luck, Tony" <tony.luck@intel.com> - 2016-11-07 18:50 +0100
        Re: [PATCH] x86/MCE: Remove MCP_TIMESTAMP Borislav Petkov <bp@alien8.de> - 2016-11-07 19:10 +0100
          RE: [PATCH] x86/MCE: Remove MCP_TIMESTAMP "Luck, Tony" <tony.luck@intel.com> - 2016-11-07 19:40 +0100
            Re: [PATCH] x86/MCE: Remove MCP_TIMESTAMP Borislav Petkov <bp@alien8.de> - 2016-11-08 19:10 +0100
              RE: [PATCH] x86/MCE: Remove MCP_TIMESTAMP "Luck, Tony" <tony.luck@intel.com> - 2016-11-08 19:30 +0100
              Re: [PATCH] x86/MCE: Remove MCP_TIMESTAMP Thomas Gleixner <tglx@linutronix.de> - 2016-11-08 21:50 +0100
                Re: [PATCH] x86/MCE: Remove MCP_TIMESTAMP Borislav Petkov <bp@alien8.de> - 2016-11-08 22:10 +0100
                  Re: [PATCH] x86/MCE: Remove MCP_TIMESTAMP Thomas Gleixner <tglx@linutronix.de> - 2016-11-08 22:20 +0100
                    Re: [PATCH] x86/MCE: Remove MCP_TIMESTAMP Borislav Petkov <bp@alien8.de> - 2016-11-08 22:30 +0100
                      Re: [PATCH] x86/MCE: Remove MCP_TIMESTAMP Thomas Gleixner <tglx@linutronix.de> - 2016-11-08 23:00 +0100
                        Re: [PATCH] x86/MCE: Remove MCP_TIMESTAMP Borislav Petkov <bp@alien8.de> - 2016-11-09 19:10 +0100
    Re: [RFC PATCH 0/3] x86/RAS: Dump error record to dmesg if no  consumers Ingo Molnar <mingo@kernel.org> - 2016-11-07 08:40 +0100

#1513248 — [RFC PATCH 0/3] x86/RAS: Dump error record to dmesg if no consumers

From: Borislav Petkov <bp@suse.de>

Right,

so this is not a good thing: systems may not have any error record
consumers registered and in such cases, any logged MCEs disappear into
the void. And this shouldn't happen.

So let's dump them to dmesg as a last resort.

Borislav Petkov (3):
  notifiers: Document notifier priority
  x86/RAS: Add TSC to the injected MCE
  x86/MCE: Dump MCE to dmesg if no consumers

 arch/x86/kernel/cpu/mcheck/mce.c | 52 +++++++++++++++++++++++++++++++++++-----
 arch/x86/ras/mce_amd_inj.c       |  2 ++
 include/linux/notifier.h         |  1 +
 3 files changed, 49 insertions(+), 6 deletions(-)

-- 
2.10.0

[toc] | [next] | [standalone]

#1515543 — [PATCH] x86/MCE: Remove MCP_TIMESTAMP

Whoops,

one more:

---
From: Borislav Petkov <bp@suse.de>
Date: Sat, 5 Nov 2016 12:47:03 +0100
Subject: [PATCH] x86/MCE: Remove MCP_TIMESTAMP

MCP_TIMESTAMP controls whether current TSC value should be added to
the MCE record. Most of machine_check_poll() callers supply it, except
__mcheck_cpu_init_generic() but this is wrong because we could be
logging an MCE right at the same time and thus log one without the TSC
value.

What is more, machine_check_poll() did unconditionally clear mce.tsc
which is another bug.

So, get rid of all that and simply log an MCE with a TSC value always.
Simplifies the code a bit too.

Signed-off-by: Borislav Petkov <bp@suse.de>
Cc: Tony Luck <tony.luck@intel.com>
---
 arch/x86/include/asm/mce.h             | 5 ++---
 arch/x86/kernel/cpu/mcheck/mce.c       | 6 +-----
 arch/x86/kernel/cpu/mcheck/mce_intel.c | 6 +++---
 3 files changed, 6 insertions(+), 11 deletions(-)

diff --git a/arch/x86/include/asm/mce.h b/arch/x86/include/asm/mce.h
index 748b8da8e627..3b53c260e0be 100644
--- a/arch/x86/include/asm/mce.h
+++ b/arch/x86/include/asm/mce.h
@@ -265,9 +265,8 @@ typedef DECLARE_BITMAP(mce_banks_t, MAX_NR_BANKS);
 DECLARE_PER_CPU(mce_banks_t, mce_poll_banks);
 
 enum mcp_flags {
-	MCP_TIMESTAMP	= BIT(0),	/* log time stamp */
-	MCP_UC		= BIT(1),	/* log uncorrected errors */
-	MCP_DONTLOG	= BIT(2),	/* only clear, don't log */
+	MCP_UC		= BIT(0),	/* log uncorrected errors */
+	MCP_DONTLOG	= BIT(1),	/* only clear, don't log */
 };
 bool machine_check_poll(enum mcp_flags flags, mce_banks_t *b);
 
diff --git a/arch/x86/kernel/cpu/mcheck/mce.c b/arch/x86/kernel/cpu/mcheck/mce.c
index 4ca00474804b..82564156d6ab 100644
--- a/arch/x86/kernel/cpu/mcheck/mce.c
+++ b/arch/x86/kernel/cpu/mcheck/mce.c
@@ -713,7 +713,6 @@ bool machine_check_poll(enum mcp_flags flags, mce_banks_t *b)
 		m.misc = 0;
 		m.addr = 0;
 		m.bank = i;
-		m.tsc = 0;
 
 		barrier();
 		m.status = mce_rdmsrl(msr_ops.status(i));
@@ -735,9 +734,6 @@ bool machine_check_poll(enum mcp_flags flags, mce_banks_t *b)
 
 		mce_read_aux(&m, i);
 
-		if (!(flags & MCP_TIMESTAMP))
-			m.tsc = 0;
-
 		severity = mce_severity(&m, mca_cfg.tolerant, NULL, false);
 
 		if (severity == MCE_DEFERRED_SEVERITY && memory_error(&m))
@@ -1394,7 +1390,7 @@ static void mce_timer_fn(unsigned long data)
 	iv = __this_cpu_read(mce_next_interval);
 
 	if (mce_available(this_cpu_ptr(&cpu_info))) {
-		machine_check_poll(MCP_TIMESTAMP, this_cpu_ptr(&mce_poll_banks));
+		machine_check_poll(0, this_cpu_ptr(&mce_poll_banks));
 
 		if (mce_intel_cmci_poll()) {
 			iv = mce_adjust_timer(iv);
diff --git a/arch/x86/kernel/cpu/mcheck/mce_intel.c b/arch/x86/kernel/cpu/mcheck/mce_intel.c
index 1defb8ea882c..3262f0d726bb 100644
--- a/arch/x86/kernel/cpu/mcheck/mce_intel.c
+++ b/arch/x86/kernel/cpu/mcheck/mce_intel.c
@@ -130,7 +130,7 @@ bool mce_intel_cmci_poll(void)
 	 * Reset the counter if we've logged an error in the last poll
 	 * during the storm.
 	 */
-	if (machine_check_poll(MCP_TIMESTAMP, this_cpu_ptr(&mce_banks_owned)))
+	if (machine_check_poll(0, this_cpu_ptr(&mce_banks_owned)))
 		this_cpu_write(cmci_backoff_cnt, INITIAL_CHECK_INTERVAL);
 	else
 		this_cpu_dec(cmci_backoff_cnt);
@@ -250,7 +250,7 @@ static void intel_threshold_interrupt(void)
 	if (cmci_storm_detect())
 		return;
 
-	machine_check_poll(MCP_TIMESTAMP, this_cpu_ptr(&mce_banks_owned));
+	machine_check_poll(0, this_cpu_ptr(&mce_banks_owned));
 }
 
 /*
@@ -342,7 +342,7 @@ void cmci_recheck(void)
 		return;
 
 	local_irq_save(flags);
-	machine_check_poll(MCP_TIMESTAMP, this_cpu_ptr(&mce_banks_owned));
+	machine_check_poll(0, this_cpu_ptr(&mce_banks_owned));
 	local_irq_restore(flags);
 }
 
-- 
2.10.0

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

[toc] | [prev] | [next] | [standalone]

#1516382 — RE: [PATCH] x86/MCE: Remove MCP_TIMESTAMP

> So, get rid of all that and simply log an MCE with a TSC value always.
> Simplifies the code a bit too.

I'm not necessarily opposed to this ... but there was once some logic behind when
logged TSC, and when we didn't.  Essentially we wanted the TSC when we were
logging from #CMCI or #MC .... because the detection of the error was fresh, and
wanted as much precision on the logged time as possible to compare with logged
errors from other banks/cpus. This might allow us to distinguish multiple errors logged
in the same #CMCI, from errors logged in separate #CMCI a tenth of a second apart.

If we found the error while polling, we didn’t want to provide a false sense of precision.
The error could have been logged up to five minutes previously (or when logging
errors during the initial poll of the banks an arbitrary time in the past).

-Tony

[toc] | [prev] | [next] | [standalone]

#1516409 — Re: [PATCH] x86/MCE: Remove MCP_TIMESTAMP

On Mon, Nov 07, 2016 at 05:48:46PM +0000, Luck, Tony wrote:
> > So, get rid of all that and simply log an MCE with a TSC value always.
> > Simplifies the code a bit too.
> 
> I'm not necessarily opposed to this ... but there was once some logic behind when
> logged TSC, and when we didn't.  Essentially we wanted the TSC when we were
> logging from #CMCI or #MC .... because the detection of the error was fresh, and
> wanted as much precision on the logged time as possible to compare with logged
> errors from other banks/cpus. This might allow us to distinguish multiple errors logged
> in the same #CMCI, from errors logged in separate #CMCI a tenth of a second apart.
> 
> If we found the error while polling, we didn’t want to provide a false sense of precision.
> The error could have been logged up to five minutes previously (or when logging
> errors during the initial poll of the banks an arbitrary time in the past).

Right, looks like we've lost that logic:

Functions calling this function: machine_check_poll

  File         Function                  Line
0 mce-inject.c raise_poll                  57 machine_check_poll(0, &b);
1 mce.c        mce_timer_fn              1358 machine_check_poll(MCP_TIMESTAMP, this_cpu_ptr(&mce_poll_banks));
2 mce.c        __mcheck_cpu_init_generic 1508 machine_check_poll(MCP_UC | m_fl, &all_banks);
3 mce_intel.c  mce_intel_cmci_poll        133 if (machine_check_poll(MCP_TIMESTAMP, this_cpu_ptr(&mce_banks_owned)))
4 mce_intel.c  intel_threshold_interrupt  253 machine_check_poll(MCP_TIMESTAMP, this_cpu_ptr(&mce_banks_owned));
5 mce_intel.c  cmci_recheck               345 machine_check_poll(MCP_TIMESTAMP, this_cpu_ptr(&mce_banks_owned));

So the TSC timestamp will be possibly inexact now in mce_timer_fn(),
__mcheck_cpu_init_generic(), mce_intel_cmci_poll() and cmci_recheck().

Should we bother and add a flag to struct mce - maybe somewhere in the
padding __u8 pad; - to denote that the logged TSC may not be exact?

Mind you, there's also

	m->time = get_seconds();

which also collects time and which could also be possibly inexact.

One other possibility would be to use ->time and write ->tsc *only*
when exact - i.e., in the handler - and this is then enough info about
timing.

->time will give you somewhere around where it happened and ->tsc - only
if set - will give you exact, well, *timestamp* :)

This sounds like a pretty straightforward logic to me...

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

[toc] | [prev] | [next] | [standalone]

#1516450 — RE: [PATCH] x86/MCE: Remove MCP_TIMESTAMP

> One other possibility would be to use ->time and write ->tsc *only*
> when exact - i.e., in the handler - and this is then enough info about
> timing.
>
> ->time will give you somewhere around where it happened and ->tsc - only
> if set - will give you exact, well, *timestamp* :)
>
> This sounds like a pretty straightforward logic to me...

Also to me ... and I think that's what used to happen (or at least was the
intent).

-Tony

[toc] | [prev] | [next] | [standalone]

#1517443 — Re: [PATCH] x86/MCE: Remove MCP_TIMESTAMP

On Mon, Nov 07, 2016 at 06:37:50PM +0000, Luck, Tony wrote:
> Also to me ... and I think that's what used to happen (or at least was the
> intent).

How's that?

This still preserves the precise TSC timestamp in intel_threshold_interrupt().

---
From: Borislav Petkov <bp@suse.de>
Date: Tue, 8 Nov 2016 16:20:05 +0100
Subject: [PATCH] x86/MCE: Correct TSC timestamping of error records

We did have logic in the MCE code which would TSC-timestamp an error
record only when it is exact - i.e., it wasn't detected by polling. This
isn't the case anymore. So let's fix that:

We have a TSC timestamp in the error record only when it has been a
precise detection, i.e., either in the #MC handler or in one of the
interrupt handlers (thresholding, deferred, ...).

All other error records still have mce.time which contains the wall time
in order to be able to place the error record in time approximately.

Also, this fixes another bug where machine_check_poll() would clear
mce.tsc unconditionally even if we requested precise MCP_TIMESTAMP
logging.

Signed-off-by: Borislav Petkov <bp@suse.de>
Cc: Tony Luck <tony.luck@intel.com>
---
 arch/x86/kernel/cpu/mcheck/mce.c       | 3 +--
 arch/x86/kernel/cpu/mcheck/mce_intel.c | 4 ++--
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kernel/cpu/mcheck/mce.c b/arch/x86/kernel/cpu/mcheck/mce.c
index 4ca00474804b..b7a976d657f3 100644
--- a/arch/x86/kernel/cpu/mcheck/mce.c
+++ b/arch/x86/kernel/cpu/mcheck/mce.c
@@ -713,7 +713,6 @@ bool machine_check_poll(enum mcp_flags flags, mce_banks_t *b)
 		m.misc = 0;
 		m.addr = 0;
 		m.bank = i;
-		m.tsc = 0;
 
 		barrier();
 		m.status = mce_rdmsrl(msr_ops.status(i));
@@ -1394,7 +1393,7 @@ static void mce_timer_fn(unsigned long data)
 	iv = __this_cpu_read(mce_next_interval);
 
 	if (mce_available(this_cpu_ptr(&cpu_info))) {
-		machine_check_poll(MCP_TIMESTAMP, this_cpu_ptr(&mce_poll_banks));
+		machine_check_poll(0, this_cpu_ptr(&mce_poll_banks));
 
 		if (mce_intel_cmci_poll()) {
 			iv = mce_adjust_timer(iv);
diff --git a/arch/x86/kernel/cpu/mcheck/mce_intel.c b/arch/x86/kernel/cpu/mcheck/mce_intel.c
index 1defb8ea882c..be0b2fad47c5 100644
--- a/arch/x86/kernel/cpu/mcheck/mce_intel.c
+++ b/arch/x86/kernel/cpu/mcheck/mce_intel.c
@@ -130,7 +130,7 @@ bool mce_intel_cmci_poll(void)
 	 * Reset the counter if we've logged an error in the last poll
 	 * during the storm.
 	 */
-	if (machine_check_poll(MCP_TIMESTAMP, this_cpu_ptr(&mce_banks_owned)))
+	if (machine_check_poll(0, this_cpu_ptr(&mce_banks_owned)))
 		this_cpu_write(cmci_backoff_cnt, INITIAL_CHECK_INTERVAL);
 	else
 		this_cpu_dec(cmci_backoff_cnt);
@@ -342,7 +342,7 @@ void cmci_recheck(void)
 		return;
 
 	local_irq_save(flags);
-	machine_check_poll(MCP_TIMESTAMP, this_cpu_ptr(&mce_banks_owned));
+	machine_check_poll(0, this_cpu_ptr(&mce_banks_owned));
 	local_irq_restore(flags);
 }
 
-- 
2.10.0

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

[toc] | [prev] | [next] | [standalone]

#1517448 — RE: [PATCH] x86/MCE: Remove MCP_TIMESTAMP

> This still preserves the precise TSC timestamp in intel_threshold_interrupt().

Yup - this looks right.

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

-Tony

[toc] | [prev] | [next] | [standalone]

#1517571 — Re: [PATCH] x86/MCE: Remove MCP_TIMESTAMP

On Tue, 8 Nov 2016, Borislav Petkov wrote:
> 
> Also, this fixes another bug where machine_check_poll() would clear
> mce.tsc unconditionally even if we requested precise MCP_TIMESTAMP
> logging.

> @@ -713,7 +713,6 @@ bool machine_check_poll(enum mcp_flags flags, mce_banks_t *b)
>  		m.misc = 0;
>  		m.addr = 0;
>  		m.bank = i;
> -		m.tsc = 0;

That does not make any sense. Where is m.tsc initialized? I couldn't find
any place which does, except this and the conditional clear farther down in
that function.

Thanks,

	tglx

[toc] | [prev] | [next] | [standalone]

#1517585 — Re: [PATCH] x86/MCE: Remove MCP_TIMESTAMP

On Tue, Nov 08, 2016 at 09:39:02PM +0100, Thomas Gleixner wrote:
> That does not make any sense. Where is m.tsc initialized? I couldn't find
> any place which does, except this and the conditional clear farther down in
> that function.

mce_gather_info->mce_setup does

m->tsc = rdtsc();

And we do that *everytime* but then we go and clear the damn thing. I
know, I know, I wanted to flip that logic too and read the TSC *only*
when we want a precise timestamp but that would require more changes as
mce_setup() is used at a bunch of places.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

[toc] | [prev] | [next] | [standalone]

#1517587 — Re: [PATCH] x86/MCE: Remove MCP_TIMESTAMP

On Tue, 8 Nov 2016, Borislav Petkov wrote:
> On Tue, Nov 08, 2016 at 09:39:02PM +0100, Thomas Gleixner wrote:
> > That does not make any sense. Where is m.tsc initialized? I couldn't find
> > any place which does, except this and the conditional clear farther down in
> > that function.
> 
> mce_gather_info->mce_setup does
> 
> m->tsc = rdtsc();
> 
> And we do that *everytime* but then we go and clear the damn thing. I
> know, I know, I wanted to flip that logic too and read the TSC *only*
> when we want a precise timestamp but that would require more changes as
> mce_setup() is used at a bunch of places.

And yes, you should spend the extra cycles. Adding a flags argument to
mce_setup() and propagate it through the various callsites shouldn't be
that hard and would make the stuff obvious instead of obfuscated.

Thanks,

	tglx

[toc] | [prev] | [next] | [standalone]

#1517595 — Re: [PATCH] x86/MCE: Remove MCP_TIMESTAMP

On Tue, Nov 08, 2016 at 10:14:04PM +0100, Thomas Gleixner wrote:
> And yes, you should spend the extra cycles. Adding a flags argument to
> mce_setup() and propagate it through the various callsites shouldn't be
> that hard and would make the stuff obvious instead of obfuscated.

Sure, that's already on my TODO. I want to take a look at it when I have
a quiet moment.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

[toc] | [prev] | [next] | [standalone]

#1517606 — Re: [PATCH] x86/MCE: Remove MCP_TIMESTAMP

On Tue, 8 Nov 2016, Borislav Petkov wrote:

> On Tue, Nov 08, 2016 at 10:14:04PM +0100, Thomas Gleixner wrote:
> > And yes, you should spend the extra cycles. Adding a flags argument to
> > mce_setup() and propagate it through the various callsites shouldn't be
> > that hard and would make the stuff obvious instead of obfuscated.
> 
> Sure, that's already on my TODO. I want to take a look at it when I have
> a quiet moment.

So for now we should fold something like the below into this patch.

Thanks,

	tglx

8<--------------------

--- a/arch/x86/kernel/cpu/mcheck/mce.c
+++ b/arch/x86/kernel/cpu/mcheck/mce.c
@@ -706,6 +706,15 @@ bool machine_check_poll(enum mcp_flags f
 
 	mce_gather_info(&m, NULL);
 
+	/*
+	 * m.tsc was set in mce_setup(). Clear it if not requested.
+	 *
+	 * FIXME: Propagate @flags to mce_gather_info/mce_setup() to avoid
+	 *	  that dance
+	 */
+	if (!(flags & MCP_TIMESTAMP))
+		m.tsc = 0;
+
 	for (i = 0; i < mca_cfg.banks; i++) {
 		if (!mce_banks[i].ctl || !test_bit(i, *b))
 			continue;
@@ -734,9 +743,6 @@ bool machine_check_poll(enum mcp_flags f
 
 		mce_read_aux(&m, i);
 
-		if (!(flags & MCP_TIMESTAMP))
-			m.tsc = 0;
-
 		severity = mce_severity(&m, mca_cfg.tolerant, NULL, false);
 
 		if (severity == MCE_DEFERRED_SEVERITY && memory_error(&m))

[toc] | [prev] | [next] | [standalone]

#1518343 — Re: [PATCH] x86/MCE: Remove MCP_TIMESTAMP

On Tue, Nov 08, 2016 at 10:54:52PM +0100, Thomas Gleixner wrote:
> So for now we should fold something like the below into this patch.

Ok, how's that?

---
From: Borislav Petkov <bp@suse.de>
Date: Tue, 8 Nov 2016 16:20:05 +0100
Subject: [PATCH] x86/MCE: Correct TSC timestamping of error records

We did have logic in the MCE code which would TSC-timestamp an error
record only when it is exact - i.e., when it wasn't detected by polling.
This isn't the case anymore. So let's fix that:

We have a valid TSC timestamp in the error record only when it has been
a precise detection, i.e., either in the #MC handler or in one of the
interrupt handlers (thresholding, deferred, ...).

All other error records still have mce.time which contains the wall
time in order to be able to place the error record in time at least
approximately.

Also, this fixes another bug where machine_check_poll() would clear
mce.tsc unconditionally even if we requested precise MCP_TIMESTAMP
logging.

The proper fix would be to generate timestamp only when it has been
requested and not always. But that would require a more thorough code
audit of all mce_gather_info/mce_setup() users. Add a FIXME for now,
courtesy of tglx.

Signed-off-by: Borislav Petkov <bp@suse.de>
Cc: Tony Luck <tony.luck@intel.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
---
 arch/x86/kernel/cpu/mcheck/mce.c       | 18 ++++++++++++------
 arch/x86/kernel/cpu/mcheck/mce_intel.c |  4 ++--
 2 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/arch/x86/kernel/cpu/mcheck/mce.c b/arch/x86/kernel/cpu/mcheck/mce.c
index 4ca00474804b..3fbe80066b4e 100644
--- a/arch/x86/kernel/cpu/mcheck/mce.c
+++ b/arch/x86/kernel/cpu/mcheck/mce.c
@@ -706,6 +706,17 @@ bool machine_check_poll(enum mcp_flags flags, mce_banks_t *b)
 
 	mce_gather_info(&m, NULL);
 
+	/*
+	 * m.tsc was set in mce_setup(). Clear it if not requested.
+	 *
+	 * FIXME: Propagate @flags to mce_gather_info/mce_setup() to avoid
+	 *	  that dance.
+	 */
+	if (!(flags & MCP_TIMESTAMP)) {
+		WARN_ON_ONCE(m.tsc);
+		m.tsc = 0;
+	}
+
 	for (i = 0; i < mca_cfg.banks; i++) {
 		if (!mce_banks[i].ctl || !test_bit(i, *b))
 			continue;
@@ -713,14 +724,12 @@ bool machine_check_poll(enum mcp_flags flags, mce_banks_t *b)
 		m.misc = 0;
 		m.addr = 0;
 		m.bank = i;
-		m.tsc = 0;
 
 		barrier();
 		m.status = mce_rdmsrl(msr_ops.status(i));
 		if (!(m.status & MCI_STATUS_VAL))
 			continue;
 
-
 		/*
 		 * Uncorrected or signalled events are handled by the exception
 		 * handler when it is enabled, so don't process those here.
@@ -735,9 +744,6 @@ bool machine_check_poll(enum mcp_flags flags, mce_banks_t *b)
 
 		mce_read_aux(&m, i);
 
-		if (!(flags & MCP_TIMESTAMP))
-			m.tsc = 0;
-
 		severity = mce_severity(&m, mca_cfg.tolerant, NULL, false);
 
 		if (severity == MCE_DEFERRED_SEVERITY && memory_error(&m))
@@ -1394,7 +1400,7 @@ static void mce_timer_fn(unsigned long data)
 	iv = __this_cpu_read(mce_next_interval);
 
 	if (mce_available(this_cpu_ptr(&cpu_info))) {
-		machine_check_poll(MCP_TIMESTAMP, this_cpu_ptr(&mce_poll_banks));
+		machine_check_poll(0, this_cpu_ptr(&mce_poll_banks));
 
 		if (mce_intel_cmci_poll()) {
 			iv = mce_adjust_timer(iv);
diff --git a/arch/x86/kernel/cpu/mcheck/mce_intel.c b/arch/x86/kernel/cpu/mcheck/mce_intel.c
index 1defb8ea882c..be0b2fad47c5 100644
--- a/arch/x86/kernel/cpu/mcheck/mce_intel.c
+++ b/arch/x86/kernel/cpu/mcheck/mce_intel.c
@@ -130,7 +130,7 @@ bool mce_intel_cmci_poll(void)
 	 * Reset the counter if we've logged an error in the last poll
 	 * during the storm.
 	 */
-	if (machine_check_poll(MCP_TIMESTAMP, this_cpu_ptr(&mce_banks_owned)))
+	if (machine_check_poll(0, this_cpu_ptr(&mce_banks_owned)))
 		this_cpu_write(cmci_backoff_cnt, INITIAL_CHECK_INTERVAL);
 	else
 		this_cpu_dec(cmci_backoff_cnt);
@@ -342,7 +342,7 @@ void cmci_recheck(void)
 		return;
 
 	local_irq_save(flags);
-	machine_check_poll(MCP_TIMESTAMP, this_cpu_ptr(&mce_banks_owned));
+	machine_check_poll(0, this_cpu_ptr(&mce_banks_owned));
 	local_irq_restore(flags);
 }
 
-- 
2.10.0

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

[toc] | [prev] | [next] | [standalone]

#1515883 — Re: [RFC PATCH 0/3] x86/RAS: Dump error record to dmesg if no consumers

* Borislav Petkov <bp@alien8.de> wrote:

> From: Borislav Petkov <bp@suse.de>
> 
> Right,
> 
> so this is not a good thing: systems may not have any error record
> consumers registered and in such cases, any logged MCEs disappear into
> the void. And this shouldn't happen.
> 
> So let's dump them to dmesg as a last resort.
> 
> Borislav Petkov (3):
>   notifiers: Document notifier priority
>   x86/RAS: Add TSC to the injected MCE
>   x86/MCE: Dump MCE to dmesg if no consumers
> 
>  arch/x86/kernel/cpu/mcheck/mce.c | 52 +++++++++++++++++++++++++++++++++++-----
>  arch/x86/ras/mce_amd_inj.c       |  2 ++
>  include/linux/notifier.h         |  1 +
>  3 files changed, 49 insertions(+), 6 deletions(-)

Sounds good to me!

Thanks,

	Ingo

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web