scsi: Add Hyper-V logical block provisioning quirks

PATCH In review Sitsofe Wheeler 6 participants 20 messages Oct 10, 2014 – Oct 23, 2014

Subsystems (2)

SCSI Hyper-V/Azure core and drivers

lore

Share & tools

lore

Sitsofe Wheeler

author

2014-10-10 07:49 UTC #0 lore raw

cover scsi: Add Hyper-V logical block provisioning quirks

Microsoft Hyper-V virtual disks currently only claim SPC-2 compliance even though they implement post SPC-2 features (such as thin provisioning) which means the Linux kernel does not go on to test for those features even though they are advertised.

A previous patch attempted to add a quirk to workaround this but the quirk was only enabled after the features had been scanned for, wouldn't work for "small" disks and would quirk on all Hyper-V SCSI devices (e.g. passthrough disks).

The new patches partially revert the previous effort, add the quirk in a more traditional manner to only Hyper-V virtual disks and work on small virtual disks.

Sitsofe Wheeler (3):
Revert "Drivers: add blist flags"
scsi: add try_rc16 blacklist flag
scsi: Use try_rc16 and try_vpd_pages quirks on Hyper-V virtual disks

 drivers/scsi/scsi_devinfo.c |  1 +
 drivers/scsi/scsi_scan.c    |  3 +++
 drivers/scsi/sd.c           |  3 +++
 drivers/scsi/storvsc_drv.c  | 10 ----------
 include/scsi/scsi_device.h  |  1 +
 include/scsi/scsi_devinfo.h |  1 +
 6 files changed, 9 insertions(+), 10 deletions(-)

Signature · 1 line

1.9.3

Sitsofe Wheeler

author

2014-10-10 07:51 UTC #1 lore raw

PATCH 1/3 Revert "Drivers: add blist flags"

This reverts commit f3cfabce7a2e92564d380de3aad4b43901fb7ae6 (Drivers:
add blist flags) as it does not enable thin provisioning for my Hyper-V 2012 R2 virtual disks.

Signed-off-by Sitsofe Wheeler <[email protected]>

---
 drivers/scsi/storvsc_drv.c | 10 ----------
 1 file changed, 10 deletions(-)

drivers/scsi/storvsc_drv.c +0 ~~-10~~

index 733e5f7..2cc094e 100644
@@ -327,8 +327,6 @@ MODULE_PARM_DESC(storvsc_ringbuffer_size, "Ring buffer size (bytes)");
  */
 static int storvsc_timeout = 180;
 
-static int msft_blist_flags = BLIST_TRY_VPD_PAGES;
-
 #define STORVSC_MAX_IO_REQUESTS				200
 
 static void storvsc_on_channel_callback(void *context);

@@ -1439,14 +1437,6 @@ static int storvsc_device_configure(struct scsi_device *sdevice)
 
 	sdevice->no_write_same = 1;
 
-	/*
-	 * Add blist flags to permit the reading of the VPD pages even when
-	 * the target may claim SPC-2 compliance. MSFT targets currently
-	 * claim SPC-2 compliance while they implement post SPC-2 features.
-	 * With this patch we can correctly handle WRITE_SAME_16 issues.
-	 */
-	sdevice->sdev_bflags |= msft_blist_flags;
-
 	return 0;
 }

Signature · 1 line

1.9.3

Sitsofe Wheeler

author

2014-10-10 07:52 UTC #2 lore raw

PATCH 2/3 scsi: add try_rc16 blacklist flag

In reply to Sitsofe Wheeler

Microsoft Hyper-V virtual disks currently only claim SPC-2 compliance causing the kernel skip checks for features such as thin provisioning even though the virtual disk advertises them.

Add a blacklist flag that can allow such devices to quirk past READ CAPACITY(16) guards.

Signed-off-by Sitsofe Wheeler <[email protected]>

---
 drivers/scsi/scsi_scan.c    | 3 +++
 drivers/scsi/sd.c           | 3 +++
 include/scsi/scsi_device.h  | 1 +
 include/scsi/scsi_devinfo.h | 1 +
 4 files changed, 8 insertions(+)

drivers/scsi/scsi_scan.c +3 -0

index ba3f1e8..d3f6267 100644
@@ -962,6 +962,9 @@ static int scsi_add_lun(struct scsi_device *sdev, unsigned char *inq_result,
 	else if (*bflags & BLIST_SKIP_VPD_PAGES)
 		sdev->skip_vpd_pages = 1;
 
+	if (*bflags & BLIST_TRY_RC16)
+		sdev->try_rc16 = 1;
+
 	transport_configure_device(&sdev->sdev_gendev);
 
 	if (sdev->host->hostt->slave_configure) {

drivers/scsi/sd.c +3 -0

index 0cb5c9f..0ccf372 100644
@@ -2143,6 +2143,9 @@ static int sd_try_rc16_first(struct scsi_device *sdp)
 		return 0;
 	if (sdp->scsi_level > SCSI_SPC_2)
 		return 1;
+	if (sdp->try_rc16) {
+		return 1;
+	}
 	if (scsi_device_protection(sdp))
 		return 1;
 	return 0;

include/scsi/scsi_device.h +1 -0

index 27ecee7..d6e2bd8 100644
@@ -155,6 +155,7 @@ struct scsi_device {
 	unsigned skip_ms_page_3f:1;	/* do not use MODE SENSE page 0x3f */
 	unsigned skip_vpd_pages:1;	/* do not read VPD pages */
 	unsigned try_vpd_pages:1;	/* attempt to read VPD pages */
+	unsigned try_rc16:1;		/* attempt READ CAPACITY(16) */
 	unsigned use_192_bytes_for_3f:1; /* ask for 192 bytes from page 0x3f */
 	unsigned no_start_on_add:1;	/* do not issue start on add */
 	unsigned allow_restart:1; /* issue START_UNIT in error handler */

include/scsi/scsi_devinfo.h +1 -0

index 183eaab..9431f5e 100644
@@ -36,5 +36,6 @@
 					     for sequential scan */
 #define BLIST_TRY_VPD_PAGES	0x10000000 /* Attempt to read VPD pages */
 #define BLIST_NO_RSOC		0x20000000 /* don't try to issue RSOC */
+#define BLIST_TRY_RC16		0x40000000 /* Attempt READ CAPACITY(16) */
 
 #endif

Signature · 1 line

1.9.3

Martin K. Petersen

2014-10-15 01:08 UTC #3 lore raw

Re: [PATCH 2/3] scsi: add try_rc16 blacklist flag

Quoted 1 line

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Sitsofe" == Sitsofe Wheeler <[email protected]> writes:

Sitsofe> Microsoft Hyper-V virtual disks currently only claim SPC-2 Sitsofe> compliance causing the kernel skip checks for features such as Sitsofe> thin provisioning even though the virtual disk advertises them.

Last time around we identified this as a problem with Microsoft's interpretation of the T10 SBC spec. And they promised that they are going to fix that.

Signature · 1 line

Martin K. Petersen	Oracle Linux Engineering

KY Srinivasan

2014-10-15 02:07 UTC #4 lore raw

RE: [PATCH 2/3] scsi: add try_rc16 blacklist flag

Quoted 13 lines

-----Original Message-----
From: Martin K. Petersen [mailto:[email protected]]
Sent: Tuesday, October 14, 2014 6:08 PM
To: Sitsofe Wheeler
Cc: KY Srinivasan; Haiyang Zhang; Christoph Hellwig; Hannes Reinecke; linux- [email protected]; [email protected]; [email protected]; James E.J. Bottomley
Subject: Re: [PATCH 2/3] scsi: add try_rc16 blacklist flag
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Sitsofe" == Sitsofe Wheeler <[email protected]> writes:

Quoted 7 lines

Sitsofe> Microsoft Hyper-V virtual disks currently only claim SPC-2 Sitsofe> compliance causing the kernel skip checks for features such as Sitsofe> thin provisioning even though the virtual disk advertises them.
Last time around we identified this as a problem with Microsoft's interpretation of the T10 SBC spec. And they promised that they are going to fix that.

It has been fixed in windows 10 and a bug has been opened for earlier hosts.

K. Y

Quoted 3 lines

--
Martin K. Petersen Oracle Linux Engineering

Sitsofe Wheeler

author

2014-10-21 04:21 UTC #5 lore raw

Re: [PATCH 2/3] scsi: add try_rc16 blacklist flag

In reply to Martin K. Petersen

Quoted 9 lines · Martin K. Petersen

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Sitsofe" == Sitsofe Wheeler <[email protected]> writes:
Sitsofe> Microsoft Hyper-V virtual disks currently only claim SPC-2 Sitsofe> compliance causing the kernel skip checks for features such as Sitsofe> thin provisioning even though the virtual disk advertises them.
Last time around we identified this as a problem with Microsoft's interpretation of the T10 SBC spec. And they promised that they are going to fix that.

OK but if we were happy to wait for Microsoft to fix the problem on the host why were the (broken and incomplete) BLIST_SKIP_VPD_PAGES patches committed to 3.17 rather than withdrawn? What's going to be done about those patches now?

Signature · 1 line

Sitsofe | http://sucs.org/~sits/

Martin K. Petersen

2014-10-23 01:46 UTC #6 lore raw

Re: [PATCH 2/3] scsi: add try_rc16 blacklist flag

Quoted 5 lines

Quoted 5 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Sitsofe" == Sitsofe Wheeler <[email protected]> writes:
Last time around we identified this as a problem with Microsoft's interpretation of the T10 SBC spec. And they promised that they are going to fix that.

Sitsofe> OK but if we were happy to wait for Microsoft to fix the Sitsofe> problem on the host why were the (broken and incomplete) Sitsofe> BLIST_SKIP_VPD_PAGES patches committed to 3.17 rather than Sitsofe> withdrawn? What's going to be done about those patches now?

There are two orthogonal problems. One being that the driver advertised conformance to an old SCSI spec. That's being addressed with the separate SPC-3 patch.

The other issue is that thin provisioning is being incorrectly advertised. Because that's being addressed by Microsoft and is an isolated use case I'm hesitant to add quirk for it. Whereas I know several other devices that will benefit from the TRY_VPD_PAGES blacklist option.

Signature · 1 line

Martin K. Petersen	Oracle Linux Engineering

Sitsofe Wheeler

author

2014-10-10 07:55 UTC #7 lore raw

PATCH 3/3 scsi: Use try_rc16 and try_vpd_pages quirks on Hyper-V virtual disks

In reply to Sitsofe Wheeler

Microsoft Hyper-V virtual disks currently only claim SPC-2 compliance causing feature tests not to be run (even though those features are correctly announced elsewhere). Make Hyper-V virtual disks quirk past READ CAPACITY(16) and VPD page guards so appropriate tests are performed.

This only impacts Hyper-V's VHD/VHDX virtual disks and not passthrough devices.

Signed-off-by Sitsofe Wheeler <[email protected]>

---
 drivers/scsi/scsi_devinfo.c | 1 +
 1 file changed, 1 insertion(+)

drivers/scsi/scsi_devinfo.c +1 -0

index 49014a1..3eadcb1 100644
@@ -210,6 +210,7 @@ static struct {
 	{"Medion", "Flash XL  MMC/SD", "2.6D", BLIST_FORCELUN},
 	{"MegaRAID", "LD", NULL, BLIST_FORCELUN},
 	{"MICROP", "4110", NULL, BLIST_NOTQ},
+	{"Msft", "Virtual Disk", "1.0", BLIST_TRY_VPD_PAGES | BLIST_TRY_RC16},
 	{"MYLEX", "DACARMRB", "*", BLIST_REPORTLUN2},
 	{"nCipher", "Fastness Crypto", NULL, BLIST_FORCELUN},
 	{"NAKAMICH", "MJ-4.8S", NULL, BLIST_FORCELUN | BLIST_SINGLELUN},

Signature · 1 line

1.9.3

Christoph Hellwig

2014-10-11 17:39 UTC #8 lore raw

In reply to Sitsofe Wheeler

Quoted 5 lines · Sitsofe Wheeler

Microsoft Hyper-V virtual disks currently only claim SPC-2 compliance even though they implement post SPC-2 features (such as thin provisioning) which means the Linux kernel does not go on to test for those features even though they are advertised.

Quoted 8 lines

A previous patch attempted to add a quirk to workaround this but the quirk was only enabled after the features had been scanned for, wouldn't work for "small" disks and would quirk on all Hyper-V SCSI devices (e.g. passthrough disks).
The new patches partially revert the previous effort, add the quirk in a more traditional manner to only Hyper-V virtual disks and work on small virtual disks.

This seems like might want a quirk to simply "force" a SPC3 compliance level?

James Bottomley

maintainer

2014-10-11 17:41 UTC #9 lore raw

Quoted 11 lines · Christoph Hellwig

Quoted 10 lines · Sitsofe Wheeler
Microsoft Hyper-V virtual disks currently only claim SPC-2 compliance even though they implement post SPC-2 features (such as thin provisioning) which means the Linux kernel does not go on to test for those features even though they are advertised.
A previous patch attempted to add a quirk to workaround this but the quirk was only enabled after the features had been scanned for, wouldn't work for "small" disks and would quirk on all Hyper-V SCSI devices (e.g. passthrough disks).

Quoted 6 lines

Quoted 3 lines
The new patches partially revert the previous effort, add the quirk in a more traditional manner to only Hyper-V virtual disks and work on small virtual disks.
This seems like might want a quirk to simply "force" a SPC3 compliance level?

This was initially suggested, but rejected by Microsoft because of other problems advertising SPC-3 compliance brings. Perhaps the hyper-v emulator has matured sufficiently that it will now work OK?

James

KY Srinivasan

2014-10-11 19:48 UTC #10 lore raw

Quoted 28 lines

-----Original Message-----
From: James Bottomley [mailto:[email protected]]
Sent: Saturday, October 11, 2014 10:42 AM
To: Christoph Hellwig
Cc: Sitsofe Wheeler; KY Srinivasan; Haiyang Zhang; Christoph Hellwig; Hannes Reinecke; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH 0/3] scsi: Add Hyper-V logical block provisioning quirks
Quoted 16 lines · Christoph Hellwig
Quoted 14 lines · Sitsofe Wheeler
Microsoft Hyper-V virtual disks currently only claim SPC-2 compliance even though they implement post SPC-2 features (such as thin
provisioning) which means the Linux kernel does not go on to test for those features even though they are advertised.
A previous patch attempted to add a quirk to workaround this but the quirk was only enabled after the features had been scanned for, wouldn't work for "small" disks and would quirk on all Hyper-V SCSI devices (e.g. passthrough disks).
The new patches partially revert the previous effort, add the quirk in a more traditional manner to only Hyper-V virtual disks and work on small virtual disks.

Quoted 6 lines

Quoted 2 lines
This seems like might want a quirk to simply "force" a SPC3 compliance level?
This was initially suggested, but rejected by Microsoft because of other problems advertising SPC-3 compliance brings. Perhaps the hyper-v emulator has matured sufficiently that it will now work OK?

James,

On the current release of Windows (windows 10), we are advertising SPC3 compliance. We are ok with declaring compliance to SPC3 in our drivers.

Regards,

K. Y

Jeff Leung

2014-10-11 20:22 UTC #11 lore raw

Quoted 2 lines

On the current release of Windows (windows 10), we are advertising SPC3 compliance. We are ok with declaring compliance to SPC3 in our drivers.

If you are going to declare SPC3 compliance in the drivers, are you going to put in checks to ensure that SPC-3 compliance doesn't get accidentally enabled for hypervisors below Win10?

I do know for a fact that Ubuntu's kernels already force SPC3 compliance to enable specific features such as TRIM on earlier versions of Hyper-V (Namely Hyper-V 2012 and 2012 R2).

--Jeff

Quoted 8 lines

Regards,
K. Y
NryزXvؖ){nǉ{zX}zj:v zZzf~zwڢ)jyA
i

_______________________________________________
devel mailing list
[email protected]
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

KY Srinivasan

2014-10-12 01:21 UTC #12 lore raw

Quoted 13 lines

-----Original Message-----
From: Jeff Leung [mailto:[email protected]]
Sent: Saturday, October 11, 2014 1:22 PM
To: KY Srinivasan; James Bottomley; Christoph Hellwig
Cc: Sitsofe Wheeler; Haiyang Zhang; Christoph Hellwig; Hannes Reinecke; [email protected]; [email protected]; [email protected]
Subject: RE: [PATCH 0/3] scsi: Add Hyper-V logical block provisioning quirks
Quoted 1 line
On the current release of Windows (windows 10), we are advertising SPC3
compliance.

Quoted 8 lines

Quoted 1 line
We are ok with declaring compliance to SPC3 in our drivers.
If you are going to declare SPC3 compliance in the drivers, are you going to put in checks to ensure that SPC-3 compliance doesn't get accidentally enabled for hypervisors below Win10?
I do know for a fact that Ubuntu's kernels already force SPC3 compliance to enable specific features such as TRIM on earlier versions of Hyper-V (Namely Hyper-V 2012 and 2012 R2).

You are right; Ubuntu has been carrying a patch that was doing just this and this has been working without any issues on many earlier versions of Windows. (2012 and 2012 R2). On windows 10 we don't need any changes in the Linux driver as the host itself is advertising SPC3 compliance. Based on the testing we have done with Ubuntu, we are comfortable picking up that patch.

Regards,

K. Y

Quoted 11 lines

--Jeff
Quoted 8 lines
Regards,
K. Y
NryزXvؖ){nǉ{zX}zj:v zZzf~zwڢ)jyA
i

_______________________________________________
devel mailing list
[email protected]
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Sitsofe Wheeler

author

2014-10-21 04:45 UTC #13 lore raw

Quoted 15 lines · KY Srinivasan

Quoted 8 lines
-----Original Message-----
From: Jeff Leung [mailto:[email protected]]
Sent: Saturday, October 11, 2014 1:22 PM
Quoted 3 lines
On the current release of Windows (windows 10), we are advertising SPC3 compliance.
We are ok with declaring compliance to SPC3 in our drivers.
Quoted 7 lines
If you are going to declare SPC3 compliance in the drivers, are you going to put in checks to ensure that SPC-3 compliance doesn't get accidentally enabled for hypervisors below Win10?
I do know for a fact that Ubuntu's kernels already force SPC3 compliance to enable specific features such as TRIM on earlier versions of Hyper-V (Namely Hyper-V 2012 and 2012 R2).

Quoted 6 lines

You are right; Ubuntu has been carrying a patch that was doing just this and this has been working without any issues on many earlier versions of Windows. (2012 and 2012 R2). On windows 10 we don't need any changes in the Linux driver as the host itself is advertising SPC3 compliance. Based on the testing we have done with Ubuntu, we are comfortable picking up that patch.

OK this seems to be the patch currently carried by Ubuntu:

From ff2c5fa3fa9adf0b919b9425e71a8ba044c31a7d Mon Sep 17 00:00:00 2001
From: Andy Whitcroft <[email protected]>
Date: Fri, 13 Sep 2013 17:49:16 +0100
Subject: [PATCH] scsi: hyper-v storsvc switch up to SPC-3

Signed-off-by Andy Whitcroft <[email protected]>

---
 drivers/scsi/storvsc_drv.c |    8 ++++++++
 1 file changed, 8 insertions(+)

drivers/scsi/storvsc_drv.c +8 -0

index 9969fa1..3903c8a 100644
@@ -1441,6 +1441,14 @@ static int storvsc_device_configure(struct scsi_device *sdevice)
 
 	sdevice->no_write_same = 1;
 
+	/*
+	 * hyper-v lies about its capabilities indicating it is only SPC-2
+	 * compliant, but actually implements the core SPC-3 features.
+	 * If we pretend to be SPC-3, we send RC16 which activates trim and
+	 * will query the appropriate VPD pages to enable trim.
+	 */
+	sdevice->scsi_level = SCSI_SPC_3;
+
 	return 0;
 }

Signature · 27 lines

1.7.9.5

(Downloaded from
http://kernel.ubuntu.com/git?p=jsalisbury/stable/trusty/ubuntu-trusty.git;a=patch;h=ff2c5fa3fa9adf0b919b9425e71a8ba044c31a7d
).

I think it's unwise to override the scsi_level at this particular point
because you are going to do it for ALL Hyper-V "disks" (perhaps all
Hyper-V SCSI devices?)...

Here's the SCSI inquiry information reported by a USB 2 hard disk being
passed passed-through by one of my 2012 R2 hosts:

# sg_inq /dev/sdc
standard INQUIRY:
  PQual=0  Device_type=0  RMB=0  version=0x02  [SCSI-2]
  [AERC=0]  [TrmTsk=0]  NormACA=0  HiSUP=0  Resp_data_format=1
  SCCS=0  ACC=0  TPGS=0  3PC=0  Protect=0  [BQue=0]
  EncServ=0  MultiP=0  [MChngr=0]  [ACKREQQ=0]  Addr16=0
  [RelAdr=0]  WBus16=0  Sync=0  Linked=0  [TranDis=0]  CmdQue=0
    length=36 (0x24)   Peripheral device type: disk
 Vendor identification: MDT MD50
 Product identification: 00AAKS-00TMA0   
 Product revision level: 

Is it OK to replace a scsi_level of SCSI-2 with SCSI_SPC_3? Additionally
is it also OK to force SCSI_SPC_3 on Hyper-V 2008?

Quoted 4 lines

Quoted 3 lines
Quoted 3 lines
NryزXvؖ){nǉ{zX}zj:v zZzf~zwڢ)jyA
i
N?????r??y????b?X??ǧv?^?)޺{.n?+????{????zX????ܨ}???Ơz?&j:+v???????zZ+??+zf???h???~????i???z??w?????????&?)ߢf??^jǫy?m??@A?a???0??h???i

^^^ Where do these characters come from? I've occasionally seen them on emails from other Microsoft folks posting to LKML too...

Signature · 5 lines

Sitsofe | http://sucs.org/~sits/
_______________________________________________
devel mailing list
[email protected]
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Jeff Leung

2014-10-21 05:19 UTC #14 lore raw

Quoted 2 lines

Is it OK to replace a scsi_level of SCSI-2 with SCSI_SPC_3? Additionally is it also OK to force SCSI_SPC_3 on Hyper-V 2008?

I would patch the driver accordingly to force the SPC-3 flag.

For a Win2k8 host, I don't know what the side effects are, so it's safe to say it's not a good idea to Force SCSI_SPC_3

KY Srinivasan

2014-10-21 18:30 UTC #15 lore raw

In reply to Sitsofe Wheeler

Quoted 71 lines

-----Original Message-----
From: Sitsofe Wheeler [mailto:[email protected]]
Sent: Monday, October 20, 2014 9:46 PM
To: KY Srinivasan
Cc: Jeff Leung; James Bottomley; Christoph Hellwig; Haiyang Zhang; Christoph Hellwig; Hannes Reinecke; [email protected]; linux- [email protected]; [email protected]
Subject: Re: [PATCH 0/3] scsi: Add Hyper-V logical block provisioning quirks
Quoted 15 lines · KY Srinivasan
Quoted 8 lines
-----Original Message-----
From: Jeff Leung [mailto:[email protected]]
Sent: Saturday, October 11, 2014 1:22 PM
Quoted 3 lines
On the current release of Windows (windows 10), we are advertising SPC3 compliance.
We are ok with declaring compliance to SPC3 in our drivers.
Quoted 7 lines
If you are going to declare SPC3 compliance in the drivers, are you going to put in checks to ensure that SPC-3 compliance doesn't get accidentally enabled for hypervisors below Win10?
I do know for a fact that Ubuntu's kernels already force SPC3 compliance to enable specific features such as TRIM on earlier versions of Hyper-V (Namely Hyper-V 2012 and 2012 R2).
Quoted 6 lines
You are right; Ubuntu has been carrying a patch that was doing just this and this has been working without any issues on many earlier versions of Windows. (2012 and 2012 R2). On windows 10 we don't need any changes in the Linux driver as the host itself is advertising SPC3 compliance. Based on the testing we have done with Ubuntu, we are comfortable picking up that patch.
OK this seems to be the patch currently carried by Ubuntu:
From ff2c5fa3fa9adf0b919b9425e71a8ba044c31a7d Mon Sep 17 00:00:00 2001
From: Andy Whitcroft <[email protected]>
Date: Fri, 13 Sep 2013 17:49:16 +0100
Subject: [PATCH] scsi: hyper-v storsvc switch up to SPC-3
Suggested-By: James Bottomley
<[email protected]>
Signed-off-by Andy Whitcroft <[email protected]>
---
 drivers/scsi/storvsc_drv.c |    8 ++++++++
 1 file changed, 8 insertions(+)
drivers/scsi/storvsc_drv.c index renamed from drivers/scsi/storvsc_drv.c +0 -0
9969fa1..3903c8a 100644
drivers/scsi/storvsc_drv.c +8 -1
@@ -1441,6 +1441,14 @@ static int storvsc_device_configure(struct
 scsi_device *sdevice)
 
 	sdevice->no_write_same = 1;
 
+	/*
+	 * hyper-v lies about its capabilities indicating it is only SPC-2
+	 * compliant, but actually implements the core SPC-3 features.
+	 * If we pretend to be SPC-3, we send RC16 which activates trim and
+	 * will query the appropriate VPD pages to enable trim.
+	 */
+	sdevice->scsi_level = SCSI_SPC_3;
+
 	return 0;
 }
 
--
1.7.9.5

Quoted 8 lines

(Downloaded from
http://kernel.ubuntu.com/git?p=jsalisbury/stable/trusty/ubuntu-
trusty.git;a=patch;h=ff2c5fa3fa9adf0b919b9425e71a8ba044c31a7d ).
I think it's unwise to override the scsi_level at this particular point because you are going to do it for ALL Hyper-V "disks" (perhaps all Hyper-V SCSI devices?)...

I agree with you. I was only referring virtual hard disks (VHDs) being presented to the guest and not pass-through devices. Furthermore, we would fix up the scsi conformance level based on the version of the host we are running on.

Regards,

K. Y

Quoted 26 lines

Here's the SCSI inquiry information reported by a USB 2 hard disk being passed passed-through by one of my 2012 R2 hosts:
# sg_inq /dev/sdc
standard INQUIRY:
  PQual=0  Device_type=0  RMB=0  version=0x02  [SCSI-2]
  [AERC=0]  [TrmTsk=0]  NormACA=0  HiSUP=0  Resp_data_format=1
  SCCS=0  ACC=0  TPGS=0  3PC=0  Protect=0  [BQue=0]
  EncServ=0  MultiP=0  [MChngr=0]  [ACKREQQ=0]  Addr16=0
  [RelAdr=0]  WBus16=0  Sync=0  Linked=0  [TranDis=0]  CmdQue=0
    length=36 (0x24)   Peripheral device type: disk
 Vendor identification: MDT MD50
 Product identification: 00AAKS-00TMA0
 Product revision level:
Is it OK to replace a scsi_level of SCSI-2 with SCSI_SPC_3? Additionally is it also OK to force SCSI_SPC_3 on Hyper-V 2008?
Quoted 4 lines
Quoted 3 lines
Quoted 3 lines
NryزXvؖ){nǉ{zX}zj:v zZzf~zwڢ)jyA
i
N?????r??y????b?X??ǧv?^?)޺{.n?+????{????zX????ܨ}???Ơz?&j:+v???
????zZ+??+zf???h???~????i???z??w?????????&?)ߢf??^jǫy?m??@A?a?? ?

Quoted 7 lines

0??h???i
^^^ Where do these characters come from? I've occasionally seen them on emails from other Microsoft folks posting to LKML too...
--
Sitsofe | http://sucs.org/~sits/

_______________________________________________
devel mailing list
[email protected]
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Martin K. Petersen

2014-10-15 01:06 UTC #16 lore raw

In reply to Sitsofe Wheeler

Quoted 1 line

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Sitsofe" == Sitsofe Wheeler <[email protected]> writes:

Sitsofe> A previous patch attempted to add a quirk to workaround this Sitsofe> but the quirk was only enabled after the features had been Sitsofe> scanned for, wouldn't work for "small" disks

What does that mean, exactly?

Signature · 1 line

Martin K. Petersen	Oracle Linux Engineering

Sitsofe Wheeler

author

2014-10-21 04:17 UTC #17 lore raw

Quoted 7 lines · Martin K. Petersen

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Sitsofe" == Sitsofe Wheeler <[email protected]> writes:
Sitsofe> A previous patch attempted to add a quirk to workaround this Sitsofe> but the quirk was only enabled after the features had been Sitsofe> scanned for, wouldn't work for "small" disks
What does that mean, exactly?

It means:

1. The committed patches never worked because the running of the code to test whether the quirk should be enabled happened after the probing code the quirk would have affected ran. So roughly:

bflag = 0;

if (feature || bflag) {
	do_stuff();
}
if (matching_dev) {
	bflag = 1; // Too late...
}

See https://lkml.org/lkml/2014/7/23/615 for prior details.

2. On top of the above, when a disk is "small" (has less than 2^32 sectors which is typically < 2 TBytes in size) READ CAPACITY(16) won't be triggered. If READ CAPACITY(16) isn't triggered then the lbpme bytes won't be checked, thin provisioning will never be enabled and the committed patch would doubly not work for such disks.

Apologies for the delay in replying.

Signature · 1 line

Sitsofe | http://sucs.org/~sits/

Martin K. Petersen

2014-10-23 01:50 UTC #18 lore raw

Quoted 1 line

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Sitsofe" == Sitsofe Wheeler <[email protected]> writes:

Sitsofe> 2. On top of the above, when a disk is "small" (has less than
Sitsofe> 2^32 sectors which is typically < 2 TBytes in size) READ
Sitsofe> CAPACITY(16) won't be triggered.

static int sd_try_rc16_first(struct scsi_device *sdp)

{
        if (sdp->host->max_cmd_len < 16)
                return 0;
        if (sdp->try_rc_10_first)
                return 0;
        if (sdp->scsi_level > SCSI_SPC_2)
                return 1;
        if (scsi_device_protection(sdp))
                return 1;
        return 0;
}

Signature · 1 line

Martin K. Petersen	Oracle Linux Engineering

Sitsofe Wheeler

author

2014-10-23 09:03 UTC #19 lore raw

Quoted 7 lines · Martin K. Petersen

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Sitsofe" == Sitsofe Wheeler <[email protected]> writes:
Sitsofe> 2. On top of the above, when a disk is "small" (has less than
Sitsofe> 2^32 sectors which is typically < 2 TBytes in size) READ
Sitsofe> CAPACITY(16) won't be triggered.
static int sd_try_rc16_first(struct scsi_device *sdp)

Quoted 11 lines

{
        if (sdp->host->max_cmd_len < 16)
                return 0;
        if (sdp->try_rc_10_first)
                return 0;
        if (sdp->scsi_level > SCSI_SPC_2)
                return 1;
        if (scsi_device_protection(sdp))
                return 1;
        return 0;
}

Yes but since SCSI_SPC_2 was being advertised by the Hyper-V virtual disk (and presumably device protection isn't advertised either) this function was returning 0. That's why the patch in
https://lkml.org/lkml/2014/10/10/49 added yet another quirk. If if SPC_3 were correctly advertised on Hyper-V virtual disks would TRY_VPD_PAGES still be needed to cope correctly with passthrough disks?

What I didn't realise was that the TRY_VPD_PAGES quirk was purely to fix WRITE_SAME Hyper-V issues and not to address anything thin provisioning related. Having said that, why was the TRY_VPD_PAGES quirk back-ported 3.14 without any users -
https://lkml.org/lkml/2014/9/15/733 ?

Signature · 1 line

Sitsofe | http://sucs.org/~sits/