1. Patchew
  2. QEMU
  3. [PATCH v2 0/8] misc AHCI cleanups
  4. View patch

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

Niklas Cassel posted 8 patches 2 years, 8 months ago
Maintainers: John Snow <jsnow@redhat.com>
There is a newer version of this series
[PATCH v2 5/8] hw/ide/ahci: PxCI should not get cleared when ERR_STAT is set
Posted by Niklas Cassel 2 years, 8 months ago 
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>
---
 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
Re: [PATCH v2 5/8] hw/ide/ahci: PxCI should not get cleared when ERR_STAT is set
Posted by John Snow 2 years, 8 months ago 
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
>
Re: [PATCH v2 5/8] hw/ide/ahci: PxCI should not get cleared when ERR_STAT is set
Posted by Niklas Cassel 2 years, 8 months ago 
On Mon, Jun 05, 2023 at 08:19:43PM -0400, John Snow wrote:
> 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.

Argh :)

Is there any simple way to run only the ahci test suite?

"make check" and "make check-qtest" are running many tests that I'm not
interested in.


Kind regards,
Niklas
Re: [PATCH v2 5/8] hw/ide/ahci: PxCI should not get cleared when ERR_STAT is set
Posted by Niklas Cassel 2 years, 8 months ago 
On Wed, Jun 07, 2023 at 06:01:17PM +0200, Niklas Cassel wrote:
> On Mon, Jun 05, 2023 at 08:19:43PM -0400, John Snow wrote:
> > 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.
> 
> Argh :)
> 
> Is there any simple way to run only the ahci test suite?

To answer my own question:
QTEST_QEMU_BINARY=./build/qemu-system-x86_64 QTEST_QEMU_IMG=./build/qemu-img gtester -k --verbose -m=quick build/tests/qtest/ahci-test -o test_log.xml


Kind regards,
Niklas

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.