Re: [PATCH] scsi/libata: Support variable-length cdb of ata pass-thru(32).

2017-06-23 Thread Bart Van Assche 
On Sat, 2017-06-24 at 02:59 +0900, Minwoo Im wrote:
> I completely agree with you. ATA_32 was moved to variable length command list.

Thanks for having addressed all my comments. Since as far as I'm familiar
with SCSI / ATA translation this patch looks fine to me, feel free to add

Reviewed-by: Bart Van Assche 

Please note the following:
- You will have to repost this patch with a proper description and as a
  separate e-mail. See also
  https://www.kernel.org/doc/Documentation/process/5.Posting.rst
- The maintainer for the libata subsystem is Tejun Heo so you will need his
  approval before this patch can be merged.

Bart.

Re: [PATCH] scsi/libata: Support variable-length cdb of ata pass-thru(32).

2017-06-23 Thread Minwoo Im 
On Sat, Jun 24, 2017 at 2:15 AM, Bart Van Assche  wrote:
> On Sat, 2017-06-24 at 01:50 +0900, Minwoo Im wrote:
>> - * Handles either 12 or 16-byte versions of the CDB.
>> + * Handles either 12 16, or 32-byte versions of the CDB.
>
> Please insert a comma between "12" and "16" to avoid that this comment
> gets confusing.

I'm sorry for making a confusion for a mistyping. It's applied to patch.

>
>> +   if ((tf->protocol = ata_scsi_map_proto(cdb[1 + cdb_offset]))
>> +   == ATA_PROT_UNKNOWN) {
>
> Have you verified this patch with checkpatch? The recommended style is
> *not* to use assignments in the expression that controls an if-statement
> and also if a comparison expression has to be split to keep the comparison
> operator at the end of the first line.

I was trying to keep it in a way it used to be.
"Assignment expression separated and comparison expression in a line" applied.

>
>> -   shost->max_cmd_len = 16;
>> +   /*
>> +* SPC says that CDB may have a fixed length of up to 16 
>> bytes
>> +* or variable length of between 12 and 260 bytes.
>> +*/
>> +   shost->max_cmd_len = 260;
>
> As mentioned before, I think a maximum CDB length of 260 is overkill.

I used to think of it only in perspective of SPC specification.
As you said, 32 would be enough for this patch which is only for ata
pass-thru(32).

>
>>  /* Values for T10/04-262r7 */
>> +#defineATA_320x1ff0/* 32-byte pass-thru,
>> service action */
>>  #defineATA_160x85  /* 16-byte pass-thru */
>>  #defineATA_120xa1  /* 12-byte pass-thru */
>
> Defining ATA_32 just above ATA_12 and ATA_16 is misleading because
> the latter two are CDB opcodes while the former is a service action
> code. Please move the definition of the ATA_32 service action code
> one line up such that it appears at the end of the list of already
> defined service codes, namely this list:

I completely agree with you. ATA_32 was moved to variable length command list.

I really appreciate those reviews for this patch.

Thanks,

Minwoo.


Signed-off-by: Minwoo Im 
---
 drivers/ata/libata-core.c |2 +-
 drivers/ata/libata-scsi.c |   72 ++
+++
 include/scsi/scsi_proto.h |1 +
 3 files changed, 69 insertions(+), 6 deletions(-)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 2d83b8c..4777e76 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -2587,7 +2587,7 @@ int ata_dev_configure(struct ata_device *dev)
}
ata_dev_config_sense_reporting(dev);
ata_dev_config_zac(dev);
-   dev->cdb_len = 16;
+   dev->cdb_len = 32;
}

/* ATAPI-specific feature tests */
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 49ba983..dc59860 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -3127,7 +3127,7 @@ static struct ata_device
*__ata_scsi_find_dev(struct ata_port *ap,
  * ata_scsi_pass_thru - convert ATA pass-thru CDB to taskfile
  * @qc: command structure to be initialized
  *
- * Handles either 12 or 16-byte versions of the CDB.
+ * Handles either 12, 16, or 32-byte versions of the CDB.
  *
  * RETURNS:
  * Zero on success, non-zero on failure.
@@ -3139,13 +3139,19 @@ static unsigned int ata_scsi_pass_thru(struct
ata_queued_cmd *qc)
struct ata_device *dev = qc->dev;
const u8 *cdb = scmd->cmnd;
u16 fp;
+   u16 cdb_offset = 0;

-   if ((tf->protocol = ata_scsi_map_proto(cdb[1])) == ATA_PROT_UNKNOWN) {
+   /* 7Fh variable length cmd means a ata pass-thru(32) */
+   if (cdb[0] == VARIABLE_LENGTH_CMD)
+   cdb_offset = 9;
+
+   tf->protocol = ata_scsi_map_proto(cdb[1 + cdb_offset]);
+   if (tf->protocol == ATA_PROT_UNKNOWN) {
fp = 1;
goto invalid_fld;
}

-   if (ata_is_ncq(tf->protocol) && (cdb[2] & 0x3) == 0)
+   if (ata_is_ncq(tf->protocol) && (cdb[2 + cdb_offset] & 0x3) == 0)
tf->protocol = ATA_PROT_NCQ_NODATA;

/* enable LBA */
@@ -3181,7 +3187,7 @@ static unsigned int ata_scsi_pass_thru(struct
ata_queued_cmd *qc)
tf->lbah = cdb[12];
tf->device = cdb[13];
tf->command = cdb[14];
-   } else {
+   } else if (cdb[0] == ATA_12) {
/*
 * 12-byte CDB - incapable of extended commands.
 */
@@ -3194,6 +3200,30 @@ static unsigned int ata_scsi_pass_thru(struct
ata_queued_cmd *qc)
tf->lbah = cdb[7];
tf->device = cdb[8];
tf->command = cdb[9];
+   } else {
+   /*
+* 32-byte CDB - may contain extended command fields.
+

Re: [PATCH] scsi/libata: Support variable-length cdb of ata pass-thru(32).

2017-06-23 Thread Bart Van Assche 
On Sat, 2017-06-24 at 01:50 +0900, Minwoo Im wrote:
> - * Handles either 12 or 16-byte versions of the CDB.
> + * Handles either 12 16, or 32-byte versions of the CDB.

Please insert a comma between "12" and "16" to avoid that this comment
gets confusing.

> +   if ((tf->protocol = ata_scsi_map_proto(cdb[1 + cdb_offset]))
> +   == ATA_PROT_UNKNOWN) {

Have you verified this patch with checkpatch? The recommended style is
*not* to use assignments in the expression that controls an if-statement
and also if a comparison expression has to be split to keep the comparison
operator at the end of the first line.

> -   shost->max_cmd_len = 16;
> +   /*
> +* SPC says that CDB may have a fixed length of up to 16 bytes
> +* or variable length of between 12 and 260 bytes.
> +*/
> +   shost->max_cmd_len = 260;

As mentioned before, I think a maximum CDB length of 260 is overkill.

>  /* Values for T10/04-262r7 */
> +#defineATA_320x1ff0/* 32-byte pass-thru,
> service action */
>  #defineATA_160x85  /* 16-byte pass-thru */
>  #defineATA_120xa1  /* 12-byte pass-thru */

Defining ATA_32 just above ATA_12 and ATA_16 is misleading because
the latter two are CDB opcodes while the former is a service action
code. Please move the definition of the ATA_32 service action code
one line up such that it appears at the end of the list of already
defined service codes, namely this list:

/* values for variable length command */
#define XDREAD_32 0x03
#define XDWRITE_320x04
#define XPWRITE_320x06
#define XDWRITEREAD_320x07
#define READ_32   0x09
#define VERIFY_32 0x0a
#define WRITE_32  0x0b
#define WRITE_SAME_32 0x0d

Thanks,

Bart.

Re: [PATCH] scsi/libata: Support variable-length cdb of ata pass-thru(32).

2017-06-23 Thread Bart Van Assche 
On Sat, 2017-06-24 at 01:50 +0900, Minwoo Im wrote:
> On Thu, Jun 22, 2017 at 3:52 AM, Bart Van Assche  
> wrote:
> > > @@ -4385,7 +4463,12 @@ int ata_scsi_add_hosts(struct ata_host *host, 
> > > struct scsi_host_template *sht)
> > >   shost->max_id = 16;
> > >   shost->max_lun = 1;
> > >   shost->max_channel = 1;
> > > - shost->max_cmd_len = 16;
> > > + /*
> > > +  * SPC-3, SPC-4: Definition of CDB
> > > +  * A CDB may have a fixed length of up to 16 bytes or
> > > +  * variable length of between 12 and 260 bytes.
> > > +  */
> > > + shost->max_cmd_len = 260;
> > 
> > Does ATA pass-through really support 260-byte CDBs or is the maximum CDB 
> > length
> > that is supported by ATA_32 perhaps 32 bytes?
> 
> Here's my opinion about your question.
> In perspective of SCSI host, I guess the max cmd len should be 260 bytes.
> Because SPC says that, in case of variable-length command, it could be
> from 12 to 260 bytes.
> That's why I have applied 260 value to max_cmd_len of scsi host.
> Please feel free to give any opinions about it.

Hello Minwoo,

Table 172 in document sat4r06.pdf shows that the CDB of the ATA PASS-THROUGH 
(32)
command is exactly 32 bytes long. My conclusion from analyzing 
__ata_scsi_queuecmd()
is that the current version of that function does not support CDBs of more than
16 bytes. Since your patch adds support for a single command with 32-byte CDB I
am convinced that max_cmd_len should be set to 32 in ata_scsi_add_hosts().

Bart.

Re: [PATCH] scsi/libata: Support variable-length cdb of ata pass-thru(32).

2017-06-23 Thread Minwoo Im 
On Thu, Jun 22, 2017 at 3:52 AM, Bart Van Assche  wrote:
> On Sat, 2017-06-17 at 20:00 +0900, Minwoo Im wrote:
>> - if ((tf->protocol = ata_scsi_map_proto(cdb[1])) == ATA_PROT_UNKNOWN) {
>> + /*
>> +  * if SCSI operation code in cdb[0] is ATA_12 or ATA_16,
>> +  * then cdb[1] will contain protocol of ATA PASS-THROUGH.
>> +  * otherwise, Its operation code shall be ATA_32(7Fh).
>> +  * in this case, cdb[10] will contain protocol of it.
>> +  * we call this command as a variable-length cdb.
>> +  */
>> + if (cdb[0] == ATA_12 || cdb[0] == ATA_16)
>> + tf->protocol = ata_scsi_map_proto(cdb[1]);
>> + else
>> + tf->protocol = ata_scsi_map_proto(cdb[10]);
>> +
>> + if (tf->protocol == ATA_PROT_UNKNOWN) {
>>   fp = 1;
>>   goto invalid_fld;
>>   }
>
> Hello Minwoo,
>
> Please consider introducing a variable (e.g. called "cdb_offset") in which the
> value 9 is stored for 32-byte CDBs and 0 for 12-byte and 16-byte CDBs. That
> will allow to rewrite the above code as follows:
>
> tf->protocol = ata_scsi_map_proto(cdb[1 + cdb_offset])
>
> I think that will allow to remove most "if (cdb[0] == ATA_12 || cdb[0] == 
> ATA_16)"
> statements introduced by this patch.

Hello Bart,

I really appreciate that you gave me a simple way to handle 12, 16
and 32 bytes commands in a function above.
I have applied those things to my patch.

>
>> + tf->auxiliary = (cdb[28] << 24) | (cdb[29] << 16)
>> + | (cdb[30] << 8) | cdb[31];
>
> Please use get_unaligned_be32() instead of open-coding it.
>
>> + const u16 sa = (cdb[8] >> 8) | cdb[9];  /* service action */
>
> Please use get_unaligned_be16() instead of open-coding it.

I have applied "get_unaligned_be16 and 32" function to those parts.
Please refer a new patch below.
Thanks, again.

>
>> @@ -4385,7 +4463,12 @@ int ata_scsi_add_hosts(struct ata_host *host, struct 
>> scsi_host_template *sht)
>>   shost->max_id = 16;
>>   shost->max_lun = 1;
>>   shost->max_channel = 1;
>> - shost->max_cmd_len = 16;
>> + /*
>> +  * SPC-3, SPC-4: Definition of CDB
>> +  * A CDB may have a fixed length of up to 16 bytes or
>> +  * variable length of between 12 and 260 bytes.
>> +  */
>> + shost->max_cmd_len = 260;
>
> Does ATA pass-through really support 260-byte CDBs or is the maximum CDB 
> length
> that is supported by ATA_32 perhaps 32 bytes?

Here's my opinion about your question.
In perspective of SCSI host, I guess the max cmd len should be 260 bytes.
Because SPC says that, in case of variable-length command, it could be
from 12 to 260 bytes.
That's why I have applied 260 value to max_cmd_len of scsi host.
Please feel free to give any opinions about it.

>
> Thanks,
>
> Bart.

Thanks,
Minwoo.

Signed-off-by: Minwoo Im 
---
 drivers/ata/libata-core.c |2 +-
 drivers/ata/libata-scsi.c |   76 ++
---
 include/scsi/scsi_proto.h |1 +
 3 files changed, 73 insertions(+), 6 deletions(-)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 2d83b8c..4777e76 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -2587,7 +2587,7 @@ int ata_dev_configure(struct ata_device *dev)
}
ata_dev_config_sense_reporting(dev);
ata_dev_config_zac(dev);
-   dev->cdb_len = 16;
+   dev->cdb_len = 32;
}

/* ATAPI-specific feature tests */
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 49ba983..d99c4e8 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -3127,7 +3127,7 @@ static struct ata_device
*__ata_scsi_find_dev(struct ata_port *ap,
  * ata_scsi_pass_thru - convert ATA pass-thru CDB to taskfile
  * @qc: command structure to be initialized
  *
- * Handles either 12 or 16-byte versions of the CDB.
+ * Handles either 12 16, or 32-byte versions of the CDB.
  *
  * RETURNS:
  * Zero on success, non-zero on failure.
@@ -3139,13 +3139,19 @@ static unsigned int ata_scsi_pass_thru(struct
ata_queued_cmd *qc)
struct ata_device *dev = qc->dev;
const u8 *cdb = scmd->cmnd;
u16 fp;
+   u16 cdb_offset = 0;

-   if ((tf->protocol = ata_scsi_map_proto(cdb[1])) == ATA_PROT_UNKNOWN) {
+   /* 7Fh variable length cmd means a ata pass-thru(32) */
+   if (cdb[0] == VARIABLE_LENGTH_CMD)
+   cdb_offset = 9;
+
+   if ((tf->protocol = ata_scsi_map_proto(cdb[1 + cdb_offset]))
+   == ATA_PROT_UNKNOWN) {
fp = 1;
goto invalid_fld;
}

-   if (ata_is_ncq(tf->protocol) && (cdb[2] & 0x3) == 0)
+   if (ata_is_ncq(tf->protocol) && (cdb[2 + cdb_offset] & 0x3) == 0)
tf->protocol =

Re: [PATCH] scsi/libata: Support variable-length cdb of ata pass-thru(32).

2017-06-21 Thread Bart Van Assche 
On Sat, 2017-06-17 at 20:00 +0900, Minwoo Im wrote: 
> - if ((tf->protocol = ata_scsi_map_proto(cdb[1])) == ATA_PROT_UNKNOWN) {
> + /*
> +  * if SCSI operation code in cdb[0] is ATA_12 or ATA_16,
> +  * then cdb[1] will contain protocol of ATA PASS-THROUGH.
> +  * otherwise, Its operation code shall be ATA_32(7Fh).
> +  * in this case, cdb[10] will contain protocol of it.
> +  * we call this command as a variable-length cdb.
> +  */
> + if (cdb[0] == ATA_12 || cdb[0] == ATA_16)
> + tf->protocol = ata_scsi_map_proto(cdb[1]);
> + else
> + tf->protocol = ata_scsi_map_proto(cdb[10]);
> +
> + if (tf->protocol == ATA_PROT_UNKNOWN) {
>   fp = 1;
>   goto invalid_fld;
>   }

Hello Minwoo,

Please consider introducing a variable (e.g. called "cdb_offset") in which the
value 9 is stored for 32-byte CDBs and 0 for 12-byte and 16-byte CDBs. That
will allow to rewrite the above code as follows:

tf->protocol = ata_scsi_map_proto(cdb[1 + cdb_offset])

I think that will allow to remove most "if (cdb[0] == ATA_12 || cdb[0] == 
ATA_16)"
statements introduced by this patch.

> + tf->auxiliary = (cdb[28] << 24) | (cdb[29] << 16)
> + | (cdb[30] << 8) | cdb[31];

Please use get_unaligned_be32() instead of open-coding it.

> + const u16 sa = (cdb[8] >> 8) | cdb[9];  /* service action */

Please use get_unaligned_be16() instead of open-coding it.

> @@ -4385,7 +4463,12 @@ int ata_scsi_add_hosts(struct ata_host *host, struct 
> scsi_host_template *sht)
>   shost->max_id = 16;
>   shost->max_lun = 1;
>   shost->max_channel = 1;
> - shost->max_cmd_len = 16;
> + /*
> +  * SPC-3, SPC-4: Definition of CDB
> +  * A CDB may have a fixed length of up to 16 bytes or
> +  * variable length of between 12 and 260 bytes.
> +  */
> + shost->max_cmd_len = 260;

Does ATA pass-through really support 260-byte CDBs or is the maximum CDB length
that is supported by ATA_32 perhaps 32 bytes?

Thanks,

Bart.

[PATCH] scsi/libata: Support variable-length cdb of ata pass-thru(32)-bug fixed.

2017-06-19 Thread Minwoo Im 
Bug fixed in libata-scsi.c from previous patch.
Bit shifting of service action value in ata_scsi_var_len_cdb_xlat() was 
reversed.
I have tested a ata pass-thru(32) command with sg_io and it worked well.
I'm sorry for making a confusion.

Signed-off-by: Minwoo Im 
---
 drivers/ata/libata-core.c |2 +-
 drivers/ata/libata-scsi.c |   95 ++---
 include/scsi/scsi_proto.h |1 +
 3 files changed, 91 insertions(+), 7 deletions(-)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index e157a0e..f1d3ba4 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -2587,7 +2587,7 @@ int ata_dev_configure(struct ata_device *dev)
}
ata_dev_config_sense_reporting(dev);
ata_dev_config_zac(dev);
-   dev->cdb_len = 16;
+   dev->cdb_len = 32;
}
 
/* ATAPI-specific feature tests */
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 49ba983..39f23e0 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -3127,7 +3127,7 @@ static struct ata_device *__ata_scsi_find_dev(struct 
ata_port *ap,
  * ata_scsi_pass_thru - convert ATA pass-thru CDB to taskfile
  * @qc: command structure to be initialized
  *
- * Handles either 12 or 16-byte versions of the CDB.
+ * Handles either 12, 16, or 32-byte versions of the CDB.
  *
  * RETURNS:
  * Zero on success, non-zero on failure.
@@ -3140,13 +3140,36 @@ static unsigned int ata_scsi_pass_thru(struct 
ata_queued_cmd *qc)
const u8 *cdb = scmd->cmnd;
u16 fp;
 
-   if ((tf->protocol = ata_scsi_map_proto(cdb[1])) == ATA_PROT_UNKNOWN) {
+   /*
+* if SCSI operation code in cdb[0] is ATA_12 or ATA_16,
+* then cdb[1] will contain protocol of ATA PASS-THROUGH.
+* otherwise, Its operation code shall be ATA_32(7Fh).
+* in this case, cdb[10] will contain protocol of it.
+* we call this command as a variable-length cdb.
+*/
+   if (cdb[0] == ATA_12 || cdb[0] == ATA_16)
+   tf->protocol = ata_scsi_map_proto(cdb[1]);
+   else
+   tf->protocol = ata_scsi_map_proto(cdb[10]);
+
+   if (tf->protocol == ATA_PROT_UNKNOWN) {
fp = 1;
goto invalid_fld;
}
 
-   if (ata_is_ncq(tf->protocol) && (cdb[2] & 0x3) == 0)
-   tf->protocol = ATA_PROT_NCQ_NODATA;
+   /*
+* if protocol has a NCQ property and transfer length is 0,
+* then the protocol will be marked as a NCQ_NODATA.
+* in case of ATA_12 and ATA_16, cdb[2] has a t_length field.
+* otherwise, cdb[11] will have a t_length field.
+*/
+   if (cdb[0] == ATA_12 || cdb[0] == ATA_16) {
+   if (ata_is_ncq(tf->protocol) && (cdb[2] & 0x3) == 0)
+   tf->protocol = ATA_PROT_NCQ_NODATA;
+   } else {
+   if (ata_is_ncq(tf->protocol) && (cdb[11] & 0x3) == 0)
+   tf->protocol = ATA_PROT_NCQ_NODATA;
+   }
 
/* enable LBA */
tf->flags |= ATA_TFLAG_LBA;
@@ -3181,7 +3204,7 @@ static unsigned int ata_scsi_pass_thru(struct 
ata_queued_cmd *qc)
tf->lbah = cdb[12];
tf->device = cdb[13];
tf->command = cdb[14];
-   } else {
+   } else if (cdb[0] == ATA_12) {
/*
 * 12-byte CDB - incapable of extended commands.
 */
@@ -3194,6 +3217,31 @@ static unsigned int ata_scsi_pass_thru(struct 
ata_queued_cmd *qc)
tf->lbah = cdb[7];
tf->device = cdb[8];
tf->command = cdb[9];
+   } else {
+   /*
+* 32-byte CDB - may contain extended command fields.
+*
+* If that is the case, copy the upper byte register values.
+*/
+   if (cdb[10] & 0x01) {
+   tf->hob_feature = cdb[20];
+   tf->hob_nsect = cdb[22];
+   tf->hob_lbal = cdb[16];
+   tf->hob_lbam = cdb[15];
+   tf->hob_lbah = cdb[14];
+   tf->flags |= ATA_TFLAG_LBA48;
+   } else
+   tf->flags &= ~ATA_TFLAG_LBA48;
+
+   tf->feature = cdb[21];
+   tf->nsect = cdb[23];
+   tf->lbal = cdb[19];
+   tf->lbam = cdb[18];
+   tf->lbah = cdb[17];
+   tf->device = cdb[24];
+   tf->command = cdb[25];
+   tf->auxiliary = (cdb[28] << 24) | (cdb[29] << 16)
+   | (cdb[30] << 8) | cdb[31];
}
 
/* For NCQ commands copy the tag value */
@@ -4068,6 +4116,33 @@ static unsigned int ata_scsi_mode_select_xlat(struct 
ata_queued_cmd *qc)
 }
 
 /**
+ * ata_scsi_var_len_cdb_xlat - SATL Variable Length CDB to Handler
+ *

[PATCH] scsi/libata: Support variable-length cdb of ata pass-thru(32).

2017-06-17 Thread Minwoo Im 
Proposal Background:
SAT-4 supports ATA PASS-THROUGH(32) SCSI command to translate
SCSI commands to further ATA command format(e.g. auxiliary field).
To support any command translation from SCSI command to ATA,
This patch would be great for it.

Patch Description:
ATA PASS-THROUGH(32) command defines 7Fh as a opcode which means
a variable length command. It could be in any size from 12 to 260.
However, first to support ATA PASS-THROUGH(32), ATA Device max CDB
length has been modified to 32(It used to be 16).
Additionally, SCSI command max length has been modified to 260 to support
variable-length cdbs in kernel.

ata_get_xlat_func() adds a condition for variable length command.
ata_scsi_var_len_cdb_xlat() checks the ATA PASS-THROUGH(32) service action.
After checking, it will call ata_scsi_pass_thru() just like
ATA PASS-THROUGH(12) and (16).

ata_scsi_pass_thru() adds some code lines to support (32) pass-thru.
pass-thru(32) has a different CDB contents from 12, or 16.

Please refer below patch and fill free to give any opinions.

Thanks,

Minwoo.

Signed-off-by: Minwoo Im 
---
 drivers/ata/libata-core.c |2 +-
 drivers/ata/libata-scsi.c |   95 ++---
 include/scsi/scsi_proto.h |1 +
 3 files changed, 91 insertions(+), 7 deletions(-)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index e157a0e..f1d3ba4 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -2587,7 +2587,7 @@ int ata_dev_configure(struct ata_device *dev)
}
ata_dev_config_sense_reporting(dev);
ata_dev_config_zac(dev);
-   dev->cdb_len = 16;
+   dev->cdb_len = 32;
}
 
/* ATAPI-specific feature tests */
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 49ba983..b6e32d9 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -3127,7 +3127,7 @@ static struct ata_device *__ata_scsi_find_dev(struct 
ata_port *ap,
  * ata_scsi_pass_thru - convert ATA pass-thru CDB to taskfile
  * @qc: command structure to be initialized
  *
- * Handles either 12 or 16-byte versions of the CDB.
+ * Handles either 12, 16, or 32-byte versions of the CDB.
  *
  * RETURNS:
  * Zero on success, non-zero on failure.
@@ -3140,13 +3140,36 @@ static unsigned int ata_scsi_pass_thru(struct 
ata_queued_cmd *qc)
const u8 *cdb = scmd->cmnd;
u16 fp;
 
-   if ((tf->protocol = ata_scsi_map_proto(cdb[1])) == ATA_PROT_UNKNOWN) {
+   /*
+* if SCSI operation code in cdb[0] is ATA_12 or ATA_16,
+* then cdb[1] will contain protocol of ATA PASS-THROUGH.
+* otherwise, Its operation code shall be ATA_32(7Fh).
+* in this case, cdb[10] will contain protocol of it.
+* we call this command as a variable-length cdb.
+*/
+   if (cdb[0] == ATA_12 || cdb[0] == ATA_16)
+   tf->protocol = ata_scsi_map_proto(cdb[1]);
+   else
+   tf->protocol = ata_scsi_map_proto(cdb[10]);
+
+   if (tf->protocol == ATA_PROT_UNKNOWN) {
fp = 1;
goto invalid_fld;
}
 
-   if (ata_is_ncq(tf->protocol) && (cdb[2] & 0x3) == 0)
-   tf->protocol = ATA_PROT_NCQ_NODATA;
+   /*
+* if protocol has a NCQ property and transfer length is 0,
+* then the protocol will be marked as a NCQ_NODATA.
+* in case of ATA_12 and ATA_16, cdb[2] has a t_length field.
+* otherwise, cdb[11] will have a t_length field.
+*/
+   if (cdb[0] == ATA_12 || cdb[0] == ATA_16) {
+   if (ata_is_ncq(tf->protocol) && (cdb[2] & 0x3) == 0)
+   tf->protocol = ATA_PROT_NCQ_NODATA;
+   } else {
+   if (ata_is_ncq(tf->protocol) && (cdb[11] & 0x3) == 0)
+   tf->protocol = ATA_PROT_NCQ_NODATA;
+   }
 
/* enable LBA */
tf->flags |= ATA_TFLAG_LBA;
@@ -3181,7 +3204,7 @@ static unsigned int ata_scsi_pass_thru(struct 
ata_queued_cmd *qc)
tf->lbah = cdb[12];
tf->device = cdb[13];
tf->command = cdb[14];
-   } else {
+   } else if (cdb[0] == ATA_12) {
/*
 * 12-byte CDB - incapable of extended commands.
 */
@@ -3194,6 +3217,31 @@ static unsigned int ata_scsi_pass_thru(struct 
ata_queued_cmd *qc)
tf->lbah = cdb[7];
tf->device = cdb[8];
tf->command = cdb[9];
+   } else {
+   /*
+* 32-byte CDB - may contain extended command fields.
+*
+* If that is the case, copy the upper byte register values.
+*/
+   if (cdb[10] & 0x01) {
+   tf->hob_feature = cdb[20];
+   tf->hob_nsect = cdb[22];
+   tf->hob_lbal = cdb[16];
+