2014-10-10 07:49:10

by Sitsofe Wheeler

[permalink] [raw]
Subject: [PATCH 0/3] 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(-)

--
1.9.3


2014-10-10 07:51:17

by Sitsofe Wheeler

[permalink] [raw]
Subject: [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(-)

diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
index 733e5f7..2cc094e 100644
--- a/drivers/scsi/storvsc_drv.c
+++ b/drivers/scsi/storvsc_drv.c
@@ -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;
}

--
1.9.3

2014-10-10 07:52:59

by Sitsofe Wheeler

[permalink] [raw]
Subject: [PATCH 2/3] scsi: add try_rc16 blacklist flag

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(+)

diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index ba3f1e8..d3f6267 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -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) {
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 0cb5c9f..0ccf372 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -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;
diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
index 27ecee7..d6e2bd8 100644
--- a/include/scsi/scsi_device.h
+++ b/include/scsi/scsi_device.h
@@ -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 */
diff --git a/include/scsi/scsi_devinfo.h b/include/scsi/scsi_devinfo.h
index 183eaab..9431f5e 100644
--- a/include/scsi/scsi_devinfo.h
+++ b/include/scsi/scsi_devinfo.h
@@ -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
--
1.9.3

2014-10-10 07:55:08

by Sitsofe Wheeler

[permalink] [raw]
Subject: [PATCH 3/3] scsi: Use try_rc16 and try_vpd_pages quirks on Hyper-V virtual disks

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(+)

diff --git a/drivers/scsi/scsi_devinfo.c b/drivers/scsi/scsi_devinfo.c
index 49014a1..3eadcb1 100644
--- a/drivers/scsi/scsi_devinfo.c
+++ b/drivers/scsi/scsi_devinfo.c
@@ -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},
--
1.9.3

2014-10-11 17:39:10

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH 0/3] scsi: Add Hyper-V logical block provisioning quirks

On Fri, Oct 10, 2014 at 08:49:01AM +0100, Sitsofe Wheeler wrote:
> 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.

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

2014-10-11 17:41:58

by James Bottomley

[permalink] [raw]
Subject: Re: [PATCH 0/3] scsi: Add Hyper-V logical block provisioning quirks

On Sat, 2014-10-11 at 10:39 -0700, Christoph Hellwig wrote:
> On Fri, Oct 10, 2014 at 08:49:01AM +0100, Sitsofe Wheeler wrote:
> > 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.
>
> 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

2014-10-11 20:02:34

by KY Srinivasan

[permalink] [raw]
Subject: RE: [PATCH 0/3] scsi: Add Hyper-V logical block provisioning quirks



> -----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
>
> On Sat, 2014-10-11 at 10:39 -0700, Christoph Hellwig wrote:
> > On Fri, Oct 10, 2014 at 08:49:01AM +0100, Sitsofe Wheeler wrote:
> > > 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.
> >
> > 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


????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2014-10-11 20:31:19

by Jeff Leung

[permalink] [raw]
Subject: RE: [PATCH 0/3] scsi: Add Hyper-V logical block provisioning quirks

> 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

> Regards,
>
> K. Y
>
>
> NryزXvؖ){nlj{zX}zj:v zZzf~zwڢ)jyA
>
> i
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2014-10-12 01:21:10

by KY Srinivasan

[permalink] [raw]
Subject: RE: [PATCH 0/3] scsi: Add Hyper-V logical block provisioning quirks



> -----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
>
> > 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).

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
>
> --Jeff
>
> > Regards,
> >
> > K. Y
> >
> >
> > NryزXvؖ){nlj{zX}zj:v zZzf~zwڢ)jyA
> >
> > i
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2014-10-15 01:07:00

by Martin K. Petersen

[permalink] [raw]
Subject: Re: [PATCH 0/3] scsi: Add Hyper-V logical block provisioning quirks

>>>>> "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?

--
Martin K. Petersen Oracle Linux Engineering

2014-10-15 01:08:43

by Martin K. Petersen

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

>>>>> "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.

--
Martin K. Petersen Oracle Linux Engineering

2014-10-15 02:07:26

by KY Srinivasan

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



> -----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
>
> >>>>> "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.

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

K. Y
>
> --
> Martin K. Petersen Oracle Linux Engineering

2014-10-21 04:17:19

by Sitsofe Wheeler

[permalink] [raw]
Subject: Re: [PATCH 0/3] scsi: Add Hyper-V logical block provisioning quirks

On Tue, Oct 14, 2014 at 09:06:37PM -0400, Martin K. Petersen wrote:
> >>>>> "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.

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

2014-10-21 04:21:14

by Sitsofe Wheeler

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

On Tue, Oct 14, 2014 at 09:08:28PM -0400, Martin K. Petersen wrote:
> >>>>> "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?

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

2014-10-21 04:45:56

by Sitsofe Wheeler

[permalink] [raw]
Subject: Re: [PATCH 0/3] scsi: Add Hyper-V logical block provisioning quirks

On Sun, Oct 12, 2014 at 01:21:01AM +0000, KY Srinivasan wrote:
>
> > -----Original Message-----
> > From: Jeff Leung [mailto:[email protected]]
> > Sent: Saturday, October 11, 2014 1:22 PM
> >
> > > 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).
> 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(+)

diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
index 9969fa1..3903c8a 100644
--- a/drivers/scsi/storvsc_drv.c
+++ b/drivers/scsi/storvsc_drv.c
@@ -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

(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?

> > > NryزXvؖ){nlj{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...

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

2014-10-21 05:19:39

by Jeff Leung

[permalink] [raw]
Subject: RE: [PATCH 0/3] scsi: Add Hyper-V logical block provisioning quirks

> 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
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2014-10-21 18:30:37

by KY Srinivasan

[permalink] [raw]
Subject: RE: [PATCH 0/3] scsi: Add Hyper-V logical block provisioning quirks



> -----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
>
> On Sun, Oct 12, 2014 at 01:21:01AM +0000, KY Srinivasan wrote:
> >
> > > -----Original Message-----
> > > From: Jeff Leung [mailto:[email protected]]
> > > Sent: Saturday, October 11, 2014 1:22 PM
> > >
> > > > 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).
> > 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(+)
>
> diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c index
> 9969fa1..3903c8a 100644
> --- a/drivers/scsi/storvsc_drv.c
> +++ b/drivers/scsi/storvsc_drv.c
> @@ -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
>
> (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
>
> 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?
>
> > > > NryزXvؖ){nlj{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...
>
> --
> Sitsofe | http://sucs.org/~sits/
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2014-10-23 01:46:55

by Martin K. Petersen

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

>>>>> "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.

--
Martin K. Petersen Oracle Linux Engineering

2014-10-23 01:50:40

by Martin K. Petersen

[permalink] [raw]
Subject: Re: [PATCH 0/3] scsi: Add Hyper-V logical block provisioning quirks

>>>>> "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;
}

--
Martin K. Petersen Oracle Linux Engineering

2014-10-23 09:03:35

by Sitsofe Wheeler

[permalink] [raw]
Subject: Re: [PATCH 0/3] scsi: Add Hyper-V logical block provisioning quirks

On 23 October 2014 02:50, Martin K. Petersen <[email protected]> wrote:
>>>>>> "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;
> }

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 ?

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