Skip to contentSkip to conversation

RAS: Use MCE tracepoint for decoded MCEs

RFC PATCH In review Borislav Petkov 4 participants 39 messages Feb 28, 2012 – Mar 2, 2012

Subsystems (4)
lore
Share & tools
lore
#0 lore raw

cover RAS: Use MCE tracepoint for decoded MCEs

From: Borislav Petkov <[email protected]>

Hi all,

this is an initial, more or less serious attempt to collect decoded MCE info into a buffer and jettison it into userspace using the MCE tracepoint trace_mce_record(). This initial approach needs userspace to do

$ echo 1 > /sys/devices/system/ras/agent

and decoded MCE info gets collected into a buffer which enlarges itself to accomodate differently-sized error messages. Then, when decoding is finished, the tracepoint is called and the MCE info along with the decoded information lands in the ring buffer and at possible userspace consumers.

Also, the commit messages of the single patches contain additional info.

For example, the data looks like this:

mcegen.py-2318 [001] .N.. 580.902409: mce_record: [Hardware Error]: CPU:0 MC4_STATUS[Over|CE|-|PCC|AddrV|CECC]: 0xd604c00006080a41 MC4_ADDR: 0x0000000000000016
[Hardware Error]: Northbridge Error (node 0): DRAM ECC error detected on the NB.
[Hardware Error]: ERR_ADDR: 0x16 row: 0, channel: 0
[Hardware Error]: cache level: L1, mem/io: MEM, mem-tx: DWR, part-proc: RES (no timeout)
[Hardware Error]: CPU: 0, MCGc/s: 0/0, MC4: d604c00006080a41, ADDR/MISC: 0000000000000016/dead57ac1ba0babe, RIP: 00:<0000000000000000>, TSC: 0, TIME: 0)

       mcegen.py-2326  [001] .N..   598.795494: mce_record: [Hardware Error]: CPU:0 MC4_STATUS[Over|UE|MiscV|PCC|-|UECC]: 0xfa002000001c011b

[Hardware Error]: Northbridge Error (node 0): L3 ECC data cache error.
[Hardware Error]: cache level: L3/GEN, tx: GEN, mem-tx: RD
[Hardware Error]: CPU: 0, MCGc/s: 0/0, MC4: fa002000001c011b, ADDR/MISC: 0000000000000016/dead57ac1ba0babe, RIP: 00:<0000000000000000>, TSC: 0, TIME: 0)

mcegen.py-2343 [013] .N.. 619.620698: mce_record: [Hardware Error]: CPU:0 MC4_STATUS[-|UE|MiscV|PCC|-|UECC]: 0xba002100000f001b[HardwareError]: Northbridge Error (node 0): GART Table Walk data error.
[Hardware Error]: cache level: L3/GEN, tx: GEN
[Hardware Error]: CPU: 0, MCGc/s: 0/0, MC4: ba002100000f001b, ADDR/MISC: 0000000000000016/dead57ac1ba0babe, RIP: 00:<0000000000000000>, TSC: 0, TIME: 0)

As always, reviews and comments are welcome.

Thanks.

#1 lore raw

PATCH 1/3 mce: Add a msg string to the MCE tracepoint

From: Borislav Petkov <[email protected]>

The idea here is to pass an additional decoded MCE message through the tracepoint and into the ring buffer for userspace to consume. The designated consumers are RAS daemons and other tools collecting RAS information.

Drop unneeded fields while at it, thus saving some room in the ring buffer.

---
 arch/x86/kernel/cpu/mcheck/mce.c |    2 +-
 include/trace/events/mce.h       |   22 +++++++---------------
 2 files changed, 8 insertions(+), 16 deletions(-)
arch/x86/kernel/cpu/mcheck/mce.c +1 -1
index 5a11ae2e9e91..072e020ecaf3 100644
@@ -144,7 +144,7 @@ void mce_log(struct mce *mce)
 	int ret = 0;
 
 	/* Emit the trace record: */
-	trace_mce_record(mce);
+	trace_mce_record("", mce);
 
 	ret = atomic_notifier_call_chain(&x86_mce_decoder_chain, 0, mce);
 	if (ret == NOTIFY_STOP)
include/trace/events/mce.h +7 -15
index 4cbbcef6baa8..5fada9a40708 100644
@@ -10,11 +10,12 @@
 
 TRACE_EVENT(mce_record,
 
-	TP_PROTO(struct mce *m),
+	TP_PROTO(const char *msg, struct mce *m),
 
-	TP_ARGS(m),
+	TP_ARGS(msg, m),
 
 	TP_STRUCT__entry(
+		__string(	msg,		msg		)
 		__field(	u64,		mcgcap		)
 		__field(	u64,		mcgstatus	)
 		__field(	u64,		status		)
@@ -24,15 +25,12 @@ TRACE_EVENT(mce_record,
 		__field(	u64,		tsc		)
 		__field(	u64,		walltime	)
 		__field(	u32,		cpu		)
-		__field(	u32,		cpuid		)
-		__field(	u32,		apicid		)
-		__field(	u32,		socketid	)
 		__field(	u8,		cs		)
 		__field(	u8,		bank		)
-		__field(	u8,		cpuvendor	)
 	),
 
 	TP_fast_assign(
+		__assign_str(msg,	msg);
 		__entry->mcgcap		= m->mcgcap;
 		__entry->mcgstatus	= m->mcgstatus;
 		__entry->status		= m->status;
@@ -42,25 +40,19 @@ TRACE_EVENT(mce_record,
 		__entry->tsc		= m->tsc;
 		__entry->walltime	= m->time;
 		__entry->cpu		= m->extcpu;
-		__entry->cpuid		= m->cpuid;
-		__entry->apicid		= m->apicid;
-		__entry->socketid	= m->socketid;
 		__entry->cs		= m->cs;
 		__entry->bank		= m->bank;
-		__entry->cpuvendor	= m->cpuvendor;
 	),
 
-	TP_printk("CPU: %d, MCGc/s: %llx/%llx, MC%d: %016Lx, ADDR/MISC: %016Lx/%016Lx, RIP: %02x:<%016Lx>, TSC: %llx, PROCESSOR: %u:%x, TIME: %llu, SOCKET: %u, APIC: %x",
+	TP_printk("%s" HW_ERR "CPU: %d, MCGc/s: %llx/%llx, MC%d: %016Lx, ADDR/MISC: %016Lx/%016Lx, RIP: %02x:<%016Lx>, TSC: %llx, TIME: %llu)",
+		__get_str(msg),
 		__entry->cpu,
 		__entry->mcgcap, __entry->mcgstatus,
 		__entry->bank, __entry->status,
 		__entry->addr, __entry->misc,
 		__entry->cs, __entry->ip,
 		__entry->tsc,
-		__entry->cpuvendor, __entry->cpuid,
-		__entry->walltime,
-		__entry->socketid,
-		__entry->apicid)
+		__entry->walltime)
 );
 
 #endif /* _TRACE_MCE_H */
Signature · 1 line
1.7.8.rc0
#2 lore raw

Re: [PATCH 1/3] mce: Add a msg string to the MCE tracepoint

Quoted 6 lines · Borislav Petkov

From: Borislav Petkov <[email protected]>

The idea here is to pass an additional decoded MCE message through the tracepoint and into the ring buffer for userspace to consume. The designated consumers are RAS daemons and other tools collecting RAS information.

I could not catch the point... Why you need this msg field?

I think that all of information about the error is already packed in the record and that we can make a string from the bits in the record soon afterward. From my point of view it seems that what you are doing here is just consuming the ring buffer by repeating same contents in another format with dynamic length which might be short but otherwise could be too long.

And one more unacceptable point is that filling this msg field is expected to be done in machine check context where have many limitations in kernel's subsystems such as use of memory allocators.

Suggestion; How about having a kind of translator function for userland, e.g. an exported function named mce_record_to_msg()? Tool obtains raw data from the record in the tracepoint's ring buffer, and if it likes, optionally it can pass the record to the translator function to get some accomplished string.

Quoted 3 lines

Drop unneeded fields while at it, thus saving some room in the ring buffer.

Really unneeded and should be killed?

Thanks,
H.Seto

#3 lore raw

Re: [PATCH 1/3] mce: Add a msg string to the MCE tracepoint

Quoted 8 lines · Hidetoshi Seto
Quoted 6 lines · Borislav Petkov

From: Borislav Petkov <[email protected]>

The idea here is to pass an additional decoded MCE message through the tracepoint and into the ring buffer for userspace to consume. The designated consumers are RAS daemons and other tools collecting RAS information.

Quoted 8 lines

I could not catch the point... Why you need this msg field?

I think that all of information about the error is already packed in the record and that we can make a string from the bits in the record soon afterward. From my point of view it seems that what you are doing here is just consuming the ring buffer by repeating same contents in another format with dynamic length which might be short but otherwise could be too long.

Right, to answer your immediate question: we've already decoded the MCE so we carry that decoded info to userspace.

To address your indirect question: why aren't we using the MCE fields to decode the MCE in userspace? Well, this has been a long discussion already and one of the strong arguments for decoding hardware errors in the kernel is that the kernel simply knows its hardware better. Imagine a big server farm with heterogeneous hw configurations - if you get an MCE there you have to also have collected the hardware platform details so that you are able to decode it. If the kernel can do that for ya, you don't have to do anything!

Or the case where you get an uncorrectable error and the machine panics
- it is much more convenient to see the decoded error on the screen before the machine dies instead of some MCA register dumps which you have to jot down and go and decode them by hand.

Quoted 3 lines

And one more unacceptable point is that filling this msg field is expected to be done in machine check context where have many limitations in kernel's subsystems such as use of memory allocators.

Doh, I should've seen that, thanks to you and Tony for pointing that out.

Quoted 5 lines

Suggestion; How about having a kind of translator function for userland, e.g. an exported function named mce_record_to_msg()? Tool obtains raw data from the record in the tracepoint's ring buffer, and if it likes, optionally it can pass the record to the translator function to get some accomplished string.

Either that or I could simply allocate a large enough buffer from the get-go, as Tony suggests. I'll experiment with my MCE generation script and see how large a buffer can become.

Quoted 4 lines
Quoted 2 lines

Drop unneeded fields while at it, thus saving some room in the ring buffer.

Really unneeded and should be killed?

Right, so this is me suggesting to remove those because I don't see why we'd need them, I'm expecting other people to come and say either "Boris, no no, this is needed in... " or "Yeah, go ahead and remove them, no one uses those." So feel free to argue either way.

Thanks.

Signature · 8 lines
Regards/Gruss,
Boris.

Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
GM: Alberto Bozzo
Reg: Dornach, Landkreis Muenchen
HRB Nr. 43632 WEEE Registernr: 129 19551
#4 lore raw

Re: [PATCH 1/3] mce: Add a msg string to the MCE tracepoint

Em 29-02-2012 07:10, Borislav Petkov escreveu:

Quoted 9 lines · Hidetoshi Seto
Quoted 8 lines
Quoted 6 lines · Borislav Petkov

From: Borislav Petkov <[email protected]>

The idea here is to pass an additional decoded MCE message through the tracepoint and into the ring buffer for userspace to consume. The designated consumers are RAS daemons and other tools collecting RAS information.

Quoted 8 lines
Quoted 8 lines

I could not catch the point... Why you need this msg field?

I think that all of information about the error is already packed in the record and that we can make a string from the bits in the record soon afterward. From my point of view it seems that what you are doing here is just consuming the ring buffer by repeating same contents in another format with dynamic length which might be short but otherwise could be too long.

Not all information is packed in the record. The record packs only what it is inside the MCE registers. However, for certain errors, it is needed to parse other hardware registers to decode the error (for example, on Sandy Bridge, the MCE registers don't contain the affected dimms).

Quoted 30 lines

Right, to answer your immediate question: we've already decoded the MCE so we carry that decoded info to userspace.

To address your indirect question: why aren't we using the MCE fields to decode the MCE in userspace? Well, this has been a long discussion already and one of the strong arguments for decoding hardware errors in the kernel is that the kernel simply knows its hardware better. Imagine a big server farm with heterogeneous hw configurations - if you get an MCE there you have to also have collected the hardware platform details so that you are able to decode it. If the kernel can do that for ya, you don't have to do anything!

Or the case where you get an uncorrectable error and the machine panics
- it is much more convenient to see the decoded error on the screen before the machine dies instead of some MCA register dumps which you have to jot down and go and decode them by hand.

Quoted 3 lines

And one more unacceptable point is that filling this msg field is expected to be done in machine check context where have many limitations in kernel's subsystems such as use of memory allocators.

Doh, I should've seen that, thanks to you and Tony for pointing that out.

Quoted 5 lines

Suggestion; How about having a kind of translator function for userland, e.g. an exported function named mce_record_to_msg()? Tool obtains raw data from the record in the tracepoint's ring buffer, and if it likes, optionally it can pass the record to the translator function to get some accomplished string.

Quoted 3 lines

Either that or I could simply allocate a large enough buffer from the get-go, as Tony suggests. I'll experiment with my MCE generation script and see how large a buffer can become.

Just allocate one page. 4096 should be enough even for the most hungry needs.

Quoted 9 lines
Quoted 4 lines
Quoted 2 lines

Drop unneeded fields while at it, thus saving some room in the ring buffer.

Really unneeded and should be killed?

Right, so this is me suggesting to remove those because I don't see why we'd need them, I'm expecting other people to come and say either "Boris, no no, this is needed in... " or "Yeah, go ahead and remove them, no one uses those." So feel free to argue either way.

IMHO, before removing those fields, it would be better to first implement what is there at the mcelog userspace parser for the Intel machines into kernelspace (or to look into its source code), and check what registers aren't used by either AMD 64 MCE decoder or by the Intel MCE decoder.

Tony,

Is there anyone at Intel working on porting it to kernelspace?

Regards,
Mauro

#5 lore raw

Re: [PATCH 1/3] mce: Add a msg string to the MCE tracepoint

Quoted 4 lines · Mauro Carvalho Chehab

Not all information is packed in the record. The record packs only what it is inside the MCE registers. However, for certain errors, it is needed to parse other hardware registers to decode the error (for example, on Sandy Bridge, the MCE registers don't contain the affected dimms).

If SB is not using MCA to report the error, it should use either a generic TP like the trace_hw_error() example I gave last week, or rather a TP which matches the hw registers of the reporting hardware scheme.

Signature · 8 lines
Regards/Gruss,
Boris.

Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
GM: Alberto Bozzo
Reg: Dornach, Landkreis Muenchen
HRB Nr. 43632 WEEE Registernr: 129 19551
#6 lore raw

Re: [PATCH 1/3] mce: Add a msg string to the MCE tracepoint

Em 29-02-2012 09:19, Borislav Petkov escreveu:

Quoted 9 lines · Mauro Carvalho Chehab
Quoted 4 lines

Not all information is packed in the record. The record packs only what it is inside the MCE registers. However, for certain errors, it is needed to parse other hardware registers to decode the error (for example, on Sandy Bridge, the MCE registers don't contain the affected dimms).

If SB is not using MCA to report the error, it should use either a generic TP like the trace_hw_error() example I gave last week, or rather a TP which matches the hw registers of the reporting hardware scheme.

This is not what I said. On intel, both SB and Nehalem use MCA to report errors. Older chipsets don't use MCA.

However, there's a fundamental difference between SB and Nehalem:

- on Nehalem, the MCE status register encodes not only the error message; it also encodes the DIMM that generated the error. So, it is possible to completely decode the error on userspace, using only the MCE registers.

- on SB, the MCE status register only has the error message. In order to get the DIMM location, the driver needs to parse the registers that describe how the DIMM's are organized (this is spread on dozens of PCI devices, and 200+ registers), and how they're interlaced, in order to convert the error address reported by the MCA into a DIMM location.

So, just storing the values of the MCE registers is not enough to completely decode the error.

Regards,
Mauro

#7 lore raw

Re: [PATCH 1/3] mce: Add a msg string to the MCE tracepoint

Quoted 11 lines · Mauro Carvalho Chehab

Em 29-02-2012 09:19, Borislav Petkov escreveu:

Quoted 9 lines · Mauro Carvalho Chehab
Quoted 4 lines

Not all information is packed in the record. The record packs only what it is inside the MCE registers. However, for certain errors, it is needed to parse other hardware registers to decode the error (for example, on Sandy Bridge, the MCE registers don't contain the affected dimms).

If SB is not using MCA to report the error, it should use either a generic TP like the trace_hw_error() example I gave last week, or rather a TP which matches the hw registers of the reporting hardware scheme.

Quoted 8 lines

This is not what I said. On intel, both SB and Nehalem use MCA to report errors. Older chipsets don't use MCA.

However, there's a fundamental difference between SB and Nehalem:

- on Nehalem, the MCE status register encodes not only the error message; it also encodes the DIMM that generated the error. So, it is possible to completely decode the error on userspace, using only the MCE registers.

Well, depending on what Tony wants to do there, either decode the error in the kernel and pass it on with the 'msg' arg or do the whole decoding in userspace.

Quoted 5 lines

- on SB, the MCE status register only has the error message. In order to get the DIMM location, the driver needs to parse the registers that describe how the DIMM's are organized (this is spread on dozens of PCI devices, and 200+ registers), and how they're interlaced, in order to convert the error address reported by the MCA into a DIMM location.

As I already said, amd64_edac does a similar thing does already so I don't see any difference in the solutions there: decode to the DIMM and pass the info through 'msg'.

Signature · 8 lines
Regards/Gruss,
Boris.

Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
GM: Alberto Bozzo
Reg: Dornach, Landkreis Muenchen
HRB Nr. 43632 WEEE Registernr: 129 19551
maintainer
#8 lore raw

RE: [PATCH 1/3] mce: Add a msg string to the MCE tracepoint

Quoted 7 lines
Quoted 3 lines

- on Nehalem, the MCE status register encodes not only the error message; it also encodes the DIMM that generated the error. So, it is possible to completely decode the error on userspace, using only the MCE registers.

Well, depending on what Tony wants to do there, either decode the error in the kernel and pass it on with the 'msg' arg or do the whole decoding in userspace.

For best results - we should decode right away in the kernel. Decoding later requires that we carry a lot of additional information about the system configuration at the time of the error. Consider the case of a hard error (either fatal or recoverable). If the system reboots, then the DIMM with the error should fail self test - and thus be mapped out of the system. If the error analyzer doesn't realize that this has happened, it will be very confused. Even if it does notice - the Sandy bridge decoder won't be able to check that the right DIMM was mapped out (since the configuration registers it reads to map addresses to DIMMS will now be set for the new configuration, with different mappings).

-Tony

#9 lore raw

Re: [PATCH 1/3] mce: Add a msg string to the MCE tracepoint

Quoted 8 lines · Tony
Quoted 7 lines
Quoted 3 lines

- on Nehalem, the MCE status register encodes not only the error message; it also encodes the DIMM that generated the error. So, it is possible to completely decode the error on userspace, using only the MCE registers.

Well, depending on what Tony wants to do there, either decode the error in the kernel and pass it on with the 'msg' arg or do the whole decoding in userspace.

Quoted 10 lines

For best results - we should decode right away in the kernel. Decoding later requires that we carry a lot of additional information about the system configuration at the time of the error. Consider the case of a hard error (either fatal or recoverable). If the system reboots, then the DIMM with the error should fail self test - and thus be mapped out of the system. If the error analyzer doesn't realize that this has happened, it will be very confused. Even if it does notice - the Sandy bridge decoder won't be able to check that the right DIMM was mapped out (since the configuration registers it reads to map addresses to DIMMS will now be set for the new configuration, with different mappings).

Absolutely!

And also, you've lost the moment the system reboots and you haven't gotten all the info needed for decoding the error.

Signature · 8 lines
Regards/Gruss,
Boris.

Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
GM: Alberto Bozzo
Reg: Dornach, Landkreis Muenchen
HRB Nr. 43632 WEEE Registernr: 129 19551
#10 lore raw

Re: [PATCH 1/3] mce: Add a msg string to the MCE tracepoint

In reply to Borislav Petkov
Quoted 11 lines · Borislav Petkov
Quoted 6 lines · Mauro Carvalho Chehab

Em 29-02-2012 09:19, Borislav Petkov escreveu:
- on SB, the MCE status register only has the error message. In order to get the DIMM location, the driver needs to parse the registers that describe how the DIMM's are organized (this is spread on dozens of PCI devices, and 200+ registers), and how they're interlaced, in order to convert the error address reported by the MCA into a DIMM location.

As I already said, amd64_edac does a similar thing does already so I don't see any difference in the solutions there: decode to the DIMM and pass the info through 'msg'.

My concern is; on Sandy Bridge, is it safe to gather info about the DIMM location in/from machine check context in a reasonable time span? I know that for corrected errors which is handled in normal context it is safe to refer the vast PCI configuration space...

Or is it really possible to determine the erroneous DIMM location from OS? It looks like that how to get the location is highly depending on the hardware, processor's vendor/family/model and firmware configuration etc.. Even if OS tells me "please replace memory seated on slot#3 at node#5" or so, I'm not sure whether these numbers are consistent over reboot if there are some hot-plugged node and/or memory. Order of numbering can be changed by how firmware enumerate ACPI namespace or so... Actually in these days we usually use firmware's system event log to determine which module should be replaced, assuming that firmware knows hardware better than OSes running on that machine.

Getting back to the "msg" I think it is not necessary if it does not contain any new data which is not available in the mce_record today. If you just want to add field about physical memory location, I think string "msg" is not only way to do so.

Thanks,
H.Seto

#11 lore raw

Re: [PATCH 1/3] mce: Add a msg string to the MCE tracepoint

Quoted 9 lines · Hidetoshi Seto
Quoted 8 lines · Borislav Petkov
Quoted 6 lines · Mauro Carvalho Chehab

Em 29-02-2012 09:19, Borislav Petkov escreveu:
- on SB, the MCE status register only has the error message. In order to get the DIMM location, the driver needs to parse the registers that describe how the DIMM's are organized (this is spread on dozens of PCI devices, and 200+ registers), and how they're interlaced, in order to convert the error address reported by the MCA into a DIMM location.

Quoted 6 lines
Quoted 3 lines

As I already said, amd64_edac does a similar thing does already so I don't see any difference in the solutions there: decode to the DIMM and pass the info through 'msg'.

My concern is; on Sandy Bridge, is it safe to gather info about the DIMM location in/from machine check context in a reasonable time span?

Well, what amd64_edac does is "buffer" the required lookup info so whenever you get an error, you simply lookup the channel and chip select
- all ops which can be done in atomic context.

[..]

Quoted 4 lines

Getting back to the "msg" I think it is not necessary if it does not contain any new data which is not available in the mce_record today. If you just want to add field about physical memory location, I think string "msg" is not only way to do so.

No, currently, the mce_record contains the following, for example:

CPU: 0, MCGc/s: 0/0, MC4: d604c00006080a41, ADDR/MISC: 0000000000000016/dead57ac1ba0babe, RIP: 00:<0000000000000000>, TSC: 0, TIME: 0

With the decoded info added, it becomes:

[Hardware Error]: CPU:0 MC4_STATUS[Over|CE|-|PCC|AddrV|CECC]: 0xd604c00006080a41 MC4_ADDR: 0x0000000000000016
[Hardware Error]: Northbridge Error (node 0): DRAM ECC error detected on the NB.
[Hardware Error]: ERR_ADDR: 0x16 row: 0, channel: 0
[Hardware Error]: cache level: L1, mem/io: MEM, mem-tx: DWR, part-proc: RES (no timeout)
[Hardware Error]: CPU: 0, MCGc/s: 0/0, MC4: d604c00006080a41, ADDR/MISC: 0000000000000016/dead57ac1ba0babe, RIP: 00:<0000000000000000>, TSC: 0, TIME: 0)

where the ERR_ADDR line comes from amd64_edac looking up the error address.

This way, you get all the info needed to understand what the MCi_STATUS of this MCE is telling you without any APM searching you'd normally have to do to understand what each field means.

HTH.

Signature · 8 lines
Regards/Gruss,
Boris.

Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
GM: Alberto Bozzo
Reg: Dornach, Landkreis Muenchen
HRB Nr. 43632 WEEE Registernr: 129 19551
maintainer
#12 lore raw

RE: [PATCH 1/3] mce: Add a msg string to the MCE tracepoint

Quoted 6 lines
Quoted 2 lines

My concern is; on Sandy Bridge, is it safe to gather info about the DIMM location in/from machine check context in a reasonable time span?

Well, what amd64_edac does is "buffer" the required lookup info so whenever you get an error, you simply lookup the channel and chip select
- all ops which can be done in atomic context.

Yes - we could pre-read all the config space registers ahead of time and save them in memory (none of the values should change - except if the platform supports hot-plug for memory). Total is only a few Kbytes. Then decode in machine check context is both safe, and fast.

-Tony

#13 lore raw

Re: [PATCH 1/3] mce: Add a msg string to the MCE tracepoint

Quoted 11 lines · Tony
Quoted 6 lines
Quoted 2 lines

My concern is; on Sandy Bridge, is it safe to gather info about the DIMM location in/from machine check context in a reasonable time span?

Well, what amd64_edac does is "buffer" the required lookup info so whenever you get an error, you simply lookup the channel and chip select
- all ops which can be done in atomic context.

Yes - we could pre-read all the config space registers ahead of time and save them in memory (none of the values should change - except if the platform supports hot-plug for memory). Total is only a few Kbytes. Then decode in machine check context is both safe, and fast.

To sort out my thought:

- First of all, OS gathers info about physical location of DIMMs from DMI/ACPI/PCI etc., before enabling MCE mechanism.
- Make a kind of "physical memory location table" on memory buffer, to ease mapping a physical address to the location of a DIMM module and/or chip which have the memory cell pointed by the address.
- It would be better to have a well organized table rather than having a raw copy of config space etc.
- Likewise it will also nice if we can map logical processor numbers to the location of physical sockets on motherboard.
- Happy if user can refer the table via sysfs.
- Allow updating the table if the platform supports hot-plug.
- Once MCE is enabled, handler can refer the table on memory to determine an erroneous device which should be replaced.

This storyline up to here is reasonable and acceptable, I think.

Then now it is clear that the last point where I feel uneasy about is putting a string into the ring buffer instead of binary bits like index of location table. Please use binary (or "binary + string") to tell the error location to userland.

Thanks,
H.Seto

#14 lore raw

Re: [PATCH 1/3] mce: Add a msg string to the MCE tracepoint

Em 02-03-2012 01:02, Hidetoshi Seto escreveu:

Quoted 15 lines
Quoted 11 lines · Tony
Quoted 6 lines
Quoted 2 lines

My concern is; on Sandy Bridge, is it safe to gather info about the DIMM location in/from machine check context in a reasonable time span?

Well, what amd64_edac does is "buffer" the required lookup info so whenever you get an error, you simply lookup the channel and chip select
- all ops which can be done in atomic context.

Yes - we could pre-read all the config space registers ahead of time and save them in memory (none of the values should change - except if the platform supports hot-plug for memory). Total is only a few Kbytes. Then decode in machine check context is both safe, and fast.

To sort out my thought:

Quoted 9 lines

- First of all, OS gathers info about physical location of DIMMs from DMI/ACPI/PCI etc., before enabling MCE mechanism.
- Make a kind of "physical memory location table" on memory buffer, to ease mapping a physical address to the location of a DIMM module and/or chip which have the memory cell pointed by the address.
- It would be better to have a well organized table rather than having a raw copy of config space etc.
- Likewise it will also nice if we can map logical processor numbers to the location of physical sockets on motherboard.

If you take a look at the SB driver, my original idea was do to it: there's a routine there (get_memory_layout) that parses all PCI registers used to describe the memory.

On my preliminary versions, I was storing their contents into the private data.

About the same logic is duplicated at get_memory_error_data(), which is the function that actually parses the memory error. In the past, it was using the cached data.

I ended by removing the cache due to a few reasons:

- how to detect and re-run the get_memory_error_data() when memory is
hot-plugged?

- I had to re-write the logic several times, as I was discovering more
things about the SB. Writing a more clever/faster logic for storing the data for each new change were taking too much time;

- While I never tried to measure the performance of that logic, I
suspect that the performance benefits may not justify the cache;

- It was more important to have something work, than a very optimized
driver;

- Paraphrasing Blaise Pascal, "I have made the decoding logic slower, because I have not had the time to make it faster".

Feel free to write some patches to optimize it.

Quoted 1 line

- Happy if user can refer the table via sysfs.

Representing the logic via sysfs is not easy, due to interlacing. Sandy Bridge has several ways to interlace memories, among their channels, plus arranging the address on NUMA/non-NUMA mode. The way it is represented internally is complex to understand.

I would love to find a way to tell the user that addresses between x and y belongs to DIMM z, but, due to certain interlace modes, this would be a very complex task.

Quoted 1 line

- Allow updating the table if the platform supports hot-plug.

This change is not trivial. The EDAC core requires to allocate the memory at the beginning.

Not sure if it is easy to support hot-plug there. It will likely need to receive a hot-plug event, and call some routine that would call a memory discovery callback.

If is there any MCA event for memory hot-plug, it sounds feasible.

Quoted 2 lines

- Once MCE is enabled, handler can refer the table on memory to determine an erroneous device which should be replaced.

MCE enabled is a requirement for the SB driver:

config EDAC_SBRIDGE

	tristate "Intel Sandy-Bridge Integrated MC"
	depends on EDAC_MM_EDAC && PCI && X86_64 && X86_MCE_INTEL
Quoted 6 lines

This storyline up to here is reasonable and acceptable, I think.

Then now it is clear that the last point where I feel uneasy about is putting a string into the ring buffer instead of binary bits like index of location table. Please use binary (or "binary + string") to tell the error location to userland.

I agree with you that merging everything into a single string is very bad, and this is the main point we're discussing.

Let me make a summary of the status.

As you know, there are two types of location (silkscreen label and the location itself, as recognized internally by the hardware.

I'm fixing the EDAC core, as it is currently not able to handle the real location, and requires that everything to be converted into a per-rank memory description (csrow/channel), with doesn't work for most non-legacy drivers.

The EDAC core provides a way to convert from location into the silkscreen label.

Basically, there's a sysfs node with the dimm label, with is filled at the machine boot time, via the edac utils.

Depending on the driver, the location, can actually be:
branch/channel/slot => DIMM
channel/slot => DIMM
csrow/channel => DIMM rank

The patch series I'm working add a way for the driver to tell the core what of the above applies, and, when an error occurs, it allows the driver to tell exactly on what location the error happened [1].

[1] With the upstream version, the "location" is a meaningless, fake information, on all Intel non-legacy drivers (i5xxx edac drivers, i7core_edac, sb_edac).

It is possible to use the internal representation I've added on it, storing them on a binary way, and exporting as such to userspace.

Borislav is arguing against it, for MCA based events, as such kind of representation would require a memory-specific tracepoint.

For non-MCA events, there's not much problems, as there aren't many types of errors.

In the end, we have a few alternatives, and no consensus was reached with regards to it.

The alternatives for MCA errors are either to use a single tracepoint for all hardware types, forcing the driver/core to convert the internal binary representation into a string, or to have multiple traces for hardware errors.

Three different strategies were actually proposed:

1) Everything goes into a single string (Borislav's patchset [2]);

2) A memory error specific tracepoint, plus other tracepoints for other types of errors (my patch [3]).

3) An hybrid alternative, splitting the severity, location and silk_screen label from the error message (my proposal, no patch for it yet [4]). As the location there should be independent of the type of the hardware, it has to be a string.

[2] https://lkml.org/lkml/2012/2/28/280

[3] http://git.kernel.org/?p=linux/kernel/git/mchehab/linux-edac.git;a=commitdiff;h=4eb2a29419c1fefd76c8dbcd308b84a4b52faf4d

[4] https://lkml.org/lkml/2012/2/29/187

Looking at the existing code, what happens with regards to location is that there are currently two types of location:

a) a memory location (could be 2 or 3 integers);

b) a CPU location (one integer).

Internally, for each trace, a printk string should be defined, like:

TP_printk("%s error %s on \"%s\" (location: %s)",

	  __get_str(msg),
	  __get_str(label),
	  __get_str(location))

It is easy to write such macro for strings, but doing it for a variable-length location field would likely require more work.

A binary way to represent the memory location would be as an array, whose size is specified by a parameter, or with an structure that would have a location name, value pair, like:

struct {
	char *name;
	int value;
} key_value_pair mem_location[] = {
	{ "csrow", 0},
	{ "channel", 1},
};

I don't think that trace as some trace macro that would allow to print something like that, currently. Not sure how easy/difficult do add support for it at perf, on both kernelspace and userspace.

Regards,
Mauro

maintainer
#15 lore raw

RE: [PATCH 1/3] mce: Add a msg string to the MCE tracepoint

In reply to Hidetoshi Seto

- It would be better to have a well organized table rather than having a raw copy of config space etc.

I think it would be challenging to come up with a table format that can handle all the address mapping options that SNB memory controller can set up.

E.g. I have a very simple setup with all DIMM slots populated with the same size/speed DIMMS. If I pick a page at random and lookup which DIMM slot each cache line translates to, I'll see a simple pattern:

	A1 B1 C1 D1 A1 B1 C1 D1 ...

as lines are sequentially placed across all four channels. Looking at other pages I'll sometimes see that pattern, and sometimes see that the other set of DIMM slots was chosen

	A2 B2 C2 D2 A2 B2 C2 D2 ...

The decision to use slot1 vs. slot2 DIMMs on the channel may depend on the XOR of a couple of bit fields from the full physical address.

But life might not be that simple. I can fill DIMM slots with different size/speed DIMMs - which will affect the interleave options. About the only restriction on placement is that you cannot fill a "2" slot on a channel unless you have a DIMM present in the "1" slot.

Then there are a handful of BIOS options that may change things - e.g. you could turn off NUMA mode - then you'll find that pages are striped across DIMMs from all nodes.

Ultimately your table generation code would have to take into account all of the mapping possibilities in the h/w ... at which point it would be far simpler to just write a routine that performs the same translation that the h/w does (based on the values from those memory controller config registers).

-Tony

maintainer
#16 lore raw

RE: [PATCH 1/3] mce: Add a msg string to the MCE tracepoint

In reply to Mauro Carvalho Chehab
Quoted 8 lines

IMHO, before removing those fields, it would be better to first implement what is there at the mcelog userspace parser for the Intel machines into kernelspace (or to look into its source code), and check what registers aren't used by either AMD 64 MCE decoder or by the Intel MCE decoder.

Tony,

Is there anyone at Intel working on porting it to kernelspace?

The mcelog code just looks at model specific fields in MCi_STATUS and MCi_MISC. We could move it to the kernel - but I don't see much value in doing so. In this case all the information we need is carried in status/misc - so as long as we keep all of those bits (and the cpu family/model) we can safely decode/analyze later.

-Tony

#17 lore raw

Re: [PATCH 1/3] mce: Add a msg string to the MCE tracepoint

Em 29-02-2012 14:20, Luck, Tony escreveu:

Quoted 5 lines
Quoted 5 lines

IMHO, before removing those fields, it would be better to first implement what is there at the mcelog userspace parser for the Intel machines into kernelspace (or to look into its source code), and check what registers aren't used by either AMD 64 MCE decoder or by the Intel MCE decoder.

Quoted 7 lines
Quoted 3 lines

Tony,

Is there anyone at Intel working on porting it to kernelspace?

The mcelog code just looks at model specific fields in MCi_STATUS and MCi_MISC. We could move it to the kernel - but I don't see much value in doing so.

I see a few reasons:

- it would be consistent with what's being done at AMD. So, all x86 arch will report errors at the same way;

- userspace won't need to run an extra daemon/tool to decode the errors;

- fatal errors won't be lost (well, in fact, you have there already a parser for fatal errors. Not sure if all possible fatal errors are covered here, nor what else is needed, as you have already there part of mcelog decoder);

- a single place to maintain, when new cpu families are added;

- it makes easier to centralize the hardware error information, as there's no need to enrich the error on userspace.

Quoted 3 lines

In this case all the information we need
is carried in status/misc - so as long as we keep all of those bits (and the cpu family/model) we can safely decode/analyze later.

Hmm... the mcelog tool opens the /proc/cpuinfo:

$ grep cpuinfo *
mcelog.c:	f = fopen("/proc/cpuinfo","r");
mcelog.c:			Eprintf("warning: Cannot parse /proc/cpuinfo\n"); 
mcelog.c:		Eprintf("warning: Cannot open /proc/cpuinfo\n");
tsc.c:	asprintf(&fn, "/sys/devices/system/cpu/cpu%d/cpufreq/cpuinfo_max_freq", cpu);
tsc.c:		/* /sys exists, but no cpufreq -- use value from cpuinfo */

That probably means that the needed cpu family/model info is not (or may not) be stored at the MCE structure.

Quoted 2 lines

-Tony

maintainer
#18 lore raw

RE: [PATCH 1/3] mce: Add a msg string to the MCE tracepoint

Quoted 2 lines

That probably means that the needed cpu family/model info is not (or may not) be stored at the MCE structure.

mcelog can run as a daemon, or one-off to process reports from earlier (or even from other systems). It picks up cpu/family from /proc/cpuinfo - but can also take a command line argument. These get checked against the cpuid field in the structure that is read from /dev/mcelog (or decoded from a console log with the --ascii option).

In some cases the checks may be redundant (e.g. running in daemon mode, there is no way that /dev/mcelog will not have a cpuid that matches the cpu that you are running on!) But better to have too many checks than too few.

-Tony

#19 lore raw

Re: [PATCH 1/3] mce: Add a msg string to the MCE tracepoint

In reply to Borislav Petkov

Em 28-02-2012 13:11, Borislav Petkov escreveu:

Quoted 34 lines

From: Borislav Petkov <[email protected]>

The idea here is to pass an additional decoded MCE message through the tracepoint and into the ring buffer for userspace to consume. The designated consumers are RAS daemons and other tools collecting RAS information.

Drop unneeded fields while at it, thus saving some room in the ring buffer.

---
 arch/x86/kernel/cpu/mcheck/mce.c |    2 +-
 include/trace/events/mce.h       |   22 +++++++---------------
 2 files changed, 8 insertions(+), 16 deletions(-)
arch/x86/kernel/cpu/mcheck/mce.c +1 -1
index 5a11ae2e9e91..072e020ecaf3 100644
@@ -144,7 +144,7 @@ void mce_log(struct mce *mce)
 	int ret = 0;
 
 	/* Emit the trace record: */
-	trace_mce_record(mce);
+	trace_mce_record("", mce);
 
 	ret = atomic_notifier_call_chain(&x86_mce_decoder_chain, 0, mce);
 	if (ret == NOTIFY_STOP)
include/trace/events/mce.h +0 -0
index 4cbbcef6baa8..5fada9a40708 100644

include/trace/events/mce.h line 13

Quoted 5 lines
include/trace/events/mce.h +1 -1
@@ -10,11 +10,12 @@
 
 TRACE_EVENT(mce_record,
 
-	TP_PROTO(struct mce *m),
+	TP_PROTO(const char *msg, struct mce *m),

While is very easy to fold everything into a single errror message field, doing that makes harder for userspace to break the string into the components, as already discussed.

The big advantage of using perf, instead of printk is that it allows to pass the fields into a binary form, instead of passing a single string to userspace. We should use this feature, otherwise, there's no sense on using perf at all.

Also, even if we agree on some string format to make parsing easier, we're not good on preserving the kernel-userspace API for string messages, so regressions can (and will likely) be introduced by a simple change at the "msg" field.

On a complex environment, the network farm is monitored by a Network Management System. On such systems, there are some fields that are very relevant:
- how critical is the error;
- the location of the error;
- the error message.

So, we should, at least, break it into 3 fields. Yet, for location, there are two ways of reporting it: The silkscreen label of the affected component (a CPU socket label, a memory slot label, a PCI slot label, etc), and the location of the affected element, which generally more granular, like the cache level, the memory bank, etc.

What I'm saying is that, we should, at least, add 4 fields there, instead of one:

- severity
- location
- silkscreen_label
- error_msg

The severity should be an enum type.

include/trace/events/mce.h TRACE_EVENT(mce_record, · line 27

Quoted 13 lines
include/trace/events/mce.h +2 -1
-	TP_ARGS(m),
+	TP_ARGS(msg, m),
 
 	TP_STRUCT__entry(
+		__string(	msg,		msg		)
 		__field(	u64,		mcgcap		)
 		__field(	u64,		mcgstatus	)
 		__field(	u64,		status		)
@@ -24,15 +25,12 @@ TRACE_EVENT(mce_record,
 		__field(	u64,		tsc		)
 		__field(	u64,		walltime	)
 		__field(	u32,		cpu		)

This one can be merged into the location field, for the errors where it is pertinent.

include/trace/events/mce.h TRACE_EVENT(mce_record, · line 28

Quoted 2 lines
include/trace/events/mce.h +0 -2
-		__field(	u32,		cpuid		)
-		__field(	u32,		apicid		)

It seems safe to get rid of them.

include/trace/events/mce.h TRACE_EVENT(mce_record, · line 29

Quoted 1 line
include/trace/events/mce.h +0 -1
-		__field(	u32,		socketid	)

For CPU errors, the socket ID is the location of the error. This is probably the most important "ID" field, as it represents the component that may need to be replaced.

Yet, by adding a silkscreen_label field, it seems safe to get rid of it.

include/trace/events/mce.h TRACE_EVENT(mce_record, · line 32

Quoted 3 lines
include/trace/events/mce.h +0 -1
 		__field(	u8,		cs		)
 		__field(	u8,		bank		)
-		__field(	u8,		cpuvendor	)

Assuming that some daemon would collect those errors on the monitored machine and pass them to another machine that will store it and parse (this is a typical scenario for monitored environments), I don't think we can get rid of the cpuvendor, as this field is required to decode the MCE registers. It is likely that we need to add also the CPU family and step here, as the MCE decoding depends on them.

On the other hand, the userspace daemon can easily get it by parsing /proc/cpuinfo.

So, it is probably ok to remove it.

Quoted 36 lines
include/trace/events/mce.h +4 -9
 	),
 
 	TP_fast_assign(
+		__assign_str(msg,	msg);
 		__entry->mcgcap		= m->mcgcap;
 		__entry->mcgstatus	= m->mcgstatus;
 		__entry->status		= m->status;
@@ -42,25 +40,19 @@ TRACE_EVENT(mce_record,
 		__entry->tsc		= m->tsc;
 		__entry->walltime	= m->time;
 		__entry->cpu		= m->extcpu;
-		__entry->cpuid		= m->cpuid;
-		__entry->apicid		= m->apicid;
-		__entry->socketid	= m->socketid;
 		__entry->cs		= m->cs;
 		__entry->bank		= m->bank;
-		__entry->cpuvendor	= m->cpuvendor;
 	),
 
-	TP_printk("CPU: %d, MCGc/s: %llx/%llx, MC%d: %016Lx, ADDR/MISC: %016Lx/%016Lx, RIP: %02x:<%016Lx>, TSC: %llx, PROCESSOR: %u:%x, TIME: %llu, SOCKET: %u, APIC: %x",
+	TP_printk("%s" HW_ERR "CPU: %d, MCGc/s: %llx/%llx, MC%d: %016Lx, ADDR/MISC: %016Lx/%016Lx, RIP: %02x:<%016Lx>, TSC: %llx, TIME: %llu)",
+		__get_str(msg),
 		__entry->cpu,
 		__entry->mcgcap, __entry->mcgstatus,
 		__entry->bank, __entry->status,
 		__entry->addr, __entry->misc,
 		__entry->cs, __entry->ip,
 		__entry->tsc,
-		__entry->cpuvendor, __entry->cpuid,
-		__entry->walltime,
-		__entry->socketid,
-		__entry->apicid)
+		__entry->walltime)
 );
 
 #endif /* _TRACE_MCE_H */
#20 lore raw

Re: [PATCH 1/3] mce: Add a msg string to the MCE tracepoint

On Wed, Feb 29, 2012 at 09:52:33AM -0300, Mauro Carvalho Chehab wrote:

[snip a bunch of crap I'm tired of correcting again, for the 100th time]

Quoted 4 lines

What I'm saying is that, we should, at least, add 4 fields there, instead of one:

- severity
- location

I've already explained to you why this format doesn't fit the MCE tracepoint - go and read my replies to you again.

Quoted 1 line

- silkscreen_label

ditto.

Quoted 1 line

- error_msg

[ snip more bullshit ]

Signature · 8 lines
Regards/Gruss,
Boris.

Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
GM: Alberto Bozzo
Reg: Dornach, Landkreis Muenchen
HRB Nr. 43632 WEEE Registernr: 129 19551
#21 lore raw

Re: [PATCH 1/3] mce: Add a msg string to the MCE tracepoint

Em 29-02-2012 10:45, Borislav Petkov escreveu:

Quoted 11 lines · Mauro Carvalho Chehab

[snip a bunch of crap I'm tired of correcting again, for the 100th time]

Quoted 4 lines

What I'm saying is that, we should, at least, add 4 fields there, instead of one:

- severity
- location

I've already explained to you why this format doesn't fit the MCE tracepoint - go and read my replies to you again.

No, you didn't. Every time i touch on this point, you just say that it doesn't fit without giving any explanation why not.

The severity is there, even on your patches, but encoded as string:

+#define PR_EMERG BIT(0)
+#define PR_WARNING BIT(1)

There are, actually, 3 variants, on Intel MCA:

- corrected error;
- uncorrected error (bit 61 of status);
- uncorrected, recoverable error (bit 61 + bit 56 of status).

The location is also there encoded at the MCA. It is specific to the type of the error, but all errors happen somewhere: a CPU, a BUS, a memory, ...

Quoted 3 lines
Quoted 1 line

- silkscreen_label

ditto.

ditto: all errors happen somewhere. The silkscreen label is the first replaceable unit to be replaced by the user, in order to get rid of the error.

Quoted 5 lines
Quoted 2 lines

- error_msg

[ snip more bullshit ]

Running away from this discussion won't help at all.

Regards,
Mauro

#22 lore raw

Re: [PATCH 1/3] mce: Add a msg string to the MCE tracepoint

Quoted 2 lines · Mauro Carvalho Chehab

No, you didn't. Every time i touch on this point, you just say that it doesn't fit without giving any explanation why not.

Let me explain it to you one _last_ time:

- severity: No real need for it. If the error is severe enough, the kernel handles automatically, i.e. memory poisoning and recovery. In all the other cases it is not severe enough.

- location: this is contained in the ->cpu field.

- silkscreen_label: <sarcasm> yeah, I'm getting a, say, a Data Cache error during an L1 linefill from L2, what the f*ck does the silkscreen label mean for such an error?! Well, nobody knows wtf it means!</sarcasm>

- error_msg: already there in my patch.

So go and read and try _understanding_ this before you come back with more crap, ok?

Quoted 1 line

Running away from this discussion won't help at all.

Not running away - trying not to waste time with bullshit.

Signature · 8 lines
Regards/Gruss,
Boris.

Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
GM: Alberto Bozzo
Reg: Dornach, Landkreis Muenchen
HRB Nr. 43632 WEEE Registernr: 129 19551
maintainer
#23 lore raw

RE: [PATCH 1/3] mce: Add a msg string to the MCE tracepoint

Quoted 3 lines

- severity: No real need for it. If the error is severe enough, the kernel handles automatically, i.e. memory poisoning and recovery. In all the other cases it is not severe enough.

We'll never see fatal errors via the perf/tracepoint (no way the RAS daemon will run to pull them). But we will see both corrected error chatter and recovered uncorrectable errors. I would be able to tell these apart. Corrected errors in small doses are normal and don't require any action beyond logging so you can see whether there are enough to cross a threshold and cause alarm. Recovered uncorrectable errors are going to be much rarer, and I think deserve closer scrutiny - even when there is just one of them.
If you drop the severity field, is there some other way to make this distinction?

Quoted 4 lines

- silkscreen_label: <sarcasm> yeah, I'm getting a, say, a Data Cache error during an L1 linefill from L2, what the f*ck does the silkscreen label mean for such an error?! Well, nobody knows wtf it means!</sarcasm>

Cache error should point to a cpu socket - I'd like to have a silk screen label for that (are they numbered "0, 1, 2 ..." on the motherboard or "1, 2, 3 ..."?) No idea where we'd get that information from. dmidecode shows "Socket Designation: CPU 1" (and "2") for my current Sandy Bridge system. I'd have to pull the system apart to see if those are helpful in identifying which physical cpu is which.

-Tony

#24 lore raw

Re: [PATCH 1/3] mce: Add a msg string to the MCE tracepoint

Quoted 4 lines · Tony
Quoted 3 lines

- severity: No real need for it. If the error is severe enough, the kernel handles automatically, i.e. memory poisoning and recovery. In all the other cases it is not severe enough.

Quoted 10 lines

We'll never see fatal errors via the perf/tracepoint (no way the RAS daemon will run to pull them). But we will see both corrected error chatter and recovered uncorrectable errors. I would be able to tell these apart. Corrected errors in small doses are normal and don't require any action beyond logging so you can see whether there are enough to cross a threshold and cause alarm. Recovered uncorrectable errors are going to be much rarer, and I think deserve closer scrutiny - even when there is just one of them.
If you drop the severity field, is there some other way to make this distinction?

Err, MCi_STATUS bits like bit 55 (Action Required) and 56 (Signaled #MC) in your case...?

Quoted 11 lines
Quoted 4 lines

- silkscreen_label: <sarcasm> yeah, I'm getting a, say, a Data Cache error during an L1 linefill from L2, what the f*ck does the silkscreen label mean for such an error?! Well, nobody knows wtf it means!</sarcasm>

Cache error should point to a cpu socket - I'd like to have a silk screen label for that (are they numbered "0, 1, 2 ..." on the motherboard or "1, 2, 3 ..."?) No idea where we'd get that information from. dmidecode shows "Socket Designation: CPU 1" (and "2") for my current Sandy Bridge system. I'd have to pull the system apart to see if those are helpful in identifying which physical cpu is which.

First of all, silkscreen label denotes DIMM slots in this context AFAICT. Concerning CPU sockets, I'm not aware of a method to read out the silkscreen labels at the CPU sockets, are you? Or am I missing something?

IOW, we want to assume that cores 0, 1, 2 ... k-1 are on node 0; k, k+1 ... 2k-1 belong to node 1, etc., where k is the number of cores on a socket and thus we have a regular core enumeration on the box.

Signature · 8 lines
Regards/Gruss,
Boris.

Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
GM: Alberto Bozzo
Reg: Dornach, Landkreis Muenchen
HRB Nr. 43632 WEEE Registernr: 129 19551
maintainer
#25 lore raw

RE: [PATCH 1/3] mce: Add a msg string to the MCE tracepoint

Quoted 3 lines

IOW, we want to assume that cores 0, 1, 2 ... k-1 are on node 0; k, k+1 ... 2k-1 belong to node 1, etc., where k is the number of cores on a socket and thus we have a regular core enumeration on the box.

Sounds dubious:

Booting Node   0, Processors  #1 #2 #3 #4 #5 #6 #7 Ok.
Booting Node   1, Processors  #8 #9 #10 #11 #12 #13 #14 #15 Ok.
Booting Node   0, Processors  #16 #17 #18 #19 #20 #21 #22 #23 Ok.
Booting Node   1, Processors  #24 #25 #26 #27 #28 #29 #30 #31

Brought up 32 CPUs

Now those are logical cpu numbers, and we brought up the first HT thread on each core first, and then came around for a 2nd pass bringing up the other HT thread. This order is determined by how the BIOS lists the cpus (and in this case it seems to be doing so according to recommendations) - so here our core numbers will match what you said. But the BIOS could do something strange and list logical cpus alternating between sockets. In which case cores 0, 2, 4, 6 ... would be on node 0, and cores 1, 3, 5, 7, ... on node 1.

-Tony

#26 lore raw

Re: [PATCH 1/3] mce: Add a msg string to the MCE tracepoint

Quoted 12 lines · Tony
Quoted 3 lines

IOW, we want to assume that cores 0, 1, 2 ... k-1 are on node 0; k, k+1 ... 2k-1 belong to node 1, etc., where k is the number of cores on a socket and thus we have a regular core enumeration on the box.

Sounds dubious:

Booting Node   0, Processors  #1 #2 #3 #4 #5 #6 #7 Ok.
Booting Node   1, Processors  #8 #9 #10 #11 #12 #13 #14 #15 Ok.
Booting Node   0, Processors  #16 #17 #18 #19 #20 #21 #22 #23 Ok.
Booting Node   1, Processors  #24 #25 #26 #27 #28 #29 #30 #31

Brought up 32 CPUs

Quoted 9 lines

Now those are logical cpu numbers, and we brought up the first HT thread on each core first, and then came around for a 2nd pass bringing up the other HT thread. This order is determined by how the BIOS lists the cpus (and in this case it seems to be doing so according to recommendations) - so here our core numbers will match what you said. But the BIOS could do something strange and list logical cpus alternating between sockets. In which case cores 0, 2, 4, 6 ... would be on node 0, and cores 1, 3, 5, 7, ... on node 1.

Ok, the example above actually confirms my fear that you won't be always able to map back to a physical socket from the CPU number. So, we'll need the ->socketid field which is the physical processor ID we get from CPUID leafs.

Then, mapping back the socketid to the silkscreen labels on the boards should be easy because on the boxes I have here, they go like this: P0, P1, ..., where P0 is the socket containing the BSP, P1 is the second socket etc. I'm guessing this is similar on Intel boards...?

Thanks.

Signature · 8 lines
Regards/Gruss,
Boris.

Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
GM: Alberto Bozzo
Reg: Dornach, Landkreis Muenchen
HRB Nr. 43632 WEEE Registernr: 129 19551
#27 lore raw

Re: [PATCH 1/3] mce: Add a msg string to the MCE tracepoint

Em 01-03-2012 08:29, Borislav Petkov escreveu:

Quoted 28 lines · Tony
Quoted 12 lines
Quoted 3 lines

IOW, we want to assume that cores 0, 1, 2 ... k-1 are on node 0; k, k+1 ... 2k-1 belong to node 1, etc., where k is the number of cores on a socket and thus we have a regular core enumeration on the box.

Sounds dubious:

Booting Node   0, Processors  #1 #2 #3 #4 #5 #6 #7 Ok.
Booting Node   1, Processors  #8 #9 #10 #11 #12 #13 #14 #15 Ok.
Booting Node   0, Processors  #16 #17 #18 #19 #20 #21 #22 #23 Ok.
Booting Node   1, Processors  #24 #25 #26 #27 #28 #29 #30 #31

Brought up 32 CPUs

Quoted 9 lines

Now those are logical cpu numbers, and we brought up the first HT thread on each core first, and then came around for a 2nd pass bringing up the other HT thread. This order is determined by how the BIOS lists the cpus (and in this case it seems to be doing so according to recommendations) - so here our core numbers will match what you said. But the BIOS could do something strange and list logical cpus alternating between sockets. In which case cores 0, 2, 4, 6 ... would be on node 0, and cores 1, 3, 5, 7, ... on node 1.

Ok, the example above actually confirms my fear that you won't be always able to map back to a physical socket from the CPU number. So, we'll need the ->socketid field which is the physical processor ID we get from CPUID leafs.

Quoted 4 lines

Then, mapping back the socketid to the silkscreen labels on the boards should be easy because on the boxes I have here, they go like this: P0, P1, ..., where P0 is the socket containing the BSP, P1 is the second socket etc. I'm guessing this is similar on Intel boards...?

The mapping from socket id to silkscreen label is motherboard-dependent. I did some tests with a few different machines:

[root@dell-pe1950-05 ~]# dmidecode | grep -i "Processor Information" -A10 Processor Information
Socket Designation: CPU1
Type: Central Processor
Family: Xeon
Manufacturer: Intel
ID: F6 06 00 00 FF FB EB BF
Signature: Type 0, Family 6, Model 15, Stepping 6 Flags:
FPU (Floating-point unit on-chip)
VME (Virtual mode extension)
DE (Debugging extension)
--
Processor Information
Socket Designation: CPU2
Type: Central Processor
Family: Xeon
Manufacturer: Intel
ID: F6 06 00 00 FF FB EB BF
Signature: Type 0, Family 6, Model 15, Stepping 6 Flags:
FPU (Floating-point unit on-chip)
VME (Virtual mode extension)
DE (Debugging extension)

[root@hp-dl580g5-01 ~]# dmidecode | grep -i "Processor Information" -A10 Processor Information
Socket Designation: Proc 1
Type: Central Processor
Family: Xeon MP
Manufacturer: Intel
ID: FB 06 00 00 FF FB EB BF
Signature: Type 0, Family 6, Model 15, Stepping 11 Flags:
FPU (Floating-point unit on-chip)
VME (Virtual mode extension)
DE (Debugging extension)
--
Processor Information
Socket Designation: Proc 2
Type: Central Processor
Family: Xeon MP
Manufacturer: Intel
ID: FB 06 00 00 FF FB EB BF
Signature: Type 0, Family 6, Model 15, Stepping 11 Flags:
FPU (Floating-point unit on-chip)
VME (Virtual mode extension)
DE (Debugging extension)
--
Processor Information
Socket Designation: Proc 3
Type: Central Processor
Family: Xeon MP
Manufacturer: Intel
ID: FB 06 00 00 FF FB EB BF
Signature: Type 0, Family 6, Model 15, Stepping 11 Flags:
FPU (Floating-point unit on-chip)
VME (Virtual mode extension)
DE (Debugging extension)
--
Processor Information
Socket Designation: Proc 4
Type: Central Processor
Family: Xeon MP
Manufacturer: Intel
ID: FB 06 00 00 FF FB EB BF
Signature: Type 0, Family 6, Model 15, Stepping 11 Flags:
FPU (Floating-point unit on-chip)
VME (Virtual mode extension)
DE (Debugging extension)

dmidecode | grep -i "Processor Information" -A10 Processor Information
Socket Designation: CPU 1
Type: Central Processor
Family: Pentium M
Manufacturer: Intel
ID: A4 06 01 00 FF FB EB BF
Signature: Type 0, Family 6, Model 26, Stepping 4 Flags:
FPU (Floating-point unit on-chip) VME (Virtual mode extension)
DE (Debugging extension)
--
Processor Information
Socket Designation: CPU 2
Type: Central Processor
Family: Pentium M
Manufacturer: Intel
ID: A4 06 01 00 FF FB EB BF
Signature: Type 0, Family 6, Model 26, Stepping 4 Flags:
FPU (Floating-point unit on-chip) VME (Virtual mode extension)
DE (Debugging extension)

[root@hp-bl260cg5-01 ~]# dmidecode | grep "Processor Information" -A10 Processor Information
Socket Designation: Proc 1
Type: Central Processor
Family: Xeon
Manufacturer: Intel
ID: 76 06 01 00 FF FB EB BF
Signature: Type 0, Family 6, Model 23, Stepping 6 Flags:
FPU (Floating-point unit on-chip)
VME (Virtual mode extension)
DE (Debugging extension)
--
Processor Information
Socket Designation: Proc 2
Type: Central Processor
Family: Xeon
Manufacturer: Intel
ID: 00 00 00 00 00 00 00 00
Signature: Type 0, Family 0, Model 0, Stepping 0
Flags: None
Version: Not Specified
Voltage: 1.1 V
External Clock: 200 MHz

On all the above, the socket label starts on 1, but I bet we'll find cases where it start from 0.

Regards,
Mauro

maintainer
#28 lore raw

RE: [PATCH 1/3] mce: Add a msg string to the MCE tracepoint

Quoted 2 lines

On all the above, the socket label starts on 1, but I bet we'll find cases where it start from 0.

Agreed. "Socket Designation" seems to be a free format string at the whim of the board designer (and then interpreted and string filled in by a BIOS writer with all the usual caveats that implies).

I suppose a smart reporting system would try to query this stuff ahead of time to determine whether socket numbers were zero based, one based, or something else altogether (Socket A, B, C ????)

-Tony

#29 lore raw

Re: [PATCH 1/3] mce: Add a msg string to the MCE tracepoint

Quoted 10 lines · Tony
Quoted 2 lines

On all the above, the socket label starts on 1, but I bet we'll find cases where it start from 0.

Agreed. "Socket Designation" seems to be a free format string at the whim of the board designer (and then interpreted and string filled in by a BIOS writer with all the usual caveats that implies).

I suppose a smart reporting system would try to query this stuff ahead of time to determine whether socket numbers were zero based, one based, or something else altogether (Socket A, B, C ????)

Query from where? (And pls don't tell me DMI tables. :-))

I don't think there's a mapping between board designer nomenclature and BIOS values for the node ID you get from apic IDs and etc CPUID leafs.

Signature · 8 lines
Regards/Gruss,
Boris.

Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
GM: Alberto Bozzo
Reg: Dornach, Landkreis Muenchen
HRB Nr. 43632 WEEE Registernr: 129 19551
maintainer
#30 lore raw

RE: [PATCH 1/3] mce: Add a msg string to the MCE tracepoint

Quoted 1 line

Query from where? (And pls don't tell me DMI tables. :-))

Sorry - but DMI is all we've got. We need to start using it, and have people report bugs when it is wrong. Then it might get better. Linux has had some success in shaming BIOS writers with:

printk("Your BIOS is buggy ...")

Our biggest problem here is the life-cycle ... by the time Linux users start seeing these problems, the BIOS people have already moved on to the next generation system.

BUT - if Linux wants to play in RAS space - we need this to be right - and to do that we need SMBIOS (DMI) bugs to be regarded as priority #1.

-Tony

#31 lore raw

Re: [PATCH 1/3] mce: Add a msg string to the MCE tracepoint

Em 01-03-2012 15:58, Luck, Tony escreveu:

Quoted 8 lines
Quoted 1 line

Query from where? (And pls don't tell me DMI tables. :-))

Sorry - but DMI is all we've got. We need to start using it, and have people report bugs when it is wrong. Then it might get better. Linux has had some success in shaming BIOS writers with:

printk("Your BIOS is buggy ...")

Quoted 7 lines

Our biggest problem here is the life-cycle ... by the time Linux users start seeing these problems, the BIOS people have already moved on to the next generation system.

BUT - if Linux wants to play in RAS space - we need this to be right - and to do that we need SMBIOS (DMI) bugs to be regarded as priority #1.

This approach may work, if we put enough pressure on BIOS bugs. It is a way better for us than the way EDAC took: adding a sysfs node for some util to write the labels there. Yet, they did that because a lot of DMI tables were wrong.

Well, we can always have a buggy-BIOS quirk file shaming the BIOS writers, while providing the right values to the end users, as we got bugs reporting issues with certain motherboards and/or manufacturers.

Regards,
Mauro.

#32 lore raw

Re: [PATCH 1/3] mce: Add a msg string to the MCE tracepoint

In reply to Borislav Petkov

Em 29-02-2012 14:16, Borislav Petkov escreveu:

Quoted 7 lines · Tony
Quoted 6 lines
Quoted 3 lines

- severity: No real need for it. If the error is severe enough, the kernel handles automatically, i.e. memory poisoning and recovery. In all the other cases it is not severe enough.

We'll never see fatal errors via the perf/tracepoint (no way the RAS daemon will run to pull them).

With the current approach, that's true.

I remember you've mentioned an idea of storing fatal errors on an APEI non-volatile memory for them to be sent to userspace after a machine reboot. If this is implemented some day, those type of errors could also be reported via trace, depending on how such feature would be implemented, as one possibility would be to just store there the contents of the last dmesg content.

Quoted 10 lines
Quoted 9 lines

But we will see both corrected error chatter and
recovered uncorrectable errors. I would be able to tell these apart. Corrected errors in small doses are normal and don't require any action beyond logging so you can see whether there are enough to cross a threshold and cause alarm. Recovered uncorrectable errors are going to be much rarer, and I think deserve closer scrutiny - even when there is just one of them.
If you drop the severity field, is there some other way to make this distinction?

Quoted 2 lines

Err, MCi_STATUS bits like bit 55 (Action Required) and 56 (Signaled #MC) in your case...?

That would force all userspace tools that handle such errors to have some MCA-specific logic inside, which is one of the things we're trying to avoid. Also, non-MCA drivers will have a severity that aren't present at the MCA status.

Assuming that the same tool can work with both MCA and non-MCA drivers, for API consistency, we should try to use the same way to describe severity (and label/location) on both MCA and non-MCA cases.

With regards to Intel, as far as I know, there are some cpu-family specific stuff for recovered uncorrectable errors.

Quoted 12 lines
Quoted 11 lines
Quoted 4 lines

- silkscreen_label: <sarcasm> yeah, I'm getting a, say, a Data Cache error during an L1 linefill from L2, what the f*ck does the silkscreen label mean for such an error?! Well, nobody knows wtf it means!</sarcasm>

Cache error should point to a cpu socket - I'd like to have a silk screen label for that (are they numbered "0, 1, 2 ..." on the motherboard or "1, 2, 3 ..."?) No idea where we'd get that information from. dmidecode shows "Socket Designation: CPU 1" (and "2") for my current Sandy Bridge system. I'd have to pull the system apart to see if those are helpful in identifying which physical cpu is which.

Quoted 2 lines

First of all, silkscreen label denotes DIMM slots in this context AFAICT.

No. I'm referring to the Silkscreen label (and the location) of the affected component, and not to just DIMMs.

Quoted 3 lines

Concerning CPU sockets, I'm not aware of a method to read out the silkscreen labels at the CPU sockets, are you? Or am I missing something?

The same strategy used by edac can be used there: add a 'label' node to
/sys/devices/system/cpu/cpu?/

To allow userspace to fill it/override it.

Quoted 3 lines

IOW, we want to assume that cores 0, 1, 2 ... k-1 are on node 0; k, k+1 ... 2k-1 belong to node 1, etc., where k is the number of cores on a socket and thus we have a regular core enumeration on the box.

Initially, the RAS code could fill the 'label' using the above criteria, while allowing an userspace tool to get the labels from dmidecode and use it there.

Regards,
Mauro

#33 lore raw

Re: [PATCH 1/3] mce: Add a msg string to the MCE tracepoint

In reply to Borislav Petkov

Em 29-02-2012 11:40, Borislav Petkov escreveu:

Quoted 5 lines · Mauro Carvalho Chehab
Quoted 2 lines

No, you didn't. Every time i touch on this point, you just say that it doesn't fit without giving any explanation why not.

Let me explain it to you one _last_ time:

Thanks! Your view is now clear.

Quoted 4 lines

- severity: No real need for it. If the error is severe enough, the kernel handles automatically, i.e. memory poisoning and recovery. In all the other cases it is not severe enough.

I see your point, but opone thing is to recover from severe errors; another thing is to properly report it.

In general, when an error occurs, what users do is to account them, taking other measures (like replace the affected hardware) only if the error count is above a certain threshold.

The threshold criteria for non-severe errors is generally different from the criteria for severe ones. That's why the severity information should be reported.

Quoted 2 lines

- location: this is contained in the ->cpu field.

Assuming that the CPU field contains the location of the error is a bad assumption. On SB, the CPU that reports a memory error is the CPU where some code tried to access the RAM, and not the CPU where the memory controller is. So, the value for CPU is bogus for those error types. This is also true, at least on Intel, for other types of errors, like bus/interconnect ones: the CPU that reports the error is the one trying to access the bus.

Also, on almost all memory error cases, the location of the affected component is not the memory controller at the CPU. Instead, it is the DRAM chip, located inside a DIMM.

So, while there are several error types where the location is cpu field, there are also several other cases where location != cpu.

The kernel decoder knows the error location, on most cases. So, instead of letting the userspace to guess the error location, it should report what it was decoded.

Quoted 4 lines

- silkscreen_label: <sarcasm> yeah, I'm getting a, say, a Data Cache error during an L1 linefill from L2, what the f*ck does the silkscreen label mean for such an error?! Well, nobody knows wtf it means!</sarcasm>

It means what component needs to be replaced, because there are too many errors there, and it is likely damaged.

Silkscreen label for a L1 error: CPU0, CPU1, CPU2, CPU3 (the socket "name" for the CPU socket at the motherboard, as labeled at the silkscreen). Again, this is not the CPU field, as one CPU socket has several cores, and the core/socket ID order can be different than the actual CPU slots (btw, this is explicitly noticed on a few Intel datasheets).

Of course, for both location and silkscreen label fields, if, for any reason, the location of the affected component can't be identified, those fields should be filled with an empty string (or with something like "unknown").

Silkscreen label for a memory error: DIMM1A, DIMM2B, etc.

Quoted 4 lines

- error_msg: already there in my patch.

So go and read and try _understanding_ this before you come back with more crap, ok?

Ok, but please do the same and try to see the question from the users perspective that needs to know what is the damn broken FRU (Field Replaceable Unit) that needs a replacement on their critical systems.

Quoted 5 lines
Quoted 2 lines

Running away from this discussion won't help at all.

Not running away - trying not to waste time with bullshit.

Thanks,
Mauro

#34 lore raw

PATCH 2/3 x86, RAS: Add a decoded msg buffer

In reply to Borislav Petkov

From: Borislav Petkov <[email protected]>

Echoing 1 into /sys/devices/system/ras/agent causes the ras_printk() function to buffer a string describing a hardware error. This is meant for userspace daemons which are running on the system and are going to consume decoded information through the MCE tracepoint.

Also, upon first use, the buffer enlarges itself from its initial size so that it can accomodate longer messages.

---
 arch/x86/Kconfig           |    9 +++
 arch/x86/Makefile          |    3 +
 arch/x86/include/asm/ras.h |   13 ++++
 arch/x86/ras/Makefile      |    1 +
 arch/x86/ras/ras.c         |  162 ++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 188 insertions(+), 0 deletions(-)
 create mode 100644 arch/x86/include/asm/ras.h
 create mode 100644 arch/x86/ras/Makefile
 create mode 100644 arch/x86/ras/ras.c
arch/x86/Kconfig +9 -0
index 5bed94e189fa..bda1480241b2 100644
@@ -657,6 +657,15 @@ config X86_CYCLONE_TIMER
 	def_bool y
 	depends on X86_SUMMIT
 
+config X86_RAS
+	def_bool y
+	prompt "X86 RAS features"
+	---help---
+	A collection of Reliability, Availability and Serviceability
+	software features which aim to enable hardware error logging
+	and reporting. Leave it at 'y' unless you really know what
+	you're doing
+
 source "arch/x86/Kconfig.cpu"
 
 config HPET_TIMER
arch/x86/Makefile +3 -0
index 209ba1294592..a6b6bb1f308b 100644
@@ -146,6 +146,9 @@ drivers-$(CONFIG_OPROFILE) += arch/x86/oprofile/
 # suspend and hibernation support
 drivers-$(CONFIG_PM) += arch/x86/power/
 
+# RAS support
+core-y += arch/x86/ras/
+
 drivers-$(CONFIG_FB) += arch/x86/video/
 
 ####
arch/x86/include/asm/ras.h new file +13 -0
new file mode 100644
index 000000000000..17aa2679032b
@@ -0,0 +1,13 @@
+#ifndef _ASM_X86_RAS_H
+#define _ASM_X86_RAS_H
+
+extern bool ras_agent;
+
+#define PR_EMERG	BIT(0)
+#define PR_WARNING	BIT(1)
+#define PR_CONT		BIT(2)
+
+extern const char *ras_get_decoded_err(void);
+extern void ras_printk(unsigned long flags, const char *fmt, ...);
+
+#endif /* _ASM_X86_RAS_H */
arch/x86/ras/Makefile new file +1 -0
new file mode 100644
index 000000000000..7a70bb5cd057
@@ -0,0 +1 @@
+obj-y		:= ras.o
arch/x86/ras/ras.c new file +162 -0

162 lines not on this page · raw message

Signature · 1 line
1.7.8.rc0
maintainer
#35 lore raw

RE: [PATCH 2/3] x86, RAS: Add a decoded msg buffer

+ if (left <= 50) {
+ /* enlarge arbitrarily by 50 chars */
+ err_str_sz += 50;
+ left += 50;
+
+ err_str = krealloc(err_str, err_str_sz, GFP_KERNEL);
+ if (!err_str) {
+ pr_err("Error enlarging decode buffer.\n");
+ return;
+ }
+ }

This looks worrying to me. Some bad stuff has happened, we are perhaps in machine check context, and you want to start allocating memory!

Is there some upper bound for how long this string can get? Why not just allocate a generous amount at boot time?

-Tony

#36 lore raw

Re: [PATCH 2/3] x86, RAS: Add a decoded msg buffer

Quoted 12 lines · Tony
arch/x86/ras/ras.c new file +11 -0
+	if (left <= 50) {
+		/* enlarge arbitrarily by 50 chars */
+		err_str_sz += 50;
+		left += 50;
+
+		err_str = krealloc(err_str, err_str_sz, GFP_KERNEL);
+		if (!err_str) {
+			pr_err("Error enlarging decode buffer.\n");
+			return;
+		}
+	}
Quoted 2 lines

This looks worrying to me. Some bad stuff has happened, we are perhaps in machine check context, and you want to start allocating memory!

Yes, absolutely. See my other mail to Seto-san.

Quoted 2 lines

Is there some upper bound for how long this string can get? Why not just allocate a generous amount at boot time?

Yep, this sounds like the easiest thing to do, let me play with MCE injection a bit.

Signature · 8 lines
Regards/Gruss,
Boris.

Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
GM: Alberto Bozzo
Reg: Dornach, Landkreis Muenchen
HRB Nr. 43632 WEEE Registernr: 129 19551
#37 lore raw

Re: [PATCH 2/3] x86, RAS: Add a decoded msg buffer

Quoted 5 lines · Borislav Petkov
Quoted 2 lines

Is there some upper bound for how long this string can get? Why not just allocate a generous amount at boot time?

Yep, this sounds like the easiest thing to do, let me play with MCE injection a bit.

So I left it inject all kinds of errors overnight and the buffer got enlarged to 400 bytes:

[ 1115.162484] Buffer enlarged to 250
[ 1115.166059] Buffer enlarged to 300
[ 1115.169604] Buffer enlarged to 350
[ 3855.129854] Buffer enlarged to 400

So I'm guessing something like 500 bytes would be a safe bet in the current situation.

Now whether 500 bytes is a lot or not, dunno, we'll see how it plays out in testing. Also, the question is what would happen if we need to enlarge it later for some more info, hm. Maybe I should go over the decoded strings and try slimming them up a bit...

Signature · 8 lines
Regards/Gruss,
Boris.

Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
GM: Alberto Bozzo
Reg: Dornach, Landkreis Muenchen
HRB Nr. 43632 WEEE Registernr: 129 19551
#38 lore raw

PATCH 3/3 EDAC: Convert AMD EDAC pieces to use RAS printk buffer

In reply to Borislav Petkov

From: Borislav Petkov <[email protected]>

This is an initial version of the patch which converts MCE decoding facilities to use the RAS printk buffer. When there's no userspace agent running (i.e., /sys/devices/system/ras/agent == 0), we fall back to the default printk's into dmesg which is what we've been doing so far.

---
 drivers/edac/amd64_edac.c |    8 ++-
 drivers/edac/edac_mc.c    |   42 ++++++---
 drivers/edac/mce_amd.c    |  215 ++++++++++++++++++++++++---------------------
 3 files changed, 147 insertions(+), 118 deletions(-)
drivers/edac/amd64_edac.c +6 -2
index c9eee6d33e9a..2d9a64b01ed5 100644
@@ -1,6 +1,7 @@
-#include "amd64_edac.h"
 #include <asm/amd_nb.h>
+#include <asm/ras.h>
 
+#include "amd64_edac.h"
 static struct edac_pci_ctl_info *amd64_ctl_pci;
 
 static int report_gart_errors;
@@ -1901,7 +1902,10 @@ static void amd64_handle_ce(struct mem_ctl_info *mci, struct mce *m)
 	sys_addr = get_error_address(m);
 	syndrome = extract_syndrome(m->status);
 
-	amd64_mc_err(mci, "CE ERROR_ADDRESS= 0x%llx\n", sys_addr);
+	if (ras_agent)
+		ras_printk(PR_EMERG, "ERR_ADDR: 0x%llx", sys_addr);
+	else
+		amd64_mc_err(mci, "CE ERROR_ADDRESS= 0x%llx\n", sys_addr);
 
 	pvt->ops->map_sysaddr_to_csrow(mci, sys_addr, syndrome);
 }
drivers/edac/edac_mc.c +28 -14

62 lines not on this page · raw message

drivers/edac/mce_amd.c +113 -102

593 lines not on this page · raw message

Signature · 1 line
1.7.8.rc0