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

Groups > linux.kernel > #1551630 > unrolled thread

Re: Dell XPS13: MCE (Hardware Error) reported

Back to article view | Back to linux.kernel

Contents

  Re: Dell XPS13: MCE (Hardware Error) reported Daniel J Blueman <daniel@quora.org> - 2017-01-05 06:10 +0100
    Re: Dell XPS13: MCE (Hardware Error) reported Daniel J Blueman <daniel@quora.org> - 2017-01-05 15:10 +0100
      Re: Dell XPS13: MCE (Hardware Error) reported Alexander Alemayhu <alexander@alemayhu.com> - 2017-01-05 21:20 +0100
        Re: Dell XPS13: MCE (Hardware Error) reported Borislav Petkov <bp@suse.de> - 2017-01-05 21:40 +0100
          Re: Dell XPS13: MCE (Hardware Error) reported "Raj, Ashok" <ashok.raj@intel.com> - 2017-01-05 21:50 +0100
            Re: Dell XPS13: MCE (Hardware Error) reported "Pandruvada, Srinivas" <srinivas.pandruvada@intel.com> - 2017-01-05 22:10 +0100
              Re: Dell XPS13: MCE (Hardware Error) reported Alexander Alemayhu <alexander@alemayhu.com> - 2017-01-06 00:30 +0100
          Re: Dell XPS13: MCE (Hardware Error) reported Alexander Alemayhu <alexander@alemayhu.com> - 2017-01-05 22:40 +0100
          Re: Dell XPS13: MCE (Hardware Error) reported "Raj, Ashok" <ashok.raj@intel.com> - 2017-01-06 00:30 +0100
            Re: Dell XPS13: MCE (Hardware Error) reported Borislav Petkov <bp@suse.de> - 2017-01-06 01:00 +0100
              Re: Dell XPS13: MCE (Hardware Error) reported "Raj, Ashok" <ashok.raj@intel.com> - 2017-01-06 02:30 +0100
                Re: Dell XPS13: MCE (Hardware Error) reported Borislav Petkov <bp@suse.de> - 2017-01-06 12:20 +0100
                  Re: Dell XPS13: MCE (Hardware Error) reported "Raj, Ashok" <ashok.raj@intel.com> - 2017-01-06 17:00 +0100
                    Re: Dell XPS13: MCE (Hardware Error) reported Borislav Petkov <bp@suse.de> - 2017-01-06 18:00 +0100
                      Re: Dell XPS13: MCE (Hardware Error) reported "Raj, Ashok" <ashok.raj@intel.com> - 2017-01-06 18:10 +0100
                      Re: Dell XPS13: MCE (Hardware Error) reported Paul Menzel <pmenzel@molgen.mpg.de> - 2017-01-09 12:00 +0100
                        Re: Dell XPS13: MCE (Hardware Error) reported Borislav Petkov <bp@suse.de> - 2017-01-09 12:10 +0100
                          Re: Dell XPS13: MCE (Hardware Error) reported Paul Menzel <pmenzel@molgen.mpg.de> - 2017-01-09 12:20 +0100

#1551630 — Re: Dell XPS13: MCE (Hardware Error) reported

On Thursday, January 5, 2017 at 9:20:04 AM UTC+8, Raj, Ashok wrote:
> Hi Boris
>
> thanks for forwarding.
>
> > > CPUID Vendor Intel Family 6 Model 142
> This is Kabylake Mobile
>
> > > Hardware event. This is not a software error.
> > > MCE 1
> > > CPU 0 BANK 7
> > > MISC 7880018086 ADDR fef1ce40
> > > TIME 1483543069 Wed Jan  4 16:17:49 2017
> > > MCG status:
> > > MCi status:
> > > Error overflow
> > > Uncorrected error
> > > MCi_MISC register valid
> > > MCi_ADDR register valid
> > > Processor context corrupt
> > > MCA: corrected filtering (some unreported errors in same region)
> > > Generic CACHE Level-2 Generic Error
> > > STATUS ee0000000040110a MCGSTATUS 0
>
> Decoding the bits further from MCi_STATUS above:
> Val=1, OVER=1, UC=1, but EN=0 indicates this isn't a MCE, hence should have
> been signaled by a CMCI.
>
> PCC=1, but should be ignored when EN=0.
> MCACOD: 110a MSCOD: 0040
>
> If the system is stable enough after the report, can you send the output of
> /proc/interrupts to confirm that.
>
> Although its reported as a L2 error, some memory errors can also manifest
> itself as a cache error in certain cases.  In this case it looks like
> some speculative fetch from bad memory might be the cause.
>
> > > MCGCAP c08 APICID 0 SOCKETID 0
>
> MCG_CAP: c08
> Support CMCI(bit 10) - Corrected Machine Check Interrupt (CMCI_P) and
> Threshold based error reporting (bit 11) (TES_P).
>
>
> Do you have another machine which doesn't report these errors? if so try
> swapping memory between them to see if the error disappears.
>
> I don't have the model specific error handy.. will check that in the meantime
> to get some decoding as well.
>
> If you haven't already running some memory tests would also help.
>
> If you replaced the motherboard, did that involve both cpu and memory?
> or just the motheboard swap?

I see the MCE on my XPS 9360 also. It's not related to DRAM, as the
physical address is in the non-coherent low MMIO window:
MISC 7880018086 ADDR fef1ce40

Which is declared as device memory:
[    0.000000] PM: Registered nosave memory: [mem 0xfee01000-0xfeffffff]

For core-generated cycles, it is between the local APIC space at
FEE00000:FEEFFFF and SPI BIOS at FFE00000:FFFFFFFF, so will be
subtractively decoded to the PCH, maybe being aborted due to a device
not being enabled (hello TPM3 or new image processor).

As it is logged as soon as the MCE driver initialises, it was probably
logged during BIOS init, so there's not much we can do about it
anyways.

Dan
-- 
Daniel J Blueman

[toc] | [next] | [standalone]

#1552003

On 5 January 2017 at 13:00, Daniel J Blueman <daniel@quora.org> wrote:
> On Thursday, January 5, 2017 at 9:20:04 AM UTC+8, Raj, Ashok wrote:
>> Hi Boris
>>
>> thanks for forwarding.
>>
>> > > CPUID Vendor Intel Family 6 Model 142
>> This is Kabylake Mobile
>>
>> > > Hardware event. This is not a software error.
>> > > MCE 1
>> > > CPU 0 BANK 7
>> > > MISC 7880018086 ADDR fef1ce40
>> > > TIME 1483543069 Wed Jan  4 16:17:49 2017
>> > > MCG status:
>> > > MCi status:
>> > > Error overflow
>> > > Uncorrected error
>> > > MCi_MISC register valid
>> > > MCi_ADDR register valid
>> > > Processor context corrupt
>> > > MCA: corrected filtering (some unreported errors in same region)
>> > > Generic CACHE Level-2 Generic Error
>> > > STATUS ee0000000040110a MCGSTATUS 0
>>
>> Decoding the bits further from MCi_STATUS above:
>> Val=1, OVER=1, UC=1, but EN=0 indicates this isn't a MCE, hence should have
>> been signaled by a CMCI.
>>
>> PCC=1, but should be ignored when EN=0.
>> MCACOD: 110a MSCOD: 0040
>>
>> If the system is stable enough after the report, can you send the output of
>> /proc/interrupts to confirm that.
>>
>> Although its reported as a L2 error, some memory errors can also manifest
>> itself as a cache error in certain cases.  In this case it looks like
>> some speculative fetch from bad memory might be the cause.
>>
>> > > MCGCAP c08 APICID 0 SOCKETID 0
>>
>> MCG_CAP: c08
>> Support CMCI(bit 10) - Corrected Machine Check Interrupt (CMCI_P) and
>> Threshold based error reporting (bit 11) (TES_P).
>>
>>
>> Do you have another machine which doesn't report these errors? if so try
>> swapping memory between them to see if the error disappears.
>>
>> I don't have the model specific error handy.. will check that in the meantime
>> to get some decoding as well.
>>
>> If you haven't already running some memory tests would also help.
>>
>> If you replaced the motherboard, did that involve both cpu and memory?
>> or just the motheboard swap?
>
> I see the MCE on my XPS 9360 also. It's not related to DRAM, as the
> physical address is in the non-coherent low MMIO window:
> MISC 7880018086 ADDR fef1ce40
>
> Which is declared as device memory:
> [    0.000000] PM: Registered nosave memory: [mem 0xfee01000-0xfeffffff]
>
> For core-generated cycles, it is between the local APIC space at
> FEE00000:FEEFFFF and SPI BIOS at FFE00000:FFFFFFFF, so will be
> subtractively decoded to the PCH, maybe being aborted due to a device
> not being enabled (hello TPM3 or new image processor).
>
> As it is logged as soon as the MCE driver initialises, it was probably
> logged during BIOS init, so there's not much we can do about it
> anyways.

That said, I have seen this reoccur after boot; there were no other
kernel messages around 300s uptime, and it hasn't occurred in the last
hours since:

$ dmesg | grep Machine
[    0.039072] mce: [Hardware Error]: Machine check events logged
[  300.069176] mce: [Hardware Error]: Machine check events logged

As I don't see a driver controlling this area of address space, the
access is likely initiated from the UEFI BIOS System Management Mode
handler, and we see the same pair of registers FEF1FF40, FEF1CE40
accessed each time.

Dan
-- 
Daniel J Blueman

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

#1552305

On Thu, Jan 05, 2017 at 10:05:39PM +0800, Daniel J Blueman wrote:
> 
> That said, I have seen this reoccur after boot; there were no other
> kernel messages around 300s uptime, and it hasn't occurred in the last
> hours since:
>

Not sure if it is related, but I am also seeing those messages on my
MacBookPro11,3:

grep -e "Linux version" -e "Machine" oops-2017-01-05-10:32:10-1076-0/dmesg

[    0.000000] Linux version 4.9.0 (scanf@hafza) (gcc version 6.3.1 20161221 (Red Hat 6.3.1-1) (GCC) ) #1 SMP Sun Dec 25 22:25:17 CET 2016
[ 4231.274376] mce: [Hardware Error]: Machine check events logged
[ 4231.274893] mce: [Hardware Error]: Machine check events logged
[ 4531.292608] mce: [Hardware Error]: Machine check events logged
[ 4531.292610] mce: [Hardware Error]: Machine check events logged
[ 4833.369927] mce: [Hardware Error]: Machine check events logged
[ 4833.370906] mce: [Hardware Error]: Machine check events logged
[ 5135.449222] mce: [Hardware Error]: Machine check events logged
[ 5135.449228] mce: [Hardware Error]: Machine check events logged
[ 5435.564152] mce: [Hardware Error]: Machine check events logged
[ 5435.564153] mce: [Hardware Error]: Machine check events logged
[ 5735.592854] mce: [Hardware Error]: Machine check events logged
[ 5735.592862] mce: [Hardware Error]: Machine check events logged
[ 6038.070068] mce: [Hardware Error]: Machine check events logged
[ 6038.070073] mce: [Hardware Error]: Machine check events logged
[ 6338.170948] mce: [Hardware Error]: Machine check events logged
[ 6338.171930] mce: [Hardware Error]: Machine check events logged
[ 6920.788280] mce: [Hardware Error]: Machine check events logged
[ 6920.788284] mce: [Hardware Error]: Machine check events logged

full output: https://gist.githubusercontent.com/scanf/5b9dc1940c4913f393fbfbbe40ef6788/raw/2fe98a912979b8b4346c4c55582e092fc42bce8a/7ba692efe.txt
-- 
Mit freundlichen Grüßen

Alexander Alemayhu

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

#1552313

On Thu, Jan 05, 2017 at 09:10:34PM +0100, Alexander Alemayhu wrote:
> Not sure if it is related, but I am also seeing those messages on my
> MacBookPro11,3:

Yours look to me like thermal throttling MCEs. And TBH we whould
not issue those as actual MCEs because they are not - they *signal*
overheating condition only and should be handled differently.

What does your /var/log/mcelog* log file contain?

-- 
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 

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

#1552324

Hi Boris


On Thu, Jan 05, 2017 at 09:31:47PM +0100, Borislav Petkov wrote:
> On Thu, Jan 05, 2017 at 09:10:34PM +0100, Alexander Alemayhu wrote:
> > Not sure if it is related, but I am also seeing those messages on my
> > MacBookPro11,3:
> 
> Yours look to me like thermal throttling MCEs. And TBH we whould
> not issue those as actual MCEs because they are not - they *signal*
> overheating condition only and should be handled differently.

That's right.. the thermal interrupts are being reported, that should have
started running cpu's at lower frequencies via some thermald. 
If that's not handled and the PCU starts enforcing trying to keep the temps 
under control system starts logging MCE's. 

Ccing Srinivas who might be able to give a better pointer to check why 
that's not happening.

The log didn't have the exact MCE's reported.. if you have mcelog, please
attach that in the report.

> 
> What does your /var/log/mcelog* log file contain?
> 
> -- 
> Regards/Gruss,
>     Boris.
> 
> SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
> -- 

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

#1552330

On Thu, 2017-01-05 at 12:43 -0800, Raj, Ashok wrote:
> Hi Boris
> 
> 
> On Thu, Jan 05, 2017 at 09:31:47PM +0100, Borislav Petkov wrote:
> > 
> > On Thu, Jan 05, 2017 at 09:10:34PM +0100, Alexander Alemayhu wrote:
> > > 
> > > Not sure if it is related, but I am also seeing those messages on
> > > my
> > > MacBookPro11,3:
> > 
> > Yours look to me like thermal throttling MCEs. And TBH we whould
> > not issue those as actual MCEs because they are not - they *signal*
> > overheating condition only and should be handled differently.
> 
> That's right.. the thermal interrupts are being reported, that should
> have
> started running cpu's at lower frequencies via some thermald. 
> If that's not handled and the PCU starts enforcing trying to keep the
> temps 
> under control system starts logging MCE's. 
> 
> Ccing Srinivas who might be able to give a better pointer to check
> why 
> that's not happening.
I suggest trying with the following kernel command line, if your
getting notification to throttle from SMM before:

	intel_pstate=support_acpi_ppc

opensuse doesn't start thermald by default.

Thanks,
Srinivas

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

#1552400

On Thu, Jan 05, 2017 at 09:03:10PM +0000, Pandruvada, Srinivas wrote:
> I suggest trying with the following kernel command line, if your
> getting notification to throttle from SMM before:
> 
> 	intel_pstate=support_acpi_ppc
> 
> opensuse doesn't start thermald by default.
> 

Used the suggested kernel command line change and made sure thermald[0] is
running.  I see no more mce* related errors and my fans are making less noise
during workloads.  Thank you :)

[0]: https://github.com/01org/thermal_daemon

-- 
Mit freundlichen Grüßen

Alexander Alemayhu

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

#1552344

[Multipart message — attachments visible in raw view] — view raw

On Thu, Jan 05, 2017 at 09:31:47PM +0100, Borislav Petkov wrote:
> 
> What does your /var/log/mcelog* log file contain?
>

There are no files there, but in the attached backtrace mcelog is mentioned
several times. Sorry I am not familiar with mcelog so I don't know where it
would be logging on Fedora.

-- 
Mit freundlichen Grüßen

Alexander Alemayhu

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

#1552403

Hi Boris

On Thu, Jan 05, 2017 at 09:31:47PM +0100, Borislav Petkov wrote:
> On Thu, Jan 05, 2017 at 09:10:34PM +0100, Alexander Alemayhu wrote:
> > Not sure if it is related, but I am also seeing those messages on my
> > MacBookPro11,3:
> 
> Yours look to me like thermal throttling MCEs. And TBH we whould
> not issue those as actual MCEs because they are not - they *signal*
> overheating condition only and should be handled differently.

After looking at the code, seems like these events are logged as MCE's
but are really picked from real lvt thermal event interrupts.  via a fake
bank 128 for MCE_THERMAL. These are not really HW MCE's, but fake ones created 
and logged as mcelog entries. (arch/x86/kernel/cpu/mcheck/therm_throt.c)

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

#1552410

On Thu, Jan 05, 2017 at 03:28:00PM -0800, Raj, Ashok wrote:
> After looking at the code, seems like these events are logged as MCE's
> but are really picked from real lvt thermal event interrupts.  via a fake
> bank 128 for MCE_THERMAL. These are not really HW MCE's, but fake ones created 
> and logged as mcelog entries. (arch/x86/kernel/cpu/mcheck/therm_throt.c)

Right, we've done that since forever but I do think that it confuses
people. This thread case-in-point. I mean, we already scream:

	pr_crit("CPU%d: %s temperature above threshold, cpu clock throttled (total events = %lu)\n",

to dmesg, why do we have to log a fake MCE too?!

Hell, we even log an MCE when things go back to normal:

        if (old_event) {
                if (event == THERMAL_THROTTLING_EVENT)
                        pr_info("CPU%d: %s temperature/speed normal\n", this_cpu,
                                level == CORE_LEVEL ? "Core" : "Package");
                return 1;

And Alexander's log shows exactly that:

[ 6338.170924] CPU1: Core temperature above threshold, cpu clock throttled (total events = 21068)
[ 6338.170925] CPU5: Core temperature above threshold, cpu clock throttled (total events = 21068)
[ 6338.170928] CPU7: Package temperature above threshold, cpu clock throttled (total events = 22842)
[ 6338.170931] CPU4: Package temperature above threshold, cpu clock throttled (total events = 22842)
[ 6338.170932] CPU0: Package temperature above threshold, cpu clock throttled (total events = 22842)
[ 6338.170933] CPU6: Package temperature above threshold, cpu clock throttled (total events = 22842)
[ 6338.170935] CPU2: Package temperature above threshold, cpu clock throttled (total events = 22842)
[ 6338.170936] CPU3: Package temperature above threshold, cpu clock throttled (total events = 22842)
[ 6338.170937] CPU5: Package temperature above threshold, cpu clock throttled (total events = 22842)
[ 6338.170945] CPU1: Package temperature above threshold, cpu clock throttled (total events = 22842)
[ 6338.170947] mce_notify_irq: 1 callbacks suppressed
[ 6338.170948] mce: [Hardware Error]: Machine check events logged				<--- new event
[ 6338.171917] CPU1: Core temperature/speed normal
[ 6338.171918] CPU5: Core temperature/speed normal
[ 6338.171920] CPU4: Package temperature/speed normal
[ 6338.171920] CPU0: Package temperature/speed normal
[ 6338.171922] CPU2: Package temperature/speed normal
[ 6338.171923] CPU6: Package temperature/speed normal
[ 6338.171924] CPU3: Package temperature/speed normal
[ 6338.171925] CPU7: Package temperature/speed normal
[ 6338.171927] CPU5: Package temperature/speed normal
[ 6338.171929] CPU1: Package temperature/speed normal
[ 6338.171930] mce: [Hardware Error]: Machine check events logged				<--- old event

Oh, and it's not like the user can do anything - there's a thermald
which is supposed to deal with all that. Which is not really
trouble-free too, TBH. What happens if that thing dies? Fried CPU?

So I say we should rip out that mce_log_therm_throt_event() and never
ever handle thermal events with MCEs. It is a bad idea.

-- 
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 

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

#1552446

On Fri, Jan 06, 2017 at 12:56:11AM +0100, Borislav Petkov wrote:
> Oh, and it's not like the user can do anything - there's a thermald
> which is supposed to deal with all that. Which is not really
> trouble-free too, TBH. What happens if that thing dies? Fried CPU?
> 
> So I say we should rip out that mce_log_therm_throt_event() and never
> ever handle thermal events with MCEs. It is a bad idea.

Agree, since we have both a log and another agent to deal with it, it makes
no good reason to continue... Will pass this along, and have someone look at
cleaning this up.


Cheers,
Ashok

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

#1552701

On Thu, Jan 05, 2017 at 05:26:17PM -0800, Raj, Ashok wrote:
> Agree, since we have both a log and another agent to deal with it, it makes
> no good reason to continue... Will pass this along, and have someone look at
> cleaning this up.

Like this?

---
From: Borislav Petkov <bp@suse.de>
Date: Fri, 6 Jan 2017 12:07:08 +0100
Subject: [PATCH] x86/MCE/therm_throt: Do not log a fake MCE for a thermal event

We log a fake bank 128 MCE to note that we're handling a CPU thermal
event. However, this confuses people into thinking that their hardware
generates MCEs. Hijacking MCA for logging thermal events is a gross
misuse anyway and it should've been done in the first place. And besides
we have other means for dealing with thermal events which are much more
suitable.

So let's kill the MCE logging part.

Signed-off-by: Borislav Petkov <bp@suse.de>
---
 arch/x86/include/asm/mce.h               |  6 ------
 arch/x86/kernel/cpu/mcheck/mce.c         | 25 -------------------------
 arch/x86/kernel/cpu/mcheck/therm_throt.c |  9 ++++-----
 3 files changed, 4 insertions(+), 36 deletions(-)

diff --git a/arch/x86/include/asm/mce.h b/arch/x86/include/asm/mce.h
index 5132f2a6c0a2..a09ed05725c2 100644
--- a/arch/x86/include/asm/mce.h
+++ b/arch/x86/include/asm/mce.h
@@ -97,10 +97,6 @@
 
 #define MCE_OVERFLOW 0		/* bit 0 in flags means overflow */
 
-/* Software defined banks */
-#define MCE_EXTENDED_BANK	128
-#define MCE_THERMAL_BANK	(MCE_EXTENDED_BANK + 0)
-
 #define MCE_LOG_LEN 32
 #define MCE_LOG_SIGNATURE	"MACHINECHECK"
 
@@ -306,8 +302,6 @@ extern void (*deferred_error_int_vector)(void);
 
 void intel_init_thermal(struct cpuinfo_x86 *c);
 
-void mce_log_therm_throt_event(__u64 status);
-
 /* Interrupt Handler for core thermal thresholds */
 extern int (*platform_thermal_notify)(__u64 msr_val);
 
diff --git a/arch/x86/kernel/cpu/mcheck/mce.c b/arch/x86/kernel/cpu/mcheck/mce.c
index 00ef43233e03..6eef6fde0f02 100644
--- a/arch/x86/kernel/cpu/mcheck/mce.c
+++ b/arch/x86/kernel/cpu/mcheck/mce.c
@@ -1331,31 +1331,6 @@ static void mce_process_work(struct work_struct *dummy)
 	mce_gen_pool_process();
 }
 
-#ifdef CONFIG_X86_MCE_INTEL
-/***
- * mce_log_therm_throt_event - Logs the thermal throttling event to mcelog
- * @cpu: The CPU on which the event occurred.
- * @status: Event status information
- *
- * This function should be called by the thermal interrupt after the
- * event has been processed and the decision was made to log the event
- * further.
- *
- * The status parameter will be saved to the 'status' field of 'struct mce'
- * and historically has been the register value of the
- * MSR_IA32_THERMAL_STATUS (Intel) msr.
- */
-void mce_log_therm_throt_event(__u64 status)
-{
-	struct mce m;
-
-	mce_setup(&m);
-	m.bank = MCE_THERMAL_BANK;
-	m.status = status;
-	mce_log(&m);
-}
-#endif /* CONFIG_X86_MCE_INTEL */
-
 /*
  * Periodic polling timer for "silent" machine check errors.  If the
  * poller finds an MCE, poll 2x faster.  When the poller finds no more
diff --git a/arch/x86/kernel/cpu/mcheck/therm_throt.c b/arch/x86/kernel/cpu/mcheck/therm_throt.c
index 465aca8be009..109fbb25c851 100644
--- a/arch/x86/kernel/cpu/mcheck/therm_throt.c
+++ b/arch/x86/kernel/cpu/mcheck/therm_throt.c
@@ -6,7 +6,7 @@
  *
  * Maintains a counter in /sys that keeps track of the number of thermal
  * events, such that the user knows how bad the thermal problem might be
- * (since the logging to syslog and mcelog is rate limited).
+ * (since the logging to syslog is rate limited).
  *
  * Author: Dmitriy Zavin (dmitriyz@google.com)
  *
@@ -365,10 +365,9 @@ static void intel_thermal_interrupt(void)
 	/* Check for violation of core thermal thresholds*/
 	notify_thresholds(msr_val);
 
-	if (therm_throt_process(msr_val & THERM_STATUS_PROCHOT,
-				THERMAL_THROTTLING_EVENT,
-				CORE_LEVEL) != 0)
-		mce_log_therm_throt_event(msr_val);
+	therm_throt_process(msr_val & THERM_STATUS_PROCHOT,
+			    THERMAL_THROTTLING_EVENT,
+			    CORE_LEVEL);
 
 	if (this_cpu_has(X86_FEATURE_PLN) && int_pln_enable)
 		therm_throt_process(msr_val & THERM_STATUS_POWER_LIMIT,
-- 
2.11.0

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 

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

#1552905

Hi Boris

On Fri, Jan 06, 2017 at 12:16:17PM +0100, Borislav Petkov wrote:
> On Thu, Jan 05, 2017 at 05:26:17PM -0800, Raj, Ashok wrote:
> > Agree, since we have both a log and another agent to deal with it, it makes
> > no good reason to continue... Will pass this along, and have someone look at
> > cleaning this up.
> 
> Like this?

That was quick :-).

> -	if (therm_throt_process(msr_val & THERM_STATUS_PROCHOT,
> -				THERMAL_THROTTLING_EVENT,
> -				CORE_LEVEL) != 0)

Looks like we don't need a return value from therm_throt_process(),
we can fix that as void as well.

Otherwise it looks good. 

> +	therm_throt_process(msr_val & THERM_STATUS_PROCHOT,
> +			    THERMAL_THROTTLING_EVENT,
> +			    CORE_LEVEL);
>  

Cheers,
Ashok

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

#1552958

On Fri, Jan 06, 2017 at 07:58:31AM -0800, Raj, Ashok wrote:
> Looks like we don't need a return value from therm_throt_process(),
> we can fix that as void as well.

Right you are, here's v2:

---
From a8151fa6f18c2605eb7972061234f05e79b372c4 Mon Sep 17 00:00:00 2001
From: Borislav Petkov <bp@suse.de>
Date: Fri, 6 Jan 2017 12:07:08 +0100
Subject: [PATCH] x86/MCE/therm_throt: Do not log a fake MCE for a thermal event

We log a fake bank 128 MCE to note that we're handling a CPU thermal
event. However, this confuses people into thinking that their hardware
generates MCEs. Hijacking MCA for logging thermal events is a gross
misuse anyway and it should've been done in the first place. And besides
we have other means for dealing with thermal events which are much more
suitable.

So let's kill the MCE logging part.

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

v2: Ashok: make therm_throt_process() void.

 arch/x86/include/asm/mce.h               |  6 ------
 arch/x86/kernel/cpu/mcheck/mce.c         | 25 -------------------------
 arch/x86/kernel/cpu/mcheck/therm_throt.c | 30 +++++++++++-------------------
 3 files changed, 11 insertions(+), 50 deletions(-)

diff --git a/arch/x86/include/asm/mce.h b/arch/x86/include/asm/mce.h
index 5132f2a6c0a2..a09ed05725c2 100644
--- a/arch/x86/include/asm/mce.h
+++ b/arch/x86/include/asm/mce.h
@@ -97,10 +97,6 @@
 
 #define MCE_OVERFLOW 0		/* bit 0 in flags means overflow */
 
-/* Software defined banks */
-#define MCE_EXTENDED_BANK	128
-#define MCE_THERMAL_BANK	(MCE_EXTENDED_BANK + 0)
-
 #define MCE_LOG_LEN 32
 #define MCE_LOG_SIGNATURE	"MACHINECHECK"
 
@@ -306,8 +302,6 @@ extern void (*deferred_error_int_vector)(void);
 
 void intel_init_thermal(struct cpuinfo_x86 *c);
 
-void mce_log_therm_throt_event(__u64 status);
-
 /* Interrupt Handler for core thermal thresholds */
 extern int (*platform_thermal_notify)(__u64 msr_val);
 
diff --git a/arch/x86/kernel/cpu/mcheck/mce.c b/arch/x86/kernel/cpu/mcheck/mce.c
index 00ef43233e03..6eef6fde0f02 100644
--- a/arch/x86/kernel/cpu/mcheck/mce.c
+++ b/arch/x86/kernel/cpu/mcheck/mce.c
@@ -1331,31 +1331,6 @@ static void mce_process_work(struct work_struct *dummy)
 	mce_gen_pool_process();
 }
 
-#ifdef CONFIG_X86_MCE_INTEL
-/***
- * mce_log_therm_throt_event - Logs the thermal throttling event to mcelog
- * @cpu: The CPU on which the event occurred.
- * @status: Event status information
- *
- * This function should be called by the thermal interrupt after the
- * event has been processed and the decision was made to log the event
- * further.
- *
- * The status parameter will be saved to the 'status' field of 'struct mce'
- * and historically has been the register value of the
- * MSR_IA32_THERMAL_STATUS (Intel) msr.
- */
-void mce_log_therm_throt_event(__u64 status)
-{
-	struct mce m;
-
-	mce_setup(&m);
-	m.bank = MCE_THERMAL_BANK;
-	m.status = status;
-	mce_log(&m);
-}
-#endif /* CONFIG_X86_MCE_INTEL */
-
 /*
  * Periodic polling timer for "silent" machine check errors.  If the
  * poller finds an MCE, poll 2x faster.  When the poller finds no more
diff --git a/arch/x86/kernel/cpu/mcheck/therm_throt.c b/arch/x86/kernel/cpu/mcheck/therm_throt.c
index 465aca8be009..85469f84c921 100644
--- a/arch/x86/kernel/cpu/mcheck/therm_throt.c
+++ b/arch/x86/kernel/cpu/mcheck/therm_throt.c
@@ -6,7 +6,7 @@
  *
  * Maintains a counter in /sys that keeps track of the number of thermal
  * events, such that the user knows how bad the thermal problem might be
- * (since the logging to syslog and mcelog is rate limited).
+ * (since the logging to syslog is rate limited).
  *
  * Author: Dmitriy Zavin (dmitriyz@google.com)
  *
@@ -141,13 +141,8 @@ static struct attribute_group thermal_attr_group = {
  * IRQ has been acknowledged.
  *
  * It will take care of rate limiting and printing messages to the syslog.
- *
- * Returns: 0 : Event should NOT be further logged, i.e. still in
- *              "timeout" from previous log message.
- *          1 : Event should be logged further, and a message has been
- *              printed to the syslog.
  */
-static int therm_throt_process(bool new_event, int event, int level)
+static void therm_throt_process(bool new_event, int event, int level)
 {
 	struct _thermal_state *state;
 	unsigned int this_cpu = smp_processor_id();
@@ -162,16 +157,16 @@ static int therm_throt_process(bool new_event, int event, int level)
 		else if (event == POWER_LIMIT_EVENT)
 			state = &pstate->core_power_limit;
 		else
-			 return 0;
+			return;
 	} else if (level == PACKAGE_LEVEL) {
 		if (event == THERMAL_THROTTLING_EVENT)
 			state = &pstate->package_throttle;
 		else if (event == POWER_LIMIT_EVENT)
 			state = &pstate->package_power_limit;
 		else
-			return 0;
+			return;
 	} else
-		return 0;
+		return;
 
 	old_event = state->new_event;
 	state->new_event = new_event;
@@ -181,7 +176,7 @@ static int therm_throt_process(bool new_event, int event, int level)
 
 	if (time_before64(now, state->next_check) &&
 			state->count != state->last_count)
-		return 0;
+		return;
 
 	state->next_check = now + CHECK_INTERVAL;
 	state->last_count = state->count;
@@ -193,16 +188,14 @@ static int therm_throt_process(bool new_event, int event, int level)
 				this_cpu,
 				level == CORE_LEVEL ? "Core" : "Package",
 				state->count);
-		return 1;
+		return;
 	}
 	if (old_event) {
 		if (event == THERMAL_THROTTLING_EVENT)
 			pr_info("CPU%d: %s temperature/speed normal\n", this_cpu,
 				level == CORE_LEVEL ? "Core" : "Package");
-		return 1;
+		return;
 	}
-
-	return 0;
 }
 
 static int thresh_event_valid(int level, int event)
@@ -365,10 +358,9 @@ static void intel_thermal_interrupt(void)
 	/* Check for violation of core thermal thresholds*/
 	notify_thresholds(msr_val);
 
-	if (therm_throt_process(msr_val & THERM_STATUS_PROCHOT,
-				THERMAL_THROTTLING_EVENT,
-				CORE_LEVEL) != 0)
-		mce_log_therm_throt_event(msr_val);
+	therm_throt_process(msr_val & THERM_STATUS_PROCHOT,
+			    THERMAL_THROTTLING_EVENT,
+			    CORE_LEVEL);
 
 	if (this_cpu_has(X86_FEATURE_PLN) && int_pln_enable)
 		therm_throt_process(msr_val & THERM_STATUS_POWER_LIMIT,
-- 
2.11.0

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 

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

#1552966

Hi Boris

This looks good to me!

On Fri, Jan 06, 2017 at 05:54:23PM +0100, Borislav Petkov wrote:
> On Fri, Jan 06, 2017 at 07:58:31AM -0800, Raj, Ashok wrote:
> > Looks like we don't need a return value from therm_throt_process(),
> > we can fix that as void as well.
> 
> Right you are, here's v2:
> 
> Signed-off-by: Borislav Petkov <bp@suse.de>
> ---
> 
> v2: Ashok: make therm_throt_process() void.

Acked-by: Ashok Raj <ashok.raj@intel.com>
> 
>  arch/x86/include/asm/mce.h               |  6 ------
>  arch/x86/kernel/cpu/mcheck/mce.c         | 25 -------------------------
>  arch/x86/kernel/cpu/mcheck/therm_throt.c | 30 +++++++++++-------------------
>  3 files changed, 11 insertions(+), 50 deletions(-)

Cheers,
Ashok

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

#1554216

On 01/06/17 17:54, Borislav Petkov wrote:
> On Fri, Jan 06, 2017 at 07:58:31AM -0800, Raj, Ashok wrote:
>> Looks like we don't need a return value from therm_throt_process(),
>> we can fix that as void as well.
>
> Right you are, here's v2:
>
> ---
> From a8151fa6f18c2605eb7972061234f05e79b372c4 Mon Sep 17 00:00:00 2001
> From: Borislav Petkov <bp@suse.de>
> Date: Fri, 6 Jan 2017 12:07:08 +0100
> Subject: [PATCH] x86/MCE/therm_throt: Do not log a fake MCE for a thermal event
>
> We log a fake bank 128 MCE to note that we're handling a CPU thermal
> event. However, this confuses people into thinking that their hardware
> generates MCEs. Hijacking MCA for logging thermal events is a gross
> misuse anyway and it should've been done in the first place. And besides

Do you mean *shouldn’t have been done*?

> we have other means for dealing with thermal events which are much more
> suitable.
>
> So let's kill the MCE logging part.
>
> Signed-off-by: Borislav Petkov <bp@suse.de>

Should the discussion be referenced?

Also, is that just for MacBookPro11,3? The MCE for the Dell XPS13 looks 
different from what I see, doesn’t it?

[…]


Kind regards,

Paul

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

#1554221

On Mon, Jan 09, 2017 at 11:55:41AM +0100, Paul Menzel wrote:
> Do you mean *shouldn’t have been done*?

Yes.

> Should the discussion be referenced?

Yap, it will be.

> Also, is that just for MacBookPro11,3? The MCE for the Dell XPS13 looks
> different from what I see, doesn’t it?

Yes, yours is different. I'm still waiting for you to reply to Ashok's
questions here:

https://lkml.kernel.org/r/20170105011236.GA80100@otc-brkl-03

Thanks.

-- 
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 

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

#1554224

Dear Boris,


On 01/09/17 12:05, Borislav Petkov wrote:
> On Mon, Jan 09, 2017 at 11:55:41AM +0100, Paul Menzel wrote:

[…]

>> Also, is that just for MacBookPro11,3? The MCE for the Dell XPS13 looks
>> different from what I see, doesn’t it?
>
> Yes, yours is different. I'm still waiting for you to reply to Ashok's
> questions here:
>
> https://lkml.kernel.org/r/20170105011236.GA80100@otc-brkl-03

I see. I thought Daniel’s answered them. I’ll reply now.


Kind regards,

Paul

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web