[PATCH 0/3] Fix the problem of SATA devices within SAS enclosures

[PATCH 0/3] Fix the problem of SATA devices within SAS enclosures

[James Bottomley]

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(-)

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

[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.

Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
---
 drivers/scsi/scsi_transport_sas.c |   16 ++++++++++++++++
 include/scsi/scsi_transport_sas.h |    9 +++++++++
 2 files changed, 25 insertions(+), 0 deletions(-)

diff --git a/drivers/scsi/scsi_transport_sas.c b/drivers/scsi/scsi_transport_sas.c
index 30d26e3..b17f763 100644
--- a/drivers/scsi/scsi_transport_sas.c
+++ b/drivers/scsi/scsi_transport_sas.c
@@ -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
  *
diff --git a/include/scsi/scsi_transport_sas.h b/include/scsi/scsi_transport_sas.h
index 0bd71e2..a8fdd10 100644
--- a/include/scsi/scsi_transport_sas.h
+++ b/include/scsi/scsi_transport_sas.h
@@ -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) ||
-- 
1.7.4.1

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

[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.

Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
---
 drivers/scsi/scsi_transport_sas.c |   14 ++++++++++++++
 include/scsi/scsi_transport_sas.h |    1 +
 2 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/drivers/scsi/scsi_transport_sas.c b/drivers/scsi/scsi_transport_sas.c
index b17f763..80520e2 100644
--- a/drivers/scsi/scsi_transport_sas.c
+++ b/drivers/scsi/scsi_transport_sas.c
@@ -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
  *
diff --git a/include/scsi/scsi_transport_sas.h b/include/scsi/scsi_transport_sas.h
index a8fdd10..13c0b2b 100644
--- a/include/scsi/scsi_transport_sas.h
+++ b/include/scsi/scsi_transport_sas.h
@@ -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 *);
-- 
1.7.4.1

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

[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.

Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
---
 drivers/scsi/ses.c |   22 ++++------------------
 1 files changed, 4 insertions(+), 18 deletions(-)

diff --git a/drivers/scsi/ses.c b/drivers/scsi/ses.c
index 7d9cec5..1736935 100644
--- a/drivers/scsi/ses.c
+++ b/drivers/scsi/ses.c
@@ -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;
 
-- 
1.7.4.1

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

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 776 bytes --]

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

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 24194 bytes --]

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

[James Bottomley]

On Thu, 2015-12-10 at 04:38 +0800, kbuild test robot wrote:
> 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'

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

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

[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.

Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>

diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
index 5f692ae..2a1d20e 100644
--- a/drivers/scsi/Kconfig
+++ b/drivers/scsi/Kconfig
@@ -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
diff --git a/drivers/scsi/ses.c b/drivers/scsi/ses.c
index 7d9cec5..1736935 100644
--- a/drivers/scsi/ses.c
+++ b/drivers/scsi/ses.c
@@ -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;
 

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

[Hannes Reinecke]

On 12/09/2015 08:12 PM, James Bottomley wrote:
> 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.
>
> Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
> ---
>   drivers/scsi/scsi_transport_sas.c |   16 ++++++++++++++++
>   include/scsi/scsi_transport_sas.h |    9 +++++++++
>   2 files changed, 25 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/scsi/scsi_transport_sas.c b/drivers/scsi/scsi_transport_sas.c
> index 30d26e3..b17f763 100644
> --- a/drivers/scsi/scsi_transport_sas.c
> +++ b/drivers/scsi/scsi_transport_sas.c
> @@ -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
>    *
> diff --git a/include/scsi/scsi_transport_sas.h b/include/scsi/scsi_transport_sas.h
> index 0bd71e2..a8fdd10 100644
> --- a/include/scsi/scsi_transport_sas.h
> +++ b/include/scsi/scsi_transport_sas.h
> @@ -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) ||
>
Reviewed-by: Hannes Reinecke <hare@suse.com>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		               zSeries & Storage
hare@suse.de			               +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 majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Hannes Reinecke]

On 12/09/2015 08:13 PM, James Bottomley wrote:
> 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.
>
> Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
> ---
>   drivers/scsi/scsi_transport_sas.c |   14 ++++++++++++++
>   include/scsi/scsi_transport_sas.h |    1 +
>   2 files changed, 15 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/scsi/scsi_transport_sas.c b/drivers/scsi/scsi_transport_sas.c
> index b17f763..80520e2 100644
> --- a/drivers/scsi/scsi_transport_sas.c
> +++ b/drivers/scsi/scsi_transport_sas.c
> @@ -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
>    *
> diff --git a/include/scsi/scsi_transport_sas.h b/include/scsi/scsi_transport_sas.h
> index a8fdd10..13c0b2b 100644
> --- a/include/scsi/scsi_transport_sas.h
> +++ b/include/scsi/scsi_transport_sas.h
> @@ -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 *);
>
Reviewed-by: Hannes Reinecke <hare@suse.com>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		               zSeries & Storage
hare@suse.de			               +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 majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Hannes Reinecke]

On 12/09/2015 09:56 PM, James Bottomley wrote:
> 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.
>
> Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
>
> diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
> index 5f692ae..2a1d20e 100644
> --- a/drivers/scsi/Kconfig
> +++ b/drivers/scsi/Kconfig
> @@ -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
> diff --git a/drivers/scsi/ses.c b/drivers/scsi/ses.c
> index 7d9cec5..1736935 100644
> --- a/drivers/scsi/ses.c
> +++ b/drivers/scsi/ses.c
> @@ -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: Hannes Reinecke <hare@suse.com>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		               zSeries & Storage
hare@suse.de			               +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 majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html