Hello James, On Thu, Aug 23, 2018 at 5:29 AM James Morse <james.mo...@arm.com> wrote: > On 19/07/18 19:36, Tyler Baicar wrote: > > On 7/19/2018 10:46 AM, James Morse wrote: > >> On 19/07/18 15:01, Borislav Petkov wrote: > >>> On Mon, Jul 16, 2018 at 01:26:49PM -0400, Tyler Baicar wrote: > >>>> Enable per-layer error reporting for ARM systems so that the error > >>>> counters are incremented per-DIMM. > > This 'layer' term seems to be EDAC's artificial view of memory. >
Yes, it's just the terminology that EDAC uses for locating a DIMM. "Layer" can mean several things here: https://elixir.bootlin.com/linux/latest/source/include/linux/edac.h#L318 We should be able to avoid the layer definitions with the SMBIOS handles. > > >> Does this work on x86, and its just the dmi/cper fields have a subtle > >> difference? > > > There are CPU specific EDAC drivers for a lot of x86 folks and those drivers > > populate the layer information in a custom way. > > Not for GHES surely? > Correct, the x86 drivers that properly increment the DIMM error counters are not tied to the ghes_edac driver. > > (DPC == DIMM per Channel?) > Yes. > > The goal is to be able to enable the per layer error reporting in the > > ghes_edac > > driver so that the per dimm counters exposed in the EDAC sysfs nodes are > > properly > > updated. > > What do you mean by layer? I can't find anything in the ACPI/UEFI/SMBIOS specs > that uses this term... > > If its just 'per dimm counters' you're after, this looks straightforward. > Yes, we just need a way to increment the per DIMM counters that are exposed by the EDAC sysfs nodes. > [re-ordered hunk:] > > This seems pretty hacky to me, so if anyone has other suggestions please > > share > > them. > > CPER's "Memory Error Record 2" thinks that "NODE, CARD and MODULE should > provide > the information necessary to identify the failing FRU". As EDAC has three > 'levels', these are what they should correspond to for ghes-edac. > > I assume NODE means rack/chassis in some distributed system. Lets ignore it as > it doesn't seem to map to anything in the SMBIOS table. I believe NODE should map to socket number for multi-socket systems. > The CPER record's card and module numbers are useless to us, as we need to > know > how many there will be in advance. (does this version of firmware count from 0 > or 1?) > > ... but CPER also gives us a 'Card Handle' and 'Module Handle'. > 'Module Handle' maps to SMBIOS:17 Memory Device (aka, a DIMM). The Handle is a > word-value in the structure, so it doesn't depend on the layout/parse-order of > the SMBIOS tables. When we count the DIMMs in edac-ghes we can give them some > level-idx, then use the handle to find which level-idx to use for this DIMM. > > ghes_edac_report_mem_error() already picks up the module-handle, but only uses > it to print the bank/device. > > 'Card' doesn't mean much to me, but it maps to SMBIOS:17 "Memory Array > Structure", which the Memory Device structure also points to. > Card then must mean "a collection of memory devices (DIMMs) that operate > together to form an address space". > > This might be what I think of as a memory-controller, or it might be something > more complicated. Regardless, the CPER records think its relevant. > > For the edac:layers, we could walk the DMI table to find these structures, and > build the layers from them. If the Memory-array-structures are missing, we can > use the existing 1:NUM_DIMMS approach. > I think the proper way to get this working would be to use these handles. We can avoid populating this layer information and instead have a mapping of type 17 index number (how edac is numbering the DIMMs today) to the handle number. Then we will need a new function to increment the counter based on the handle number rather than this layer information. Is that how you are envisioning it? Thanks, Tyler
