Re: [Qemu-devel] [PATCH 1/4] macio: use the existing IDEDMA aiocb to hold the active DMA aiocb

[John Snow <jsnow@redhat.com>]

On 01/06/2016 03:37 PM, Mark Cave-Ayland wrote:
> Currently the aiocb is held within MACIOIDEState, however the IDE core code
> assumes that the current actvie DMA aiocb is held in aiocb in a few places,
> e.g. ide_bus_reset() and ide_reset().
> 
> Switch over to using IDEDMA aiocb to store the aiocb for the current active
> DMA request so that bus resets and restarts are handled correctly. As a
> consequence we can now use ide_set_inactive() rather than handling its
> functionality ourselves.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
>  hw/ide/macio.c |   20 ++++++++++++--------
>  hw/ppc/mac.h   |    1 -
>  2 files changed, 12 insertions(+), 9 deletions(-)
> 
> diff --git a/hw/ide/macio.c b/hw/ide/macio.c
> index 3ee962f..560c071 100644
> --- a/hw/ide/macio.c
> +++ b/hw/ide/macio.c
> @@ -119,8 +119,8 @@ static void pmac_dma_read(BlockBackend *blk,
>      MACIO_DPRINTF("--- Block read transfer - sector_num: %" PRIx64 "  "
>                    "nsector: %x\n", (offset >> 9), (bytes >> 9));
>  
> -    m->aiocb = blk_aio_readv(blk, (offset >> 9), &io->iov, (bytes >> 9),
> -                             cb, io);
> +    s->bus->dma->aiocb = blk_aio_readv(blk, (offset >> 9), &io->iov,
> +                             (bytes >> 9), cb, io);
>  }
>  
>  static void pmac_dma_write(BlockBackend *blk,
> @@ -204,8 +204,8 @@ static void pmac_dma_write(BlockBackend *blk,
>      MACIO_DPRINTF("--- Block write transfer - sector_num: %" PRIx64 "  "
>                    "nsector: %x\n", (offset >> 9), (bytes >> 9));
>  
> -    m->aiocb = blk_aio_writev(blk, (offset >> 9), &io->iov, (bytes >> 9),
> -                              cb, io);
> +    s->bus->dma->aiocb = blk_aio_writev(blk, (offset >> 9), &io->iov,
> +                             (bytes >> 9), cb, io);
>  }
>  
>  static void pmac_dma_trim(BlockBackend *blk,
> @@ -231,8 +231,8 @@ static void pmac_dma_trim(BlockBackend *blk,
>      s->io_buffer_index += io->len;
>      io->len = 0;
>  
> -    m->aiocb = ide_issue_trim(blk, (offset >> 9), &io->iov, (bytes >> 9),
> -                              cb, io);
> +    s->bus->dma->aiocb = ide_issue_trim(blk, (offset >> 9), &io->iov,
> +                             (bytes >> 9), cb, io);
>  }
>  
>  static void pmac_ide_atapi_transfer_cb(void *opaque, int ret)
> @@ -291,6 +291,8 @@ done:
>      } else {
>          block_acct_done(blk_get_stats(s->blk), &s->acct);
>      }
> +
> +    ide_set_inactive(s, false);
>      io->dma_end(opaque);
>  
>      return;
> @@ -307,7 +309,6 @@ static void pmac_ide_transfer_cb(void *opaque, int ret)
>  
>      if (ret < 0) {
>          MACIO_DPRINTF("DMA error: %d\n", ret);
> -        m->aiocb = NULL;
>          ide_dma_error(s);
>          goto done;
>      }
> @@ -358,6 +359,8 @@ done:
>              block_acct_done(blk_get_stats(s->blk), &s->acct);
>          }
>      }
> +
> +    ide_set_inactive(s, false);
>      io->dma_end(opaque);
>  }
>  
> @@ -395,8 +398,9 @@ static void pmac_ide_transfer(DBDMA_io *io)
>  static void pmac_ide_flush(DBDMA_io *io)
>  {
>      MACIOIDEState *m = io->opaque;
> +    IDEState *s = idebus_active_if(&m->bus);
>  
> -    if (m->aiocb) {
> +    if (s->bus->dma->aiocb) {
>          blk_drain_all();
>      }
>  }
> diff --git a/hw/ppc/mac.h b/hw/ppc/mac.h
> index e375ed2..ecf7792 100644
> --- a/hw/ppc/mac.h
> +++ b/hw/ppc/mac.h
> @@ -134,7 +134,6 @@ typedef struct MACIOIDEState {
>  
>      MemoryRegion mem;
>      IDEBus bus;
> -    BlockAIOCB *aiocb;
>      IDEDMA dma;
>      void *dbdma;
>      bool dma_active;
> 

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