diff mbox

[03/66] dma: keep a device alive while it has SGLists

Message ID 1372950842-32422-4-git-send-email-pbonzini@redhat.com
State New
Headers show

Commit Message

Paolo Bonzini July 4, 2013, 3:12 p.m. UTC 
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 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(-)

Comments

Jan Kiszka July 4, 2013, 3:51 p.m. UTC | #1 
On 2013-07-04 17:12, Paolo Bonzini wrote:
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  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
Jan Kiszka July 4, 2013, 3:59 p.m. UTC | #2 
On 2013-07-04 17:51, Jan Kiszka wrote:
> On 2013-07-04 17:12, Paolo Bonzini wrote:
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>> ---
>>  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.

The problem is that EHCIState is embedded into the actual QOM device
states, but not at a known location. So you likely need a generic device
pointer from EHCIState back to the device using it.

Jan
diff mbox

Patch

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);
 
     while (bytes > 0) {
         if (cpage > 4) {
@@ -1484,7 +1484,7 @@  static int ehci_process_itd(EHCIState *ehci,
                 return -1;
             }
 
-            qemu_sglist_init(&ehci->isgl, 2, ehci->as);
+            qemu_sglist_init(&ehci->isgl, DEVICE(ehci), 2, ehci->as);
             if (off + len > 4096) {
                 /* transfer crosses page border */
                 uint32_t len2 = off + len - 4096;
diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index 6ef1f97..de406d6 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -702,7 +702,7 @@  static inline void pci_dma_unmap(PCIDevice *dev, void *buffer, dma_addr_t len,
 static inline void pci_dma_sglist_init(QEMUSGList *qsg, PCIDevice *dev,
                                        int alloc_hint)
 {
-    qemu_sglist_init(qsg, alloc_hint, pci_get_address_space(dev));
+    qemu_sglist_init(qsg, DEVICE(dev), alloc_hint, pci_get_address_space(dev));
 }
 
 extern const VMStateDescription vmstate_pci_device;
diff --git a/include/sysemu/dma.h b/include/sysemu/dma.h
index 031d1f5..00f21f3 100644
--- a/include/sysemu/dma.h
+++ b/include/sysemu/dma.h
@@ -29,6 +29,7 @@  struct QEMUSGList {
     int nsg;
     int nalloc;
     size_t size;
+    DeviceState *dev;
     AddressSpace *as;
 };
 
@@ -189,7 +190,8 @@  struct ScatterGatherEntry {
     dma_addr_t len;
 };
 
-void qemu_sglist_init(QEMUSGList *qsg, int alloc_hint, AddressSpace *as);
+void qemu_sglist_init(QEMUSGList *qsg, DeviceState *dev, int alloc_hint,
+                      AddressSpace *as);
 void qemu_sglist_add(QEMUSGList *qsg, dma_addr_t base, dma_addr_t len);
 void qemu_sglist_destroy(QEMUSGList *qsg);
 #endif