[v6,1/8] scsi: libsas: Add sas_ata_device_link_abort()
Commit Message
Similar to how AHCI handles NCQ errors in ahci_error_intr() ->
ata_port_abort() -> ata_do_link_abort(), add an NCQ error handler for LLDDs
to call to initiate a link abort.
This will mark all outstanding QCs as failed and kick-off EH.
Note:
A "force reset" argument is added for drivers which require the ATA error
handling to always reset the device.
A driver may require this feature for when SATA device per-SCSI cmnd
resources are only released during reset for ATA EH. As such, we need an
option to force reset to be done, regardless of what any EH autopsy
decides.
The SATA device FIS fields are set to indicate a device error from
ata_eh_analyze_tf().
Suggested-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Suggested-by: Tested-by: Niklas Cassel <niklas.cassel@wdc.com>
Signed-off-by: John Garry <john.garry@huawei.com>
Tested-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Reviewed-by: Jason Yan <yanaijie@huawei.com>
Tested-by: Niklas Cassel <niklas.cassel@wdc.com> # pm80xx
---
drivers/scsi/libsas/sas_ata.c | 15 +++++++++++++++
include/scsi/sas_ata.h | 6 ++++++
2 files changed, 21 insertions(+)
Comments
On Mon, Oct 17, 2022 at 05:20:28PM +0800, John Garry wrote:
> Similar to how AHCI handles NCQ errors in ahci_error_intr() ->
> ata_port_abort() -> ata_do_link_abort(), add an NCQ error handler for LLDDs
> to call to initiate a link abort.
>
> This will mark all outstanding QCs as failed and kick-off EH.
>
> Note:
> A "force reset" argument is added for drivers which require the ATA error
> handling to always reset the device.
>
> A driver may require this feature for when SATA device per-SCSI cmnd
> resources are only released during reset for ATA EH. As such, we need an
> option to force reset to be done, regardless of what any EH autopsy
> decides.
>
> The SATA device FIS fields are set to indicate a device error from
> ata_eh_analyze_tf().
>
> Suggested-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
> Suggested-by: Tested-by: Niklas Cassel <niklas.cassel@wdc.com>
Nit: is this perhaps a typo?
(Since there is another Tested-by tag later in the list.)
Checkpatch doesn't complain, so I guess no biggie,
but might be worth fixing if you decide to roll a v7.
Kind regards,
Niklas
> Signed-off-by: John Garry <john.garry@huawei.com>
> Tested-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
> Reviewed-by: Jason Yan <yanaijie@huawei.com>
> Tested-by: Niklas Cassel <niklas.cassel@wdc.com> # pm80xx
On 17/10/2022 11:52, Niklas Cassel wrote:
>> The SATA device FIS fields are set to indicate a device error from
>> ata_eh_analyze_tf().
>>
>> Suggested-by: Damien Le Moal<damien.lemoal@opensource.wdc.com>
>> Suggested-by: Tested-by: Niklas Cassel<niklas.cassel@wdc.com>
> Nit: is this perhaps a typo?
Yeah, a copy and paste error.
> (Since there is another Tested-by tag later in the list.)
>
> Checkpatch doesn't complain, so I guess no biggie,
> but might be worth fixing if you decide to roll a v7.
Sure
Thanks,
John
@@ -861,6 +861,21 @@ void sas_ata_wait_eh(struct domain_device *dev)
ata_port_wait_eh(ap);
}
+void sas_ata_device_link_abort(struct domain_device *device, bool force_reset)
+{
+ struct ata_port *ap = device->sata_dev.ap;
+ struct ata_link *link = &ap->link;
+
+ device->sata_dev.fis[2] = ATA_ERR | ATA_DRDY; /* tf status */
+ device->sata_dev.fis[3] = ATA_ABORTED; /* tf error */
+
+ link->eh_info.err_mask |= AC_ERR_DEV;
+ if (force_reset)
+ link->eh_info.action |= ATA_EH_RESET;
+ ata_link_abort(link);
+}
+EXPORT_SYMBOL_GPL(sas_ata_device_link_abort);
+
int sas_execute_ata_cmd(struct domain_device *device, u8 *fis, int force_phy_id)
{
struct sas_tmf_task tmf_task = {};
@@ -32,6 +32,7 @@ void sas_probe_sata(struct asd_sas_port *port);
void sas_suspend_sata(struct asd_sas_port *port);
void sas_resume_sata(struct asd_sas_port *port);
void sas_ata_end_eh(struct ata_port *ap);
+void sas_ata_device_link_abort(struct domain_device *dev, bool force_reset);
int sas_execute_ata_cmd(struct domain_device *device, u8 *fis,
int force_phy_id);
int sas_ata_wait_after_reset(struct domain_device *dev, unsigned long deadline);
@@ -87,6 +88,11 @@ static inline void sas_ata_end_eh(struct ata_port *ap)
{
}
+static inline void sas_ata_device_link_abort(struct domain_device *dev,
+ bool force_reset)
+{
+}
+
static inline int sas_execute_ata_cmd(struct domain_device *device, u8 *fis,
int force_phy_id)
{