[PATCH 1/3] ibmvfc: byte swap login_buf.resp values in attribute show functions

Both ibmvfc_show_host_(capabilities|npiv_version) functions retrieve
values from vhost->login_buf.resp buffer. This is the MAD response
buffer from the VIOS and as such any multi-byte non-string values are in
big endian format.

Byte swap these values to host cpu endian format for better human
readability.

Signed-off-by: Tyrel Datwyler <tyreld@linux.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 070cf516b98f..01fe65de9086 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc.c
+++ b/drivers/scsi/ibmvscsi/ibmvfc.c

@@ -3025,7 +3025,7 @@ static ssize_t ibmvfc_show_host_npiv_version(struct device *dev,
 {
 	struct Scsi_Host *shost = class_to_shost(dev);
 	struct ibmvfc_host *vhost = shost_priv(shost);
-	return snprintf(buf, PAGE_SIZE, "%d\n", vhost->login_buf->resp.version);
+	return snprintf(buf, PAGE_SIZE, "%d\n", be32_to_cpu(vhost->login_buf->resp.version));
 }
 
 static ssize_t ibmvfc_show_host_capabilities(struct device *dev,

@@ -3033,7 +3033,7 @@ static ssize_t ibmvfc_show_host_capabilities(struct device *dev,
 {
 	struct Scsi_Host *shost = class_to_shost(dev);
 	struct ibmvfc_host *vhost = shost_priv(shost);
-	return snprintf(buf, PAGE_SIZE, "%llx\n", vhost->login_buf->resp.capabilities);
+	return snprintf(buf, PAGE_SIZE, "%llx\n", be64_to_cpu(vhost->login_buf->resp.capabilities));
 }
 
 /**

-- 
2.27.0

Remove a superfluous semicolon following a closing function block
bracket.

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

diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c
index 01fe65de9086..0cab4b852b48 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc.c
+++ b/drivers/scsi/ibmvscsi/ibmvfc.c

@@ -4391,7 +4391,7 @@ static void ibmvfc_npiv_login(struct ibmvfc_host *vhost)
 		ibmvfc_dbg(vhost, "Sent NPIV login\n");
 	else
 		ibmvfc_link_down(vhost, IBMVFC_LINK_DEAD);
-};
+}
 
 /**
  * ibmvfc_npiv_logout_done - Completion handler for NPIV Logout

-- 
2.27.0

The vfcFrame correlation field is 64bit handle that is intended to trace
I/O operations through both the client stack and VIOS stack when the
underlying physical FC adapter supports tagging.

Tag vfcFrames with the associated ibmvfc_event pointer handle.

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

diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c
index 0cab4b852b48..3922441a117d 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc.c
+++ b/drivers/scsi/ibmvscsi/ibmvfc.c

@@ -1693,6 +1693,8 @@ static int ibmvfc_queuecommand_lck(struct scsi_cmnd *cmnd,
 		vfc_cmd->iu.pri_task_attr = IBMVFC_SIMPLE_TASK;
 	}
 
+	vfc_cmd->correlation = cpu_to_be64(evt);
+
 	if (likely(!(rc = ibmvfc_map_sg_data(cmnd, evt, vfc_cmd, vhost->dev))))
 		return ibmvfc_send_event(evt, vhost, 0);
 

@@ -2370,6 +2372,8 @@ static int ibmvfc_abort_task_set(struct scsi_device *sdev)
 		tmf->iu.tmf_flags = IBMVFC_ABORT_TASK_SET;
 		evt->sync_iu = &rsp_iu;
 
+		tmf->correlation = cpu_to_be64(evt);
+
 		init_completion(&evt->comp);
 		rsp_rc = ibmvfc_send_event(evt, vhost, default_timeout);
 	}

-- 
2.27.0

On Tue, 17 Nov 2020 12:50:29 -0600, Tyrel Datwyler wrote:

Both ibmvfc_show_host_(capabilities|npiv_version) functions retrieve
values from vhost->login_buf.resp buffer. This is the MAD response
buffer from the VIOS and as such any multi-byte non-string values are in
big endian format.

Byte swap these values to host cpu endian format for better human
readability.

Applied to 5.11/scsi-queue, thanks!

[1/3] scsi: ibmvfc: Byte swap login_buf.resp values in attribute show functions
      https://git.kernel.org/mkp/scsi/c/61bdb4eec8d1
[2/3] scsi: ibmvfc: Remove trailing semicolon
      https://git.kernel.org/mkp/scsi/c/4e0716199ab6
[3/3] scsi: ibmvfc: Use correlation token to tag commands
      https://git.kernel.org/mkp/scsi/c/2aa0102c6688

-- 
Martin K. Petersen	Oracle Linux Engineering

On Tue, Nov 17, 2020 at 12:50:31PM -0600, Tyrel Datwyler wrote:

This patch introduces a clang warning, is this intentional behavior?

$ make -skj"$(nproc)" ARCH=powerpc CROSS_COMPILE=powerpc64le-linux-gnu- LLVM=1 O=out distclean ppc64le_defconfig drivers/scsi/ibmvscsi/ibmvfc.o
Using ../arch/powerpc/configs/ppc64_defconfig as base
Merging ../arch/powerpc/configs/le.config
#
# merged configuration written to .config (needs make)
#
../drivers/scsi/ibmvscsi/ibmvfc.c:1747:25: warning: incompatible pointer to integer conversion passing 'struct ibmvfc_event *' to parameter of type '__u64' (aka 'unsigned long long') [-Wint-conversion]
        vfc_cmd->correlation = cpu_to_be64(evt);
                               ^~~~~~~~~~~~~~~~
../include/linux/byteorder/generic.h:92:21: note: expanded from macro 'cpu_to_be64'
#define cpu_to_be64 __cpu_to_be64
                    ^
../include/uapi/linux/byteorder/little_endian.h:37:52: note: expanded from macro '__cpu_to_be64'
#define __cpu_to_be64(x) ((__force __be64)__swab64((x)))
                                          ~~~~~~~~~^~~~
../include/uapi/linux/swab.h:133:12: note: expanded from macro '__swab64'
        __fswab64(x))
                  ^
../include/uapi/linux/swab.h:66:57: note: passing argument to parameter 'val' here
static inline __attribute_const__ __u64 __fswab64(__u64 val)
                                                        ^
../drivers/scsi/ibmvscsi/ibmvfc.c:2421:22: warning: incompatible pointer to integer conversion passing 'struct ibmvfc_event *' to parameter of type '__u64' (aka 'unsigned long long') [-Wint-conversion]
                tmf->correlation = cpu_to_be64(evt);
                                   ^~~~~~~~~~~~~~~~
../include/linux/byteorder/generic.h:92:21: note: expanded from macro 'cpu_to_be64'
#define cpu_to_be64 __cpu_to_be64
                    ^
../include/uapi/linux/byteorder/little_endian.h:37:52: note: expanded from macro '__cpu_to_be64'
#define __cpu_to_be64(x) ((__force __be64)__swab64((x)))
                                          ~~~~~~~~~^~~~
../include/uapi/linux/swab.h:133:12: note: expanded from macro '__swab64'
        __fswab64(x))
                  ^
../include/uapi/linux/swab.h:66:57: note: passing argument to parameter 'val' here
static inline __attribute_const__ __u64 __fswab64(__u64 val)
                                                        ^
2 warnings generated.

Cheers,
Nathan

On 12/21/20 10:24 PM, Nathan Chancellor wrote:

On Tue, Nov 17, 2020 at 12:50:31PM -0600, Tyrel Datwyler wrote:

quoted

The vfcFrame correlation field is 64bit handle that is intended to trace
I/O operations through both the client stack and VIOS stack when the
underlying physical FC adapter supports tagging.

Tag vfcFrames with the associated ibmvfc_event pointer handle.

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

diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c
index 0cab4b852b48..3922441a117d 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc.c
+++ b/drivers/scsi/ibmvscsi/ibmvfc.c

@@ -1693,6 +1693,8 @@ static int ibmvfc_queuecommand_lck(struct scsi_cmnd *cmnd,
 		vfc_cmd->iu.pri_task_attr = IBMVFC_SIMPLE_TASK;
 	}
 
+	vfc_cmd->correlation = cpu_to_be64(evt);
+
 	if (likely(!(rc = ibmvfc_map_sg_data(cmnd, evt, vfc_cmd, vhost->dev))))
 		return ibmvfc_send_event(evt, vhost, 0);
 

@@ -2370,6 +2372,8 @@ static int ibmvfc_abort_task_set(struct scsi_device *sdev)
 		tmf->iu.tmf_flags = IBMVFC_ABORT_TASK_SET;
 		evt->sync_iu = &rsp_iu;
 
+		tmf->correlation = cpu_to_be64(evt);
+
 		init_completion(&evt->comp);
 		rsp_rc = ibmvfc_send_event(evt, vhost, default_timeout);
 	}

-- 
2.27.0

This patch introduces a clang warning, is this intentional behavior?

Nope, I just missed the required cast. I've got a fixes patch queued up. I just
haven't sent it yet.

-Tyrel