libata and software reset

libata and software reset

[John Garry]

Hi guys,

In the hisi_sas driver there are times in which we need to issue an ATA 
software reset. For this we use hisi_sas_softreset_ata_disk() -> 
sas_execute_ata_cmd() -> sas_execute_tmf(), which uses libsas "slow 
task" mechanism to issue the command.

I would like if libata provided such a function to issue a software 
reset, such that we can send the command as an ATA queued command.

The problem is that often when we would want to issue this software 
reset the associated ata port is frozen, like in ATA EH, and so we 
cannot issue ATA queued commands - internal or normal - at that time.

Is there any way to solve this? Or I am just misunderstanding how and 
when ATA queued commands can and should be used?

I assume that ata_port_operations.softreset callback requires a method 
to be able to issue the softreset directly from the driver, like 
ahci_softreset() -> ahci_do_softreset() -> ahci_exec_polled_cmd().

Thanks,
John

Re: libata and software reset

[Niklas Cassel]

On Tue, Oct 18, 2022 at 02:24:00PM +0100, John Garry wrote:
> Hi guys,
>
> In the hisi_sas driver there are times in which we need to issue an ATA
> software reset. For this we use hisi_sas_softreset_ata_disk() ->
> sas_execute_ata_cmd() -> sas_execute_tmf(), which uses libsas "slow task"
> mechanism to issue the command.
>
> I would like if libata provided such a function to issue a software reset,
> such that we can send the command as an ATA queued command.
>
> The problem is that often when we would want to issue this software reset
> the associated ata port is frozen, like in ATA EH, and so we cannot issue
> ATA queued commands - internal or normal - at that time.
>
> Is there any way to solve this? Or I am just misunderstanding how and when
> ATA queued commands can and should be used?
>

Hello John,

See the kdoc above __ata_port_freeze():
"This function is called when HSM violation or some other
condition disrupts normal operation of the port.  Frozen port
is not allowed to perform any operation until the port is
thawed, which usually follows a successful reset.

ap->ops->freeze() callback can be used for freezing the port
hardware-wise (e.g. mask interrupt and stop DMA engine).  If a
port cannot be frozen hardware-wise, the interrupt handler
must ack and clear interrupts unconditionally while the port
is frozen."


ata_port_operations.qc_issue() is obviously an operation on the port,
so it makes sense that it is not allowed.
Interrupts are also usually masked or disabled at this time, so we
won't get an IRQ with the completion.

Perhaps one could argue that there could be an API to execute a polled
command. But if the port is in a bad state, e.g. a HSM error (RDY bit
is not set), issuing a command would likely fail anyway, regardless if
using polling or IRQs.


> I assume that ata_port_operations.softreset callback requires a method to be
> able to issue the softreset directly from the driver, like ahci_softreset()
> -> ahci_do_softreset() -> ahci_exec_polled_cmd().

Yes, looking .softreset in a few ata drivers, they all seem issue the
softreset directly from the driver.
(e.g. ahci_do_softreset() calls ahci_exec_polled_cmd() which just always
uses bit 0 in PORT_CMD_ISSUE, so it ignores hw_tag.)

But I don't think that I fully understand your problem.

hisi_sas_softreset_ata_disk() -> sas_execute_ata_cmd() -> sas_execute_tmf()
calls lldd_execute_task() (hisi_sas_queue_command()) and then calls
waits for completion.

How is this different from e.g. the libahci case?
Doesn't this end up being the same as resetting the port directly from the
driver? (if we ignore all the callbacks)
Or do you actually get stuck on a ata_port_is_frozen() check somewhere?


Kind regards,
Niklas

Re: libata and software reset

[Damien Le Moal]

On 10/18/22 22:24, John Garry wrote:
> Hi guys,
> 
> In the hisi_sas driver there are times in which we need to issue an ATA
> software reset. For this we use hisi_sas_softreset_ata_disk() ->
> sas_execute_ata_cmd() -> sas_execute_tmf(), which uses libsas "slow
> task" mechanism to issue the command.

Something is wrong here... The reset command sent by that function is
for ATAPI (DEVICE RESET command). There is no device reset command for
SATA disks following the ACS standard.

So hisi_sas_softreset_ata_disk() seems totally bogus to me, unless you
have a CD/DVD drive connected to the HBA :)

This is why the softreset function is a port operation defined by LLDs.
How you reset the device depends on the adapter. E.g., for AHCI, you
need to send a host2device FIS with the software reset bit set.

> 
> I would like if libata provided such a function to issue a software
> reset, such that we can send the command as an ATA queued command.
> 
> The problem is that often when we would want to issue this software
> reset the associated ata port is frozen, like in ATA EH, and so we
> cannot issue ATA queued commands - internal or normal - at that time.
> 
> Is there any way to solve this? Or I am just misunderstanding how and
> when ATA queued commands can and should be used?
> 
> I assume that ata_port_operations.softreset callback requires a method
> to be able to issue the softreset directly from the driver, like
> ahci_softreset() -> ahci_do_softreset() -> ahci_exec_polled_cmd().
> 
> Thanks,
> John

-- 
Damien Le Moal
Western Digital Research

Re: libata and software reset

[Damien Le Moal]

On 10/19/22 14:03, Damien Le Moal wrote:
> On 10/18/22 22:24, John Garry wrote:
>> Hi guys,
>>
>> In the hisi_sas driver there are times in which we need to issue an ATA
>> software reset. For this we use hisi_sas_softreset_ata_disk() ->
>> sas_execute_ata_cmd() -> sas_execute_tmf(), which uses libsas "slow
>> task" mechanism to issue the command.
> 
> Something is wrong here... The reset command sent by that function is
> for ATAPI (DEVICE RESET command). There is no device reset command for
> SATA disks following the ACS standard.
> 
> So hisi_sas_softreset_ata_disk() seems totally bogus to me, unless you
> have a CD/DVD drive connected to the HBA :)
> 
> This is why the softreset function is a port operation defined by LLDs.
> How you reset the device depends on the adapter. E.g., for AHCI, you
> need to send a host2device FIS with the software reset bit set.

See: ahci_do_softreset() for AHCI.

> 
>>
>> I would like if libata provided such a function to issue a software
>> reset, such that we can send the command as an ATA queued command.
>>
>> The problem is that often when we would want to issue this software
>> reset the associated ata port is frozen, like in ATA EH, and so we
>> cannot issue ATA queued commands - internal or normal - at that time.
>>
>> Is there any way to solve this? Or I am just misunderstanding how and
>> when ATA queued commands can and should be used?
>>
>> I assume that ata_port_operations.softreset callback requires a method
>> to be able to issue the softreset directly from the driver, like
>> ahci_softreset() -> ahci_do_softreset() -> ahci_exec_polled_cmd().
>>
>> Thanks,
>> John
> 

-- 
Damien Le Moal
Western Digital Research

Re: libata and software reset

[John Garry]

On 18/10/2022 16:04, Niklas Cassel wrote:
>> Hi guys,
>>
>> In the hisi_sas driver there are times in which we need to issue an ATA
>> software reset. For this we use hisi_sas_softreset_ata_disk() ->
>> sas_execute_ata_cmd() -> sas_execute_tmf(), which uses libsas "slow task"
>> mechanism to issue the command.
>>
>> I would like if libata provided such a function to issue a software reset,
>> such that we can send the command as an ATA queued command.
>>
>> The problem is that often when we would want to issue this software reset
>> the associated ata port is frozen, like in ATA EH, and so we cannot issue
>> ATA queued commands - internal or normal - at that time.
>>
>> Is there any way to solve this? Or I am just misunderstanding how and when
>> ATA queued commands can and should be used?
>>
> Hello John,

Hi Niklas,

> 
> See the kdoc above __ata_port_freeze():
> "This function is called when HSM violation or some other
> condition disrupts normal operation of the port.  Frozen port
> is not allowed to perform any operation until the port is
> thawed, which usually follows a successful reset.

ok, I see.

> 
> ap->ops->freeze() callback can be used for freezing the port
> hardware-wise (e.g. mask interrupt and stop DMA engine).  If a
> port cannot be frozen hardware-wise, the interrupt handler
> must ack and clear interrupts unconditionally while the port
> is frozen."
> 
> 
> ata_port_operations.qc_issue() is obviously an operation on the port,
> so it makes sense that it is not allowed.

hmmm..ok, then.


> Interrupts are also usually masked or disabled at this time, so we
> won't get an IRQ with the completion.

Doesn't this policy really just depend on the host controller driver?

> 
> Perhaps one could argue that there could be an API to execute a polled
> command. But if the port is in a bad state,
  e.g. a HSM error (RDY bit
> is not set), issuing a command would likely fail anyway, regardless if
> using polling or IRQs.
> 
> 
>> I assume that ata_port_operations.softreset callback requires a method to be
>> able to issue the softreset directly from the driver, like ahci_softreset()
>> -> ahci_do_softreset() -> ahci_exec_polled_cmd().
> Yes, looking .softreset in a few ata drivers, they all seem issue the
> softreset directly from the driver.
> (e.g. ahci_do_softreset() calls ahci_exec_polled_cmd() which just always
> uses bit 0 in PORT_CMD_ISSUE, so it ignores hw_tag.)
> 
> But I don't think that I fully understand your problem.
> 
> hisi_sas_softreset_ata_disk() -> sas_execute_ata_cmd() -> sas_execute_tmf()
> calls lldd_execute_task() (hisi_sas_queue_command()) and then calls
> waits for completion.
> 
> How is this different from e.g. the libahci case?

The difference really comes down to the controller programming interface.

For ahci we have a MMIO interface to issue the software reset command.

For my SAS controller of interest, there is no such MMIO interface. To 
issue the reset we build a h2d fis with a SRST set, and send on the 
controller ring buffer like any other IO.

As I mentioned, we can set the SRST for the h2d fis on the HW interface 
without issue, and it works fine. The problem for me is that the command 
comes via libsas/driver, and I would like it to come from libata such 
that it has a ATA queued command associated. But then we have the 
problem that the port is frozen at such times that we want to issue this 
command.

> Doesn't this end up being the same as resetting the port directly from the
> driver? (if we ignore all the callbacks)
> Or do you actually get stuck on a ata_port_is_frozen() check somewhere?


Thanks,
John

Re: libata and software reset

[John Garry]

On 19/10/2022 06:04, Damien Le Moal wrote:
> On 10/19/22 14:03, Damien Le Moal wrote:
>> On 10/18/22 22:24, John Garry wrote:
>>> Hi guys,
>>>
>>> In the hisi_sas driver there are times in which we need to issue an ATA
>>> software reset. For this we use hisi_sas_softreset_ata_disk() ->
>>> sas_execute_ata_cmd() -> sas_execute_tmf(), which uses libsas "slow
>>> task" mechanism to issue the command.
>> Something is wrong here... The reset command sent by that function is
>> for ATAPI (DEVICE RESET command). There is no device reset command for
>> SATA disks following the ACS standard.

Yeah, that looks wrong.

>>
>> So hisi_sas_softreset_ata_disk() seems totally bogus to me, unless you
>> have a CD/DVD drive connected to the HBA:)

Sure

>>
>> This is why the softreset function is a port operation defined by LLDs.
>> How you reset the device depends on the adapter. E.g., for AHCI, you
>> need to send a host2device FIS with the software reset bit set.

This would be quite a standard method, right?

> See: ahci_do_softreset() for AHCI.

For ahci_do_softreset(), do you just implicitly use ATA_CMD_NOP as the 
command?

For hisi_sas, maybe ATA_CMD_DEV_RESET is silently ignored when issued 
for a SATA disk, but having SRST set/unset still takes effect (and that 
is how it still works). I need to check on that.

Thanks,
John

Re: libata and software reset

[Damien Le Moal]

On 10/19/22 18:42, John Garry wrote:
> On 19/10/2022 06:04, Damien Le Moal wrote:
>> On 10/19/22 14:03, Damien Le Moal wrote:
>>> On 10/18/22 22:24, John Garry wrote:
>>>> Hi guys,
>>>>
>>>> In the hisi_sas driver there are times in which we need to issue an ATA
>>>> software reset. For this we use hisi_sas_softreset_ata_disk() ->
>>>> sas_execute_ata_cmd() -> sas_execute_tmf(), which uses libsas "slow
>>>> task" mechanism to issue the command.
>>> Something is wrong here... The reset command sent by that function is
>>> for ATAPI (DEVICE RESET command). There is no device reset command for
>>> SATA disks following the ACS standard.
> 
> Yeah, that looks wrong.
> 
>>>
>>> So hisi_sas_softreset_ata_disk() seems totally bogus to me, unless you
>>> have a CD/DVD drive connected to the HBA:)
> 
> Sure
> 
>>>
>>> This is why the softreset function is a port operation defined by LLDs.
>>> How you reset the device depends on the adapter. E.g., for AHCI, you
>>> need to send a host2device FIS with the software reset bit set.
> 
> This would be quite a standard method, right?

The TF part should be standard. Would need to dig in SATA-IO specs to
check. How the TF/FIS should be issued/polled for is definitely
dependent on the adapter itself I think, exactly like issuing a qc is.

> 
>> See: ahci_do_softreset() for AHCI.
> 
> For ahci_do_softreset(), do you just implicitly use ATA_CMD_NOP as the
> command?

To be frank, it is the first time I really look at the reset code :)
I need to read SATA-IO specs to understand why it is doing things like that.

> 
> For hisi_sas, maybe ATA_CMD_DEV_RESET is silently ignored when issued
> for a SATA disk, but having SRST set/unset still takes effect (and that
> is how it still works). I need to check on that.

Yes, it may be that having the ATA_SRST bit set causes the device to
ignore the command field, hence you never saw any problem. Still feels
terribly wrong to use an ATAPI command for an ATA disk. ATAPI device
reset command code is 0x08 which is not defined as a command code in ACS.

> 
> Thanks,
> John

-- 
Damien Le Moal
Western Digital Research

Re: libata and software reset

[Damien Le Moal]

On 10/19/22 18:32, John Garry wrote:
> On 18/10/2022 16:04, Niklas Cassel wrote:
>>> Hi guys,
>>>
>>> In the hisi_sas driver there are times in which we need to issue an ATA
>>> software reset. For this we use hisi_sas_softreset_ata_disk() ->
>>> sas_execute_ata_cmd() -> sas_execute_tmf(), which uses libsas "slow
>>> task"
>>> mechanism to issue the command.
>>>
>>> I would like if libata provided such a function to issue a software
>>> reset,
>>> such that we can send the command as an ATA queued command.
>>>
>>> The problem is that often when we would want to issue this software
>>> reset
>>> the associated ata port is frozen, like in ATA EH, and so we cannot
>>> issue
>>> ATA queued commands - internal or normal - at that time.
>>>
>>> Is there any way to solve this? Or I am just misunderstanding how and
>>> when
>>> ATA queued commands can and should be used?
>>>
>> Hello John,
> 
> Hi Niklas,
> 
>>
>> See the kdoc above __ata_port_freeze():
>> "This function is called when HSM violation or some other
>> condition disrupts normal operation of the port.  Frozen port
>> is not allowed to perform any operation until the port is
>> thawed, which usually follows a successful reset.
> 
> ok, I see.
> 
>>
>> ap->ops->freeze() callback can be used for freezing the port
>> hardware-wise (e.g. mask interrupt and stop DMA engine).  If a
>> port cannot be frozen hardware-wise, the interrupt handler
>> must ack and clear interrupts unconditionally while the port
>> is frozen."
>>
>>
>> ata_port_operations.qc_issue() is obviously an operation on the port,
>> so it makes sense that it is not allowed.
> 
> hmmm..ok, then.
> 
> 
>> Interrupts are also usually masked or disabled at this time, so we
>> won't get an IRQ with the completion.
> 
> Doesn't this policy really just depend on the host controller driver?
> 
>>
>> Perhaps one could argue that there could be an API to execute a polled
>> command. But if the port is in a bad state,
>  e.g. a HSM error (RDY bit
>> is not set), issuing a command would likely fail anyway, regardless if
>> using polling or IRQs.
>>
>>
>>> I assume that ata_port_operations.softreset callback requires a
>>> method to be
>>> able to issue the softreset directly from the driver, like
>>> ahci_softreset()
>>> -> ahci_do_softreset() -> ahci_exec_polled_cmd().
>> Yes, looking .softreset in a few ata drivers, they all seem issue the
>> softreset directly from the driver.
>> (e.g. ahci_do_softreset() calls ahci_exec_polled_cmd() which just always
>> uses bit 0 in PORT_CMD_ISSUE, so it ignores hw_tag.)
>>
>> But I don't think that I fully understand your problem.
>>
>> hisi_sas_softreset_ata_disk() -> sas_execute_ata_cmd() ->
>> sas_execute_tmf()
>> calls lldd_execute_task() (hisi_sas_queue_command()) and then calls
>> waits for completion.
>>
>> How is this different from e.g. the libahci case?
> 
> The difference really comes down to the controller programming interface.
> 
> For ahci we have a MMIO interface to issue the software reset command.
> 
> For my SAS controller of interest, there is no such MMIO interface. To
> issue the reset we build a h2d fis with a SRST set, and send on the
> controller ring buffer like any other IO.
> 
> As I mentioned, we can set the SRST for the h2d fis on the HW interface
> without issue, and it works fine. The problem for me is that the command
> comes via libsas/driver, and I would like it to come from libata such
> that it has a ATA queued command associated. But then we have the
> problem that the port is frozen at such times that we want to issue this
> command.

Yeah, qc is too high level for this to work. But we could probably do
something generic at TF or FIS level. libata-sata.c has already some
code in that area, something for a "reset TF/FIS" would fit in that file
too. libahci could also use that too.

> 
>> Doesn't this end up being the same as resetting the port directly from
>> the
>> driver? (if we ignore all the callbacks)
>> Or do you actually get stuck on a ata_port_is_frozen() check somewhere?
> 
> 
> Thanks,
> John

-- 
Damien Le Moal
Western Digital Research

Re: libata and software reset

[Damien Le Moal]

On 10/19/22 18:42, John Garry wrote:
> On 19/10/2022 06:04, Damien Le Moal wrote:
>> On 10/19/22 14:03, Damien Le Moal wrote:
>>> On 10/18/22 22:24, John Garry wrote:
>>>> Hi guys,
>>>>
>>>> In the hisi_sas driver there are times in which we need to issue an ATA
>>>> software reset. For this we use hisi_sas_softreset_ata_disk() ->
>>>> sas_execute_ata_cmd() -> sas_execute_tmf(), which uses libsas "slow
>>>> task" mechanism to issue the command.
>>> Something is wrong here... The reset command sent by that function is
>>> for ATAPI (DEVICE RESET command). There is no device reset command for
>>> SATA disks following the ACS standard.
> 
> Yeah, that looks wrong.
> 
>>>
>>> So hisi_sas_softreset_ata_disk() seems totally bogus to me, unless you
>>> have a CD/DVD drive connected to the HBA:)
> 
> Sure
> 
>>>
>>> This is why the softreset function is a port operation defined by LLDs.
>>> How you reset the device depends on the adapter. E.g., for AHCI, you
>>> need to send a host2device FIS with the software reset bit set.
> 
> This would be quite a standard method, right?
> 
>> See: ahci_do_softreset() for AHCI.
> 
> For ahci_do_softreset(), do you just implicitly use ATA_CMD_NOP as the
> command?
> 
> For hisi_sas, maybe ATA_CMD_DEV_RESET is silently ignored when issued
> for a SATA disk, but having SRST set/unset still takes effect (and that
> is how it still works). I need to check on that.

Checked SATA-IO v3.5a. Software reset is explained in "11.4
Software reset protocol" and involves 2 things for the host to do:

DSR0: Software_reset_asserted, this state is entered if a Register Host
to Device FIS is received with the C bit in the FIS cleared to zero and
the SRST bit set to one in the Device Control register.
If in this state, the device begins its initialization and diagnostics
processing and awaits the clearing of the SRST bit.

DSR1: Execute_diagnostics, this state is entered if a Register Host to
Device FIS is received with the C bit in the FIS cleared to zero and the
SRST bit cleared to zero in the Device Control register.
If in this state, the device completes initialization and processing of
its diagnostics.

Which confirms what libahci is doing: essentially zeroing a tf with
ata_tf_init() and setting + resetting the SRST bit, sending the tf each
time.

-- 
Damien Le Moal
Western Digital Research

Re: libata and software reset

[John Garry]

On 19/10/2022 10:56, Damien Le Moal wrote:
>> The difference really comes down to the controller programming interface.
>>
>> For ahci we have a MMIO interface to issue the software reset command.
>>
>> For my SAS controller of interest, there is no such MMIO interface. To
>> issue the reset we build a h2d fis with a SRST set, and send on the
>> controller ring buffer like any other IO.
>>
>> As I mentioned, we can set the SRST for the h2d fis on the HW interface
>> without issue, and it works fine. The problem for me is that the command
>> comes via libsas/driver, and I would like it to come from libata such
>> that it has a ATA queued command associated. But then we have the
>> problem that the port is frozen at such times that we want to issue this
>> command.
> Yeah, qc is too high level for this to work.

Some more background is that this is all related to the "reserved" 
commands work. The issue is that it is difficult to differentiate 
between this libsas softreset command and normal ATA queued commands - 
the normal check is "is the device associated sata", but that doesn't 
work. If we could always have a ATA queued command, then that would be 
better. But, as you say, it is too high level.

Let me check what else I would do. BTW, I will send an update on that 
work in a day or so.

> But we could probably do
> something generic at TF or FIS level. libata-sata.c has already some
> code in that area, something for a "reset TF/FIS" would fit in that file
> too. libahci could also use that too.
>

yeah, that would seem a good consolidation.

Thanks!

Re: libata and software reset

[John Garry]

On 19/10/2022 11:15, Damien Le Moal wrote:
>> For hisi_sas, maybe ATA_CMD_DEV_RESET is silently ignored when issued
>> for a SATA disk, but having SRST set/unset still takes effect (and that
>> is how it still works). I need to check on that.
> Checked SATA-IO v3.5a. Software reset is explained in "11.4
> Software reset protocol" and involves 2 things for the host to do:
> 
> DSR0: Software_reset_asserted, this state is entered if a Register Host
> to Device FIS is received with the C bit in the FIS cleared to zero and
> the SRST bit set to one in the Device Control register.
> If in this state, the device begins its initialization and diagnostics
> processing and awaits the clearing of the SRST bit.
> 
> DSR1: Execute_diagnostics, this state is entered if a Register Host to
> Device FIS is received with the C bit in the FIS cleared to zero and the
> SRST bit cleared to zero in the Device Control register.
> If in this state, the device completes initialization and processing of
> its diagnostics.
> 
> Which confirms what libahci is doing: essentially zeroing a tf with
> ata_tf_init() and setting + resetting the SRST bit, sending the tf each
> time.

Ah, so since the C bit is not set in hisi_sas_fill_ata_reset_cmd(), I 
think that the command field is just ignored. Indeed, the spec says that 
setting C and SRST together is invalid.

Thanks,
John