[V3,01/16] scsi: support well known logical units

Commit Message

Dolev Raviv Sept. 10, 2014, 11:54 a.m. UTC

From: Subhash Jadavani <[e-mail]>

REPORT LUNS command has "SELECT REPORT" field which controls what type of
logical units to be reported by device server. According to UFS device
standard, if this field is set to 0, REPORT LUNS would report only report
standard logical units. If it's set to 1 then it would report only well
known logical unit and if it's set to 2 then device would report both
standard and well known logical units.

Signed-off-by: Subhash Jadavani <[e-mail]>
Signed-off-by: Sujit Reddy Thumma <[e-mail]>
Signed-off-by: Dolev Raviv <[e-mail]>

Comments

Christoph Hellwig Sept. 10, 2014, 6:58 p.m. UTC | #1

On Wed, Sep 10, 2014 at 02:54:08PM +0300, Dolev Raviv wrote:
> From: Subhash Jadavani <[e-mail]>
> 
> REPORT LUNS command has "SELECT REPORT" field which controls what type of
> logical units to be reported by device server. According to UFS device
> standard, if this field is set to 0, REPORT LUNS would report only report
> standard logical units. If it's set to 1 then it would report only well
> known logical unit and if it's set to 2 then device would report both
> standard and well known logical units.

This is the defintion of the field in SPC (does the UFS spec duplicate
all of SPC?), but still doesn't explain why you want it.

Also the SELECT REPORT field only appeared in SPC-3, so we should not
set it for devices that report older standards compliance.

--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[e-mail] Sept. 12, 2014, 12:41 a.m. UTC | #2

> This is the defintion of the field in SPC (does the UFS spec duplicate all
of SPC?), 
Yes, UFS device specification (both 1.1 and 2.0) supports
[SAM], INCITS T10 draft standard: SCSI Architecture Model - 5 (SAM-5),
Revision 05, 19 May 2010  
[SPC], INCITS T10 draft standard: SCSI Primary Commands - 4 (SPC-4),
Revision 27, 11 October 2010 
[SBC], INCITS T10 draft standard: SCSI Block Commands - 3 (SBC-3), Revision
24, 05 August 2010  

In addition, this is what specification says explicitly " SCSI SBC and SPC
commands are the baseline for UFS. UFS will not modify the SBC and SPC
Compliant commands.  The goal is to maximize re-use and leverage of the
software codebase available on platforms (PC, netbook, MID) that are already
supporting SCSI."

> but still doesn't explain why you want it.

UFS device has supports 4 different well known logical units: "REPORT_LUNS"
(address: 01h), "UFS Device" (address: 50h), "RPMB" (address: 44h) and
"BOOT" (address: 30h).

UFS device's power management needs to be controlled by "POWER CONDITION"
field of SSU (START STOP UNIT) command. But this "power condition"  field
will take effect only when its sent to "UFS device" well known logical unit
(address: 50h) hence we require the scsi_device instance to represent this
logical unit in order for the UFS host driver to send the SSU command for
power management.

We also require the scsi_device instance for "RPMB" (Replay Protected Memory
Block) LU so user space process can control this LU.

> Also the SELECT REPORT field only appeared in SPC-3, so we should not set
it for devices that report older standards compliance.

Thanks for pointing this out, was not aware of this. We will add appropriate
check (if (sdev->scsi_level >= SCSI_SPC_3))  before setting the SELECT
REPORT field.

Regards,
Subhash

-----Original Message-----
From: [e-mail]
[mailto:[e-mail]] On Behalf Of Christoph Hellwig
Sent: Wednesday, September 10, 2014 11:59 AM
To: Dolev Raviv
Cc: [e-mail]; [e-mail];
[e-mail]; [e-mail];
[e-mail]; [e-mail]; Subhash Jadavani; Sujit
Reddy Thumma; Martin K. Petersen
Subject: Re: [PATCH V3 01/16] scsi: support well known logical units

On Wed, Sep 10, 2014 at 02:54:08PM +0300, Dolev Raviv wrote:
> From: Subhash Jadavani <[e-mail]>
> 
> REPORT LUNS command has "SELECT REPORT" field which controls what type 
> of logical units to be reported by device server. According to UFS 
> device standard, if this field is set to 0, REPORT LUNS would report 
> only report standard logical units. If it's set to 1 then it would 
> report only well known logical unit and if it's set to 2 then device 
> would report both standard and well known logical units.

This is the defintion of the field in SPC (does the UFS spec duplicate all
of SPC?), but still doesn't explain why you want it.

Also the SELECT REPORT field only appeared in SPC-3, so we should not set it
for devices that report older standards compliance.

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

--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Christoph Hellwig Sept. 13, 2014, 6:54 p.m. UTC | #3

On Thu, Sep 11, 2014 at 05:41:18PM -0700, Subhash Jadavani wrote:
> 
> UFS device has supports 4 different well known logical units: "REPORT_LUNS"
> (address: 01h), "UFS Device" (address: 50h), "RPMB" (address: 44h) and
> "BOOT" (address: 30h).
> 
> UFS device's power management needs to be controlled by "POWER CONDITION"
> field of SSU (START STOP UNIT) command. But this "power condition"  field
> will take effect only when its sent to "UFS device" well known logical unit
> (address: 50h) hence we require the scsi_device instance to represent this
> logical unit in order for the UFS host driver to send the SSU command for
> power management.
> 
> We also require the scsi_device instance for "RPMB" (Replay Protected Memory
> Block) LU so user space process can control this LU.

If those are the only LUs you specificly need I'd suggest you just
manually call scsi_add_device from your driver for those instead of
listing them in REPORT_LUNS and making them part of the normal LUN
scan.  One advantage of the well known LUNs is that you always know
where in the LUN namespace they are :)

--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[e-mail] Sept. 18, 2014, 5:18 p.m. UTC | #4

> If those are the only LUs you specificly need I'd suggest you just
manually call scsi_add_device from your driver for those instead of listing
them in REPORT_LUNS and making them part of the normal LUN scan. 

Yes, we can do that but not sure what would be advantage of making the LLDs
to add these ad-hoc hooks instead of scsi_scan handle it on its own and
leaving the LLDs from adding them explicitly? Do you foresee any issues
(other than few extra scsi_device instances which may not be useful all scsi
devices) with scsi_scan scanning all the LUs (including well known LUs)?

Regards,
Subhash

-----Original Message-----
From: [e-mail]
[mailto:[e-mail]] On Behalf Of 'Christoph Hellwig'
Sent: Saturday, September 13, 2014 11:55 AM
To: Subhash Jadavani
Cc: 'Christoph Hellwig'; 'Dolev Raviv';
[e-mail]; [e-mail];
[e-mail]; [e-mail];
[e-mail]; 'Sujit Reddy Thumma'; 'Martin K. Petersen'
Subject: Re: [PATCH V3 01/16] scsi: support well known logical units

On Thu, Sep 11, 2014 at 05:41:18PM -0700, Subhash Jadavani wrote:
> 
> UFS device has supports 4 different well known logical units:
"REPORT_LUNS"
> (address: 01h), "UFS Device" (address: 50h), "RPMB" (address: 44h) and 
> "BOOT" (address: 30h).
> 
> UFS device's power management needs to be controlled by "POWER CONDITION"
> field of SSU (START STOP UNIT) command. But this "power condition"  
> field will take effect only when its sent to "UFS device" well known 
> logical unit
> (address: 50h) hence we require the scsi_device instance to represent 
> this logical unit in order for the UFS host driver to send the SSU 
> command for power management.
> 
> We also require the scsi_device instance for "RPMB" (Replay Protected 
> Memory
> Block) LU so user space process can control this LU.

If those are the only LUs you specificly need I'd suggest you just manually
call scsi_add_device from your driver for those instead of listing them in
REPORT_LUNS and making them part of the normal LUN scan.  One advantage of
the well known LUNs is that you always know where in the LUN namespace they
are :)

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

--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[e-mail] Sept. 18, 2014, 7:12 p.m. UTC | #5

Hi Chris,

While testing with another vendor's UFS devices, I realized that SELECT
REPORT need not to be set to 0x02 for making the device report the W-LUs.
Even when SELECT REPORT is set 0x00, this particular UFS device reports the
W-LUs so I looked at the SCSI specification in details and it seems ideally
device should report all the well-known logical units (as it follows the
extended logical unit addressing format) when SELECT REPORT is set 0x00.
Which means one vendor's UFS devices are following SCSI spec for W-LUs
reporting while another vendor's UFS devices don't.

I have reported violation of spec to this other vendor and have also asked
them to fix it. Until they fix their non-standard behavior, we can
workaround their behavior using the device specific quirks in UFS driver
itself.

For now in this patch, we will skip modifying SELECT REPORT field but we
would still require the other part of this patch for fixing the scsi_device
type to wlun (sdev->type = TYPE_WLUN). Hence we will change the commit text
accordingly to reflect the exact purpose of the change.

Regards,
Subhash

-----Original Message-----
From: [e-mail]
[mailto:[e-mail]] On Behalf Of Subhash Jadavani
Sent: Thursday, September 18, 2014 10:18 AM
To: 'Christoph Hellwig'
Cc: 'Dolev Raviv'; [e-mail];
[e-mail]; [e-mail];
[e-mail]; [e-mail]; 'Sujit Reddy Thumma';
'Martin K. Petersen'
Subject: RE: [PATCH V3 01/16] scsi: support well known logical units

> If those are the only LUs you specificly need I'd suggest you just
manually call scsi_add_device from your driver for those instead of listing
them in REPORT_LUNS and making them part of the normal LUN scan. 

Yes, we can do that but not sure what would be advantage of making the LLDs
to add these ad-hoc hooks instead of scsi_scan handle it on its own and
leaving the LLDs from adding them explicitly? Do you foresee any issues
(other than few extra scsi_device instances which may not be useful all scsi
devices) with scsi_scan scanning all the LUs (including well known LUs)?

Regards,
Subhash

-----Original Message-----
From: [e-mail]
[mailto:[e-mail]] On Behalf Of 'Christoph Hellwig'
Sent: Saturday, September 13, 2014 11:55 AM
To: Subhash Jadavani
Cc: 'Christoph Hellwig'; 'Dolev Raviv';
[e-mail]; [e-mail];
[e-mail]; [e-mail];
[e-mail]; 'Sujit Reddy Thumma'; 'Martin K. Petersen'
Subject: Re: [PATCH V3 01/16] scsi: support well known logical units

On Thu, Sep 11, 2014 at 05:41:18PM -0700, Subhash Jadavani wrote:
> 
> UFS device has supports 4 different well known logical units:
"REPORT_LUNS"
> (address: 01h), "UFS Device" (address: 50h), "RPMB" (address: 44h) and 
> "BOOT" (address: 30h).
> 
> UFS device's power management needs to be controlled by "POWER CONDITION"
> field of SSU (START STOP UNIT) command. But this "power condition"  
> field will take effect only when its sent to "UFS device" well known 
> logical unit
> (address: 50h) hence we require the scsi_device instance to represent 
> this logical unit in order for the UFS host driver to send the SSU 
> command for power management.
> 
> We also require the scsi_device instance for "RPMB" (Replay Protected 
> Memory
> Block) LU so user space process can control this LU.

If those are the only LUs you specificly need I'd suggest you just manually
call scsi_add_device from your driver for those instead of listing them in
REPORT_LUNS and making them part of the normal LUN scan.  One advantage of
the well known LUNs is that you always know where in the LUN namespace they
are :)

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

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

--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Christoph Hellwig Sept. 22, 2014, 2:28 p.m. UTC | #6

On Thu, Sep 18, 2014 at 12:12:22PM -0700, Subhash Jadavani wrote:
> Hi Chris,
> 
> While testing with another vendor's UFS devices, I realized that SELECT
> REPORT need not to be set to 0x02 for making the device report the W-LUs.
> Even when SELECT REPORT is set 0x00, this particular UFS device reports the
> W-LUs so I looked at the SCSI specification in details and it seems ideally
> device should report all the well-known logical units (as it follows the
> extended logical unit addressing format) when SELECT REPORT is set 0x00.

I don't really know devices exporting wluns by default. The wording in
the latests SPC-4 draft I have says:

00h	The list shall contain the logical units accessible to the I_T nexus
	with the following addressing methods (see SAM-4):
	  a) Logical unit addressing method,
	  b) Peripheral device addressing method; and
	  c) Flat space addressing method.
	If there are no logical units, the LUN LIST LENGTH field shall be
	zero.
01h	The list shall contain only well known logical units, if any.
	If there are no well known logical units, the LUN LIST LENGTH field
	shall be zero.
02h The list shall contain all logical units accessible to the I_T nexus.


SAM-4 defines WLUNs as their own addressing format in 4.6.11:

Well known logical units are addressed using the well known logical unit
extended address format (see table 28).


Note that the reason I don't like reporting wluns by default is that
so far we haven't come up with a use case for them in either the
kernel or generic userspace.  As long as usage of the wluns you want
to support is confirmed to ufs, which is an unusually rightly coupled
implementation of the initiator and target side I'd like to keep it
isolated to ufs.  If we ever come up with a more generic usage of
other wluns we should also report them during the initial target scan.

> For now in this patch, we will skip modifying SELECT REPORT field but we
> would still require the other part of this patch for fixing the scsi_device
> type to wlun (sdev->type = TYPE_WLUN). Hence we will change the commit text
> accordingly to reflect the exact purpose of the change.

Yes, please resend this change as a separate patch with an updated
description.  I'd love to merge the whole ufs update for this merge
window, so a timeline resent would be appreciated.

--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[e-mail] Sept. 23, 2014, 9:40 p.m. UTC | #7

Hi Chris,

> I don't really know devices exporting wluns by default. The wording in the
latests SPC-4 draft I have says:

That's true. In fact further finding showed me this: UFS device
specification has explicitly mentioned the purpose of SELECT REPORT field
when its set to 00h. When SELECT REPORT field is cleared, device should only
report LUs which support peripheral device addressing method. Even UFS
conformance test specification seems to confirm this explicitly. Hence if
the UFS device adhere the UFS device specification then its expected for
device to not report the W-LUs when SELECT REPORT is 00h.

But I have seen one UFS device vendor specifically reporting w-lus even when
select report is cleared. I am going to follow up them to fix their
behavior. But for now, I guess we are on same page that device shouldn't
report W-LUS when SELECT REPORT field is cleared.


>> For now in this patch, we will skip modifying SELECT REPORT field but 
>> we would still require the other part of this patch for fixing the 
>> scsi_device type to wlun (sdev->type = TYPE_WLUN). Hence we will 
>> change the commit text accordingly to reflect the exact purpose of the
change.

> Yes, please resend this change as a separate patch with an updated
description.  I'd love to merge the whole ufs update for this merge window,
so a timeline resent would be appreciated.

Yes, rest of the part in this patch would still remain applicable with
updated description. We will send it soon along with one more patch in UFS
driver which would manually add scsi_device instance for the UFS W-LUs.

Regards,
Subhash


-----Original Message-----
From: [e-mail]
[mailto:[e-mail]] On Behalf Of 'Christoph Hellwig'
Sent: Monday, September 22, 2014 7:29 AM
To: Subhash Jadavani
Cc: 'Dolev Raviv'; [e-mail];
[e-mail]; [e-mail];
[e-mail]; [e-mail]; 'Sujit Reddy Thumma';
'Martin K. Petersen'
Subject: Re: [PATCH V3 01/16] scsi: support well known logical units

On Thu, Sep 18, 2014 at 12:12:22PM -0700, Subhash Jadavani wrote:
> Hi Chris,
> 
> While testing with another vendor's UFS devices, I realized that 
> SELECT REPORT need not to be set to 0x02 for making the device report the
W-LUs.
> Even when SELECT REPORT is set 0x00, this particular UFS device 
> reports the W-LUs so I looked at the SCSI specification in details and 
> it seems ideally device should report all the well-known logical units 
> (as it follows the extended logical unit addressing format) when SELECT
REPORT is set 0x00.

I don't really know devices exporting wluns by default. The wording in the
latests SPC-4 draft I have says:

00h	The list shall contain the logical units accessible to the I_T nexus
	with the following addressing methods (see SAM-4):
	  a) Logical unit addressing method,
	  b) Peripheral device addressing method; and
	  c) Flat space addressing method.
	If there are no logical units, the LUN LIST LENGTH field shall be
	zero.
01h	The list shall contain only well known logical units, if any.
	If there are no well known logical units, the LUN LIST LENGTH field
	shall be zero.
02h The list shall contain all logical units accessible to the I_T nexus.


SAM-4 defines WLUNs as their own addressing format in 4.6.11:

Well known logical units are addressed using the well known logical unit
extended address format (see table 28).


Note that the reason I don't like reporting wluns by default is that so far
we haven't come up with a use case for them in either the kernel or generic
userspace.  As long as usage of the wluns you want to support is confirmed
to ufs, which is an unusually rightly coupled implementation of the
initiator and target side I'd like to keep it isolated to ufs.  If we ever
come up with a more generic usage of other wluns we should also report them
during the initial target scan.

> For now in this patch, we will skip modifying SELECT REPORT field but 
> we would still require the other part of this patch for fixing the 
> scsi_device type to wlun (sdev->type = TYPE_WLUN). Hence we will 
> change the commit text accordingly to reflect the exact purpose of the
change.

Yes, please resend this change as a separate patch with an updated
description.  I'd love to merge the whole ufs update for this merge window,
so a timeline resent would be appreciated.

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

--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index 56675db..d7f0df8 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -805,6 +805,14 @@  static int scsi_add_lun(struct scsi_device *sdev, unsigned char *inq_result,
 	} else {
 		sdev->type = (inq_result[0] & 0x1f);
 		sdev->removable = (inq_result[1] & 0x80) >> 7;
+
+		/*
+		 * some devices may respond with wrong type for
+		 * well-known logical units. Force well-known type
+		 * to enumerate them correctly.
+		 */
+		if (scsi_is_wlun(sdev->lun))
+			sdev->type = TYPE_WLUN;
 	}
 
 	if (sdev->type == TYPE_RBC || sdev->type == TYPE_ROM) {
@@ -1400,6 +1408,12 @@  static int scsi_report_lun_scan(struct scsi_target *starget, int bflags,
 	memset(&scsi_cmd[1], 0, 5);
 
 	/*
+	 * Set "SELECT REPORT" field to 0x2 which will make device to
+	 * report well known logical units along with standard LUs.
+	 */
+	scsi_cmd[2] = 0x2;
+
+	/*
 	 * bytes 6 - 9: length of the command.
 	 */
 	scsi_cmd[6] = (unsigned char) (length >> 24) & 0xff;
diff --git a/include/scsi/scsi.h b/include/scsi/scsi.h
index 261e708..d17178e 100644
--- a/include/scsi/scsi.h
+++ b/include/scsi/scsi.h
@@ -333,6 +333,7 @@  static inline int scsi_status_is_good(int status)
 #define TYPE_RBC	    0x0e
 #define TYPE_OSD            0x11
 #define TYPE_ZBC            0x14
+#define TYPE_WLUN           0x1e    /* well-known logical unit */
 #define TYPE_NO_LUN         0x7f
 
 /* SCSI protocols; these are taken from SPC-3 section 7.5 */