Re: [PATCH] libsas: add host SMP processing
On Sat, 2007-12-29 at 14:24 +0900, FUJITA Tomonori wrote:
From: James Bottomley [EMAIL PROTECTED]
--- a/drivers/scsi/libsas/sas_expander.c
+++ b/drivers/scsi/libsas/sas_expander.c
@@ -1896,11 +1896,9 @@ int sas_smp_handler(struct Scsi_Host *shost, struct
sas_rphy *rphy,
}
/* no rphy means no smp target support (ie aic94xx host) */
- if (!rphy) {
- printk(%s: can we send a smp request to a host?\n,
- __FUNCTION__);
- return -EINVAL;
- }
+ if (!rphy)
+ return sas_smp_host_handler(shost, req, rsp);
+
I have one related question.
Currently, bsg doesn't return an error to user space since I had no
idea how to convert errors such as EINVAL and ENOMEM into
driver_status, transport_status, and device_status in struct sg_io_v4.
I think that it's confusing that bsg don't return an error even if SMP
requests aren't sent (e.g. devices are offline).
Do we need to map errors to the current error code in scsi/scsi.h
(like DID_*) or define a new one for SMP?
Neither, I think ... the DID codes are only for things that actually
pass through the SCSI stack. The way you implemented the smp functions
in bsg, they're direct queue handlers themselves (Incidentally, that's
another point about this: I think almost every use of bsg like this is
going to be for SG_IO only, so it makes sense to move the actual queue
handler into bsg, since they'll all share it).
The attached is the simplest patch that implements this. However, it
unfortunately can't be applied yet ... the current SMP tools send
receive buffers too large and libsas actually returns a data underrun
error (which is now propagated).
I'll fix it with a series of patches transferring the protocol handler
into bsg, adding error handling and correcting libsas.
James
diff --git a/block/bsg.c b/block/bsg.c
index 8e181ab..fbf0be3 100644
--- a/block/bsg.c
+++ b/block/bsg.c
@@ -837,6 +837,7 @@ static long bsg_ioctl(struct file *file, unsigned int cmd,
unsigned long arg)
{
struct bsg_device *bd = file-private_data;
int __user *uarg = (int __user *) arg;
+ int ret;
switch (cmd) {
/*
@@ -889,12 +890,13 @@ static long bsg_ioctl(struct file *file, unsigned int
cmd, unsigned long arg)
if (rq-next_rq)
bidi_bio = rq-next_rq-bio;
blk_execute_rq(bd-queue, NULL, rq, 0);
+ ret = rq-errors;
blk_complete_sgv4_hdr_rq(rq, hdr, bio, bidi_bio);
if (copy_to_user(uarg, hdr, sizeof(hdr)))
return -EFAULT;
- return 0;
+ return ret;
}
/*
* block device ioctls
diff --git a/drivers/scsi/scsi_transport_sas.c
b/drivers/scsi/scsi_transport_sas.c
index 87e786d..f2149d0 100644
--- a/drivers/scsi/scsi_transport_sas.c
+++ b/drivers/scsi/scsi_transport_sas.c
@@ -173,6 +173,7 @@ static void sas_smp_request(struct request_queue *q, struct
Scsi_Host *shost,
handler = to_sas_internal(shost-transportt)-f-smp_handler;
ret = handler(shost, rphy, req);
+ req-errors = ret;
spin_lock_irq(q-queue_lock);
-
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
Re: [PATCH] libsas: add host SMP processing
On Sat, 29 Dec 2007 09:44:32 -0600
James Bottomley [EMAIL PROTECTED] wrote:
On Sat, 2007-12-29 at 14:24 +0900, FUJITA Tomonori wrote:
From: James Bottomley [EMAIL PROTECTED]
--- a/drivers/scsi/libsas/sas_expander.c
+++ b/drivers/scsi/libsas/sas_expander.c
@@ -1896,11 +1896,9 @@ int sas_smp_handler(struct Scsi_Host *shost,
struct sas_rphy *rphy,
}
/* no rphy means no smp target support (ie aic94xx host) */
- if (!rphy) {
- printk(%s: can we send a smp request to a host?\n,
-__FUNCTION__);
- return -EINVAL;
- }
+ if (!rphy)
+ return sas_smp_host_handler(shost, req, rsp);
+
I have one related question.
Currently, bsg doesn't return an error to user space since I had no
idea how to convert errors such as EINVAL and ENOMEM into
driver_status, transport_status, and device_status in struct sg_io_v4.
I think that it's confusing that bsg don't return an error even if SMP
requests aren't sent (e.g. devices are offline).
Do we need to map errors to the current error code in scsi/scsi.h
(like DID_*) or define a new one for SMP?
Neither, I think ... the DID codes are only for things that actually
pass through the SCSI stack. The way you implemented the smp functions
in bsg, they're direct queue handlers themselves (Incidentally, that's
another point about this: I think almost every use of bsg like this is
going to be for SG_IO only, so it makes sense to move the actual queue
handler into bsg, since they'll all share it).
The attached is the simplest patch that implements this. However, it
unfortunately can't be applied yet ... the current SMP tools send
receive buffers too large and libsas actually returns a data underrun
error (which is now propagated).
bsg read/write interface doens't return errors in this way (compatible
with sg3 read/write interface). If we support only SG_IO for non SCSI
request/response protocols, then that's fine.
-
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
Re: [PATCH] libsas: add host SMP processing
On Sun, 2007-12-30 at 01:06 +0900, FUJITA Tomonori wrote:
On Sat, 29 Dec 2007 09:44:32 -0600
James Bottomley [EMAIL PROTECTED] wrote:
On Sat, 2007-12-29 at 14:24 +0900, FUJITA Tomonori wrote:
From: James Bottomley [EMAIL PROTECTED]
--- a/drivers/scsi/libsas/sas_expander.c
+++ b/drivers/scsi/libsas/sas_expander.c
@@ -1896,11 +1896,9 @@ int sas_smp_handler(struct Scsi_Host *shost,
struct sas_rphy *rphy,
}
/* no rphy means no smp target support (ie aic94xx host) */
- if (!rphy) {
- printk(%s: can we send a smp request to a host?\n,
- __FUNCTION__);
- return -EINVAL;
- }
+ if (!rphy)
+ return sas_smp_host_handler(shost, req, rsp);
+
I have one related question.
Currently, bsg doesn't return an error to user space since I had no
idea how to convert errors such as EINVAL and ENOMEM into
driver_status, transport_status, and device_status in struct sg_io_v4.
I think that it's confusing that bsg don't return an error even if SMP
requests aren't sent (e.g. devices are offline).
Do we need to map errors to the current error code in scsi/scsi.h
(like DID_*) or define a new one for SMP?
Neither, I think ... the DID codes are only for things that actually
pass through the SCSI stack. The way you implemented the smp functions
in bsg, they're direct queue handlers themselves (Incidentally, that's
another point about this: I think almost every use of bsg like this is
going to be for SG_IO only, so it makes sense to move the actual queue
handler into bsg, since they'll all share it).
The attached is the simplest patch that implements this. However, it
unfortunately can't be applied yet ... the current SMP tools send
receive buffers too large and libsas actually returns a data underrun
error (which is now propagated).
bsg read/write interface doens't return errors in this way (compatible
with sg3 read/write interface). If we support only SG_IO for non SCSI
request/response protocols, then that's fine.
OK, guilty of disliking the read/write interface (and therefore not
thinking about it). However, it's easy to fix. How about this? Note,
I think returning errors when they occur is more important than
preserving compatibility and swallowing the error somewhere, especially
as the bsg v3 interface *only* dealt with SCSI, so this is more like an
extension to non-scsi error returning. But if I'd had my way, bsg
wouldn't have had a read/write interface, so I'm happy to do whatever
people who actually use it want.
James
diff --git a/block/bsg.c b/block/bsg.c
index 8e181ab..69b0a9d 100644
--- a/block/bsg.c
+++ b/block/bsg.c
@@ -445,6 +445,15 @@ static int blk_complete_sgv4_hdr_rq(struct request *rq,
struct sg_io_v4 *hdr,
else
hdr-dout_resid = rq-data_len;
+ /*
+* If the request generated a negative error number, return it
+* (providing we aren't already returning an error); if it's
+* just a protocol response (i.e. non negative), that gets
+* processed above.
+*/
+ if (!ret rq-errors 0)
+ ret = rq-errors;
+
blk_rq_unmap_user(bio);
blk_put_request(rq);
@@ -837,6 +846,7 @@ static long bsg_ioctl(struct file *file, unsigned int cmd,
unsigned long arg)
{
struct bsg_device *bd = file-private_data;
int __user *uarg = (int __user *) arg;
+ int ret;
switch (cmd) {
/*
@@ -889,12 +899,12 @@ static long bsg_ioctl(struct file *file, unsigned int
cmd, unsigned long arg)
if (rq-next_rq)
bidi_bio = rq-next_rq-bio;
blk_execute_rq(bd-queue, NULL, rq, 0);
- blk_complete_sgv4_hdr_rq(rq, hdr, bio, bidi_bio);
+ ret = blk_complete_sgv4_hdr_rq(rq, hdr, bio, bidi_bio);
if (copy_to_user(uarg, hdr, sizeof(hdr)))
return -EFAULT;
- return 0;
+ return ret;
}
/*
* block device ioctls
diff --git a/drivers/scsi/scsi_transport_sas.c
b/drivers/scsi/scsi_transport_sas.c
index 87e786d..f2149d0 100644
--- a/drivers/scsi/scsi_transport_sas.c
+++ b/drivers/scsi/scsi_transport_sas.c
@@ -173,6 +173,7 @@ static void sas_smp_request(struct request_queue *q, struct
Scsi_Host *shost,
handler = to_sas_internal(shost-transportt)-f-smp_handler;
ret = handler(shost, rphy, req);
+ req-errors = ret;
spin_lock_irq(q-queue_lock);
-
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
Re: [PATCH] libsas: add host SMP processing
From: James Bottomley [EMAIL PROTECTED]
Subject: Re: [PATCH] libsas: add host SMP processing
Date: Sat, 29 Dec 2007 10:59:53 -0600
On Sun, 2007-12-30 at 01:06 +0900, FUJITA Tomonori wrote:
On Sat, 29 Dec 2007 09:44:32 -0600
James Bottomley [EMAIL PROTECTED] wrote:
On Sat, 2007-12-29 at 14:24 +0900, FUJITA Tomonori wrote:
From: James Bottomley [EMAIL PROTECTED]
--- a/drivers/scsi/libsas/sas_expander.c
+++ b/drivers/scsi/libsas/sas_expander.c
@@ -1896,11 +1896,9 @@ int sas_smp_handler(struct Scsi_Host *shost,
struct sas_rphy *rphy,
}
/* no rphy means no smp target support (ie aic94xx host) */
- if (!rphy) {
- printk(%s: can we send a smp request to a host?\n,
-__FUNCTION__);
- return -EINVAL;
- }
+ if (!rphy)
+ return sas_smp_host_handler(shost, req, rsp);
+
I have one related question.
Currently, bsg doesn't return an error to user space since I had no
idea how to convert errors such as EINVAL and ENOMEM into
driver_status, transport_status, and device_status in struct sg_io_v4.
I think that it's confusing that bsg don't return an error even if SMP
requests aren't sent (e.g. devices are offline).
Do we need to map errors to the current error code in scsi/scsi.h
(like DID_*) or define a new one for SMP?
Neither, I think ... the DID codes are only for things that actually
pass through the SCSI stack. The way you implemented the smp functions
in bsg, they're direct queue handlers themselves (Incidentally, that's
another point about this: I think almost every use of bsg like this is
going to be for SG_IO only, so it makes sense to move the actual queue
handler into bsg, since they'll all share it).
The attached is the simplest patch that implements this. However, it
unfortunately can't be applied yet ... the current SMP tools send
receive buffers too large and libsas actually returns a data underrun
error (which is now propagated).
bsg read/write interface doens't return errors in this way (compatible
with sg3 read/write interface). If we support only SG_IO for non SCSI
request/response protocols, then that's fine.
OK, guilty of disliking the read/write interface (and therefore not
thinking about it). However, it's easy to fix. How about this? Note,
I think returning errors when they occur is more important than
preserving compatibility and swallowing the error somewhere, especially
as the bsg v3 interface *only* dealt with SCSI, so this is more like an
extension to non-scsi error returning.
Oops, I remember wrongly. I've just realized that sgv3 returns errors
in this way (that is, returns negative values) though bsg doesn't (I
guess Jens did that because bsg read/write interface handle multiple
request/responses). There isn't the compatibility issue.
The patch looks fine.
Acked-by: FUJITA Tomonori [EMAIL PROTECTED]
But if I'd had my way, bsg
wouldn't have had a read/write interface, so I'm happy to do whatever
people who actually use it want.
I don't like the read/write interface much but OSD people need an
efficient interface to perform SCSI commands (the interface should to
handle multiple requests/response at a time and work
asynchronously). Some of them seem to use bsg read/write interface. So
I guess that we to invent a substitute to kill the read/write
interface.
-
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
[PATCH] libsas: add host SMP processing
This patch allows the sas host device to accept SMP commands. This
brings the libsas attached hosts on to a par with the firmware based
ones like mptsas.
James
diff --git a/drivers/scsi/libsas/Kconfig b/drivers/scsi/libsas/Kconfig
index c01a40d..18f33cd 100644
--- a/drivers/scsi/libsas/Kconfig
+++ b/drivers/scsi/libsas/Kconfig
@@ -38,6 +38,15 @@ config SCSI_SAS_ATA
Builds in ATA support into libsas. Will necessitate
the loading of libata along with libsas.
+config SCSI_SAS_HOST_SMP
+ bool Support for SMP interpretation for SAS hosts
+ default y
+ depends on SCSI_SAS_LIBSAS
+ help
+ Allows sas hosts to receive SMP frames. Selecting this
+ option builds an SMP interpreter into libsas. Say
+ N here if you want to save the few kb this consumes.
+
config SCSI_SAS_LIBSAS_DEBUG
bool Compile the SAS Domain Transport Attributes in debug mode
default y
diff --git a/drivers/scsi/libsas/Makefile b/drivers/scsi/libsas/Makefile
index fd387b9..60d6e93 100644
--- a/drivers/scsi/libsas/Makefile
+++ b/drivers/scsi/libsas/Makefile
@@ -35,3 +35,4 @@ libsas-y += sas_init.o \
sas_expander.o \
sas_scsi_host.o
libsas-$(CONFIG_SCSI_SAS_ATA) += sas_ata.o
+libsas-$(CONFIG_SCSI_SAS_HOST_SMP) += sas_host_smp.o
\ No newline at end of file
diff --git a/drivers/scsi/libsas/sas_expander.c
b/drivers/scsi/libsas/sas_expander.c
index 27674fe..76555b1 100644
--- a/drivers/scsi/libsas/sas_expander.c
+++ b/drivers/scsi/libsas/sas_expander.c
@@ -1896,11 +1896,9 @@ int sas_smp_handler(struct Scsi_Host *shost, struct
sas_rphy *rphy,
}
/* no rphy means no smp target support (ie aic94xx host) */
- if (!rphy) {
- printk(%s: can we send a smp request to a host?\n,
- __FUNCTION__);
- return -EINVAL;
- }
+ if (!rphy)
+ return sas_smp_host_handler(shost, req, rsp);
+
type = rphy-identify.device_type;
if (type != SAS_EDGE_EXPANDER_DEVICE
diff --git a/drivers/scsi/libsas/sas_host_smp.c
b/drivers/scsi/libsas/sas_host_smp.c
new file mode 100644
index 000..5b9370a
--- /dev/null
+++ b/drivers/scsi/libsas/sas_host_smp.c
@@ -0,0 +1,247 @@
+/*
+ * Serial Attached SCSI (SAS) Expander discovery and configuration
+ *
+ * Copyright (C) 2007 James E.J. Bottomley
+ * [EMAIL PROTECTED]
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; version 2 only.
+ */
+#include linux/scatterlist.h
+#include linux/blkdev.h
+
+#include sas_internal.h
+
+#include scsi/scsi_transport.h
+#include scsi/scsi_transport_sas.h
+#include ../scsi_sas_internal.h
+
+static void sas_host_smp_discover(struct sas_ha_struct *sas_ha, u8 *resp_data,
+ u8 phy_id)
+{
+ struct sas_phy *phy;
+ struct sas_rphy *rphy;
+
+ if (phy_id = sas_ha-num_phys) {
+ resp_data[2] = SMP_RESP_NO_PHY;
+ return;
+ }
+ resp_data[2] = SMP_RESP_FUNC_ACC;
+
+ phy = sas_ha-sas_phy[phy_id]-phy;
+ resp_data[9] = phy_id;
+ resp_data[13] = phy-negotiated_linkrate;
+ memcpy(resp_data + 16, sas_ha-sas_addr, SAS_ADDR_SIZE);
+ memcpy(resp_data + 24, sas_ha-sas_phy[phy_id]-attached_sas_addr,
+ SAS_ADDR_SIZE);
+ resp_data[40] = (phy-minimum_linkrate 4) |
+ phy-minimum_linkrate_hw;
+ resp_data[41] = (phy-maximum_linkrate 4) |
+ phy-maximum_linkrate_hw;
+
+ if (!sas_ha-sas_phy[phy_id]-port)
+ return;
+
+ rphy = sas_ha-sas_phy[phy_id]-port-port_dev-rphy;
+ resp_data[12] = rphy-identify.device_type 4;
+ resp_data[14] = rphy-identify.initiator_port_protocols;
+ resp_data[15] = rphy-identify.target_port_protocols;
+}
+
+static void sas_report_phy_sata(struct sas_ha_struct *sas_ha, u8 *resp_data,
+ u8 phy_id)
+{
+ struct sas_rphy *rphy;
+ struct dev_to_host_fis *fis;
+ int i;
+
+ if (phy_id = sas_ha-num_phys) {
+ resp_data[2] = SMP_RESP_NO_PHY;
+ return;
+ }
+
+ resp_data[2] = SMP_RESP_PHY_NO_SATA;
+
+ if (!sas_ha-sas_phy[phy_id]-port)
+ return;
+
+ rphy = sas_ha-sas_phy[phy_id]-port-port_dev-rphy;
+ fis = (struct dev_to_host_fis *)
+ sas_ha-sas_phy[phy_id]-port-port_dev-frame_rcvd;
+ if (rphy-identify.target_port_protocols != SAS_PROTOCOL_SATA)
+ return;
+
+ resp_data[2] = SMP_RESP_FUNC_ACC;
+ resp_data[9] = phy_id;
+ memcpy(resp_data + 16, sas_ha-sas_phy[phy_id]-attached_sas_addr,
+ SAS_ADDR_SIZE);
+
+ /* check to see if we have a valid d2h fis */
+ if (fis-fis_type != 0x34)
+
Re: [PATCH] libsas: add host SMP processing
From: James Bottomley [EMAIL PROTECTED]
Subject: [PATCH] libsas: add host SMP processing
Date: Fri, 28 Dec 2007 17:36:38 -0600
This patch allows the sas host device to accept SMP commands. This
brings the libsas attached hosts on to a par with the firmware based
ones like mptsas.
James
diff --git a/drivers/scsi/libsas/Kconfig b/drivers/scsi/libsas/Kconfig
index c01a40d..18f33cd 100644
--- a/drivers/scsi/libsas/Kconfig
+++ b/drivers/scsi/libsas/Kconfig
@@ -38,6 +38,15 @@ config SCSI_SAS_ATA
Builds in ATA support into libsas. Will necessitate
the loading of libata along with libsas.
+config SCSI_SAS_HOST_SMP
+ bool Support for SMP interpretation for SAS hosts
+ default y
+ depends on SCSI_SAS_LIBSAS
+ help
+ Allows sas hosts to receive SMP frames. Selecting this
+ option builds an SMP interpreter into libsas. Say
+ N here if you want to save the few kb this consumes.
+
config SCSI_SAS_LIBSAS_DEBUG
bool Compile the SAS Domain Transport Attributes in debug mode
default y
diff --git a/drivers/scsi/libsas/Makefile b/drivers/scsi/libsas/Makefile
index fd387b9..60d6e93 100644
--- a/drivers/scsi/libsas/Makefile
+++ b/drivers/scsi/libsas/Makefile
@@ -35,3 +35,4 @@ libsas-y += sas_init.o \
sas_expander.o \
sas_scsi_host.o
libsas-$(CONFIG_SCSI_SAS_ATA) += sas_ata.o
+libsas-$(CONFIG_SCSI_SAS_HOST_SMP) +=sas_host_smp.o
\ No newline at end of file
diff --git a/drivers/scsi/libsas/sas_expander.c
b/drivers/scsi/libsas/sas_expander.c
index 27674fe..76555b1 100644
--- a/drivers/scsi/libsas/sas_expander.c
+++ b/drivers/scsi/libsas/sas_expander.c
@@ -1896,11 +1896,9 @@ int sas_smp_handler(struct Scsi_Host *shost, struct
sas_rphy *rphy,
}
/* no rphy means no smp target support (ie aic94xx host) */
- if (!rphy) {
- printk(%s: can we send a smp request to a host?\n,
-__FUNCTION__);
- return -EINVAL;
- }
+ if (!rphy)
+ return sas_smp_host_handler(shost, req, rsp);
+
I have one related question.
Currently, bsg doesn't return an error to user space since I had no
idea how to convert errors such as EINVAL and ENOMEM into
driver_status, transport_status, and device_status in struct sg_io_v4.
I think that it's confusing that bsg don't return an error even if SMP
requests aren't sent (e.g. devices are offline).
Do we need to map errors to the current error code in scsi/scsi.h
(like DID_*) or define a new one for SMP?
-
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
