Re: [PATCH v2 5/8] hw/ide/ahci: PxCI should not get cleared when ERR_STAT is set

[John Snow]

On Thu, Jun 1, 2023 at 9:46 AM Niklas Cassel <nks@flawful.org> wrote:
>
> From: Niklas Cassel <niklas.cassel@wdc.com>
>
> For NCQ, PxCI is cleared on command queued successfully.
> For non-NCQ, PxCI is cleared on command completed successfully.
> Successfully means ERR_STAT, BUSY and DRQ are all cleared.
>
> A command that has ERR_STAT set, does not get to clear PxCI.
> See AHCI 1.3.1, section 5.3.8, states RegFIS:Entry and RegFIS:ClearCI,
> and 5.3.16.5 ERR:FatalTaskfile.
>
> In the case of non-NCQ commands, not clearing PxCI is needed in order
> for host software to be able to see which command slot that failed.
>
> Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>

This patch causes the ahci test suite to hang. You might just need to
update the AHCI test suite.

"make check" will hang on the ahci-test as of this patch.

--js

> ---
>  hw/ide/ahci.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
> index 3deaf01add..1237f94ddc 100644
> --- a/hw/ide/ahci.c
> +++ b/hw/ide/ahci.c
> @@ -1518,7 +1518,8 @@ static void ahci_clear_cmd_issue(AHCIDevice *ad, 
> uint8_t slot)
>  {
>      IDEState *ide_state = &ad->port.ifs[0];
>
> -    if (!(ide_state->status & (BUSY_STAT | DRQ_STAT))) {
> +    if (!(ide_state->status & ERR_STAT) &&
> +        !(ide_state->status & (BUSY_STAT | DRQ_STAT))) {
>          ad->port_regs.cmd_issue &= ~(1 << slot);
>      }
>  }
> @@ -1527,6 +1528,7 @@ static void ahci_clear_cmd_issue(AHCIDevice *ad, 
> uint8_t slot)
>  static void ahci_cmd_done(const IDEDMA *dma)
>  {
>      AHCIDevice *ad = DO_UPCAST(AHCIDevice, dma, dma);
> +    IDEState *ide_state = &ad->port.ifs[0];
>
>      trace_ahci_cmd_done(ad->hba, ad->port_no);
>
> @@ -1543,7 +1545,8 @@ static void ahci_cmd_done(const IDEDMA *dma)
>       */
>      ahci_write_fis_d2h(ad, true);
>
> -    if (ad->port_regs.cmd_issue && !ad->check_bh) {
> +    if (!(ide_state->status & ERR_STAT) &&
> +        ad->port_regs.cmd_issue && !ad->check_bh) {
>          ad->check_bh = qemu_bh_new_guarded(ahci_check_cmd_bh, ad,
>                                             &ad->mem_reentrancy_guard);
>          qemu_bh_schedule(ad->check_bh);
> --
> 2.40.1
>