On 6/24/2015 12:14 PM, Luck, Tony wrote:
Another option would be to have 2 structs, the first one "struct cper_sec_mem_err" holds the structure as defined by UEFI 2.1, the 2nd one "struct cper_sec_mem_err_24_ext" holds the 4 elements added in UEFI 2.3.1.
Reading some more of the UEFI 2.5 spec ... I see we are in for a world of pain here.
2.5 adds some small tweaks to the memory structure (adding a couple of extra bits to the "row" entry that can be grabbed from the formerly reserved byte at offset 73).
I think this can be dealt with easily as long as all platforms observe the rule that if a bit is reserved, the bit should be set a '0' instead of being set randomly. Is this a fair assumption on all platforms?
But then there is a whole new GUID for a "Memory Error Section 2" which has doubled the width of the device, row, column, rank, and bit_pos fields together with adding two new fields for chip_id and status. This will be painful because we hardwired the old sizes into extlog_mem_event in <ras/ras_event.h>
The old size is encoded in "stuct cper_mem_err_compact", other members of the trace data are the same between "Memory Error Section" and "Memory Error Section 2". One option we have without having to disturb user space handler of memory error trace data would be to change "struct cper_mem_err_compact" so the affected elements are of __u32 instead of __u16. Drawback of this option is that the trace buffer will be unnecessarily bigger if a platform generates "Memory Error Section" data instead of "Memory Error Section 2" data. Such drawback is not a big issue given that uncorrected memory error happens infrequently and corrected memory error should be grouped by platform.
"Memory Error Section 2". One option we have without having to disturb user space handler of memory error trace data would be to change "struct cper_mem_err_compact" so the affected elements are of __u32 instead of __u16. Drawback of this option is that the trace buffer will be unnecessarily bigger if a platform generates "Memory Error Section" data instead of "Memory Error Section 2" data.
That structure is visible to user level consumers of the event (perhaps just one of those right now?):
git://git.fedorahosted.org/rasdaemon.git
We were not as smart as UEFI and didn't include a version number, or other plan, that would allow us to transition to a new format cleanly.
Perhaps we could re-purpose some of the high order "validation_bits" as a version number? It's a u64 and UEFI 2.5 is only up to bit21 so far, so perhaps it will be a long, long time before they get that many fields in some future standard.
-Tony
+Mauro
On 6/24/2015 1:12 PM, Luck, Tony wrote:
"Memory Error Section 2". One option we have without having to disturb user space handler of memory error trace data would be to change "struct cper_mem_err_compact" so the affected elements are of __u32 instead of __u16. Drawback of this option is that the trace buffer will be unnecessarily bigger if a platform generates "Memory Error Section" data instead of "Memory Error Section 2" data.
That structure is visible to user level consumers of the event (perhaps just one of those right now?):
git://git.fedorahosted.org/rasdaemon.git
We were not as smart as UEFI and didn't include a version number, or other plan, that would allow us to transition to a new format cleanly.
Perhaps we could re-purpose some of the high order "validation_bits" as a version number? It's a u64 and UEFI 2.5 is only up to bit21 so far, so perhaps it will be a long, long time before they get that many fields in some future standard.
I agree that we could re-purpose some of the high order "validation_bits" as a version number. That being said, I am not sure whether that approach is preferred by user space tool authors. My feeling is such approach would make user space tool more complicated (eg. has to understand versions). Mauro, pls. correct me is I am wrong; pls. refer to previous email in this thread for context related to challenge brought forth by UEFI 2.5.
perf interface has debugfs interface, so if user space tool does following, compatibility with different kernel version and different spec version will be maintained: * Use debugfs interface to discover format of trace data. * use largest size known for user space structure; check size of member in user space structure and size of corresponding field in trace data, adjust data as necessary.
In the mean time, I think when kernel defines TRACE_EVENT, it should try not having to use __field_struct, that would make format inside that field opaque to user space tool. For instance: __field_struct(struct cper_mem_err_compact, data) Because of above, ras-extlog-handler.c in rasdaemon has to hardcode this structure, making it harder to maintain forward/backward compatibility.
-
Luck, Tony -
Zhang, Jonathan Zhixiong