| Message ID | 20180417153945.20737-6-pbonzini@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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>
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); } } }
The ATAPI_INT_REASON_IO interrupt is raised when I/O starts, but in the 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(-)