Interoperability problem with Quantum CD72SH SATA tape drive and libata

Interoperability problem with Quantum CD72SH SATA tape drive and libata

[John Clark]

Hello,

I've discovered an issue with the Quantum CD72SH SATA tape drive.  I set 
it to variable block size, try to write a 5317 byte block with an ATAPI 
issued WRITE(6), and it hangs.  The drive is connected to an Intel ICH9R 
in AHCI mode.

My debugging efforts have determined that it's a problem with the 
drive's firmware.  It does not like ATAPI CDBs with odd (& 1) transfer 
lengths with PIO.  If I let it use DMA, even if it's not a multiple of 
16 bytes, then it seems to work:

--- libata-core.c.orig    2008-09-27 16:12:46.000000000 -0400
+++ libata-core.c    2008-09-27 16:11:29.000000000 -0400
@@ -4669,8 +4669,8 @@
     /* Don't allow DMA if it isn't multiple of 16 bytes.  Quite a
      * few ATAPI devices choke on such DMA requests.
      */
-    if (unlikely(qc->nbytes & 15))
-        return 1;
+    // if (unlikely(qc->nbytes & 15))
+    //     return 1;
 
     if (ap->ops->check_atapi_dma)
         return ap->ops->check_atapi_dma(qc);



This is an acceptable solution for my limited use, but I'd like to see 
it work out of the box.  It seems frightening that there are ATAPI 
devices out there that choke on PIO requests.  What would you like to do?

Thank you.

- John

Re: Interoperability problem with Quantum CD72SH SATA tape drive and libata

[Tejun Heo]

John Clark wrote:
> Hello,
> 
> I've discovered an issue with the Quantum CD72SH SATA tape drive.  I set
> it to variable block size, try to write a 5317 byte block with an ATAPI
> issued WRITE(6), and it hangs.  The drive is connected to an Intel ICH9R
> in AHCI mode.
> 
> My debugging efforts have determined that it's a problem with the
> drive's firmware.  It does not like ATAPI CDBs with odd (& 1) transfer
> lengths with PIO.  If I let it use DMA, even if it's not a multiple of
> 16 bytes, then it seems to work:
> 
> --- libata-core.c.orig    2008-09-27 16:12:46.000000000 -0400
> +++ libata-core.c    2008-09-27 16:11:29.000000000 -0400
> @@ -4669,8 +4669,8 @@
>     /* Don't allow DMA if it isn't multiple of 16 bytes.  Quite a
>      * few ATAPI devices choke on such DMA requests.
>      */
> -    if (unlikely(qc->nbytes & 15))
> -        return 1;
> +    // if (unlikely(qc->nbytes & 15))
> +    //     return 1;
> 
>     if (ap->ops->check_atapi_dma)
>         return ap->ops->check_atapi_dma(qc);
> 
> 
> 
> This is an acceptable solution for my limited use, but I'd like to see
> it work out of the box.  It seems frightening that there are ATAPI
> devices out there that choke on PIO requests.  What would you like to do?

We can create a HORKAGE for it.  Can you please post the result of
"hdparm -I" of the device?

-- 
tejun

Re: Interoperability problem with Quantum CD72SH SATA tape drive and libata

[John Clark]

Tejun Heo wrote:
> John Clark wrote:
>   
>> Hello,
>>
>> I've discovered an issue with the Quantum CD72SH SATA tape drive.  I set
>> it to variable block size, try to write a 5317 byte block with an ATAPI
>> issued WRITE(6), and it hangs.  The drive is connected to an Intel ICH9R
>> in AHCI mode.
>>
>> My debugging efforts have determined that it's a problem with the
>> drive's firmware.  It does not like ATAPI CDBs with odd (& 1) transfer
>> lengths with PIO.  If I let it use DMA, even if it's not a multiple of
>> 16 bytes, then it seems to work:
>>
>> --- libata-core.c.orig    2008-09-27 16:12:46.000000000 -0400
>> +++ libata-core.c    2008-09-27 16:11:29.000000000 -0400
>> @@ -4669,8 +4669,8 @@
>>     /* Don't allow DMA if it isn't multiple of 16 bytes.  Quite a
>>      * few ATAPI devices choke on such DMA requests.
>>      */
>> -    if (unlikely(qc->nbytes & 15))
>> -        return 1;
>> +    // if (unlikely(qc->nbytes & 15))
>> +    //     return 1;
>>
>>     if (ap->ops->check_atapi_dma)
>>         return ap->ops->check_atapi_dma(qc);
>>
>>
>>
>> This is an acceptable solution for my limited use, but I'd like to see
>> it work out of the box.  It seems frightening that there are ATAPI
>> devices out there that choke on PIO requests.  What would you like to do?
>>     
>
> We can create a HORKAGE for it.  Can you please post the result of
> "hdparm -I" of the device?
>
>   
Tejun,

Here it is:

/dev/nst0:

ATAPI Sequential-access device, with removable media
    Model Number:       QUANTUM DAT    DAT72-000               
    Serial Number:      JR001H1            
    Firmware Revision:  VC090000
Standards:
    Used: ATA/ATAPI-7 T13 1532D revision 4a
    Supported: 7 6 5 4
Configuration:
    DRQ response: 50us.
    Packet size: 12 bytes
Capabilities:
    LBA, IORDY(can be disabled)
    DMA: mdma0 mdma1 mdma2 udma0 udma1 udma2 udma3 udma4 udma5 *udma6
         Cycle time: min=120ns recommended=120ns
    PIO: pio0 pio1 pio2 pio3 pio4
         Cycle time: no flow control=120ns  IORDY flow control=120ns
Commands/features:
    Enabled    Supported:
       *    Power Management feature set
       *    PACKET command feature set
       *    DEVICE_RESET command
       *    NOP cmd
       *    SATA-I signaling speed (1.5Gb/s)
Integrity word not set (found 0x0000, expected 0x6da5)

Thank you.

- John

Re: Interoperability problem with Quantum CD72SH SATA tape drive and libata

[Tejun Heo]

[-- Attachment #1: Type: text/plain, Size: 742 bytes --]

John Clark wrote:
>> We can create a HORKAGE for it.  Can you please post the result of
>> "hdparm -I" of the device?
>>
>>   
> Tejun,
> 
> Here it is:
> 
> /dev/nst0:
> 
> ATAPI Sequential-access device, with removable media
>    Model Number:       QUANTUM DAT    DAT72-000

Can you please test the attached patch?

Alan, John Clark is reporting that the above tape drive chokes on odd
byte PIO transfer, so the default condition check (qc->nbytes & 15) in
atapi_check_dma() breaks the SATA tape drive, so there's always one
which breaks the other way around.  :-(

For the time being, I think the horkage should do.  I was wondering
whether you still think we should lift the default mod16 check in
atapi_check_dma().

Thanks.

-- 
tejun

[-- Attachment #2: horkage_atapi_mod16_dma.patch --]
[-- Type: text/x-patch, Size: 1384 bytes --]

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 1ee9499..e97afce 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -4023,6 +4023,7 @@ static const struct ata_blacklist_entry ata_device_blacklist [] = {
 
 	/* Weird ATAPI devices */
 	{ "TORiSAN DVD-ROM DRD-N216", NULL,	ATA_HORKAGE_MAX_SEC_128 },
+	{ "QUANTUM DAT    DAT72-000", NULL,	ATA_HORKAGE_ATAPI_MOD16_DMA },
 
 	/* Devices we expect to fail diagnostics */
 
@@ -4436,7 +4437,8 @@ int atapi_check_dma(struct ata_queued_cmd *qc)
 	/* Don't allow DMA if it isn't multiple of 16 bytes.  Quite a
 	 * few ATAPI devices choke on such DMA requests.
 	 */
-	if (unlikely(qc->nbytes & 15))
+	if (!(qc->dev->horkage & ATA_HORKAGE_ATAPI_MOD16_DMA) &&
+	    unlikely(qc->nbytes & 15))
 		return 1;
 
 	if (ap->ops->check_atapi_dma)
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 8bb81d0..9d903b3 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -371,6 +371,8 @@ enum {
 	ATA_HORKAGE_IPM		= (1 << 7),	/* Link PM problems */
 	ATA_HORKAGE_IVB		= (1 << 8),	/* cbl det validity bit bugs */
 	ATA_HORKAGE_STUCK_ERR	= (1 << 9),	/* stuck ERR on next PACKET */
+	ATA_HORKAGE_ATAPI_MOD16_DMA = (1 << 10), /* use ATAPI DMA for commands
+						    not multiple of 16 bytes */
 
 	 /* DMA mask for user DMA control: User visible values; DO NOT
 	    renumber */

Re: Interoperability problem with Quantum CD72SH SATA tape drive and libata

[John Clark]

Tejun Heo wrote:
> Can you please test the attached patch?
>
> Alan, John Clark is reporting that the above tape drive chokes on odd
> byte PIO transfer, so the default condition check (qc->nbytes & 15) in
> atapi_check_dma() breaks the SATA tape drive, so there's always one
> which breaks the other way around.  :-(
>
> For the time being, I think the horkage should do.  I was wondering
> whether you still think we should lift the default mod16 check in
> atapi_check_dma().
>
> Thanks.
>   

Tejun,

I have tested the patch you supplied.  It works, as expected.

Thanks.

- John

Re: Interoperability problem with Quantum CD72SH SATA tape drive and libata

[Tejun Heo]

John Clark wrote:
> Tejun Heo wrote:
>> Can you please test the attached patch?
>>
>> Alan, John Clark is reporting that the above tape drive chokes on odd
>> byte PIO transfer, so the default condition check (qc->nbytes & 15) in
>> atapi_check_dma() breaks the SATA tape drive, so there's always one
>> which breaks the other way around.  :-(
>>
>> For the time being, I think the horkage should do.  I was wondering
>> whether you still think we should lift the default mod16 check in
>> atapi_check_dma().
>>
>> Thanks.
>>   
> 
> Tejun,
> 
> I have tested the patch you supplied.  It works, as expected.

applied to #tj-upstream.  Thanks.

-- 
tejun