[PATCH 1/2] sd: add missing scenario for sd_config_write_same

2016-02-26 Thread tom . ty89
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

2016-02-26 Thread tom . ty89
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

2016-02-26 Thread tom . ty89
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

2016-03-08 Thread tom . ty89
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

2016-03-10 Thread tom . ty89
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

2016-05-02 Thread tom . ty89
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

2016-05-02 Thread tom . ty89
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

2016-05-02 Thread tom . ty89
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

2016-05-17 Thread tom . ty89
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

2016-05-23 Thread tom . ty89
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

2016-05-23 Thread tom . ty89
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

2017-08-14 Thread tom . ty89
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

2016-07-04 Thread tom . ty89
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

2016-07-04 Thread tom . ty89
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

2016-07-04 Thread tom . ty89
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

2016-07-04 Thread tom . ty89
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

2016-07-06 Thread tom . ty89
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

2016-07-06 Thread tom . ty89
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

2016-07-06 Thread tom . ty89
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

2016-07-06 Thread tom . ty89
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

2016-07-06 Thread tom . ty89
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

2016-07-06 Thread tom . ty89
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

2016-07-06 Thread tom . ty89
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

2016-07-06 Thread tom . ty89
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

2016-07-06 Thread tom . ty89
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

2016-07-06 Thread tom . ty89
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()

2016-07-06 Thread tom . ty89
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()

2016-07-06 Thread tom . ty89
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

2016-07-07 Thread tom . ty89
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()

2016-07-12 Thread tom . ty89
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()

2016-07-12 Thread tom . ty89
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

2016-07-12 Thread tom . ty89
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

2016-07-12 Thread tom . ty89
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()

2016-07-12 Thread tom . ty89
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()

2016-07-12 Thread tom . ty89
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

2016-07-12 Thread tom . ty89
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

2016-07-12 Thread tom . ty89
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()

2016-07-12 Thread tom . ty89
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

2016-07-12 Thread tom . ty89
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

2016-07-12 Thread tom . ty89
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

2016-07-12 Thread tom . ty89
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

2016-07-12 Thread tom . ty89
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

2016-07-12 Thread tom . ty89
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}

2016-07-12 Thread tom . ty89
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_*()

2016-07-19 Thread tom . ty89
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_*()

2016-07-19 Thread tom . ty89
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_*()

2016-07-19 Thread tom . ty89
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_*()

2016-07-19 Thread tom . ty89
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_*()

2016-07-21 Thread tom . ty89
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_*()

2016-07-21 Thread tom . ty89
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

2016-07-21 Thread tom . ty89
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_*()

2016-07-21 Thread tom . ty89
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

2016-07-21 Thread tom . ty89
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_*()

2016-07-21 Thread tom . ty89
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

2016-07-21 Thread tom . ty89
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_*()

2016-07-21 Thread tom . ty89
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

2016-07-21 Thread tom . ty89
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

2016-07-22 Thread tom . ty89
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

2016-08-09 Thread tom . ty89
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()

2016-08-09 Thread tom . ty89
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

2016-08-09 Thread tom . ty89
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

2016-08-09 Thread tom . ty89
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

2016-08-11 Thread tom . ty89
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

2016-08-11 Thread tom . ty89
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

2016-08-12 Thread tom . ty89
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

2016-08-12 Thread tom . ty89
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

2016-08-12 Thread tom . ty89
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

2016-08-12 Thread tom . ty89
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()

2016-08-22 Thread tom . ty89
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