Re: [PATCH] scsi: ibmvscsi: Improve strings handling

[Tyrel Datwyler]

On 06/26/2018 01:35 PM, Breno Leitao wrote:

The subject line should have been updated to [PATCH v2] to clue recipients to 
the fact that this is an updated version and not a resend or accidental 
duplicate send.

> Currently an open firmware property is copied into partition_name variable
> without keeping a room for \0.
> 
> Later one, this variable (partition_name), which is 97 bytes long, is
> strncpyed into ibmvcsci_host_data->madapter_info->partition_name, which
> is 96 bytes long, possibly truncating it 'again' and removing the \0.
> 
> This patch simply decreases the partition name to 96 and just copy using
> strlcpy() which guarantees that the string is \0 terminated. I think there
> is no issue if this there is a truncation in this very first copy, i.e,
> when the open firmware property is read and copied into the driver for the
> very first time;
> 
> This issue also causes the following warning on GCC 8:
> 
>   drivers/scsi/ibmvscsi/ibmvscsi.c:281:2: warning: ‘strncpy’ output may 
> be truncated copying 96 bytes from a string of length 96 
> [-Wstringop-truncation]
>   ...
>   inlined from ‘ibmvscsi_probe’ at 
> drivers/scsi/ibmvscsi/ibmvscsi.c:2221:7:
>   drivers/scsi/ibmvscsi/ibmvscsi.c:265:3: warning: ‘strncpy’ specified 
> bound 97 equals destination size [-Wstringop-truncation]
> 
> CC: Bart Van Assche 
> CC: Tyrel Datwyler 
> Signed-off-by: Breno Leitao 
> ---

Also, it is generally recommended that you record your revision history here 
for the readers/reviewers to quickly see what changed, and to make sure once 
the patch is pulled this info isn't included in the commit log.

ie.

Changes in v2:
- Addressed Bart's comment by replacing strncpy() with strlcpy()


Otherwise,

Acked-by: Tyrel Datwyler 

>  drivers/scsi/ibmvscsi/ibmvscsi.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/scsi/ibmvscsi/ibmvscsi.c 
> b/drivers/scsi/ibmvscsi/ibmvscsi.c
> index 17df76f0be3c..67a2c844e30d 100644
> --- a/drivers/scsi/ibmvscsi/ibmvscsi.c
> +++ b/drivers/scsi/ibmvscsi/ibmvscsi.c
> @@ -93,7 +93,7 @@ static int max_requests = IBMVSCSI_MAX_REQUESTS_DEFAULT;
>  static int max_events = IBMVSCSI_MAX_REQUESTS_DEFAULT + 2;
>  static int fast_fail = 1;
>  static int client_reserve = 1;
> -static char partition_name[97] = "UNKNOWN";
> +static char partition_name[96] = "UNKNOWN";
>  static unsigned int partition_number = -1;
>  static LIST_HEAD(ibmvscsi_head);
> 
> @@ -262,7 +262,7 @@ static void gather_partition_info(void)
> 
>   ppartition_name = of_get_property(of_root, "ibm,partition-name", NULL);
>   if (ppartition_name)
> - strncpy(partition_name, ppartition_name,
> + strlcpy(partition_name, ppartition_name,
>   sizeof(partition_name));
>   p_number_ptr = of_get_property(of_root, "ibm,partition-no", NULL);
>   if (p_number_ptr)
> 

[PATCH] ibmvfc: fix misdefined reserved field in ibmvfc_fcp_rsp_info

[Tyrel Datwyler]

The fcp_rsp_info structure as defined in the FC spec has an initial 3 bytes
reserved field. The ibmvfc driver mistakenly defined this field as 4 bytes
resulting in the rsp_code field being defined in what should be the start of
the second reserved field and thus always being reported as zero by the
driver.

Ideally, we should wire ibmvfc up with libfc for the sake of code
deduplication, and ease of maintaining standardized structures in a single
place. However, for now simply fixup the definition in ibmvfc for
backporting to distros on older kernels. Wiring up with libfc will be done
in a followup patch.

Cc: sta...@vger.kernel.org
Reported-by: Hannes Reinecke <h...@suse.de>
Signed-off-by: Tyrel Datwyler <tyr...@linux.vnet.ibm.com>
---
 drivers/scsi/ibmvscsi/ibmvfc.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/ibmvscsi/ibmvfc.h b/drivers/scsi/ibmvscsi/ibmvfc.h
index 9a0696f..b81a53c 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc.h
+++ b/drivers/scsi/ibmvscsi/ibmvfc.h
@@ -367,7 +367,7 @@ enum ibmvfc_fcp_rsp_info_codes {
 };
 
 struct ibmvfc_fcp_rsp_info {
-   __be16 reserved;
+   u8 reserved[3];
u8 rsp_code;
u8 reserved2[4];
 }__attribute__((packed, aligned (2)));
-- 
2.7.4

Re: [PATCH-next] ibmvfc: Remove unneeded semicolons

[Tyrel Datwyler]

On 01/17/2018 05:38 PM, Christopher Díaz Riveros wrote:
> Trivial fix removes unneeded semicolons after switch blocks.
> 
> This issue was detected by using the Coccinelle software.
> 
> Signed-off-by: Christopher Díaz Riveros <chris...@gentoo.org>
> ---

Resending from the correct email address. :)

Acked-by: Tyrel Datwyler <tyr...@linux.vnet.ibm.com>

Re: [PATCH-next] ibmvfc: Remove unneeded semicolons

[Tyrel Datwyler]

On 01/17/2018 05:38 PM, Christopher Díaz Riveros wrote:
> Trivial fix removes unneeded semicolons after switch blocks.
> 
> This issue was detected by using the Coccinelle software.
> 
> Signed-off-by: Christopher Díaz Riveros <chris...@gentoo.org>
> ---

Acked-by: Tyrel Datwyler <tyr...@linux.vnet.ibm.com>

Re: [PATCH] scsi: ibmvscsi: Convert timers to use timer_setup()

[Tyrel Datwyler]

On 10/25/2017 03:06 AM, Kees Cook wrote:
> In preparation for unconditionally passing the struct timer_list pointer to
> all timer callbacks, switch to using the new timer_setup() and from_timer()
> to pass the timer pointer explicitly.
> 
> Cc: "Martin K. Petersen" <martin.peter...@oracle.com>
> Cc: Tyrel Datwyler <tyr...@linux.vnet.ibm.com>
> Cc: Benjamin Herrenschmidt <b...@kernel.crashing.org>
> Cc: Paul Mackerras <pau...@samba.org>
> Cc: Michael Ellerman <m...@ellerman.id.au>
> Cc: "James E.J. Bottomley" <j...@linux.vnet.ibm.com>
> Cc: linux-scsi@vger.kernel.org
> Cc: linuxppc-...@lists.ozlabs.org
> Signed-off-by: Kees Cook <keesc...@chromium.org>
> ---
>  drivers/scsi/ibmvscsi/ibmvfc.c   | 14 ++
>  drivers/scsi/ibmvscsi/ibmvscsi.c |  7 +++
>  2 files changed, 9 insertions(+), 12 deletions(-)

Acked-by: Tyrel Datwyler <tyr...@linux.vnet.ibm.com>

Re: ibmvfc oddities

[Tyrel Datwyler]

On 09/21/2017 08:13 AM, Brian King wrote:
> On 09/21/2017 05:02 AM, Hannes Reinecke wrote:
>> Hi Brian,
>>
>> I was looking at the ibmvfc code (trying to hook up libfc), and have
>> found this definition:
>>
>> struct ibmvfc_fcp_rsp_info {
>>  __be16 reserved;
>>  u8 rsp_code;
>>  u8 reserved2[4];
>> }__attribute__((packed, aligned (2)));
>>
>> in comparison, libfc has this:
>>
>> struct fcp_resp_rsp_info {
>> __u8  _fr_resvd[3];   /* reserved */
>> __u8  rsp_code;   /* Response Info Code */
>> __u8  _fr_resvd2[4];  /* reserved */
>> };
>>
>> So both look _nearly_ identical, except the missing byte at the start.
>> It might be inserted due to some compile alignment magic, but I'd rather
>> not rely on this.
>> Could you clarify if the two structures really are different, or if this
>> is a simple oversight?
> 
> Looks like a bug to me. We should probably just have ibmvfc use the
> libfc definition.

Yes, after looking at the FC spec we most definitely have it defined wrong, and 
I'm pretty
certain that we aren't getting saved by any compiler magic.

> 
> Tyrel - can you do this conversion and run a bit of regression testing?
> Looking at the possible values of rsp_code, the most likely place where
> this might cause us some issues is in TMF handling. I'm a little
> worried this could result in a potential double completion in error
> handling in some rare cases Tyrel - are you aware of any issues
> like that, which this might explain?

I certainly can. I recollect something like a double completion issue, but its 
been so
long I can't remember if we were seeing it in the vfc driver or the vscsi 
driver. Anyhow,
I do feel like from what I recall it seems like rsp_code is always zero in 
reported error
messages.

-Tyrel

> 
> Thanks,
> 
> Brian
> 

Re: [PATCH] scsi: shost->async_scan should be protected by mutex_lock

[Tyrel Datwyler]

On 09/07/2017 11:54 PM, Ouyangzhaowei (Charles) wrote:
> shost->async_scan should be protected by mutex_lock, otherwise the check
> of "called twice" won't work.
> 
> Signed-off-by: Ouyang Zhaowei 
> ---
>  drivers/scsi/scsi_scan.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
> index fd88dab..1d1df51 100644
> --- a/drivers/scsi/scsi_scan.c
> +++ b/drivers/scsi/scsi_scan.c
> @@ -1722,6 +1722,7 @@ static struct async_scan_data
> *scsi_prep_async_scan(struct Scsi_Host *shost)
> if (strncmp(scsi_scan_type, "sync", 4) == 0)
> return NULL;
> 
> +   mutex_lock(>scan_mutex);

The mutex will not be unlocked in the event that either the host has called
scsi_prep_async_scan() twice, or a condition is meet the branches to the "err" 
label prior
to where the original mutex_lock() was located below.

-Tyrel

> if (shost->async_scan) {
> shost_printk(KERN_DEBUG, shost, "%s called twice\n",
> __func__);
> return NULL;
> @@ -1735,7 +1736,6 @@ static struct async_scan_data
> *scsi_prep_async_scan(struct Scsi_Host *shost)
> goto err;
> init_completion(>prev_finished);
> 
> -   mutex_lock(>scan_mutex);
> spin_lock_irqsave(shost->host_lock, flags);
> shost->async_scan = 1;
> spin_unlock_irqrestore(shost->host_lock, flags);
> 

Re: [PATCH 19/31] timer: Remove open-coded casts for .data and .function

[Tyrel Datwyler]

On 08/31/2017 04:29 PM, Kees Cook wrote:
> This standardizes the callback and data prototypes in several places that
> perform casting, in an effort to remove more open-coded .data and
> .function uses in favor of setup_timer().
> 
> Cc: Samuel Ortiz <sam...@sortiz.org>
> Cc: Tyrel Datwyler <tyr...@linux.vnet.ibm.com>
> Cc: Benjamin Herrenschmidt <b...@kernel.crashing.org>
> Cc: Paul Mackerras <pau...@samba.org>
> Cc: Michael Ellerman <m...@ellerman.id.au>
> Cc: "James E.J. Bottomley" <j...@linux.vnet.ibm.com>
> Cc: "Martin K. Petersen" <martin.peter...@oracle.com>
> Cc: net...@vger.kernel.org
> Cc: linux-scsi@vger.kernel.org
> Cc: linuxppc-...@lists.ozlabs.org
> Signed-off-by: Kees Cook <keesc...@chromium.org>
> ---
>  drivers/net/irda/bfin_sir.c  |  5 +++--
>  drivers/scsi/ibmvscsi/ibmvfc.c   | 14 ++
>  drivers/scsi/ibmvscsi/ibmvscsi.c |  8 
>  3 files changed, 13 insertions(+), 14 deletions(-)

Resending from correct email address.

For ibmvfc & ibmvscsi portions:

Acked-by: Tyrel Datwyler <tyr...@linux.vnet.ibm.com>

Re: [PATCH 19/31] timer: Remove open-coded casts for .data and .function

[Tyrel Datwyler]

On 08/31/2017 04:29 PM, Kees Cook wrote:
> This standardizes the callback and data prototypes in several places that
> perform casting, in an effort to remove more open-coded .data and
> .function uses in favor of setup_timer().
> 
> Cc: Samuel Ortiz <sam...@sortiz.org>
> Cc: Tyrel Datwyler <tyr...@linux.vnet.ibm.com>
> Cc: Benjamin Herrenschmidt <b...@kernel.crashing.org>
> Cc: Paul Mackerras <pau...@samba.org>
> Cc: Michael Ellerman <m...@ellerman.id.au>
> Cc: "James E.J. Bottomley" <j...@linux.vnet.ibm.com>
> Cc: "Martin K. Petersen" <martin.peter...@oracle.com>
> Cc: net...@vger.kernel.org
> Cc: linux-scsi@vger.kernel.org
> Cc: linuxppc-...@lists.ozlabs.org
> Signed-off-by: Kees Cook <keesc...@chromium.org>
> ---
>  drivers/net/irda/bfin_sir.c  |  5 +++--
>  drivers/scsi/ibmvscsi/ibmvfc.c   | 14 ++
>  drivers/scsi/ibmvscsi/ibmvscsi.c |  8 
>  3 files changed, 13 insertions(+), 14 deletions(-)
For ibmvfc & ibmvscsi portions:

Acked-by: Tyrel Datwyler <tyr...@linux.vnet.ibm.com>

Re: [PATCH][V2] scsi: qedf: fix spelling mistake: "offlading" -> "offloading"

[Tyrel Datwyler]

On 07/03/2017 12:21 PM, Colin King wrote:
> From: Colin Ian King <colin.k...@canonical.com>
> 
> Trivial fix to spelling mistake in QEDF_INFO message and 
> remove duplicated "since" (thanks to Tyrel Datwyler for spotting
> the latter issue).
> 
> Signed-off-by: Colin Ian King <colin.k...@canonical.com>

Reviewed-by: Tyrel Datwyler <tyr...@linux.vnet.ibm.com>

Re: [PATCH] scsi: qedf: fix spelling mistake: "offlading" -> "offloading"

[Tyrel Datwyler]

On 07/03/2017 02:44 AM, Colin King wrote:
> From: Colin Ian King 
> 
> Trivial fix to spelling mistake in QEDF_INFO message
> 
> Signed-off-by: Colin Ian King 
> ---
>  drivers/scsi/qedf/qedf_main.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/scsi/qedf/qedf_main.c b/drivers/scsi/qedf/qedf_main.c
> index b58bba4604e8..dc93e968a448 100644
> --- a/drivers/scsi/qedf/qedf_main.c
> +++ b/drivers/scsi/qedf/qedf_main.c
> @@ -1227,7 +1227,7 @@ static void qedf_rport_event_handler(struct fc_lport 
> *lport,
>  
>   if (rdata->spp_type != FC_TYPE_FCP) {
>   QEDF_INFO(&(qedf->dbg_ctx), QEDF_LOG_DISC,
> - "Not offlading since since spp type isn't FCP\n");
> + "Not offloading since since spp type isn't FCP\n");

Seems like a "since" could be dropped here as well.

-Tyrel

>   break;
>   }
>   if (!(rdata->ids.roles & FC_RPORT_ROLE_FCP_TARGET)) {
> 

Re: [PATCH] scsi: ibmvfc: constify dev_pm_ops structures.

[Tyrel Datwyler]

On 06/29/2017 12:54 AM, Arvind Yadav wrote:
> dev_pm_ops are not supposed to change at runtime. All functions
> working with dev_pm_ops provided by  work with const
> dev_pm_ops. So mark the non-const structs as const.
> 
> File size before:
>text  data bss dec hex filename
>   41937  1296  20   43253a8f5 drivers/scsi/ibmvscsi/ibmvfc.o
> 
> File size After adding 'const':
>text  data bss dec hex filename
>   42129  1104  20   43253a8f5 drivers/scsi/ibmvscsi/ibmvfc.o
> 
> Signed-off-by: Arvind Yadav <arvind.yadav...@gmail.com>

Acked-by: Tyrel Datwyler <tyr...@linux.vnet.ibm.com>

Re: [PATCH] scsi: ibmvscsi: constify dev_pm_ops structures.

[Tyrel Datwyler]

On 06/29/2017 12:43 AM, Arvind Yadav wrote:
> dev_pm_ops are not supposed to change at runtime. All functions
> working with dev_pm_ops provided by  work with const
> dev_pm_ops. So mark the non-const structs as const.
> 
> File size before:
>text  data bss dec hex filename
>   17956  1456   8   194204bdc drivers/scsi/ibmvscsi/ibmvscsi.o
> 
> File size After adding 'const':
>text  data bss dec hex filename
>   18164  1264   8   194364bec drivers/scsi/ibmvscsi/ibmvscsi.o
> 
> Signed-off-by: Arvind Yadav <arvind.yadav...@gmail.com>

Acked-by: Tyrel Datwyler <tyr...@linux.vnet.ibm.com>

[PATCH] scsi: fix typos and grammar in comments of scsi_transport_fc.c

[Tyrel Datwyler]

Signed-off-by: Tyrel Datwyler <tyr...@linux.vnet.ibm.com>
---
 drivers/scsi/scsi_transport_fc.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/scsi/scsi_transport_fc.c b/drivers/scsi/scsi_transport_fc.c
index d4cf32d..6dd0922 100644
--- a/drivers/scsi/scsi_transport_fc.c
+++ b/drivers/scsi/scsi_transport_fc.c
@@ -1692,7 +1692,7 @@ static FC_DEVICE_ATTR(host, dev_loss_tmo, S_IRUGO | 
S_IWUSR,
  * Host Statistics Management
  */
 
-/* Show a given an attribute in the statistics group */
+/* Show a given attribute in the statistics group */
 static ssize_t
 fc_stat_show(const struct device *dev, char *buf, unsigned long offset)
 {
@@ -2923,7 +2923,7 @@ struct fc_rport *
  *   attached to it. However, we want to semi-persist the target id assigned
  *   to that port if it eventually does exist. The port structure will
  *   remain (although with minimal information) so that the target id
- *   bindings remails.
+ *   bindings also remain.
  *
  * If the remote port is not an FCP Target, it will be fully torn down
  * and deallocated, including the fc_remote_port class device.
@@ -2937,7 +2937,7 @@ struct fc_rport *
  *   If the remote port does not return (signaled by a LLDD call to
  *   fc_remote_port_add()) within the dev_loss_tmo timeout, then the
  *   scsi target is removed - killing all outstanding i/o and removing the
- *   scsi devices attached ot it. The port structure will be marked Not
+ *   scsi devices attached to it. The port structure will be marked Not
  *   Present and be partially cleared, leaving only enough information to
  *   recognize the remote port relative to the scsi target id binding if
  *   it later appears.  The port will remain as long as there is a valid
@@ -3056,7 +3056,7 @@ struct fc_rport *
 * There may have been a delete timer running on the
 * port. Ensure that it is cancelled as we now know
 * the port is an FCP Target.
-* Note: we know the rport is exists and in an online
+* Note: we know the rport exists and is in an online
 *  state as the LLDD would not have had an rport
 *  reference to pass us.
 *
@@ -3317,7 +3317,7 @@ int fc_block_scsi_eh(struct scsi_cmnd *cmnd)
  * @ret_vport: The pointer to the created vport.
  *
  * Allocates and creates the vport structure, calls the parent host
- * to instantiate the vport, the completes w/ class and sysfs creation.
+ * to instantiate the vport, this completes w/ class and sysfs creation.
  *
  * Notes:
  * This routine assumes no locks are held on entry.
@@ -3397,7 +3397,7 @@ int fc_block_scsi_eh(struct scsi_cmnd *cmnd)
 
/*
 * if the parent isn't the physical adapter's Scsi_Host, ensure
-* the Scsi_Host at least contains ia symlink to the vport.
+* the Scsi_Host at least contains a symlink to the vport.
 */
if (pdev != >shost_gendev) {
error = sysfs_create_link(>shost_gendev.kobj,
-- 
1.8.3.1

Re: [PATCH] target: remove dead code

[Tyrel Datwyler]

On 05/09/2017 02:46 PM, Gustavo A. R. Silva wrote:
> Local variable _ret_ is assigned to a constant value and it is never
> updated again. Remove this variable and the dead code it guards.
> 
> Addresses-Coverity-ID: 140761
> Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
> ---

Reviewed-by: Tyrel Datwyler <tyr...@linux.vnet.ibm.com>

Re: [PATCH] scsi: pmcraid: remove redundant check to see if request_size is less than zero

[Tyrel Datwyler]

On 05/03/2017 09:29 AM, Colin King wrote:
> From: Colin Ian King <colin.k...@canonical.com>
> 
> The 2nd check to see if request_size is less than zero is redundant
> because the first check takes error exit path on this condition. So,
> since it is redundant, remove it.
> 
> Detected by CoverityScan, CID#146149 ("Logically Dead Code")
> 
> Signed-off-by: Colin Ian King <colin.k...@canonical.com>

Reviewed-by: Tyrel Datwyler <tyr...@linux.vnet.ibm.com>

Re: [PATCH] ibmvscsis: Do not send aborted task response

[Tyrel Datwyler]

On 04/10/2017 11:52 AM, Bryant G. Ly wrote:
> The driver is sending a response to the aborted task response
> along with LIO sending the tmr response.

I think this could be better worded. Something like the driver is sending a 
response to
the actual ***scsi op*** that was aborted by an abort task TM, while LIO is 
sending a
response to the abort task TM.

> ibmvscsis_tgt does not send the response to the client until
> release_cmd time. The reason for this was because if we did it
> at queue_status time, then the client would be free to reuse the
> tag for that command, but we're still using the tag until the
> command is released at release_cmd time, so we chose to delay
> sending the response until then. That then caused this issue, because
> release_cmd is always called, even if queue_status is not.

The above portion of the commit message is little convoluted in my opinion, and 
a bit hard
to follow. Otherwise,

Reviewed-by: Tyrel Datwyler <tyr...@linux.vnet.ibm.com>

> 
> SCSI spec says that the initiator that sends the abort task
> TM NEVER gets a response to the aborted op and with the current
> code it will send a response. Thus this fix will remove that response
> if the TAS bit is set.
> 
> Cc: <sta...@vger.kernel.org> # v4.8+
> Signed-off-by: Bryant G. Ly <bryan...@linux.vnet.ibm.com>
> ---
>  drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c | 60 
> +---
>  1 file changed, 40 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c 
> b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
> index 4bb5635..f75948a 100644
> --- a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
> +++ b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
> @@ -1758,33 +1758,42 @@ static void ibmvscsis_send_messages(struct scsi_info 
> *vscsi)
>  
>   if (!(vscsi->flags & RESPONSE_Q_DOWN)) {
>   list_for_each_entry_safe(cmd, nxt, >waiting_rsp, list) {
> - iue = cmd->iue;
> + /*
> +  * If Task Abort Status Bit is set, then dont send a
> +  * response.
> +  */
> + if (cmd->se_cmd.transport_state & CMD_T_TAS) {
> + list_del(>list);
> + ibmvscsis_free_cmd_resources(vscsi, cmd);
> + } else {
> + iue = cmd->iue;
>  
> - crq->valid = VALID_CMD_RESP_EL;
> - crq->format = cmd->rsp.format;
> + crq->valid = VALID_CMD_RESP_EL;
> + crq->format = cmd->rsp.format;
>  
> - if (cmd->flags & CMD_FAST_FAIL)
> - crq->status = VIOSRP_ADAPTER_FAIL;
> + if (cmd->flags & CMD_FAST_FAIL)
> + crq->status = VIOSRP_ADAPTER_FAIL;
>  
> - crq->IU_length = cpu_to_be16(cmd->rsp.len);
> + crq->IU_length = cpu_to_be16(cmd->rsp.len);
>  
> - rc = h_send_crq(vscsi->dma_dev->unit_address,
> - be64_to_cpu(msg_hi),
> - be64_to_cpu(cmd->rsp.tag));
> + rc = h_send_crq(vscsi->dma_dev->unit_address,
> + be64_to_cpu(msg_hi),
> + be64_to_cpu(cmd->rsp.tag));
>  
> - pr_debug("send_messages: cmd %p, tag 0x%llx, rc %ld\n",
> -  cmd, be64_to_cpu(cmd->rsp.tag), rc);
> + pr_debug("send_messages: cmd %p, tag 0x%llx, rc 
> %ld\n",
> +  cmd, be64_to_cpu(cmd->rsp.tag), rc);
>  
> - /* if all ok free up the command element resources */
> - if (rc == H_SUCCESS) {
> - /* some movement has occurred */
> - vscsi->rsp_q_timer.timer_pops = 0;
> - list_del(>list);
> + /* if all ok free up the command element 
> resources */
> + if (rc == H_SUCCESS) {
> + /* some movement has occurred */
> + vscsi->rsp_q_timer.timer_pops = 0;
> + list_del(>list);
>  
> - ibmvscsis_free_cmd_reso

Re: [PATCH] scsi: ibmvfc: don't check for failure from mempool_alloc()

[Tyrel Datwyler]

On 04/09/2017 07:15 PM, NeilBrown wrote:
> 
> mempool_alloc() cannot fail when passed GFP_NOIO or any
> other gfp setting that is permitted to sleep.
> So remove this pointless code.
> 
> Signed-off-by: NeilBrown <ne...@suse.com>

Acked-by: Tyrel Datwyler <tyr...@linux.vnet.ibm.com>

> ---
>  drivers/scsi/ibmvscsi/ibmvfc.c | 6 --
>  1 file changed, 6 deletions(-)
> 
> diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c
> index 2c92dabb55f6..26cd3c28186a 100644
> --- a/drivers/scsi/ibmvscsi/ibmvfc.c
> +++ b/drivers/scsi/ibmvscsi/ibmvfc.c
> @@ -3910,12 +3910,6 @@ static int ibmvfc_alloc_target(struct ibmvfc_host 
> *vhost, u64 scsi_id)
>   spin_unlock_irqrestore(vhost->host->host_lock, flags);
>  
>   tgt = mempool_alloc(vhost->tgt_pool, GFP_NOIO);
> - if (!tgt) {
> - dev_err(vhost->dev, "Target allocation failure for scsi id 
> %08llx\n",
> - scsi_id);
> - return -ENOMEM;
> - }
> -
>   memset(tgt, 0, sizeof(*tgt));
>   tgt->scsi_id = scsi_id;
>   tgt->new_scsi_id = scsi_id;
> 

Re: [PATCH] scsi: fc: remove redundant check of an unsigned long being less than zero

[Tyrel Datwyler]

On 04/14/2017 06:58 AM, Colin King wrote:
> From: Colin Ian King <colin.k...@canonical.com>
> 
> The check for an unsigned long being less than zero is always false
> so it is a redundant check and can be removed.
> 
> Detected by static analysis with by PVS-Studio
> 
> Signed-off-by: Colin Ian King <colin.k...@canonical.com>

Reviewed-by: Tyrel Datwyler <tyr...@linux.vnet.ibm.com>

> ---
>  drivers/scsi/scsi_transport_fc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/scsi/scsi_transport_fc.c 
> b/drivers/scsi/scsi_transport_fc.c
> index 2d753c93e07a..87b8f9d64d9b 100644
> --- a/drivers/scsi/scsi_transport_fc.c
> +++ b/drivers/scsi/scsi_transport_fc.c
> @@ -850,7 +850,7 @@ static int fc_str_to_dev_loss(const char *buf, unsigned 
> long *val)
>   char *cp;
>  
>   *val = simple_strtoul(buf, , 0);
> - if ((*cp && (*cp != '\n')) || (*val < 0))
> + if (*cp && (*cp != '\n'))
>   return -EINVAL;
>   /*
>* Check for overflow; dev_loss_tmo is u32
> 

Re: [PATCH] ibmvscsi: add write memory barrier to CRQ processing

[Tyrel Datwyler]

On 12/09/2016 01:20 PM, Benjamin Herrenschmidt wrote:
> On Wed, 2016-12-07 at 17:31 -0600, Tyrel Datwyler wrote:
>> The first byte of each CRQ entry is used to indicate whether an entry is
>> a valid response or free for the VIOS to use. After processing a
>> response the driver sets the valid byte to zero to indicate the entry is
>> now free to be reused. Add a memory barrier after this write to ensure
>> no other stores are reordered when updating the valid byte.
> 
> Which "other stores" specifically ? This smells fishy without that
> precision. It's important to always understand what exactly barriers
> order with.

So, this patch initially came about while chasing a data integrity issue
based on the observation that we were already doing this same write
barrier in the virtual fibre channel driver.

However, the more I stare at it I agree it does seem fishy and I can't
see any other stores that we need to order with here. In terms of the
16byte CRQ entries we only ever write to the first byte as a sort of
doorbell to tell the VIOS that we have processed the data and the entry
is free to be re-used. The remainder of the CRQ is only ever read from.

The wmb() in the ibmvfc driver was part of a commit from Brian that also
involved a necessary rmb() that prevented stale data from load
re-ordering by ensuring that a read from the first byte completed and
contained a valid value prior to doing any other reads of the CRQ entry.

I'd have to defer to Brian as to whether he remembers a legitimate
reason for the wmb().

-Tyrel

> 
> Cheers,
> Ben.
> 
>> Signed-off-by: Tyrel Datwyler <tyr...@linux.vnet.ibm.com>
>> ---
>>  drivers/scsi/ibmvscsi/ibmvscsi.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/scsi/ibmvscsi/ibmvscsi.c 
>> b/drivers/scsi/ibmvscsi/ibmvscsi.c
>> index d9534ee..2f5b07e 100644
>> --- a/drivers/scsi/ibmvscsi/ibmvscsi.c
>> +++ b/drivers/scsi/ibmvscsi/ibmvscsi.c
>> @@ -232,6 +232,7 @@ static void ibmvscsi_task(void *data)
>>> while ((crq = crq_queue_next_crq(>queue)) != NULL) {
>>> ibmvscsi_handle_crq(crq, hostdata);
>>> crq->valid = VIOSRP_CRQ_FREE;
>>> +   wmb();
>>> }
>>  
>>> vio_enable_interrupts(vdev);
>> @@ -240,6 +241,7 @@ static void ibmvscsi_task(void *data)
>>> vio_disable_interrupts(vdev);
>>> ibmvscsi_handle_crq(crq, hostdata);
>>> crq->valid = VIOSRP_CRQ_FREE;
>>> +   wmb();
>>> } else {
>>> done = 1;
>>> }
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ibmvscsi: add write memory barrier to CRQ processing

[Tyrel Datwyler]

On 12/08/2016 01:06 AM, Johannes Thumshirn wrote:
> On Wed, Dec 07, 2016 at 05:31:26PM -0600, Tyrel Datwyler wrote:
>> The first byte of each CRQ entry is used to indicate whether an entry is
>> a valid response or free for the VIOS to use. After processing a
>> response the driver sets the valid byte to zero to indicate the entry is
>> now free to be reused. Add a memory barrier after this write to ensure
>> no other stores are reordered when updating the valid byte.
>>
>> Signed-off-by: Tyrel Datwyler <tyr...@linux.vnet.ibm.com>
>> ---
>>  drivers/scsi/ibmvscsi/ibmvscsi.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/scsi/ibmvscsi/ibmvscsi.c 
>> b/drivers/scsi/ibmvscsi/ibmvscsi.c
>> index d9534ee..2f5b07e 100644
>> --- a/drivers/scsi/ibmvscsi/ibmvscsi.c
>> +++ b/drivers/scsi/ibmvscsi/ibmvscsi.c
>> @@ -232,6 +232,7 @@ static void ibmvscsi_task(void *data)
>>  while ((crq = crq_queue_next_crq(>queue)) != NULL) {
>>  ibmvscsi_handle_crq(crq, hostdata);
>>  crq->valid = VIOSRP_CRQ_FREE;
>> +wmb();
>>  }
>>  
>>  vio_enable_interrupts(vdev);
>> @@ -240,6 +241,7 @@ static void ibmvscsi_task(void *data)
>>  vio_disable_interrupts(vdev);
>>  ibmvscsi_handle_crq(crq, hostdata);
>>  crq->valid = VIOSRP_CRQ_FREE;
>> +wmb();
>>  } else {
>>  done = 1;
>>  }
> 
> Is this something you have seen in the wild or just a "better save than sorry"
> barrier?

I myself have not observed or heard of anybody hitting an issue here.
However, based on conversation with the VIOS developers, who have
indicated it is required, this is a "better safe than sorry" scenario.
Further, it matches what we already do in the ibmvfc driver for the CRQ
processing logic.

-Tyrel

> 
> Thanks,
>   Johannes
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ibmvscsi: add write memory barrier to CRQ processing

[Tyrel Datwyler]

On 12/08/2016 03:29 PM, Paolo Bonzini wrote:
> 
> 
> On 08/12/2016 00:31, Tyrel Datwyler wrote:
>> The first byte of each CRQ entry is used to indicate whether an entry is
>> a valid response or free for the VIOS to use. After processing a
>> response the driver sets the valid byte to zero to indicate the entry is
>> now free to be reused. Add a memory barrier after this write to ensure
>> no other stores are reordered when updating the valid byte.
>>
>> Signed-off-by: Tyrel Datwyler <tyr...@linux.vnet.ibm.com>
>> ---
>>  drivers/scsi/ibmvscsi/ibmvscsi.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/scsi/ibmvscsi/ibmvscsi.c 
>> b/drivers/scsi/ibmvscsi/ibmvscsi.c
>> index d9534ee..2f5b07e 100644
>> --- a/drivers/scsi/ibmvscsi/ibmvscsi.c
>> +++ b/drivers/scsi/ibmvscsi/ibmvscsi.c
>> @@ -232,6 +232,7 @@ static void ibmvscsi_task(void *data)
>>  while ((crq = crq_queue_next_crq(>queue)) != NULL) {
>>  ibmvscsi_handle_crq(crq, hostdata);
>>  crq->valid = VIOSRP_CRQ_FREE;
>> +wmb();
>>  }
>>  
>>  vio_enable_interrupts(vdev);
>> @@ -240,6 +241,7 @@ static void ibmvscsi_task(void *data)
>>  vio_disable_interrupts(vdev);
>>  ibmvscsi_handle_crq(crq, hostdata);
>>  crq->valid = VIOSRP_CRQ_FREE;
>> +wmb();
> 
> Should this driver use virt_wmb instead?

Both virt_wmb and wmb reduce to a lwsync instruction under PowerPC.

-Tyrel

> 
> Paolo
> 
>>  } else {
>>  done = 1;
>>  }
>>

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] ibmvscsi: add write memory barrier to CRQ processing

[Tyrel Datwyler]

The first byte of each CRQ entry is used to indicate whether an entry is
a valid response or free for the VIOS to use. After processing a
response the driver sets the valid byte to zero to indicate the entry is
now free to be reused. Add a memory barrier after this write to ensure
no other stores are reordered when updating the valid byte.

Signed-off-by: Tyrel Datwyler <tyr...@linux.vnet.ibm.com>
---
 drivers/scsi/ibmvscsi/ibmvscsi.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/scsi/ibmvscsi/ibmvscsi.c b/drivers/scsi/ibmvscsi/ibmvscsi.c
index d9534ee..2f5b07e 100644
--- a/drivers/scsi/ibmvscsi/ibmvscsi.c
+++ b/drivers/scsi/ibmvscsi/ibmvscsi.c
@@ -232,6 +232,7 @@ static void ibmvscsi_task(void *data)
while ((crq = crq_queue_next_crq(>queue)) != NULL) {
ibmvscsi_handle_crq(crq, hostdata);
crq->valid = VIOSRP_CRQ_FREE;
+   wmb();
}
 
vio_enable_interrupts(vdev);
@@ -240,6 +241,7 @@ static void ibmvscsi_task(void *data)
vio_disable_interrupts(vdev);
ibmvscsi_handle_crq(crq, hostdata);
crq->valid = VIOSRP_CRQ_FREE;
+   wmb();
} else {
done = 1;
}
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/2] ibmvscsi: log bad SRP response opcode in hex format

[Tyrel Datwyler]

An unrecogonized or unsupported SRP response has its opcode currently
logged in decimal format. Log it in hex format instead so it can easily
be validated against the SRP specs values which are in hex.

Signed-off-by: Tyrel Datwyler <tyr...@linux.vnet.ibm.com>
---
 drivers/scsi/ibmvscsi/ibmvscsi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/ibmvscsi/ibmvscsi.c b/drivers/scsi/ibmvscsi/ibmvscsi.c
index a0ee16f..7752656 100644
--- a/drivers/scsi/ibmvscsi/ibmvscsi.c
+++ b/drivers/scsi/ibmvscsi/ibmvscsi.c
@@ -993,7 +993,7 @@ static void handle_cmd_rsp(struct srp_event_struct 
*evt_struct)
if (unlikely(rsp->opcode != SRP_RSP)) {
if (printk_ratelimit())
dev_warn(evt_struct->hostdata->dev,
-"bad SRP RSP type %d\n", rsp->opcode);
+"bad SRP RSP type %#02x\n", rsp->opcode);
}

if (cmnd) {
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/2] ibmvscsi: add vscsi hosts to global list_head

[Tyrel Datwyler]

Add each vscsi host adatper to a new global list_head named
ibmvscsi_head. There is no functional change. This is meant primarily
as a convience for locating adatpers from within the debugger or crash
utility.

Signed-off-by: Tyrel Datwyler <tyr...@linux.vnet.ibm.com>
---
 drivers/scsi/ibmvscsi/ibmvscsi.c | 3 +++
 drivers/scsi/ibmvscsi/ibmvscsi.h | 1 +
 2 files changed, 4 insertions(+)

diff --git a/drivers/scsi/ibmvscsi/ibmvscsi.c b/drivers/scsi/ibmvscsi/ibmvscsi.c
index d9534ee..a0ee16f 100644
--- a/drivers/scsi/ibmvscsi/ibmvscsi.c
+++ b/drivers/scsi/ibmvscsi/ibmvscsi.c
@@ -95,6 +95,7 @@ static int fast_fail = 1;
 static int client_reserve = 1;
 static char partition_name[97] = "UNKNOWN";
 static unsigned int partition_number = -1;
+static LIST_HEAD(ibmvscsi_head);
 
 static struct scsi_transport_template *ibmvscsi_transport_template;
 
@@ -2270,6 +2271,7 @@ static int ibmvscsi_probe(struct vio_dev *vdev, const 
struct vio_device_id *id)
}
 
dev_set_drvdata(>dev, hostdata);
+   list_add_tail(>host_list, _head);
return 0;
 
   add_srp_port_failed:
@@ -2291,6 +2293,7 @@ static int ibmvscsi_probe(struct vio_dev *vdev, const 
struct vio_device_id *id)
 static int ibmvscsi_remove(struct vio_dev *vdev)
 {
struct ibmvscsi_host_data *hostdata = dev_get_drvdata(>dev);
+   list_del(>host_list);
unmap_persist_bufs(hostdata);
release_event_pool(>pool, hostdata);
ibmvscsi_release_crq_queue(>queue, hostdata,
diff --git a/drivers/scsi/ibmvscsi/ibmvscsi.h b/drivers/scsi/ibmvscsi/ibmvscsi.h
index e0f6c3a..3a78755 100644
--- a/drivers/scsi/ibmvscsi/ibmvscsi.h
+++ b/drivers/scsi/ibmvscsi/ibmvscsi.h
@@ -90,6 +90,7 @@ struct event_pool {
 
 /* all driver data associated with a host adapter */
 struct ibmvscsi_host_data {
+   struct list_head host_list;
atomic_t request_limit;
int client_migrated;
int reset_crq;
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH RESEND v4 02/15] scsi: don't use fc_bsg_job::request and fc_bsg_job::reply directly

[Tyrel Datwyler]

On 11/17/2016 07:09 AM, Johannes Thumshirn wrote:
> Don't use fc_bsg_job::request and fc_bsg_job::reply directly, but use
> helper variables bsg_request and bsg_reply. This will be helpfull  when
> transitioning to bsg-lib.
> 
> Signed-off-by: Johannes Thumshirn <jthumsh...@suse.de>
> Reviewed-by: Hannes Reinecke <h...@suse.com>
> ---
>  drivers/s390/scsi/zfcp_fc.c  |   9 +-
>  drivers/scsi/bfa/bfad_bsg.c  |  40 +++---
>  drivers/scsi/ibmvscsi/ibmvfc.c   |  22 ++--
>  drivers/scsi/libfc/fc_lport.c|  23 ++--
>  drivers/scsi/lpfc/lpfc_bsg.c | 199 ++---
>  drivers/scsi/qla2xxx/qla_bsg.c   | 264 
> ++-
>  drivers/scsi/qla2xxx/qla_iocb.c  |   5 +-
>  drivers/scsi/qla2xxx/qla_isr.c   |  46 ---
>  drivers/scsi/qla2xxx/qla_mr.c|  10 +-
>  drivers/scsi/scsi_transport_fc.c |  55 
>  10 files changed, 398 insertions(+), 275 deletions(-)

For ibmvfc portion...

Acked-by: Tyrel Datwyler <tyr...@linux.vnet.ibm.com>

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 13/16] scsi: fc: use bsg_job_done

[Tyrel Datwyler]

On 10/13/2016 08:00 AM, Johannes Thumshirn wrote:
> fc_bsg_jobdone() and bsg_job_done() are 1:1 copies now so use the bsg-lib one
> instead of the FC private implementation.
> 
> Signed-off-by: Johannes Thumshirn <jthumsh...@suse.de>
> Reviewed-by: Hannes Reinecke <h...@suse.com>

For ibmvfc parts,

Acked-by: Tyrel Datwyler <tyr...@linux.vnet.ibm.com>

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 10/16] scsi: change FC drivers to use 'struct bsg_job'

[Tyrel Datwyler]

On 10/13/2016 08:00 AM, Johannes Thumshirn wrote:
> Change FC drivers to use 'struct bsg_job' from bsg-lib.h instead of 'struct
> fc_bsg_job' from scsi_transport_fc.h and remove 'struct fc_bsg_job'.
> 
> Signed-off-by: Johannes Thumshirn <jthumsh...@suse.de>
> Reviewed-by: Hannes Reinecke <h...@suse.com>

For ibmvfc parts,

Acked-by: Tyrel Datwyler <tyr...@linux.vnet.ibm.com>

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 06/16] scsi: fc: provide fc_bsg_to_rport() helper

[Tyrel Datwyler]

On 10/13/2016 08:00 AM, Johannes Thumshirn wrote:
> Provide fc_bsg_to_rport() helper that will become handy when we're moving
> from struct fc_bsg_job to a plain struct bsg_job. Also move all LLDDs to use
> the new helper.
> 
> Signed-off-by: Johannes Thumshirn <jthumsh...@suse.de>
> Reviewed-by: Hannes Reinecke <h...@suse.com>


For ibmvfc parts,

Acked-by: Tyrel Datwyler <tyr...@linux.vnet.ibm.com>

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 05/16] scsi: fc: provide fc_bsg_to_shost() helper

[Tyrel Datwyler]

On 10/13/2016 08:00 AM, Johannes Thumshirn wrote:
> Provide fc_bsg_to_shost() helper that will become handy when we're moving from
> struct fc_bsg_job to a plain struct bsg_job. Also use this little helper in
> the LLDDs.
> 
> Signed-off-by: Johannes Thumshirn <jthumsh...@suse.de>
> Reviewed-by: Hannes Reinecke <h...@suse.com>

For ibmvfc parts,

Acked-by: Tyrel Datwyler <tyr...@linux.vnet.ibm.com>

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 04/16] scsi: Unify interfaces of fc_bsg_jobdone and bsg_job_done

[Tyrel Datwyler]

On 10/13/2016 08:00 AM, Johannes Thumshirn wrote:
> Unify the interfaces of fc_bsg_jobdone and bsg_job_done. This will reduce the
> diff when moving from 'struct fc_bsg_job' to a plain 'struct bsg_job' later
> on.
> 
> Signed-off-by: Johannes Thumshirn <jthumsh...@suse.de>
> Reviewed-by: Hannes Reinecke <h...@suse.com>

For ibmvfc parts,

Acked-by: Tyrel Datwyler <tyr...@linux.vnet.ibm.com>

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 0/6] Fixes for ibmvscsis driver

[Tyrel Datwyler]

On 10/14/2016 03:25 PM, Steven Royer wrote:
> On 2016-10-13 11:02, Michael Cyr wrote:
>> Various fixes and cleanups for the ibmvscsis driver.
>> The first is a sort of prequel to the second, which is the primary
>> change (and the biggest).  The rest are fairly small fixes.
>>
>> Michael Cyr (6):
>>   ibmvscsis: Rearrange functions for future patches
>>   ibmvscsis: Synchronize cmds at tpg_enable_store time
>>   ibmvscsis: Synchronize cmds at remove time
>>   ibmvscsis: Clean up properly if target_submit_cmd/tmr fails
>>   ibmvscsis: Return correct partition name/# to client
>>   ibmvscsis: Issues from Dan Carpenter/Smatch
>>
>>  drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c | 1096 
>> +-
>>  drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.h |5 +-
>>  2 files changed, 494 insertions(+), 607 deletions(-)
> 
> I've tested with this patch.  Seems to fix cleanup issues, in particular 
> after a target is removed or at target reboot time.
> Signed-off-by: Steven Royer 
> 

I think the "Tested-by" tag is more appropriate seeing as you tested the
patch.

-Tyrel

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/1] ibmvfc: Fix I/O hang when port is not mapped

[Tyrel Datwyler]

On 09/19/2016 06:59 AM, Brian King wrote:
> If a VFC port gets unmapped in the VIOS, it may not respond with a CRQ init
> complete following H_REG_CRQ. If this occurs, we can end up having called
> scsi_block_requests and not a resulting unblock until the init complete
> happens, which may never occur, and we end up hanging I/O requests.
> This patch ensures the host action stay set to IBMVFC_HOST_ACTION_TGT_DEL so
> we move all rports into devloss state and unblock unless we receive an
> init complete. 
> 
> Signed-off-by: Brian King 

In retrospect this probably should have been queued up for stable as well.

-Tyrel

> ---
> 
>  drivers/scsi/ibmvscsi/ibmvfc.c |1 -
>  1 file changed, 1 deletion(-)
> 
> diff -puN drivers/scsi/ibmvscsi/ibmvfc.c~ibmvfc_unmap_hang_fix 
> drivers/scsi/ibmvscsi/ibmvfc.c
> --- linux-2.6.git/drivers/scsi/ibmvscsi/ibmvfc.c~ibmvfc_unmap_hang_fix
> 2016-09-09 15:46:36.452011778 -0500
> +++ linux-2.6.git-bjking1/drivers/scsi/ibmvscsi/ibmvfc.c  2016-09-09 
> 15:47:07.026632886 -0500
> @@ -717,7 +717,6 @@ static int ibmvfc_reset_crq(struct ibmvf
>   spin_lock_irqsave(vhost->host->host_lock, flags);
>   vhost->state = IBMVFC_NO_CRQ;
>   vhost->logged_in = 0;
> - ibmvfc_set_host_action(vhost, IBMVFC_HOST_ACTION_NONE);
>  
>   /* Clean out the queue */
>   memset(crq->msgs, 0, PAGE_SIZE);
> _
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/1] ibmvfc: Fix I/O hang when port is not mapped

[Tyrel Datwyler]

On 09/19/2016 06:59 AM, Brian King wrote:
> If a VFC port gets unmapped in the VIOS, it may not respond with a CRQ init
> complete following H_REG_CRQ. If this occurs, we can end up having called
> scsi_block_requests and not a resulting unblock until the init complete
> happens, which may never occur, and we end up hanging I/O requests.
> This patch ensures the host action stay set to IBMVFC_HOST_ACTION_TGT_DEL so
> we move all rports into devloss state and unblock unless we receive an
> init complete. 
> 
> Signed-off-by: Brian King <brk...@linux.vnet.ibm.com>

Acked-by: Tyrel Datwyler <tyr...@linux.vnet.ibm.com>

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v1] Ibmvscsis: Fixed unused variable

[Tyrel Datwyler]

On 09/16/2016 07:25 AM, Bryant G. Ly wrote:
> Signed-off-by: Bryant G. Ly <bryan...@linux.vnet.ibm.com>

Reviewed-by: Tyrel Datwyler <tyr...@linux.vnet.ibm.com>

> ---
>  drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c 
> b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
> index b29fef9..f8ba2a0 100644
> --- a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
> +++ b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
> @@ -3193,8 +3193,6 @@ static int ibmvscsis_rdma(struct ibmvscsis_cmd *cmd, 
> struct scatterlist *sg,
>server_ioba);
>   } else {
>   /* write to client */
> - struct srp_cmd *srp = (struct srp_cmd *)iue->sbuf->buf;
> -
>   if (!READ_CMD(srp->cdb))
>   print_hex_dump_bytes(" data:", DUMP_PREFIX_NONE,
>sg_virt(sgp), buf_len);
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] scsi: replace broken specification URL

[Tyrel Datwyler]

On 09/09/2016 01:16 AM, Michael Opdenacker wrote:
> Hi James,
> 
> Thank you very much for your help...
> 
> On 02/07/2016 16:49, James Bottomley wrote:
>> On Sat, 2016-07-02 at 08:56 +0200, Michael Opdenacker wrote:
>>> The t10.org website containing SCSI-2 draft specifications now
>>> requires to be from a member company to access the documents.
>>>
>>> This replaces the now broken link with another public resource
>>> where the specifications can be found.
>> Just because T10 implemented a pay wall for standards, doesn't mean
>> they're not still the definitive source.
>>
>> Adding a note about where you can get free versions is a useful
>> service, please do, but we have to keep the official links.  To be
>> honest the Duisberg site doesn't seem useful because it only has the
>> CAM standard.
> 
> Understood. I found another location where all the documents seem to be 
> available:
> http://www.csit-sun.pub.ro/~cpop/Documentatie_SMP/Standarde_magistrale/SCSI/

This link is just a blank page with the CSIT background image when I
follow it. This worked for me though:

http://www.csit-sun.pub.ro/~cpop/?dir=./Documentatie_SMP/Standarde_magistrale/SCSI

-Tyrel

>>
>> The Wayback machine is more useful because it keeps a copy of the site
>> (with the attached standards) just before the paywall went up:
>>
>> https://web.archive.org/web/20080828112749/http://t10.org/drafts.htm
> 
> However, the PDF file from 
> https://web.archive.org/web/20080828112749/http://t10.org/ftp/t10/drafts/cam/cam-r12b.pdf
>  
> fails to load at a 130810 byte limit. Other people have reported a 
> similar file size issue in the past.
> 
> So, should we only that the cam-r12b document can be found from 
> http://www.t10.org/t10docs.htm (registration required)?, and tell that a 
> copy can be found on 
> http://www.csit-sun.pub.ro/~cpop/Documentatie_SMP/Standarde_magistrale/SCSI/?
> 
> I'm trying to fix broken links in kernel documentation, which I publish 
> on http://free-electrons.com/kerneldoc/ . I have a broken link checker 
> for the http://free-electrons.com/ website, and it finds all the broken 
> links on http://free-electrons.com/kerneldoc/ . That's a good thing, 
> isn't need, but it means I have to get rid of the broken links :)
> 
> Thanks again for your help,
> 
> Cheers,
> 
> Michael.
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH -next] scsi: ibmvfc: fix typo in parameter description

[Tyrel Datwyler]

On 09/08/2016 08:04 AM, Wei Yongjun wrote:
> From: Wei Yongjun <weiyongj...@huawei.com>
> 
> Fix typo in parameter description.
> 
> Signed-off-by: Wei Yongjun <weiyongj...@huawei.com>

Reviewed-by: Tyrel Datwyler <tyr...@linux.vnet.ibm.com>

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH -next] ibmvscsis: Use list_move_tail instead of list_del/list_add_tail

[Tyrel Datwyler]

On 07/22/2016 07:03 AM, Wei Yongjun wrote:
> Using list_move_tail() instead of list_del() + list_add_tail().
> 
> Signed-off-by: Wei Yongjun <weiyj...@gmail.com>

Reviewed-by: Tyrel Datwyler <tyr...@linux.vnet.ibm.com>

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2] scsi: fix upper bounds check of sense key in scsi_sense_key_string()

[Tyrel Datwyler]

Commit 655ee63cf371 added a "Completed" sense string with key 0xF to
snstext[], but failed to updated the upper bounds check of the sense key in
scsi_sense_key_string().

Fixes: 655ee63cf371 ("[SCSI] scsi constants: command, sense key + additional 
sense strings")
Signed-off-by: Tyrel Datwyler <tyr...@linux.vnet.ibm.com>
Reviewed-by: Bart Van Assche <bart.vanass...@sandisk.com>
---

Changes since v1:
 - Addressed Bart's comments
 - Moved opening function brace to match kernel style guidelines
 - Changed upperbounds check to use ARRAY_SIZE instead of fixed value

 drivers/scsi/constants.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/constants.c b/drivers/scsi/constants.c
index 83458f7..6dc96c8 100644
--- a/drivers/scsi/constants.c
+++ b/drivers/scsi/constants.c
@@ -361,8 +361,9 @@ static const char * const snstext[] = {
 
 /* Get sense key string or NULL if not available */
 const char *
-scsi_sense_key_string(unsigned char key) {
-   if (key <= 0xE)
+scsi_sense_key_string(unsigned char key)
+{
+   if (key < ARRAY_SIZE(snstext))
return snstext[key];
return NULL;
 }
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] scsi: fix upper bounds check of sense key in scsi_sense_key_string()

[Tyrel Datwyler]

On 08/09/2016 01:08 PM, Bart Van Assche wrote:
> On 08/04/2016 02:32 PM, Tyrel Datwyler wrote:
>> Commit 655ee63cf371 added a "Completed" sense string with key 0xF to
>> snstext[], but failed to updated the upper bounds check of the sense key in
>> scsi_sense_key_string().
>>
>> Fixes: 655ee63cf371 ("[SCSI] scsi constants: command, sense key + additional 
>> sense strings")
>> Signed-off-by: Tyrel Datwyler <tyr...@linux.vnet.ibm.com>
>> ---
>>  drivers/scsi/constants.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/scsi/constants.c b/drivers/scsi/constants.c
>> index 83458f7..70d8dc4 100644
>> --- a/drivers/scsi/constants.c
>> +++ b/drivers/scsi/constants.c
>> @@ -362,7 +362,7 @@ static const char * const snstext[] = {
>>  /* Get sense key string or NULL if not available */
>>  const char *
>>  scsi_sense_key_string(unsigned char key) {
>> -if (key <= 0xE)
>> +if (key <= 0xF)
>>  return snstext[key];
>>  return NULL;
>>  }
> 
> Hello Tyrel,
> 
> Please move the opening brace ("{") to a line of its own.
> 
> If you change "<= 0xF" into "< ARRAY_SIZE(snstext)" then you are welcome

This actually occurred to me as a better long term solution after I had
already posted the patch.

> to add:
> 
> Reviewed-by: Bart Van Assche <bart.vanass...@sandisk.com>

Thanks for your comments. I'll address and spin a version 2.

-Tyrel

> 
> Thanks,
> 
> Bart.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/2] ibmvfc: FC-TAPE Support

[Tyrel Datwyler]

On 08/03/2016 02:36 PM, Tyrel Datwyler wrote:
> This patchset introduces optional FC-TAPE/FC Class 3 Error Recovery to the
> ibmvfc client driver.
> 
> Tyrel Datwyler (2):
>   ibmvfc: Set READ FCP_XFER_READY DISABLED bit in PRLI
>   ibmvfc: add FC Class 3 Error Recovery support
> 
>  drivers/scsi/ibmvscsi/ibmvfc.c | 11 +++
>  drivers/scsi/ibmvscsi/ibmvfc.h |  1 +
>  2 files changed, 12 insertions(+)
> 

ping?

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] scsi: fix upper bounds check of sense key in scsi_sense_key_string()

[Tyrel Datwyler]

Commit 655ee63cf371 added a "Completed" sense string with key 0xF to
snstext[], but failed to updated the upper bounds check of the sense key in
scsi_sense_key_string().

Fixes: 655ee63cf371 ("[SCSI] scsi constants: command, sense key + additional 
sense strings")
Signed-off-by: Tyrel Datwyler <tyr...@linux.vnet.ibm.com>
---
 drivers/scsi/constants.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/constants.c b/drivers/scsi/constants.c
index 83458f7..70d8dc4 100644
--- a/drivers/scsi/constants.c
+++ b/drivers/scsi/constants.c
@@ -362,7 +362,7 @@ static const char * const snstext[] = {
 /* Get sense key string or NULL if not available */
 const char *
 scsi_sense_key_string(unsigned char key) {
-   if (key <= 0xE)
+   if (key <= 0xF)
return snstext[key];
return NULL;
 }
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] ibmvfc: Set READ FCP_XFER_READY DISABLED bit in PRLI

[Tyrel Datwyler]

The READ FCP_XFER_READY DISABLED bit is required to always be set to
one since FCP-3. Set it in the service parameter page frame during
process login.

Signed-off-by: Tyrel Datwyler <tyr...@linux.vnet.ibm.com>
---
 drivers/scsi/ibmvscsi/ibmvfc.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c
index ab67ec4..4a680ce 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc.c
+++ b/drivers/scsi/ibmvscsi/ibmvfc.c
@@ -3381,6 +3381,7 @@ static void ibmvfc_tgt_send_prli(struct ibmvfc_target 
*tgt)
prli->parms.type = IBMVFC_SCSI_FCP_TYPE;
prli->parms.flags = cpu_to_be16(IBMVFC_PRLI_EST_IMG_PAIR);
prli->parms.service_parms = cpu_to_be32(IBMVFC_PRLI_INITIATOR_FUNC);
+   prli->parms.service_parms |= 
cpu_to_be32(IBMVFC_PRLI_READ_FCP_XFER_RDY_DISABLED);
 
ibmvfc_set_tgt_action(tgt, IBMVFC_TGT_ACTION_INIT_WAIT);
if (ibmvfc_send_event(evt, vhost, default_timeout)) {
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 0/2] ibmvfc: FC-TAPE Support

[Tyrel Datwyler]

This patchset introduces optional FC-TAPE/FC Class 3 Error Recovery to the
ibmvfc client driver.

Tyrel Datwyler (2):
  ibmvfc: Set READ FCP_XFER_READY DISABLED bit in PRLI
  ibmvfc: add FC Class 3 Error Recovery support

 drivers/scsi/ibmvscsi/ibmvfc.c | 11 +++
 drivers/scsi/ibmvscsi/ibmvfc.h |  1 +
 2 files changed, 12 insertions(+)

-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] ibmvfc: add FC Class 3 Error Recovery support

[Tyrel Datwyler]

The ibmvfc driver currently doesn't support FC Class 3 Error Recovery.
However, it is simply a matter of informing the VIOS that the payload
expects to use sequence level error recovery via a bit flag in the
ibmvfc_cmd structure.

This patch adds a module parameter to enable error recovery support
at boot time. When enabled the RETRY service parameter bit is set
during PRLI, and ibmvfc_cmd->flags includes the IBMVFC_CLASS_3_ERR
bit.

Signed-off-by: Tyrel Datwyler <tyr...@linux.vnet.ibm.com>
---
 drivers/scsi/ibmvscsi/ibmvfc.c | 10 ++
 drivers/scsi/ibmvscsi/ibmvfc.h |  1 +
 2 files changed, 11 insertions(+)

diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c
index 4a680ce..6b92169 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc.c
+++ b/drivers/scsi/ibmvscsi/ibmvfc.c
@@ -52,6 +52,7 @@ static unsigned int max_requests = 
IBMVFC_MAX_REQUESTS_DEFAULT;
 static unsigned int disc_threads = IBMVFC_MAX_DISC_THREADS;
 static unsigned int ibmvfc_debug = IBMVFC_DEBUG;
 static unsigned int log_level = IBMVFC_DEFAULT_LOG_LEVEL;
+static unsigned int cls3_error = IBMVFC_CLS3_ERROR;
 static LIST_HEAD(ibmvfc_head);
 static DEFINE_SPINLOCK(ibmvfc_driver_lock);
 static struct scsi_transport_template *ibmvfc_transport_template;
@@ -86,6 +87,9 @@ MODULE_PARM_DESC(debug, "Enable driver debug information. "
 module_param_named(log_level, log_level, uint, 0);
 MODULE_PARM_DESC(log_level, "Set to 0 - 4 for increasing verbosity of device 
driver. "
 "[Default=" __stringify(IBMVFC_DEFAULT_LOG_LEVEL) "]");
+module_param_named(cls3_error, cls3_error, uint, 0);
+MODULE_PARM_DESC(log_level, "Enable FC Class 3 Error Recovery. "
+"[Default=" __stringify(IBMVFC_CLS3_ERROR) "]");
 
 static const struct {
u16 status;
@@ -1335,6 +1339,9 @@ static int ibmvfc_map_sg_data(struct scsi_cmnd *scmd,
struct srp_direct_buf *data = _cmd->ioba;
struct ibmvfc_host *vhost = dev_get_drvdata(dev);
 
+   if (cls3_error)
+   vfc_cmd->flags |= cpu_to_be16(IBMVFC_CLASS_3_ERR);
+
sg_mapped = scsi_dma_map(scmd);
if (!sg_mapped) {
vfc_cmd->flags |= cpu_to_be16(IBMVFC_NO_MEM_DESC);
@@ -3383,6 +3390,9 @@ static void ibmvfc_tgt_send_prli(struct ibmvfc_target 
*tgt)
prli->parms.service_parms = cpu_to_be32(IBMVFC_PRLI_INITIATOR_FUNC);
prli->parms.service_parms |= 
cpu_to_be32(IBMVFC_PRLI_READ_FCP_XFER_RDY_DISABLED);
 
+   if (cls3_error)
+   prli->parms.service_parms |= cpu_to_be32(IBMVFC_PRLI_RETRY);
+
ibmvfc_set_tgt_action(tgt, IBMVFC_TGT_ACTION_INIT_WAIT);
if (ibmvfc_send_event(evt, vhost, default_timeout)) {
vhost->discovery_threads--;
diff --git a/drivers/scsi/ibmvscsi/ibmvfc.h b/drivers/scsi/ibmvscsi/ibmvfc.h
index 8fae032..7f9bb07 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc.h
+++ b/drivers/scsi/ibmvscsi/ibmvfc.h
@@ -54,6 +54,7 @@
 #define IBMVFC_DEV_LOSS_TMO(5 * 60)
 #define IBMVFC_DEFAULT_LOG_LEVEL   2
 #define IBMVFC_MAX_CDB_LEN 16
+#define IBMVFC_CLS3_ERROR  0
 
 /*
  * Ensure we have resources for ERP and initialization:
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] lpfc: Fix possible NULL pointer dereference

[Tyrel Datwyler]

On 07/29/2016 06:30 AM, Johannes Thumshirn wrote:
> Check for the existence of piocb->vport before accessing it.
> 
> Signed-off-by: Johannes Thumshirn <jthumsh...@suse.de>

Reviewed-by: Tyrel Datwyler <tyr...@linux.vnet.ibm.com>
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] lpfc: Fix possible NULL pointer dereference

[Tyrel Datwyler]

On 06/15/2016 06:00 AM, Johannes Thumshirn wrote:
> Check for the existance of pciob->vport before accessing it.

piocb mispelled.

> 
> Signed-off-by: Johannes Thumshirn 
> ---
>  drivers/scsi/lpfc/lpfc_sli.c | 13 -
>  1 file changed, 4 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/scsi/lpfc/lpfc_sli.c b/drivers/scsi/lpfc/lpfc_sli.c
> index 70edf21..134078f 100644
> --- a/drivers/scsi/lpfc/lpfc_sli.c
> +++ b/drivers/scsi/lpfc/lpfc_sli.c
> @@ -1329,15 +1329,10 @@ lpfc_sli_ringtxcmpl_put(struct lpfc_hba *phba, struct 
> lpfc_sli_ring *pring,
>   if ((unlikely(pring->ringno == LPFC_ELS_RING)) &&
>  (piocb->iocb.ulpCommand != CMD_ABORT_XRI_CN) &&
>  (piocb->iocb.ulpCommand != CMD_CLOSE_XRI_CN) &&
> -  (!(piocb->vport->load_flag & FC_UNLOADING))) {
> - if (!piocb->vport)
> - BUG();

Granted the previous code would crash and burn in the if statement prior
to the BUG() assertion if piocb->vport was NULL, but is the condition
!piocb->vport still a bug here? Should that case still be asserted?

-Tyrel

> - else
> - mod_timer(>vport->els_tmofunc,
> - jiffies +
> - msecs_to_jiffies(1000 * (phba->fc_ratov << 1)));
> - }
> -
> + piocb->vport && !(piocb->vport->load_flag & FC_UNLOADING))
> + mod_timer(>vport->els_tmofunc,
> +   jiffies +
> +   msecs_to_jiffies(1000 * (phba->fc_ratov << 1)));
>  
>   return 0;
>  }
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

SHOST_RECOVERY hang with host_busy < host_failed

[Tyrel Datwyler]

I'm seeing a non-deterministic I/O hang with the ibmvfc driver when we
migrate a guest/partition to a different machine. So, on the occasions
when this hits the systems becomes unresponsive blocked on I/O. I
started by inspecting Scsi_Host of the driver which shows shost->state =
SHOST_RECOVERY explaining the blocked I/O. What I found a little weird
however was shost->host_failed > shost->host_busy.

>From the EH documentation SHOST_RECOVERY is set by scsi_eh_scmd_add thus
blocking any new scmd's from the block queue to the host. So, every
scmnd in flight either completes or fails with failed commands being
inclusive of busy commands count, and the EH thread started once all
busy commands are failed commands (ie. host_busy == host_failed). So,
this should mean host_failed should never by greater than host_busy,
correct?

-Tyrel

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 3/7] ibmvscsi: Replace magic values in set_adpater_info() with defines

[Tyrel Datwyler]

On 02/12/2016 08:43 AM, James Bottomley wrote:
> On Wed, 2016-02-10 at 19:32 -0600, Tyrel Datwyler wrote:
>> Add defines for mad version and mad os_type, and replace the magic
>> numbers in set_adapter_info() accordingly.
>>
>> Signed-off-by: Tyrel Datwyler <tyr...@linux.vnet.ibm.com>
>> ---
> 
> Is there some reason you didn't carry the review tag over from this:
> 
> http://mid.gmane.org/20160204084459.gw27...@c203.arch.suse.de
> 
> ?
> 
> James

The patch is slightly changed from v1. A define for AIX os type was
added as mentioned in the cover letter v2 changes, and I moved the
defines to the mad_adapter_info_data structure around the fields they apply.

-Tyrel

> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] ibmvfc: byteswap scsi_id, wwpn, and node_name prior to logging

[Tyrel Datwyler]

When logging async events the scsi_id, wwpn, and node_name values
are used directly from the CRQ struct which are of type __be64. This
can be confusing to someone looking through the log on a LE system.
Instead byteswap these values to host endian prior to logging.

Signed-off-by: Tyrel Datwyler <tyr...@linux.vnet.ibm.com>
---
 drivers/scsi/ibmvscsi/ibmvfc.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c
index 6aa317c..fc523c3 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc.c
+++ b/drivers/scsi/ibmvscsi/ibmvfc.c
@@ -2636,7 +2636,8 @@ static void ibmvfc_handle_async(struct ibmvfc_async_crq 
*crq,
struct ibmvfc_target *tgt;
 
ibmvfc_log(vhost, desc->log_level, "%s event received. scsi_id: %llx, 
wwpn: %llx,"
-  " node_name: %llx%s\n", desc->desc, crq->scsi_id, crq->wwpn, 
crq->node_name,
+  " node_name: %llx%s\n", desc->desc, 
be64_to_cpu(crq->scsi_id),
+  be64_to_cpu(crq->wwpn), be64_to_cpu(crq->node_name),
   ibmvfc_get_link_state(crq->link_state));
 
switch (be64_to_cpu(crq->event)) {
-- 
2.5.0

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 0/7] ibmvscsi code cleanup

[Tyrel Datwyler]

Fixed up a couple spots that were out of line with the PAPR in regards
to its defined VSCSI protocol. Did away with some magic numbers directly
in the code. Fixed a minor endian issue.

--

v2 changes:

-Renamed CRQ header enums and added enums for INIT formats
-Check that crq->valid != VIOSRP_CRQ_FREE before handling in place
of hacky bitwise & to check for first bit being set.
-Added define for AIX os_type
-Left sysfs config attribute to prevent breaking userspace

Tyrel Datwyler (7):
  ibmvscsi: Correct values for several viosrp_crq_format enums
  ibmvscsi: Add and use enums for valid CRQ header values
  ibmvscsi: Replace magic values in set_adpater_info() with defines
  ibmvscsi: Use of_root to access OF device tree root node
  ibmvscsi: Remove unsupported host config MAD
  ibmvscsi: Add endian conversions to sysfs attribute show functions
  ibmvscsi: use H_CLOSED instead of magic number

 drivers/scsi/ibmvscsi/ibmvscsi.c | 128 ++-
 drivers/scsi/ibmvscsi/viosrp.h   |  26 +---
 2 files changed, 49 insertions(+), 105 deletions(-)

-- 
2.5.0

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 4/7] ibmvscsi: Use of_root to access OF device tree root node

[Tyrel Datwyler]

The root node of the OF device tree is exported as of_root. No need
to look up the root by path name. Instead just get a reference
directly via of_root.

Signed-off-by: Tyrel Datwyler <tyr...@linux.vnet.ibm.com>
Reviewed-by: Johannes Thumshirn <jthumsh...@suse.de>
---
 drivers/scsi/ibmvscsi/ibmvscsi.c | 14 ++
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/scsi/ibmvscsi/ibmvscsi.c b/drivers/scsi/ibmvscsi/ibmvscsi.c
index 4b09a9b..c208295 100644
--- a/drivers/scsi/ibmvscsi/ibmvscsi.c
+++ b/drivers/scsi/ibmvscsi/ibmvscsi.c
@@ -248,25 +248,23 @@ static void ibmvscsi_task(void *data)
 
 static void gather_partition_info(void)
 {
-   struct device_node *rootdn;
-
const char *ppartition_name;
const __be32 *p_number_ptr;
 
/* Retrieve information about this partition */
-   rootdn = of_find_node_by_path("/");
-   if (!rootdn) {
+   if (!of_root)
return;
-   }
 
-   ppartition_name = of_get_property(rootdn, "ibm,partition-name", NULL);
+   of_node_get(of_root);
+
+   ppartition_name = of_get_property(of_root, "ibm,partition-name", NULL);
if (ppartition_name)
strncpy(partition_name, ppartition_name,
sizeof(partition_name));
-   p_number_ptr = of_get_property(rootdn, "ibm,partition-no", NULL);
+   p_number_ptr = of_get_property(of_root, "ibm,partition-no", NULL);
if (p_number_ptr)
partition_number = of_read_number(p_number_ptr, 1);
-   of_node_put(rootdn);
+   of_node_put(of_root);
 }
 
 static void set_adapter_info(struct ibmvscsi_host_data *hostdata)
-- 
2.5.0

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 6/7] ibmvscsi: Add endian conversions to sysfs attribute show functions

[Tyrel Datwyler]

The values returned by the show functions for the host os_type,
mad_version, and partition_number attributes get their values
directly from the madapter_info struct whose associated fields are
__be32 typed. Added endian conversion to ensure these values are
sane on LE platforms.

Signed-off-by: Tyrel Datwyler <tyr...@linux.vnet.ibm.com>
Reviewed-by: Johannes Thumshirn <jthumsh...@suse.de>
---
 drivers/scsi/ibmvscsi/ibmvscsi.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/ibmvscsi/ibmvscsi.c b/drivers/scsi/ibmvscsi/ibmvscsi.c
index e8d4af5..6025481 100644
--- a/drivers/scsi/ibmvscsi/ibmvscsi.c
+++ b/drivers/scsi/ibmvscsi/ibmvscsi.c
@@ -1983,7 +1983,7 @@ static ssize_t show_host_partition_number(struct device 
*dev,
int len;
 
len = snprintf(buf, PAGE_SIZE, "%d\n",
-  hostdata->madapter_info.partition_number);
+  be32_to_cpu(hostdata->madapter_info.partition_number));
return len;
 }
 
@@ -2003,7 +2003,7 @@ static ssize_t show_host_mad_version(struct device *dev,
int len;
 
len = snprintf(buf, PAGE_SIZE, "%d\n",
-  hostdata->madapter_info.mad_version);
+  be32_to_cpu(hostdata->madapter_info.mad_version));
return len;
 }
 
@@ -2022,7 +2022,8 @@ static ssize_t show_host_os_type(struct device *dev,
struct ibmvscsi_host_data *hostdata = shost_priv(shost);
int len;
 
-   len = snprintf(buf, PAGE_SIZE, "%d\n", hostdata->madapter_info.os_type);
+   len = snprintf(buf, PAGE_SIZE, "%d\n",
+  be32_to_cpu(hostdata->madapter_info.os_type));
return len;
 }
 
-- 
2.5.0

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 7/7] ibmvscsi: use H_CLOSED instead of magic number

[Tyrel Datwyler]

In a couple places the magic value of 2 is used to check the return
code of hypercalls. This translates to H_CLOSED.

Signed-off-by: Tyrel Datwyler <tyr...@linux.vnet.ibm.com>
---
 drivers/scsi/ibmvscsi/ibmvscsi.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/ibmvscsi/ibmvscsi.c b/drivers/scsi/ibmvscsi/ibmvscsi.c
index 6025481..d9534ee 100644
--- a/drivers/scsi/ibmvscsi/ibmvscsi.c
+++ b/drivers/scsi/ibmvscsi/ibmvscsi.c
@@ -314,7 +314,7 @@ static int ibmvscsi_reset_crq_queue(struct crq_queue *queue,
rc = plpar_hcall_norets(H_REG_CRQ,
vdev->unit_address,
queue->msg_token, PAGE_SIZE);
-   if (rc == 2) {
+   if (rc == H_CLOSED) {
/* Adapter is good, but other end is not ready */
dev_warn(hostdata->dev, "Partner adapter not ready\n");
} else if (rc != 0) {
@@ -364,7 +364,7 @@ static int ibmvscsi_init_crq_queue(struct crq_queue *queue,
rc = ibmvscsi_reset_crq_queue(queue,
  hostdata);
 
-   if (rc == 2) {
+   if (rc == H_CLOSED) {
/* Adapter is good, but other end is not ready */
dev_warn(hostdata->dev, "Partner adapter not ready\n");
retrc = 0;
-- 
2.5.0

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 3/7] ibmvscsi: Replace magic values in set_adpater_info() with defines

[Tyrel Datwyler]

Add defines for mad version and mad os_type, and replace the magic
numbers in set_adapter_info() accordingly.

Signed-off-by: Tyrel Datwyler <tyr...@linux.vnet.ibm.com>
---
 drivers/scsi/ibmvscsi/ibmvscsi.c | 8 
 drivers/scsi/ibmvscsi/viosrp.h   | 3 +++
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/ibmvscsi/ibmvscsi.c b/drivers/scsi/ibmvscsi/ibmvscsi.c
index c888ea1..4b09a9b 100644
--- a/drivers/scsi/ibmvscsi/ibmvscsi.c
+++ b/drivers/scsi/ibmvscsi/ibmvscsi.c
@@ -283,8 +283,8 @@ static void set_adapter_info(struct ibmvscsi_host_data 
*hostdata)
hostdata->madapter_info.partition_number =
cpu_to_be32(partition_number);
 
-   hostdata->madapter_info.mad_version = cpu_to_be32(1);
-   hostdata->madapter_info.os_type = cpu_to_be32(2);
+   hostdata->madapter_info.mad_version = cpu_to_be32(SRP_MAD_VERSION_1);
+   hostdata->madapter_info.os_type = cpu_to_be32(SRP_MAD_OS_LINUX);
 }
 
 /**
@@ -1398,7 +1398,7 @@ static void adapter_info_rsp(struct srp_event_struct 
*evt_struct)
hostdata->host->max_sectors = 

be32_to_cpu(hostdata->madapter_info.port_max_txu[0]) >> 9;

-   if (be32_to_cpu(hostdata->madapter_info.os_type) == 3 &&
+   if (be32_to_cpu(hostdata->madapter_info.os_type) == 
SRP_MAD_OS_AIX &&
strcmp(hostdata->madapter_info.srp_version, "1.6a") <= 0) {
dev_err(hostdata->dev, "host (Ver. %s) doesn't support 
large transfers\n",
hostdata->madapter_info.srp_version);
@@ -1407,7 +1407,7 @@ static void adapter_info_rsp(struct srp_event_struct 
*evt_struct)
hostdata->host->sg_tablesize = MAX_INDIRECT_BUFS;
}
 
-   if (be32_to_cpu(hostdata->madapter_info.os_type) == 3) {
+   if (be32_to_cpu(hostdata->madapter_info.os_type) == 
SRP_MAD_OS_AIX) {
enable_fast_fail(hostdata);
return;
}
diff --git a/drivers/scsi/ibmvscsi/viosrp.h b/drivers/scsi/ibmvscsi/viosrp.h
index 3d20851..d0f689b 100644
--- a/drivers/scsi/ibmvscsi/viosrp.h
+++ b/drivers/scsi/ibmvscsi/viosrp.h
@@ -221,7 +221,10 @@ struct mad_adapter_info_data {
char srp_version[8];
char partition_name[96];
__be32 partition_number;
+#define SRP_MAD_VERSION_1 1
__be32 mad_version;
+#define SRP_MAD_OS_LINUX 2
+#define SRP_MAD_OS_AIX 3
__be32 os_type;
__be32 port_max_txu[8]; /* per-port maximum transfer */
 };
-- 
2.5.0

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 5/7] ibmvscsi: Remove unsupported host config MAD

[Tyrel Datwyler]

A VIOSRP_HOST_CONFIG_TYPE management datagram (MAD) has existed in
the code for some time. From what information I've gathered from
Brian King this was likely implemented on the host side in a SLES 9
based VIOS, which is no longer supported anywhere. Further, it is
not defined in PAPR or supported by any AIX based VIOS.

Treating as bit rot and removing the associated host config code.
The config attribute and its show function are left as not to break
userspace. The behavior remains the same returning nothing.

Signed-off-by: Tyrel Datwyler <tyr...@linux.vnet.ibm.com>
---
 drivers/scsi/ibmvscsi/ibmvscsi.c | 71 +++-
 drivers/scsi/ibmvscsi/viosrp.h   |  7 
 2 files changed, 4 insertions(+), 74 deletions(-)

diff --git a/drivers/scsi/ibmvscsi/ibmvscsi.c b/drivers/scsi/ibmvscsi/ibmvscsi.c
index c208295..e8d4af5 100644
--- a/drivers/scsi/ibmvscsi/ibmvscsi.c
+++ b/drivers/scsi/ibmvscsi/ibmvscsi.c
@@ -1853,62 +1853,6 @@ static void ibmvscsi_handle_crq(struct viosrp_crq *crq,
 }
 
 /**
- * ibmvscsi_get_host_config: Send the command to the server to get host
- * configuration data.  The data is opaque to us.
- */
-static int ibmvscsi_do_host_config(struct ibmvscsi_host_data *hostdata,
-  unsigned char *buffer, int length)
-{
-   struct viosrp_host_config *host_config;
-   struct srp_event_struct *evt_struct;
-   unsigned long flags;
-   dma_addr_t addr;
-   int rc;
-
-   evt_struct = get_event_struct(>pool);
-   if (!evt_struct) {
-   dev_err(hostdata->dev, "couldn't allocate event for 
HOST_CONFIG!\n");
-   return -1;
-   }
-
-   init_event_struct(evt_struct,
- sync_completion,
- VIOSRP_MAD_FORMAT,
- info_timeout);
-
-   host_config = _struct->iu.mad.host_config;
-
-   /* The transport length field is only 16-bit */
-   length = min(0x, length);
-
-   /* Set up a lun reset SRP command */
-   memset(host_config, 0x00, sizeof(*host_config));
-   host_config->common.type = cpu_to_be32(VIOSRP_HOST_CONFIG_TYPE);
-   host_config->common.length = cpu_to_be16(length);
-   addr = dma_map_single(hostdata->dev, buffer, length, DMA_BIDIRECTIONAL);
-
-   if (dma_mapping_error(hostdata->dev, addr)) {
-   if (!firmware_has_feature(FW_FEATURE_CMO))
-   dev_err(hostdata->dev,
-   "dma_mapping error getting host config\n");
-   free_event_struct(>pool, evt_struct);
-   return -1;
-   }
-
-   host_config->buffer = cpu_to_be64(addr);
-
-   init_completion(_struct->comp);
-   spin_lock_irqsave(hostdata->host->host_lock, flags);
-   rc = ibmvscsi_send_srp_event(evt_struct, hostdata, info_timeout * 2);
-   spin_unlock_irqrestore(hostdata->host->host_lock, flags);
-   if (rc == 0)
-   wait_for_completion(_struct->comp);
-   dma_unmap_single(hostdata->dev, addr, length, DMA_BIDIRECTIONAL);
-
-   return rc;
-}
-
-/**
  * ibmvscsi_slave_configure: Set the "allow_restart" flag for each disk.
  * @sdev:  struct scsi_device device to configure
  *
@@ -2093,21 +2037,14 @@ static struct device_attribute ibmvscsi_host_os_type = {
 static ssize_t show_host_config(struct device *dev,
struct device_attribute *attr, char *buf)
 {
-   struct Scsi_Host *shost = class_to_shost(dev);
-   struct ibmvscsi_host_data *hostdata = shost_priv(shost);
-
-   /* returns null-terminated host config data */
-   if (ibmvscsi_do_host_config(hostdata, buf, PAGE_SIZE) == 0)
-   return strlen(buf);
-   else
-   return 0;
+   return 0;
 }
 
 static struct device_attribute ibmvscsi_host_config = {
.attr = {
-.name = "config",
-.mode = S_IRUGO,
-},
+   .name = "config",
+   .mode = S_IRUGO,
+   },
.show = show_host_config,
 };
 
diff --git a/drivers/scsi/ibmvscsi/viosrp.h b/drivers/scsi/ibmvscsi/viosrp.h
index d0f689b..c1ab8a4 100644
--- a/drivers/scsi/ibmvscsi/viosrp.h
+++ b/drivers/scsi/ibmvscsi/viosrp.h
@@ -99,7 +99,6 @@ enum viosrp_mad_types {
VIOSRP_EMPTY_IU_TYPE = 0x01,
VIOSRP_ERROR_LOG_TYPE = 0x02,
VIOSRP_ADAPTER_INFO_TYPE = 0x03,
-   VIOSRP_HOST_CONFIG_TYPE = 0x04,
VIOSRP_CAPABILITIES_TYPE = 0x05,
VIOSRP_ENABLE_FAST_FAIL = 0x08,
 };
@@ -165,11 +164,6 @@ struct viosrp_adapter_info {
__be64 buffer;
 };
 
-struct viosrp_host_config {
-   struct mad_common common;
-   __be64 buffer;
-};
-
 struct viosrp_fast_fail {
struct mad_common common;
 };
@@ -207,7 +201,6 @@ union mad_iu {
struct viosrp_empty_iu empty_iu;
struct vio

[PATCH v2 2/7] ibmvscsi: Add and use enums for valid CRQ header values

[Tyrel Datwyler]

The PAPR defines four valid header values for the first byte of a
CRQ message. Namely, an unused/empty message (0x00), a valid
command/response entry (0x80), a valid initialization entry (0xC0),
and a valid transport event (0xFF). Further, initialization responses
have two formats namely initialize (0x01) and initialize complete
(0x02). Define these values as enums and use them in the code in
place of their magic number equivalents.

Signed-off-by: Tyrel Datwyler <tyr...@linux.vnet.ibm.com>
---
 drivers/scsi/ibmvscsi/ibmvscsi.c | 18 +-
 drivers/scsi/ibmvscsi/viosrp.h   | 12 
 2 files changed, 21 insertions(+), 9 deletions(-)

diff --git a/drivers/scsi/ibmvscsi/ibmvscsi.c b/drivers/scsi/ibmvscsi/ibmvscsi.c
index adfef9d..c888ea1 100644
--- a/drivers/scsi/ibmvscsi/ibmvscsi.c
+++ b/drivers/scsi/ibmvscsi/ibmvscsi.c
@@ -182,7 +182,7 @@ static struct viosrp_crq *crq_queue_next_crq(struct 
crq_queue *queue)
 
spin_lock_irqsave(>lock, flags);
crq = >msgs[queue->cur];
-   if (crq->valid & 0x80) {
+   if (crq->valid != VIOSRP_CRQ_FREE) {
if (++queue->cur == queue->size)
queue->cur = 0;
 
@@ -231,7 +231,7 @@ static void ibmvscsi_task(void *data)
/* Pull all the valid messages off the CRQ */
while ((crq = crq_queue_next_crq(>queue)) != NULL) {
ibmvscsi_handle_crq(crq, hostdata);
-   crq->valid = 0x00;
+   crq->valid = VIOSRP_CRQ_FREE;
}
 
vio_enable_interrupts(vdev);
@@ -239,7 +239,7 @@ static void ibmvscsi_task(void *data)
if (crq != NULL) {
vio_disable_interrupts(vdev);
ibmvscsi_handle_crq(crq, hostdata);
-   crq->valid = 0x00;
+   crq->valid = VIOSRP_CRQ_FREE;
} else {
done = 1;
}
@@ -474,7 +474,7 @@ static int initialize_event_pool(struct event_pool *pool,
struct srp_event_struct *evt = >events[i];
memset(>crq, 0x00, sizeof(evt->crq));
atomic_set(>free, 1);
-   evt->crq.valid = 0x80;
+   evt->crq.valid = VIOSRP_CRQ_CMD_RSP;
evt->crq.IU_length = cpu_to_be16(sizeof(*evt->xfer_iu));
evt->crq.IU_data_ptr = cpu_to_be64(pool->iu_token +
sizeof(*evt->xfer_iu) * i);
@@ -1767,9 +1767,9 @@ static void ibmvscsi_handle_crq(struct viosrp_crq *crq,
struct srp_event_struct *evt_struct =
(__force struct srp_event_struct *)crq->IU_data_ptr;
switch (crq->valid) {
-   case 0xC0:  /* initialization */
+   case VIOSRP_CRQ_INIT_RSP:   /* initialization */
switch (crq->format) {
-   case 0x01:  /* Initialization message */
+   case VIOSRP_CRQ_INIT:   /* Initialization message */
dev_info(hostdata->dev, "partner initialized\n");
/* Send back a response */
rc = ibmvscsi_send_crq(hostdata, 0xC002LL, 
0);
@@ -1781,7 +1781,7 @@ static void ibmvscsi_handle_crq(struct viosrp_crq *crq,
}
 
break;
-   case 0x02:  /* Initialization response */
+   case VIOSRP_CRQ_INIT_COMPLETE:  /* Initialization response */
dev_info(hostdata->dev, "partner initialization 
complete\n");
 
/* Now login */
@@ -1791,7 +1791,7 @@ static void ibmvscsi_handle_crq(struct viosrp_crq *crq,
dev_err(hostdata->dev, "unknown crq message type: 
%d\n", crq->format);
}
return;
-   case 0xFF:  /* Hypervisor telling us the connection is closed */
+   case VIOSRP_CRQ_XPORT_EVENT:/* Hypervisor telling us the connection 
is closed */
scsi_block_requests(hostdata->host);
atomic_set(>request_limit, 0);
if (crq->format == 0x06) {
@@ -1807,7 +1807,7 @@ static void ibmvscsi_handle_crq(struct viosrp_crq *crq,
ibmvscsi_reset_host(hostdata);
}
return;
-   case 0x80:  /* real payload */
+   case VIOSRP_CRQ_CMD_RSP:/* real payload */
break;
default:
dev_err(hostdata->dev, "got an invalid message type 0x%02x\n",
diff --git a/drivers/scsi/ibmvscsi/viosrp.h b/drivers/scsi/ibmvscsi/viosrp.h
index d1044e9..3d20851 100644
--- a/drivers/scsi/ibmvscsi/viosrp.h
+++ b/drivers/scsi/ibmvscsi/viosrp.h
@@ -51,6 +51,18 @@ union srp_iu {
u8 reserved[SRP_MAX_IU_LEN];
 };
 
+enum viosrp

[PATCH v2 1/7] ibmvscsi: Correct values for several viosrp_crq_format enums

[Tyrel Datwyler]

The enum values for VIOSRP_LINUX_FORMAT and VIOSRP_INLINE_FORMAT are
off by one. They are currently defined as 0x06 and 0x07 respetively.
These values are defined in PAPR correctly as 0x05 and 0x06. This
inconsistency has gone unnoticed as neither enum is currently used.
The possible future support of PING messages between the VIOS and
client adapter relies on VIOSRP_INLINE_FORMAT crq messages.
Corrected these enum values to match PAPR definitions.

Signed-off-by: Tyrel Datwyler <tyr...@linux.vnet.ibm.com>
Reviewed-by: Johannes Thumshirn <jthumsh...@suse.de>
---
 drivers/scsi/ibmvscsi/viosrp.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/ibmvscsi/viosrp.h b/drivers/scsi/ibmvscsi/viosrp.h
index 1162430..d1044e9 100644
--- a/drivers/scsi/ibmvscsi/viosrp.h
+++ b/drivers/scsi/ibmvscsi/viosrp.h
@@ -56,8 +56,8 @@ enum viosrp_crq_formats {
VIOSRP_MAD_FORMAT = 0x02,
VIOSRP_OS400_FORMAT = 0x03,
VIOSRP_AIX_FORMAT = 0x04,
-   VIOSRP_LINUX_FORMAT = 0x06,
-   VIOSRP_INLINE_FORMAT = 0x07
+   VIOSRP_LINUX_FORMAT = 0x05,
+   VIOSRP_INLINE_FORMAT = 0x06
 };
 
 enum viosrp_crq_status {
-- 
2.5.0

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/6] ibmvscsi: Add and use enums for valid CRQ header values

[Tyrel Datwyler]

On 02/09/2016 09:41 AM, Manoj Kumar wrote:
>> Yeah, I can see how that is confusing. Since, all three possible valid
>> crq message types have the first bit set I think this was originally a
>> cute hack to grab anything that was likely valid. Then in
>> ibmvscsi_handle_crq() we explicitly match the full header value in a
>> switch statement logging anything that turned out actually invalid.
>>
>>>
>>> If 'valid' will only have one of these four enums defined, would
>>> this be better written as:
>>>
>>> if (crq->valid != VIOSRP_CRQ_FREE)
>>
>> This definitely would make the logic easier to read and follow. Also,
>> this would make sure any crq with an invalid header that doesn't have
>> its first bit set will also be logged by the ibmvscsi_handle_crq()
>> switch statement default block and not silently ignored.
>>
>> -Tyrel
> 
> Sounds good, Tyrel. Does this mean I should expect a v2 of this patch
> series?
> 
> - Manoj N. Kumar

Haven't had a chance to clean up and resubmit, but yes there will be a
v2 coming along soon.

-Tyrel

> 
> ___
> Linuxppc-dev mailing list
> linuxppc-...@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/6] ibmvscsi: Add and use enums for valid CRQ header values

[Tyrel Datwyler]

On 02/04/2016 11:55 AM, Manoj Kumar wrote:
> On 2/3/2016 5:28 PM, Tyrel Datwyler wrote:
>> The PAPR defines four valid header values for the first byte of a
>> CRQ message. Namely, an unused/empty message (0x00), a valid
>> command/response entry (0x80), a valid initialization entry (0xC0),
>> and a transport event (0xFF). Define these values as enums and use
>> them in the code in place of their magic number equivalents.
>>
>> Signed-off-by: Tyrel Datwyler <tyr...@linux.vnet.ibm.com>
>> ---
>>   drivers/scsi/ibmvscsi/ibmvscsi.c | 14 +++---
>>   drivers/scsi/ibmvscsi/viosrp.h   |  7 +++
>>   2 files changed, 14 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/scsi/ibmvscsi/ibmvscsi.c 
>> b/drivers/scsi/ibmvscsi/ibmvscsi.c
>> index adfef9d..176260d 100644
>> --- a/drivers/scsi/ibmvscsi/ibmvscsi.c
>> +++ b/drivers/scsi/ibmvscsi/ibmvscsi.c
>> @@ -182,7 +182,7 @@ static struct viosrp_crq *crq_queue_next_crq(struct 
>> crq_queue *queue)
>>
>>  spin_lock_irqsave(>lock, flags);
>>  crq = >msgs[queue->cur];
>> -if (crq->valid & 0x80) {
>> +if (crq->valid & VIOSRP_CRQ_VALID) {
> 
> After the switch to enums, bitwise operators are a bit misleading.
> Especially in this case since multiple values would satisfy this
> condition: VIOSRP_CRQ_VALID, VIOSRP_CRQ_INIT and
> VIOSRP_CRQ_TRANSPORT.

Yeah, I can see how that is confusing. Since, all three possible valid
crq message types have the first bit set I think this was originally a
cute hack to grab anything that was likely valid. Then in
ibmvscsi_handle_crq() we explicitly match the full header value in a
switch statement logging anything that turned out actually invalid.

> 
> If 'valid' will only have one of these four enums defined, would
> this be better written as:
> 
>   if (crq->valid != VIOSRP_CRQ_FREE)

This definitely would make the logic easier to read and follow. Also,
this would make sure any crq with an invalid header that doesn't have
its first bit set will also be logged by the ibmvscsi_handle_crq()
switch statement default block and not silently ignored.

-Tyrel

> 
>>  if (++queue->cur == queue->size)
>>  queue->cur = 0;
>>
>> @@ -231,7 +231,7 @@ static void ibmvscsi_task(void *data)
>>  /* Pull all the valid messages off the CRQ */
>>  while ((crq = crq_queue_next_crq(>queue)) != NULL) {
>>  ibmvscsi_handle_crq(crq, hostdata);
>> -crq->valid = 0x00;
>> +crq->valid = VIOSRP_CRQ_FREE;
>>  }
>>
>>  vio_enable_interrupts(vdev);
>> @@ -239,7 +239,7 @@ static void ibmvscsi_task(void *data)
>>  if (crq != NULL) {
>>  vio_disable_interrupts(vdev);
>>  ibmvscsi_handle_crq(crq, hostdata);
>> -crq->valid = 0x00;
>> +crq->valid = VIOSRP_CRQ_FREE;
>>  } else {
>>  done = 1;
>>  }
>> @@ -474,7 +474,7 @@ static int initialize_event_pool(struct event_pool *pool,
>>  struct srp_event_struct *evt = >events[i];
>>  memset(>crq, 0x00, sizeof(evt->crq));
>>  atomic_set(>free, 1);
>> -evt->crq.valid = 0x80;
>> +evt->crq.valid = VIOSRP_CRQ_VALID;
>>  evt->crq.IU_length = cpu_to_be16(sizeof(*evt->xfer_iu));
>>  evt->crq.IU_data_ptr = cpu_to_be64(pool->iu_token +
>>  sizeof(*evt->xfer_iu) * i);
>> @@ -1767,7 +1767,7 @@ static void ibmvscsi_handle_crq(struct viosrp_crq *crq,
>>  struct srp_event_struct *evt_struct =
>>  (__force struct srp_event_struct *)crq->IU_data_ptr;
>>  switch (crq->valid) {
>> -case 0xC0:  /* initialization */
>> +case VIOSRP_CRQ_INIT:   /* initialization */
>>  switch (crq->format) {
>>  case 0x01:  /* Initialization message */
>>  dev_info(hostdata->dev, "partner initialized\n");
>> @@ -1791,7 +1791,7 @@ static void ibmvscsi_handle_crq(struct viosrp_crq *crq,
>>  dev_err(hostdata->dev, "unknown crq message type: 
>> %d\n", crq->format);
>>  }
>>  return;
>> -case 0xFF:  /* Hypervisor telling us the connection is closed */
>> +case VIOSRP_CRQ_TRANSPORT:  /* Hypervisor telling us the connection 
>> is

Re: [PATCH 5/6] ibmvscsi: Remove unsupported host config MAD and sysfs interface

[Tyrel Datwyler]

On 02/04/2016 12:03 AM, Johannes Thumshirn wrote:
> On Wed, Feb 03, 2016 at 05:28:33PM -0600, Tyrel Datwyler wrote:
>> A VIOSRP_HOST_CONFIG_TYPE management datagram (MAD) has existed in
>> the code for some time. From what information I've gathered from
>> Brian King this was likely implemented on the host side in a SLES 9
>> based VIOS, which is no longer supported anywhere. Further, it is
>> not defined in PAPR or supported by any AIX based VIOS.
>>
>> Treating as bit rot and removing the sysfs interface and associated
>> host config code accordingly.
> 
> Doesn't removing a sysfs interface potentially break userspace code?
> 

In the general case yes, but I feel in this case no. First, Reading from
this config attribute of a vscsi host adapter always returns nothing.
Second, any userspace code using this attribute better be checking for
the existence of config. Just a quick look for
/sys/class/scsi_host/host*/config under other host adapters on my system
I find that attribute doesn't exist for any of them.

If there is truly enough concern that somebody may actually be accessing
this useless attribute from userspace then we can still strip out the
unsupported code, but leave the attribute and return nothing directly
from the show function.

-Tyrel




--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 4/6] ibmvscsi: Use of_root to access OF device tree root node

[Tyrel Datwyler]

The root node of the OF device tree is exported as of_root. No need
to look up the root by path name. Instead just get a reference
directly via of_root.

Signed-off-by: Tyrel Datwyler <tyr...@linux.vnet.ibm.com>
---
 drivers/scsi/ibmvscsi/ibmvscsi.c | 14 ++
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/scsi/ibmvscsi/ibmvscsi.c b/drivers/scsi/ibmvscsi/ibmvscsi.c
index 7e51615..47cfe33 100644
--- a/drivers/scsi/ibmvscsi/ibmvscsi.c
+++ b/drivers/scsi/ibmvscsi/ibmvscsi.c
@@ -248,25 +248,23 @@ static void ibmvscsi_task(void *data)
 
 static void gather_partition_info(void)
 {
-   struct device_node *rootdn;
-
const char *ppartition_name;
const __be32 *p_number_ptr;
 
/* Retrieve information about this partition */
-   rootdn = of_find_node_by_path("/");
-   if (!rootdn) {
+   if (!of_root)
return;
-   }
 
-   ppartition_name = of_get_property(rootdn, "ibm,partition-name", NULL);
+   of_node_get(of_root);
+
+   ppartition_name = of_get_property(of_root, "ibm,partition-name", NULL);
if (ppartition_name)
strncpy(partition_name, ppartition_name,
sizeof(partition_name));
-   p_number_ptr = of_get_property(rootdn, "ibm,partition-no", NULL);
+   p_number_ptr = of_get_property(of_root, "ibm,partition-no", NULL);
if (p_number_ptr)
partition_number = of_read_number(p_number_ptr, 1);
-   of_node_put(rootdn);
+   of_node_put(of_root);
 }
 
 static void set_adapter_info(struct ibmvscsi_host_data *hostdata)
-- 
2.5.0

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3/6] ibmvscsi: Replace magic values in set_adpater_info() with defines

[Tyrel Datwyler]

Add defines for mad version and mad os_type, and replace the magic
numbers in set_adapter_info() accordingly.

Signed-off-by: Tyrel Datwyler <tyr...@linux.vnet.ibm.com>
---
 drivers/scsi/ibmvscsi/ibmvscsi.c | 4 ++--
 drivers/scsi/ibmvscsi/viosrp.h   | 2 ++
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/ibmvscsi/ibmvscsi.c b/drivers/scsi/ibmvscsi/ibmvscsi.c
index 176260d..7e51615 100644
--- a/drivers/scsi/ibmvscsi/ibmvscsi.c
+++ b/drivers/scsi/ibmvscsi/ibmvscsi.c
@@ -283,8 +283,8 @@ static void set_adapter_info(struct ibmvscsi_host_data 
*hostdata)
hostdata->madapter_info.partition_number =
cpu_to_be32(partition_number);
 
-   hostdata->madapter_info.mad_version = cpu_to_be32(1);
-   hostdata->madapter_info.os_type = cpu_to_be32(2);
+   hostdata->madapter_info.mad_version = cpu_to_be32(SRP_MAD_VERSION_1);
+   hostdata->madapter_info.os_type = cpu_to_be32(SRP_MAD_OS_LINUX);
 }
 
 /**
diff --git a/drivers/scsi/ibmvscsi/viosrp.h b/drivers/scsi/ibmvscsi/viosrp.h
index 17f2de0..da9bb29 100644
--- a/drivers/scsi/ibmvscsi/viosrp.h
+++ b/drivers/scsi/ibmvscsi/viosrp.h
@@ -38,6 +38,8 @@
 #define SRP_VERSION "16.a"
 #define SRP_MAX_IU_LEN 256
 #define SRP_MAX_LOC_LEN 32
+#define SRP_MAD_VERSION_1 1
+#define SRP_MAD_OS_LINUX 2
 
 union srp_iu {
struct srp_login_req login_req;
-- 
2.5.0

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 5/6] ibmvscsi: Remove unsupported host config MAD and sysfs interface

[Tyrel Datwyler]

A VIOSRP_HOST_CONFIG_TYPE management datagram (MAD) has existed in
the code for some time. From what information I've gathered from
Brian King this was likely implemented on the host side in a SLES 9
based VIOS, which is no longer supported anywhere. Further, it is
not defined in PAPR or supported by any AIX based VIOS.

Treating as bit rot and removing the sysfs interface and associated
host config code accordingly.

Signed-off-by: Tyrel Datwyler <tyr...@linux.vnet.ibm.com>
---
 drivers/scsi/ibmvscsi/ibmvscsi.c | 78 
 drivers/scsi/ibmvscsi/viosrp.h   |  7 
 2 files changed, 85 deletions(-)

diff --git a/drivers/scsi/ibmvscsi/ibmvscsi.c b/drivers/scsi/ibmvscsi/ibmvscsi.c
index 47cfe33..a2b8db1 100644
--- a/drivers/scsi/ibmvscsi/ibmvscsi.c
+++ b/drivers/scsi/ibmvscsi/ibmvscsi.c
@@ -1853,62 +1853,6 @@ static void ibmvscsi_handle_crq(struct viosrp_crq *crq,
 }
 
 /**
- * ibmvscsi_get_host_config: Send the command to the server to get host
- * configuration data.  The data is opaque to us.
- */
-static int ibmvscsi_do_host_config(struct ibmvscsi_host_data *hostdata,
-  unsigned char *buffer, int length)
-{
-   struct viosrp_host_config *host_config;
-   struct srp_event_struct *evt_struct;
-   unsigned long flags;
-   dma_addr_t addr;
-   int rc;
-
-   evt_struct = get_event_struct(>pool);
-   if (!evt_struct) {
-   dev_err(hostdata->dev, "couldn't allocate event for 
HOST_CONFIG!\n");
-   return -1;
-   }
-
-   init_event_struct(evt_struct,
- sync_completion,
- VIOSRP_MAD_FORMAT,
- info_timeout);
-
-   host_config = _struct->iu.mad.host_config;
-
-   /* The transport length field is only 16-bit */
-   length = min(0x, length);
-
-   /* Set up a lun reset SRP command */
-   memset(host_config, 0x00, sizeof(*host_config));
-   host_config->common.type = cpu_to_be32(VIOSRP_HOST_CONFIG_TYPE);
-   host_config->common.length = cpu_to_be16(length);
-   addr = dma_map_single(hostdata->dev, buffer, length, DMA_BIDIRECTIONAL);
-
-   if (dma_mapping_error(hostdata->dev, addr)) {
-   if (!firmware_has_feature(FW_FEATURE_CMO))
-   dev_err(hostdata->dev,
-   "dma_mapping error getting host config\n");
-   free_event_struct(>pool, evt_struct);
-   return -1;
-   }
-
-   host_config->buffer = cpu_to_be64(addr);
-
-   init_completion(_struct->comp);
-   spin_lock_irqsave(hostdata->host->host_lock, flags);
-   rc = ibmvscsi_send_srp_event(evt_struct, hostdata, info_timeout * 2);
-   spin_unlock_irqrestore(hostdata->host->host_lock, flags);
-   if (rc == 0)
-   wait_for_completion(_struct->comp);
-   dma_unmap_single(hostdata->dev, addr, length, DMA_BIDIRECTIONAL);
-
-   return rc;
-}
-
-/**
  * ibmvscsi_slave_configure: Set the "allow_restart" flag for each disk.
  * @sdev:  struct scsi_device device to configure
  *
@@ -2090,27 +2034,6 @@ static struct device_attribute ibmvscsi_host_os_type = {
.show = show_host_os_type,
 };
 
-static ssize_t show_host_config(struct device *dev,
-   struct device_attribute *attr, char *buf)
-{
-   struct Scsi_Host *shost = class_to_shost(dev);
-   struct ibmvscsi_host_data *hostdata = shost_priv(shost);
-
-   /* returns null-terminated host config data */
-   if (ibmvscsi_do_host_config(hostdata, buf, PAGE_SIZE) == 0)
-   return strlen(buf);
-   else
-   return 0;
-}
-
-static struct device_attribute ibmvscsi_host_config = {
-   .attr = {
-.name = "config",
-.mode = S_IRUGO,
-},
-   .show = show_host_config,
-};
-
 static struct device_attribute *ibmvscsi_attrs[] = {
_host_vhost_loc,
_host_vhost_name,
@@ -2119,7 +2042,6 @@ static struct device_attribute *ibmvscsi_attrs[] = {
_host_partition_number,
_host_mad_version,
_host_os_type,
-   _host_config,
NULL
 };
 
diff --git a/drivers/scsi/ibmvscsi/viosrp.h b/drivers/scsi/ibmvscsi/viosrp.h
index da9bb29..04bae63 100644
--- a/drivers/scsi/ibmvscsi/viosrp.h
+++ b/drivers/scsi/ibmvscsi/viosrp.h
@@ -96,7 +96,6 @@ enum viosrp_mad_types {
VIOSRP_EMPTY_IU_TYPE = 0x01,
VIOSRP_ERROR_LOG_TYPE = 0x02,
VIOSRP_ADAPTER_INFO_TYPE = 0x03,
-   VIOSRP_HOST_CONFIG_TYPE = 0x04,
VIOSRP_CAPABILITIES_TYPE = 0x05,
VIOSRP_ENABLE_FAST_FAIL = 0x08,
 };
@@ -162,11 +161,6 @@ struct viosrp_adapter_info {
__be64 buffer;
 };
 
-struct viosrp_host_config {
-   struct mad_common common;
-   __be64 buffer;
-};
-
 struct viosrp_fast_fail {
struct 

[PATCH 6/6] ibmvscsi: Add endian conversions to sysfs attribute show functions

[Tyrel Datwyler]

The values returned by the show functions for the host os_type,
mad_version, and partition_number attributes get their values
directly from the madapter_info struct whose associated fields are
__be32 typed. Added endian conversion to ensure these values are
sane on LE platforms.

Signed-off-by: Tyrel Datwyler <tyr...@linux.vnet.ibm.com>
---
 drivers/scsi/ibmvscsi/ibmvscsi.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/ibmvscsi/ibmvscsi.c b/drivers/scsi/ibmvscsi/ibmvscsi.c
index a2b8db1..3621ac5 100644
--- a/drivers/scsi/ibmvscsi/ibmvscsi.c
+++ b/drivers/scsi/ibmvscsi/ibmvscsi.c
@@ -1983,7 +1983,7 @@ static ssize_t show_host_partition_number(struct device 
*dev,
int len;
 
len = snprintf(buf, PAGE_SIZE, "%d\n",
-  hostdata->madapter_info.partition_number);
+  be32_to_cpu(hostdata->madapter_info.partition_number));
return len;
 }
 
@@ -2003,7 +2003,7 @@ static ssize_t show_host_mad_version(struct device *dev,
int len;
 
len = snprintf(buf, PAGE_SIZE, "%d\n",
-  hostdata->madapter_info.mad_version);
+  be32_to_cpu(hostdata->madapter_info.mad_version));
return len;
 }
 
@@ -2022,7 +2022,8 @@ static ssize_t show_host_os_type(struct device *dev,
struct ibmvscsi_host_data *hostdata = shost_priv(shost);
int len;
 
-   len = snprintf(buf, PAGE_SIZE, "%d\n", hostdata->madapter_info.os_type);
+   len = snprintf(buf, PAGE_SIZE, "%d\n",
+  be32_to_cpu(hostdata->madapter_info.os_type));
return len;
 }
 
-- 
2.5.0

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/6] ibmvscsi: Correct values for several viosrp_crq_format enums

[Tyrel Datwyler]

The enum values for VIOSRP_LINUX_FORMAT and VIOSRP_INLINE_FORMAT are
off by one. They are currently defined as 0x06 and 0x07 respetively.
These values are defined in PAPR correctly as 0x05 and 0x06. This
inconsistency has gone unnoticed as neither enum is currently used.
The possible future support of PING messages between the VIOS and
client adapter relies on VIOSRP_INLINE_FORMAT crq messages.
Corrected these enum values to match PAPR definitions.

Signed-off-by: Tyrel Datwyler <tyr...@linux.vnet.ibm.com>
---
 drivers/scsi/ibmvscsi/viosrp.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/ibmvscsi/viosrp.h b/drivers/scsi/ibmvscsi/viosrp.h
index 1162430..d1044e9 100644
--- a/drivers/scsi/ibmvscsi/viosrp.h
+++ b/drivers/scsi/ibmvscsi/viosrp.h
@@ -56,8 +56,8 @@ enum viosrp_crq_formats {
VIOSRP_MAD_FORMAT = 0x02,
VIOSRP_OS400_FORMAT = 0x03,
VIOSRP_AIX_FORMAT = 0x04,
-   VIOSRP_LINUX_FORMAT = 0x06,
-   VIOSRP_INLINE_FORMAT = 0x07
+   VIOSRP_LINUX_FORMAT = 0x05,
+   VIOSRP_INLINE_FORMAT = 0x06
 };
 
 enum viosrp_crq_status {
-- 
2.5.0

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/6] ibmvscsi: Add and use enums for valid CRQ header values

[Tyrel Datwyler]

The PAPR defines four valid header values for the first byte of a
CRQ message. Namely, an unused/empty message (0x00), a valid
command/response entry (0x80), a valid initialization entry (0xC0),
and a transport event (0xFF). Define these values as enums and use
them in the code in place of their magic number equivalents.

Signed-off-by: Tyrel Datwyler <tyr...@linux.vnet.ibm.com>
---
 drivers/scsi/ibmvscsi/ibmvscsi.c | 14 +++---
 drivers/scsi/ibmvscsi/viosrp.h   |  7 +++
 2 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/drivers/scsi/ibmvscsi/ibmvscsi.c b/drivers/scsi/ibmvscsi/ibmvscsi.c
index adfef9d..176260d 100644
--- a/drivers/scsi/ibmvscsi/ibmvscsi.c
+++ b/drivers/scsi/ibmvscsi/ibmvscsi.c
@@ -182,7 +182,7 @@ static struct viosrp_crq *crq_queue_next_crq(struct 
crq_queue *queue)
 
spin_lock_irqsave(>lock, flags);
crq = >msgs[queue->cur];
-   if (crq->valid & 0x80) {
+   if (crq->valid & VIOSRP_CRQ_VALID) {
if (++queue->cur == queue->size)
queue->cur = 0;
 
@@ -231,7 +231,7 @@ static void ibmvscsi_task(void *data)
/* Pull all the valid messages off the CRQ */
while ((crq = crq_queue_next_crq(>queue)) != NULL) {
ibmvscsi_handle_crq(crq, hostdata);
-   crq->valid = 0x00;
+   crq->valid = VIOSRP_CRQ_FREE;
}
 
vio_enable_interrupts(vdev);
@@ -239,7 +239,7 @@ static void ibmvscsi_task(void *data)
if (crq != NULL) {
vio_disable_interrupts(vdev);
ibmvscsi_handle_crq(crq, hostdata);
-   crq->valid = 0x00;
+   crq->valid = VIOSRP_CRQ_FREE;
} else {
done = 1;
}
@@ -474,7 +474,7 @@ static int initialize_event_pool(struct event_pool *pool,
struct srp_event_struct *evt = >events[i];
memset(>crq, 0x00, sizeof(evt->crq));
atomic_set(>free, 1);
-   evt->crq.valid = 0x80;
+   evt->crq.valid = VIOSRP_CRQ_VALID;
evt->crq.IU_length = cpu_to_be16(sizeof(*evt->xfer_iu));
evt->crq.IU_data_ptr = cpu_to_be64(pool->iu_token +
sizeof(*evt->xfer_iu) * i);
@@ -1767,7 +1767,7 @@ static void ibmvscsi_handle_crq(struct viosrp_crq *crq,
struct srp_event_struct *evt_struct =
(__force struct srp_event_struct *)crq->IU_data_ptr;
switch (crq->valid) {
-   case 0xC0:  /* initialization */
+   case VIOSRP_CRQ_INIT:   /* initialization */
switch (crq->format) {
case 0x01:  /* Initialization message */
dev_info(hostdata->dev, "partner initialized\n");
@@ -1791,7 +1791,7 @@ static void ibmvscsi_handle_crq(struct viosrp_crq *crq,
dev_err(hostdata->dev, "unknown crq message type: 
%d\n", crq->format);
}
return;
-   case 0xFF:  /* Hypervisor telling us the connection is closed */
+   case VIOSRP_CRQ_TRANSPORT:  /* Hypervisor telling us the connection 
is closed */
scsi_block_requests(hostdata->host);
atomic_set(>request_limit, 0);
if (crq->format == 0x06) {
@@ -1807,7 +1807,7 @@ static void ibmvscsi_handle_crq(struct viosrp_crq *crq,
ibmvscsi_reset_host(hostdata);
}
return;
-   case 0x80:  /* real payload */
+   case VIOSRP_CRQ_VALID:  /* real payload */
break;
default:
dev_err(hostdata->dev, "got an invalid message type 0x%02x\n",
diff --git a/drivers/scsi/ibmvscsi/viosrp.h b/drivers/scsi/ibmvscsi/viosrp.h
index d1044e9..17f2de0 100644
--- a/drivers/scsi/ibmvscsi/viosrp.h
+++ b/drivers/scsi/ibmvscsi/viosrp.h
@@ -51,6 +51,13 @@ union srp_iu {
u8 reserved[SRP_MAX_IU_LEN];
 };
 
+enum viosrp_crq_headers {
+   VIOSRP_CRQ_FREE = 0x00,
+   VIOSRP_CRQ_VALID = 0x80,
+   VIOSRP_CRQ_INIT = 0xC0,
+   VIOSRP_CRQ_TRANSPORT = 0xFF
+};
+
 enum viosrp_crq_formats {
VIOSRP_SRP_FORMAT = 0x01,
VIOSRP_MAD_FORMAT = 0x02,
-- 
2.5.0

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 0/6] ibmvscsi: code cleanup

[Tyrel Datwyler]

Fixed up a couple spots that were out of line with the PAPR in regards
to its defined VSCSI protocol. Did away with some magic numbers directly
in the code. Fixed a minor endian issue.

Tyrel Datwyler (6):
  ibmvscsi: Correct values for several viosrp_crq_format enums
  ibmvscsi: Add and use enums for valid CRQ header values
  ibmvscsi: Replace magic values in set_adpater_info() with defines
  ibmvscsi: Use of_root to access OF device tree root node
  ibmvscsi: Remove unsupported host config MAD and sysfs interface
  ibmvscsi: Add endian conversions to sysfs attribute show functions

 drivers/scsi/ibmvscsi/ibmvscsi.c | 117 +++
 drivers/scsi/ibmvscsi/viosrp.h   |  20 ---
 2 files changed, 30 insertions(+), 107 deletions(-)

-- 
2.5.0

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] ibmvsci: make parameters max_id and max_channel read-only

[Tyrel Datwyler]

On 09/10/2015 02:23 AM, Laurent Vivier wrote:
> The value of the parameter is never re-read by the driver,
> so a new value is ignored. Let know the user he
> can't modify it by removing writable attribute.
> 
> Signed-off-by: Laurent Vivier <lviv...@redhat.com>

Acked-by: Tyrel Datwyler <tyr...@linux.vnet.ibm.com>

> ---
> I resend this patch as James was not cc'ed.
> 
>  drivers/scsi/ibmvscsi/ibmvscsi.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/scsi/ibmvscsi/ibmvscsi.c 
> b/drivers/scsi/ibmvscsi/ibmvscsi.c
> index 6a41c36..3e76490 100644
> --- a/drivers/scsi/ibmvscsi/ibmvscsi.c
> +++ b/drivers/scsi/ibmvscsi/ibmvscsi.c
> @@ -105,9 +105,9 @@ MODULE_AUTHOR("Dave Boutcher");
>  MODULE_LICENSE("GPL");
>  MODULE_VERSION(IBMVSCSI_VERSION);
> 
> -module_param_named(max_id, max_id, int, S_IRUGO | S_IWUSR);
> +module_param_named(max_id, max_id, int, S_IRUGO);
>  MODULE_PARM_DESC(max_id, "Largest ID value for each channel");
> -module_param_named(max_channel, max_channel, int, S_IRUGO | S_IWUSR);
> +module_param_named(max_channel, max_channel, int, S_IRUGO);
>  MODULE_PARM_DESC(max_channel, "Largest channel value");
>  module_param_named(init_timeout, init_timeout, int, S_IRUGO | S_IWUSR);
>  MODULE_PARM_DESC(init_timeout, "Initialization timeout in seconds");
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/3] ibmvsci: Allow to configure maximum LUN

[Tyrel Datwyler]

On 09/10/2015 02:23 AM, Laurent Vivier wrote:
> QEMU allows until 32 LUNs.
> 
> Signed-off-by: Laurent Vivier <lviv...@redhat.com>

Acked-by: Tyrel Datwyler <tyr...@linux.vnet.ibm.com>

> ---
>  drivers/scsi/ibmvscsi/ibmvscsi.c | 5 -
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/scsi/ibmvscsi/ibmvscsi.c 
> b/drivers/scsi/ibmvscsi/ibmvscsi.c
> index f9d7ec4..e5478b0 100644
> --- a/drivers/scsi/ibmvscsi/ibmvscsi.c
> +++ b/drivers/scsi/ibmvscsi/ibmvscsi.c
> @@ -84,6 +84,7 @@
>   */
>  static int max_id = 64;
>  static int max_channel = 3;
> +static int max_lun = 8;
>  static int init_timeout = 300;
>  static int login_timeout = 60;
>  static int info_timeout = 30;
> @@ -117,6 +118,8 @@ module_param_named(fast_fail, fast_fail, int, S_IRUGO | 
> S_IWUSR);
>  MODULE_PARM_DESC(fast_fail, "Enable fast fail. [Default=1]");
>  module_param_named(client_reserve, client_reserve, int, S_IRUGO );
>  MODULE_PARM_DESC(client_reserve, "Attempt client managed reserve/release");
> +module_param(max_lun, int, S_IRUGO);
> +MODULE_PARM_DESC(max_lun, "Maximum LUN value [Default=8]");
> 
>  static void ibmvscsi_handle_crq(struct viosrp_crq *crq,
>   struct ibmvscsi_host_data *hostdata);
> @@ -2289,7 +2292,7 @@ static int ibmvscsi_probe(struct vio_dev *vdev, const 
> struct vio_device_id *id)
>   goto init_pool_failed;
>   }
> 
> - host->max_lun = 8;
> + host->max_lun = max_lun;
>   host->max_id = max_id;
>   host->max_channel = max_channel;
>   host->max_cmd_len = 16;
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] ibmvcsci: display default value for max_id, max_lun and max_channel.

[Tyrel Datwyler]

On 09/10/2015 02:23 AM, Laurent Vivier wrote:
> As devices with values greater than that are silently ignored,
> this gives some hints to the sys admin to know why he doesn't see
> his devices...
> 
> Signed-off-by: Laurent Vivier <lviv...@redhat.com>

Acked-by: Tyrel Datwyler <tyr...@linux.vnet.ibm.com>

> ---
>  drivers/scsi/ibmvscsi/ibmvscsi.c | 8 ++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/scsi/ibmvscsi/ibmvscsi.c 
> b/drivers/scsi/ibmvscsi/ibmvscsi.c
> index 3e76490..f9d7ec4 100644
> --- a/drivers/scsi/ibmvscsi/ibmvscsi.c
> +++ b/drivers/scsi/ibmvscsi/ibmvscsi.c
> @@ -106,9 +106,9 @@ MODULE_LICENSE("GPL");
>  MODULE_VERSION(IBMVSCSI_VERSION);
> 
>  module_param_named(max_id, max_id, int, S_IRUGO);
> -MODULE_PARM_DESC(max_id, "Largest ID value for each channel");
> +MODULE_PARM_DESC(max_id, "Largest ID value for each channel [Default=64]");
>  module_param_named(max_channel, max_channel, int, S_IRUGO);
> -MODULE_PARM_DESC(max_channel, "Largest channel value");
> +MODULE_PARM_DESC(max_channel, "Largest channel value [Default=3]");
>  module_param_named(init_timeout, init_timeout, int, S_IRUGO | S_IWUSR);
>  MODULE_PARM_DESC(init_timeout, "Initialization timeout in seconds");
>  module_param_named(max_requests, max_requests, int, S_IRUGO);
> @@ -2294,6 +2294,10 @@ static int ibmvscsi_probe(struct vio_dev *vdev, const 
> struct vio_device_id *id)
>   host->max_channel = max_channel;
>   host->max_cmd_len = 16;
> 
> + dev_info(dev,
> +  "Maximum ID: %d Maximum LUN: %d Maximum Channel: %d\n",
> +  host->max_id, host->max_lun, host->max_channel);
> +
>   if (scsi_add_host(hostdata->host, hostdata->dev))
>   goto add_host_failed;
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ibmvscsi:Make the variable rc to be a integer in the function ibmvfc_handle_crq

[Tyrel Datwyler]

On 07/09/2015 01:10 PM, Nicholas Krause wrote:
 This makes the variable rc to be a integer in the function
 ibmvfc_handle_crq due to this function's value never being
 greater then a standard C sized integer. In addition due to
 this make the printk statement that prints this variable out
 if the call to ibmvfc_sen_crq_init_complete fails to use the
 string format %d rather then %ld in other to avoid build
 warnings due to the variable rc having a change in type from
 long to int now.
 
 Signed-off-by: Nicholas Krause xerofo...@gmail.com

Again, the wording is a little difficult and much too verbose for such a
trivial change. You really don't need to explain the change of printk
conversion specifier as it is obvious and implied by the patch itself.
Something like the following would be better:

ibmvscsi: change type of rc to int in ibmvfc_handle_crq

Change type of rc variable in ibmvfc_handle_crq() to match return type
of ibmvfc_send_crq_init_complete().

Otherwise,

Acked-by: Tyrel Datwyler tyr...@linux.vnet.ibm.com


 ---
  drivers/scsi/ibmvscsi/ibmvfc.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)
 
 diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c
 index 057d277..a33e2af 100644
 --- a/drivers/scsi/ibmvscsi/ibmvfc.c
 +++ b/drivers/scsi/ibmvscsi/ibmvfc.c
 @@ -2720,7 +2720,7 @@ static void ibmvfc_handle_async(struct ibmvfc_async_crq 
 *crq,
   **/
  static void ibmvfc_handle_crq(struct ibmvfc_crq *crq, struct ibmvfc_host 
 *vhost)
  {
 - long rc;
 + int rc;
   struct ibmvfc_event *evt = (struct ibmvfc_event 
 *)be64_to_cpu(crq-ioba);
 
   switch (crq-valid) {
 @@ -2733,7 +2733,7 @@ static void ibmvfc_handle_crq(struct ibmvfc_crq *crq, 
 struct ibmvfc_host *vhost)
   if (rc == 0)
   ibmvfc_init_host(vhost);
   else
 - dev_err(vhost-dev, Unable to send init rsp. 
 rc=%ld\n, rc);
 + dev_err(vhost-dev, Unable to send init rsp. 
 rc=%d\n, rc);
   break;
   case IBMVFC_CRQ_INIT_COMPLETE:
   dev_info(vhost-dev, Partner initialization 
 complete\n);
 

--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ibmvscsi:Remove no longer required comment for the function send_mad_adapter_info

[Tyrel Datwyler]

On 07/09/2015 10:24 AM, Nicholas Krause wrote:
 This removes the no longer required comment for the function
 send_mad_adapter_info stating that it always return zero due
 to this function being declared as void and thus never returning
 any useful value.
 
 Signed-off-by: Nicholas Krause xerofo...@gmail.com

Acked-by: Tyrel Datwyler tyr...@linux.vnet.ibm.com

 ---
  drivers/scsi/ibmvscsi/ibmvscsi.c | 1 -
  1 file changed, 1 deletion(-)
 
 diff --git a/drivers/scsi/ibmvscsi/ibmvscsi.c 
 b/drivers/scsi/ibmvscsi/ibmvscsi.c
 index 6a41c36..70ea976 100644
 --- a/drivers/scsi/ibmvscsi/ibmvscsi.c
 +++ b/drivers/scsi/ibmvscsi/ibmvscsi.c
 @@ -1423,7 +1423,6 @@ static void adapter_info_rsp(struct srp_event_struct 
 *evt_struct)
   *  returned SRP version doesn't match ours.
   * @hostdata:ibmvscsi_host_data of host
   * 
 - * Returns zero if successful.
  */
  static void send_mad_adapter_info(struct ibmvscsi_host_data *hostdata)
  {
 

--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ibmvscsi:Remove no longer required comments about return values in the file ibmvfc.c

[Tyrel Datwyler]

On 07/09/2015 10:41 AM, Nicholas Krause wrote:
 This removes the no longer require comments about the return values
 for the functions ibmvfc_init_host and  ibmvfc_reinit_host due to
 these functions being declared to have a return type of void thus
 making this comments invalid.
 
 Signed-off-by: Nicholas Krause xerofo...@gmail.com

Some grammar nit picking. Aside from a couple spelling errors the
wording is a little difficult. Refer to Documentation/SubmittingPatches
section 2. In particular it suggests using the imperative mood.
Something like this would work better:

Remove comments about return values from ibmvfc_init_host() and
ibmvfc_reinit_host() as they are both declared to have return type void.

Otherwise,

Acked-by: Tyrel Datwyler tyr...@linux.vnet.ibm.com

 ---
  drivers/scsi/ibmvscsi/ibmvfc.c | 4 
  1 file changed, 4 deletions(-)
 
 diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c
 index 057d277..c6582db 100644
 --- a/drivers/scsi/ibmvscsi/ibmvfc.c
 +++ b/drivers/scsi/ibmvscsi/ibmvfc.c
 @@ -528,8 +528,6 @@ static void ibmvfc_set_host_action(struct ibmvfc_host 
 *vhost,
   * ibmvfc_reinit_host - Re-start host initialization (no NPIV Login)
   * @vhost:   ibmvfc host struct
   *
 - * Return value:
 - *   nothing
   **/
  static void ibmvfc_reinit_host(struct ibmvfc_host *vhost)
  {
 @@ -570,8 +568,6 @@ static void ibmvfc_link_down(struct ibmvfc_host *vhost,
   * ibmvfc_init_host - Start host initialization
   * @vhost:   ibmvfc host struct
   *
 - * Return value:
 - *   nothing
   **/
  static void ibmvfc_init_host(struct ibmvfc_host *vhost)
  {
 

--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 08/17] ibmvscsi: Fix bidi command test

[Tyrel Datwyler]

On 01/23/2015 04:10 AM, Bart Van Assche wrote:
 Signed-off-by: Bart Van Assche bart.vanass...@sandisk.com
 Cc: Brian King brk...@linux.vnet.ibm.com
 Cc: Nathan Fontenot nf...@linux.vnet.ibm.com
 ---
  drivers/scsi/ibmvscsi/ibmvscsi.c | 10 ++
  1 file changed, 6 insertions(+), 4 deletions(-)
 
 diff --git a/drivers/scsi/ibmvscsi/ibmvscsi.c 
 b/drivers/scsi/ibmvscsi/ibmvscsi.c
 index acea5d6..cf26b33 100644
 --- a/drivers/scsi/ibmvscsi/ibmvscsi.c
 +++ b/drivers/scsi/ibmvscsi/ibmvscsi.c
 @@ -765,16 +765,18 @@ static int map_data_for_srp_cmd(struct scsi_cmnd *cmd,
   struct srp_event_struct *evt_struct,
   struct srp_cmd *srp_cmd, struct device *dev)
  {
 + if (scsi_bidi_cmnd(cmd)) {
 + sdev_printk(KERN_ERR, cmd-device,
 + Bidirectional commands are not yet supported\n);
 + return 0;
 + }
 +

Is there a particular problem this solves, or is this simply a change to
use the bidi API in place of checking sc_data_direction for
DMA_BIDIRECTIONAL?

-Tyrel

   switch (cmd-sc_data_direction) {
   case DMA_FROM_DEVICE:
   case DMA_TO_DEVICE:
   break;
   case DMA_NONE:
   return 1;
 - case DMA_BIDIRECTIONAL:
 - sdev_printk(KERN_ERR, cmd-device,
 - Can't map DMA_BIDIRECTIONAL to read/write\n);
 - return 0;
   default:
   sdev_printk(KERN_ERR, cmd-device,
   Unknown data direction 0x%02x; can't map!\n,
 

--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/2] MAINTAINERS: ibmvfc driver maintainer change

[Tyrel Datwyler]

Change maintainer of ibmvfc driver to Tyrel Datwyler.

Signed-off-by: Tyrel Datwyler tyr...@linux.vnet.ibm.com
Cc: Nathan Fontenot nf...@linux.vnet.ibm.com
Cc: Brian King brk...@linux.vnet.ibm.com
---
 MAINTAINERS | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index a646b94..f0d7593 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4761,7 +4761,7 @@ F:drivers/scsi/ibmvscsi/ibmvscsi*
 F: drivers/scsi/ibmvscsi/viosrp.h
 
 IBM Power Virtual FC Device Drivers
-M: Brian King brk...@linux.vnet.ibm.com
+M: Tyrel Datwyler tyr...@linux.vnet.ibm.com
 L: linux-scsi@vger.kernel.org
 S: Supported
 F: drivers/scsi/ibmvscsi/ibmvfc*
-- 
1.7.12.2

--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/2] MAINTAINERS: ibmvscsi driver maintainer change

[Tyrel Datwyler]

Change maintainer of ibmvscsi driver to Tyrel Datwyler.

Signed-off-by: Tyrel Datwyler tyr...@linux.vnet.ibm.com
Cc: Nathan Fontenot nf...@linux.vnet.ibm.com
Cc: Brian King brk...@linux.vnet.ibm.com
---
 MAINTAINERS | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 79b2e4b..a646b94 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4754,7 +4754,7 @@ S:Supported
 F: drivers/net/ethernet/ibm/ibmveth.*
 
 IBM Power Virtual SCSI Device Drivers
-M: Nathan Fontenot nf...@linux.vnet.ibm.com
+M: Tyrel Datwyler tyr...@linux.vnet.ibm.com
 L: linux-scsi@vger.kernel.org
 S: Supported
 F: drivers/scsi/ibmvscsi/ibmvscsi*
-- 
1.7.12.2

--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] ibmvfc: fix little endian issues

[Tyrel Datwyler]

Added big endian annotations to relevant data structure fields, and necessary
byte swappings to support little endian builds.

Signed-off-by: Brian King brk...@linux.vnet.ibm.com
Signed-off-by: Tyrel Datwyler tyr...@linux.vnet.ibm.com
---
 drivers/scsi/ibmvscsi/ibmvfc.c | 473 +
 drivers/scsi/ibmvscsi/ibmvfc.h | 268 +++
 2 files changed, 374 insertions(+), 367 deletions(-)

diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c
index 8dd4768..aaee066 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc.c
+++ b/drivers/scsi/ibmvscsi/ibmvfc.c
@@ -166,13 +166,13 @@ static void ibmvfc_trc_start(struct ibmvfc_event *evt)
switch (entry-fmt) {
case IBMVFC_CMD_FORMAT:
entry-op_code = vfc_cmd-iu.cdb[0];
-   entry-scsi_id = vfc_cmd-tgt_scsi_id;
+   entry-scsi_id = be64_to_cpu(vfc_cmd-tgt_scsi_id);
entry-lun = scsilun_to_int(vfc_cmd-iu.lun);
entry-tmf_flags = vfc_cmd-iu.tmf_flags;
-   entry-u.start.xfer_len = vfc_cmd-iu.xfer_len;
+   entry-u.start.xfer_len = be32_to_cpu(vfc_cmd-iu.xfer_len);
break;
case IBMVFC_MAD_FORMAT:
-   entry-op_code = mad-opcode;
+   entry-op_code = be32_to_cpu(mad-opcode);
break;
default:
break;
@@ -199,18 +199,18 @@ static void ibmvfc_trc_end(struct ibmvfc_event *evt)
switch (entry-fmt) {
case IBMVFC_CMD_FORMAT:
entry-op_code = vfc_cmd-iu.cdb[0];
-   entry-scsi_id = vfc_cmd-tgt_scsi_id;
+   entry-scsi_id = be64_to_cpu(vfc_cmd-tgt_scsi_id);
entry-lun = scsilun_to_int(vfc_cmd-iu.lun);
entry-tmf_flags = vfc_cmd-iu.tmf_flags;
-   entry-u.end.status = vfc_cmd-status;
-   entry-u.end.error = vfc_cmd-error;
+   entry-u.end.status = be16_to_cpu(vfc_cmd-status);
+   entry-u.end.error = be16_to_cpu(vfc_cmd-error);
entry-u.end.fcp_rsp_flags = vfc_cmd-rsp.flags;
entry-u.end.rsp_code = vfc_cmd-rsp.data.info.rsp_code;
entry-u.end.scsi_status = vfc_cmd-rsp.scsi_status;
break;
case IBMVFC_MAD_FORMAT:
-   entry-op_code = mad-opcode;
-   entry-u.end.status = mad-status;
+   entry-op_code = be32_to_cpu(mad-opcode);
+   entry-u.end.status = be16_to_cpu(mad-status);
break;
default:
break;
@@ -270,14 +270,14 @@ static int ibmvfc_get_err_result(struct ibmvfc_cmd 
*vfc_cmd)
 {
int err;
struct ibmvfc_fcp_rsp *rsp = vfc_cmd-rsp;
-   int fc_rsp_len = rsp-fcp_rsp_len;
+   int fc_rsp_len = be32_to_cpu(rsp-fcp_rsp_len);
 
if ((rsp-flags  FCP_RSP_LEN_VALID) 
((fc_rsp_len  fc_rsp_len != 4  fc_rsp_len != 8) ||
 rsp-data.info.rsp_code))
return DID_ERROR  16;
 
-   err = ibmvfc_get_err_index(vfc_cmd-status, vfc_cmd-error);
+   err = ibmvfc_get_err_index(be16_to_cpu(vfc_cmd-status), 
be16_to_cpu(vfc_cmd-error));
if (err = 0)
return rsp-scsi_status | (cmd_status[err].result  16);
return rsp-scsi_status | (DID_ERROR  16);
@@ -807,7 +807,7 @@ static void ibmvfc_fail_request(struct ibmvfc_event *evt, 
int error_code)
evt-cmnd-result = (error_code  16);
evt-done = ibmvfc_scsi_eh_done;
} else
-   evt-xfer_iu-mad_common.status = IBMVFC_MAD_DRIVER_FAILED;
+   evt-xfer_iu-mad_common.status = 
cpu_to_be16(IBMVFC_MAD_DRIVER_FAILED);
 
list_del(evt-queue);
del_timer(evt-timer);
@@ -955,7 +955,7 @@ static void ibmvfc_get_host_speed(struct Scsi_Host *shost)
 
spin_lock_irqsave(shost-host_lock, flags);
if (vhost-state == IBMVFC_ACTIVE) {
-   switch (vhost-login_buf-resp.link_speed / 100) {
+   switch (be64_to_cpu(vhost-login_buf-resp.link_speed) / 100) {
case 1:
fc_host_speed(shost) = FC_PORTSPEED_1GBIT;
break;
@@ -976,7 +976,7 @@ static void ibmvfc_get_host_speed(struct Scsi_Host *shost)
break;
default:
ibmvfc_log(vhost, 3, Unknown port speed: %lld Gbit\n,
-  vhost-login_buf-resp.link_speed / 100);
+  
be64_to_cpu(vhost-login_buf-resp.link_speed) / 100);
fc_host_speed(shost) = FC_PORTSPEED_UNKNOWN;
break;
}
@@ -1171,21 +1171,21 @@ static void ibmvfc_set_login_info(struct ibmvfc_host 
*vhost)
 
memset(login_info, 0, sizeof(*login_info));
 
-   login_info-ostype = IBMVFC_OS_LINUX;
-   login_info-max_dma_len = IBMVFC_MAX_SECTORS  9;
-   login_info-max_payload = sizeof(struct