On 4/5/23 01:27, Damien Le Moal wrote:
On 4/5/23 03:48, Hannes Reinecke wrote:
On 4/4/23 20:24, Niklas Cassel wrote:
From: Damien Le Moal <damien.lemoal@xxxxxxxxxxxxxxxxxx>
Introduce the function scsi_cdl_check() to detect if a device supports
command duration limits (CDL). Support for the READ 16, WRITE 16,
READ 32 and WRITE 32 commands are checked using the function
scsi_report_opcode() to probe the rwcdlp and cdlp bits as they indicate
the mode page defining the command duration limits descriptors that
apply to the command being tested.
If any of these commands support CDL, the field cdl_supported of
struct scsi_device is set to 1 to indicate that the device supports CDL.
Support for CDL for a device is advertizes through sysfs using the new
cdl_supported device attribute. This attribute value is 1 for a device
supporting CDL and 0 otherwise.
Signed-off-by: Damien Le Moal <damien.lemoal@xxxxxxxxxxxxxxxxxx>
Co-developed-by: Niklas Cassel <niklas.cassel@xxxxxxx>
Signed-off-by: Niklas Cassel <niklas.cassel@xxxxxxx>
---
Documentation/ABI/testing/sysfs-block-device | 9 +++
drivers/scsi/scsi.c | 81 ++++++++++++++++++++
drivers/scsi/scsi_scan.c | 3 +
drivers/scsi/scsi_sysfs.c | 2 +
include/scsi/scsi_device.h | 3 +
5 files changed, 98 insertions(+)
diff --git a/Documentation/ABI/testing/sysfs-block-device b/Documentation/ABI/testing/sysfs-block-device
index 7ac7b19b2f72..ee3610a25845 100644
--- a/Documentation/ABI/testing/sysfs-block-device
+++ b/Documentation/ABI/testing/sysfs-block-device
@@ -95,3 +95,12 @@ Description:
This file does not exist if the HBA driver does not implement
support for the SATA NCQ priority feature, regardless of the
device support for this feature.
+
+
+What: /sys/block/*/device/cdl_supported
+Date: Mar, 2023
+KernelVersion: v6.4
+Contact: linux-scsi@xxxxxxxxxxxxxxx
+Description:
+ (RO) Indicates if the device supports the command duration
+ limits feature found in some ATA and SCSI devices.
diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
index 62d9472e08e9..c03814ce23ca 100644
--- a/drivers/scsi/scsi.c
+++ b/drivers/scsi/scsi.c
@@ -570,6 +570,87 @@ int scsi_report_opcode(struct scsi_device *sdev, unsigned char *buffer,
}
EXPORT_SYMBOL(scsi_report_opcode);
+#define SCSI_CDL_CHECK_BUF_LEN 64
+
+static bool scsi_cdl_check_cmd(struct scsi_device *sdev, u8 opcode, u16 sa,
+ unsigned char *buf)
+{
+ int ret;
+ u8 cdlp;
+
+ /* Check operation code */
+ ret = scsi_report_opcode(sdev, buf, SCSI_CDL_CHECK_BUF_LEN, opcode, sa);
+ if (ret <= 0)
+ return false;
+
+ if ((buf[1] & 0x03) != 0x03)
+ return false;
+
+ /* See SPC-6, one command format of REPORT SUPPORTED OPERATION CODES */
+ cdlp = (buf[1] & 0x18) >> 3;
+ if (buf[0] & 0x01) {
+ /* rwcdlp == 1 */
+ switch (cdlp) {
+ case 0x01:
+ /* T2A page */
+ return true;
+ case 0x02:
+ /* T2B page */
+ return true;
+ }
+ } else {
+ /* rwcdlp == 0 */
+ switch (cdlp) {
+ case 0x01:
+ /* A page */
+ return true;
+ case 0x02:
+ /* B page */
+ return true;
+ }
+ }
+
+ return false;
+}
+
Why do we need to check this when writing to sysfs? Shouldn't we detect
this during startup / revalidate?
Hmm, I think you missed the call chain on this one (the patch starting with the
sys attribute doc changes is a little confusing).
scsi_cdl_check_cmd() is called from scsi_cdl_check(), which is itself called
from scsi_add_lun() (for initial device scan) and scsi_rescan_device() for
revalidate. scsi_cdl_check() will set sdev->cdl_supported to 1 for devices
supporting CDL, which is what sysfs cdl_supported show method uses, and later,
the cdl_enable attribute show/store method use that too. That function is not
called from sysfs attributes methods.
Note that this function was moved from sd.c to scsi.c as CDL is defined is SPC,
not SBC.
Indeed, you are right.
Reviewed-by: Hannes Reinecke <hare@xxxxxxx>
Cheers,
Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
hare@xxxxxxx +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Ivo Totev, Andrew
Myers, Andrew McDonald, Martje Boudien Moerman
