ahci: Fix FLUSH command
PATCH 1/1 ahci: Fix FLUSH command
AHCI couldn't cope with asynchronous commands that aren't doing DMA, it simply wouldn't complete them. Due to the bug fixed in commit f68ec837, FLUSH commands would seem to have completed immediately even if they were still running on the host. After the commit, they would simply hang and never unset the BSY bit, rendering AHCI unusable on any OS sending flushes.
This patch adds another callback for the completion of asynchronous commands. This is what AHCI really wants to use for its command completion logic rather than an DMA completion callback.
--- hw/ide/ahci.c | 8 +++++++- hw/ide/core.c | 9 +++++++++ hw/ide/internal.h | 1 + 3 files changed, 17 insertions(+), 1 deletion(-)
hw/ide/ahci.c +7 -1
@@ -1107,9 +1107,14 @@ static int ahci_dma_add_status(IDEDMA *dma, int status)static int ahci_dma_set_inactive (IDEDMA *dma) { +return 0 ; +} + +static int ahci_async_cmd_done (IDEDMA *dma) +{AHCIDevice *ad =DO_UPCAST (AHCIDevice, dma, dma); -DPRINTF (ad->port_no,"dma done\n" ); +DPRINTF (ad->port_no,"async cmd done\n" );/* update d2h status */ ahci_write_fis_d2h (ad,NULL );
@@ -1144,6 +1149,7 @@ static const IDEDMAOps ahci_dma_ops = { .set_unit = ahci_dma_set_unit, .add_status = ahci_dma_add_status, .set_inactive = ahci_dma_set_inactive, + .async_cmd_done = ahci_async_cmd_done, .restart_cb = ahci_dma_restart_cb, .reset = ahci_dma_reset, };
hw/ide/core.c +9 -0
@@ -568,10 +568,18 @@ static void dma_buf_commit(IDEState *s)qemu_sglist_destroy (&s->sg); } +static void ide_async_cmd_done (IDEState *s) +{ +if (s->bus->dma->ops->async_cmd_done) { + s->bus->dma->ops->async_cmd_done (s->bus->dma); + } +} +void ide_set_inactive (IDEState *s) { s->bus->dma->aiocb =NULL ; s->bus->dma->ops->set_inactive (s->bus->dma); +ide_async_cmd_done (s); }void ide_dma_error (IDEState *s)
@@ -804,6 +812,7 @@ static void ide_flush_cb(void *opaque, int ret)bdrv_acct_done (s->bs, &s->acct); s->status =READY_STAT |SEEK_STAT ; +ide_async_cmd_done (s);ide_set_irq (s->bus); }
hw/ide/internal.h +1 -0
@@ -433,6 +433,7 @@ struct IDEDMAOps {DMAIntFunc *set_unit;DMAIntFunc *add_status;DMAFunc *set_inactive; +DMAFunc *async_cmd_done;DMARestartFunc *restart_cb;DMAFunc *reset; };
Signature · 1 line
1.8.1.4
Quoted 11 lines · Kevin Wolf
AHCI couldn't cope with asynchronous commands that aren't doing DMA, it simply wouldn't complete them. Due to the bug fixed in commit
f68ec837, FLUSH commands would seem to have completed immediately even if they were still running on the host. After the commit, they would simply hang and never unset the BSY bit, rendering AHCI unusable on any OS sending flushes.This patch adds another callback for the completion of asynchronous commands. This is what AHCI really wants to use for its command completion logic rather than an DMA completion callback.
Quoted 7 lines
Signed-off-by Kevin Wolf <[email protected]>--- hw/ide/ahci.c | 8 +++++++- hw/ide/core.c | 9 +++++++++ hw/ide/internal.h | 1 + 3 files changed, 17 insertions(+), 1 deletion(-)
Quoted 7 lines · Kevin Wolf
AHCI couldn't cope with asynchronous commands that aren't doing DMA, it simply wouldn't complete them. Due to the bug fixed in commit
f68ec837, FLUSH commands would seem to have completed immediately even if they were still running on the host. After the commit, they would simply hang and never unset the BSY bit, rendering AHCI unusable on any OS sending flushes.
Quoted 6 lines
This patch adds another callback for the completion of asynchronous commands. This is what AHCI really wants to use for its command completion logic rather than an DMA completion callback.
Signed-off-by Kevin Wolf <[email protected]>
Works!
Quoted 77 lines
--- hw/ide/ahci.c | 8 +++++++- hw/ide/core.c | 9 +++++++++ hw/ide/internal.h | 1 + 3 files changed, 17 insertions(+), 1 deletion(-)hw/ide/ahci.c +7
-1@@ -1107,9 +1107,14 @@ static int ahci_dma_add_status(IDEDMA *dma, int status)static int ahci_dma_set_inactive (IDEDMA *dma) { +return 0 ; +} + +static int ahci_async_cmd_done (IDEDMA *dma) +{AHCIDevice *ad =DO_UPCAST (AHCIDevice, dma, dma); -DPRINTF (ad->port_no,"dma done\n" ); +DPRINTF (ad->port_no,"async cmd done\n" );/* update d2h status */ ahci_write_fis_d2h (ad,NULL );@@ -1144,6 +1149,7 @@ static const IDEDMAOps ahci_dma_ops = { .set_unit = ahci_dma_set_unit, .add_status = ahci_dma_add_status, .set_inactive = ahci_dma_set_inactive, + .async_cmd_done = ahci_async_cmd_done, .restart_cb = ahci_dma_restart_cb, .reset = ahci_dma_reset, };hw/ide/core.c +9
-0@@ -568,10 +568,18 @@ static void dma_buf_commit(IDEState *s)qemu_sglist_destroy (&s->sg); } +static void ide_async_cmd_done (IDEState *s) +{ +if (s->bus->dma->ops->async_cmd_done) { + s->bus->dma->ops->async_cmd_done (s->bus->dma); + } +} +void ide_set_inactive (IDEState *s) { s->bus->dma->aiocb =NULL ; s->bus->dma->ops->set_inactive (s->bus->dma); +ide_async_cmd_done (s); }void ide_dma_error (IDEState *s)@@ -804,6 +812,7 @@ static void ide_flush_cb(void *opaque, int ret)bdrv_acct_done (s->bs, &s->acct); s->status =READY_STAT |SEEK_STAT ; +ide_async_cmd_done (s);ide_set_irq (s->bus); }hw/ide/internal.h +1
-0@@ -433,6 +433,7 @@ struct IDEDMAOps {DMAIntFunc *set_unit;DMAIntFunc *add_status;DMAFunc *set_inactive; +DMAFunc *async_cmd_done;DMARestartFunc *restart_cb;DMAFunc *reset; };
Quoted 7 lines · Kevin Wolf
AHCI couldn't cope with asynchronous commands that aren't doing DMA, it simply wouldn't complete them. Due to the bug fixed in commit
f68ec837, FLUSH commands would seem to have completed immediately even if they were still running on the host. After the commit, they would simply hang and never unset the BSY bit, rendering AHCI unusable on any OS sending flushes.
Quoted 6 lines
This patch adds another callback for the completion of asynchronous commands. This is what AHCI really wants to use for its command completion logic rather than an DMA completion callback.
Signed-off-by Kevin Wolf <[email protected]>
Works for me, thanks.
Am 15.07.2013 11:34, schrieb Kevin Wolf:
Quoted 24 lines
AHCI couldn't cope with asynchronous commands that aren't doing DMA, it simply wouldn't complete them. Due to the bug fixed in commit
f68ec837, FLUSH commands would seem to have completed immediately even if they were still running on the host. After the commit, they would simply hang and never unset the BSY bit, rendering AHCI unusable on any OS sending flushes.This patch adds another callback for the completion of asynchronous commands. This is what AHCI really wants to use for its command completion logic rather than an DMA completion callback.
Signed-off-by Kevin Wolf <[email protected]>--- hw/ide/ahci.c | 8 +++++++- hw/ide/core.c | 9 +++++++++ hw/ide/internal.h | 1 + 3 files changed, 17 insertions(+), 1 deletion(-)hw/ide/ahci.c +0
-0
hw/ide/ahci.c static int ahci_dma_add_status(IDEDMA *dma, int status) · line 1110
Quoted 4 lines
hw/ide/ahci.c +1
-0@@ -1107,9 +1107,14 @@ static int ahci_dma_add_status(IDEDMA *dma, int status)static int ahci_dma_set_inactive (IDEDMA *dma) { +return 0 ;
Is it intentional that this is now no-op rather than calling dma->ops->set_inactive(dma) like core IDE does below?
Other than that it looks sensible to me.
Andreas
Quoted 63 lines
hw/ide/ahci.c +6
-1+} + +static int ahci_async_cmd_done (IDEDMA *dma) +{AHCIDevice *ad =DO_UPCAST (AHCIDevice, dma, dma); -DPRINTF (ad->port_no,"dma done\n" ); +DPRINTF (ad->port_no,"async cmd done\n" );/* update d2h status */ ahci_write_fis_d2h (ad,NULL );@@ -1144,6 +1149,7 @@ static const IDEDMAOps ahci_dma_ops = { .set_unit = ahci_dma_set_unit, .add_status = ahci_dma_add_status, .set_inactive = ahci_dma_set_inactive, + .async_cmd_done = ahci_async_cmd_done, .restart_cb = ahci_dma_restart_cb, .reset = ahci_dma_reset, };hw/ide/core.c +9
-0@@ -568,10 +568,18 @@ static void dma_buf_commit(IDEState *s)qemu_sglist_destroy (&s->sg); } +static void ide_async_cmd_done (IDEState *s) +{ +if (s->bus->dma->ops->async_cmd_done) { + s->bus->dma->ops->async_cmd_done (s->bus->dma); + } +} +void ide_set_inactive (IDEState *s) { s->bus->dma->aiocb =NULL ; s->bus->dma->ops->set_inactive (s->bus->dma); +ide_async_cmd_done (s); }void ide_dma_error (IDEState *s)@@ -804,6 +812,7 @@ static void ide_flush_cb(void *opaque, int ret)bdrv_acct_done (s->bs, &s->acct); s->status =READY_STAT |SEEK_STAT ; +ide_async_cmd_done (s);ide_set_irq (s->bus); }hw/ide/internal.h +1
-0@@ -433,6 +433,7 @@ struct IDEDMAOps {DMAIntFunc *set_unit;DMAIntFunc *add_status;DMAFunc *set_inactive; +DMAFunc *async_cmd_done;DMARestartFunc *restart_cb;DMAFunc *reset; };
Signature · 2 lines
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Am 16.07.2013 um 14:30 hat Andreas Färber geschrieben:
Quoted 26 lines
Am 15.07.2013 11:34, schrieb Kevin Wolf:
Quoted 25 lines
AHCI couldn't cope with asynchronous commands that aren't doing DMA, it simply wouldn't complete them. Due to the bug fixed in commit
f68ec837, FLUSH commands would seem to have completed immediately even if they were still running on the host. After the commit, they would simply hang and never unset the BSY bit, rendering AHCI unusable on any OS sending flushes.This patch adds another callback for the completion of asynchronous commands. This is what AHCI really wants to use for its command completion logic rather than an DMA completion callback.
Signed-off-by Kevin Wolf <[email protected]>--- hw/ide/ahci.c | 8 +++++++- hw/ide/core.c | 9 +++++++++ hw/ide/internal.h | 1 + 3 files changed, 17 insertions(+), 1 deletion(-)hw/ide/ahci.c +0
-0@@ -1107,9 +1107,14 @@ static int ahci_dma_add_status(IDEDMA *dma, int status)
Quoted 6 lines
hw/ide/ahci.c static int ahci_dma_add_status(IDEDMA *dma, int status) · line 1110
Quoted 3 lines
hw/ide/ahci.c +1
-0static int ahci_dma_set_inactive (IDEDMA *dma) { +return 0 ;Is it intentional that this is now no-op rather than calling
dma->ops->set_inactive(dma)like core IDE does below?
This _is_ the dma->ops->set_inactive() callback of AHCI that is called by the IDE code. For plain IDE commands the same IDEDMAOps function is implemented by ide_nop().
Kevin
Quoted 72 lines
Other than that it looks sensible to me.
Andreas
Quoted 63 lines
hw/ide/ahci.c +6
-1+} + +static int ahci_async_cmd_done (IDEDMA *dma) +{AHCIDevice *ad =DO_UPCAST (AHCIDevice, dma, dma); -DPRINTF (ad->port_no,"dma done\n" ); +DPRINTF (ad->port_no,"async cmd done\n" );/* update d2h status */ ahci_write_fis_d2h (ad,NULL );@@ -1144,6 +1149,7 @@ static const IDEDMAOps ahci_dma_ops = { .set_unit = ahci_dma_set_unit, .add_status = ahci_dma_add_status, .set_inactive = ahci_dma_set_inactive, + .async_cmd_done = ahci_async_cmd_done, .restart_cb = ahci_dma_restart_cb, .reset = ahci_dma_reset, };hw/ide/core.c +9
-0@@ -568,10 +568,18 @@ static void dma_buf_commit(IDEState *s)qemu_sglist_destroy (&s->sg); } +static void ide_async_cmd_done (IDEState *s) +{ +if (s->bus->dma->ops->async_cmd_done) { + s->bus->dma->ops->async_cmd_done (s->bus->dma); + } +} +void ide_set_inactive (IDEState *s) { s->bus->dma->aiocb =NULL ; s->bus->dma->ops->set_inactive (s->bus->dma); +ide_async_cmd_done (s); }void ide_dma_error (IDEState *s)@@ -804,6 +812,7 @@ static void ide_flush_cb(void *opaque, int ret)bdrv_acct_done (s->bs, &s->acct); s->status =READY_STAT |SEEK_STAT ; +ide_async_cmd_done (s);ide_set_irq (s->bus); }hw/ide/internal.h +1
-0@@ -433,6 +433,7 @@ struct IDEDMAOps {DMAIntFunc *set_unit;DMAIntFunc *add_status;DMAFunc *set_inactive; +DMAFunc *async_cmd_done;DMARestartFunc *restart_cb;DMAFunc *reset; };Signature · 2 lines
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Quoted 7 lines · Kevin Wolf
AHCI couldn't cope with asynchronous commands that aren't doing DMA, it simply wouldn't complete them. Due to the bug fixed in commit
f68ec837, FLUSH commands would seem to have completed immediately even if they were still running on the host. After the commit, they would simply hang and never unset the BSY bit, rendering AHCI unusable on any OS sending flushes.
Quoted 6 lines
This patch adds another callback for the completion of asynchronous commands. This is what AHCI really wants to use for its command completion logic rather than an DMA completion callback.
Signed-off-by Kevin Wolf <[email protected]>
Quoted 79 lines
--- hw/ide/ahci.c | 8 +++++++- hw/ide/core.c | 9 +++++++++ hw/ide/internal.h | 1 + 3 files changed, 17 insertions(+), 1 deletion(-)hw/ide/ahci.c +7
-1@@ -1107,9 +1107,14 @@ static int ahci_dma_add_status(IDEDMA *dma, int status)static int ahci_dma_set_inactive (IDEDMA *dma) { +return 0 ; +} + +static int ahci_async_cmd_done (IDEDMA *dma) +{AHCIDevice *ad =DO_UPCAST (AHCIDevice, dma, dma); -DPRINTF (ad->port_no,"dma done\n" ); +DPRINTF (ad->port_no,"async cmd done\n" );/* update d2h status */ ahci_write_fis_d2h (ad,NULL );@@ -1144,6 +1149,7 @@ static const IDEDMAOps ahci_dma_ops = { .set_unit = ahci_dma_set_unit, .add_status = ahci_dma_add_status, .set_inactive = ahci_dma_set_inactive, + .async_cmd_done = ahci_async_cmd_done, .restart_cb = ahci_dma_restart_cb, .reset = ahci_dma_reset, };hw/ide/core.c +9
-0@@ -568,10 +568,18 @@ static void dma_buf_commit(IDEState *s)qemu_sglist_destroy (&s->sg); } +static void ide_async_cmd_done (IDEState *s) +{ +if (s->bus->dma->ops->async_cmd_done) { + s->bus->dma->ops->async_cmd_done (s->bus->dma); + } +} +void ide_set_inactive (IDEState *s) { s->bus->dma->aiocb =NULL ; s->bus->dma->ops->set_inactive (s->bus->dma); +ide_async_cmd_done (s); }void ide_dma_error (IDEState *s)@@ -804,6 +812,7 @@ static void ide_flush_cb(void *opaque, int ret)bdrv_acct_done (s->bs, &s->acct); s->status =READY_STAT |SEEK_STAT ; +ide_async_cmd_done (s);ide_set_irq (s->bus); }hw/ide/internal.h +1
-0@@ -433,6 +433,7 @@ struct IDEDMAOps {DMAIntFunc *set_unit;DMAIntFunc *add_status;DMAFunc *set_inactive; +DMAFunc *async_cmd_done;DMARestartFunc *restart_cb;DMAFunc *reset; };Signature · 1 line
1.8.1.4