[PATCH 1/2] sd: add missing scenario for sd_config_write_same
From: Tom Yan Example: [root@localhost ~]# sg_opcodes /dev/sdb > /dev/null Report supported operation codes: Illegal request, invalid opcode [root@localhost ~]# sg_vpd -p bl /dev/sdb | grep 'write same' Maximum write same length: 0x0 blocks [root@localhost ~]# cat /sys/block/sdb/queue/write_same_max_bytes 33553920 Signed-off-by: Tom Yan diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c index d749da7..1179ec1 100644 --- a/drivers/scsi/sd.c +++ b/drivers/scsi/sd.c @@ -807,6 +807,8 @@ static void sd_config_write_same(struct scsi_disk *sdkp) if (sdkp->max_ws_blocks > SD_MAX_WS10_BLOCKS) sdkp->max_ws_blocks = min_not_zero(sdkp->max_ws_blocks, (u32)SD_MAX_WS16_BLOCKS); + else if (sdkp->max_ws_blocks == 0 && sdkp->device->no_report_opcodes) + sdkp->device->no_write_same = 1; else if (sdkp->ws16 || sdkp->ws10 || sdkp->device->no_report_opcodes) sdkp->max_ws_blocks = min_not_zero(sdkp->max_ws_blocks, (u32)SD_MAX_WS10_BLOCKS); -- 2.7.1 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] sd: disable write same for SAT as per the comment
From: Tom Yan Signed-off-by: Tom Yan diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c index 1179ec1..951 100644 --- a/drivers/scsi/sd.c +++ b/drivers/scsi/sd.c @@ -2786,7 +2786,7 @@ static void sd_read_write_same(struct scsi_disk *sdkp, unsigned char *buffer) * CODES is unsupported and the device has an ATA * Information VPD page (SAT). */ - if (!scsi_get_vpd_page(sdev, 0x89, buffer, vpd_buf_len)) + if (scsi_get_vpd_page(sdev, 0x89, buffer, vpd_buf_len)) sdev->no_write_same = 1; } -- 2.7.1 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 1/1] sd: add missing scenario for sd_config_write_same
From: Tom Yan Example: [root@localhost ~]# sg_opcodes /dev/sdb > /dev/null Report supported operation codes: Illegal request, invalid opcode [root@localhost ~]# sg_vpd -p bl /dev/sdb | grep 'write same' Maximum write same length: 0x0 blocks [root@localhost ~]# cat /sys/block/sdb/queue/write_same_max_bytes 33553920 Signed-off-by: Tom Yan diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c index d749da7..1179ec1 100644 --- a/drivers/scsi/sd.c +++ b/drivers/scsi/sd.c @@ -807,6 +807,8 @@ static void sd_config_write_same(struct scsi_disk *sdkp) if (sdkp->max_ws_blocks > SD_MAX_WS10_BLOCKS) sdkp->max_ws_blocks = min_not_zero(sdkp->max_ws_blocks, (u32)SD_MAX_WS16_BLOCKS); + else if (sdkp->max_ws_blocks == 0 && sdkp->device->no_report_opcodes) + sdkp->device->no_write_same = 1; else if (sdkp->ws16 || sdkp->ws10 || sdkp->device->no_report_opcodes) sdkp->max_ws_blocks = min_not_zero(sdkp->max_ws_blocks, (u32)SD_MAX_WS10_BLOCKS); -- 2.7.1 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/1] sd: do not let LBPME bit stop the VPDs speak
From: Tom Yan Some devices have details of their support on unmapping on the Block Limits and/or Logical Block Provisioning VPDs while they do not set the LBPME bit to 1. Though this is required by the SCSI standards, the VPDs are giving even more concrete details about the support, so they should be used even when the bit is set to 0. Signed-off-by: Tom Yan diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c index d749da7..a0d7c73 100644 --- a/drivers/scsi/sd.c +++ b/drivers/scsi/sd.c @@ -2670,9 +2670,6 @@ static void sd_read_block_limits(struct scsi_disk *sdkp) sdkp->max_ws_blocks = (u32)get_unaligned_be64(&buffer[36]); - if (!sdkp->lbpme) - goto out; - lba_count = get_unaligned_be32(&buffer[20]); desc_count = get_unaligned_be32(&buffer[24]); @@ -2747,9 +2744,6 @@ static void sd_read_block_provisioning(struct scsi_disk *sdkp) unsigned char *buffer; const int vpd_len = 8; - if (sdkp->lbpme == 0) - return; - buffer = kmalloc(vpd_len, GFP_KERNEL); if (!buffer || scsi_get_vpd_page(sdkp->device, 0xb2, buffer, vpd_len)) -- 2.7.2 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/1] sd: fix lbprz discard granularity as expected
From: Tom Yan According to its own comment, the discard granularity should fixed to the logical block size. However, the actual code has it hardcoded as 1 byte. Changing it to logical_block_size. Signed-off-by: Tom Yan diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c index d749da7..5a5457a 100644 --- a/drivers/scsi/sd.c +++ b/drivers/scsi/sd.c @@ -648,7 +648,7 @@ static void sd_config_discard(struct scsi_disk *sdkp, unsigned int mode) */ if (sdkp->lbprz) { q->limits.discard_alignment = 0; - q->limits.discard_granularity = 1; + q->limits.discard_granularity = logical_block_size; } else { q->limits.discard_alignment = sdkp->unmap_alignment * logical_block_size; -- 2.7.2 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/3] libata-scsi: Do not partially report ATA read look-ahead
From: Tom Yan Nothing in the kernel actually makes use of the DRA bit in SCSI MODE SENSE. Neither is there a sysfs file (like 'cache_type' for WCE) that allows users to change the bit nor SCSI-ATA Translation that can toggle the ATA feature. Hence removing the MODE SENSE SCSI-ATA Translation that is only triggered by the change of WCE to avoid silliness like this: https://bugzilla.kernel.org/show_bug.cgi?id=105861#c2 Signed-off-by: Tom Yan diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c index 0295c38..8c07db8 100644 --- a/drivers/ata/libata-scsi.c +++ b/drivers/ata/libata-scsi.c @@ -2316,8 +2316,6 @@ static unsigned int ata_msense_caching(u16 *id, u8 *buf, bool changeable) modecpy(buf, def_cache_mpage, sizeof(def_cache_mpage), changeable); if (changeable || ata_id_wcache_enabled(id)) buf[2] |= (1 << 2); /* write cache enable */ - if (!changeable && !ata_id_rahead_enabled(id)) - buf[12] |= (1 << 5);/* disable read ahead */ return sizeof(def_cache_mpage); } -- 2.8.2 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/3] libata-scsi: Fix SCSI INQUIRY version descriptor
From: Tom Yan https://bugzilla.kernel.org/show_bug.cgi?id=106931 Signed-off-by: Tom Yan diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c index cd30f11..0295c38 100644 --- a/drivers/ata/libata-scsi.c +++ b/drivers/ata/libata-scsi.c @@ -1985,8 +1985,8 @@ static unsigned int ata_scsiop_inq_std(struct ata_scsi_args *args, u8 *rbuf) 0x03, 0x20, /* SBC-2 (no version claimed) */ - 0x02, - 0x60/* SPC-3 (no version claimed) */ + 0x03, + 0x00/* SPC-3 (no version claimed) */ }; const u8 versions_zbc[] = { 0x00, -- 2.8.2 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/3] libata-scsi: Set CmdQue=1 when NCQ is enabled
From: Tom Yan https://bugzilla.kernel.org/show_bug.cgi?id=105931 This might look trivial at first sight. However, it can be important to have the bit set accordingly when the device/SATL is SCSI-passthrough'd to a virtual machine with scsi-block in qemu: https://github.com/YanVugenfirer/kvm-guest-drivers-windows/issues/63#issuecomment-216199929 Signed-off-by: Tom Yan diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c index 567859c..cd30f11 100644 --- a/drivers/ata/libata-scsi.c +++ b/drivers/ata/libata-scsi.c @@ -2007,7 +2007,10 @@ static unsigned int ata_scsiop_inq_std(struct ata_scsi_args *args, u8 *rbuf) 0, 0x5,/* claim SPC-3 version compatibility */ 2, - 95 - 4 + 95 - 4, + 0, + 0, + 0 }; VPRINTK("ENTER\n"); @@ -2024,6 +2027,9 @@ static unsigned int ata_scsiop_inq_std(struct ata_scsi_args *args, u8 *rbuf) hdr[2] = 0x6; /* ZBC is defined in SPC-4 */ } + if (ata_ncq_enabled(args->dev)) + hdr[7] |= (1 << 1); + memcpy(rbuf, hdr, sizeof(hdr)); memcpy(&rbuf[8], "ATA ", 8); ata_id_string(args->id, &rbuf[16], ATA_ID_PROD, 16); -- 2.8.2 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/1] scsi: bump SCSI_DEFAULT_MAX_SECTORS to SD_DEF_XFER_BLOCKS
From: Tom Yan As of commit ca369d51b3e1649be4a72addd6d6a168cfb3f537, max_sectors (rw_max) of a scsi disk can be as high as SD_DEF_XFER_BLOCKS. Therefore, bump SCSI_DEFAULT_MAX_SECTORS to 65535 to make hosts that do not have max_sectors set in their templates benefit from the commit. Hosts that are not capable of handling SCSI_DEFAULT_MAX_SECTORS should have max_sectors set in their templates (like usb-storage does in scsiglue.c). Signed-off-by: Tom Yan diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h index fcfa3d7..f3c43bd 100644 --- a/include/scsi/scsi_host.h +++ b/include/scsi/scsi_host.h @@ -383,7 +383,7 @@ struct scsi_host_template { * maximum, and may be over the transfer limits allowed for * individual devices (e.g. 256 for SCSI-1). */ -#define SCSI_DEFAULT_MAX_SECTORS 1024 +#define SCSI_DEFAULT_MAX_SECTORS65535 /* * True if this host adapter can make good use of linked commands. -- 2.8.2 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/1] uas: leave can_queue as MAX_CMNDS if device reports larger qdepth
From: Tom Yan Commit 198de51dbc34 ("USB: uas: Limit qdepth at the scsi-host level") made qdepth limit set in host template (`.can_queue = MAX_CMNDS`) useless. Instead of removing the template limit, now we only change limit according to the qdepth reported by the device if it is smaller than MAX_CMNDS. Signed-off-by: Tom Yan diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 4d49fce..d7790e6 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -972,7 +972,8 @@ static int uas_probe(struct usb_interface *intf, const struct usb_device_id *id) * 1 tag is reserved for untagged commands + * 1 tag to avoid off by one errors in some bridge firmwares */ - shost->can_queue = devinfo->qdepth - 2; + if (devinfo->qdepth - 2 < MAX_CMNDS) + shost->can_queue = devinfo->qdepth - 2; usb_set_intfdata(intf, shost); result = scsi_add_host(shost, &intf->dev); -- 2.8.2 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 1/1] uas: remove can_queue set in host template
From: Tom Yan Commit 198de51dbc34 ("USB: uas: Limit qdepth at the scsi-host level") made qdepth limit set in host template (`.can_queue = MAX_CMNDS`) redundant. Removing it to avoid confusion. Signed-off-by: Tom Yan diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 4d49fce..e03c490 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -848,7 +848,6 @@ static struct scsi_host_template uas_host_template = { .slave_configure = uas_slave_configure, .eh_abort_handler = uas_eh_abort_handler, .eh_bus_reset_handler = uas_eh_bus_reset_handler, - .can_queue = MAX_CMNDS, .this_id = -1, .sg_tablesize = SG_NONE, .skip_settle_delay = 1, -- 2.8.2 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] sd: read unmap block limits even if lbpme=0
From: Tom Yan Some devices may not be decent enough to report lbpme bit properly even when they do support unmap and report relevant information in the block limits and logical block provisioning VPDs properly (Namely, ASMedia ASM1351, a UAS-SATA bridge). One of the reasons is, lbpme=1 is not a requirement for "DeleteNotify" in Windows to be activated: [root@archlinux ~]# sg_readcap -l /dev/sda | grep lb Logical block provisioning: lbpme=0, lbprz=0 [root@archlinux ~]# sg_vpd -p lbpv /dev/sda | grep \(LB Unmap command supported (LBPU): 1 Write same (16) with unmap bit supported (LBWS): 0 Write same (10) with unmap bit supported (LBWS10): 0 Logical block provisioning read zeros (LBPRZ): 0 [root@archlinux ~]# sg_vpd -p bl /dev/sda | grep -i unmap Maximum unmap LBA count: 4194240 Maximum unmap block descriptor count: 1 Optimal unmap granularity: 1 Unmap granularity alignment valid: 0 Unmap granularity alignment: 0 While there may be a point to retain the "strict policy" on this, by not configuring discard for such device automatically, there is little reason not to read the relevant data from the VPD, for users are allowed to configure discard for a device via the provisioning_mode sysfs file. While discard_max_bytes can be changed manually, it is better if the value would be limited by a discard_max_hw_bytes that is set from the hardware limit that is reported in the VPD. Before this commit: [root@archlinux ~]# cat (...)/provisioning_mode full [root@archlinux ~]# grep . /sys/block/sda/queue/discard_* /sys/block/sda/queue/discard_granularity:0 /sys/block/sda/queue/discard_max_bytes:0 /sys/block/sda/queue/discard_max_hw_bytes:0 /sys/block/sda/queue/discard_zeroes_data:0 [root@archlinux ~]# echo -n unmap > (...)/provisioning_mode [root@archlinux ~]# grep . /sys/block/sda/queue/discard_* /sys/block/sda/queue/discard_granularity:512 /sys/block/sda/queue/discard_max_bytes:4294966784 /sys/block/sda/queue/discard_max_hw_bytes:4294966784 /sys/block/sda/queue/discard_zeroes_data:0 After this commit: [root@archlinux ~]# cat (...)/provisioning_mode full [root@archlinux ~]# grep . /sys/block/sda/queue/discard_* /sys/block/sda/queue/discard_granularity:0 /sys/block/sda/queue/discard_max_bytes:0 /sys/block/sda/queue/discard_max_hw_bytes:0 /sys/block/sda/queue/discard_zeroes_data:0 [root@archlinux ~]# echo -n unmap > (...)/provisioning_mode [root@archlinux ~]# grep . /sys/block/sda/queue/discard_* /sys/block/sda/queue/discard_granularity:512 /sys/block/sda/queue/discard_max_bytes:2147450880 /sys/block/sda/queue/discard_max_hw_bytes:2147450880 /sys/block/sda/queue/discard_zeroes_data:0 Signed-off-by: Tom Yan diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c index bea36adeee17..ea9e6fc76b63 100644 --- a/drivers/scsi/sd.c +++ b/drivers/scsi/sd.c @@ -2883,9 +2883,6 @@ static void sd_read_block_limits(struct scsi_disk *sdkp) sdkp->max_ws_blocks = (u32)get_unaligned_be64(&buffer[36]); - if (!sdkp->lbpme) - goto out; - lba_count = get_unaligned_be32(&buffer[20]); desc_count = get_unaligned_be32(&buffer[24]); @@ -2898,6 +2895,9 @@ static void sd_read_block_limits(struct scsi_disk *sdkp) sdkp->unmap_alignment = get_unaligned_be32(&buffer[32]) & ~(1 << 31); + if (!sdkp->lbpme) + goto out; + if (!sdkp->lbpvpd) { /* LBP VPD page not provided */ if (sdkp->max_unmap_blocks) -- 2.14.1
[PATCH 2/2] libata-scsi: do not response with "invalid field" for FORMAT UNIT
From: Tom Yan It does not make sense and is confusing to response with "Invalid field in cbd" while we have no support at all implemented for FORMAT UNIT. It is decent to let it go to the default, which will response with "Invalid command operation code" instead. Signed-off-by: Tom Yan diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c index 80d732c..f70f9d1 100644 --- a/drivers/ata/libata-scsi.c +++ b/drivers/ata/libata-scsi.c @@ -4045,11 +4045,6 @@ void ata_scsi_simulate(struct ata_device *dev, struct scsi_cmnd *cmd) args.done = cmd->scsi_done; switch(scsicmd[0]) { - /* TODO: worth improving? */ - case FORMAT_UNIT: - ata_scsi_invalid_field(dev, cmd, 0); - break; - case INQUIRY: if (scsicmd[1] & 2)/* is CmdDt set? */ ata_scsi_invalid_field(dev, cmd, 1); -- 2.9.0 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] libata-scsi: improve TRIM translation
From: Tom Yan Define TRIM_RANGE_SIZE and TRIM_RANGE_NUM so that the corresponding functions can be more generalized. Also, conform SBC by rejecting WRITE SAME (16) commands with number of blocks that exceeds the limit that is defined in the SATL. Signed-off-by: Tom Yan diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c index bfec66f..80d732c 100644 --- a/drivers/ata/libata-scsi.c +++ b/drivers/ata/libata-scsi.c @@ -75,6 +75,9 @@ static struct ata_device *ata_scsi_find_dev(struct ata_port *ap, #define ALL_MPAGES 0x3f #define ALL_SUB_MPAGES 0xff +#define TRIM_RANGE_SIZE 0x +#define TRIM_RANGE_NUM 64 /* 512-byte payload / (6-byte LBA + 2-byte range per entry) */ + static const u8 def_rw_recovery_mpage[RW_RECOVERY_MPAGE_LEN] = { RW_RECOVERY_MPAGE, @@ -2314,7 +2317,7 @@ static unsigned int ata_scsiop_inq_b0(struct ata_scsi_args *args, u8 *rbuf) * with the unmap bit set. */ if (ata_id_has_trim(args->id)) { - put_unaligned_be64(65535 * 512 / 8, &rbuf[36]); + put_unaligned_be64(TRIM_RANGE_SIZE * TRIM_RANGE_NUM, &rbuf[36]); put_unaligned_be32(1, &rbuf[28]); } @@ -3305,7 +3308,10 @@ static unsigned int ata_scsi_write_same_xlat(struct ata_queued_cmd *qc) goto invalid_param_len; buf = page_address(sg_page(scsi_sglist(scmd))); - size = ata_set_lba_range_entries(buf, 512, block, n_block); + if (n_block <= TRIM_RANGE_SIZE * TRIM_RANGE_NUM) + size = ata_set_lba_range_entries(buf, TRIM_RANGE_NUM, block, n_block); + else + goto invalid_fld; if (ata_ncq_enabled(dev) && ata_fpdma_dsm_supported(dev)) { /* Newer devices support queued TRIM commands */ diff --git a/include/linux/ata.h b/include/linux/ata.h index 99346be..0971c3f 100644 --- a/include/linux/ata.h +++ b/include/linux/ata.h @@ -1071,7 +1071,7 @@ static inline unsigned ata_set_lba_range_entries(void *_buffer, __le64 *buffer = _buffer; unsigned i = 0, used_bytes; - while (i < buf_size / 8 ) { /* 6-byte LBA + 2-byte range per entry */ + while (i < buf_size) { u64 entry = sector | ((u64)(count > 0x ? 0x : count) << 48); buffer[i++] = __cpu_to_le64(entry); -- 2.9.0 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 2/2] libata-scsi: do not respond with "invalid field" for FORMAT UNIT
From: Tom Yan It does not make sense and is confusing to respond with "Invalid field in CDB" while we have no support at all implemented for FORMAT UNIT. It is decent to let it go to the default, which will respond with "Invalid command operation code" instead. Signed-off-by: Tom Yan diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c index 029e738..ac5676e 100644 --- a/drivers/ata/libata-scsi.c +++ b/drivers/ata/libata-scsi.c @@ -307,7 +307,7 @@ static void ata_scsi_set_invalid_field(struct ata_device *dev, struct scsi_cmnd *cmd, u16 field, u8 bit) { ata_scsi_set_sense(dev, cmd, ILLEGAL_REQUEST, 0x24, 0x0); - /* "Invalid field in cbd" */ + /* "Invalid field in CDB" */ scsi_set_sense_field_pointer(cmd->sense_buffer, SCSI_SENSE_BUFFERSIZE, field, bit, 1); } @@ -4045,11 +4045,6 @@ void ata_scsi_simulate(struct ata_device *dev, struct scsi_cmnd *cmd) args.done = cmd->scsi_done; switch(scsicmd[0]) { - /* TODO: worth improving? */ - case FORMAT_UNIT: - ata_scsi_invalid_field(dev, cmd, 0); - break; - case INQUIRY: if (scsicmd[1] & 2)/* is CmdDt set? */ ata_scsi_invalid_field(dev, cmd, 1); -- 2.9.0 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 1/2] libata-scsi: improve TRIM translation
From: Tom Yan Define TRIM_RANGE_SIZE and TRIM_RANGE_NUM so that the corresponding functions can be more generalized. Also, conform to SBC by rejecting WRITE SAME (16) commands with number of blocks that exceeds the limit that is defined in the SATL. Signed-off-by: Tom Yan diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c index bfec66f..029e738 100644 --- a/drivers/ata/libata-scsi.c +++ b/drivers/ata/libata-scsi.c @@ -75,6 +75,9 @@ static struct ata_device *ata_scsi_find_dev(struct ata_port *ap, #define ALL_MPAGES 0x3f #define ALL_SUB_MPAGES 0xff +#define TRIM_RANGE_SIZE 0x +#define TRIM_RANGE_NUM 64 /* 512-byte payload / (6-byte LBA + 2-byte range per entry) */ + static const u8 def_rw_recovery_mpage[RW_RECOVERY_MPAGE_LEN] = { RW_RECOVERY_MPAGE, @@ -2314,7 +2317,7 @@ static unsigned int ata_scsiop_inq_b0(struct ata_scsi_args *args, u8 *rbuf) * with the unmap bit set. */ if (ata_id_has_trim(args->id)) { - put_unaligned_be64(65535 * 512 / 8, &rbuf[36]); + put_unaligned_be64(TRIM_RANGE_SIZE * TRIM_RANGE_NUM, &rbuf[36]); put_unaligned_be32(1, &rbuf[28]); } @@ -3305,7 +3308,10 @@ static unsigned int ata_scsi_write_same_xlat(struct ata_queued_cmd *qc) goto invalid_param_len; buf = page_address(sg_page(scsi_sglist(scmd))); - size = ata_set_lba_range_entries(buf, 512, block, n_block); + if (n_block <= TRIM_RANGE_SIZE * TRIM_RANGE_NUM) + size = ata_set_lba_range_entries(buf, TRIM_RANGE_NUM, block, n_block); + else + goto invalid_fld; if (ata_ncq_enabled(dev) && ata_fpdma_dsm_supported(dev)) { /* Newer devices support queued TRIM commands */ diff --git a/include/linux/ata.h b/include/linux/ata.h index 99346be..0971c3f 100644 --- a/include/linux/ata.h +++ b/include/linux/ata.h @@ -1071,7 +1071,7 @@ static inline unsigned ata_set_lba_range_entries(void *_buffer, __le64 *buffer = _buffer; unsigned i = 0, used_bytes; - while (i < buf_size / 8 ) { /* 6-byte LBA + 2-byte range per entry */ + while (i < buf_size) { u64 entry = sector | ((u64)(count > 0x ? 0x : count) << 48); buffer[i++] = __cpu_to_le64(entry); -- 2.9.0 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 1/2] libata-scsi: reject WRITE SAME (16) with n_block that exceeds limit
From: Tom Yan Currently if a WRITE SAME (16) command is issued to the SATL with "number of blocks" that is larger than the "Maximum write same length" (which is the maximum number of blocks per TRIM command allowed in libata, currently 65535 * 512 / 8 blocks), the SATL will accept the command and translate it to a TRIM command with the upper limit. However, according to SBC (as of sbc4r11.pdf), the "device server" should terminate the command with "Invalid field in CBD" in that case. Signed-off-by: Tom Yan diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c index bfec66f..a1f061a 100644 --- a/drivers/ata/libata-scsi.c +++ b/drivers/ata/libata-scsi.c @@ -3305,7 +3305,11 @@ static unsigned int ata_scsi_write_same_xlat(struct ata_queued_cmd *qc) goto invalid_param_len; buf = page_address(sg_page(scsi_sglist(scmd))); - size = ata_set_lba_range_entries(buf, 512, block, n_block); + + if (n_block <= 65535 * 512 / 8) + size = ata_set_lba_range_entries(buf, 512, block, n_block); + else + goto invalid_fld; if (ata_ncq_enabled(dev) && ata_fpdma_dsm_supported(dev)) { /* Newer devices support queued TRIM commands */ -- 2.9.0 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 2/2] libata-scsi: avoid repeated calculation of number of TRIM ranges
From: Tom Yan Currently libata statically allows only 1-block (512-byte) payload for each TRIM command. Each payload can carry 64 TRIM ranges since each range requires 8 bytes. It is silly to keep doing the calculation (512 / 8) in different places. Hence, define the new ATA_MAX_TRIM_RNUM for the result. Signed-off-by: Tom Yan diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c index a1f061a..05a5f44 100644 --- a/drivers/ata/libata-scsi.c +++ b/drivers/ata/libata-scsi.c @@ -3306,8 +3306,8 @@ static unsigned int ata_scsi_write_same_xlat(struct ata_queued_cmd *qc) buf = page_address(sg_page(scsi_sglist(scmd))); - if (n_block <= 65535 * 512 / 8) - size = ata_set_lba_range_entries(buf, 512, block, n_block); + if (n_block <= 65535 * ATA_MAX_TRIM_RNUM) + size = ata_set_lba_range_entries(buf, ATA_MAX_TRIM_RNUM, block, n_block); else goto invalid_fld; diff --git a/include/linux/ata.h b/include/linux/ata.h index 99346be..ce59500 100644 --- a/include/linux/ata.h +++ b/include/linux/ata.h @@ -48,6 +48,7 @@ enum { ATA_MAX_SECTORS_1024= 1024, ATA_MAX_SECTORS_LBA48 = 65535,/* TODO: 65536? */ ATA_MAX_SECTORS_TAPE= 65535, + ATA_MAX_TRIM_RNUM = 64, /* 512-byte payload / (6-byte LBA + 2-byte range per entry) */ ATA_ID_WORDS= 256, ATA_ID_CONFIG = 0, @@ -1071,7 +1072,7 @@ static inline unsigned ata_set_lba_range_entries(void *_buffer, __le64 *buffer = _buffer; unsigned i = 0, used_bytes; - while (i < buf_size / 8 ) { /* 6-byte LBA + 2-byte range per entry */ + while (i < buf_size) { u64 entry = sector | ((u64)(count > 0x ? 0x : count) << 48); buffer[i++] = __cpu_to_le64(entry); -- 2.9.0 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 1/2] libata-scsi: reject WRITE SAME (16) with n_block that exceeds limit
From: Tom Yan Currently if a WRITE SAME (16) command is issued to the SATL with "number of blocks" that is larger than the "Maximum write same length" (which is the maximum number of blocks per TRIM command allowed in libata, currently 65535 * 512 / 8 blocks), the SATL will accept the command and translate it to a TRIM command with the upper limit. However, according to SBC (as of sbc4r11.pdf), the "device server" should terminate the command with "Invalid field in CBD" in that case. Signed-off-by: Tom Yan diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c index bfec66f..a1f061a 100644 --- a/drivers/ata/libata-scsi.c +++ b/drivers/ata/libata-scsi.c @@ -3305,7 +3305,11 @@ static unsigned int ata_scsi_write_same_xlat(struct ata_queued_cmd *qc) goto invalid_param_len; buf = page_address(sg_page(scsi_sglist(scmd))); - size = ata_set_lba_range_entries(buf, 512, block, n_block); + + if (n_block <= 65535 * 512 / 8) + size = ata_set_lba_range_entries(buf, 512, block, n_block); + else + goto invalid_fld; if (ata_ncq_enabled(dev) && ata_fpdma_dsm_supported(dev)) { /* Newer devices support queued TRIM commands */ -- 2.9.0 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 2/2] libata-scsi: avoid repeated calculation of number of TRIM ranges
From: Tom Yan Currently libata statically allows only 1-block (512-byte) payload for each TRIM command. Each payload can carry 64 TRIM ranges since each range requires 8 bytes. It is silly to keep doing the calculation (512 / 8) in different places. Hence, define the new ATA_MAX_TRIM_RNUM for the result. Signed-off-by: Tom Yan diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c index a1f061a..82739be 100644 --- a/drivers/ata/libata-scsi.c +++ b/drivers/ata/libata-scsi.c @@ -2314,7 +2314,7 @@ static unsigned int ata_scsiop_inq_b0(struct ata_scsi_args *args, u8 *rbuf) * with the unmap bit set. */ if (ata_id_has_trim(args->id)) { - put_unaligned_be64(65535 * 512 / 8, &rbuf[36]); + put_unaligned_be64(65535 * ATA_MAX_TRIM_RNUM, &rbuf[36]); put_unaligned_be32(1, &rbuf[28]); } @@ -3306,8 +3306,8 @@ static unsigned int ata_scsi_write_same_xlat(struct ata_queued_cmd *qc) buf = page_address(sg_page(scsi_sglist(scmd))); - if (n_block <= 65535 * 512 / 8) - size = ata_set_lba_range_entries(buf, 512, block, n_block); + if (n_block <= 65535 * ATA_MAX_TRIM_RNUM) + size = ata_set_lba_range_entries(buf, ATA_MAX_TRIM_RNUM, block, n_block); else goto invalid_fld; diff --git a/include/linux/ata.h b/include/linux/ata.h index 99346be..ce59500 100644 --- a/include/linux/ata.h +++ b/include/linux/ata.h @@ -48,6 +48,7 @@ enum { ATA_MAX_SECTORS_1024= 1024, ATA_MAX_SECTORS_LBA48 = 65535,/* TODO: 65536? */ ATA_MAX_SECTORS_TAPE= 65535, + ATA_MAX_TRIM_RNUM = 64, /* 512-byte payload / (6-byte LBA + 2-byte range per entry) */ ATA_ID_WORDS= 256, ATA_ID_CONFIG = 0, @@ -1071,7 +1072,7 @@ static inline unsigned ata_set_lba_range_entries(void *_buffer, __le64 *buffer = _buffer; unsigned i = 0, used_bytes; - while (i < buf_size / 8 ) { /* 6-byte LBA + 2-byte range per entry */ + while (i < buf_size) { u64 entry = sector | ((u64)(count > 0x ? 0x : count) << 48); buffer[i++] = __cpu_to_le64(entry); -- 2.9.0 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH resend 1/2] libata-scsi: do not respond with "invalid field" for FORMAT UNIT
From: Tom Yan It does not make sense and is confusing to respond with "Invalid field in CDB" while we have no support at all implemented for FORMAT UNIT. It is decent to let it go to the default, which will respond with "Invalid command operation code" instead. Signed-off-by: Tom Yan diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c index bfec66f..f1125fd 100644 --- a/drivers/ata/libata-scsi.c +++ b/drivers/ata/libata-scsi.c @@ -4039,11 +4039,6 @@ void ata_scsi_simulate(struct ata_device *dev, struct scsi_cmnd *cmd) args.done = cmd->scsi_done; switch(scsicmd[0]) { - /* TODO: worth improving? */ - case FORMAT_UNIT: - ata_scsi_invalid_field(dev, cmd, 0); - break; - case INQUIRY: if (scsicmd[1] & 2)/* is CmdDt set? */ ata_scsi_invalid_field(dev, cmd, 1); -- 2.9.0 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH resend 2/2] libata-scsi: correct cbd to CDB in comment
From: Tom Yan It's Command Descriptor Block. Also capitalized it. Signed-off-by: Tom Yan diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c index f1125fd..c9cd216 100644 --- a/drivers/ata/libata-scsi.c +++ b/drivers/ata/libata-scsi.c @@ -304,7 +304,7 @@ static void ata_scsi_set_invalid_field(struct ata_device *dev, struct scsi_cmnd *cmd, u16 field, u8 bit) { ata_scsi_set_sense(dev, cmd, ILLEGAL_REQUEST, 0x24, 0x0); - /* "Invalid field in cbd" */ + /* "Invalid field in CDB" */ scsi_set_sense_field_pointer(cmd->sense_buffer, SCSI_SENSE_BUFFERSIZE, field, bit, 1); } -- 2.9.0 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 1/2] libata-scsi: reject WRITE SAME (16) with n_block that exceeds limit
From: Tom Yan Currently if a WRITE SAME (16) command is issued to the SATL with "number of blocks" that is larger than the "Maximum write same length" (which is the maximum number of blocks per TRIM command allowed in libata, currently 65535 * 512 / 8 blocks), the SATL will accept the command and translate it to a TRIM command with the upper limit. However, according to SBC (as of sbc4r11.pdf), the "device server" should terminate the command with "Invalid field in CDB" in that case. Signed-off-by: Tom Yan diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c index bfec66f..a1f061a 100644 --- a/drivers/ata/libata-scsi.c +++ b/drivers/ata/libata-scsi.c @@ -3305,7 +3305,11 @@ static unsigned int ata_scsi_write_same_xlat(struct ata_queued_cmd *qc) goto invalid_param_len; buf = page_address(sg_page(scsi_sglist(scmd))); - size = ata_set_lba_range_entries(buf, 512, block, n_block); + + if (n_block <= 65535 * 512 / 8) + size = ata_set_lba_range_entries(buf, 512, block, n_block); + else + goto invalid_fld; if (ata_ncq_enabled(dev) && ata_fpdma_dsm_supported(dev)) { /* Newer devices support queued TRIM commands */ -- 2.9.0 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 2/2] libata-scsi: avoid repeated calculation of number of TRIM ranges
From: Tom Yan Currently libata statically allows only 1-block (512-byte) payload for each TRIM command. Each payload can carry 64 TRIM ranges since each range requires 8 bytes. It is silly to keep doing the calculation (512 / 8) in different places. Hence, define the new ATA_MAX_TRIM_RNUM for the result. Signed-off-by: Tom Yan diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c index a1f061a..82739be 100644 --- a/drivers/ata/libata-scsi.c +++ b/drivers/ata/libata-scsi.c @@ -2314,7 +2314,7 @@ static unsigned int ata_scsiop_inq_b0(struct ata_scsi_args *args, u8 *rbuf) * with the unmap bit set. */ if (ata_id_has_trim(args->id)) { - put_unaligned_be64(65535 * 512 / 8, &rbuf[36]); + put_unaligned_be64(65535 * ATA_MAX_TRIM_RNUM, &rbuf[36]); put_unaligned_be32(1, &rbuf[28]); } @@ -3306,8 +3306,8 @@ static unsigned int ata_scsi_write_same_xlat(struct ata_queued_cmd *qc) buf = page_address(sg_page(scsi_sglist(scmd))); - if (n_block <= 65535 * 512 / 8) - size = ata_set_lba_range_entries(buf, 512, block, n_block); + if (n_block <= 65535 * ATA_MAX_TRIM_RNUM) + size = ata_set_lba_range_entries(buf, ATA_MAX_TRIM_RNUM, block, n_block); else goto invalid_fld; diff --git a/include/linux/ata.h b/include/linux/ata.h index 99346be..ce59500 100644 --- a/include/linux/ata.h +++ b/include/linux/ata.h @@ -48,6 +48,7 @@ enum { ATA_MAX_SECTORS_1024= 1024, ATA_MAX_SECTORS_LBA48 = 65535,/* TODO: 65536? */ ATA_MAX_SECTORS_TAPE= 65535, + ATA_MAX_TRIM_RNUM = 64, /* 512-byte payload / (6-byte LBA + 2-byte range per entry) */ ATA_ID_WORDS= 256, ATA_ID_CONFIG = 0, @@ -1071,7 +1072,7 @@ static inline unsigned ata_set_lba_range_entries(void *_buffer, __le64 *buffer = _buffer; unsigned i = 0, used_bytes; - while (i < buf_size / 8 ) { /* 6-byte LBA + 2-byte range per entry */ + while (i < buf_size) { u64 entry = sector | ((u64)(count > 0x ? 0x : count) << 48); buffer[i++] = __cpu_to_le64(entry); -- 2.9.0 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] libata-scsi: do not return designator for serial number
From: Tom Yan SAT (as of sat4r05f.pdf) does not require this vendor specific designator. Besides, we already have the Unit Serial Number VPD. Signed-off-by: Tom Yan diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c index bfec66f..9f478ad 100644 --- a/drivers/ata/libata-scsi.c +++ b/drivers/ata/libata-scsi.c @@ -2210,14 +2210,6 @@ static unsigned int ata_scsiop_inq_83(struct ata_scsi_args *args, u8 *rbuf) rbuf[1] = 0x83; /* this page code */ num = 4; - /* piv=0, assoc=lu, code_set=ACSII, designator=vendor */ - rbuf[num + 0] = 2; - rbuf[num + 3] = ATA_ID_SERNO_LEN; - num += 4; - ata_id_string(args->id, (unsigned char *) rbuf + num, - ATA_ID_SERNO, ATA_ID_SERNO_LEN); - num += ATA_ID_SERNO_LEN; - /* SAT defined lu model and serial numbers descriptor */ /* piv=0, assoc=lu, code_set=ACSII, designator=t10 vendor id */ rbuf[num + 0] = 2; -- 2.9.0 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] libata-scsi: do not return t10 designator if drive has WWN
From: Tom Yan SAT (as of sat4r05f.pdf) only requires the t10 designator if the drive does not support/have WWN. Besides, we already have the ATA information VPD. Signed-off-by: Tom Yan diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c index 9f478ad..84b3d42 100644 --- a/drivers/ata/libata-scsi.c +++ b/drivers/ata/libata-scsi.c @@ -2210,21 +2210,6 @@ static unsigned int ata_scsiop_inq_83(struct ata_scsi_args *args, u8 *rbuf) rbuf[1] = 0x83; /* this page code */ num = 4; - /* SAT defined lu model and serial numbers descriptor */ - /* piv=0, assoc=lu, code_set=ACSII, designator=t10 vendor id */ - rbuf[num + 0] = 2; - rbuf[num + 1] = 1; - rbuf[num + 3] = sat_model_serial_desc_len; - num += 4; - memcpy(rbuf + num, "ATA ", 8); - num += 8; - ata_id_string(args->id, (unsigned char *) rbuf + num, ATA_ID_PROD, - ATA_ID_PROD_LEN); - num += ATA_ID_PROD_LEN; - ata_id_string(args->id, (unsigned char *) rbuf + num, ATA_ID_SERNO, - ATA_ID_SERNO_LEN); - num += ATA_ID_SERNO_LEN; - if (ata_id_has_wwn(args->id)) { /* SAT defined lu world wide name */ /* piv=0, assoc=lu, code_set=binary, designator=NAA */ @@ -2236,6 +2221,23 @@ static unsigned int ata_scsiop_inq_83(struct ata_scsi_args *args, u8 *rbuf) ATA_ID_WWN, ATA_ID_WWN_LEN); num += ATA_ID_WWN_LEN; } + else { + /* SAT defined lu model and serial numbers descriptor */ + /* piv=0, assoc=lu, code_set=ACSII, designator=t10 vendor id */ + rbuf[num + 0] = 2; + rbuf[num + 1] = 1; + rbuf[num + 3] = sat_model_serial_desc_len; + num += 4; + memcpy(rbuf + num, "ATA ", 8); + num += 8; + ata_id_string(args->id, (unsigned char *) rbuf + num, ATA_ID_PROD, + ATA_ID_PROD_LEN); + num += ATA_ID_PROD_LEN; + ata_id_string(args->id, (unsigned char *) rbuf + num, ATA_ID_SERNO, + ATA_ID_SERNO_LEN); + num += ATA_ID_SERNO_LEN; + } + rbuf[3] = num - 4;/* page len (assume less than 256 bytes) */ return 0; } -- 2.9.0 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] libata-scsi: better style in ata_msense_caching()
From: Tom Yan Signed-off-by: Tom Yan diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c index bfec66f..e3f5751 100644 --- a/drivers/ata/libata-scsi.c +++ b/drivers/ata/libata-scsi.c @@ -2424,10 +2424,12 @@ static void modecpy(u8 *dest, const u8 *src, int n, bool changeable) static unsigned int ata_msense_caching(u16 *id, u8 *buf, bool changeable) { modecpy(buf, def_cache_mpage, sizeof(def_cache_mpage), changeable); - if (changeable || ata_id_wcache_enabled(id)) - buf[2] |= (1 << 2); /* write cache enable */ - if (!changeable && !ata_id_rahead_enabled(id)) - buf[12] |= (1 << 5);/* disable read ahead */ + if (changeable) + buf[2] |= (1 << 2); /* ata_mselect_caching() */ + else { + buf[2] |= (ata_id_wcache_enabled(id) << 2); /* write cache enable */ + buf[12] |= (!ata_id_rahead_enabled(id) << 5); /* disable read ahead */ + } return sizeof(def_cache_mpage); } -- 2.9.0 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] libata-scsi: fix SET FEATURES "filtering" for ata_msense_caching()
From: Tom Yan Without this fix, the DRA bit of the caching mode page would not be updated when the read look-ahead feature is toggled (e.g. with `smartctl --set`), but will only be until, for example, the write cache feature is touched. Signed-off-by: Tom Yan diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c index 6be7770..077daf0 100644 --- a/drivers/ata/libata-core.c +++ b/drivers/ata/libata-core.c @@ -5127,7 +5127,9 @@ void ata_qc_complete(struct ata_queued_cmd *qc) switch (qc->tf.command) { case ATA_CMD_SET_FEATURES: if (qc->tf.feature != SETFEATURES_WC_ON && - qc->tf.feature != SETFEATURES_WC_OFF) + qc->tf.feature != SETFEATURES_WC_OFF && + qc->tf.feature != SETFEATURES_RA_ON && + qc->tf.feature != SETFEATURES_RA_OFF) break; /* fall through */ case ATA_CMD_INIT_DEV_PARAMS: /* CHS translation changed */ diff --git a/include/linux/ata.h b/include/linux/ata.h index 99346be..2d68793 100644 --- a/include/linux/ata.h +++ b/include/linux/ata.h @@ -409,6 +409,9 @@ enum { SETFEATURES_WC_ON = 0x02, /* Enable write cache */ SETFEATURES_WC_OFF = 0x82, /* Disable write cache */ + SETFEATURES_RA_ON = 0xaa, /* Enable read look-ahead */ + SETFEATURES_RA_OFF = 0x55, /* Disable read look-ahead */ + /* Enable/Disable Automatic Acoustic Management */ SETFEATURES_AAM_ON = 0x42, SETFEATURES_AAM_OFF = 0xC2, -- 2.9.0 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC] libata-scsi: introducing SANITIZE translation
From: Tom Yan With this patch, users can make use of the SANITIZE DEVICE feature set through utility like sg_sanitize. Support for BLOCK ERASE, CRYPTOGRAPHIC ERASE and EXIT FAILURE MODE has been implemented. Support for OVERWRITE that involves a parameter list has been left out for now. Further support for command with IMMED bit set to zero, REQUEST SENSE translation for user-space status polling, and support checking in IDENTIFY DEVICE data log (return proper sense data when designated method is not supported) should be implemented in the future as well. `sg_sanitize -e -B|-C|-F /dev/sdX` should work fine with this. Signed-off-by: Tom Yan diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c index bfec66f..a64991b 100644 --- a/drivers/ata/libata-scsi.c +++ b/drivers/ata/libata-scsi.c @@ -3346,6 +3346,63 @@ invalid_opcode: return 1; } +static unsigned int ata_scsi_sanitize_xlat(struct ata_queued_cmd *qc) { + struct ata_taskfile *tf = &qc->tf; + struct scsi_cmnd *scmd = qc->scsicmd; + struct ata_device *dev = qc->dev; + const u8 *cdb = scmd->cmnd; + u16 fp; + u8 bp = 0xff; + + /* for now we only support SANITIZE with IMMED bit set */ + if (unlikely(!(cdb[1] & 0x80))) { + fp = 1; + bp = 7; + goto invalid_fld; + } + + tf->protocol = ATA_PROT_NODATA; + tf->command = ATA_CMD_SANITIZE_DEVICE; + tf->hob_nsect |= (cdb[1] & 0x40) << 1; + tf->nsect |= (cdb[1] & 0x20) >> 1; + tf->flags |= ATA_TFLAG_LBA48 | ATA_TFLAG_ISADDR; + + switch (cdb[1] & 0x1f) { + /* TODO: add support for OVERWRITE */ + case 0x2: /* BLOCK ERASE */ + tf->hob_feature = 0x0; + tf->feature = 0x12; + tf->hob_lbal = 0x42; + tf->lbah = 0x6b; + tf->lbam = 0x45; + tf->lbal = 0x72; + break; + case 0x3: /* CRYPTOGRAPHIC ERASE */ + tf->hob_feature = 0x0; + tf->feature = 0x11; + tf->hob_lbal = 0x43; + tf->lbah = 0x72; + tf->lbam = 0x79; + tf->lbal = 0x70; + break; + case 0x1f: /* EXIT FAILURE MODE */ + tf->hob_feature = 0x0; + tf->feature = 0x0; + tf->nsect |= 0x1; + break; + default: + fp = 1; + bp = 4; + goto invalid_fld; + } + + return 0; + +invalid_fld: + ata_scsi_set_invalid_field(dev, scmd, fp, bp); + return 1; +} + /** * ata_scsi_report_zones_complete - convert ATA output * @qc: command structure returning the data @@ -3869,6 +3926,9 @@ static inline ata_xlat_func_t ata_get_xlat_func(struct ata_device *dev, u8 cmd) case WRITE_SAME_16: return ata_scsi_write_same_xlat; + case SANITIZE: + return ata_scsi_sanitize_xlat; + case SYNCHRONIZE_CACHE: if (ata_try_flush_cache(dev)) return ata_scsi_flush_xlat; diff --git a/include/scsi/scsi_proto.h b/include/scsi/scsi_proto.h index d1defd1..20d6085 100644 --- a/include/scsi/scsi_proto.h +++ b/include/scsi/scsi_proto.h @@ -72,6 +72,7 @@ #define UNMAP0x42 #define READ_TOC 0x43 #define READ_HEADER 0x44 +#define SANITIZE 0x48 #define GET_EVENT_STATUS_NOTIFICATION 0x4a #define LOG_SELECT0x4c #define LOG_SENSE 0x4d -- 2.9.0 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 1/2] libata-scsi: fix SET FEATURES "filtering" for ata_msense_caching()
From: Tom Yan Without this fix, the DRA bit of the caching mode page would not be updated when the read look-ahead feature is toggled (e.g. with `smartctl --set`), but will only be until, for example, the write cache feature is touched. Signed-off-by: Tom Yan diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c index 6be7770..077daf0 100644 --- a/drivers/ata/libata-core.c +++ b/drivers/ata/libata-core.c @@ -5127,7 +5127,9 @@ void ata_qc_complete(struct ata_queued_cmd *qc) switch (qc->tf.command) { case ATA_CMD_SET_FEATURES: if (qc->tf.feature != SETFEATURES_WC_ON && - qc->tf.feature != SETFEATURES_WC_OFF) + qc->tf.feature != SETFEATURES_WC_OFF && + qc->tf.feature != SETFEATURES_RA_ON && + qc->tf.feature != SETFEATURES_RA_OFF) break; /* fall through */ case ATA_CMD_INIT_DEV_PARAMS: /* CHS translation changed */ diff --git a/include/linux/ata.h b/include/linux/ata.h index 99346be..2d68793 100644 --- a/include/linux/ata.h +++ b/include/linux/ata.h @@ -409,6 +409,9 @@ enum { SETFEATURES_WC_ON = 0x02, /* Enable write cache */ SETFEATURES_WC_OFF = 0x82, /* Disable write cache */ + SETFEATURES_RA_ON = 0xaa, /* Enable read look-ahead */ + SETFEATURES_RA_OFF = 0x55, /* Disable read look-ahead */ + /* Enable/Disable Automatic Acoustic Management */ SETFEATURES_AAM_ON = 0x42, SETFEATURES_AAM_OFF = 0xC2, -- 2.9.0 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 2/2] libata-scsi: better style in ata_msense_caching()
From: Tom Yan Signed-off-by: Tom Yan diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c index bfec66f..6f7c626 100644 --- a/drivers/ata/libata-scsi.c +++ b/drivers/ata/libata-scsi.c @@ -2424,10 +2424,13 @@ static void modecpy(u8 *dest, const u8 *src, int n, bool changeable) static unsigned int ata_msense_caching(u16 *id, u8 *buf, bool changeable) { modecpy(buf, def_cache_mpage, sizeof(def_cache_mpage), changeable); - if (changeable || ata_id_wcache_enabled(id)) - buf[2] |= (1 << 2); /* write cache enable */ - if (!changeable && !ata_id_rahead_enabled(id)) - buf[12] |= (1 << 5);/* disable read ahead */ + if (changeable) { + buf[2] |= 1 << 2; /* ata_mselect_caching() */ + } + else { + buf[2] |= ata_id_wcache_enabled(id) << 2; /* write cache enable */ + buf[12] |= !ata_id_rahead_enabled(id) << 5; /* disable read ahead */ + } return sizeof(def_cache_mpage); } -- 2.9.0 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH resend 1/2] libata-scsi: set CmdQue bit in standard INQUIRY data to 1
From: Tom Yan Avoid performance bottleneck when being SCSI pass-through'd to virtual machines with other OSes (e.g. Windows) via virtio-scsi and scsi-block in qemu. Ref.: https://github.com/YanVugenfirer/kvm-guest-drivers-windows/issues/63 Signed-off-by: Tom Yan diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c index bfec66f..0a35164 100644 --- a/drivers/ata/libata-scsi.c +++ b/drivers/ata/libata-scsi.c @@ -2097,7 +2097,10 @@ static unsigned int ata_scsiop_inq_std(struct ata_scsi_args *args, u8 *rbuf) 0, 0x5,/* claim SPC-3 version compatibility */ 2, - 95 - 4 + 95 - 4, + 0, + 0, + 2 }; VPRINTK("ENTER\n"); -- 2.9.0 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH resend 2/2] libata-scsi: correct SPC version descriptor
From: Tom Yan The comment suggests we should be having an SPC-3 version descriptor but the 0260h is the code for "SPC-2 (no version claimed)". Correct it to 0300h so that it has the "SPC-3 (no version claimed)" descriptor. Note that we are claiming SPC-3 version compatibility in the VERSION field of the standard INQUIRY data. Therefore, I assume the typo was on the code but not on the comment. Signed-off-by: Tom Yan diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c index 0a35164..8221800 100644 --- a/drivers/ata/libata-scsi.c +++ b/drivers/ata/libata-scsi.c @@ -2075,8 +2075,8 @@ static unsigned int ata_scsiop_inq_std(struct ata_scsi_args *args, u8 *rbuf) 0x03, 0x20, /* SBC-2 (no version claimed) */ - 0x02, - 0x60/* SPC-3 (no version claimed) */ + 0x03, + 0x00/* SPC-3 (no version claimed) */ }; const u8 versions_zbc[] = { 0x00, -- 2.9.0 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 2/2] libata-scsi: better style in ata_msense_caching()
From: Tom Yan Signed-off-by: Tom Yan diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c index bfec66f..48ea887 100644 --- a/drivers/ata/libata-scsi.c +++ b/drivers/ata/libata-scsi.c @@ -2424,10 +2424,12 @@ static void modecpy(u8 *dest, const u8 *src, int n, bool changeable) static unsigned int ata_msense_caching(u16 *id, u8 *buf, bool changeable) { modecpy(buf, def_cache_mpage, sizeof(def_cache_mpage), changeable); - if (changeable || ata_id_wcache_enabled(id)) - buf[2] |= (1 << 2); /* write cache enable */ - if (!changeable && !ata_id_rahead_enabled(id)) - buf[12] |= (1 << 5);/* disable read ahead */ + if (changeable) { + buf[2] |= 1 << 2; /* ata_mselect_caching() */ + } else { + buf[2] |= ata_id_wcache_enabled(id) << 2; /* write cache enable */ + buf[12] |= !ata_id_rahead_enabled(id) << 5; /* disable read ahead */ + } return sizeof(def_cache_mpage); } -- 2.9.0 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 1/2] libata-scsi: fix SET FEATURES "filtering" for ata_msense_caching()
From: Tom Yan Without this fix, the DRA bit of the caching mode page would not be updated when the read look-ahead feature is toggled (e.g. with `smartctl --set`), but will only be until, for example, the write cache feature is touched. Signed-off-by: Tom Yan diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c index 6be7770..077daf0 100644 --- a/drivers/ata/libata-core.c +++ b/drivers/ata/libata-core.c @@ -5127,7 +5127,9 @@ void ata_qc_complete(struct ata_queued_cmd *qc) switch (qc->tf.command) { case ATA_CMD_SET_FEATURES: if (qc->tf.feature != SETFEATURES_WC_ON && - qc->tf.feature != SETFEATURES_WC_OFF) + qc->tf.feature != SETFEATURES_WC_OFF && + qc->tf.feature != SETFEATURES_RA_ON && + qc->tf.feature != SETFEATURES_RA_OFF) break; /* fall through */ case ATA_CMD_INIT_DEV_PARAMS: /* CHS translation changed */ diff --git a/include/linux/ata.h b/include/linux/ata.h index 99346be..2d68793 100644 --- a/include/linux/ata.h +++ b/include/linux/ata.h @@ -409,6 +409,9 @@ enum { SETFEATURES_WC_ON = 0x02, /* Enable write cache */ SETFEATURES_WC_OFF = 0x82, /* Disable write cache */ + SETFEATURES_RA_ON = 0xaa, /* Enable read look-ahead */ + SETFEATURES_RA_OFF = 0x55, /* Disable read look-ahead */ + /* Enable/Disable Automatic Acoustic Management */ SETFEATURES_AAM_ON = 0x42, SETFEATURES_AAM_OFF = 0xC2, -- 2.9.0 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] libata-scsi: set correct VERSION field for ZAC devices
From: Tom Yan Commit 856c46639309 ("libata: support device-managed ZAC devices") had the line that "bumps" the VERSION field in standard INQUIRY data removed. Add it back and claim SPC-5 version compatibility, which matches with the current version descriptor "SPC-5 (no version claimed)" that is used for ZAC devices. Signed-off-by: Tom Yan diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c index bfec66f..27e29e1 100644 --- a/drivers/ata/libata-scsi.c +++ b/drivers/ata/libata-scsi.c @@ -2109,8 +2109,10 @@ static unsigned int ata_scsiop_inq_std(struct ata_scsi_args *args, u8 *rbuf) (args->dev->link->ap->pflags & ATA_PFLAG_EXTERNAL)) hdr[1] |= (1 << 7); - if (args->dev->class == ATA_DEV_ZAC) + if (args->dev->class == ATA_DEV_ZAC) { hdr[0] = TYPE_ZBC; + hdr[2] = 0x7; /* claim SPC-5 version compatibility */ + } memcpy(rbuf, hdr, sizeof(hdr)); memcpy(&rbuf[8], "ATA ", 8); -- 2.9.0 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] libata-scsi: fix D_SENSE bit relection in control mode page
From: Tom Yan The bit should always be set to 1 when the requested version of page is "changeable" because we've made it so in ata_mselect_control(). Also, it should always be set to 1 if ATA_DFLAG_D_SENSE is set (when the requested version of page is "current" or "default"). Signed-off-by: Tom Yan diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c index bfec66f..7e24f0a 100644 --- a/drivers/ata/libata-scsi.c +++ b/drivers/ata/libata-scsi.c @@ -2446,7 +2446,7 @@ static unsigned int ata_msense_ctl_mode(struct ata_device *dev, u8 *buf, bool changeable) { modecpy(buf, def_control_mpage, sizeof(def_control_mpage), changeable); - if (changeable && (dev->flags & ATA_DFLAG_D_SENSE)) + if (changeable || (dev->flags & ATA_DFLAG_D_SENSE)) buf[2] |= (1 << 2); /* Descriptor sense requested */ return sizeof(def_control_mpage); } -- 2.9.0 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 2/2] libata-scsi: rename ata_msense_ctl_mode() to ata_msense_control()
From: Tom Yan To make it consistent with the recently added ata_mselect_control(). We probably shouldn't have the word "mode" in its name anyway, since that's not the case for other ata_msense_*() / ata_mselect_*() either. Signed-off-by: Tom Yan diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c index 7e24f0a..bf4cb21 100644 --- a/drivers/ata/libata-scsi.c +++ b/drivers/ata/libata-scsi.c @@ -2432,7 +2432,7 @@ static unsigned int ata_msense_caching(u16 *id, u8 *buf, bool changeable) } /** - * ata_msense_ctl_mode - Simulate MODE SENSE control mode page + * ata_msense_control - Simulate MODE SENSE control mode page * @dev: ATA device of interest * @buf: output buffer * @changeable: whether changeable parameters are requested @@ -2442,7 +2442,7 @@ static unsigned int ata_msense_caching(u16 *id, u8 *buf, bool changeable) * LOCKING: * None. */ -static unsigned int ata_msense_ctl_mode(struct ata_device *dev, u8 *buf, +static unsigned int ata_msense_control(struct ata_device *dev, u8 *buf, bool changeable) { modecpy(buf, def_control_mpage, sizeof(def_control_mpage), changeable); @@ -2566,13 +2566,13 @@ static unsigned int ata_scsiop_mode_sense(struct ata_scsi_args *args, u8 *rbuf) break; case CONTROL_MPAGE: - p += ata_msense_ctl_mode(args->dev, p, page_control == 1); + p += ata_msense_control(args->dev, p, page_control == 1); break; case ALL_MPAGES: p += ata_msense_rw_recovery(p, page_control == 1); p += ata_msense_caching(args->id, p, page_control == 1); - p += ata_msense_ctl_mode(args->dev, p, page_control == 1); + p += ata_msense_control(args->dev, p, page_control == 1); break; default:/* invalid page code */ @@ -3667,7 +3667,7 @@ static int ata_mselect_control(struct ata_queued_cmd *qc, /* * Check that read-only bits are not modified. */ - ata_msense_ctl_mode(dev, mpage, false); + ata_msense_control(dev, mpage, false); for (i = 0; i < CONTROL_MPAGE_LEN - 2; i++) { if (i == 0) continue; -- 2.9.0 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 1/2] libata-scsi: fix D_SENSE bit relection in control mode page
From: Tom Yan The bit should always be set to 1 when the requested version of page is "changeable" because we've made it so in ata_mselect_control(). Also, it should always be set to 1 if ATA_DFLAG_D_SENSE is set (when the requested version of page is "current" or "default"). Signed-off-by: Tom Yan diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c index bfec66f..7e24f0a 100644 --- a/drivers/ata/libata-scsi.c +++ b/drivers/ata/libata-scsi.c @@ -2446,7 +2446,7 @@ static unsigned int ata_msense_ctl_mode(struct ata_device *dev, u8 *buf, bool changeable) { modecpy(buf, def_control_mpage, sizeof(def_control_mpage), changeable); - if (changeable && (dev->flags & ATA_DFLAG_D_SENSE)) + if (changeable || (dev->flags & ATA_DFLAG_D_SENSE)) buf[2] |= (1 << 2); /* Descriptor sense requested */ return sizeof(def_control_mpage); } -- 2.9.0 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6 2/2] libata-scsi: avoid repeated calculation of number of TRIM ranges
From: Tom Yan Currently libata statically allows only 1-block (512-byte) payload for each TRIM command. Each payload can carry 64 TRIM ranges since each range requires 8 bytes. It is silly to keep doing the calculation (512 / 8) in different places. Hence, define the new ATA_MAX_TRIM_RNUM for the result. Signed-off-by: Tom Yan diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c index 392aebb..62fb5b9 100644 --- a/drivers/ata/libata-scsi.c +++ b/drivers/ata/libata-scsi.c @@ -2314,7 +2314,7 @@ static unsigned int ata_scsiop_inq_b0(struct ata_scsi_args *args, u8 *rbuf) * with the unmap bit set. */ if (ata_id_has_trim(args->id)) { - put_unaligned_be64(65535 * 512 / 8, &rbuf[36]); + put_unaligned_be64(65535 * ATA_MAX_TRIM_RNUM, &rbuf[36]); put_unaligned_be32(1, &rbuf[28]); } @@ -3306,8 +3306,8 @@ static unsigned int ata_scsi_write_same_xlat(struct ata_queued_cmd *qc) buf = page_address(sg_page(scsi_sglist(scmd))); - if (n_block <= 65535 * 512 / 8) { - size = ata_set_lba_range_entries(buf, 512, block, n_block); + if (n_block <= 65535 * ATA_MAX_TRIM_RNUM) { + size = ata_set_lba_range_entries(buf, ATA_MAX_TRIM_RNUM, block, n_block); } else { fp = 2; goto invalid_fld; diff --git a/include/linux/ata.h b/include/linux/ata.h index 99346be..2d2d072 100644 --- a/include/linux/ata.h +++ b/include/linux/ata.h @@ -48,6 +48,7 @@ enum { ATA_MAX_SECTORS_1024= 1024, ATA_MAX_SECTORS_LBA48 = 65535,/* TODO: 65536? */ ATA_MAX_SECTORS_TAPE= 65535, + ATA_MAX_TRIM_RNUM = 64, /* 512-byte payload / (6-byte LBA + 2-byte range per entry) */ ATA_ID_WORDS= 256, ATA_ID_CONFIG = 0, @@ -1066,12 +1067,12 @@ static inline void ata_id_to_hd_driveid(u16 *id) * TO NV CACHE PINNED SET. */ static inline unsigned ata_set_lba_range_entries(void *_buffer, - unsigned buf_size, u64 sector, unsigned long count) + unsigned num, u64 sector, unsigned long count) { __le64 *buffer = _buffer; unsigned i = 0, used_bytes; - while (i < buf_size / 8 ) { /* 6-byte LBA + 2-byte range per entry */ + while (i < num) { u64 entry = sector | ((u64)(count > 0x ? 0x : count) << 48); buffer[i++] = __cpu_to_le64(entry); -- 2.9.0 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6 1/2] libata-scsi: reject WRITE SAME (16) with n_block that exceeds limit
From: Tom Yan Currently if a WRITE SAME (16) command is issued to the SATL with "number of blocks" that is larger than the "Maximum write same length" (which is the maximum number of blocks per TRIM command allowed in libata, currently 65535 * 512 / 8 blocks), the SATL will accept the command and translate it to a TRIM command with the upper limit. However, according to SBC (as of sbc4r11.pdf), the "device server" should terminate the command with "Invalid field in CDB" in that case. Signed-off-by: Tom Yan diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c index bfec66f..392aebb 100644 --- a/drivers/ata/libata-scsi.c +++ b/drivers/ata/libata-scsi.c @@ -3305,7 +3305,13 @@ static unsigned int ata_scsi_write_same_xlat(struct ata_queued_cmd *qc) goto invalid_param_len; buf = page_address(sg_page(scsi_sglist(scmd))); - size = ata_set_lba_range_entries(buf, 512, block, n_block); + + if (n_block <= 65535 * 512 / 8) { + size = ata_set_lba_range_entries(buf, 512, block, n_block); + } else { + fp = 2; + goto invalid_fld; + } if (ata_ncq_enabled(dev) && ata_fpdma_dsm_supported(dev)) { /* Newer devices support queued TRIM commands */ -- 2.9.0 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC 3/3] libata-scsi: add optimal transfer length to block limits VPD
From: Tom Yan As of commit 6b7e9cde4969 ("sd: Fix rw_max for devices that report an optimal xfer size"), the scsi disk driver (correctly) derive both of the queue limits "io_opt" and "max_sectors" from the optimal transfer length field. In case we would like the two limits to be derived from a value other than BLK_DEF_MAX_SECTORS for ATA disks in the future, this patch has made it easy. Signed-off-by: Tom Yan diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c index bfec66f..ab75b5e 100644 --- a/drivers/ata/libata-scsi.c +++ b/drivers/ata/libata-scsi.c @@ -2305,6 +2305,13 @@ static unsigned int ata_scsiop_inq_b0(struct ata_scsi_args *args, u8 *rbuf) put_unaligned_be16(min_io_sectors, &rbuf[6]); /* +* Optimal transfer length. +* +* This is used to derive the queue limit "max_sector" and "io_opt". +*/ + put_unaligned_be32(BLK_DEF_MAX_SECTORS, &rbuf[12]); + + /* * Optimal unmap granularity. * * The ATA spec doesn't even know about a granularity or alignment -- 2.9.0 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC 1/3] ata: bump ATA_MAX_SECTORS_LBA48 to 65536
From: Tom Yan ATA_MAX_SECTORS_LBA48 is only used for setting the queue limit "max_hw_sectors", which only serves as the cap for "max_sectors", which is in turn the actual limit being used. Therefore, it should be alright for us to bump ATA_MAX_SECTORS_LBA48 to 65536. Also, lba_48_ok() has been accepting the number of blocks to be 65536. Signed-off-by: Tom Yan diff --git a/include/linux/ata.h b/include/linux/ata.h index 99346be..24f886c 100644 --- a/include/linux/ata.h +++ b/include/linux/ata.h @@ -46,7 +46,7 @@ enum { ATA_MAX_SECTORS_128 = 128, ATA_MAX_SECTORS = 256, ATA_MAX_SECTORS_1024= 1024, - ATA_MAX_SECTORS_LBA48 = 65535,/* TODO: 65536? */ + ATA_MAX_SECTORS_LBA48 = 65536, ATA_MAX_SECTORS_TAPE= 65535, ATA_ID_WORDS= 256, -- 2.9.0 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC 2/3] ata: make lba_{28,48}_ok() use ATA_MAX_SECTORS{,_LBA48}
From: Tom Yan Signed-off-by: Tom Yan diff --git a/include/linux/ata.h b/include/linux/ata.h index 24f886c..d4bb802 100644 --- a/include/linux/ata.h +++ b/include/linux/ata.h @@ -1095,13 +1095,13 @@ static inline bool ata_ok(u8 status) static inline bool lba_28_ok(u64 block, u32 n_block) { /* check the ending block number: must be LESS THAN 0x0fff */ - return ((block + n_block) < ((1 << 28) - 1)) && (n_block <= 256); + return ((block + n_block) < ((1 << 28) - 1)) && (n_block <= ATA_MAX_SECTORS); } static inline bool lba_48_ok(u64 block, u32 n_block) { /* check the ending block number */ - return ((block + n_block - 1) < ((u64)1 << 48)) && (n_block <= 65536); + return ((block + n_block - 1) < ((u64)1 << 48)) && (n_block <= ATA_MAX_SECTORS_LBA48); } #define sata_pmp_gscr_vendor(gscr) ((gscr)[SATA_PMP_GSCR_PROD_ID] & 0x) -- 2.9.0 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4] libata-scsi: better style in ata_msense_*()
From: Tom Yan `changeable` is the "version" of mode page requested by the user. It will be less confusing/misleading if we do not check it "together" with the setting bits of the drive. Not to mention that we currently have ata_mselect_*() implemented in a way that each of them will serve exclusively a particular bit on each page. The old style will hence make the condition look even more unnecessarily arcane if the ata_msense_*() is reflecting more than one bit. Signed-off-by: Tom Yan diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c index bf4cb21..b00521b 100644 --- a/drivers/ata/libata-scsi.c +++ b/drivers/ata/libata-scsi.c @@ -2424,10 +2424,12 @@ static void modecpy(u8 *dest, const u8 *src, int n, bool changeable) static unsigned int ata_msense_caching(u16 *id, u8 *buf, bool changeable) { modecpy(buf, def_cache_mpage, sizeof(def_cache_mpage), changeable); - if (changeable || ata_id_wcache_enabled(id)) - buf[2] |= (1 << 2); /* write cache enable */ - if (!changeable && !ata_id_rahead_enabled(id)) - buf[12] |= (1 << 5);/* disable read ahead */ + if (changeable) { + buf[2] |= (1 << 2); /* ata_mselect_caching() */ + } else { + buf[2] |= (ata_id_wcache_enabled(id) << 2); /* write cache enable */ + buf[12] |= (!ata_id_rahead_enabled(id) << 5); /* disable read ahead */ + } return sizeof(def_cache_mpage); } @@ -2446,8 +2448,13 @@ static unsigned int ata_msense_control(struct ata_device *dev, u8 *buf, bool changeable) { modecpy(buf, def_control_mpage, sizeof(def_control_mpage), changeable); - if (changeable || (dev->flags & ATA_DFLAG_D_SENSE)) - buf[2] |= (1 << 2); /* Descriptor sense requested */ + if (changeable) { + buf[2] |= (1 << 2); /* ata_mselect_control() */ + } else { + bool d_sense = (dev->flags & ATA_DFLAG_D_SENSE); + + buf[2] |= (d_sense << 2); /* descriptor format sense data */ + } return sizeof(def_control_mpage); } -- 2.9.0 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4] libata-scsi: minor cleanup in ata_mselect_*()
From: Tom Yan 1. Removed a repeated bit masking in ata_mselect_control() 2. Moved `wce`/`d_sense` assignment below the page validity checks 3. Added/Removed empty lines where appropriate Signed-off-by: Tom Yan diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c index b00521b..06afe63 100644 --- a/drivers/ata/libata-scsi.c +++ b/drivers/ata/libata-scsi.c @@ -3604,7 +3604,6 @@ static int ata_mselect_caching(struct ata_queued_cmd *qc, * The first two bytes of def_cache_mpage are a header, so offsets * in mpage are off by 2 compared to buf. Same for len. */ - if (len != CACHE_MPAGE_LEN - 2) { if (len < CACHE_MPAGE_LEN - 2) *fp = len; @@ -3613,8 +3612,6 @@ static int ata_mselect_caching(struct ata_queued_cmd *qc, return -EINVAL; } - wce = buf[0] & (1 << 2); - /* * Check that read-only bits are not modified. */ @@ -3628,6 +3625,8 @@ static int ata_mselect_caching(struct ata_queued_cmd *qc, } } + wce = buf[0] & (1 << 2); + tf->flags |= ATA_TFLAG_DEVICE | ATA_TFLAG_ISADDR; tf->protocol = ATA_PROT_NODATA; tf->nsect = 0; @@ -3660,7 +3659,6 @@ static int ata_mselect_control(struct ata_queued_cmd *qc, * The first two bytes of def_control_mpage are a header, so offsets * in mpage are off by 2 compared to buf. Same for len. */ - if (len != CONTROL_MPAGE_LEN - 2) { if (len < CONTROL_MPAGE_LEN - 2) *fp = len; @@ -3669,8 +3667,6 @@ static int ata_mselect_control(struct ata_queued_cmd *qc, return -EINVAL; } - d_sense = buf[0] & (1 << 2); - /* * Check that read-only bits are not modified. */ @@ -3683,7 +3679,10 @@ static int ata_mselect_control(struct ata_queued_cmd *qc, return -EINVAL; } } - if (d_sense & (1 << 2)) + + d_sense = buf[0] & (1 << 2); + + if (d_sense) dev->flags |= ATA_DFLAG_D_SENSE; else dev->flags &= ~ATA_DFLAG_D_SENSE; -- 2.9.0 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] libata-scsi: fix read-only bits checking in ata_mselect_*()
From: Tom Yan Commit 7780081c1f04 ("libata-scsi: Set information sense field for invalid parameter") changed how ata_mselect_*() make sure read-only bits are not modified. The new implementation introduced a bug that the read-only bits in the byte that has a changeable bit will not be checked. Added the necessary check, with comments explaining the heuristic. Signed-off-by: Tom Yan diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c index 06afe63..005d186 100644 --- a/drivers/ata/libata-scsi.c +++ b/drivers/ata/libata-scsi.c @@ -3617,8 +3617,18 @@ static int ata_mselect_caching(struct ata_queued_cmd *qc, */ ata_msense_caching(dev->id, mpage, false); for (i = 0; i < CACHE_MPAGE_LEN - 2; i++) { - if (i == 0) - continue; + /* Check the first byte */ + if (i == 0) { + /* except the WCE bit */ + if (mpage[i + 2] & 0xfb != buf[i] & 0xfb) { + continue; + } else { + *fp = i; + return -EINVAL; + } + } + + /* Check the remaining bytes */ if (mpage[i + 2] != buf[i]) { *fp = i; return -EINVAL; @@ -3672,8 +3682,18 @@ static int ata_mselect_control(struct ata_queued_cmd *qc, */ ata_msense_control(dev, mpage, false); for (i = 0; i < CONTROL_MPAGE_LEN - 2; i++) { - if (i == 0) - continue; + /* Check the first byte */ + if (i == 0) { + /* except the D_SENSE bit */ + if (mpage[i + 2] & 0xfb != buf[i] & 0xfb) { + continue; + } else { + *fp = i; + return -EINVAL; + } + } + + /* Check the remaining bytes */ if (mpage[2 + i] != buf[i]) { *fp = i; return -EINVAL; -- 2.9.0 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] libata-scsi: fix read-only bits checking in ata_mselect_*()
From: Tom Yan Commit 7780081c1f04 ("libata-scsi: Set information sense field for invalid parameter") changed how ata_mselect_*() make sure read-only bits are not modified. The new implementation introduced a bug that the read-only bits in the byte that has a changeable bit will not be checked. Added the necessary check, with comments explaining the heuristic. Signed-off-by: Tom Yan diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c index 06afe63..b47c3ce 100644 --- a/drivers/ata/libata-scsi.c +++ b/drivers/ata/libata-scsi.c @@ -3617,8 +3617,18 @@ static int ata_mselect_caching(struct ata_queued_cmd *qc, */ ata_msense_caching(dev->id, mpage, false); for (i = 0; i < CACHE_MPAGE_LEN - 2; i++) { - if (i == 0) - continue; + /* Check the first byte */ + if (i == 0) { + /* except the WCE bit */ + if (mpage[i + 2] & 0xfb != buf[i] & 0xfb) { + *fp = i; + return -EINVAL; + } else { + continue; + } + } + + /* Check the remaining bytes */ if (mpage[i + 2] != buf[i]) { *fp = i; return -EINVAL; @@ -3672,8 +3682,18 @@ static int ata_mselect_control(struct ata_queued_cmd *qc, */ ata_msense_control(dev, mpage, false); for (i = 0; i < CONTROL_MPAGE_LEN - 2; i++) { - if (i == 0) - continue; + /* Check the first byte */ + if (i == 0) { + /* except the D_SENSE bit */ + if (mpage[i + 2] & 0xfb != buf[i] & 0xfb) { + *fp = i; + return -EINVAL; + } else { + continue; + } + } + + /* Check the remaining bytes */ if (mpage[2 + i] != buf[i]) { *fp = i; return -EINVAL; -- 2.9.0 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH resend 1/5] libata-scsi: minor cleanup in ata_mselect_*()
From: Tom Yan 1. Removed a repeated bit masking in ata_mselect_control() 2. Moved `wce`/`d_sense` assignment below the page validity checks 3. Added/Removed empty lines where appropriate Signed-off-by: Tom Yan diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c index 2bdb5da..eb5e8ff 100644 --- a/drivers/ata/libata-scsi.c +++ b/drivers/ata/libata-scsi.c @@ -3618,7 +3618,6 @@ static int ata_mselect_caching(struct ata_queued_cmd *qc, * The first two bytes of def_cache_mpage are a header, so offsets * in mpage are off by 2 compared to buf. Same for len. */ - if (len != CACHE_MPAGE_LEN - 2) { if (len < CACHE_MPAGE_LEN - 2) *fp = len; @@ -3627,8 +3626,6 @@ static int ata_mselect_caching(struct ata_queued_cmd *qc, return -EINVAL; } - wce = buf[0] & (1 << 2); - /* * Check that read-only bits are not modified. */ @@ -3642,6 +3639,8 @@ static int ata_mselect_caching(struct ata_queued_cmd *qc, } } + wce = buf[0] & (1 << 2); + tf->flags |= ATA_TFLAG_DEVICE | ATA_TFLAG_ISADDR; tf->protocol = ATA_PROT_NODATA; tf->nsect = 0; @@ -3674,7 +3673,6 @@ static int ata_mselect_control(struct ata_queued_cmd *qc, * The first two bytes of def_control_mpage are a header, so offsets * in mpage are off by 2 compared to buf. Same for len. */ - if (len != CONTROL_MPAGE_LEN - 2) { if (len < CONTROL_MPAGE_LEN - 2) *fp = len; @@ -3683,8 +3681,6 @@ static int ata_mselect_control(struct ata_queued_cmd *qc, return -EINVAL; } - d_sense = buf[0] & (1 << 2); - /* * Check that read-only bits are not modified. */ @@ -3697,7 +3693,10 @@ static int ata_mselect_control(struct ata_queued_cmd *qc, return -EINVAL; } } - if (d_sense & (1 << 2)) + + d_sense = buf[0] & (1 << 2); + + if (d_sense) dev->flags |= ATA_DFLAG_D_SENSE; else dev->flags &= ~ATA_DFLAG_D_SENSE; -- 2.9.0 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH resend 2/5] libata-scsi: fix read-only bits checking in ata_mselect_*()
From: Tom Yan Commit 7780081c1f04 ("libata-scsi: Set information sense field for invalid parameter") changed how ata_mselect_*() make sure read-only bits are not modified. The new implementation introduced a bug that the read-only bits in the byte that has a changeable bit will not be checked. Added the necessary check, with comments explaining the heuristic. Signed-off-by: Tom Yan diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c index eb5e8ff..ac90676 100644 --- a/drivers/ata/libata-scsi.c +++ b/drivers/ata/libata-scsi.c @@ -3631,8 +3631,18 @@ static int ata_mselect_caching(struct ata_queued_cmd *qc, */ ata_msense_caching(dev->id, mpage, false); for (i = 0; i < CACHE_MPAGE_LEN - 2; i++) { - if (i == 0) - continue; + /* Check the first byte */ + if (i == 0) { + /* except the WCE bit */ + if ((mpage[i + 2] & 0xfb) != (buf[i] & 0xfb)) { + *fp = i; + return -EINVAL; + } else { + continue; + } + } + + /* Check the remaining bytes */ if (mpage[i + 2] != buf[i]) { *fp = i; return -EINVAL; @@ -3686,8 +3696,18 @@ static int ata_mselect_control(struct ata_queued_cmd *qc, */ ata_msense_control(dev, mpage, false); for (i = 0; i < CONTROL_MPAGE_LEN - 2; i++) { - if (i == 0) - continue; + /* Check the first byte */ + if (i == 0) { + /* except the D_SENSE bit */ + if ((mpage[i + 2] & 0xfb) != (buf[i] & 0xfb)) { + *fp = i; + return -EINVAL; + } else { + continue; + } + } + + /* Check the remaining bytes */ if (mpage[2 + i] != buf[i]) { *fp = i; return -EINVAL; -- 2.9.0 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH resend 3/5] libata-scsi: fix overflow in mode page copy
From: Tom Yan ata_mselect_*() would initialize a char array for storing a copy of the current mode page. However, if char was actually signed char, overflow could occur. For example, `0xff` from def_control_mpage[] would be "truncated" to `-1`. This prevented ata_mselect_control() from working at all, since when it did the read-only bits check, there would always be a mismatch. Signed-off-by: Tom Yan diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c index ac90676..3c93341 100644 --- a/drivers/ata/libata-scsi.c +++ b/drivers/ata/libata-scsi.c @@ -3610,7 +3610,7 @@ static int ata_mselect_caching(struct ata_queued_cmd *qc, { struct ata_taskfile *tf = &qc->tf; struct ata_device *dev = qc->dev; - char mpage[CACHE_MPAGE_LEN]; + u8 mpage[CACHE_MPAGE_LEN]; u8 wce; int i; @@ -3675,7 +3675,7 @@ static int ata_mselect_control(struct ata_queued_cmd *qc, const u8 *buf, int len, u16 *fp) { struct ata_device *dev = qc->dev; - char mpage[CONTROL_MPAGE_LEN]; + u8 mpage[CONTROL_MPAGE_LEN]; u8 d_sense; int i; -- 2.9.0 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH resend 4/5] libata-scsi: have all checks done before calling ata_mselect_*()
From: Tom Yan The one-page-at-a-time check in ata_scsi_mode_select_xlat() should be done before either of the ata_mselect_*() is called. Also updated the comment. We have more than one mode page that has changeable bit since commit 06dbde5f3a44 ("libata: Implement control mode page to select sense format"). Signed-off-by: Tom Yan diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c index 3c93341..6c424c5 100644 --- a/drivers/ata/libata-scsi.c +++ b/drivers/ata/libata-scsi.c @@ -3837,6 +3837,12 @@ static unsigned int ata_scsi_mode_select_xlat(struct ata_queued_cmd *qc) if (pg_len > len) goto invalid_param_len; + /* +* Currently we only support setting one page at a time. +*/ + if (len > pg_len) + goto invalid_param; + switch (pg) { case CACHE_MPAGE: if (ata_mselect_caching(qc, p, pg_len, &fp) < 0) { @@ -3855,13 +3861,6 @@ static unsigned int ata_scsi_mode_select_xlat(struct ata_queued_cmd *qc) goto invalid_param; } - /* -* Only one page has changeable data, so we only support setting one -* page at a time. -*/ - if (len > pg_len) - goto invalid_param; - return 0; invalid_fld: -- 2.9.0 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH resend 5/5] libata-scsi: fix MODE SELECT translation for Control mode page
From: Tom Yan scsi_done() was called repeatedly and apparently because of that, the kernel would call trace when we touch the Control mode page: Call Trace: [] dump_stack+0x63/0x81 [] __warn+0xcb/0xf0 [] warn_slowpath_null+0x1d/0x20 [] ata_eh_finish+0xe0/0xf0 [libata] [] sata_pmp_error_handler+0x640/0xa50 [libata] [] ahci_error_handler+0x1d/0x70 [libahci] [] ata_scsi_port_error_handler+0x430/0x770 [libata] [] ? ata_scsi_cmd_error_handler+0xdd/0x160 [libata] [] ata_scsi_error+0xa7/0xf0 [libata] [] scsi_error_handler+0xaa/0x560 [scsi_mod] [] ? scsi_eh_get_sense+0x180/0x180 [scsi_mod] [] kthread+0xd8/0xf0 [] ret_from_fork+0x1f/0x40 [] ? kthread_worker_fn+0x170/0x170 ---[ end trace 8b7501047e928a17 ]--- Removed the unnecessary code and let ata_scsi_translate() do the job. Also, since ata_mselect_control() has no ATA command to send to the device, ata_scsi_mode_select_xlat() should return 1 for it, so that ata_scsi_translate() will finish early to avoid ata_qc_issue(). Signed-off-by: Tom Yan diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c index 6c424c5..f5c4200 100644 --- a/drivers/ata/libata-scsi.c +++ b/drivers/ata/libata-scsi.c @@ -3720,8 +3720,6 @@ static int ata_mselect_control(struct ata_queued_cmd *qc, dev->flags |= ATA_DFLAG_D_SENSE; else dev->flags &= ~ATA_DFLAG_D_SENSE; - qc->scsicmd->result = SAM_STAT_GOOD; - qc->scsicmd->scsi_done(qc->scsicmd); return 0; } @@ -3854,6 +3852,8 @@ static unsigned int ata_scsi_mode_select_xlat(struct ata_queued_cmd *qc) if (ata_mselect_control(qc, p, pg_len, &fp) < 0) { fp += hdr_len + bd_len; goto invalid_param; + } else { + goto skip; /* No ATA command to send */ } break; default:/* invalid page code */ -- 2.9.0 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH resend v2 2/5] libata-scsi: fix read-only bits checking in ata_mselect_*()
From: Tom Yan Commit 7780081c1f04 ("libata-scsi: Set information sense field for invalid parameter") changed how ata_mselect_*() make sure read-only bits are not modified. The new implementation introduced a bug that the read-only bits in the byte that has a changeable bit will not be checked. Made it check every byte but mask the changeable bit. Signed-off-by: Tom Yan diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c index eb5e8ff..4a4e6f1 100644 --- a/drivers/ata/libata-scsi.c +++ b/drivers/ata/libata-scsi.c @@ -3611,7 +3611,7 @@ static int ata_mselect_caching(struct ata_queued_cmd *qc, struct ata_taskfile *tf = &qc->tf; struct ata_device *dev = qc->dev; char mpage[CACHE_MPAGE_LEN]; - u8 wce; + u8 wce, mask; int i; /* @@ -3632,8 +3632,11 @@ static int ata_mselect_caching(struct ata_queued_cmd *qc, ata_msense_caching(dev->id, mpage, false); for (i = 0; i < CACHE_MPAGE_LEN - 2; i++) { if (i == 0) - continue; - if (mpage[i + 2] != buf[i]) { + mask = 0xfb; + else + mask = 0xff; + + if ((mpage[i + 2] & mask) != (buf[i] & mask)) { *fp = i; return -EINVAL; } @@ -3666,7 +3669,7 @@ static int ata_mselect_control(struct ata_queued_cmd *qc, { struct ata_device *dev = qc->dev; char mpage[CONTROL_MPAGE_LEN]; - u8 d_sense; + u8 d_sense, mask; int i; /* @@ -3687,8 +3690,11 @@ static int ata_mselect_control(struct ata_queued_cmd *qc, ata_msense_control(dev, mpage, false); for (i = 0; i < CONTROL_MPAGE_LEN - 2; i++) { if (i == 0) - continue; - if (mpage[2 + i] != buf[i]) { + mask = 0xfb; + else + mask = 0xff; + + if ((mpage[2 + i] & mask) != (buf[i] & mask)) { *fp = i; return -EINVAL; } -- 2.9.0 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH resend v2 3/5] libata-scsi: use u8 array to store mode page copy
From: Tom Yan ata_mselect_*() would initialize a char array for storing a copy of the current mode page. However, char could be signed char. In that case, bytes larger than 127 would be converted to negative number. For example, 0xff from def_control_mpage[] would become -1. This prevented ata_mselect_control() from working at all, since when it did the read-only bits check, there would always be a mismatch. Signed-off-by: Tom Yan diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c index 4a4e6f1..a28e2ea94 100644 --- a/drivers/ata/libata-scsi.c +++ b/drivers/ata/libata-scsi.c @@ -3610,7 +3610,7 @@ static int ata_mselect_caching(struct ata_queued_cmd *qc, { struct ata_taskfile *tf = &qc->tf; struct ata_device *dev = qc->dev; - char mpage[CACHE_MPAGE_LEN]; + u8 mpage[CACHE_MPAGE_LEN]; u8 wce, mask; int i; @@ -3668,7 +3668,7 @@ static int ata_mselect_control(struct ata_queued_cmd *qc, const u8 *buf, int len, u16 *fp) { struct ata_device *dev = qc->dev; - char mpage[CONTROL_MPAGE_LEN]; +u8 mpage[CONTROL_MPAGE_LEN]; u8 d_sense, mask; int i; -- 2.9.0 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH resend v2 2/5] libata-scsi: fix read-only bits checking in ata_mselect_*()
From: Tom Yan Commit 7780081c1f04 ("libata-scsi: Set information sense field for invalid parameter") changed how ata_mselect_*() make sure read-only bits are not modified. The new implementation introduced a bug that the read-only bits in the byte that has a changeable bit will not be checked. Made it check every byte but mask the changeable bit. Signed-off-by: Tom Yan diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c index eb5e8ff..4a4e6f1 100644 --- a/drivers/ata/libata-scsi.c +++ b/drivers/ata/libata-scsi.c @@ -3611,7 +3611,7 @@ static int ata_mselect_caching(struct ata_queued_cmd *qc, struct ata_taskfile *tf = &qc->tf; struct ata_device *dev = qc->dev; char mpage[CACHE_MPAGE_LEN]; - u8 wce; + u8 wce, mask; int i; /* @@ -3632,8 +3632,11 @@ static int ata_mselect_caching(struct ata_queued_cmd *qc, ata_msense_caching(dev->id, mpage, false); for (i = 0; i < CACHE_MPAGE_LEN - 2; i++) { if (i == 0) - continue; - if (mpage[i + 2] != buf[i]) { + mask = 0xfb; + else + mask = 0xff; + + if ((mpage[i + 2] & mask) != (buf[i] & mask)) { *fp = i; return -EINVAL; } @@ -3666,7 +3669,7 @@ static int ata_mselect_control(struct ata_queued_cmd *qc, { struct ata_device *dev = qc->dev; char mpage[CONTROL_MPAGE_LEN]; - u8 d_sense; + u8 d_sense, mask; int i; /* @@ -3687,8 +3690,11 @@ static int ata_mselect_control(struct ata_queued_cmd *qc, ata_msense_control(dev, mpage, false); for (i = 0; i < CONTROL_MPAGE_LEN - 2; i++) { if (i == 0) - continue; - if (mpage[2 + i] != buf[i]) { + mask = 0xfb; + else + mask = 0xff; + + if ((mpage[2 + i] & mask) != (buf[i] & mask)) { *fp = i; return -EINVAL; } -- 2.9.0 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH resend v2 3/5] libata-scsi: use u8 array to store mode page copy
From: Tom Yan ata_mselect_*() would initialize a char array for storing a copy of the current mode page. However, char could be signed char. In that case, bytes larger than 127 would be converted to negative number. For example, 0xff from def_control_mpage[] would become -1. This prevented ata_mselect_control() from working at all, since when it did the read-only bits check, there would always be a mismatch. Signed-off-by: Tom Yan diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c index 4a4e6f1..a28e2ea94 100644 --- a/drivers/ata/libata-scsi.c +++ b/drivers/ata/libata-scsi.c @@ -3610,7 +3610,7 @@ static int ata_mselect_caching(struct ata_queued_cmd *qc, { struct ata_taskfile *tf = &qc->tf; struct ata_device *dev = qc->dev; - char mpage[CACHE_MPAGE_LEN]; + u8 mpage[CACHE_MPAGE_LEN]; u8 wce, mask; int i; @@ -3668,7 +3668,7 @@ static int ata_mselect_control(struct ata_queued_cmd *qc, const u8 *buf, int len, u16 *fp) { struct ata_device *dev = qc->dev; - char mpage[CONTROL_MPAGE_LEN]; +u8 mpage[CONTROL_MPAGE_LEN]; u8 d_sense, mask; int i; -- 2.9.0 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH resend v3 3/5] libata-scsi: use u8 array to store mode page copy
From: Tom Yan ata_mselect_*() would initialize a char array for storing a copy of the current mode page. However, char could be signed char. In that case, bytes larger than 127 would be converted to negative number. For example, 0xff from def_control_mpage[] would become -1. This prevented ata_mselect_control() from working at all, since when it did the read-only bits check, there would always be a mismatch. Signed-off-by: Tom Yan diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c index 4a4e6f1..5cfcb4b 100644 --- a/drivers/ata/libata-scsi.c +++ b/drivers/ata/libata-scsi.c @@ -3610,7 +3610,7 @@ static int ata_mselect_caching(struct ata_queued_cmd *qc, { struct ata_taskfile *tf = &qc->tf; struct ata_device *dev = qc->dev; - char mpage[CACHE_MPAGE_LEN]; + u8 mpage[CACHE_MPAGE_LEN]; u8 wce, mask; int i; @@ -3668,7 +3668,7 @@ static int ata_mselect_control(struct ata_queued_cmd *qc, const u8 *buf, int len, u16 *fp) { struct ata_device *dev = qc->dev; - char mpage[CONTROL_MPAGE_LEN]; + u8 mpage[CONTROL_MPAGE_LEN]; u8 d_sense, mask; int i; -- 2.9.0 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] libata: do not set max_sectors for LBA48 device
From: Tom Yan Along with commit 1dc8fff24187 ("libata-scsi: do not call blk_queue_max_hw_sectors()"), devices with LBA48 support will have max_sectors set to SCSI_DEFAULT_MAX_SECTORS (currently 1024), by the scsi driver. Note that the "max_sectors" here is actually the block layer limit "max_hw_sectors", while the block layer limit "max_sectors" (which is upper-bounded by "max_hw_sectors") will be set to BLK_DEF_MAX_SECTORS (currently 2560) by the scsi disk driver, since our SATL does not report an Optimal Transfer Length. Therefore, it should make more sense to have the "max_sectors" here set to SCSI_DEFAULT_MAX_SECTORS, so that both of the block layer limits will be set to 1024, than to have "max_hw_sectors" set to 65535 and have "max_sectors" set to 2560. Not to mention that neither of them seems to be a "safe" value (see ATA_HORKAGE_MAX_SEC_1024). Besides, it is in doubt that whether having max_sectors (for a single drive, which is our case) set to a value higher than 1024 would actually improve performance anyway. diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c index 0749f71..2a08458 100644 --- a/drivers/ata/libata-core.c +++ b/drivers/ata/libata-core.c @@ -2408,7 +2408,6 @@ int ata_dev_configure(struct ata_device *dev) /* initialize to-be-configured parameters */ dev->flags &= ~ATA_DFLAG_CFG_MASK; - dev->max_sectors = 0; dev->cdb_len = 0; dev->n_sectors = 0; dev->cylinders = 0; @@ -2610,10 +2609,8 @@ int ata_dev_configure(struct ata_device *dev) dma_dir_string); } - /* determine max_sectors */ - dev->max_sectors = ATA_MAX_SECTORS; - if (dev->flags & ATA_DFLAG_LBA48) - dev->max_sectors = ATA_MAX_SECTORS_LBA48; + if (!(dev->flags & ATA_DFLAG_LBA48)) + dev->max_sectors = ATA_MAX_SECTORS; /* Limit PATA drive on SATA cable bridge transfers to udma5, 200 sectors */ -- 2.9.2 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] libata-scsi: do not call blk_queue_max_hw_sectors()
From: Tom Yan We should just let the scsi driver (hosts.c) call the function. It has better heuristic anyway (i.e. use SCSI_DEFAULT_MAX_SECTORS as fallback when max_sectors is not set). diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c index 2bdb5da..495d916 100644 --- a/drivers/ata/libata-scsi.c +++ b/drivers/ata/libata-scsi.c @@ -1204,9 +1204,6 @@ static int ata_scsi_dev_config(struct scsi_device *sdev, if (!ata_id_has_unload(dev->id)) dev->flags |= ATA_DFLAG_NO_UNLOAD; - /* configure max sectors */ - blk_queue_max_hw_sectors(q, dev->max_sectors); - if (dev->class == ATA_DEV_ATAPI) { void *buf; -- 2.9.2 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 2/2] libata-core: do not set dev->max_sectors for LBA48 devices
From: Tom Yan Currently block layer limit max_hw_sectors is set to ATA_MAX_SECTORS_LBA48 (65535), for devices with LBA48 support. However, block layer limit max_sectors (which is the effective one; also adjustable, upper-bounded by max_hw_sectors) is set to BLK_DEF_MAX_SECTORS (currently 2560) by the scsi disk driver, since libata's SATL does not report an Optimal Transfer Length. This does not make much sense, especially when the current BLK_DEF_MAX_SECTORS appears to be unsafe for some ATA devices (see ATA_HORKAGE_MAX_SEC_1024). Truth is, the current value appears to be arbitrary anyway. See commit d2be537c3ba3 ("block: bump BLK_DEF_MAX_SECTORS to 2560"). Therefore, avoid setting dev->max_sectors when it is strictly necessary. Leave it as 0 otherwise, so that both block layer limits will remain as SCSI_DEFAULT_MAX_SECTORS (currently 1024). diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c index 0749f71..07259d8 100644 --- a/drivers/ata/libata-core.c +++ b/drivers/ata/libata-core.c @@ -2610,10 +2610,8 @@ int ata_dev_configure(struct ata_device *dev) dma_dir_string); } - /* determine max_sectors */ - dev->max_sectors = ATA_MAX_SECTORS; - if (dev->flags & ATA_DFLAG_LBA48) - dev->max_sectors = ATA_MAX_SECTORS_LBA48; + if (!(dev->flags & ATA_DFLAG_LBA48)) + dev->max_sectors = ATA_MAX_SECTORS; /* Limit PATA drive on SATA cable bridge transfers to udma5, 200 sectors */ -- 2.9.2 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 1/2] libata-scsi: set max_hw_sectors again only when dev->max_sectors is set
From: Tom Yan When the request queue is initialized (see __scsi_init_queue() in scsi_lib.c), the block layer limit max_hw_sectors is set to shost->max_sectors, which will be set to the "machine infinity" SCSI_DEFAULT_MAX_SECTORS (currently 1024) if sht->max_sectors is not set (see scsi_host_alloc() in hosts.c). Therefore, if dev->max_sectors is not set (or, initialized to 0), we do not call blk_queue_max_hw_sectors() again with that. diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c index 2bdb5da..6fee950 100644 --- a/drivers/ata/libata-scsi.c +++ b/drivers/ata/libata-scsi.c @@ -1205,7 +1205,8 @@ static int ata_scsi_dev_config(struct scsi_device *sdev, dev->flags |= ATA_DFLAG_NO_UNLOAD; /* configure max sectors */ - blk_queue_max_hw_sectors(q, dev->max_sectors); + if (dev->max_sectors) + blk_queue_max_hw_sectors(q, dev->max_sectors); if (dev->class == ATA_DEV_ATAPI) { void *buf; -- 2.9.2 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC] libata-scsi: make sure Maximum Write Same Length is not too large
From: Tom Yan Currently we advertise Maximum Write Same Length based on the maximum number of sectors that one-block TRIM payload can cover. The field are used to derived discard_max_bytes and write_same_max_bytes limits in the block layer, which currently can at max be 0x (32-bit). However, with a AF 4Kn drive, the derived limits would be 65535 * 64 * 4096 = 0x3fffc (34-bit). Therefore, we now devide ATA_MAX_TRIM_RNUM with (logical sector size / 512), so that the derived limits will not overflow. The limits are now also consistent among drives with different logical sector sizes. (Although that may or may not be what we want ultimately when the SCSI / block layer allows larger representation in the future.) Although 4Kn ATA SSDs may not be a thing on the market yet, this patch is necessary for forthcoming SCT Write Same translation support, which could be available on traditional HDDs where 4Kn is already a thing. Also it should not change the current behavior on drives with 512-byte logical sectors. Note: this patch is not about AF 512e drives. Signed-off-by: Tom Yan diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c index be9c76c..dcadcaf 100644 --- a/drivers/ata/libata-scsi.c +++ b/drivers/ata/libata-scsi.c @@ -2295,6 +2295,7 @@ static unsigned int ata_scsiop_inq_89(struct ata_scsi_args *args, u8 *rbuf) static unsigned int ata_scsiop_inq_b0(struct ata_scsi_args *args, u8 *rbuf) { u16 min_io_sectors; + u32 sector_size; rbuf[1] = 0xb0; rbuf[3] = 0x3c; /* required VPD size with unmap support */ @@ -2309,17 +2310,27 @@ static unsigned int ata_scsiop_inq_b0(struct ata_scsi_args *args, u8 *rbuf) min_io_sectors = 1 << ata_id_log2_per_physical_sector(args->id); put_unaligned_be16(min_io_sectors, &rbuf[6]); - /* -* Optimal unmap granularity. -* -* The ATA spec doesn't even know about a granularity or alignment -* for the TRIM command. We can leave away most of the unmap related -* VPD page entries, but we have specifify a granularity to signal -* that we support some form of unmap - in thise case via WRITE SAME -* with the unmap bit set. -*/ + sector_size = ata_id_logical_sector_size(args->id); if (ata_id_has_trim(args->id)) { - put_unaligned_be64(65535 * ATA_MAX_TRIM_RNUM, &rbuf[36]); + /* +* Maximum write same length. +* +* Avoid overflow in discard_max_bytes and write_same_max_bytes +* with AF 4Kn drives. Also make them consistent among drives +* with different logical sector sizes. +*/ + put_unaligned_be64(65535 * ATA_MAX_TRIM_RNUM / + (sector_size / 512), &rbuf[36]); + + /* +* Optimal unmap granularity. +* +* The ATA spec doesn't even know about a granularity or alignment +* for the TRIM command. We can leave away most of the unmap related +* VPD page entries, but we have specifify a granularity to signal +* that we support some form of unmap - in thise case via WRITE SAME +* with the unmap bit set. +*/ put_unaligned_be32(1, &rbuf[28]); } -- 2.9.2 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC] sd: dynamically adjust SD_MAX_WS16_BLOCKS as per the actual logical block size
From: Tom Yan WRITE SAME (16) command can technically handle up to 32-bit number of blocks. However, since 32-bit is also the limitation of the maximum number of bytes that can be represented in the block layer, the current SD_MAX_WS16_BLOCKS was hence derived from the technical limit devided by 512. However, SD_MAX_WS16_BLOCKS is used to check values that are, for example, orignated from Maximum Write Same Length field on the Block Limit VPD. Such field expresses the number of blocks in terms of the actual logical sector size of the specific drive instead of the block size that the block layer is based on (512). Therefore, the original hack would work fine for drives with 512-byte logical sectors. However, for drives with larger logical sector size (e.g. AF 4Kn drives), the hack would be in vain. So let's bump the macro set in sd.h back to the technical limit, and adjust it as per the actual logical block size when it is used. Signed-off-by: Tom Yan diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c index d3e852a..601afd6 100644 --- a/drivers/scsi/sd.c +++ b/drivers/scsi/sd.c @@ -452,6 +452,8 @@ max_write_same_blocks_store(struct device *dev, struct device_attribute *attr, { struct scsi_disk *sdkp = to_scsi_disk(dev); struct scsi_device *sdp = sdkp->device; + unsigned int logical_block_size = sdp->sector_size; + unsigned int max_ws16_blocks = SD_MAX_WS16_BLOCKS / logical_block_size; unsigned long max; int err; @@ -468,7 +470,7 @@ max_write_same_blocks_store(struct device *dev, struct device_attribute *attr, if (max == 0) sdp->no_write_same = 1; - else if (max <= SD_MAX_WS16_BLOCKS) { + else if (max <= max_ws16_blocks) { sdp->no_write_same = 0; sdkp->max_ws_blocks = max; } @@ -635,6 +637,7 @@ static void sd_config_discard(struct scsi_disk *sdkp, unsigned int mode) { struct request_queue *q = sdkp->disk->queue; unsigned int logical_block_size = sdkp->device->sector_size; + unsigned int max_ws16_blocks = SD_MAX_WS16_BLOCKS / logical_block_size; unsigned int max_blocks = 0; q->limits.discard_zeroes_data = 0; @@ -668,12 +671,12 @@ static void sd_config_discard(struct scsi_disk *sdkp, unsigned int mode) case SD_LBP_UNMAP: max_blocks = min_not_zero(sdkp->max_unmap_blocks, - (u32)SD_MAX_WS16_BLOCKS); + (u32)max_ws16_blocks); break; case SD_LBP_WS16: max_blocks = min_not_zero(sdkp->max_ws_blocks, - (u32)SD_MAX_WS16_BLOCKS); + (u32)max_ws16_blocks); q->limits.discard_zeroes_data = sdkp->lbprz; break; @@ -793,6 +796,7 @@ static void sd_config_write_same(struct scsi_disk *sdkp) { struct request_queue *q = sdkp->disk->queue; unsigned int logical_block_size = sdkp->device->sector_size; + unsigned int max_ws16_blocks = SD_MAX_WS16_BLOCKS / logical_block_size; if (sdkp->device->no_write_same) { sdkp->max_ws_blocks = 0; @@ -806,7 +810,7 @@ static void sd_config_write_same(struct scsi_disk *sdkp) */ if (sdkp->max_ws_blocks > SD_MAX_WS10_BLOCKS) sdkp->max_ws_blocks = min_not_zero(sdkp->max_ws_blocks, - (u32)SD_MAX_WS16_BLOCKS); + (u32)max_ws16_blocks); else if (sdkp->ws16 || sdkp->ws10 || sdkp->device->no_report_opcodes) sdkp->max_ws_blocks = min_not_zero(sdkp->max_ws_blocks, (u32)SD_MAX_WS10_BLOCKS); diff --git a/drivers/scsi/sd.h b/drivers/scsi/sd.h index 765a6f1..56ff88c 100644 --- a/drivers/scsi/sd.h +++ b/drivers/scsi/sd.h @@ -47,7 +47,7 @@ enum { SD_DEF_XFER_BLOCKS = 0x, SD_MAX_XFER_BLOCKS = 0x, SD_MAX_WS10_BLOCKS = 0x, - SD_MAX_WS16_BLOCKS = 0x7f, + SD_MAX_WS16_BLOCKS = 0x, }; enum { -- 2.9.2 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] libata-scsi: use dev->max_sectors from libata-core appropriately
From: Tom Yan Currently we use dev->max_sectors to set max_hw_sectors, which is actually supposed to be a host controller limit (that get set by the host controller driver like ahci, and if not it would be the fallback SCSI_DEFAULT_MAX_SECTORS). That means we have been doing the wrong thing. Therefore, use it to set the correct request queue limit that is used to report device limit: max_dev_sectors. For disk devices, it is reported through the Maximum Transfer Length field on the Block Limits VPD to the SCSI disk driver, which will then set and make use of max_dev_sectors. For cdrom devices, since they are not supposed to have any VPD, and neither will the SCSI cdrom (sr) driver touch any of the max sectors limits, we are hence setting the limit directly in libata-scsi. Note that when max_dev_sectors is larger than max_hw_sectors, it does not have any effect. Therefore, setting dev->max_sectors to ATA_MAX_SECTORS_LBA48 will only have some effect when the host driver has set max_hw_sectors to a value that is as large as that. This should not matter for typical devices, since according to our past experience, 1024 (the current SCSI_DEFAULT_MAX_SECTORS) is a decent and safe value for max_sectors. ATA_MAX_SECTORS_LBA48 has actually appeared to be too large for some devices. Also note that ATA_HORKAGE_MAX_SEC_LBA48 is not supposed to work automatically anyway, even when max_hw_sectors is as high as 65535, since the effective max_sectors will be set by the SCSI disk driver. Signed-off-by: Tom Yan diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c index be9c76c..4e2d8e7 100644 --- a/drivers/ata/libata-scsi.c +++ b/drivers/ata/libata-scsi.c @@ -1204,14 +1204,26 @@ static int ata_scsi_dev_config(struct scsi_device *sdev, if (!ata_id_has_unload(dev->id)) dev->flags |= ATA_DFLAG_NO_UNLOAD; - /* configure max sectors */ - blk_queue_max_hw_sectors(q, dev->max_sectors); - if (dev->class == ATA_DEV_ATAPI) { void *buf; sdev->sector_size = ATA_SECT_SIZE; + /* +* We are setting the limit here merely because CD/DVD device does not +* have Block Limits VPD. +* +* Supposedly dev->max_sectors should be left shifted by +* (ilog2(sdev->sector_size) - 9). But since ATAPI class device has a +* static logical sector size of 512 (ATA_SECT_SIZE), the shift became +* unnecessary. +*/ + q->limits.max_dev_sectors = dev->max_sectors; + /* Make max_dev_sectors effective by adjusting max_sectors accordingly, + while leave max_hw_sectors, which is supposed to be host controller + limit, untouched. */ + blk_queue_max_hw_sectors(q, queue_max_hw_sectors(q)); + /* set DMA padding */ blk_queue_update_dma_pad(q, ATA_DMA_PAD_SZ - 1); @@ -2310,6 +2322,13 @@ static unsigned int ata_scsiop_inq_b0(struct ata_scsi_args *args, u8 *rbuf) put_unaligned_be16(min_io_sectors, &rbuf[6]); /* +* Maximum transfer length. +* +* This will be used by the SCSI disk driver to set max_dev_sectors. +*/ + put_unaligned_be32(args->dev->max_sectors, &rbuf[8]); + + /* * Optimal unmap granularity. * * The ATA spec doesn't even know about a granularity or alignment -- 2.9.2 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] sd: check BLK_DEF_MAX_SECTORS against max_dev_sectors
From: Tom Yan The SCSI disk driver sets max_sector to BLK_DEF_MAX_SECTORS when the device does not report Optimal Transfer Length. However, it checks only whether it is smaller than max_hw_sectors, but not max_dev_sectors. Signed-off-by: Tom Yan diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c index d3e852a..2b6da13 100644 --- a/drivers/scsi/sd.c +++ b/drivers/scsi/sd.c @@ -2875,8 +2875,11 @@ static int sd_revalidate_disk(struct gendisk *disk) logical_to_bytes(sdp, sdkp->opt_xfer_blocks) >= PAGE_SIZE) { q->limits.io_opt = logical_to_bytes(sdp, sdkp->opt_xfer_blocks); rw_max = logical_to_sectors(sdp, sdkp->opt_xfer_blocks); - } else + } else { rw_max = BLK_DEF_MAX_SECTORS; + /* Combine with device limits */ + rw_max = min(rw_max, q->limits.max_dev_sectors); + } /* Combine with controller limits */ q->limits.max_sectors = min(rw_max, queue_max_hw_sectors(q)); -- 2.9.2 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 1/2] libata-scsi: use dev->max_sectors from libata-core appropriately
From: Tom Yan Currently we use dev->max_sectors to set max_hw_sectors, which is actually supposed to be a host controller limit (that get set by the host controller driver like ahci, and if not it would be the fallback SCSI_DEFAULT_MAX_SECTORS). That means we have been doing the wrong thing. Therefore, use it to set the correct request queue limit that is used to report device limit: max_dev_sectors. For disk devices, it is reported through the Maximum Transfer Length field on the Block Limits VPD to the SCSI disk driver, which will then set and make use of max_dev_sectors. Note that when max_dev_sectors is larger than max_hw_sectors, it does not have any effect. Therefore, setting dev->max_sectors to ATA_MAX_SECTORS_LBA48 will only have some effect when the host driver has set max_hw_sectors to a value that is as large as that. This should not matter for typical devices, since according to our past experience, 1024 (the current SCSI_DEFAULT_MAX_SECTORS) is a decent and safe value for max_sectors. ATA_MAX_SECTORS_LBA48 has actually appeared to be too large for some devices. For cdrom devices, there is a different story. Since a few ATAPI devices needed ATA_HORKAGE_MAX_SEC_LBA48 to work. Therefore, max_hw_sectors must be set to match with that, so that the effective max_sectors can be set to ATA_MAX_SECTORS_LBA48. Signed-off-by: Tom Yan diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c index 223a770..39374236 100644 --- a/drivers/ata/libata-core.c +++ b/drivers/ata/libata-core.c @@ -2639,6 +2639,11 @@ int ata_dev_configure(struct ata_device *dev) dev->max_sectors = min_t(unsigned int, ATA_MAX_SECTORS_1024, dev->max_sectors); + /* For disk devices, max sectors set here will only be used as +* max_dev_sectors. We should never expect max sectors horkage +* that sets the value larger than BLK_DEF_MAX_SECTORS to work +* for non-ATAPI devices. */ + if (dev->horkage & ATA_HORKAGE_MAX_SEC_LBA48) dev->max_sectors = ATA_MAX_SECTORS_LBA48; diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c index be9c76c..13f518b 100644 --- a/drivers/ata/libata-scsi.c +++ b/drivers/ata/libata-scsi.c @@ -1204,14 +1204,26 @@ static int ata_scsi_dev_config(struct scsi_device *sdev, if (!ata_id_has_unload(dev->id)) dev->flags |= ATA_DFLAG_NO_UNLOAD; - /* configure max sectors */ - blk_queue_max_hw_sectors(q, dev->max_sectors); - if (dev->class == ATA_DEV_ATAPI) { void *buf; sdev->sector_size = ATA_SECT_SIZE; + /* +* max_hw_sectors is supposed to be host controller limit, it is +* changed here only to make sure ATA_HORKAGE_MAX_SEC_LBA48 that +* is needed by a few ATAPI devices works. +* +* Unlike the SCSI disk driver, the SCSI cdrom (sr) driver will +* not further change max_sectors. Therefore, the value that is +* also set here is guaranteed to be the effective one. +* +* For disk devices, we should only report dev->max_sectors in +* the Maximum Transfer Length field on the Block Limits VPD +* (which CD/DVD devices are not supposed to have). +*/ + blk_queue_max_hw_sectors(q, dev->max_sectors); + /* set DMA padding */ blk_queue_update_dma_pad(q, ATA_DMA_PAD_SZ - 1); @@ -2310,6 +2322,13 @@ static unsigned int ata_scsiop_inq_b0(struct ata_scsi_args *args, u8 *rbuf) put_unaligned_be16(min_io_sectors, &rbuf[6]); /* +* Maximum transfer length. +* +* This will be used by the SCSI disk driver to set max_dev_sectors. +*/ + put_unaligned_be32(args->dev->max_sectors, &rbuf[8]); + + /* * Optimal unmap granularity. * * The ATA spec doesn't even know about a granularity or alignment -- 2.9.2 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 2/2] sd: check BLK_DEF_MAX_SECTORS against max_dev_sectors
From: Tom Yan The SCSI disk driver sets max_sectors to BLK_DEF_MAX_SECTORS when the device does not report Optimal Transfer Length. However, it checks only whether it is smaller than max_hw_sectors, but not max_dev_sectors. Signed-off-by: Tom Yan diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c index d3e852a..2b6da13 100644 --- a/drivers/scsi/sd.c +++ b/drivers/scsi/sd.c @@ -2875,8 +2875,11 @@ static int sd_revalidate_disk(struct gendisk *disk) logical_to_bytes(sdp, sdkp->opt_xfer_blocks) >= PAGE_SIZE) { q->limits.io_opt = logical_to_bytes(sdp, sdkp->opt_xfer_blocks); rw_max = logical_to_sectors(sdp, sdkp->opt_xfer_blocks); - } else + } else { rw_max = BLK_DEF_MAX_SECTORS; + /* Combine with device limits */ + rw_max = min(rw_max, q->limits.max_dev_sectors); + } /* Combine with controller limits */ q->limits.max_sectors = min(rw_max, queue_max_hw_sectors(q)); -- 2.9.2 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] ata: do not hard code limit in ata_set_lba_range_entries()
From: Tom Yan In commit 5c79097a28c2 ("libata-scsi: reject WRITE SAME (16) with n_block that exceeds limit"), it is made sure that ata_set_lba_range_entries() will never be called with a request size (n_block) that is larger than the number of blocks that a 512-byte block TRIM payload can describe (65535 * 64 = 4194240), in addition to acknowlegding the SCSI/block layer with the same limit by advertising it as the Maximum Write Same Length. Therefore, it is unnecessary to hard code the same limit in ata_set_lba_range_entries() itself, which would only cost extra maintenance effort. Such effort can be noticed in, for example, commit 2983860c7668 ("libata-scsi: avoid repeated calculation of number of TRIM ranges"). Signed-off-by: Tom Yan diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c index be9c76c..9b74ecb 100644 --- a/drivers/ata/libata-scsi.c +++ b/drivers/ata/libata-scsi.c @@ -3322,7 +3322,7 @@ static unsigned int ata_scsi_write_same_xlat(struct ata_queued_cmd *qc) buf = page_address(sg_page(scsi_sglist(scmd))); if (n_block <= 65535 * ATA_MAX_TRIM_RNUM) { - size = ata_set_lba_range_entries(buf, ATA_MAX_TRIM_RNUM, block, n_block); + size = ata_set_lba_range_entries(buf, block, n_block); } else { fp = 2; goto invalid_fld; diff --git a/include/linux/ata.h b/include/linux/ata.h index adbc812..5e2e9ad 100644 --- a/include/linux/ata.h +++ b/include/linux/ata.h @@ -1077,19 +1077,19 @@ static inline void ata_id_to_hd_driveid(u16 *id) * TO NV CACHE PINNED SET. */ static inline unsigned ata_set_lba_range_entries(void *_buffer, - unsigned num, u64 sector, unsigned long count) + u64 sector, unsigned long count) { __le64 *buffer = _buffer; unsigned i = 0, used_bytes; - while (i < num) { - u64 entry = sector | - ((u64)(count > 0x ? 0x : count) << 48); + while (count > 0) { + u64 range, entry; + + range = count > 0x ? 0x : count; + entry = sector | (range << 48); buffer[i++] = __cpu_to_le64(entry); - if (count <= 0x) - break; - count -= 0x; - sector += 0x; + count -= range; + sector += range; } used_bytes = ALIGN(i * 8, 512); -- 2.9.3 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html