[PATCH 0/6] scsi: storvsc: Some miscellaneous cleanup

[PATCH 0/6] scsi: storvsc: Some miscellaneous cleanup

[K. Y. Srinivasan]

Cleanup version handling as well as base feature detection on storage
version as opposed to host version.

keith.mange@microsoft.com (6):
  scsi: storvsc: Rather than look for sets of specific protocol
    versions, make decisions based on ranges.
  scsi: storvsc: Use a single value to track protocol versions
  hv:scsi:Untangle the storage protocol negotiation from the vmbus
    protocol negotiation.
  scsi: storvsc: use correct defaults for values determined by protocol
    negotiation
  scsi: storvsc: use storage protocol version to determine storage
    capabilities
  scsi: storvsc: Allow write_same when host is windows 10

 drivers/scsi/storvsc_drv.c |  194 ++++++++++++++++++++++++++++----------------
 1 files changed, 123 insertions(+), 71 deletions(-)

-- 
1.7.4.1

[PATCH 1/6] scsi: storvsc: Rather than look for sets of specific protocol versions, make decisions based on ranges.

[K. Y. Srinivasan]

From: keith.mange@microsoft.com <keith.mange@microsoft.com>

Rather than look for sets of specific protocol versions,
make decisions based on ranges. This will be safer and require fewer changes
going forward as we add more storage protocol versions.

Tested-by: Alex Ng <alexng@microsoft.com>
Signed-off-by: Keith Mange <keith.mange@microsoft.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
 drivers/scsi/storvsc_drv.c |   11 +++--------
 1 files changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
index 3c6584f..582f3b5 100644
--- a/drivers/scsi/storvsc_drv.c
+++ b/drivers/scsi/storvsc_drv.c
@@ -988,8 +988,7 @@ static int storvsc_channel_init(struct hv_device *device)
 	 * support multi-channel.
 	 */
 	max_chns = vstor_packet->storage_channel_properties.max_channel_cnt;
-	if ((vmbus_proto_version != VERSION_WIN7) &&
-	   (vmbus_proto_version != VERSION_WS2008))  {
+	if (vmbus_proto_version >= VERSION_WIN8) {
 		if (vstor_packet->storage_channel_properties.flags &
 		    STORAGE_CHANNEL_SUPPORTS_MULTI_CHANNEL)
 			process_sub_channels = true;
@@ -1758,9 +1757,7 @@ static int storvsc_probe(struct hv_device *device,
 	 * set state to properly communicate with the host.
 	 */
 
-	switch (vmbus_proto_version) {
-	case VERSION_WS2008:
-	case VERSION_WIN7:
+	if (vmbus_proto_version < VERSION_WIN8) {
 		sense_buffer_size = PRE_WIN8_STORVSC_SENSE_BUFFER_SIZE;
 		vmscsi_size_delta = sizeof(struct vmscsi_win8_extension);
 		vmstor_current_major = VMSTOR_WIN7_MAJOR;
@@ -1768,8 +1765,7 @@ static int storvsc_probe(struct hv_device *device,
 		max_luns_per_target = STORVSC_IDE_MAX_LUNS_PER_TARGET;
 		max_targets = STORVSC_IDE_MAX_TARGETS;
 		max_channels = STORVSC_IDE_MAX_CHANNELS;
-		break;
-	default:
+	} else {
 		sense_buffer_size = POST_WIN7_STORVSC_SENSE_BUFFER_SIZE;
 		vmscsi_size_delta = 0;
 		vmstor_current_major = VMSTOR_WIN8_MAJOR;
@@ -1783,7 +1779,6 @@ static int storvsc_probe(struct hv_device *device,
 		 * VCPUs in the guest.
 		 */
 		max_sub_channels = (num_cpus / storvsc_vcpus_per_sub_channel);
-		break;
 	}
 
 	scsi_driver.can_queue = (max_outstanding_req_per_channel *
-- 
1.7.4.1

[PATCH 2/6] scsi: storvsc: Use a single value to track protocol versions

[K. Y. Srinivasan]

From: keith.mange@microsoft.com <keith.mange@microsoft.com>

Use a single value to track protocol versions to simplify
comparisons and to be consistent with vmbus version tracking.

Tested-by: Alex Ng <alexng@microsoft.com>
Signed-off-by: Keith Mange <keith.mange@microsoft.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
 drivers/scsi/storvsc_drv.c |   35 +++++++++--------------------------
 1 files changed, 9 insertions(+), 26 deletions(-)

diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
index 582f3b5..5f9d133 100644
--- a/drivers/scsi/storvsc_drv.c
+++ b/drivers/scsi/storvsc_drv.c
@@ -58,12 +58,11 @@
  * Win8: 5.1
  */
 
+#define VMSTOR_PROTO_VERSION(MAJOR_, MINOR_)	((((MAJOR_) & 0xff) << 8) | \
+						(((MINOR_) & 0xff)))
 
-#define VMSTOR_WIN7_MAJOR 4
-#define VMSTOR_WIN7_MINOR 2
-
-#define VMSTOR_WIN8_MAJOR 5
-#define VMSTOR_WIN8_MINOR 1
+#define VMSTOR_PROTO_VERSION_WIN7	VMSTOR_PROTO_VERSION(4, 2)
+#define VMSTOR_PROTO_VERSION_WIN8	VMSTOR_PROTO_VERSION(5, 1)
 
 
 /*  Packet structure describing virtual storage requests. */
@@ -161,8 +160,7 @@ static int sense_buffer_size;
  */
 
 static int vmscsi_size_delta;
-static int vmstor_current_major;
-static int vmstor_current_minor;
+static int vmstor_proto_version;
 
 struct vmscsi_win8_extension {
 	/*
@@ -481,18 +479,6 @@ done:
 	kfree(wrk);
 }
 
-/*
- * Major/minor macros.  Minor version is in LSB, meaning that earlier flat
- * version numbers will be interpreted as "0.x" (i.e., 1 becomes 0.1).
- */
-
-static inline u16 storvsc_get_version(u8 major, u8 minor)
-{
-	u16 version;
-
-	version = ((major << 8) | minor);
-	return version;
-}
 
 /*
  * We can get incoming messages from the host that are not in response to
@@ -930,8 +916,7 @@ static int storvsc_channel_init(struct hv_device *device)
 	vstor_packet->operation = VSTOR_OPERATION_QUERY_PROTOCOL_VERSION;
 	vstor_packet->flags = REQUEST_COMPLETION_FLAG;
 
-	vstor_packet->version.major_minor =
-		storvsc_get_version(vmstor_current_major, vmstor_current_minor);
+	vstor_packet->version.major_minor = vmstor_proto_version;
 
 	/*
 	 * The revision number is only used in Windows; set it to 0.
@@ -1562,7 +1547,7 @@ static int storvsc_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *scmnd)
 	u32 payload_sz;
 	u32 length;
 
-	if (vmstor_current_major <= VMSTOR_WIN8_MAJOR) {
+	if (vmstor_proto_version <= VMSTOR_PROTO_VERSION_WIN8) {
 		/*
 		 * On legacy hosts filter unimplemented commands.
 		 * Future hosts are expected to correctly handle
@@ -1760,16 +1745,14 @@ static int storvsc_probe(struct hv_device *device,
 	if (vmbus_proto_version < VERSION_WIN8) {
 		sense_buffer_size = PRE_WIN8_STORVSC_SENSE_BUFFER_SIZE;
 		vmscsi_size_delta = sizeof(struct vmscsi_win8_extension);
-		vmstor_current_major = VMSTOR_WIN7_MAJOR;
-		vmstor_current_minor = VMSTOR_WIN7_MINOR;
+		vmstor_proto_version = VMSTOR_PROTO_VERSION_WIN7;
 		max_luns_per_target = STORVSC_IDE_MAX_LUNS_PER_TARGET;
 		max_targets = STORVSC_IDE_MAX_TARGETS;
 		max_channels = STORVSC_IDE_MAX_CHANNELS;
 	} else {
 		sense_buffer_size = POST_WIN7_STORVSC_SENSE_BUFFER_SIZE;
 		vmscsi_size_delta = 0;
-		vmstor_current_major = VMSTOR_WIN8_MAJOR;
-		vmstor_current_minor = VMSTOR_WIN8_MINOR;
+		vmstor_proto_version = VMSTOR_PROTO_VERSION_WIN8;
 		max_luns_per_target = STORVSC_MAX_LUNS_PER_TARGET;
 		max_targets = STORVSC_MAX_TARGETS;
 		max_channels = STORVSC_MAX_CHANNELS;
-- 
1.7.4.1

[PATCH 3/6] hv:scsi:Untangle the storage protocol negotiation from the vmbus protocol negotiation.

[K. Y. Srinivasan]

From: keith.mange@microsoft.com <keith.mange@microsoft.com>

Currently we are making decisions based on vmbus protocol versions
that have been negotiated; use storage potocol versions instead.

Tested-by: Alex Ng <alexng@microsoft.com>
Signed-off-by: Keith Mange <keith.mange@microsoft.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
 drivers/scsi/storvsc_drv.c |  109 +++++++++++++++++++++++++++++++++++---------
 1 files changed, 87 insertions(+), 22 deletions(-)

diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
index 5f9d133..f29871e 100644
--- a/drivers/scsi/storvsc_drv.c
+++ b/drivers/scsi/storvsc_drv.c
@@ -56,14 +56,18 @@
  * V1 RC > 2008/1/31:  2.0
  * Win7: 4.2
  * Win8: 5.1
+ * Win8.1: 6.0
+ * Win10: 6.2
  */
 
 #define VMSTOR_PROTO_VERSION(MAJOR_, MINOR_)	((((MAJOR_) & 0xff) << 8) | \
 						(((MINOR_) & 0xff)))
 
+#define VMSTOR_PROTO_VERSION_WIN6	VMSTOR_PROTO_VERSION(2, 0)
 #define VMSTOR_PROTO_VERSION_WIN7	VMSTOR_PROTO_VERSION(4, 2)
 #define VMSTOR_PROTO_VERSION_WIN8	VMSTOR_PROTO_VERSION(5, 1)
-
+#define VMSTOR_PROTO_VERSION_WIN8_1	VMSTOR_PROTO_VERSION(6, 0)
+#define VMSTOR_PROTO_VERSION_WIN10	VMSTOR_PROTO_VERSION(6, 2)
 
 /*  Packet structure describing virtual storage requests. */
 enum vstor_packet_operation {
@@ -205,6 +209,46 @@ struct vmscsi_request {
 
 
 /*
+ * The list of storage protocols in order of preference.
+ */
+struct vmstor_protocol {
+	int protocol_version;
+	int sense_buffer_size;
+	int vmscsi_size_delta;
+};
+
+#define VMSTOR_NUM_PROTOCOLS    5
+
+const struct vmstor_protocol vmstor_protocols[VMSTOR_NUM_PROTOCOLS] = {
+	{
+		VMSTOR_PROTO_VERSION_WIN10,
+		POST_WIN7_STORVSC_SENSE_BUFFER_SIZE,
+		0
+	},
+	{
+		VMSTOR_PROTO_VERSION_WIN8_1,
+		POST_WIN7_STORVSC_SENSE_BUFFER_SIZE,
+		0
+	},
+	{
+		VMSTOR_PROTO_VERSION_WIN8,
+		POST_WIN7_STORVSC_SENSE_BUFFER_SIZE,
+		0
+	},
+	{
+		VMSTOR_PROTO_VERSION_WIN7,
+		PRE_WIN8_STORVSC_SENSE_BUFFER_SIZE,
+		sizeof(struct vmscsi_win8_extension),
+	},
+	{
+		VMSTOR_PROTO_VERSION_WIN6,
+		PRE_WIN8_STORVSC_SENSE_BUFFER_SIZE,
+		sizeof(struct vmscsi_win8_extension),
+	}
+};
+
+
+/*
  * This structure is sent during the intialization phase to get the different
  * properties of the channel.
  */
@@ -871,7 +915,7 @@ static int storvsc_channel_init(struct hv_device *device)
 	struct storvsc_device *stor_device;
 	struct storvsc_cmd_request *request;
 	struct vstor_packet *vstor_packet;
-	int ret, t;
+	int ret, t, i;
 	int max_chns;
 	bool process_sub_channels = false;
 
@@ -911,36 +955,59 @@ static int storvsc_channel_init(struct hv_device *device)
 		goto cleanup;
 
 
-	/* reuse the packet for version range supported */
-	memset(vstor_packet, 0, sizeof(struct vstor_packet));
-	vstor_packet->operation = VSTOR_OPERATION_QUERY_PROTOCOL_VERSION;
-	vstor_packet->flags = REQUEST_COMPLETION_FLAG;
+	for (i = 0; i < VMSTOR_NUM_PROTOCOLS; i++) {
+		/* reuse the packet for version range supported */
+		memset(vstor_packet, 0, sizeof(struct vstor_packet));
+		vstor_packet->operation =
+			VSTOR_OPERATION_QUERY_PROTOCOL_VERSION;
+		vstor_packet->flags = REQUEST_COMPLETION_FLAG;
 
-	vstor_packet->version.major_minor = vmstor_proto_version;
+		vstor_packet->version.major_minor =
+			vmstor_protocols[i].protocol_version;
 
-	/*
-	 * The revision number is only used in Windows; set it to 0.
-	 */
-	vstor_packet->version.revision = 0;
+		/*
+		 * The revision number is only used in Windows; set it to 0.
+		 */
+		vstor_packet->version.revision = 0;
 
-	ret = vmbus_sendpacket(device->channel, vstor_packet,
+		ret = vmbus_sendpacket(device->channel, vstor_packet,
 			       (sizeof(struct vstor_packet) -
 				vmscsi_size_delta),
 			       (unsigned long)request,
 			       VM_PKT_DATA_INBAND,
 			       VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED);
-	if (ret != 0)
-		goto cleanup;
+		if (ret != 0)
+			goto cleanup;
 
-	t = wait_for_completion_timeout(&request->wait_event, 5*HZ);
-	if (t == 0) {
-		ret = -ETIMEDOUT;
-		goto cleanup;
+		t = wait_for_completion_timeout(&request->wait_event, 5*HZ);
+		if (t == 0) {
+			ret = -ETIMEDOUT;
+			goto cleanup;
+		}
+
+		if (vstor_packet->operation != VSTOR_OPERATION_COMPLETE_IO) {
+			ret = -EINVAL;
+			goto cleanup;
+		}
+
+		if (vstor_packet->status == 0) {
+			vmstor_proto_version =
+				vmstor_protocols[i].protocol_version;
+
+			sense_buffer_size =
+				vmstor_protocols[i].sense_buffer_size;
+
+			vmscsi_size_delta =
+				vmstor_protocols[i].vmscsi_size_delta;
+
+			break;
+		}
 	}
 
-	if (vstor_packet->operation != VSTOR_OPERATION_COMPLETE_IO ||
-	    vstor_packet->status != 0)
+	if (vstor_packet->status != 0) {
+		ret = -EINVAL;
 		goto cleanup;
+	}
 
 
 	memset(vstor_packet, 0, sizeof(struct vstor_packet));
@@ -1745,14 +1812,12 @@ static int storvsc_probe(struct hv_device *device,
 	if (vmbus_proto_version < VERSION_WIN8) {
 		sense_buffer_size = PRE_WIN8_STORVSC_SENSE_BUFFER_SIZE;
 		vmscsi_size_delta = sizeof(struct vmscsi_win8_extension);
-		vmstor_proto_version = VMSTOR_PROTO_VERSION_WIN7;
 		max_luns_per_target = STORVSC_IDE_MAX_LUNS_PER_TARGET;
 		max_targets = STORVSC_IDE_MAX_TARGETS;
 		max_channels = STORVSC_IDE_MAX_CHANNELS;
 	} else {
 		sense_buffer_size = POST_WIN7_STORVSC_SENSE_BUFFER_SIZE;
 		vmscsi_size_delta = 0;
-		vmstor_proto_version = VMSTOR_PROTO_VERSION_WIN8;
 		max_luns_per_target = STORVSC_MAX_LUNS_PER_TARGET;
 		max_targets = STORVSC_MAX_TARGETS;
 		max_channels = STORVSC_MAX_CHANNELS;
-- 
1.7.4.1

[PATCH 4/6] scsi: storvsc: use correct defaults for values determined by protocol negotiation

[K. Y. Srinivasan]

From: keith.mange@microsoft.com <keith.mange@microsoft.com>

Use correct defaults for values determined by protocol negotiation,
instead of resetting them with every scsi controller.

Tested-by: Alex Ng <alexng@microsoft.com>
Signed-off-by: Keith Mange <keith.mange@microsoft.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
 drivers/scsi/storvsc_drv.c |   33 +++++++++++++++++++--------------
 1 files changed, 19 insertions(+), 14 deletions(-)

diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
index f29871e..6f38cdf 100644
--- a/drivers/scsi/storvsc_drv.c
+++ b/drivers/scsi/storvsc_drv.c
@@ -151,19 +151,17 @@ struct hv_fc_wwn_packet {
 
 /*
  * Sense buffer size changed in win8; have a run-time
- * variable to track the size we should use.
+ * variable to track the size we should use.  This value will
+ * likely change during protocol negotiation but it is valid
+ * to start by assuming pre-Win8.
  */
-static int sense_buffer_size;
+static int sense_buffer_size = PRE_WIN8_STORVSC_SENSE_BUFFER_SIZE;
 
 /*
- * The size of the vmscsi_request has changed in win8. The
- * additional size is because of new elements added to the
- * structure. These elements are valid only when we are talking
- * to a win8 host.
- * Track the correction to size we need to apply.
- */
-
-static int vmscsi_size_delta;
+ * The storage protocol version is determined during the
+ * initial exchange with the host.  It will indicate which
+ * storage functionality is available in the host.
+*/
 static int vmstor_proto_version;
 
 struct vmscsi_win8_extension {
@@ -209,6 +207,17 @@ struct vmscsi_request {
 
 
 /*
+ * The size of the vmscsi_request has changed in win8. The
+ * additional size is because of new elements added to the
+ * structure. These elements are valid only when we are talking
+ * to a win8 host.
+ * Track the correction to size we need to apply. This value
+ * will likely change during protocol negotiation but it is
+ * valid to start by assuming pre-Win8.
+ */
+static int vmscsi_size_delta = sizeof(struct vmscsi_win8_extension);
+
+/*
  * The list of storage protocols in order of preference.
  */
 struct vmstor_protocol {
@@ -1810,14 +1819,10 @@ static int storvsc_probe(struct hv_device *device,
 	 */
 
 	if (vmbus_proto_version < VERSION_WIN8) {
-		sense_buffer_size = PRE_WIN8_STORVSC_SENSE_BUFFER_SIZE;
-		vmscsi_size_delta = sizeof(struct vmscsi_win8_extension);
 		max_luns_per_target = STORVSC_IDE_MAX_LUNS_PER_TARGET;
 		max_targets = STORVSC_IDE_MAX_TARGETS;
 		max_channels = STORVSC_IDE_MAX_CHANNELS;
 	} else {
-		sense_buffer_size = POST_WIN7_STORVSC_SENSE_BUFFER_SIZE;
-		vmscsi_size_delta = 0;
 		max_luns_per_target = STORVSC_MAX_LUNS_PER_TARGET;
 		max_targets = STORVSC_MAX_TARGETS;
 		max_channels = STORVSC_MAX_CHANNELS;
-- 
1.7.4.1

[PATCH 5/6] scsi: storvsc: use storage protocol version to determine storage capabilities

[K. Y. Srinivasan]

From: keith.mange@microsoft.com <keith.mange@microsoft.com>

Use storage protocol version instead of vmbus protocol
version when determining storage capabilities.

Tested-by: Alex Ng <alexng@microsoft.com>
Signed-off-by: Keith Mange <keith.mange@microsoft.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
 drivers/scsi/storvsc_drv.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
index 6f38cdf..58fa47a 100644
--- a/drivers/scsi/storvsc_drv.c
+++ b/drivers/scsi/storvsc_drv.c
@@ -1049,7 +1049,7 @@ static int storvsc_channel_init(struct hv_device *device)
 	 * support multi-channel.
 	 */
 	max_chns = vstor_packet->storage_channel_properties.max_channel_cnt;
-	if (vmbus_proto_version >= VERSION_WIN8) {
+	if (vmstor_proto_version >= VMSTOR_PROTO_VERSION_WIN8) {
 		if (vstor_packet->storage_channel_properties.flags &
 		    STORAGE_CHANNEL_SUPPORTS_MULTI_CHANNEL)
 			process_sub_channels = true;
@@ -1491,9 +1491,9 @@ static int storvsc_device_configure(struct scsi_device *sdevice)
 	 * if the device is a MSFT virtual device.
 	 */
 	if (!strncmp(sdevice->vendor, "Msft", 4)) {
-		switch (vmbus_proto_version) {
-		case VERSION_WIN8:
-		case VERSION_WIN8_1:
+		switch (vmstor_proto_version) {
+		case VMSTOR_PROTO_VERSION_WIN8:
+		case VMSTOR_PROTO_VERSION_WIN8_1:
 			sdevice->scsi_level = SCSI_SPC_3;
 			break;
 		}
-- 
1.7.4.1

[PATCH 6/6] scsi: storvsc: Allow write_same when host is windows 10

[K. Y. Srinivasan]

From: keith.mange@microsoft.com <keith.mange@microsoft.com>

Allow WRITE_SAME for Windows10 and above hosts.

Tested-by: Alex Ng <alexng@microsoft.com>
Signed-off-by: Keith Mange <keith.mange@microsoft.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
 drivers/scsi/storvsc_drv.c |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
index 58fa47a..021cbdf 100644
--- a/drivers/scsi/storvsc_drv.c
+++ b/drivers/scsi/storvsc_drv.c
@@ -1488,7 +1488,8 @@ static int storvsc_device_configure(struct scsi_device *sdevice)
 
 	/*
 	 * If the host is WIN8 or WIN8 R2, claim conformance to SPC-3
-	 * if the device is a MSFT virtual device.
+	 * if the device is a MSFT virtual device.  If the host is
+	 * WIN10 or newer, allow write_same.
 	 */
 	if (!strncmp(sdevice->vendor, "Msft", 4)) {
 		switch (vmstor_proto_version) {
@@ -1497,6 +1498,9 @@ static int storvsc_device_configure(struct scsi_device *sdevice)
 			sdevice->scsi_level = SCSI_SPC_3;
 			break;
 		}
+
+		if (vmstor_proto_version >= VMSTOR_PROTO_VERSION_WIN10)
+			sdevice->no_write_same = 0;
 	}
 
 	return 0;
-- 
1.7.4.1

RE: [PATCH 1/6] scsi: storvsc: Rather than look for sets of specific protocol versions, make decisions based on ranges.

[Long Li]

> -----Original Message-----
> From: devel [mailto:driverdev-devel-bounces@linuxdriverproject.org] On
> Behalf Of K. Y. Srinivasan
> Sent: Friday, May 29, 2015 1:29 PM
> To: gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; ohering@suse.com; jbottomley@parallels.com;
> hch@infradead.org; linux-scsi@vger.kernel.org; apw@canonical.com;
> vkuznets@redhat.com; jasowang@redhat.com
> Cc: Keith Mange
> Subject: [PATCH 1/6] scsi: storvsc: Rather than look for sets of specific protocol
> versions, make decisions based on ranges.
> 
> From: keith.mange@microsoft.com <keith.mange@microsoft.com>
> 
> Rather than look for sets of specific protocol versions, make decisions based on
> ranges. This will be safer and require fewer changes going forward as we add
> more storage protocol versions.
> 
Reviewed-by: Long Li <longli@microsoft.com>
> Tested-by: Alex Ng <alexng@microsoft.com>
> Signed-off-by: Keith Mange <keith.mange@microsoft.com>
> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> ---
>  drivers/scsi/storvsc_drv.c |   11 +++--------
>  1 files changed, 3 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c index
> 3c6584f..582f3b5 100644
> --- a/drivers/scsi/storvsc_drv.c
> +++ b/drivers/scsi/storvsc_drv.c
> @@ -988,8 +988,7 @@ static int storvsc_channel_init(struct hv_device *device)
>  	 * support multi-channel.
>  	 */
>  	max_chns = vstor_packet-
> >storage_channel_properties.max_channel_cnt;
> -	if ((vmbus_proto_version != VERSION_WIN7) &&
> -	   (vmbus_proto_version != VERSION_WS2008))  {
> +	if (vmbus_proto_version >= VERSION_WIN8) {
>  		if (vstor_packet->storage_channel_properties.flags &
>  		    STORAGE_CHANNEL_SUPPORTS_MULTI_CHANNEL)
>  			process_sub_channels = true;
> @@ -1758,9 +1757,7 @@ static int storvsc_probe(struct hv_device *device,
>  	 * set state to properly communicate with the host.
>  	 */
> 
> -	switch (vmbus_proto_version) {
> -	case VERSION_WS2008:
> -	case VERSION_WIN7:
> +	if (vmbus_proto_version < VERSION_WIN8) {
>  		sense_buffer_size = PRE_WIN8_STORVSC_SENSE_BUFFER_SIZE;
>  		vmscsi_size_delta = sizeof(struct vmscsi_win8_extension);
>  		vmstor_current_major = VMSTOR_WIN7_MAJOR; @@ -
> 1768,8 +1765,7 @@ static int storvsc_probe(struct hv_device *device,
>  		max_luns_per_target =
> STORVSC_IDE_MAX_LUNS_PER_TARGET;
>  		max_targets = STORVSC_IDE_MAX_TARGETS;
>  		max_channels = STORVSC_IDE_MAX_CHANNELS;
> -		break;
> -	default:
> +	} else {
>  		sense_buffer_size =
> POST_WIN7_STORVSC_SENSE_BUFFER_SIZE;
>  		vmscsi_size_delta = 0;
>  		vmstor_current_major = VMSTOR_WIN8_MAJOR; @@ -
> 1783,7 +1779,6 @@ static int storvsc_probe(struct hv_device *device,
>  		 * VCPUs in the guest.
>  		 */
>  		max_sub_channels = (num_cpus /
> storvsc_vcpus_per_sub_channel);
> -		break;
>  	}
> 
>  	scsi_driver.can_queue = (max_outstanding_req_per_channel *
> --
> 1.7.4.1
> 
> _______________________________________________
> devel mailing list
> devel@linuxdriverproject.org
> http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

RE: [PATCH 2/6] scsi: storvsc: Use a single value to track protocol versions

[Long Li]

> -----Original Message-----
> From: devel [mailto:driverdev-devel-bounces@linuxdriverproject.org] On
> Behalf Of K. Y. Srinivasan
> Sent: Friday, May 29, 2015 1:29 PM
> To: gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; ohering@suse.com; jbottomley@parallels.com;
> hch@infradead.org; linux-scsi@vger.kernel.org; apw@canonical.com;
> vkuznets@redhat.com; jasowang@redhat.com
> Cc: Keith Mange
> Subject: [PATCH 2/6] scsi: storvsc: Use a single value to track protocol versions
> 
> From: keith.mange@microsoft.com <keith.mange@microsoft.com>
> 
> Use a single value to track protocol versions to simplify comparisons and to be
> consistent with vmbus version tracking.
> 
Reviewed-by: Long Li <longli@microsoft.com>
> Tested-by: Alex Ng <alexng@microsoft.com>
> Signed-off-by: Keith Mange <keith.mange@microsoft.com>
> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> ---
>  drivers/scsi/storvsc_drv.c |   35 +++++++++--------------------------
>  1 files changed, 9 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c index
> 582f3b5..5f9d133 100644
> --- a/drivers/scsi/storvsc_drv.c
> +++ b/drivers/scsi/storvsc_drv.c
> @@ -58,12 +58,11 @@
>   * Win8: 5.1
>   */
> 
> +#define VMSTOR_PROTO_VERSION(MAJOR_, MINOR_)	((((MAJOR_) & 0xff)
> << 8) | \
> +						(((MINOR_) & 0xff)))
> 
> -#define VMSTOR_WIN7_MAJOR 4
> -#define VMSTOR_WIN7_MINOR 2
> -
> -#define VMSTOR_WIN8_MAJOR 5
> -#define VMSTOR_WIN8_MINOR 1
> +#define VMSTOR_PROTO_VERSION_WIN7	VMSTOR_PROTO_VERSION(4,
> 2)
> +#define VMSTOR_PROTO_VERSION_WIN8	VMSTOR_PROTO_VERSION(5,
> 1)
> 
> 
>  /*  Packet structure describing virtual storage requests. */ @@ -161,8 +160,7
> @@ static int sense_buffer_size;
>   */
> 
>  static int vmscsi_size_delta;
> -static int vmstor_current_major;
> -static int vmstor_current_minor;
> +static int vmstor_proto_version;
> 
>  struct vmscsi_win8_extension {
>  	/*
> @@ -481,18 +479,6 @@ done:
>  	kfree(wrk);
>  }
> 
> -/*
> - * Major/minor macros.  Minor version is in LSB, meaning that earlier flat
> - * version numbers will be interpreted as "0.x" (i.e., 1 becomes 0.1).
> - */
> -
> -static inline u16 storvsc_get_version(u8 major, u8 minor) -{
> -	u16 version;
> -
> -	version = ((major << 8) | minor);
> -	return version;
> -}
> 
>  /*
>   * We can get incoming messages from the host that are not in response to
> @@ -930,8 +916,7 @@ static int storvsc_channel_init(struct hv_device *device)
>  	vstor_packet->operation =
> VSTOR_OPERATION_QUERY_PROTOCOL_VERSION;
>  	vstor_packet->flags = REQUEST_COMPLETION_FLAG;
> 
> -	vstor_packet->version.major_minor =
> -		storvsc_get_version(vmstor_current_major,
> vmstor_current_minor);
> +	vstor_packet->version.major_minor = vmstor_proto_version;
> 
>  	/*
>  	 * The revision number is only used in Windows; set it to 0.
> @@ -1562,7 +1547,7 @@ static int storvsc_queuecommand(struct Scsi_Host
> *host, struct scsi_cmnd *scmnd)
>  	u32 payload_sz;
>  	u32 length;
> 
> -	if (vmstor_current_major <= VMSTOR_WIN8_MAJOR) {
> +	if (vmstor_proto_version <= VMSTOR_PROTO_VERSION_WIN8) {
>  		/*
>  		 * On legacy hosts filter unimplemented commands.
>  		 * Future hosts are expected to correctly handle @@ -1760,16
> +1745,14 @@ static int storvsc_probe(struct hv_device *device,
>  	if (vmbus_proto_version < VERSION_WIN8) {
>  		sense_buffer_size = PRE_WIN8_STORVSC_SENSE_BUFFER_SIZE;
>  		vmscsi_size_delta = sizeof(struct vmscsi_win8_extension);
> -		vmstor_current_major = VMSTOR_WIN7_MAJOR;
> -		vmstor_current_minor = VMSTOR_WIN7_MINOR;
> +		vmstor_proto_version = VMSTOR_PROTO_VERSION_WIN7;
>  		max_luns_per_target =
> STORVSC_IDE_MAX_LUNS_PER_TARGET;
>  		max_targets = STORVSC_IDE_MAX_TARGETS;
>  		max_channels = STORVSC_IDE_MAX_CHANNELS;
>  	} else {
>  		sense_buffer_size =
> POST_WIN7_STORVSC_SENSE_BUFFER_SIZE;
>  		vmscsi_size_delta = 0;
> -		vmstor_current_major = VMSTOR_WIN8_MAJOR;
> -		vmstor_current_minor = VMSTOR_WIN8_MINOR;
> +		vmstor_proto_version = VMSTOR_PROTO_VERSION_WIN8;
>  		max_luns_per_target = STORVSC_MAX_LUNS_PER_TARGET;
>  		max_targets = STORVSC_MAX_TARGETS;
>  		max_channels = STORVSC_MAX_CHANNELS;
> --
> 1.7.4.1
> 
> _______________________________________________
> devel mailing list
> devel@linuxdriverproject.org
> http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

RE: [PATCH 3/6] hv:scsi:Untangle the storage protocol negotiation from the vmbus protocol negotiation.

[Long Li]

> -----Original Message-----
> From: devel [mailto:driverdev-devel-bounces@linuxdriverproject.org] On
> Behalf Of K. Y. Srinivasan
> Sent: Friday, May 29, 2015 1:29 PM
> To: gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; ohering@suse.com; jbottomley@parallels.com;
> hch@infradead.org; linux-scsi@vger.kernel.org; apw@canonical.com;
> vkuznets@redhat.com; jasowang@redhat.com
> Cc: Keith Mange
> Subject: [PATCH 3/6] hv:scsi:Untangle the storage protocol negotiation from
> the vmbus protocol negotiation.
> 
> From: keith.mange@microsoft.com <keith.mange@microsoft.com>
> 
> Currently we are making decisions based on vmbus protocol versions that have
> been negotiated; use storage potocol versions instead.
> 
Reviewed-by: Long Li <longli@microsoft.com>
> Tested-by: Alex Ng <alexng@microsoft.com>
> Signed-off-by: Keith Mange <keith.mange@microsoft.com>
> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> ---
>  drivers/scsi/storvsc_drv.c |  109 +++++++++++++++++++++++++++++++++++------
> ---
>  1 files changed, 87 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c index
> 5f9d133..f29871e 100644
> --- a/drivers/scsi/storvsc_drv.c
> +++ b/drivers/scsi/storvsc_drv.c
> @@ -56,14 +56,18 @@
>   * V1 RC > 2008/1/31:  2.0
>   * Win7: 4.2
>   * Win8: 5.1
> + * Win8.1: 6.0
> + * Win10: 6.2
>   */
> 
>  #define VMSTOR_PROTO_VERSION(MAJOR_, MINOR_)	((((MAJOR_) & 0xff)
> << 8) | \
>  						(((MINOR_) & 0xff)))
> 
> +#define VMSTOR_PROTO_VERSION_WIN6	VMSTOR_PROTO_VERSION(2,
> 0)
>  #define VMSTOR_PROTO_VERSION_WIN7	VMSTOR_PROTO_VERSION(4,
> 2)
>  #define VMSTOR_PROTO_VERSION_WIN8	VMSTOR_PROTO_VERSION(5,
> 1)
> -
> +#define VMSTOR_PROTO_VERSION_WIN8_1	VMSTOR_PROTO_VERSION(6,
> 0)
> +#define VMSTOR_PROTO_VERSION_WIN10	VMSTOR_PROTO_VERSION(6,
> 2)
> 
>  /*  Packet structure describing virtual storage requests. */  enum
> vstor_packet_operation { @@ -205,6 +209,46 @@ struct vmscsi_request {
> 
> 
>  /*
> + * The list of storage protocols in order of preference.
> + */
> +struct vmstor_protocol {
> +	int protocol_version;
> +	int sense_buffer_size;
> +	int vmscsi_size_delta;
> +};
> +
> +#define VMSTOR_NUM_PROTOCOLS    5
> +
> +const struct vmstor_protocol vmstor_protocols[VMSTOR_NUM_PROTOCOLS]
> = {
> +	{
> +		VMSTOR_PROTO_VERSION_WIN10,
> +		POST_WIN7_STORVSC_SENSE_BUFFER_SIZE,
> +		0
> +	},
> +	{
> +		VMSTOR_PROTO_VERSION_WIN8_1,
> +		POST_WIN7_STORVSC_SENSE_BUFFER_SIZE,
> +		0
> +	},
> +	{
> +		VMSTOR_PROTO_VERSION_WIN8,
> +		POST_WIN7_STORVSC_SENSE_BUFFER_SIZE,
> +		0
> +	},
> +	{
> +		VMSTOR_PROTO_VERSION_WIN7,
> +		PRE_WIN8_STORVSC_SENSE_BUFFER_SIZE,
> +		sizeof(struct vmscsi_win8_extension),
> +	},
> +	{
> +		VMSTOR_PROTO_VERSION_WIN6,
> +		PRE_WIN8_STORVSC_SENSE_BUFFER_SIZE,
> +		sizeof(struct vmscsi_win8_extension),
> +	}
> +};
> +
> +
> +/*
>   * This structure is sent during the intialization phase to get the different
>   * properties of the channel.
>   */
> @@ -871,7 +915,7 @@ static int storvsc_channel_init(struct hv_device *device)
>  	struct storvsc_device *stor_device;
>  	struct storvsc_cmd_request *request;
>  	struct vstor_packet *vstor_packet;
> -	int ret, t;
> +	int ret, t, i;
>  	int max_chns;
>  	bool process_sub_channels = false;
> 
> @@ -911,36 +955,59 @@ static int storvsc_channel_init(struct hv_device
> *device)
>  		goto cleanup;
> 
> 
> -	/* reuse the packet for version range supported */
> -	memset(vstor_packet, 0, sizeof(struct vstor_packet));
> -	vstor_packet->operation =
> VSTOR_OPERATION_QUERY_PROTOCOL_VERSION;
> -	vstor_packet->flags = REQUEST_COMPLETION_FLAG;
> +	for (i = 0; i < VMSTOR_NUM_PROTOCOLS; i++) {
> +		/* reuse the packet for version range supported */
> +		memset(vstor_packet, 0, sizeof(struct vstor_packet));
> +		vstor_packet->operation =
> +			VSTOR_OPERATION_QUERY_PROTOCOL_VERSION;
> +		vstor_packet->flags = REQUEST_COMPLETION_FLAG;
> 
> -	vstor_packet->version.major_minor = vmstor_proto_version;
> +		vstor_packet->version.major_minor =
> +			vmstor_protocols[i].protocol_version;
> 
> -	/*
> -	 * The revision number is only used in Windows; set it to 0.
> -	 */
> -	vstor_packet->version.revision = 0;
> +		/*
> +		 * The revision number is only used in Windows; set it to 0.
> +		 */
> +		vstor_packet->version.revision = 0;
> 
> -	ret = vmbus_sendpacket(device->channel, vstor_packet,
> +		ret = vmbus_sendpacket(device->channel, vstor_packet,
>  			       (sizeof(struct vstor_packet) -
>  				vmscsi_size_delta),
>  			       (unsigned long)request,
>  			       VM_PKT_DATA_INBAND,
> 
> VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED);
> -	if (ret != 0)
> -		goto cleanup;
> +		if (ret != 0)
> +			goto cleanup;
> 
> -	t = wait_for_completion_timeout(&request->wait_event, 5*HZ);
> -	if (t == 0) {
> -		ret = -ETIMEDOUT;
> -		goto cleanup;
> +		t = wait_for_completion_timeout(&request->wait_event, 5*HZ);
> +		if (t == 0) {
> +			ret = -ETIMEDOUT;
> +			goto cleanup;
> +		}
> +
> +		if (vstor_packet->operation !=
> VSTOR_OPERATION_COMPLETE_IO) {
> +			ret = -EINVAL;
> +			goto cleanup;
> +		}
> +
> +		if (vstor_packet->status == 0) {
> +			vmstor_proto_version =
> +				vmstor_protocols[i].protocol_version;
> +
> +			sense_buffer_size =
> +				vmstor_protocols[i].sense_buffer_size;
> +
> +			vmscsi_size_delta =
> +				vmstor_protocols[i].vmscsi_size_delta;
> +
> +			break;
> +		}
>  	}
> 
> -	if (vstor_packet->operation != VSTOR_OPERATION_COMPLETE_IO ||
> -	    vstor_packet->status != 0)
> +	if (vstor_packet->status != 0) {
> +		ret = -EINVAL;
>  		goto cleanup;
> +	}
> 
> 
>  	memset(vstor_packet, 0, sizeof(struct vstor_packet)); @@ -1745,14
> +1812,12 @@ static int storvsc_probe(struct hv_device *device,
>  	if (vmbus_proto_version < VERSION_WIN8) {
>  		sense_buffer_size = PRE_WIN8_STORVSC_SENSE_BUFFER_SIZE;
>  		vmscsi_size_delta = sizeof(struct vmscsi_win8_extension);
> -		vmstor_proto_version = VMSTOR_PROTO_VERSION_WIN7;
>  		max_luns_per_target =
> STORVSC_IDE_MAX_LUNS_PER_TARGET;
>  		max_targets = STORVSC_IDE_MAX_TARGETS;
>  		max_channels = STORVSC_IDE_MAX_CHANNELS;
>  	} else {
>  		sense_buffer_size =
> POST_WIN7_STORVSC_SENSE_BUFFER_SIZE;
>  		vmscsi_size_delta = 0;
> -		vmstor_proto_version = VMSTOR_PROTO_VERSION_WIN8;
>  		max_luns_per_target = STORVSC_MAX_LUNS_PER_TARGET;
>  		max_targets = STORVSC_MAX_TARGETS;
>  		max_channels = STORVSC_MAX_CHANNELS;
> --
> 1.7.4.1
> 
> _______________________________________________
> devel mailing list
> devel@linuxdriverproject.org
> http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

RE: [PATCH 4/6] scsi: storvsc: use correct defaults for values determined by protocol negotiation

[Long Li]

> -----Original Message-----
> From: devel [mailto:driverdev-devel-bounces@linuxdriverproject.org] On
> Behalf Of K. Y. Srinivasan
> Sent: Friday, May 29, 2015 1:29 PM
> To: gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; ohering@suse.com; jbottomley@parallels.com;
> hch@infradead.org; linux-scsi@vger.kernel.org; apw@canonical.com;
> vkuznets@redhat.com; jasowang@redhat.com
> Cc: Keith Mange
> Subject: [PATCH 4/6] scsi: storvsc: use correct defaults for values determined
> by protocol negotiation
> 
> From: keith.mange@microsoft.com <keith.mange@microsoft.com>
> 
> Use correct defaults for values determined by protocol negotiation, instead of
> resetting them with every scsi controller.
> 
Reviewed-by: Long Li <longli@microsoft.com>
> Tested-by: Alex Ng <alexng@microsoft.com>
> Signed-off-by: Keith Mange <keith.mange@microsoft.com>
> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> ---
>  drivers/scsi/storvsc_drv.c |   33 +++++++++++++++++++--------------
>  1 files changed, 19 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c index
> f29871e..6f38cdf 100644
> --- a/drivers/scsi/storvsc_drv.c
> +++ b/drivers/scsi/storvsc_drv.c
> @@ -151,19 +151,17 @@ struct hv_fc_wwn_packet {
> 
>  /*
>   * Sense buffer size changed in win8; have a run-time
> - * variable to track the size we should use.
> + * variable to track the size we should use.  This value will
> + * likely change during protocol negotiation but it is valid
> + * to start by assuming pre-Win8.
>   */
> -static int sense_buffer_size;
> +static int sense_buffer_size = PRE_WIN8_STORVSC_SENSE_BUFFER_SIZE;
> 
>  /*
> - * The size of the vmscsi_request has changed in win8. The
> - * additional size is because of new elements added to the
> - * structure. These elements are valid only when we are talking
> - * to a win8 host.
> - * Track the correction to size we need to apply.
> - */
> -
> -static int vmscsi_size_delta;
> + * The storage protocol version is determined during the
> + * initial exchange with the host.  It will indicate which
> + * storage functionality is available in the host.
> +*/
>  static int vmstor_proto_version;
> 
>  struct vmscsi_win8_extension {
> @@ -209,6 +207,17 @@ struct vmscsi_request {
> 
> 
>  /*
> + * The size of the vmscsi_request has changed in win8. The
> + * additional size is because of new elements added to the
> + * structure. These elements are valid only when we are talking
> + * to a win8 host.
> + * Track the correction to size we need to apply. This value
> + * will likely change during protocol negotiation but it is
> + * valid to start by assuming pre-Win8.
> + */
> +static int vmscsi_size_delta = sizeof(struct vmscsi_win8_extension);
> +
> +/*
>   * The list of storage protocols in order of preference.
>   */
>  struct vmstor_protocol {
> @@ -1810,14 +1819,10 @@ static int storvsc_probe(struct hv_device *device,
>  	 */
> 
>  	if (vmbus_proto_version < VERSION_WIN8) {
> -		sense_buffer_size = PRE_WIN8_STORVSC_SENSE_BUFFER_SIZE;
> -		vmscsi_size_delta = sizeof(struct vmscsi_win8_extension);
>  		max_luns_per_target =
> STORVSC_IDE_MAX_LUNS_PER_TARGET;
>  		max_targets = STORVSC_IDE_MAX_TARGETS;
>  		max_channels = STORVSC_IDE_MAX_CHANNELS;
>  	} else {
> -		sense_buffer_size =
> POST_WIN7_STORVSC_SENSE_BUFFER_SIZE;
> -		vmscsi_size_delta = 0;
>  		max_luns_per_target = STORVSC_MAX_LUNS_PER_TARGET;
>  		max_targets = STORVSC_MAX_TARGETS;
>  		max_channels = STORVSC_MAX_CHANNELS;
> --
> 1.7.4.1
> 
> _______________________________________________
> devel mailing list
> devel@linuxdriverproject.org
> http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

RE: [PATCH 5/6] scsi: storvsc: use storage protocol version to determine storage capabilities

[Long Li]

> -----Original Message-----
> From: devel [mailto:driverdev-devel-bounces@linuxdriverproject.org] On
> Behalf Of K. Y. Srinivasan
> Sent: Friday, May 29, 2015 1:29 PM
> To: gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; ohering@suse.com; jbottomley@parallels.com;
> hch@infradead.org; linux-scsi@vger.kernel.org; apw@canonical.com;
> vkuznets@redhat.com; jasowang@redhat.com
> Cc: Keith Mange
> Subject: [PATCH 5/6] scsi: storvsc: use storage protocol version to determine
> storage capabilities
> 
> From: keith.mange@microsoft.com <keith.mange@microsoft.com>
> 
> Use storage protocol version instead of vmbus protocol version when
> determining storage capabilities.
> 
Reviewed-by: Long Li <longli@microsoft.com>
> Tested-by: Alex Ng <alexng@microsoft.com>
> Signed-off-by: Keith Mange <keith.mange@microsoft.com>
> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> ---
>  drivers/scsi/storvsc_drv.c |    8 ++++----
>  1 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c index
> 6f38cdf..58fa47a 100644
> --- a/drivers/scsi/storvsc_drv.c
> +++ b/drivers/scsi/storvsc_drv.c
> @@ -1049,7 +1049,7 @@ static int storvsc_channel_init(struct hv_device
> *device)
>  	 * support multi-channel.
>  	 */
>  	max_chns = vstor_packet-
> >storage_channel_properties.max_channel_cnt;
> -	if (vmbus_proto_version >= VERSION_WIN8) {
> +	if (vmstor_proto_version >= VMSTOR_PROTO_VERSION_WIN8) {
>  		if (vstor_packet->storage_channel_properties.flags &
>  		    STORAGE_CHANNEL_SUPPORTS_MULTI_CHANNEL)
>  			process_sub_channels = true;
> @@ -1491,9 +1491,9 @@ static int storvsc_device_configure(struct
> scsi_device *sdevice)
>  	 * if the device is a MSFT virtual device.
>  	 */
>  	if (!strncmp(sdevice->vendor, "Msft", 4)) {
> -		switch (vmbus_proto_version) {
> -		case VERSION_WIN8:
> -		case VERSION_WIN8_1:
> +		switch (vmstor_proto_version) {
> +		case VMSTOR_PROTO_VERSION_WIN8:
> +		case VMSTOR_PROTO_VERSION_WIN8_1:
>  			sdevice->scsi_level = SCSI_SPC_3;
>  			break;
>  		}
> --
> 1.7.4.1
> 
> _______________________________________________
> devel mailing list
> devel@linuxdriverproject.org
> http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

RE: [PATCH 6/6] scsi: storvsc: Allow write_same when host is windows 10

[Long Li]

> -----Original Message-----
> From: devel [mailto:driverdev-devel-bounces@linuxdriverproject.org] On
> Behalf Of K. Y. Srinivasan
> Sent: Friday, May 29, 2015 1:29 PM
> To: gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; ohering@suse.com; jbottomley@parallels.com;
> hch@infradead.org; linux-scsi@vger.kernel.org; apw@canonical.com;
> vkuznets@redhat.com; jasowang@redhat.com
> Cc: Keith Mange
> Subject: [PATCH 6/6] scsi: storvsc: Allow write_same when host is windows 10
> 
> From: keith.mange@microsoft.com <keith.mange@microsoft.com>
> 
> Allow WRITE_SAME for Windows10 and above hosts.
> 
Reviewed-by: Long Li <longli@microsoft.com>
> Tested-by: Alex Ng <alexng@microsoft.com>
> Signed-off-by: Keith Mange <keith.mange@microsoft.com>
> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> ---
>  drivers/scsi/storvsc_drv.c |    6 +++++-
>  1 files changed, 5 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c index
> 58fa47a..021cbdf 100644
> --- a/drivers/scsi/storvsc_drv.c
> +++ b/drivers/scsi/storvsc_drv.c
> @@ -1488,7 +1488,8 @@ static int storvsc_device_configure(struct
> scsi_device *sdevice)
> 
>  	/*
>  	 * If the host is WIN8 or WIN8 R2, claim conformance to SPC-3
> -	 * if the device is a MSFT virtual device.
> +	 * if the device is a MSFT virtual device.  If the host is
> +	 * WIN10 or newer, allow write_same.
>  	 */
>  	if (!strncmp(sdevice->vendor, "Msft", 4)) {
>  		switch (vmstor_proto_version) {
> @@ -1497,6 +1498,9 @@ static int storvsc_device_configure(struct
> scsi_device *sdevice)
>  			sdevice->scsi_level = SCSI_SPC_3;
>  			break;
>  		}
> +
> +		if (vmstor_proto_version >=
> VMSTOR_PROTO_VERSION_WIN10)
> +			sdevice->no_write_same = 0;
>  	}
> 
>  	return 0;
> --
> 1.7.4.1
> 
> _______________________________________________
> devel mailing list
> devel@linuxdriverproject.org
> http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 3/6] hv:scsi:Untangle the storage protocol negotiation from the vmbus protocol negotiation.

[Dan Carpenter]

On Fri, May 29, 2015 at 01:29:16PM -0700, K. Y. Srinivasan wrote:
> -	if (vstor_packet->operation != VSTOR_OPERATION_COMPLETE_IO ||
> -	    vstor_packet->status != 0)
> +	if (vstor_packet->status != 0) {
> +		ret = -EINVAL;
>  		goto cleanup;
> +	}

There is not actually any cleanup, goto cleanup is just a do-nothing
goto.

In the original code, we returned success here.  That always looked like
a "forgot to set the error code" bug to me, but do-nothing labels always
introduce ambiguous looking "forgot to set the error code" bugs so I can
never be positive.

Could you take a look at the other "goto cleanup;" places in this
function and maybe add a comment, change it to something more clear like
"return 0;" or fix the error code?

regards,
dan carpenter

Re: [PATCH 1/6] scsi: storvsc: Rather than look for sets of specific protocol versions, make decisions based on ranges.

[Dan Carpenter]

On Fri, May 29, 2015 at 01:29:14PM -0700, K. Y. Srinivasan wrote:
> From: keith.mange@microsoft.com <keith.mange@microsoft.com>

Keith's name is wrong.

regards,
dan carpenter

RE: [PATCH 1/6] scsi: storvsc: Rather than look for sets of specific protocol versions, make decisions based on ranges.

[KY Srinivasan]

> -----Original Message-----
> From: Dan Carpenter [mailto:dan.carpenter@oracle.com]
> Sent: Monday, June 1, 2015 3:57 AM
> To: KY Srinivasan
> Cc: gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; ohering@suse.com;
> jbottomley@parallels.com; hch@infradead.org; linux-scsi@vger.kernel.org;
> apw@canonical.com; vkuznets@redhat.com; jasowang@redhat.com; Keith
> Mange
> Subject: Re: [PATCH 1/6] scsi: storvsc: Rather than look for sets of specific
> protocol versions, make decisions based on ranges.
> 
> On Fri, May 29, 2015 at 01:29:14PM -0700, K. Y. Srinivasan wrote:
> > From: keith.mange@microsoft.com <keith.mange@microsoft.com>
> 
> Keith's name is wrong.

Thanks Dan; I will fix this and resend.

K. Y
> 
> regards,
> dan carpenter

RE: [PATCH 3/6] hv:scsi:Untangle the storage protocol negotiation from the vmbus protocol negotiation.

[KY Srinivasan]

> -----Original Message-----
> From: Dan Carpenter [mailto:dan.carpenter@oracle.com]
> Sent: Monday, June 1, 2015 3:43 AM
> To: KY Srinivasan
> Cc: gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; ohering@suse.com;
> jbottomley@parallels.com; hch@infradead.org; linux-scsi@vger.kernel.org;
> apw@canonical.com; vkuznets@redhat.com; jasowang@redhat.com; Keith
> Mange
> Subject: Re: [PATCH 3/6] hv:scsi:Untangle the storage protocol negotiation
> from the vmbus protocol negotiation.
> 
> On Fri, May 29, 2015 at 01:29:16PM -0700, K. Y. Srinivasan wrote:
> > -	if (vstor_packet->operation != VSTOR_OPERATION_COMPLETE_IO
> ||
> > -	    vstor_packet->status != 0)
> > +	if (vstor_packet->status != 0) {
> > +		ret = -EINVAL;
> >  		goto cleanup;
> > +	}
> 
> There is not actually any cleanup, goto cleanup is just a do-nothing
> goto.
> 
> In the original code, we returned success here.  That always looked like
> a "forgot to set the error code" bug to me, but do-nothing labels always
> introduce ambiguous looking "forgot to set the error code" bugs so I can
> never be positive.
> 
> Could you take a look at the other "goto cleanup;" places in this
> function and maybe add a comment, change it to something more clear like
> "return 0;" or fix the error code?

Thanks Dan; will do.

K. Y
> 
> regards,
> dan carpenter