Support for extended FRU data for memory modules #147
Conversation
|
@dcrowell77, what do you think about this? |
Decode some DIMM SPD fields according to JEDEC format to write detailed info into an IMPI FRU message: 1. Manufacturer field contains real orgainsation name instead of ID. 2. Module name contains type, capacity, speed and other attributes. Signed-off-by: Artem Senichev <[email protected]> Signed-off-by: Maxim Polyakov <[email protected]>
| } | ||
|
|
||
| // Put detailed description into the message buffer | ||
| const size_t l_len = strlen(l_desc); |
There was a problem hiding this comment.
According to IPMI FRU specification, value 0 for the length means that the fields is empty, while value 1 indicates 'end-of-fields'. Thus, value 1 is always reserved and can't be used. Which means that the minimum size of a string is 2. This is not checked here.
Also, the maximum length of plain ASCII string in IPMI is 31 bytes. This is 1 byte shorter than, for instance, "DDR4-2400 16GiB 64-bit ECC RDIMM". Things like "LPDDR4-2400 16GiB 64-bit ECC SORDIMM 72b" will get even more fiercely truncated. Something must be invented to overcome that. I'd propose converting the string to all-uppercase and encoding it using 6-bit ASCII (see FRU spec). That would give us 41 bytes max. We could also save on using non-SI units for size (GB instead of GiB), on saying 64B instead of 64-BIT, and on shortening long module types to, say these:
MINIRDIMMMINIUDIMMSORDIMM72SODIMM16SODIMM32
That way the longest string will be LPDDR4-2400 16GB 32B NON-ECC MINIRDIMM which is 38 characters.
It must be checked though whether or not the BMC firmwares can properly decode 6-bit ASCII.
If we ditch Mini and SO modules, that are, AFAIK, rarely used in OpenPOWER systems, and also use empty string instead of 'NON-ECC', then we can fit into 31 bytes with the longest LPDDR4-2400 16GB 32b ECC LRDIMM, and that's the good old plain ASCII+Latin1 encoding.
There was a problem hiding this comment.
If we use something other than straight ascii, what are the chances that the BMC will actually display it in any useful way? Might need to ask in the openbmc forum what they support. Have you by chance tried this change on a system running OpenBMC (or SuperMicro or AMI)? Where/how is the data displayed?
There was a problem hiding this comment.
OpenBMC always shows manufacture and pretty names as text.
We have checked this solution on our VESNIN server.
WebUI example:
Inventory list:
There was a problem hiding this comment.
@artemsen, I believe @dcrowell77 was referring to my proposal to use IPMI FRU's 6-bit ASCII encoding. I have checked and am quite sure now that openbmc/ipmi-fru-parser does not support any encodings beyond binary (code 0) and plain ASCII (code 3 / 0xC0). Can't say for AMI. We of course could add support for 6-bit ASCII to openbmc, but hardly to MegaRAC or any other commercial products.
This means that our best way is to use plain ASCII and ditch support for the uncommon module types.
There was a problem hiding this comment.
Also, the maximum length of plain ASCII string in IPMI is 31 bytes
Mea culpa. I misread the spec. The type/length byte has 6 bits for data length, which gives maximum string length of 63, not 31. I don't know where I took that '31' from. Nothing needs to be done in regard to string length.
|
@wghoffa - take a look |
|
@dcrowell77 -- Will do first thing tomorrow. I'll try and load this on a Witherspoon and Boston and see how it works as well. |
This patch is to master-p8 branch. I'm not sure it can be applied to P9. |
|
@artemsen -- Thanks I didn't catch that. I'll try a P8 system, though may also apply to P9 and see how it goes. I don't think things in this area have really changed at all. |
|
@dcrowell77 / @artemsen -- Looks good! AMI output worked well too (ran from a habanero): FRU Device Description : DIMM8 (ID 11) |
|
Merged here: 30b88ed Thanks for the submission! |
|
@wghoffa, could you please also backport this to master-p8? Our system is P8-based and we currently carry this patch in our op-build fork. It would be great if this was a part of upstream branch. |
|
@AlexanderAmelkin -- It should be in for master-p8 (I did not yet port it to master, but am hoping to). I see it here? https://github.com/open-power/hostboot/commits/master-p8 I think the update to op-build will come shortly. We have an automated process that runs daily to do those updates, it has not triggered yet today but should trigger in the next few hours or so. |
|
@AlexanderAmelkin -- Looks like the update came with a bunch of P9 stuff that appears to have an issue that is being sorted out (open-power/op-build#2381). I pulled the P8 stuff to the side -- just waiting on CI: open-power/op-build#2382 Also -- I gave it a go for p9 and all seems well. I'll push that up soon as well. |
Decode some DIMM SPD fields according to JEDEC format to write
detailed info into an IMPI FRU message:
Resolves #132
Signed-off-by: Artem Senichev [email protected]
Signed-off-by: Maxim Polyakov [email protected]