Page MenuHomeFreeBSD

nvmecontrol: Add Samsung Extended SMART Information logpage support.
ClosedPublic

Authored by wanpengqian_gmail.com on Jan 5 2022, 7:26 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Oct 26, 8:13 PM
Unknown Object (File)
Oct 2 2024, 11:25 AM
Unknown Object (File)
Oct 2 2024, 11:25 AM
Unknown Object (File)
Oct 2 2024, 11:25 AM
Unknown Object (File)
Sep 30 2024, 1:40 PM
Unknown Object (File)
Sep 30 2024, 1:33 PM
Unknown Object (File)
Sep 30 2024, 1:31 PM
Unknown Object (File)
Sep 30 2024, 7:14 AM

Details

Summary

Samsung PM983 SSD has a 0xca logpage. It has more information compared to Intel's
this patch tested on PM983 M2 SSD and works as expected.

Product Datasheet can be found here:
https://gzhls.at/blob/ldb/2/b/4/9/d8f8e423e0134a3f116548621be52b49a323.pdf

Signed-off-by: Wanpeng Qian <wanpengqian@gmail.com>

Test Plan

Currently only tested on PM983 SSDs.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Generally this looks good. I know you've used the datasheet names for these items, but since the datasheet doesn't actually contain what they mean, it might make sense to tweak the descriptions slightly to give a better clue.

sbin/nvmecontrol/modules/samsung/samsung.c
130

All the printfs from here on down need to be converted from le to host.

142

that's a quite odd name for erase block reserve

145

That's an odd name for number of sectors reallocated....

148

NPO? That's an odd name for the number of clean shutdowns. Number of Power Off spelled out would be slightly less bad, but still obscure.

151

That's an odd name for number of unclean shutdowns, though I see the data sheet uses sudden power off here.

data is converted from le to host
use more accurate description

You are right. the datasheet is not so clear for what it means. I correct some descriptions for better meaning.

sbin/nvmecontrol/modules/samsung/samsung.c
130

For 128bit values, do I need to convert two 64bits first? I am not sure.

142

I use Reserved Erase Block Count to match others' style

144

I change UECC to Uncorrectable ECC here. seems reasonable.

148

is Normal Power Off Count seems odd?

151

use Sudden Power Off Count to match the datasheet's description.

With the revisions, I think this is ready to go based on the posted datasheet pointer.

sbin/nvmecontrol/modules/samsung/samsung.c
130

Yes. However, we should have a le128dec function that we can use here. We don't and now that I'm looking at a lot of the rest of the code, I see that printing the other log pages isn't quite endian correct (Which is why we don't have a le128dec). For now, leave the 128 bit stuff as is and I'll do a pass to fix them all (if someone else does not do it first).

142

That sounds fine.

144

That's fine. UECC or Uncorrectable ECC are both reasonable, and the latter is a little better.

148

That's fine. I think it's what NPO is short for and it will save others having to puzzle it out.

151

Great!

This revision is now accepted and ready to land.Jan 6 2022, 6:28 AM
This revision now requires review to proceed.Jan 7 2022, 5:46 AM

Output SMART ID for better classification

pauamma_gundo.com added inline comments.
sbin/nvmecontrol/nvmecontrol.8
247

Also, "Western Digital" may be clearer than WDC.

Who's going to commit this? Seems like it's as complete as it can get.

In D33749#835893, @bcr wrote:

Who's going to commit this? Seems like it's as complete as it can get.

Yes, please.

sbin/nvmecontrol/nvmecontrol.8
247

Also, "Western Digital" may be clearer than WDC.

I think it is well-known for WDC nowadays. Since HGST is a short for Hitachi Global Storage Technologies, use of WDC will match HGST. what do you think?

Explicitly approve the man page change.
@imp: Can you take care of the commit? I associate nvmecontrol mainly with you and you have a src bit to do the commit. Thank you!

pauamma_gundo.com added inline comments.
sbin/nvmecontrol/nvmecontrol.8
247

Also, "Western Digital" may be clearer than WDC.

I think it is well-known for WDC nowadays. Since HGST is a short for Hitachi Global Storage Technologies, use of WDC will match HGST. what do you think?

I think you know the audience for this and I don't. If you say they will understand WDC the way you mean it, that's good enough for it.

This revision is now accepted and ready to land.Oct 5 2022, 12:55 PM