SATL could decode page 83 for device IDs
The LUN device IDs are generally available on modern SATA devices (required by T13 specifications), but our SATL doesn't decode them.
The recommendation in the latest SAT spec is to translate requests to obtain VPD page 0x83 correctly. The best way for us to do that (safest) is to obtain the inquiry information at startup, and store it, and then return it in software.
We have an implementation of this at RackTop.
There is a low risk on upgrade that devices may upon reboot may start enumerating under a /dev/ link with a WWN, instead of a simple cXtYdZ link. ZFS does seem to cope well with this, but folks using legacy filesystems may have some difficulties, particularly if the /dev directory is not persistent (as is the case with some non-persistent root distributions.)
Still, we feel this offers better interoperability if the disk is moved to another adapter, as the names will be preserved, and the use of page 83 data is becoming more and more common in various situations.
Modern hardware based SATLs do translate this already.
Updated by Garrett D'Amore about 2 months ago
Here's the delta I implemented for RackTop. Note that this code assumes that the device is a "modern" SATA drive. Probably we should update it to better accommodate drives that simply lack this page; I've not been consciously aware of any using any such drive, and I have to believe any such drives are long past their manufacturer warranties/expected life times (I don't think anyone has made one in over a decade):
Note that these should carry a RackTop copyright (2021), but they aren't in the diffs because of other changes that we have made in this driver as well.
commit d6784f114e629c6d0f7647cb47b6cdfdb932544a Author: Garrett D'Amore <email@example.com> Date: Thu Oct 14 12:40:17 2021 -0700 BSR-9997 SATL should translate page 83 diff --git a/usr/src/uts/common/io/sata/impl/sata.c b/usr/src/uts/common/io/sata/impl/sata.c index dbeed36833..69f0ff201d 100644 --- a/usr/src/uts/common/io/sata/impl/sata.c +++ b/usr/src/uts/common/io/sata/impl/sata.c @@ -3647,7 +3647,7 @@ sata_txlt_nodata_cmd_immediate(sata_pkt_txlate_t *spx) #define INQUIRY_BDC_PAGE 0xB1 /* Block Device Characteristics Page */ /* Code */ #define INQUIRY_ATA_INFO_PAGE 0x89 /* ATA Information Page Code */ -#define INQUIRY_DEV_IDENTIFICATION_PAGE 0x83 /* Not needed yet */ +#define INQUIRY_DEV_IDENTIFICATION_PAGE 0x83 /* Device identifiers */ static int sata_txlt_inquiry(sata_pkt_txlate_t *spx) @@ -3750,13 +3750,15 @@ sata_txlt_inquiry(sata_pkt_txlate_t *spx) page_buf = peripheral_device_type; page_buf = INQUIRY_SUP_VPD_PAGE; page_buf = 0; - page_buf = 4; /* page length */ + page_buf = 5; /* page length */ page_buf = INQUIRY_SUP_VPD_PAGE; page_buf = INQUIRY_USN_PAGE; page_buf = INQUIRY_BDC_PAGE; - page_buf = INQUIRY_ATA_INFO_PAGE; + page_buf = INQUIRY_DEV_IDENTIFICATION_PAGE; + page_buf = INQUIRY_ATA_INFO_PAGE; + /* Copy no more than requested */ - count = MIN(bp->b_bcount, 8); + count = MIN(bp->b_bcount, 9); bcopy(page_buf, bp->b_un.b_addr, count); break; @@ -3923,11 +3925,25 @@ sata_txlt_inquiry(sata_pkt_txlate_t *spx) case INQUIRY_DEV_IDENTIFICATION_PAGE: /* - * We may want to implement this page, when - * identifiers are common for SATA devices - * But not now. + * Page 83; SAT-5 requires this, and modern + * SATA devices all support a WWN. */ - /*FALLTHROUGH*/ + page_buf = peripheral_device_type; + page_buf = INQUIRY_DEV_IDENTIFICATION_PAGE; + page_buf = 0; + page_buf = 12; /* remaining length */ + page_buf = 0x01; /* protocol 0, code set 1 */ + page_buf = 0x03; /* LUN, NAA type */ + page_buf = 0; + page_buf = 0x08; /* length (64-bit WWN) */ +#ifdef _LITTLE_ENDIAN + swab(&sdinfo->satadrv_id.ai_naa_ieee_oui, &page_buf, 8); +#else + bcopy(&sdinfo->statadrv_id.ai_naa_ieee_oui, &page_buf, 8); +#endif + count = MIN(bp->b_bcount, 12 + 4); /* header + designator */ + bcopy(page_buf, bp->b_un.b_addr, count); + break; default: /* Request for unsupported VPD page */
Updated by Garrett D'Amore about 2 months ago
A little more information.
ATA-8/ACS makes the NAA identifier mandatory.
However, if you look at SATA-1.0, it defines the identify device data per ATA/ATAPI-5. In that specification, these bytes are reserved, and the specification requires that reserved bits are zero.
It would be illegal for the first byte, or the first ushort_t to be all zero, because the NAA nibble must be a valid value and zero is not legal. (Legal NAA format types are 1, 2, 5, 6 (would be 128-bit WWN), and 0xC, 0xD, 0xE, 0xF.
We can (and probably should) examine that first byte and not translate if it is zero.
Updated by Garrett D'Amore about 1 month ago
We've been using this code to collect VPD page 83, which is used in our stack for a variety of purposes, for some months now. It has been used in with two different generations of Intel motherboards utilizing the AHCI controller.
Obviously there are some older generation AHCI controllers that we have not tested, as well as some truly ancient SATA drives that we no longer have access to. However, any drive manufactured in the past decade is going to support the WWN information in the DEVICE INFORMATION response for SATA drives.
Updated by Joshua M. Clulow about 1 month ago
To clarify, our expectations are that this will not result in any changes to
/devices paths for disks. As per Garrett's mail:
I think my concern was misguided. I don’t think this causes any of the links or device paths to change, because the SATA drives don’t enumerate using
DDI_NT_BLOCK_SAS… they just use
A little more context – I know that our Intel platforms still enumerate sata drives using this driver using
cXtYdZ– where Y is a small integer like 0 or 1, instead of a large WWN based target number – which is what I was afraid would happen.
Updated by Electric Monk about 1 month ago
- Status changed from Pending RTI to Closed
- % Done changed from 80 to 100
commit 8118bef4ce6388ad51cc4ab94dbedc03621ee1e3 Author: Garrett D'Amore <firstname.lastname@example.org> Date: 2022-07-07T21:43:42.000Z 14765 SATL could decode page 83 for device IDs Reviewed by: Hans Rosenfeld <email@example.com> Reviewed by: Andy Fiddaman <firstname.lastname@example.org> Reviewed by: Toomas Soome <email@example.com> Reviewed by: Jerry Jelinek <firstname.lastname@example.org> Reviewed by: Gordon Ross <email@example.com> Reviewed by: Albert Lee <firstname.lastname@example.org> Reviewed by: Andrew Stormont <email@example.com> Approved by: Approved by: Joshua M. Clulow <firstname.lastname@example.org>