Message ID | 20230124190308.127318-9-niklas.cassel@wdc.com (mailing list archive) |
---|---|
State | Changes Requested |
Headers | show |
Series | Add Command Duration Limits support | expand |
On 1/24/23 20:02, Niklas Cassel wrote: > From: Damien Le Moal <damien.lemoal@opensource.wdc.com> > > Introduce the command duration limits helper function > sd_cdl_cmd_limit() to retrieve and set the DLD bits of the > READ/WRITE 16 and READ/WRITE 32 commands to indicate to the device > the command duration limit descriptor to apply to the command. > > When command duration limits are enabled, sd_cdl_cmd_limit() obtains the > index of the descriptor to apply to the command for requests that have > the IOPRIO_CLASS_DL priority class with a priority data sepcifying a > valid descriptor index (1 to 7). > > The read-write sysfs attribute "enable" is introduced to control > setting the command duration limits indexes. If this attribute is set > to 0 (default), command duration limits specified by the user are > ignored. The user must set this attribute to 1 for command duration > limits to be set. Enabling and disabling the command duration limits > feature for ATA devices must be done using the ATA feature sub-page of > the control mode page. The sd_cdl_enable() function is introduced to > check if this mode page is supported by the device and if it is, use > it to enable/disable CDL. > > Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com> > Co-developed-by: Niklas Cassel <niklas.cassel@wdc.com> > Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com> > --- > drivers/scsi/sd.c | 16 +++-- > drivers/scsi/sd.h | 10 ++++ > drivers/scsi/sd_cdl.c | 134 +++++++++++++++++++++++++++++++++++++++++- > 3 files changed, 152 insertions(+), 8 deletions(-) > > diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c > index 7879a5470773..d2eb01337943 100644 > --- a/drivers/scsi/sd.c > +++ b/drivers/scsi/sd.c > @@ -1045,13 +1045,14 @@ static blk_status_t sd_setup_flush_cmnd(struct scsi_cmnd *cmd) > > static blk_status_t sd_setup_rw32_cmnd(struct scsi_cmnd *cmd, bool write, > sector_t lba, unsigned int nr_blocks, > - unsigned char flags) > + unsigned char flags, unsigned int dld) > { > cmd->cmd_len = SD_EXT_CDB_SIZE; > cmd->cmnd[0] = VARIABLE_LENGTH_CMD; > cmd->cmnd[7] = 0x18; /* Additional CDB len */ > cmd->cmnd[9] = write ? WRITE_32 : READ_32; > cmd->cmnd[10] = flags; > + cmd->cmnd[11] = dld & 0x07; > put_unaligned_be64(lba, &cmd->cmnd[12]); > put_unaligned_be32(lba, &cmd->cmnd[20]); /* Expected Indirect LBA */ > put_unaligned_be32(nr_blocks, &cmd->cmnd[28]); > @@ -1061,12 +1062,12 @@ static blk_status_t sd_setup_rw32_cmnd(struct scsi_cmnd *cmd, bool write, > > static blk_status_t sd_setup_rw16_cmnd(struct scsi_cmnd *cmd, bool write, > sector_t lba, unsigned int nr_blocks, > - unsigned char flags) > + unsigned char flags, unsigned int dld) > { > cmd->cmd_len = 16; > cmd->cmnd[0] = write ? WRITE_16 : READ_16; > - cmd->cmnd[1] = flags; > - cmd->cmnd[14] = 0; > + cmd->cmnd[1] = flags | ((dld >> 2) & 0x01); > + cmd->cmnd[14] = (dld & 0x03) << 6; > cmd->cmnd[15] = 0; > put_unaligned_be64(lba, &cmd->cmnd[2]); > put_unaligned_be32(nr_blocks, &cmd->cmnd[10]); > @@ -1129,6 +1130,7 @@ static blk_status_t sd_setup_read_write_cmnd(struct scsi_cmnd *cmd) > unsigned int mask = logical_to_sectors(sdp, 1) - 1; > bool write = rq_data_dir(rq) == WRITE; > unsigned char protect, fua; > + unsigned int dld = 0; > blk_status_t ret; > unsigned int dif; > bool dix; > @@ -1178,6 +1180,8 @@ static blk_status_t sd_setup_read_write_cmnd(struct scsi_cmnd *cmd) > fua = rq->cmd_flags & REQ_FUA ? 0x8 : 0; > dix = scsi_prot_sg_count(cmd); > dif = scsi_host_dif_capable(cmd->device->host, sdkp->protection_type); > + if (sd_cdl_enabled(sdkp)) > + dld = sd_cdl_dld(sdkp, cmd); > > if (dif || dix) > protect = sd_setup_protect_cmnd(cmd, dix, dif); > @@ -1186,10 +1190,10 @@ static blk_status_t sd_setup_read_write_cmnd(struct scsi_cmnd *cmd) > > if (protect && sdkp->protection_type == T10_PI_TYPE2_PROTECTION) { > ret = sd_setup_rw32_cmnd(cmd, write, lba, nr_blocks, > - protect | fua); > + protect | fua, dld); > } else if (sdp->use_16_for_rw || (nr_blocks > 0xffff)) { > ret = sd_setup_rw16_cmnd(cmd, write, lba, nr_blocks, > - protect | fua); > + protect | fua, dld); > } else if ((nr_blocks > 0xff) || (lba > 0x1fffff) || > sdp->use_10_for_rw || protect) { > ret = sd_setup_rw10_cmnd(cmd, write, lba, nr_blocks, > diff --git a/drivers/scsi/sd.h b/drivers/scsi/sd.h > index e60d33bd222a..5b6b6dc4b92d 100644 > --- a/drivers/scsi/sd.h > +++ b/drivers/scsi/sd.h > @@ -130,8 +130,11 @@ struct sd_cdl_page { > struct sd_cdl_desc descs[SD_CDL_MAX_DESC]; > }; > > +struct scsi_disk; > + > struct sd_cdl { > struct kobject kobj; > + struct scsi_disk *sdkp; > bool sysfs_registered; > u8 perf_vs_duration_guideline; > struct sd_cdl_page pages[SD_CDL_RW]; > @@ -188,6 +191,7 @@ struct scsi_disk { > u8 zeroing_mode; > u8 nr_actuators; /* Number of actuators */ > struct sd_cdl *cdl; > + unsigned cdl_enabled : 1; > unsigned ATO : 1; /* state of disk ATO bit */ > unsigned cache_override : 1; /* temp override of WCE,RCD */ > unsigned WCE : 1; /* state of disk WCE bit */ > @@ -355,5 +359,11 @@ void sd_print_result(const struct scsi_disk *sdkp, const char *msg, int result); > /* Command duration limits support (in sd_cdl.c) */ > void sd_read_cdl(struct scsi_disk *sdkp, unsigned char *buf); > void sd_cdl_release(struct scsi_disk *sdkp); > +int sd_cdl_dld(struct scsi_disk *sdkp, struct scsi_cmnd *scmd); > + > +static inline bool sd_cdl_enabled(struct scsi_disk *sdkp) > +{ > + return sdkp->cdl && sdkp->cdl_enabled; > +} > > #endif /* _SCSI_DISK_H */ > diff --git a/drivers/scsi/sd_cdl.c b/drivers/scsi/sd_cdl.c > index 513cd989f19a..59d02dbb5ea1 100644 > --- a/drivers/scsi/sd_cdl.c > +++ b/drivers/scsi/sd_cdl.c > @@ -93,6 +93,63 @@ static const char *sd_cdl_policy_name(u8 policy) > } > } > > +/* > + * Enable/disable CDL. > + */ > +static int sd_cdl_enable(struct scsi_disk *sdkp, bool enable) > +{ > + struct scsi_device *sdp = sdkp->device; > + struct scsi_mode_data data; > + struct scsi_sense_hdr sshdr; > + struct scsi_vpd *vpd; > + bool is_ata = false; > + char buf[64]; > + int ret; > + > + rcu_read_lock(); > + vpd = rcu_dereference(sdp->vpd_pg89); > + if (vpd) > + is_ata = true; > + rcu_read_unlock(); > + > + /* > + * For ATA devices, CDL needs to be enabled with a SET FEATURES command. > + */ > + if (is_ata) { > + char *buf_data; > + int len; > + > + ret = scsi_mode_sense(sdp, 0x08, 0x0a, 0xf2, buf, sizeof(buf), > + SD_TIMEOUT, sdkp->max_retries, &data, > + NULL); > + if (ret) > + return -EINVAL; > + That is a tad odd. Is CDL always enabled for 'normal' SCSI? Cheers, Hannes
On 1/28/23 00:30, Hannes Reinecke wrote: > On 1/24/23 20:02, Niklas Cassel wrote: >> From: Damien Le Moal <damien.lemoal@opensource.wdc.com> >> >> Introduce the command duration limits helper function >> sd_cdl_cmd_limit() to retrieve and set the DLD bits of the >> READ/WRITE 16 and READ/WRITE 32 commands to indicate to the device >> the command duration limit descriptor to apply to the command. >> >> When command duration limits are enabled, sd_cdl_cmd_limit() obtains the >> index of the descriptor to apply to the command for requests that have >> the IOPRIO_CLASS_DL priority class with a priority data sepcifying a >> valid descriptor index (1 to 7). >> >> The read-write sysfs attribute "enable" is introduced to control >> setting the command duration limits indexes. If this attribute is set >> to 0 (default), command duration limits specified by the user are >> ignored. The user must set this attribute to 1 for command duration >> limits to be set. Enabling and disabling the command duration limits >> feature for ATA devices must be done using the ATA feature sub-page of >> the control mode page. The sd_cdl_enable() function is introduced to >> check if this mode page is supported by the device and if it is, use >> it to enable/disable CDL. >> >> Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com> >> Co-developed-by: Niklas Cassel <niklas.cassel@wdc.com> >> Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com> >> --- >> drivers/scsi/sd.c | 16 +++-- >> drivers/scsi/sd.h | 10 ++++ >> drivers/scsi/sd_cdl.c | 134 +++++++++++++++++++++++++++++++++++++++++- >> 3 files changed, 152 insertions(+), 8 deletions(-) >> >> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c >> index 7879a5470773..d2eb01337943 100644 >> --- a/drivers/scsi/sd.c >> +++ b/drivers/scsi/sd.c >> @@ -1045,13 +1045,14 @@ static blk_status_t sd_setup_flush_cmnd(struct scsi_cmnd *cmd) >> >> static blk_status_t sd_setup_rw32_cmnd(struct scsi_cmnd *cmd, bool write, >> sector_t lba, unsigned int nr_blocks, >> - unsigned char flags) >> + unsigned char flags, unsigned int dld) >> { >> cmd->cmd_len = SD_EXT_CDB_SIZE; >> cmd->cmnd[0] = VARIABLE_LENGTH_CMD; >> cmd->cmnd[7] = 0x18; /* Additional CDB len */ >> cmd->cmnd[9] = write ? WRITE_32 : READ_32; >> cmd->cmnd[10] = flags; >> + cmd->cmnd[11] = dld & 0x07; >> put_unaligned_be64(lba, &cmd->cmnd[12]); >> put_unaligned_be32(lba, &cmd->cmnd[20]); /* Expected Indirect LBA */ >> put_unaligned_be32(nr_blocks, &cmd->cmnd[28]); >> @@ -1061,12 +1062,12 @@ static blk_status_t sd_setup_rw32_cmnd(struct scsi_cmnd *cmd, bool write, >> >> static blk_status_t sd_setup_rw16_cmnd(struct scsi_cmnd *cmd, bool write, >> sector_t lba, unsigned int nr_blocks, >> - unsigned char flags) >> + unsigned char flags, unsigned int dld) >> { >> cmd->cmd_len = 16; >> cmd->cmnd[0] = write ? WRITE_16 : READ_16; >> - cmd->cmnd[1] = flags; >> - cmd->cmnd[14] = 0; >> + cmd->cmnd[1] = flags | ((dld >> 2) & 0x01); >> + cmd->cmnd[14] = (dld & 0x03) << 6; >> cmd->cmnd[15] = 0; >> put_unaligned_be64(lba, &cmd->cmnd[2]); >> put_unaligned_be32(nr_blocks, &cmd->cmnd[10]); >> @@ -1129,6 +1130,7 @@ static blk_status_t sd_setup_read_write_cmnd(struct scsi_cmnd *cmd) >> unsigned int mask = logical_to_sectors(sdp, 1) - 1; >> bool write = rq_data_dir(rq) == WRITE; >> unsigned char protect, fua; >> + unsigned int dld = 0; >> blk_status_t ret; >> unsigned int dif; >> bool dix; >> @@ -1178,6 +1180,8 @@ static blk_status_t sd_setup_read_write_cmnd(struct scsi_cmnd *cmd) >> fua = rq->cmd_flags & REQ_FUA ? 0x8 : 0; >> dix = scsi_prot_sg_count(cmd); >> dif = scsi_host_dif_capable(cmd->device->host, sdkp->protection_type); >> + if (sd_cdl_enabled(sdkp)) >> + dld = sd_cdl_dld(sdkp, cmd); >> >> if (dif || dix) >> protect = sd_setup_protect_cmnd(cmd, dix, dif); >> @@ -1186,10 +1190,10 @@ static blk_status_t sd_setup_read_write_cmnd(struct scsi_cmnd *cmd) >> >> if (protect && sdkp->protection_type == T10_PI_TYPE2_PROTECTION) { >> ret = sd_setup_rw32_cmnd(cmd, write, lba, nr_blocks, >> - protect | fua); >> + protect | fua, dld); >> } else if (sdp->use_16_for_rw || (nr_blocks > 0xffff)) { >> ret = sd_setup_rw16_cmnd(cmd, write, lba, nr_blocks, >> - protect | fua); >> + protect | fua, dld); >> } else if ((nr_blocks > 0xff) || (lba > 0x1fffff) || >> sdp->use_10_for_rw || protect) { >> ret = sd_setup_rw10_cmnd(cmd, write, lba, nr_blocks, >> diff --git a/drivers/scsi/sd.h b/drivers/scsi/sd.h >> index e60d33bd222a..5b6b6dc4b92d 100644 >> --- a/drivers/scsi/sd.h >> +++ b/drivers/scsi/sd.h >> @@ -130,8 +130,11 @@ struct sd_cdl_page { >> struct sd_cdl_desc descs[SD_CDL_MAX_DESC]; >> }; >> >> +struct scsi_disk; >> + >> struct sd_cdl { >> struct kobject kobj; >> + struct scsi_disk *sdkp; >> bool sysfs_registered; >> u8 perf_vs_duration_guideline; >> struct sd_cdl_page pages[SD_CDL_RW]; >> @@ -188,6 +191,7 @@ struct scsi_disk { >> u8 zeroing_mode; >> u8 nr_actuators; /* Number of actuators */ >> struct sd_cdl *cdl; >> + unsigned cdl_enabled : 1; >> unsigned ATO : 1; /* state of disk ATO bit */ >> unsigned cache_override : 1; /* temp override of WCE,RCD */ >> unsigned WCE : 1; /* state of disk WCE bit */ >> @@ -355,5 +359,11 @@ void sd_print_result(const struct scsi_disk *sdkp, const char *msg, int result); >> /* Command duration limits support (in sd_cdl.c) */ >> void sd_read_cdl(struct scsi_disk *sdkp, unsigned char *buf); >> void sd_cdl_release(struct scsi_disk *sdkp); >> +int sd_cdl_dld(struct scsi_disk *sdkp, struct scsi_cmnd *scmd); >> + >> +static inline bool sd_cdl_enabled(struct scsi_disk *sdkp) >> +{ >> + return sdkp->cdl && sdkp->cdl_enabled; >> +} >> >> #endif /* _SCSI_DISK_H */ >> diff --git a/drivers/scsi/sd_cdl.c b/drivers/scsi/sd_cdl.c >> index 513cd989f19a..59d02dbb5ea1 100644 >> --- a/drivers/scsi/sd_cdl.c >> +++ b/drivers/scsi/sd_cdl.c >> @@ -93,6 +93,63 @@ static const char *sd_cdl_policy_name(u8 policy) >> } >> } >> >> +/* >> + * Enable/disable CDL. >> + */ >> +static int sd_cdl_enable(struct scsi_disk *sdkp, bool enable) >> +{ >> + struct scsi_device *sdp = sdkp->device; >> + struct scsi_mode_data data; >> + struct scsi_sense_hdr sshdr; >> + struct scsi_vpd *vpd; >> + bool is_ata = false; >> + char buf[64]; >> + int ret; >> + >> + rcu_read_lock(); >> + vpd = rcu_dereference(sdp->vpd_pg89); >> + if (vpd) >> + is_ata = true; >> + rcu_read_unlock(); >> + >> + /* >> + * For ATA devices, CDL needs to be enabled with a SET FEATURES command. >> + */ >> + if (is_ata) { >> + char *buf_data; >> + int len; >> + >> + ret = scsi_mode_sense(sdp, 0x08, 0x0a, 0xf2, buf, sizeof(buf), >> + SD_TIMEOUT, sdkp->max_retries, &data, >> + NULL); >> + if (ret) >> + return -EINVAL; >> + > That is a tad odd. > Is CDL always enabled for 'normal' SCSI? Yes it is on the device side. There is no mode sense to turn it on/off. Not sure why it was designed like that in the specs... The sysfs duration_limits/enable attribute is a "soft" on/off switch and it is off by default, even for drives reporting supporting CDL. Hence the "if (is_ata)" to do the mode sense to enable the feature on the device side only for ATA devices. We need this to avoid having 2 different enable pathes with 2 different sysfs "enable" attributes. Doing it like this is a lot less code. > > Cheers, > > Hannes
On 1/28/23 01:03, Damien Le Moal wrote: > On 1/28/23 00:30, Hannes Reinecke wrote: >> On 1/24/23 20:02, Niklas Cassel wrote: >>> From: Damien Le Moal <damien.lemoal@opensource.wdc.com> >>> >>> Introduce the command duration limits helper function >>> sd_cdl_cmd_limit() to retrieve and set the DLD bits of the >>> READ/WRITE 16 and READ/WRITE 32 commands to indicate to the device >>> the command duration limit descriptor to apply to the command. >>> >>> When command duration limits are enabled, sd_cdl_cmd_limit() obtains the >>> index of the descriptor to apply to the command for requests that have >>> the IOPRIO_CLASS_DL priority class with a priority data sepcifying a >>> valid descriptor index (1 to 7). >>> >>> The read-write sysfs attribute "enable" is introduced to control >>> setting the command duration limits indexes. If this attribute is set >>> to 0 (default), command duration limits specified by the user are >>> ignored. The user must set this attribute to 1 for command duration >>> limits to be set. Enabling and disabling the command duration limits >>> feature for ATA devices must be done using the ATA feature sub-page of >>> the control mode page. The sd_cdl_enable() function is introduced to >>> check if this mode page is supported by the device and if it is, use >>> it to enable/disable CDL. >>> >>> Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com> >>> Co-developed-by: Niklas Cassel <niklas.cassel@wdc.com> >>> Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com> >>> --- >>> drivers/scsi/sd.c | 16 +++-- >>> drivers/scsi/sd.h | 10 ++++ >>> drivers/scsi/sd_cdl.c | 134 +++++++++++++++++++++++++++++++++++++++++- >>> 3 files changed, 152 insertions(+), 8 deletions(-) >>> >>> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c >>> index 7879a5470773..d2eb01337943 100644 >>> --- a/drivers/scsi/sd.c >>> +++ b/drivers/scsi/sd.c >>> @@ -1045,13 +1045,14 @@ static blk_status_t sd_setup_flush_cmnd(struct scsi_cmnd *cmd) >>> >>> static blk_status_t sd_setup_rw32_cmnd(struct scsi_cmnd *cmd, bool write, >>> sector_t lba, unsigned int nr_blocks, >>> - unsigned char flags) >>> + unsigned char flags, unsigned int dld) >>> { >>> cmd->cmd_len = SD_EXT_CDB_SIZE; >>> cmd->cmnd[0] = VARIABLE_LENGTH_CMD; >>> cmd->cmnd[7] = 0x18; /* Additional CDB len */ >>> cmd->cmnd[9] = write ? WRITE_32 : READ_32; >>> cmd->cmnd[10] = flags; >>> + cmd->cmnd[11] = dld & 0x07; >>> put_unaligned_be64(lba, &cmd->cmnd[12]); >>> put_unaligned_be32(lba, &cmd->cmnd[20]); /* Expected Indirect LBA */ >>> put_unaligned_be32(nr_blocks, &cmd->cmnd[28]); >>> @@ -1061,12 +1062,12 @@ static blk_status_t sd_setup_rw32_cmnd(struct scsi_cmnd *cmd, bool write, >>> >>> static blk_status_t sd_setup_rw16_cmnd(struct scsi_cmnd *cmd, bool write, >>> sector_t lba, unsigned int nr_blocks, >>> - unsigned char flags) >>> + unsigned char flags, unsigned int dld) >>> { >>> cmd->cmd_len = 16; >>> cmd->cmnd[0] = write ? WRITE_16 : READ_16; >>> - cmd->cmnd[1] = flags; >>> - cmd->cmnd[14] = 0; >>> + cmd->cmnd[1] = flags | ((dld >> 2) & 0x01); >>> + cmd->cmnd[14] = (dld & 0x03) << 6; >>> cmd->cmnd[15] = 0; >>> put_unaligned_be64(lba, &cmd->cmnd[2]); >>> put_unaligned_be32(nr_blocks, &cmd->cmnd[10]); >>> @@ -1129,6 +1130,7 @@ static blk_status_t sd_setup_read_write_cmnd(struct scsi_cmnd *cmd) >>> unsigned int mask = logical_to_sectors(sdp, 1) - 1; >>> bool write = rq_data_dir(rq) == WRITE; >>> unsigned char protect, fua; >>> + unsigned int dld = 0; >>> blk_status_t ret; >>> unsigned int dif; >>> bool dix; >>> @@ -1178,6 +1180,8 @@ static blk_status_t sd_setup_read_write_cmnd(struct scsi_cmnd *cmd) >>> fua = rq->cmd_flags & REQ_FUA ? 0x8 : 0; >>> dix = scsi_prot_sg_count(cmd); >>> dif = scsi_host_dif_capable(cmd->device->host, sdkp->protection_type); >>> + if (sd_cdl_enabled(sdkp)) >>> + dld = sd_cdl_dld(sdkp, cmd); >>> >>> if (dif || dix) >>> protect = sd_setup_protect_cmnd(cmd, dix, dif); >>> @@ -1186,10 +1190,10 @@ static blk_status_t sd_setup_read_write_cmnd(struct scsi_cmnd *cmd) >>> >>> if (protect && sdkp->protection_type == T10_PI_TYPE2_PROTECTION) { >>> ret = sd_setup_rw32_cmnd(cmd, write, lba, nr_blocks, >>> - protect | fua); >>> + protect | fua, dld); >>> } else if (sdp->use_16_for_rw || (nr_blocks > 0xffff)) { >>> ret = sd_setup_rw16_cmnd(cmd, write, lba, nr_blocks, >>> - protect | fua); >>> + protect | fua, dld); >>> } else if ((nr_blocks > 0xff) || (lba > 0x1fffff) || >>> sdp->use_10_for_rw || protect) { >>> ret = sd_setup_rw10_cmnd(cmd, write, lba, nr_blocks, >>> diff --git a/drivers/scsi/sd.h b/drivers/scsi/sd.h >>> index e60d33bd222a..5b6b6dc4b92d 100644 >>> --- a/drivers/scsi/sd.h >>> +++ b/drivers/scsi/sd.h >>> @@ -130,8 +130,11 @@ struct sd_cdl_page { >>> struct sd_cdl_desc descs[SD_CDL_MAX_DESC]; >>> }; >>> >>> +struct scsi_disk; >>> + >>> struct sd_cdl { >>> struct kobject kobj; >>> + struct scsi_disk *sdkp; >>> bool sysfs_registered; >>> u8 perf_vs_duration_guideline; >>> struct sd_cdl_page pages[SD_CDL_RW]; >>> @@ -188,6 +191,7 @@ struct scsi_disk { >>> u8 zeroing_mode; >>> u8 nr_actuators; /* Number of actuators */ >>> struct sd_cdl *cdl; >>> + unsigned cdl_enabled : 1; >>> unsigned ATO : 1; /* state of disk ATO bit */ >>> unsigned cache_override : 1; /* temp override of WCE,RCD */ >>> unsigned WCE : 1; /* state of disk WCE bit */ >>> @@ -355,5 +359,11 @@ void sd_print_result(const struct scsi_disk *sdkp, const char *msg, int result); >>> /* Command duration limits support (in sd_cdl.c) */ >>> void sd_read_cdl(struct scsi_disk *sdkp, unsigned char *buf); >>> void sd_cdl_release(struct scsi_disk *sdkp); >>> +int sd_cdl_dld(struct scsi_disk *sdkp, struct scsi_cmnd *scmd); >>> + >>> +static inline bool sd_cdl_enabled(struct scsi_disk *sdkp) >>> +{ >>> + return sdkp->cdl && sdkp->cdl_enabled; >>> +} >>> >>> #endif /* _SCSI_DISK_H */ >>> diff --git a/drivers/scsi/sd_cdl.c b/drivers/scsi/sd_cdl.c >>> index 513cd989f19a..59d02dbb5ea1 100644 >>> --- a/drivers/scsi/sd_cdl.c >>> +++ b/drivers/scsi/sd_cdl.c >>> @@ -93,6 +93,63 @@ static const char *sd_cdl_policy_name(u8 policy) >>> } >>> } >>> >>> +/* >>> + * Enable/disable CDL. >>> + */ >>> +static int sd_cdl_enable(struct scsi_disk *sdkp, bool enable) >>> +{ >>> + struct scsi_device *sdp = sdkp->device; >>> + struct scsi_mode_data data; >>> + struct scsi_sense_hdr sshdr; >>> + struct scsi_vpd *vpd; >>> + bool is_ata = false; >>> + char buf[64]; >>> + int ret; >>> + >>> + rcu_read_lock(); >>> + vpd = rcu_dereference(sdp->vpd_pg89); >>> + if (vpd) >>> + is_ata = true; >>> + rcu_read_unlock(); >>> + >>> + /* >>> + * For ATA devices, CDL needs to be enabled with a SET FEATURES command. >>> + */ >>> + if (is_ata) { >>> + char *buf_data; >>> + int len; >>> + >>> + ret = scsi_mode_sense(sdp, 0x08, 0x0a, 0xf2, buf, sizeof(buf), >>> + SD_TIMEOUT, sdkp->max_retries, &data, >>> + NULL); >>> + if (ret) >>> + return -EINVAL; >>> + >> That is a tad odd. >> Is CDL always enabled for 'normal' SCSI? > > Yes it is on the device side. There is no mode sense to turn it on/off. Not sure > why it was designed like that in the specs... The sysfs duration_limits/enable > attribute is a "soft" on/off switch and it is off by default, even for drives > reporting supporting CDL. > Hence the "if (is_ata)" to do the mode sense to enable the feature on the device > side only for ATA devices. We need this to avoid having 2 different enable > pathes with 2 different sysfs "enable" attributes. Doing it like this is a lot > less code. > Thought as much. No-one cares about 'real' SCSI devices anymore ;-) Reviewed-by: Hannes Reinecke <hare@suse.de> Cheers, Hannes
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c index 7879a5470773..d2eb01337943 100644 --- a/drivers/scsi/sd.c +++ b/drivers/scsi/sd.c @@ -1045,13 +1045,14 @@ static blk_status_t sd_setup_flush_cmnd(struct scsi_cmnd *cmd) static blk_status_t sd_setup_rw32_cmnd(struct scsi_cmnd *cmd, bool write, sector_t lba, unsigned int nr_blocks, - unsigned char flags) + unsigned char flags, unsigned int dld) { cmd->cmd_len = SD_EXT_CDB_SIZE; cmd->cmnd[0] = VARIABLE_LENGTH_CMD; cmd->cmnd[7] = 0x18; /* Additional CDB len */ cmd->cmnd[9] = write ? WRITE_32 : READ_32; cmd->cmnd[10] = flags; + cmd->cmnd[11] = dld & 0x07; put_unaligned_be64(lba, &cmd->cmnd[12]); put_unaligned_be32(lba, &cmd->cmnd[20]); /* Expected Indirect LBA */ put_unaligned_be32(nr_blocks, &cmd->cmnd[28]); @@ -1061,12 +1062,12 @@ static blk_status_t sd_setup_rw32_cmnd(struct scsi_cmnd *cmd, bool write, static blk_status_t sd_setup_rw16_cmnd(struct scsi_cmnd *cmd, bool write, sector_t lba, unsigned int nr_blocks, - unsigned char flags) + unsigned char flags, unsigned int dld) { cmd->cmd_len = 16; cmd->cmnd[0] = write ? WRITE_16 : READ_16; - cmd->cmnd[1] = flags; - cmd->cmnd[14] = 0; + cmd->cmnd[1] = flags | ((dld >> 2) & 0x01); + cmd->cmnd[14] = (dld & 0x03) << 6; cmd->cmnd[15] = 0; put_unaligned_be64(lba, &cmd->cmnd[2]); put_unaligned_be32(nr_blocks, &cmd->cmnd[10]); @@ -1129,6 +1130,7 @@ static blk_status_t sd_setup_read_write_cmnd(struct scsi_cmnd *cmd) unsigned int mask = logical_to_sectors(sdp, 1) - 1; bool write = rq_data_dir(rq) == WRITE; unsigned char protect, fua; + unsigned int dld = 0; blk_status_t ret; unsigned int dif; bool dix; @@ -1178,6 +1180,8 @@ static blk_status_t sd_setup_read_write_cmnd(struct scsi_cmnd *cmd) fua = rq->cmd_flags & REQ_FUA ? 0x8 : 0; dix = scsi_prot_sg_count(cmd); dif = scsi_host_dif_capable(cmd->device->host, sdkp->protection_type); + if (sd_cdl_enabled(sdkp)) + dld = sd_cdl_dld(sdkp, cmd); if (dif || dix) protect = sd_setup_protect_cmnd(cmd, dix, dif); @@ -1186,10 +1190,10 @@ static blk_status_t sd_setup_read_write_cmnd(struct scsi_cmnd *cmd) if (protect && sdkp->protection_type == T10_PI_TYPE2_PROTECTION) { ret = sd_setup_rw32_cmnd(cmd, write, lba, nr_blocks, - protect | fua); + protect | fua, dld); } else if (sdp->use_16_for_rw || (nr_blocks > 0xffff)) { ret = sd_setup_rw16_cmnd(cmd, write, lba, nr_blocks, - protect | fua); + protect | fua, dld); } else if ((nr_blocks > 0xff) || (lba > 0x1fffff) || sdp->use_10_for_rw || protect) { ret = sd_setup_rw10_cmnd(cmd, write, lba, nr_blocks, diff --git a/drivers/scsi/sd.h b/drivers/scsi/sd.h index e60d33bd222a..5b6b6dc4b92d 100644 --- a/drivers/scsi/sd.h +++ b/drivers/scsi/sd.h @@ -130,8 +130,11 @@ struct sd_cdl_page { struct sd_cdl_desc descs[SD_CDL_MAX_DESC]; }; +struct scsi_disk; + struct sd_cdl { struct kobject kobj; + struct scsi_disk *sdkp; bool sysfs_registered; u8 perf_vs_duration_guideline; struct sd_cdl_page pages[SD_CDL_RW]; @@ -188,6 +191,7 @@ struct scsi_disk { u8 zeroing_mode; u8 nr_actuators; /* Number of actuators */ struct sd_cdl *cdl; + unsigned cdl_enabled : 1; unsigned ATO : 1; /* state of disk ATO bit */ unsigned cache_override : 1; /* temp override of WCE,RCD */ unsigned WCE : 1; /* state of disk WCE bit */ @@ -355,5 +359,11 @@ void sd_print_result(const struct scsi_disk *sdkp, const char *msg, int result); /* Command duration limits support (in sd_cdl.c) */ void sd_read_cdl(struct scsi_disk *sdkp, unsigned char *buf); void sd_cdl_release(struct scsi_disk *sdkp); +int sd_cdl_dld(struct scsi_disk *sdkp, struct scsi_cmnd *scmd); + +static inline bool sd_cdl_enabled(struct scsi_disk *sdkp) +{ + return sdkp->cdl && sdkp->cdl_enabled; +} #endif /* _SCSI_DISK_H */ diff --git a/drivers/scsi/sd_cdl.c b/drivers/scsi/sd_cdl.c index 513cd989f19a..59d02dbb5ea1 100644 --- a/drivers/scsi/sd_cdl.c +++ b/drivers/scsi/sd_cdl.c @@ -93,6 +93,63 @@ static const char *sd_cdl_policy_name(u8 policy) } } +/* + * Enable/disable CDL. + */ +static int sd_cdl_enable(struct scsi_disk *sdkp, bool enable) +{ + struct scsi_device *sdp = sdkp->device; + struct scsi_mode_data data; + struct scsi_sense_hdr sshdr; + struct scsi_vpd *vpd; + bool is_ata = false; + char buf[64]; + int ret; + + rcu_read_lock(); + vpd = rcu_dereference(sdp->vpd_pg89); + if (vpd) + is_ata = true; + rcu_read_unlock(); + + /* + * For ATA devices, CDL needs to be enabled with a SET FEATURES command. + */ + if (is_ata) { + char *buf_data; + int len; + + ret = scsi_mode_sense(sdp, 0x08, 0x0a, 0xf2, buf, sizeof(buf), + SD_TIMEOUT, sdkp->max_retries, &data, + NULL); + if (ret) + return -EINVAL; + + /* Enable CDL using the ATA feature page */ + len = min_t(size_t, sizeof(buf), + data.length - data.header_length - + data.block_descriptor_length); + buf_data = buf + data.header_length + + data.block_descriptor_length; + if (enable) + buf_data[4] = 0x02; + else + buf_data[4] = 0; + + ret = scsi_mode_select(sdp, 1, 0, buf_data, len, SD_TIMEOUT, + sdkp->max_retries, &data, &sshdr); + if (ret) { + if (scsi_sense_valid(&sshdr)) + sd_print_sense_hdr(sdkp, &sshdr); + return -EINVAL; + } + } + + sdkp->cdl_enabled = enable; + + return 0; +} + /* * Command duration limits descriptors sysfs plumbing. */ @@ -324,6 +381,7 @@ static int sd_cdl_sysfs_register_page(struct scsi_disk *sdkp, struct sd_cdl_sysfs_entry { struct attribute attr; ssize_t (*show)(struct sd_cdl *cdl, char *buf); + ssize_t (*store)(struct sd_cdl *cdl, const char *buf, size_t length); }; #define CDL_ATTR_RO(_name) \ @@ -332,6 +390,13 @@ struct sd_cdl_sysfs_entry { .show = cdl_##_name##_show, \ } +#define CDL_ATTR_RW(_name) \ + static struct sd_cdl_sysfs_entry cdl_##_name##_entry = { \ + .attr = { .name = __stringify(_name), .mode = 0644 }, \ + .show = cdl_##_name##_show, \ + .store = cdl_##_name##_store, \ + } + static ssize_t cdl_perf_vs_duration_guideline_show(struct sd_cdl *cdl, char *buf) { @@ -340,8 +405,31 @@ static ssize_t cdl_perf_vs_duration_guideline_show(struct sd_cdl *cdl, } CDL_ATTR_RO(perf_vs_duration_guideline); +static ssize_t cdl_enable_show(struct sd_cdl *cdl, char *buf) +{ + return sysfs_emit(buf, "%d\n", (int)cdl->sdkp->cdl_enabled); +} + +static ssize_t cdl_enable_store(struct sd_cdl *cdl, + const char *buf, size_t count) +{ + int ret; + bool v; + + if (kstrtobool(buf, &v)) + return -EINVAL; + + ret = sd_cdl_enable(cdl->sdkp, v); + if (ret) + return ret; + + return count; +} +CDL_ATTR_RW(enable); + static struct attribute *sd_cdl_attrs[] = { &cdl_perf_vs_duration_guideline_entry.attr, + &cdl_enable_entry.attr, NULL, }; @@ -375,8 +463,25 @@ static ssize_t sd_cdl_sysfs_show(struct kobject *kobj, return entry->show(cdl, page); } +static ssize_t sd_cdl_sysfs_store(struct kobject *kobj, struct attribute *attr, + const char *buf, size_t length) +{ + struct sd_cdl_sysfs_entry *entry = + container_of(attr, struct sd_cdl_sysfs_entry, attr); + struct sd_cdl *cdl = container_of(kobj, struct sd_cdl, kobj); + + if (!capable(CAP_SYS_ADMIN)) + return -EACCES; + + if (!entry->store) + return -EIO; + + return entry->store(cdl, buf, length); +} + static const struct sysfs_ops sd_cdl_sysfs_ops = { .show = sd_cdl_sysfs_show, + .store = sd_cdl_sysfs_store, }; static void sd_cdl_sysfs_release(struct kobject *kobj) @@ -411,6 +516,7 @@ static void sd_cdl_sysfs_unregister(struct scsi_disk *sdkp) sd_cdl_sysfs_unregister_page(&cdl->pages[i]); } + cdl->sdkp->cdl_enabled = 0; kobject_del(&cdl->kobj); kobject_put(&cdl->kobj); } @@ -689,7 +795,7 @@ static bool sd_cdl_supported(struct scsi_disk *sdkp, enum sd_cdlp *rw_cdlp, rw_cdlp[SD_CDL_WRITE] != SD_CDLP_NONE; } -static struct sd_cdl *sd_cdl_alloc(void) +static struct sd_cdl *sd_cdl_alloc(struct scsi_disk *sdkp) { struct sd_cdl *cdl; struct sd_cdl_page *page; @@ -699,6 +805,7 @@ static struct sd_cdl *sd_cdl_alloc(void) if (!cdl) return NULL; + cdl->sdkp = sdkp; kobject_init(&cdl->kobj, &sd_cdl_ktype); for (i = 0; i < SD_CDL_RW; i++) { page = &cdl->pages[i]; @@ -725,7 +832,7 @@ void sd_read_cdl(struct scsi_disk *sdkp, unsigned char *buf) goto unregister; if (!cdl) { - cdl = sd_cdl_alloc(); + cdl = sd_cdl_alloc(sdkp); if (!cdl) return; } @@ -762,3 +869,26 @@ void sd_cdl_release(struct scsi_disk *sdkp) { sd_cdl_sysfs_unregister(sdkp); } + +/* + * Check if a command has a duration limit set. If it does, return the + * descriptor index to use and 0 if the command has no limit set. + */ +int sd_cdl_dld(struct scsi_disk *sdkp, struct scsi_cmnd *scmd) +{ + unsigned int ioprio = req_get_ioprio(scsi_cmd_to_rq(scmd)); + unsigned int dld; + + /* + * Use "no limit" if the request ioprio class is not IOPRIO_CLASS_DL + * or if the user specified an invalid CDL descriptor index. + */ + if (IOPRIO_PRIO_CLASS(ioprio) != IOPRIO_CLASS_DL) + return 0; + + dld = IOPRIO_PRIO_DATA(ioprio); + if (dld > SD_CDL_MAX_DESC) + return 0; + + return dld; +}