Re: [Qemu-devel] [PATCH 03/66] dma: keep a device alive while it has SGLists

[Jan Kiszka]

On 2013-07-04 17:12, Paolo Bonzini wrote:
> Signed-off-by: Paolo Bonzini <address@hidden>
> ---
>  dma-helpers.c         |  6 +++++-
>  hw/ide/ahci.c         |  3 ++-
>  hw/ide/macio.c        |  4 ++--
>  hw/scsi/virtio-scsi.c | 10 ++++++----
>  hw/usb/hcd-ehci.c     |  4 ++--
>  include/hw/pci/pci.h  |  2 +-
>  include/sysemu/dma.h  |  4 +++-
>  7 files changed, 21 insertions(+), 12 deletions(-)
> 
> diff --git a/dma-helpers.c b/dma-helpers.c
> index 5afba47..499550f 100644
> --- a/dma-helpers.c
> +++ b/dma-helpers.c
> @@ -34,13 +34,16 @@ int dma_memory_set(AddressSpace *as, dma_addr_t addr, 
> uint8_t c, dma_addr_t len)
>      return error;
>  }
>  
> -void qemu_sglist_init(QEMUSGList *qsg, int alloc_hint, AddressSpace *as)
> +void qemu_sglist_init(QEMUSGList *qsg, DeviceState *dev, int alloc_hint,
> +                      AddressSpace *as)
>  {
>      qsg->sg = g_malloc(alloc_hint * sizeof(ScatterGatherEntry));
>      qsg->nsg = 0;
>      qsg->nalloc = alloc_hint;
>      qsg->size = 0;
>      qsg->as = as;
> +    qsg->dev = dev;
> +    object_ref(OBJECT(dev));
>  }
>  
>  void qemu_sglist_add(QEMUSGList *qsg, dma_addr_t base, dma_addr_t len)
> @@ -57,6 +60,7 @@ void qemu_sglist_add(QEMUSGList *qsg, dma_addr_t base, 
> dma_addr_t len)
>  
>  void qemu_sglist_destroy(QEMUSGList *qsg)
>  {
> +    object_unref(OBJECT(qsg->dev));
>      g_free(qsg->sg);
>      memset(qsg, 0, sizeof(*qsg));
>  }
> diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
> index 1adfa0b..6b87549 100644
> --- a/hw/ide/ahci.c
> +++ b/hw/ide/ahci.c
> @@ -691,7 +691,8 @@ static int ahci_populate_sglist(AHCIDevice *ad, 
> QEMUSGList *sglist, int offset)
>              goto out;
>          }
>  
> -        qemu_sglist_init(sglist, (sglist_alloc_hint - off_idx), ad->hba->as);
> +        qemu_sglist_init(sglist, DEVICE(ad->hba), (sglist_alloc_hint - 
> off_idx),
> +                         ad->hba->as);
>          qemu_sglist_add(sglist, le64_to_cpu(tbl[off_idx].addr + off_pos),
>                          le32_to_cpu(tbl[off_idx].flags_size) + 1 - off_pos);
>  
> diff --git a/hw/ide/macio.c b/hw/ide/macio.c
> index a1952b0..84288ae 100644
> --- a/hw/ide/macio.c
> +++ b/hw/ide/macio.c
> @@ -70,7 +70,7 @@ static void pmac_ide_atapi_transfer_cb(void *opaque, int 
> ret)
>  
>      s->io_buffer_size = io->len;
>  
> -    qemu_sglist_init(&s->sg, io->len / MACIO_PAGE_SIZE + 1,
> +    qemu_sglist_init(&s->sg, DEVICE(m), io->len / MACIO_PAGE_SIZE + 1,
>                       &address_space_memory);
>      qemu_sglist_add(&s->sg, io->addr, io->len);
>      io->addr += io->len;
> @@ -127,7 +127,7 @@ static void pmac_ide_transfer_cb(void *opaque, int ret)
>      s->io_buffer_index = 0;
>      s->io_buffer_size = io->len;
>  
> -    qemu_sglist_init(&s->sg, io->len / MACIO_PAGE_SIZE + 1,
> +    qemu_sglist_init(&s->sg, DEVICE(m), io->len / MACIO_PAGE_SIZE + 1,
>                       &address_space_memory);
>      qemu_sglist_add(&s->sg, io->addr, io->len);
>      io->addr += io->len;
> diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
> index b8a0abf..712f0ad 100644
> --- a/hw/scsi/virtio-scsi.c
> +++ b/hw/scsi/virtio-scsi.c
> @@ -77,10 +77,12 @@ static void virtio_scsi_bad_req(void)
>      exit(1);
>  }
>  
> -static void qemu_sgl_init_external(QEMUSGList *qsgl, struct iovec *sg,
> +static void qemu_sgl_init_external(VirtIOSCSIReq *req, struct iovec *sg,
>                                     hwaddr *addr, int num)
>  {
> -    qemu_sglist_init(qsgl, num, &address_space_memory);
> +    QEMUSGList *qsgl = &req->qsgl;
> +
> +    qemu_sglist_init(qsgl, DEVICE(req->dev), num, &address_space_memory);
>      while (num--) {
>          qemu_sglist_add(qsgl, *(addr++), (sg++)->iov_len);
>      }
> @@ -99,11 +101,11 @@ static void virtio_scsi_parse_req(VirtIOSCSI *s, 
> VirtQueue *vq,
>      req->resp.buf = req->elem.in_sg[0].iov_base;
>  
>      if (req->elem.out_num > 1) {
> -        qemu_sgl_init_external(&req->qsgl, &req->elem.out_sg[1],
> +        qemu_sgl_init_external(req, &req->elem.out_sg[1],
>                                 &req->elem.out_addr[1],
>                                 req->elem.out_num - 1);
>      } else {
> -        qemu_sgl_init_external(&req->qsgl, &req->elem.in_sg[1],
> +        qemu_sgl_init_external(req, &req->elem.in_sg[1],
>                                 &req->elem.in_addr[1],
>                                 req->elem.in_num - 1);
>      }
> diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c
> index 16d6356..65e6680 100644
> --- a/hw/usb/hcd-ehci.c
> +++ b/hw/usb/hcd-ehci.c
> @@ -1245,7 +1245,7 @@ static int ehci_init_transfer(EHCIPacket *p)
>      cpage  = get_field(p->qtd.token, QTD_TOKEN_CPAGE);
>      bytes  = get_field(p->qtd.token, QTD_TOKEN_TBYTES);
>      offset = p->qtd.bufptr[0] & ~QTD_BUFPTR_MASK;
> -    qemu_sglist_init(&p->sgl, 5, p->queue->ehci->as);
> +    qemu_sglist_init(&p->sgl, DEVICE(p->queue->ehci), 5, p->queue->ehci->as);

I was just testing this from your current git tree, and it causes

qemu/hw/usb/hcd-ehci.c:1248:ehci_init_transfer: Object 0x7f3eb39c6500 is
not an instance of type device

when booting a q35 machine (as it pulls in EHCI). The cast is obviously
wrong, but I didn't spot yet what is required instead.

Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux