RE: [PATCH 3/3] mptsas: add SMP passthrough support via bsg

2007-07-31 Thread Moore, Eric 
On Monday, July 30, 2007 5:36 PM, FUJITA Tomonori wrote:
> 
> Looks good for me.
> 
> Here's an updated version. Can you add your signed-off or acked-by?
> 
> ---
> From: FUJITA Tomonori <[EMAIL PROTECTED]>
> Subject: [PATCH] mptsas: add SAS management protocol handler
> 
> This patch adds support for SAS Management Protocol (SMP) passthrough
> support via bsg.
> 
> Like libsas's smp support, user-space tools can send a smp request
> frame via sg_io_v4's dout_xferp and get a smp response frame via
> din_xferp. In addition, user-space tools can get the mpt reply via
> sg_io_v4's response field too if necessary.
> 
> Signed-off-by: FUJITA Tomonori <[EMAIL PROTECTED]>

Acked-by: Eric Moore <[EMAIL PROTECTED]>


-
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 3/3] mptsas: add SMP passthrough support via bsg

2007-07-30 Thread FUJITA Tomonori 
From: "Moore, Eric" <[EMAIL PROTECTED]>
Subject: RE: [PATCH 3/3] mptsas: add SMP passthrough support via bsg
Date: Mon, 30 Jul 2007 11:10:07 -0600

> On Sunday, July 29, 2007 1:37 AM, FUJITA Tomonori wrote:
> > > Eric, can I get your ACK on this patch?
> > 
> > One comment on the the patch:
> > 
> > +   if (!(ioc->sas_mgmt.status & MPT_IOCTL_STATUS_COMMAND_GOOD)) {
> > +   printk(KERN_ERR "%s: smp response invalid!\n", 
> > __FUNCTION__);
> > +   ret = -ENXIO;
> > +   }
> > 
> > We don't need this part since user-space can get mpt's reply, I think.
> >
> 
> Correct, this should be removed. The processing of the mpt reply has
> been pushed to user-space.On another note, according to the mpi
> specification, its says there should always be a reply message frame
> returned for every smp passthru.  So I thinking this would be the best
> way to close out the end of this function.  What do you think?
> 
> 
> + if (ioc->sas_mgmt.status & MPT_IOCTL_STATUS_RF_VALID) {
> + SmpPassthroughReply_t *smprep;
> +
> + smprep = (SmpPassthroughReply_t *)ioc->sas_mgmt.reply;
> + memcpy(req->sense, smprep, sizeof(*smprep));
> + req->sense_len = sizeof(*smprep);
> + } else
> + printk(KERN_ERR "%s: smp passthru reply failed to be
> returned\n", __FUNCTION__);
> + ret = -ENXIO;
> + }

Looks good for me.

Here's an updated version. Can you add your signed-off or acked-by?

---
From: FUJITA Tomonori <[EMAIL PROTECTED]>
Subject: [PATCH] mptsas: add SAS management protocol handler

This patch adds support for SAS Management Protocol (SMP) passthrough
support via bsg.

Like libsas's smp support, user-space tools can send a smp request
frame via sg_io_v4's dout_xferp and get a smp response frame via
din_xferp. In addition, user-space tools can get the mpt reply via
sg_io_v4's response field too if necessary.

Signed-off-by: FUJITA Tomonori <[EMAIL PROTECTED]>
---
 drivers/message/fusion/mptsas.c |  126 +++
 1 files changed, 126 insertions(+), 0 deletions(-)

diff --git a/drivers/message/fusion/mptsas.c b/drivers/message/fusion/mptsas.c
index 29add83..b9c69bf 100644
--- a/drivers/message/fusion/mptsas.c
+++ b/drivers/message/fusion/mptsas.c
@@ -1312,11 +1312,137 @@ mptsas_get_bay_identifier(struct sas_rphy *rphy)
return rc;
 }
 
+static int mptsas_smp_handler(struct Scsi_Host *shost, struct sas_rphy *rphy,
+ struct request *req)
+{
+   MPT_ADAPTER *ioc = ((MPT_SCSI_HOST *) shost->hostdata)->ioc;
+   MPT_FRAME_HDR *mf;
+   SmpPassthroughRequest_t *smpreq;
+   struct request *rsp = req->next_rq;
+   int ret;
+   int flagsLength;
+   unsigned long timeleft;
+   char *psge;
+   dma_addr_t dma_addr_in = 0;
+   dma_addr_t dma_addr_out = 0;
+   u64 sas_address = 0;
+
+   if (!rsp) {
+   printk(KERN_ERR "%s: the smp response space is missing\n",
+  __FUNCTION__);
+   return -EINVAL;
+   }
+
+   /* do we need to support multiple segments? */
+   if (req->bio->bi_vcnt > 1 || rsp->bio->bi_vcnt > 1) {
+   printk(KERN_ERR "%s: multiple segments req %u %u, rsp %u %u\n",
+  __FUNCTION__, req->bio->bi_vcnt, req->data_len,
+  rsp->bio->bi_vcnt, rsp->data_len);
+   return -EINVAL;
+   }
+
+   ret = mutex_lock_interruptible(&ioc->sas_mgmt.mutex);
+   if (ret)
+   goto out;
+
+   mf = mpt_get_msg_frame(mptsasMgmtCtx, ioc);
+   if (!mf) {
+   ret = -ENOMEM;
+   goto out_unlock;
+   }
+
+   smpreq = (SmpPassthroughRequest_t *)mf;
+   memset(smpreq, 0, sizeof(*smpreq));
+
+   smpreq->RequestDataLength = cpu_to_le16(req->data_len - 4);
+   smpreq->Function = MPI_FUNCTION_SMP_PASSTHROUGH;
+
+   if (rphy)
+   sas_address = rphy->identify.sas_address;
+   else {
+   struct mptsas_portinfo *port_info;
+
+   mutex_lock(&ioc->sas_topology_mutex);
+   port_info = mptsas_find_portinfo_by_handle(ioc, ioc->handle);
+   if (port_info && port_info->phy_info)
+   sas_address =
+   
port_info->phy_info[0].phy->identify.sas_address;
+   mutex_unlock(&ioc->sas_topology_mutex);
+   }
+
+   *((u64 *)&smpreq->SASAddress) = cpu_to_le64(sas_address);
+
+   psge = (char *)
+   (((int *) mf) + (offsetof(SmpPassthroughRequest_t, SGL) / 4)

RE: [PATCH 3/3] mptsas: add SMP passthrough support via bsg

2007-07-30 Thread Moore, Eric 
On Sunday, July 29, 2007 1:37 AM, FUJITA Tomonori wrote:
> > Eric, can I get your ACK on this patch?
> 
> One comment on the the patch:
> 
> + if (!(ioc->sas_mgmt.status & MPT_IOCTL_STATUS_COMMAND_GOOD)) {
> + printk(KERN_ERR "%s: smp response invalid!\n", 
> __FUNCTION__);
> + ret = -ENXIO;
> + }
> 
> We don't need this part since user-space can get mpt's reply, I think.
>

Correct, this should be removed. The processing of the mpt reply has
been pushed to user-space.On another note, according to the mpi
specification, its says there should always be a reply message frame
returned for every smp passthru.  So I thinking this would be the best
way to close out the end of this function.  What do you think?


+   if (ioc->sas_mgmt.status & MPT_IOCTL_STATUS_RF_VALID) {
+   SmpPassthroughReply_t *smprep;
+
+   smprep = (SmpPassthroughReply_t *)ioc->sas_mgmt.reply;
+   memcpy(req->sense, smprep, sizeof(*smprep));
+   req->sense_len = sizeof(*smprep);
+   } else
+   printk(KERN_ERR "%s: smp passthru reply failed to be
returned\n", __FUNCTION__);
+   ret = -ENXIO;
+   }

-
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 3/3] mptsas: add SMP passthrough support via bsg

2007-07-29 Thread FUJITA Tomonori 
From: FUJITA Tomonori <[EMAIL PROTECTED]>
Subject: RE: [PATCH 3/3] mptsas: add SMP passthrough support via bsg
Date: Sun, 29 Jul 2007 14:07:01 +0900

> From: "Moore, Eric" <[EMAIL PROTECTED]>
> Subject: RE: [PATCH 3/3] mptsas: add SMP passthrough support via bsg
> Date: Fri, 27 Jul 2007 17:24:19 -0600
> 
> > On  Thursday, July 26, 2007 6:44 PM, FUJITA Tomonori wrote:
> > > 
> > > Does this work for you? Sorry, I'm not at the lab now and can't test
> > > it. But I can do next week.
> > > 
> > > I also updated bsg's smp_rep_manufacturer to print the mpi's
> > > replay. You can get it from the git tree.
> > > 
> > 
> > yes this will work, and I pulled the updated sgv4, and saw your
> > inclusion of mptsas.h.
> 
> I confirmed that this patch sends mpi's unique reply back to user
> space.
> 
> When I send a wrong smp frame, I got:
> 
> nice:/home/fujita# ./sgv4-tools/smp_rep_manufacturer /sys/class/bsg/sas_host0
> IOCStatus=0x1 IOCLogInfo=0x0 SASStatus=0x2
> expected SMP frame response type, got=0x10
> 
> 
> Eric, can I get your ACK on this patch?

One comment on the the patch:

+   if (!(ioc->sas_mgmt.status & MPT_IOCTL_STATUS_COMMAND_GOOD)) {
+   printk(KERN_ERR "%s: smp response invalid!\n", __FUNCTION__);
+   ret = -ENXIO;
+   }

We don't need this part since user-space can get mpt's reply, I think.
-
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 3/3] mptsas: add SMP passthrough support via bsg

2007-07-28 Thread FUJITA Tomonori 
From: "Moore, Eric" <[EMAIL PROTECTED]>
Subject: RE: [PATCH 3/3] mptsas: add SMP passthrough support via bsg
Date: Fri, 27 Jul 2007 17:24:19 -0600

> On  Thursday, July 26, 2007 6:44 PM, FUJITA Tomonori wrote:
> > 
> > Does this work for you? Sorry, I'm not at the lab now and can't test
> > it. But I can do next week.
> > 
> > I also updated bsg's smp_rep_manufacturer to print the mpi's
> > replay. You can get it from the git tree.
> > 
> 
> yes this will work, and I pulled the updated sgv4, and saw your
> inclusion of mptsas.h.

I confirmed that this patch sends mpi's unique reply back to user
space.

When I send a wrong smp frame, I got:

nice:/home/fujita# ./sgv4-tools/smp_rep_manufacturer /sys/class/bsg/sas_host0
IOCStatus=0x1 IOCLogInfo=0x0 SASStatus=0x2
expected SMP frame response type, got=0x10


Eric, can I get your ACK on this patch?
-
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 3/3] mptsas: add SMP passthrough support via bsg

2007-07-27 Thread Moore, Eric 
On  Thursday, July 26, 2007 6:44 PM, FUJITA Tomonori wrote:
> 
> Does this work for you? Sorry, I'm not at the lab now and can't test
> it. But I can do next week.
> 
> I also updated bsg's smp_rep_manufacturer to print the mpi's
> replay. You can get it from the git tree.
> 

yes this will work, and I pulled the updated sgv4, and saw your
inclusion of mptsas.h.

Eric
-
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 3/3] mptsas: add SMP passthrough support via bsg

2007-07-26 Thread FUJITA Tomonori 
From: "Moore, Eric" <[EMAIL PROTECTED]>
Subject: RE: [PATCH 3/3] mptsas: add SMP passthrough support via bsg
Date: Thu, 26 Jul 2007 16:05:32 -0600

> On Thursday, July 26, 2007 11:09 AM, FUJITA Tomonori wrote:
> > For smp request/response, we use:
> > 
> > request -> not used
> > response -> not used
> > dout_xferp -> pointer to a smp request frame
> > din_xferp -> pointer to a smp response frame
> > 
> > 
> > So we could use response field to send vendor's unique response to
> > user space.
> > 
> > bsg wrongly assues the response field is used for sense buffer so the
> > maxium length is SCSI_SENSE_BUFFERSIZE, 96 bytes. I think that it's a
> > bit small for mpt's unique response. But, I'll fix the length
> > limitation bug soon.
> 
> Well that's cool.   It would be better the application had a description
> of failures, rather than returning a vague ENXIO. Looks like 7 bytes
> covers ioc_status(2 bytes), log_info (4 bytes), and sas_status(1 byte),
> or better yet, lets return the entire mf reply, which looks to be 28
> bytes. It doesn't matter.   However, I'm not sure how to copy smprep
> into sg_io_v4->response..I noticed that request frame is obtained from
> rsp->bio, and the reply frame is returned via rsp->bio.  Any
> suggestions?

Does this work for you? Sorry, I'm not at the lab now and can't test
it. But I can do next week.

I also updated bsg's smp_rep_manufacturer to print the mpi's
replay. You can get it from the git tree.


diff --git a/drivers/message/fusion/mptsas.c b/drivers/message/fusion/mptsas.c
index d506646..2533dad 100644
--- a/drivers/message/fusion/mptsas.c
+++ b/drivers/message/fusion/mptsas.c
@@ -1328,11 +1328,138 @@ mptsas_get_bay_identifier(struct sas_rphy *rphy)
return rc;
 }
 
+static int mptsas_smp_handler(struct Scsi_Host *shost, struct sas_rphy *rphy,
+ struct request *req)
+{
+   MPT_ADAPTER *ioc = ((MPT_SCSI_HOST *) shost->hostdata)->ioc;
+   MPT_FRAME_HDR *mf;
+   SmpPassthroughRequest_t *smpreq;
+   struct request *rsp = req->next_rq;
+   int ret;
+   int flagsLength;
+   unsigned long timeleft;
+   char *psge;
+   dma_addr_t dma_addr_in = 0;
+   dma_addr_t dma_addr_out = 0;
+   u64 sas_address = 0;
+
+   if (!rsp) {
+   printk(KERN_ERR "%s: the smp response space is missing\n",
+  __FUNCTION__);
+   return -EINVAL;
+   }
+
+   /* do we need to support multiple segments? */
+   if (req->bio->bi_vcnt > 1 || rsp->bio->bi_vcnt > 1) {
+   printk(KERN_ERR "%s: multiple segments req %u %u, rsp %u %u\n",
+  __FUNCTION__, req->bio->bi_vcnt, req->data_len,
+  rsp->bio->bi_vcnt, rsp->data_len);
+   return -EINVAL;
+   }
+
+   ret = mutex_lock_interruptible(&ioc->sas_mgmt.mutex);
+   if (ret)
+   goto out;
+
+   mf = mpt_get_msg_frame(mptsasMgmtCtx, ioc);
+   if (!mf) {
+   ret = -ENOMEM;
+   goto out_unlock;
+   }
+
+   smpreq = (SmpPassthroughRequest_t *)mf;
+   memset(smpreq, 0, sizeof(*smpreq));
+
+   smpreq->RequestDataLength = cpu_to_le16(req->data_len - 4);
+   smpreq->Function = MPI_FUNCTION_SMP_PASSTHROUGH;
+
+   if (rphy)
+   sas_address = rphy->identify.sas_address;
+   else {
+   struct mptsas_portinfo *port_info;
+
+   mutex_lock(&ioc->sas_topology_mutex);
+   port_info = mptsas_find_portinfo_by_handle(ioc, ioc->handle);
+   if (port_info && port_info->phy_info)
+   sas_address =
+   
port_info->phy_info[0].phy->identify.sas_address;
+   mutex_unlock(&ioc->sas_topology_mutex);
+   }
+
+   *((u64 *)&smpreq->SASAddress) = cpu_to_le64(sas_address);
+
+   psge = (char *)
+   (((int *) mf) + (offsetof(SmpPassthroughRequest_t, SGL) / 4));
+
+   /* request */
+   flagsLength = (MPI_SGE_FLAGS_SIMPLE_ELEMENT |
+  MPI_SGE_FLAGS_END_OF_BUFFER |
+  MPI_SGE_FLAGS_DIRECTION |
+  mpt_addr_size()) << MPI_SGE_FLAGS_SHIFT;
+   flagsLength |= (req->data_len - 4);
+
+   dma_addr_out = pci_map_single(ioc->pcidev, bio_data(req->bio),
+ req->data_len, PCI_DMA_BIDIRECTIONAL);
+   if (!dma_addr_out)
+   goto put_mf;
+   mpt_add_sge(psge, flagsLength, dma_addr_out);
+   psge += (sizeof(u32) + sizeof(dma_addr_t));
+
+   /* response */
+   flagsLength = MPT_SGE_FLAGS_SSIMPL

RE: [PATCH 3/3] mptsas: add SMP passthrough support via bsg

2007-07-26 Thread Moore, Eric 
On Thursday, July 26, 2007 11:09 AM, FUJITA Tomonori wrote:
> For smp request/response, we use:
> 
> request -> not used
> response -> not used
> dout_xferp -> pointer to a smp request frame
> din_xferp -> pointer to a smp response frame
> 
> 
> So we could use response field to send vendor's unique response to
> user space.
> 
> bsg wrongly assues the response field is used for sense buffer so the
> maxium length is SCSI_SENSE_BUFFERSIZE, 96 bytes. I think that it's a
> bit small for mpt's unique response. But, I'll fix the length
> limitation bug soon.

Well that's cool.   It would be better the application had a description
of failures, rather than returning a vague ENXIO. Looks like 7 bytes
covers ioc_status(2 bytes), log_info (4 bytes), and sas_status(1 byte),
or better yet, lets return the entire mf reply, which looks to be 28
bytes. It doesn't matter.   However, I'm not sure how to copy smprep
into sg_io_v4->response..I noticed that request frame is obtained from
rsp->bio, and the reply frame is returned via rsp->bio.  Any
suggestions?

Here is the reply message frame format,  Let me know if you need any
details on the those three fields.

/* Serial Management Protocol Passthrough Reply */
typedef struct _MSG_SMP_PASSTHROUGH_REPLY
{
U8  PassthroughFlags;   /* 00h */
U8  PhysicalPort;   /* 01h */
U8  MsgLength;  /* 02h */
U8  Function;   /* 03h */
U16 ResponseDataLength; /* 04h */
U8  Reserved1;  /* 06h */
U8  MsgFlags;   /* 07h */
U32 MsgContext; /* 08h */
U8  Reserved2;  /* 0Ch */
U8  SASStatus;  /* 0Dh */
U16 IOCStatus;  /* 0Eh */
U32 IOCLogInfo; /* 10h */
U32 Reserved3;  /* 14h */
U8  ResponseData[4];/* 18h */
}
-
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 3/3] mptsas: add SMP passthrough support via bsg

2007-07-26 Thread FUJITA Tomonori 
From: FUJITA Tomonori <[EMAIL PROTECTED]>
Subject: RE: [PATCH 3/3] mptsas: add SMP passthrough support via bsg
Date: Fri, 27 Jul 2007 02:09:05 +0900

> Use-space applications can put four memory addresses in the bsg (sg
> v4) interface: request, response, dout_xferp, and din_xferp.
> 
> For scsi commands, we use:
> 
> request -> pointer to a scsi command (cmdp in sgv3)
> response -> pointer to a sense buffer (sbp in sgv3)
> dout_xferp -> pointer to out data
> din_xferp -> pointer to in data
> 
> For smp request/response, we use:
> 
> request -> not used
> response -> not used
> dout_xferp -> pointer to a smp request frame
> din_xferp -> pointer to a smp response frame
> 
> 
> So we could use response field to send vendor's unique response to
> user space.
> 
> bsg wrongly assues the response field is used for sense buffer so the
> maxium length is SCSI_SENSE_BUFFERSIZE, 96 bytes. I think that it's a
> bit small for mpt's unique response. But, I'll fix the length
> limitation bug soon.

Oops, 96 bytes is large enough for mpt's unique response. Right?
-
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 3/3] mptsas: add SMP passthrough support via bsg

2007-07-26 Thread FUJITA Tomonori 
From: "Moore, Eric" <[EMAIL PROTECTED]>
Subject: RE: [PATCH 3/3] mptsas: add SMP passthrough support via bsg
Date: Thu, 26 Jul 2007 10:32:45 -0600

> On Thursday, July 26, 2007 4:09 AM, FUJITA Tomonori wrote: 
> > The SMP response's function result wasn't set correctly? bsg's
> > smp_rep_manufacturer didn't check the result so it showed garbase, I
> > guess.
> > 
> > BTW, I took the code to check the result from Doug's smp_utils and put
> > bsg's smp_rep_manufacturer:
> > 
> > http://www.kernel.org/pub/linux/kernel/people/tomo/smp/
> > 
> > or
> > 
> > git://git.kernel.org/pub/scm/linux/kernel/git/tomo/sgv4-tools.git
> 
> Thanks.with Doug Gilberts help, I have solved the git dilema thru
> firewall.  I need a socks proxy, which I found from
> http://tsocks.sourceforge.net/

Nice.


> > That's the reason why I said before, sense buffer is not used for smp
> > and using sense buffer would be the easiest way to push up the mpt
> > unique response to userspace.
> 
> do you suggest the scsi llds would would need to translate there own
> codes (in my case iocstatus/loginfo/sasstatus) into common sense
> sk/asc/ascq ?

No, I don't.

struct sg_io_v4 {
__s32 guard;/* [i] 'Q' to differentiate from v3 */
__u32 protocol; /* [i] 0 -> SCSI ,  */
__u32 subprotocol;  /* [i] 0 -> SCSI command, 1 -> SCSI task
   management function,  */

__u32 request_len;  /* [i] in bytes */
__u64 request;  /* [i], [*i] {SCSI: cdb} */
__u32 request_attr; /* [i] {SCSI: task attribute} */
__u32 request_tag;  /* [i] {SCSI: task tag (only if flagged)} */
__u32 request_priority; /* [i] {SCSI: task priority} */
__u32 max_response_len; /* [i] in bytes */
__u64 response; /* [i], [*o] {SCSI: (auto)sense data} */

/* "din_" for data in (from device); "dout_" for data out (to device) */
__u32 dout_xfer_len;/* [i] bytes to be transferred to device */
__u32 din_xfer_len; /* [i] bytes to be transferred from device */
__u64 dout_xferp;   /* [i], [*i] */
__u64 din_xferp;/* [i], [*o] */

...
__u32 response_len; /* [o] bytes of response actually written */


Use-space applications can put four memory addresses in the bsg (sg
v4) interface: request, response, dout_xferp, and din_xferp.

For scsi commands, we use:

request -> pointer to a scsi command (cmdp in sgv3)
response -> pointer to a sense buffer (sbp in sgv3)
dout_xferp -> pointer to out data
din_xferp -> pointer to in data

For smp request/response, we use:

request -> not used
response -> not used
dout_xferp -> pointer to a smp request frame
din_xferp -> pointer to a smp response frame


So we could use response field to send vendor's unique response to
user space.

bsg wrongly assues the response field is used for sense buffer so the
maxium length is SCSI_SENSE_BUFFERSIZE, 96 bytes. I think that it's a
bit small for mpt's unique response. But, I'll fix the length
limitation bug soon.


> > 
> > Here's an updated patch.
> > 
> 
>  The patch looks good.  It can be applied upstream.  I tested on x86_64,
> not big endian (I could try out on a ppc64 when I have time).  Here is
> the output from your latest tool with various expanders I have in my
> lab.
> 
> # ./smp_rep_manufacturer /sys/class/bsg/sas_host2
>   SAS-1.1 format: 0
>   vendor identification: LSI
>   product identification: Virtual SMP Port
>   product revision level:
> 
> # ./smp_rep_manufacturer /sys/class/bsg/expander-3:0
>   SAS-1.1 format: 1
>   vendor identification: LSILOGIC
>   product identification: x36-00.00.06.00
>   product revision level: 
>   component vendor identification: LSI
>   component id: 531
> 
> # ./smp_rep_manufacturer /sys/class/bsg/expander-3:1
>   SAS-1.1 format: 1
>   vendor identification: XYRATEX
>   product identification: SAS-01
>   product revision level: 03A
>   component vendor identification: LSI
>   component id: 517
> 
> # ./smp_rep_manufacturer /sys/class/bsg/expander-3:2
>   SAS-1.1 format: 1
>   vendor identification: HP
>   product identification: MSA 50-10D25G1
>   product revision level:
>   component vendor identification: LSI
>   component id: 512
>   component revision level: 1
> 
> # ./smp_rep_manufacturer /sys/class/bsg/expander-3:3
>   SAS-1.1 format: 0
>   vendor identification: LSILOGIC
>   product identification: SASx12 A.0
>   product revision level:

Looks great. Thanks.
-
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 3/3] mptsas: add SMP passthrough support via bsg

2007-07-26 Thread Moore, Eric 
On Thursday, July 26, 2007 4:09 AM, FUJITA Tomonori wrote: 
> The SMP response's function result wasn't set correctly? bsg's
> smp_rep_manufacturer didn't check the result so it showed garbase, I
> guess.
> 
> BTW, I took the code to check the result from Doug's smp_utils and put
> bsg's smp_rep_manufacturer:
> 
> http://www.kernel.org/pub/linux/kernel/people/tomo/smp/
> 
> or
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/tomo/sgv4-tools.git

Thanks.with Doug Gilberts help, I have solved the git dilema thru
firewall.  I need a socks proxy, which I found from
http://tsocks.sourceforge.net/


> 
> That's the reason why I said before, sense buffer is not used for smp
> and using sense buffer would be the easiest way to push up the mpt
> unique response to userspace.

do you suggest the scsi llds would would need to translate there own
codes (in my case iocstatus/loginfo/sasstatus) into common sense
sk/asc/ascq ?


> 
> Here's an updated patch.
> 

 The patch looks good.  It can be applied upstream.  I tested on x86_64,
not big endian (I could try out on a ppc64 when I have time).  Here is
the output from your latest tool with various expanders I have in my
lab.

# ./smp_rep_manufacturer /sys/class/bsg/sas_host2
  SAS-1.1 format: 0
  vendor identification: LSI
  product identification: Virtual SMP Port
  product revision level:

# ./smp_rep_manufacturer /sys/class/bsg/expander-3:0
  SAS-1.1 format: 1
  vendor identification: LSILOGIC
  product identification: x36-00.00.06.00
  product revision level: 
  component vendor identification: LSI
  component id: 531

# ./smp_rep_manufacturer /sys/class/bsg/expander-3:1
  SAS-1.1 format: 1
  vendor identification: XYRATEX
  product identification: SAS-01
  product revision level: 03A
  component vendor identification: LSI
  component id: 517

# ./smp_rep_manufacturer /sys/class/bsg/expander-3:2
  SAS-1.1 format: 1
  vendor identification: HP
  product identification: MSA 50-10D25G1
  product revision level:
  component vendor identification: LSI
  component id: 512
  component revision level: 1

# ./smp_rep_manufacturer /sys/class/bsg/expander-3:3
  SAS-1.1 format: 0
  vendor identification: LSILOGIC
  product identification: SASx12 A.0
  product revision level:
-
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 3/3] mptsas: add SMP passthrough support via bsg

2007-07-26 Thread FUJITA Tomonori 
From: "Moore, Eric" <[EMAIL PROTECTED]>
Subject: RE: [PATCH 3/3] mptsas: add SMP passthrough support via bsg
Date: Wed, 25 Jul 2007 18:21:38 -0600

> > > > # ./sgv4-tools/smp_rep_manufacturer /sys/class/bsg/expander-0:1
> > > > 
> > > > 
> > > > I think that James tested this with aic94xx, however, I guess that
> > > > nobody has tested this with mptsas.
> > > > 
> > > 
> > > I got garbage (I'm using the 2.6.23-git11 patch from last 
> > week, before
> > > rc1):
> > > 
> > > # ./smp_rep_manufacturer /sys/class/bsg/expander-2:0
> > >   SAS-1.1 format: 1
> > >   vendor identification: _BUS_ADD
> > >   product identification: RESS=unix:abstra
> > >   product revision level: ct=/
> > >   component vendor identification: tmp/dbus
> > >   component id: 11609
> > >   component revision level: 69
> > > 
> > > With Doug Gilberts tools it works:
> > > 
> > > # smp_rep_manufacturer --sa=0x500605b016b0 /dev/mptctl
> > > Report manufacturer response:
> > >   SAS-1.1 format: 0
> > >   vendor identification: LSILOGIC
> > >   product identification: SASx12 A.0
> > >   product revision level:
> > > 
> > > 
> > > Also, unloading and reloading the driver resulted in two expander
> > > entryies in /sys/class/bsg.The old entry was not deleted when I
> > > unloaded the driver.  When I tryied to run 
> > smp_rep_manufacture on the
> > > old expander instance, it panicked.
> 
> With a sas analyzer, I figured out today why the bsg version of
> smp_rep_manufacture is not working.There is a bug in
> mptsas_smp_handler. The calculation of the first scatter gather element
> size for the outbound data is incorrect.   Its being set to the resonse
> data length, when instead it should be the request data legth.
> 
> This is incorrect:
> 
> + flagsLength |= (rsp->data_len - 4);
> 
> It should be 
> 
> + flagsLength |= (smpreq->RequestDataLength);

Sorry for the bug.

Do we need to use (req->data_len - 4)? As you said, we set
smpreq->RequestDataLength with cpu_to_le16.


> > But probably, the tool still don't work against an expander. Does it
> > work against the Virtual SMP Port?
> > 
> 
> I tried out this by passing /sys/class/bsg/sas_host0, and I saw it
> return Virtual SMP.  I guess this can be left in.

Nice. With that fix, bsg's smp_rep_manufacturer works against an
expander too?


> > Oh, I thought that LSI wants to send the smp_reply back to user space
> > since Doug's smp-utils does. But If you don't want, I'll just put the
> > response check code that you suggested in the previous mail.
> > 
> 
> On second thought, It would be nice to have iocstatus, iocloginfo, and
> sasstatus available in user space, that way the appliacation will have a
> better understanding of what error occurred.

Yeah.


> Without that info, how will you it know if the response data you
> receive is valid data?  For instance, before I identified the bug in
> the sgel size, you were displaying garbage.

The SMP response's function result wasn't set correctly? bsg's
smp_rep_manufacturer didn't check the result so it showed garbase, I
guess.

BTW, I took the code to check the result from Doug's smp_utils and put
bsg's smp_rep_manufacturer:

http://www.kernel.org/pub/linux/kernel/people/tomo/smp/

or

git://git.kernel.org/pub/scm/linux/kernel/git/tomo/sgv4-tools.git


> The driver could have prevented that by returning -ENXIO I guess,
> instead how about pushing up that info to user space.

Agree.

> what do you think?maybe there should be some translation to common
> error returns code between the varous sas vendors supporting this
> portal.

aic94xx gives only the smp response. It doesn't have its unique
reply. I like the idea that user-space tools can simply put a smp
reqeust frame to out-buffer and get a smp response frame from
in-buffer. I'm not sure about inventing our smp response header and
putting it with a smp response frame to in-buffer.

That's the reason why I said before, sense buffer is not used for smp
and using sense buffer would be the easiest way to push up the mpt
unique response to userspace.

Here's an updated patch.


diff --git a/drivers/message/fusion/mptsas.c b/drivers/message/fusion/mptsas.c
index d506646..f2965e7 100644
--- a/drivers/message/fusion/mptsas.c
+++ b/drivers/message/fusion/mptsas.c
@@ -1328,11 +1328,139 @@ mptsas_get_bay_identifier(struct sas_rphy *rphy)
return rc;
 }
 
+static int mptsas_smp_handler(struct Scsi_Host *shost, struct

RE: [PATCH 3/3] mptsas: add SMP passthrough support via bsg

2007-07-25 Thread Moore, Eric 
On  Tuesday, July 24, 2007 6:48 PM, FUJITA Tomonori wrote:
> > I hadn't enabled bsg support in the linux kernel, that was 
> my problem.
> 
> What do you mean? You might hit the bug that you can't build scsi as a
> modular. It was fixed before rc1.
> 

The issue is I'm new to BSG, and I didn't know I needed to enable
CONFIG_BLK_DEV_BSG in the kernel config.   I upgraded today to rc1, and
your correct, I don't have to link the scsi mod into kernel.   Don't
worry about this point, I'm squared away now.

> 
> > > # ./sgv4-tools/smp_rep_manufacturer /sys/class/bsg/expander-0:1
> > > 
> > > 
> > > I think that James tested this with aic94xx, however, I guess that
> > > nobody has tested this with mptsas.
> > > 
> > 
> > I got garbage (I'm using the 2.6.23-git11 patch from last 
> week, before
> > rc1):
> > 
> > # ./smp_rep_manufacturer /sys/class/bsg/expander-2:0
> >   SAS-1.1 format: 1
> >   vendor identification: _BUS_ADD
> >   product identification: RESS=unix:abstra
> >   product revision level: ct=/
> >   component vendor identification: tmp/dbus
> >   component id: 11609
> >   component revision level: 69
> > 
> > With Doug Gilberts tools it works:
> > 
> > # smp_rep_manufacturer --sa=0x500605b016b0 /dev/mptctl
> > Report manufacturer response:
> >   SAS-1.1 format: 0
> >   vendor identification: LSILOGIC
> >   product identification: SASx12 A.0
> >   product revision level:
> > 
> > 
> > Also, unloading and reloading the driver resulted in two expander
> > entryies in /sys/class/bsg.The old entry was not deleted when I
> > unloaded the driver.  When I tryied to run 
> smp_rep_manufacture on the
> > old expander instance, it panicked.

With a sas analyzer, I figured out today why the bsg version of
smp_rep_manufacture is not working.There is a bug in
mptsas_smp_handler. The calculation of the first scatter gather element
size for the outbound data is incorrect.   Its being set to the resonse
data length, when instead it should be the request data legth.

This is incorrect:

+   flagsLength |= (rsp->data_len - 4);

It should be 

+   flagsLength |= (smpreq->RequestDataLength);


> 
> I forgot to remove bsg entries. James fixed the bug. Please try
> 2.6.23-rc1.

thanks, I noticed its fixed in rc1


> 
> But probably, the tool still don't work against an expander. Does it
> work against the Virtual SMP Port?
> 

I tried out this by passing /sys/class/bsg/sas_host0, and I saw it
return Virtual SMP.  I guess this can be left in.


> Oh, I thought that LSI wants to send the smp_reply back to user space
> since Doug's smp-utils does. But If you don't want, I'll just put the
> response check code that you suggested in the previous mail.
> 

On second thought, It would be nice to have iocstatus, iocloginfo, and
sasstatus available in user space, that way the appliacation will have a
better understanding of what error occurred.   Without that info, how
will you it know if the response data you receive is valid data?   For
instance, before I identified the bug in the sgel size, you were
displaying garbage.   The driver could have prevented that by returning
-ENXIO I guess, instead how about pushing up that info to user space.
what do you think?maybe there should be some translation to common
error returns code between the varous sas vendors supporting this
portal.

Eric
-
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 3/3] mptsas: add SMP passthrough support via bsg

2007-07-24 Thread FUJITA Tomonori 
From: "Moore, Eric" <[EMAIL PROTECTED]>
Subject: RE: [PATCH 3/3] mptsas: add SMP passthrough support via bsg
Date: Tue, 24 Jul 2007 18:22:08 -0600

> > > I'm not sure what the intent of this else case.
> > 
> > This code is for an "invisible" SMP target in LSI SAS HBAs. There are
> > better ways to get the target's address, I think. Any suggestions?
> > 
> 
> I've never heard that we(as in LSI) are attaching invisable SMP targets
> to HBA's.  I've seen the Virtual SMP Port, but those are not hidden, the
> driver would report them to the transport layer, therefore rphy should
> be non-zero.   For instance a 12 phy expander would have 13 phys(with
> the last being the virtual phy).   I doubt we need to have this else
> case in the code, as it will be returning the sas_address to the first
> attached device(which may not always be an expander).Are you
> executing the else path?

In the else path, I try to get the virtual phy's address. That is, I
try to do what sas_low_phy_scandir_select() in Doug's lsscsi tool
does.
-
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 3/3] mptsas: add SMP passthrough support via bsg

2007-07-24 Thread FUJITA Tomonori 
From: "Moore, Eric" <[EMAIL PROTECTED]>
Subject: RE: [PATCH 3/3] mptsas: add SMP passthrough support via bsg
Date: Tue, 24 Jul 2007 18:22:08 -0600

> On Monday, July 23, 2007 11:28 PM, FUJITA Tomonori wrote:
> 
> > 
> > With 2.6.23-rc1 + mptsas smp patch, you get directories /sys/class/bsg
> > like:
> 
> I hadn't enabled bsg support in the linux kernel, that was my problem.

What do you mean? You might hit the bug that you can't build scsi as a
modular. It was fixed before rc1.


> > # ./sgv4-tools/smp_rep_manufacturer /sys/class/bsg/expander-0:1
> > 
> > 
> > I think that James tested this with aic94xx, however, I guess that
> > nobody has tested this with mptsas.
> > 
> 
> I got garbage (I'm using the 2.6.23-git11 patch from last week, before
> rc1):
> 
> # ./smp_rep_manufacturer /sys/class/bsg/expander-2:0
>   SAS-1.1 format: 1
>   vendor identification: _BUS_ADD
>   product identification: RESS=unix:abstra
>   product revision level: ct=/
>   component vendor identification: tmp/dbus
>   component id: 11609
>   component revision level: 69
> 
> With Doug Gilberts tools it works:
> 
> # smp_rep_manufacturer --sa=0x500605b016b0 /dev/mptctl
> Report manufacturer response:
>   SAS-1.1 format: 0
>   vendor identification: LSILOGIC
>   product identification: SASx12 A.0
>   product revision level:
> 
> 
> Also, unloading and reloading the driver resulted in two expander
> entryies in /sys/class/bsg.The old entry was not deleted when I
> unloaded the driver.  When I tryied to run smp_rep_manufacture on the
> old expander instance, it panicked.

I forgot to remove bsg entries. James fixed the bug. Please try
2.6.23-rc1.

But probably, the tool still don't work against an expander. Does it
work against the Virtual SMP Port?


> > > I'm not sure what the intent of this else case.
> > 
> > This code is for an "invisible" SMP target in LSI SAS HBAs. There are
> > better ways to get the target's address, I think. Any suggestions?
> > 
> 
> I've never heard that we(as in LSI) are attaching invisable SMP targets
> to HBA's.  I've seen the Virtual SMP Port, but those are not hidden, the
> driver would report them to the transport layer, therefore rphy should
> be non-zero.

I just used Doug's word, "invisible" SMP target.


>   For instance a 12 phy expander would have 13 phys(with
> the last being the virtual phy).   I doubt we need to have this else
> case in the code, as it will be returning the sas_address to the first
> attached device(which may not always be an expander).Are you
> executing the else path?

I guess so. If I do:

./sgv4-tools/smp_rep_manufacturer /sys/class/bsg/sas_host0

I hit the else path (check sas_bsg_initialize in scsi_transport_sas.c).


> > Yeah, I guess that it would be better to send mpt's smpReply to user
> > space then user-space tools can examine it. You know, That's what mpt
> > ioctl and Doug' smp_utils do. But we can't use the in-buffer for this
> > since it's used for the smp response. We could use sense buffer for
> > this.
> >
> 
> there is no sense data associated to a SMP request.there is
> response-data, but that is something else.

Yeah, I know. I just said we can't use the in-buffer for mpt's
smpReply.


> I'm not sure rather it would be good idea to send the smp_reply back up
> the stack, as this data would be unique only to fusion.  In the reply,
> there is ioc_status, loginfo, and sas_status.  With this info, you can
> determine how to process the SMP request.  The sas_status defines are
> located at the top of mpi_sas.h (look for Values of SASStatus).
> ioc_status is in mpi.h (look for MPI_IOCSTATUS_XXX), and loginfo is
> defined in mpi_log_sas.h.

Oh, I thought that LSI wants to send the smp_reply back to user space
since Doug's smp-utils does. But If you don't want, I'll just put the
response check code that you suggested in the previous mail.

Thanks,
-
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 3/3] mptsas: add SMP passthrough support via bsg

2007-07-24 Thread Moore, Eric 
On Monday, July 23, 2007 11:28 PM, FUJITA Tomonori wrote:

> 
> With 2.6.23-rc1 + mptsas smp patch, you get directories /sys/class/bsg
> like:

I hadn't enabled bsg support in the linux kernel, that was my problem.

> 
> # ./sgv4-tools/smp_rep_manufacturer /sys/class/bsg/expander-0:1
> 
> 
> I think that James tested this with aic94xx, however, I guess that
> nobody has tested this with mptsas.
> 

I got garbage (I'm using the 2.6.23-git11 patch from last week, before
rc1):

# ./smp_rep_manufacturer /sys/class/bsg/expander-2:0
  SAS-1.1 format: 1
  vendor identification: _BUS_ADD
  product identification: RESS=unix:abstra
  product revision level: ct=/
  component vendor identification: tmp/dbus
  component id: 11609
  component revision level: 69

With Doug Gilberts tools it works:

# smp_rep_manufacturer --sa=0x500605b016b0 /dev/mptctl
Report manufacturer response:
  SAS-1.1 format: 0
  vendor identification: LSILOGIC
  product identification: SASx12 A.0
  product revision level:


Also, unloading and reloading the driver resulted in two expander
entryies in /sys/class/bsg.The old entry was not deleted when I
unloaded the driver.  When I tryied to run smp_rep_manufacture on the
old expander instance, it panicked.

> 
> > I was trying to run the smp_rep_manufacture from sgv4_tools, and I
> > got that error.  Due to that, I have not able to test this patch.
> 
> What is the error message?
> 

The application complained it couldn't bind to a bsg device.  Well  part
of the problem is I wasn't passing anything on the command line.
Anyways, I figured it out.

> > I'm not sure what the intent of this else case.
> 
> This code is for an "invisible" SMP target in LSI SAS HBAs. There are
> better ways to get the target's address, I think. Any suggestions?
> 

I've never heard that we(as in LSI) are attaching invisable SMP targets
to HBA's.  I've seen the Virtual SMP Port, but those are not hidden, the
driver would report them to the transport layer, therefore rphy should
be non-zero.   For instance a 12 phy expander would have 13 phys(with
the last being the virtual phy).   I doubt we need to have this else
case in the code, as it will be returning the sas_address to the first
attached device(which may not always be an expander).Are you
executing the else path?



> Yeah, I guess that it would be better to send mpt's smpReply to user
> space then user-space tools can examine it. You know, That's what mpt
> ioctl and Doug' smp_utils do. But we can't use the in-buffer for this
> since it's used for the smp response. We could use sense buffer for
> this.
>

there is no sense data associated to a SMP request.there is
response-data, but that is something else.

I'm not sure rather it would be good idea to send the smp_reply back up
the stack, as this data would be unique only to fusion.  In the reply,
there is ioc_status, loginfo, and sas_status.  With this info, you can
determine how to process the SMP request.  The sas_status defines are
located at the top of mpi_sas.h (look for Values of SASStatus).
ioc_status is in mpi.h (look for MPI_IOCSTATUS_XXX), and loginfo is
defined in mpi_log_sas.h.


Eric Moore

  
-
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 3/3] mptsas: add SMP passthrough support via bsg

2007-07-23 Thread FUJITA Tomonori 
From: "Moore, Eric" <[EMAIL PROTECTED]>
Subject: RE: [PATCH 3/3] mptsas: add SMP passthrough support via bsg
Date: Mon, 23 Jul 2007 18:11:34 -0600

> Tomo - do you have any documentation on how to specify a bsg device?

Sorry, I don't. But it's not difficult.

With 2.6.23-rc1 + mptsas smp patch, you get directories /sys/class/bsg
like:

[EMAIL PROTECTED]:/sys/class/bsg$ ls
0:0:0:0  0:0:1:0  end_device-0:0  end_device-0:1  sas_host0

I don't have any expanders but if you have, you have a directory like
expander-0:1 under /sys/class/bsg.

I can run smp_rep_manufacturer against "invisible" SMP target in my
LSI SAS HBA like this:

nice:/home/fujita# ./sgv4-tools/smp_rep_manufacturer /sys/class/bsg/sas_host0
  SAS-1.1 format: 0
  vendor identification: LSI
  product identification: Virtual SMP Port
  product revision level:

Hopefully, you can do against expanders:

# ./sgv4-tools/smp_rep_manufacturer /sys/class/bsg/expander-0:1


I think that James tested this with aic94xx, however, I guess that
nobody has tested this with mptsas.


> I was trying to run the smp_rep_manufacture from sgv4_tools, and I
> got that error.  Due to that, I have not able to test this patch.

What is the error message?


> However, here are some feedback with regards to the patch:

Thanks.

> 
> On Sunday, July 08, 2007 9:52 PM, FUJITA Tomonori wrote: 
> 
> > +
> > +smpreq->RequestDataLength = req->data_len - 4;
> 
> Our firmware formats the data in little endian, so you'll need to
> convert this inorder for it work under other endian formats, like sparc
> and ppc.  So this code should be:
> 
> smpreq->RequestDataLength = cpu_to_le16(req->data_len - 4);

I see. I'll fix it.


> > +smpreq->Function = MPI_FUNCTION_SMP_PASSTHROUGH;
> > +
> > +   if (rphy)
> > +   memcpy(&smpreq->SASAddress, &rphy->identify.sas_address,
> > +  sizeof(smpreq->SASAddress));
> 
> should be using cpu_to_le64 before the data is put into
> smpreq->SASAddress

Thanks, I will fix it.


> > +   else {
> > +   struct mptsas_portinfo *port_info;
> > +
> > +   mutex_lock(&ioc->sas_topology_mutex);
> > +   port_info = mptsas_find_portinfo_by_handle(ioc, 
> > ioc->handle);
> > +   if (port_info && port_info->phy_info)
> > +   memcpy(&smpreq->SASAddress,
> > +  
> > &(port_info->phy_info[0].phy->identify.sas_address),
> > +  sizeof(smpreq->SASAddress));
> > +   mutex_unlock(&ioc->sas_topology_mutex);
> > +   }
> 
> I'm not sure what the intent of this else case.

This code is for an "invisible" SMP target in LSI SAS HBAs. There are
better ways to get the target's address, I think. Any suggestions?


>  I think it should be
> deleted, and replaced with a return of ENXIO.The sas_topology is a
> flat model, with the first object the intiatiator, and the other objects
> in the list are expanders.What your your attempting to obtain is the
> first port object connected to the initiator, which could be anything,
> for instance it could be an end device, instead of expander.So I
> think if your rphy object is not returning a valid sas_address, then
> return instead of attempting to find something from the sas_topology.
> I hope the API is making sure this is an expander before mptsas is even
> called, is that handled?

> > +   /* a reply frame is expected */
> > +   if (!(ioc->sas_mgmt.status & MPT_IOCTL_STATUS_RF_VALID)) {
> > +   printk("%s: the smp response invalid!\n", __FUNCTION__);
> > +   ret = -ENXIO;
> > +   }
> > +
> 
> I think in addition to having a valid reply, I think you should look at
> the iocstatus to insure the reply itself was successfull, and that there
> were no data underruns.
> 
> ioc_status = le16_to_cpu(smpReply->IOCStatus) & MPI_IOCSTATUS_MASK;
> if ((ioc_status != MPI_IOCSTATUS_SUCCESS) &&
> (ioc_status != MPI_IOCSTATUS_SCSI_DATA_UNDERRUN)) {
>   return -ENXIO;
> }

Yeah, I guess that it would be better to send mpt's smpReply to user
space then user-space tools can examine it. You know, That's what mpt
ioctl and Doug' smp_utils do. But we can't use the in-buffer for this
since it's used for the smp response. We could use sense buffer for
this.
-
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 3/3] mptsas: add SMP passthrough support via bsg

2007-07-23 Thread Moore, Eric 
Tomo - do you have any documentation on how to specify a bsg device?  I
was trying to run the smp_rep_manufacture from sgv4_tools, and I got
that error.  Due to that, I have not able to test this patch.  However,
here are some feedback with regards to the patch:


On Sunday, July 08, 2007 9:52 PM, FUJITA Tomonori wrote: 

> +
> +smpreq->RequestDataLength = req->data_len - 4;

Our firmware formats the data in little endian, so you'll need to
convert this inorder for it work under other endian formats, like sparc
and ppc.  So this code should be:

smpreq->RequestDataLength = cpu_to_le16(req->data_len - 4);


> +smpreq->Function = MPI_FUNCTION_SMP_PASSTHROUGH;
> +
> + if (rphy)
> + memcpy(&smpreq->SASAddress, &rphy->identify.sas_address,
> +sizeof(smpreq->SASAddress));

should be using cpu_to_le64 before the data is put into
smpreq->SASAddress

> + else {
> + struct mptsas_portinfo *port_info;
> +
> + mutex_lock(&ioc->sas_topology_mutex);
> + port_info = mptsas_find_portinfo_by_handle(ioc, 
> ioc->handle);
> + if (port_info && port_info->phy_info)
> + memcpy(&smpreq->SASAddress,
> +
> &(port_info->phy_info[0].phy->identify.sas_address),
> +sizeof(smpreq->SASAddress));
> + mutex_unlock(&ioc->sas_topology_mutex);
> + }

I'm not sure what the intent of this else case.  I think it should be
deleted, and replaced with a return of ENXIO.The sas_topology is a
flat model, with the first object the intiatiator, and the other objects
in the list are expanders.What your your attempting to obtain is the
first port object connected to the initiator, which could be anything,
for instance it could be an end device, instead of expander.So I
think if your rphy object is not returning a valid sas_address, then
return instead of attempting to find something from the sas_topology.
I hope the API is making sure this is an expander before mptsas is even
called, is that handled?

> + /* a reply frame is expected */
> + if (!(ioc->sas_mgmt.status & MPT_IOCTL_STATUS_RF_VALID)) {
> + printk("%s: the smp response invalid!\n", __FUNCTION__);
> + ret = -ENXIO;
> + }
> +

I think in addition to having a valid reply, I think you should look at
the iocstatus to insure the reply itself was successfull, and that there
were no data underruns.

ioc_status = le16_to_cpu(smpReply->IOCStatus) & MPI_IOCSTATUS_MASK;
if ((ioc_status != MPI_IOCSTATUS_SUCCESS) &&
(ioc_status != MPI_IOCSTATUS_SCSI_DATA_UNDERRUN)) {
return -ENXIO;
}







-
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