Re: [Qemu-devel] [PATCH 5/6] atapi: call ide_set_irq before ide_transfer_start

[John Snow <jsnow@redhat.com>]

On 04/17/2018 11:39 AM, Paolo Bonzini wrote:
> The ATAPI_INT_REASON_IO interrupt is raised when I/O starts, but in the

This one is benefit of the doubt for me.

(I still can't quite track down our usage of the nsector register for
this, it doesn't seem supported by cmd_packet in ATA8 ... It says that
Interrupt Reason is its own "field" but that doesn't help me know which
register it's supposed to be stored in... Oh, ATA7 says that Interrupt
Reason is stored in "Sector Count." (*sigh* -- How are you supposed to
tell that from the ATA8 doc? What part of the spec tells you how to
actually read out the "Interrupt Reason" field?))

(I am still not sure which spec says when we can interrupt in ATAPI,
either. I guess that's in SCSI somewhere, probably?)

> AHCI case ide_set_irq was actually called at the end of a mutual recursion.
> Move it early, with the side effect that ide_transfer_start becomes a tail
> call in ide_atapi_cmd_reply_end.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  hw/ide/atapi.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c
> index c0509c8bf5..7168ff55a7 100644
> --- a/hw/ide/atapi.c
> +++ b/hw/ide/atapi.c
> @@ -287,6 +287,7 @@ void ide_atapi_cmd_reply_end(IDEState *s)
>          } else {
>              /* a new transfer is needed */
>              s->nsector = (s->nsector & ~7) | ATAPI_INT_REASON_IO;
> +            ide_set_irq(s->bus);
>              byte_count_limit = atapi_byte_count_limit(s);
>              trace_ide_atapi_cmd_reply_end_bcl(s, byte_count_limit);
>              size = s->packet_transfer_size;
> @@ -304,13 +305,12 @@ void ide_atapi_cmd_reply_end(IDEState *s)
>                  if (size > (s->cd_sector_size - s->io_buffer_index))
>                      size = (s->cd_sector_size - s->io_buffer_index);
>              }
> +            trace_ide_atapi_cmd_reply_end_new(s, s->status);
>              s->packet_transfer_size -= size;
>              s->elementary_transfer_size -= size;
>              s->io_buffer_index += size;
>              ide_transfer_start(s, s->io_buffer + s->io_buffer_index - size,
>                                 size, ide_atapi_cmd_reply_end);
> -            ide_set_irq(s->bus);
> -            trace_ide_atapi_cmd_reply_end_new(s, s->status);
>          }
>      }
>  }
> 

BOD:

Acked-by: John Snow <jsnow@redhat.com>