On Wed, May 17, 2023 at 05:12:57PM -0400, John Snow wrote: > On Fri, Apr 28, 2023 at 9:22 AM Niklas Cassel <n...@flawful.org> wrote: > > > > From: Niklas Cassel <niklas.cas...@wdc.com> > > > > Currently, the first time sending an unsupported command > > (e.g. READ LOG DMA EXT) will not have ERR_STAT set in the completion. > > Sending the unsupported command again, will correctly have ERR_STAT set. > > > > When ide_cmd_permitted() returns false, it calls ide_abort_command(). > > ide_abort_command() first calls ide_transfer_stop(), which will call > > ide_transfer_halt() and ide_cmd_done(), after that ide_abort_command() > > sets ERR_STAT in status. > > > > ide_cmd_done() for AHCI will call ahci_write_fis_d2h() which writes the > > current status in the FIS, and raises an IRQ. (The status here will not > > have ERR_STAT set!). > > > > Thus, we cannot call ide_transfer_stop() before setting ERR_STAT, as > > ide_transfer_stop() will result in the FIS being written and an IRQ > > being raised. > > > > The reason why it works the second time, is that ERR_STAT will still > > be set from the previous command, so when writing the FIS, the > > completion will correctly have ERR_STAT set. > > > > Set ERR_STAT before writing the FIS (calling cmd_done), so that we will > > raise an error IRQ correctly when receiving an unsupported command. > > > > Signed-off-by: Niklas Cassel <niklas.cas...@wdc.com> > > --- > > hw/ide/core.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/hw/ide/core.c b/hw/ide/core.c > > index 45d14a25e9..c144d1155d 100644 > > --- a/hw/ide/core.c > > +++ b/hw/ide/core.c > > @@ -531,9 +531,9 @@ BlockAIOCB *ide_issue_trim( > > > > void ide_abort_command(IDEState *s) > > { > > - ide_transfer_stop(s); > > s->status = READY_STAT | ERR_STAT; > > s->error = ABRT_ERR; > > + ide_transfer_stop(s); > > } > > > > static void ide_set_retry(IDEState *s) > > -- > > 2.40.0 > > > > Seems OK at a glance. Does this change the behavior of > ide_transfer_stop at all? I guess we've been using this order of > operations since 2008 at least. I didn't know C then.
Hello John, Not as far as I can see. Kind regards, Niklas