Patchwork sata_sil24 failed command: READ FPDMA QUEUED (resolved)

login
register
mail settings
Submitter Robert Hancock
Date Aug. 6, 2010, 5:03 a.m.
Message ID <4C5B979E.8050606@gmail.com>
Download mbox | patch
Permalink /patch/61065/
State Not Applicable
Delegated to: David Miller
Headers show

Comments

Robert Hancock - Aug. 6, 2010, 5:03 a.m.
On 08/02/2010 11:33 PM, Tomas Lund wrote:
> On Mon, 2 Aug 2010, Robert Hancock wrote:
> 
>> On Mon, Aug 2, 2010 at 1:21 AM, Tomas Lund <tlund@nxs.se> wrote:
>>
>>> I have unlocked and tested a total of 4 disks now, both the samsung 1TB and 3 of the WDC 2TB disks. Thank you very much! Left to do: test that everything is working with the PMP aswell.
>>
>> Good to hear.
> 
> I now have all of my drives up and working, running on the PMP and everything is looking perfectly. Thanks again for your help.
> 
>>> Question: Would it be unreasonable to request that the kernel would issue the apropriate ATA-commands to a disk as soon as it is detected, and not try to read from it if it is locked? It would have been really, really nice to just get the error message "drive is locked".
>>
>> It should be possible to indicate that, yes. I'll look into it when I get a chance, if someone doesn't beat me to it..
> 
> Perfect!
> 
> //tlund

Can you test this patch out, assuming you still have anything left that's locked
(or can relock one of them to try it out)? It should print out a warning in
dmesg upon recognizing the device if it's in the security locked state.


--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Gwendal Grignou - Aug. 12, 2010, 2:09 p.m.
Hi Robert,

I had the same problem with locked drive earlier, causing a lot of
unnecessary errors. See "Do not send RW commands to locked disks."
thread.
It works right now, but it does not take in account disks with wrong
information in their identify page. I need to improve this patch: the
first IO will go through and the error handler will check the drive is
locked and then set a bit to prevent sending more command until the
UNLOCK command is issued.

Gwendal.

On Thu, Aug 5, 2010 at 10:03 PM, Robert Hancock <hancockrwd@gmail.com> wrote:
> On 08/02/2010 11:33 PM, Tomas Lund wrote:
>> On Mon, 2 Aug 2010, Robert Hancock wrote:
>>
>>> On Mon, Aug 2, 2010 at 1:21 AM, Tomas Lund <tlund@nxs.se> wrote:
>>>
>>>> I have unlocked and tested a total of 4 disks now, both the samsung 1TB and 3 of the WDC 2TB disks. Thank you very much! Left to do: test that everything is working with the PMP aswell.
>>>
>>> Good to hear.
>>
>> I now have all of my drives up and working, running on the PMP and everything is looking perfectly. Thanks again for your help.
>>
>>>> Question: Would it be unreasonable to request that the kernel would issue the apropriate ATA-commands to a disk as soon as it is detected, and not try to read from it if it is locked? It would have been really, really nice to just get the error message "drive is locked".
>>>
>>> It should be possible to indicate that, yes. I'll look into it when I get a chance, if someone doesn't beat me to it..
>>
>> Perfect!
>>
>> //tlund
>
> Can you test this patch out, assuming you still have anything left that's locked
> (or can relock one of them to try it out)? It should print out a warning in
> dmesg upon recognizing the device if it's in the security locked state.
>
> diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
> index ddf8e48..d4ec163 100644
> --- a/drivers/ata/libata-core.c
> +++ b/drivers/ata/libata-core.c
> @@ -2561,6 +2561,11 @@ int ata_dev_configure(struct ata_device *dev)
>                                       dma_dir_string);
>        }
>
> +       if (print_info && ata_id_security_locked(dev->id))
> +               ata_dev_printk(dev, KERN_WARNING,
> +                       "appears to have ATA security enabled and locked - "
> +                       "media may be inaccessible until unlocked\n");
> +
>        /* determine max_sectors */
>        dev->max_sectors = ATA_MAX_SECTORS;
>        if (dev->flags & ATA_DFLAG_LBA48)
> diff --git a/include/linux/ata.h b/include/linux/ata.h
> index fe6e681..f2359fa 100644
> --- a/include/linux/ata.h
> +++ b/include/linux/ata.h
> @@ -684,6 +684,22 @@ static inline int ata_id_hpa_enabled(const u16 *id)
>        return id[ATA_ID_COMMAND_SET_1] & (1 << 10);
>  }
>
> +static inline int ata_id_security_locked(const u16 *id)
> +{
> +       /* Yes children, word 83 valid bits cover word 82 data */
> +       if ((id[ATA_ID_COMMAND_SET_2] & 0xC000) != 0x4000)
> +               return 0;
> +       /* And 87 covers 85-87 */
> +       if ((id[ATA_ID_CSF_DEFAULT] & 0xC000) != 0x4000)
> +               return 0;
> +       /* Check command sets enabled as well as supported */
> +       if ((id[ATA_ID_CFS_ENABLE_1] & (1 << 1)) == 0)
> +               return 0;
> +       if ((id[ATA_ID_COMMAND_SET_1] & (1 << 1)) == 0)
> +               return 0;
> +       return (id[ATA_ID_DLF] & 0x0007) == 0x0007;
> +}
> +
>  static inline int ata_id_has_wcache(const u16 *id)
>  {
>        /* Yes children, word 83 valid bits cover word 82 data */
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ide" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index ddf8e48..d4ec163 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -2561,6 +2561,11 @@  int ata_dev_configure(struct ata_device *dev)
                                       dma_dir_string);
        }
 
+       if (print_info && ata_id_security_locked(dev->id))
+               ata_dev_printk(dev, KERN_WARNING,
+                       "appears to have ATA security enabled and locked - "
+                       "media may be inaccessible until unlocked\n");
+
        /* determine max_sectors */
        dev->max_sectors = ATA_MAX_SECTORS;
        if (dev->flags & ATA_DFLAG_LBA48)
diff --git a/include/linux/ata.h b/include/linux/ata.h
index fe6e681..f2359fa 100644
--- a/include/linux/ata.h
+++ b/include/linux/ata.h
@@ -684,6 +684,22 @@  static inline int ata_id_hpa_enabled(const u16 *id)
        return id[ATA_ID_COMMAND_SET_1] & (1 << 10);
 }
 
+static inline int ata_id_security_locked(const u16 *id)
+{
+       /* Yes children, word 83 valid bits cover word 82 data */
+       if ((id[ATA_ID_COMMAND_SET_2] & 0xC000) != 0x4000)
+               return 0;
+       /* And 87 covers 85-87 */
+       if ((id[ATA_ID_CSF_DEFAULT] & 0xC000) != 0x4000)
+               return 0;
+       /* Check command sets enabled as well as supported */
+       if ((id[ATA_ID_CFS_ENABLE_1] & (1 << 1)) == 0)
+               return 0;
+       if ((id[ATA_ID_COMMAND_SET_1] & (1 << 1)) == 0)
+               return 0;
+       return (id[ATA_ID_DLF] & 0x0007) == 0x0007;
+}
+
 static inline int ata_id_has_wcache(const u16 *id)
 {
        /* Yes children, word 83 valid bits cover word 82 data */