Skip to contentSkip to conversation

Fix the problem of SATA devices within SAS enclosures

PATCH In review James Bottomley 3 participants 10 messages Dec 9, 2015 – Dec 15, 2015 SCSI

lore
Share & tools
lore

James Bottomley

author maintainer
#0 lore raw

cover Fix the problem of SATA devices within SAS enclosures

A big use of SAS enclosures is actually to hold SATA devices. Right at the moment, the ses enclosure system doesn't recognize SATA devices. The reason for this is that SAS actually makes up an endpoint address for a SATA device since SATA doesn't have an addressing scheme. The problem this causes is that the made up SAS address doesn't match the SATA NAA identifier in VPD page 0x83, so the address for the device the enclosure reports never matches.

We can fix this by using the SAS transport class to give us the actual end point address instead of using the identity VPD page. This ensures SATA devices are always correctly matched at the expense of pulling in all the SAS transport code. Instead of making ses depend on the SAS transport class, I've introduced an is_sas_attached() function that will allow us to compile out the code if the kernel isn't built with SAS. If anyone ever gets around to doing FC enclosures, they should probably be done in the same way.

James

---

James Bottomley (3):
scsi_transport_sas: add is_sas_attached() function
scsi_transport_sas: add function to get SAS endpoint address
ses: fix discovery of SATA devices in SAS enclosures

 drivers/scsi/scsi_transport_sas.c |   30 ++++++++++++++++++++++++++++++
 drivers/scsi/ses.c                |   22 ++++------------------
 include/scsi/scsi_transport_sas.h |   10 ++++++++++
 3 files changed, 44 insertions(+), 18 deletions(-)
author maintainer
#1 lore raw

PATCH 1/3 scsi_transport_sas: add is_sas_attached() function

Adds a function designed to be callable any time (regardless of whether the transport attributes are configured or not) which returns true if the device is attached over a SAS transport. The design of this function is that transport specific functions can be embedded within a

if (is_sas_attached(sdev)) {
	...
}

which would be compiled out (and thus eliminate the symbols) if SAS is not configured.

---
 drivers/scsi/scsi_transport_sas.c |   16 ++++++++++++++++
 include/scsi/scsi_transport_sas.h |    9 +++++++++
 2 files changed, 25 insertions(+), 0 deletions(-)
drivers/scsi/scsi_transport_sas.c +16 -0
index 30d26e3..b17f763 100644
@@ -341,6 +341,22 @@ static int do_sas_phy_delete(struct device *dev, void *data)
 }
 
 /**
+ * is_sas_attached - check if device is SAS attached
+ * @sdev: scsi device to check
+ *
+ * returns true if the device is SAS attached
+ */
+int is_sas_attached(struct scsi_device *sdev)
+{
+	struct Scsi_Host *shost = sdev->host;
+
+	return shost->transportt->host_attrs.ac.class ==
+		&sas_host_class.class;
+}
+EXPORT_SYMBOL(is_sas_attached);
+
+
+/**
  * sas_remove_children  -  tear down a devices SAS data structures
  * @dev:	device belonging to the sas object
  *
include/scsi/scsi_transport_sas.h +9 -0
index 0bd71e2..a8fdd10 100644
@@ -10,6 +10,15 @@ struct scsi_transport_template;
 struct sas_rphy;
 struct request;
 
+#if !IS_ENABLED(CONFIG_SCSI_SAS_ATTRS)
+static inline int is_sas_attached(struct scsi_device *sdev)
+{
+	return 0;
+}
+#else
+extern int is_sas_attached(struct scsi_device *sdev);
+#endif
+
 static inline int sas_protocol_ata(enum sas_protocol proto)
 {
 	return ((proto & SAS_PROTOCOL_SATA) ||
Signature · 1 line
1.7.4.1
Reviewed-by on v1 1/3
#2 lore raw

Re: [PATCH 1/3] scsi_transport_sas: add is_sas_attached() function

Quoted 57 lines · James Bottomley

Adds a function designed to be callable any time (regardless of whether the transport attributes are configured or not) which returns true if the device is attached over a SAS transport. The design of this function is that transport specific functions can be embedded within a

if (is_sas_attached(sdev)) {
	...
}

which would be compiled out (and thus eliminate the symbols) if SAS is not configured.

---
  drivers/scsi/scsi_transport_sas.c |   16 ++++++++++++++++
  include/scsi/scsi_transport_sas.h |    9 +++++++++
  2 files changed, 25 insertions(+), 0 deletions(-)
drivers/scsi/scsi_transport_sas.c +16 -0
index 30d26e3..b17f763 100644
@@ -341,6 +341,22 @@ static int do_sas_phy_delete(struct device *dev, void *data)
  }
 
  /**
+ * is_sas_attached - check if device is SAS attached
+ * @sdev: scsi device to check
+ *
+ * returns true if the device is SAS attached
+ */
+int is_sas_attached(struct scsi_device *sdev)
+{
+	struct Scsi_Host *shost = sdev->host;
+
+	return shost->transportt->host_attrs.ac.class ==
+		&sas_host_class.class;
+}
+EXPORT_SYMBOL(is_sas_attached);
+
+
+/**
   * sas_remove_children  -  tear down a devices SAS data structures
   * @dev:	device belonging to the sas object
   *
include/scsi/scsi_transport_sas.h +2 -0
index 0bd71e2..a8fdd10 100644
@@ -10,6 +10,15 @@ struct scsi_transport_template;
  struct sas_rphy;
  struct request;
 
+#if !IS_ENABLED(CONFIG_SCSI_SAS_ATTRS)
+static inline int is_sas_attached(struct scsi_device *sdev)

include/scsi/scsi_transport_sas.h struct scsi_transport_template; · line 24

Quoted 10 lines
include/scsi/scsi_transport_sas.h +7 -0
+{
+	return 0;
+}
+#else
+extern int is_sas_attached(struct scsi_device *sdev);
+#endif
+
  static inline int sas_protocol_ata(enum sas_protocol proto)
  {
  	return ((proto & SAS_PROTOCOL_SATA) ||

Cheers,

Hannes

Signature · 9 lines
Dr. Hannes Reinecke		               zSeries & Storage
[email protected]			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
author maintainer
#3 lore raw

PATCH 2/3 scsi_transport_sas: add function to get SAS endpoint address

In reply to James Bottomley

For a device known to be SAS connected, this will return the endpoint address. This is useful for getting the SAS address of SATA devices.

---
 drivers/scsi/scsi_transport_sas.c |   14 ++++++++++++++
 include/scsi/scsi_transport_sas.h |    1 +
 2 files changed, 15 insertions(+), 0 deletions(-)
drivers/scsi/scsi_transport_sas.c +14 -0
index b17f763..80520e2 100644
@@ -383,6 +383,20 @@ void sas_remove_host(struct Scsi_Host *shost)
 EXPORT_SYMBOL(sas_remove_host);
 
 /**
+ * sas_get_address - return the SAS address of the device
+ * @sdev: scsi device
+ *
+ * Returns the SAS address of the scsi device
+ */
+u64 sas_get_address(struct scsi_device *sdev)
+{
+	struct sas_end_device *rdev = sas_sdev_to_rdev(sdev);
+
+	return rdev->rphy.identify.sas_address;
+}
+EXPORT_SYMBOL(sas_get_address);
+
+/**
  * sas_tlr_supported - checking TLR bit in vpd 0x90
  * @sdev: scsi device struct
  *
include/scsi/scsi_transport_sas.h +1 -0
index a8fdd10..13c0b2b 100644
@@ -189,6 +189,7 @@ extern int sas_phy_add(struct sas_phy *);
 extern void sas_phy_delete(struct sas_phy *);
 extern int scsi_is_sas_phy(const struct device *);
 
+u64 sas_get_address(struct scsi_device *);
 unsigned int sas_tlr_supported(struct scsi_device *);
 unsigned int sas_is_tlr_enabled(struct scsi_device *);
 void sas_disable_tlr(struct scsi_device *);
Signature · 1 line
1.7.4.1
Reviewed-by on v1 2/3
#4 lore raw

Re: [PATCH 2/3] scsi_transport_sas: add function to get SAS endpoint address

Quoted 40 lines · James Bottomley

For a device known to be SAS connected, this will return the endpoint address. This is useful for getting the SAS address of SATA devices.

---
  drivers/scsi/scsi_transport_sas.c |   14 ++++++++++++++
  include/scsi/scsi_transport_sas.h |    1 +
  2 files changed, 15 insertions(+), 0 deletions(-)
drivers/scsi/scsi_transport_sas.c +14 -0
index b17f763..80520e2 100644
@@ -383,6 +383,20 @@ void sas_remove_host(struct Scsi_Host *shost)
  EXPORT_SYMBOL(sas_remove_host);
 
  /**
+ * sas_get_address - return the SAS address of the device
+ * @sdev: scsi device
+ *
+ * Returns the SAS address of the scsi device
+ */
+u64 sas_get_address(struct scsi_device *sdev)
+{
+	struct sas_end_device *rdev = sas_sdev_to_rdev(sdev);
+
+	return rdev->rphy.identify.sas_address;
+}
+EXPORT_SYMBOL(sas_get_address);
+
+/**
   * sas_tlr_supported - checking TLR bit in vpd 0x90
   * @sdev: scsi device struct
   *
include/scsi/scsi_transport_sas.h +0 -0
index a8fdd10..13c0b2b 100644

include/scsi/scsi_transport_sas.h extern int sas_phy_add(struct sas_phy *); · line 195

Quoted 7 lines
include/scsi/scsi_transport_sas.h +1 -0
@@ -189,6 +189,7 @@ extern int sas_phy_add(struct sas_phy *);
  extern void sas_phy_delete(struct sas_phy *);
  extern int scsi_is_sas_phy(const struct device *);
 
+u64 sas_get_address(struct scsi_device *);
  unsigned int sas_tlr_supported(struct scsi_device *);
  unsigned int sas_is_tlr_enabled(struct scsi_device *);
  void sas_disable_tlr(struct scsi_device *);

Cheers,

Hannes

Signature · 9 lines
Dr. Hannes Reinecke		               zSeries & Storage
[email protected]			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
author maintainer
#5 lore raw

PATCH 3/3 ses: fix discovery of SATA devices in SAS enclosures

In reply to James Bottomley

The current discovery routines use the VPD 0x83 inquiry page to find the device SAS address and match it to the end point in the enclosure. This doesn't work for SATA devices because expanders (or hosts) simply make up an endpoint address for STP and thus the address returned by the VPD page never matches. Instead of doing this, for SAS attached devices, match by the direct endpoint address instead.

---
 drivers/scsi/ses.c |   22 ++++------------------
 1 files changed, 4 insertions(+), 18 deletions(-)
drivers/scsi/ses.c +4 -18
index 7d9cec5..1736935 100644
@@ -34,6 +34,8 @@
 #include <scsi/scsi_driver.h>
 #include <scsi/scsi_host.h>
 
+#include <scsi/scsi_transport_sas.h>
+
 struct ses_device {
 	unsigned char *page1;
 	unsigned char *page1_types;
@@ -571,31 +573,15 @@ static void ses_enclosure_data_process(struct enclosure_device *edev,
 static void ses_match_to_enclosure(struct enclosure_device *edev,
 				   struct scsi_device *sdev)
 {
-	unsigned char *desc;
 	struct efd efd = {
 		.addr = 0,
 	};
 
 	ses_enclosure_data_process(edev, to_scsi_device(edev->edev.parent), 0);
 
-	if (!sdev->vpd_pg83_len)
-		return;
-
-	desc = sdev->vpd_pg83 + 4;
-	while (desc < sdev->vpd_pg83 + sdev->vpd_pg83_len) {
-		enum scsi_protocol proto = desc[0] >> 4;
-		u8 code_set = desc[0] & 0x0f;
-		u8 piv = desc[1] & 0x80;
-		u8 assoc = (desc[1] & 0x30) >> 4;
-		u8 type = desc[1] & 0x0f;
-		u8 len = desc[3];
-
-		if (piv && code_set == 1 && assoc == 1
-		    && proto == SCSI_PROTOCOL_SAS && type == 3 && len == 8)
-			efd.addr = get_unaligned_be64(&desc[4]);
+	if (is_sas_attached(sdev))
+		efd.addr = sas_get_address(sdev);
 
-		desc += len + 4;
-	}
 	if (efd.addr) {
 		efd.dev = &sdev->sdev_gendev;
 
Signature · 1 line
1.7.4.1
#6 lore raw

Re: [PATCH 3/3] ses: fix discovery of SATA devices in SAS enclosures

reproduce:

Hi James,

[auto build test ERROR on v4.4-rc4]
[cannot apply to scsi/for-next next-20151209]

url:    https://github.com/0day-ci/linux/commits/James-Bottomley/Fix-the-problem-of-SATA-devices-within-SAS-enclosures/20151210-031802
config: i386-randconfig-b0-12100330 (attached as .config)
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

drivers/built-in.o: In function `ses_match_to_enclosure':

>> ses.c:(.text+0x1c04ad): undefined reference to `is_sas_attached'
>> ses.c:(.text+0x1c0519): undefined reference to `sas_get_address'

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation

author maintainer
#7 lore raw

Re: [PATCH 3/3] ses: fix discovery of SATA devices in SAS enclosures

Quoted 11 lines · kbuild test robot

Hi James,

[auto build test ERROR on v4.4-rc4]
[cannot apply to scsi/for-next next-20151209]

url:    https://github.com/0day-ci/linux/commits/James-Bottomley/Fix-the-problem-of-SATA-devices-within-SAS-enclosures/20151210-031802
config: i386-randconfig-b0-12100330 (attached as .config)
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 
Quoted 5 lines

All errors (new ones prefixed by >>):

drivers/built-in.o: In function `ses_match_to_enclosure':

Quoted 2 lines
Quoted 2 lines

ses.c:(.text+0x1c04ad): undefined reference to is_sas_attached' ses.c:(.text+0x1c0519): undefined reference to sas_get_address'

Oh, that's the perennial file built in but dependencies are modules problem. The fix is to add this line to the Kconfig for SCSI_ENCLOSURE

depends on m || SCSI_SAS_ATTRS != m

I'll post a v2 with that added.

James

author maintainer
#8 lore raw

PATCH v2 3/3 ses: fix discovery of SATA devices in SAS enclosures

In reply to James Bottomley

The current discovery routines use the VPD 0x83 inquiry page to find the device SAS address and match it to the end point in the enclosure. This doesn't work for SATA devices because expanders (or hosts) simply make up an endpoint address for STP and thus the address returned by the VPD page never matches. Instead of doing this, for SAS attached devices, match by the direct endpoint address instead.

drivers/scsi/Kconfig +1 -0
index 5f692ae..2a1d20e 100644
@@ -194,6 +194,7 @@ config CHR_DEV_SCH
 config SCSI_ENCLOSURE
 	tristate "SCSI Enclosure Support"
 	depends on SCSI && ENCLOSURE_SERVICES
+	depends on m || SCSI_SAS_ATTRS != m
 	help
 	  Enclosures are devices sitting on or in SCSI backplanes that
 	  manage devices.  If you have a disk cage, the chances are that
drivers/scsi/ses.c +4 -18
index 7d9cec5..1736935 100644
@@ -34,6 +34,8 @@
 #include <scsi/scsi_driver.h>
 #include <scsi/scsi_host.h>
 
+#include <scsi/scsi_transport_sas.h>
+
 struct ses_device {
 	unsigned char *page1;
 	unsigned char *page1_types;
@@ -571,31 +573,15 @@ static void ses_enclosure_data_process(struct enclosure_device *edev,
 static void ses_match_to_enclosure(struct enclosure_device *edev,
 				   struct scsi_device *sdev)
 {
-	unsigned char *desc;
 	struct efd efd = {
 		.addr = 0,
 	};
 
 	ses_enclosure_data_process(edev, to_scsi_device(edev->edev.parent), 0);
 
-	if (!sdev->vpd_pg83_len)
-		return;
-
-	desc = sdev->vpd_pg83 + 4;
-	while (desc < sdev->vpd_pg83 + sdev->vpd_pg83_len) {
-		enum scsi_protocol proto = desc[0] >> 4;
-		u8 code_set = desc[0] & 0x0f;
-		u8 piv = desc[1] & 0x80;
-		u8 assoc = (desc[1] & 0x30) >> 4;
-		u8 type = desc[1] & 0x0f;
-		u8 len = desc[3];
-
-		if (piv && code_set == 1 && assoc == 1
-		    && proto == SCSI_PROTOCOL_SAS && type == 3 && len == 8)
-			efd.addr = get_unaligned_be64(&desc[4]);
+	if (is_sas_attached(sdev))
+		efd.addr = sas_get_address(sdev);
 
-		desc += len + 4;
-	}
 	if (efd.addr) {
 		efd.dev = &sdev->sdev_gendev;
 
Reviewed-by on v2 3/3
#9 lore raw

Re: [PATCH v2 3/3] ses: fix discovery of SATA devices in SAS enclosures

Quoted 60 lines · James Bottomley

The current discovery routines use the VPD 0x83 inquiry page to find the device SAS address and match it to the end point in the enclosure. This doesn't work for SATA devices because expanders (or hosts) simply make up an endpoint address for STP and thus the address returned by the VPD page never matches. Instead of doing this, for SAS attached devices, match by the direct endpoint address instead.

drivers/scsi/Kconfig +1 -0
index 5f692ae..2a1d20e 100644
@@ -194,6 +194,7 @@ config CHR_DEV_SCH
  config SCSI_ENCLOSURE
  	tristate "SCSI Enclosure Support"
  	depends on SCSI && ENCLOSURE_SERVICES
+	depends on m || SCSI_SAS_ATTRS != m
  	help
  	  Enclosures are devices sitting on or in SCSI backplanes that
  	  manage devices.  If you have a disk cage, the chances are that
drivers/scsi/ses.c +2 -14
index 7d9cec5..1736935 100644
@@ -34,6 +34,8 @@
  #include <scsi/scsi_driver.h>
  #include <scsi/scsi_host.h>
 
+#include <scsi/scsi_transport_sas.h>
+
  struct ses_device {
  	unsigned char *page1;
  	unsigned char *page1_types;
@@ -571,31 +573,15 @@ static void ses_enclosure_data_process(struct enclosure_device *edev,
  static void ses_match_to_enclosure(struct enclosure_device *edev,
  				   struct scsi_device *sdev)
  {
-	unsigned char *desc;
  	struct efd efd = {
  		.addr = 0,
  	};
 
  	ses_enclosure_data_process(edev, to_scsi_device(edev->edev.parent), 0);
 
-	if (!sdev->vpd_pg83_len)
-		return;
-
-	desc = sdev->vpd_pg83 + 4;
-	while (desc < sdev->vpd_pg83 + sdev->vpd_pg83_len) {
-		enum scsi_protocol proto = desc[0] >> 4;
-		u8 code_set = desc[0] & 0x0f;
-		u8 piv = desc[1] & 0x80;
-		u8 assoc = (desc[1] & 0x30) >> 4;
-		u8 type = desc[1] & 0x0f;
-		u8 len = desc[3];
-
-		if (piv && code_set == 1 && assoc == 1

drivers/scsi/ses.c static void ses_enclosure_data_process(struct enclosure_device *edev, · line 587

Quoted 10 lines
drivers/scsi/ses.c +2 -4
-		    && proto == SCSI_PROTOCOL_SAS && type == 3 && len == 8)
-			efd.addr = get_unaligned_be64(&desc[4]);
+	if (is_sas_attached(sdev))
+		efd.addr = sas_get_address(sdev);
 
-		desc += len + 4;
-	}
  	if (efd.addr) {
  		efd.dev = &sdev->sdev_gendev;
 

Cheers,

Hannes

Signature · 9 lines
Dr. Hannes Reinecke		               zSeries & Storage
[email protected]			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html