Message ID | 1499170866-9068-1-git-send-email-jasowang@redhat.com |
---|---|
State | New |
Headers | show |
On Tue, 07/04 20:21, Jason Wang wrote: > After converting to use DMA api for virtio devices, we should use > dma_as instead of address_space_memory. Otherwise it won't work if > IOMMU is enabled. > > Fixes: commit 8607f5c3072c ("virtio: convert to use DMA api") > Cc: qemu-stable@nongnu.org > Signed-off-by: Jason Wang <jasowang@redhat.com> Reviewed-by: Fam Zheng <famz@redhat.com>
On Tue, Jul 04, 2017 at 08:21:06PM +0800, Jason Wang wrote: > After converting to use DMA api for virtio devices, we should use > dma_as instead of address_space_memory. Otherwise it won't work if > IOMMU is enabled. > > Fixes: commit 8607f5c3072c ("virtio: convert to use DMA api") > Cc: qemu-stable@nongnu.org > Signed-off-by: Jason Wang <jasowang@redhat.com> > --- > hw/scsi/virtio-scsi.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c > index f46f06d..d076fe7 100644 > --- a/hw/scsi/virtio-scsi.c > +++ b/hw/scsi/virtio-scsi.c > @@ -43,12 +43,13 @@ static inline SCSIDevice *virtio_scsi_device_find(VirtIOSCSI *s, uint8_t *lun) > > void virtio_scsi_init_req(VirtIOSCSI *s, VirtQueue *vq, VirtIOSCSIReq *req) > { > + VirtIODevice *vdev = VIRTIO_DEVICE(s); > const size_t zero_skip = > offsetof(VirtIOSCSIReq, resp_iov) + sizeof(req->resp_iov); > > req->vq = vq; > req->dev = s; > - qemu_sglist_init(&req->qsgl, DEVICE(s), 8, &address_space_memory); > + qemu_sglist_init(&req->qsgl, DEVICE(s), 8, vdev->dma_as); > qemu_iovec_init(&req->resp_iov, 1); > memset((uint8_t *)req + zero_skip, 0, sizeof(*req) - zero_skip); > } Reviewed-by: Wei Xu <wexu@redhat.com> > -- > 2.7.4 > >
On Tue, 07/04 20:21, Jason Wang wrote: > After converting to use DMA api for virtio devices, we should use > dma_as instead of address_space_memory. Otherwise it won't work if > IOMMU is enabled. > > Fixes: commit 8607f5c3072c ("virtio: convert to use DMA api") > Cc: qemu-stable@nongnu.org > Signed-off-by: Jason Wang <jasowang@redhat.com> > --- > hw/scsi/virtio-scsi.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c > index f46f06d..d076fe7 100644 > --- a/hw/scsi/virtio-scsi.c > +++ b/hw/scsi/virtio-scsi.c > @@ -43,12 +43,13 @@ static inline SCSIDevice *virtio_scsi_device_find(VirtIOSCSI *s, uint8_t *lun) > > void virtio_scsi_init_req(VirtIOSCSI *s, VirtQueue *vq, VirtIOSCSIReq *req) > { > + VirtIODevice *vdev = VIRTIO_DEVICE(s); > const size_t zero_skip = > offsetof(VirtIOSCSIReq, resp_iov) + sizeof(req->resp_iov); > > req->vq = vq; > req->dev = s; > - qemu_sglist_init(&req->qsgl, DEVICE(s), 8, &address_space_memory); > + qemu_sglist_init(&req->qsgl, DEVICE(s), 8, vdev->dma_as); Not related to this patch, but is our vIOMMU iotlb implementation safe for data plane (both virtio-blk and virtio-scsi)? There we don't have BQL to synchronize between IOVA lookup and update, which means threads can race when accessing the iotlb GHashTable. Did I overlook a synchronization mechanism? Or is it missing? Paolo, MST? Fam > qemu_iovec_init(&req->resp_iov, 1); > memset((uint8_t *)req + zero_skip, 0, sizeof(*req) - zero_skip); > } > -- > 2.7.4 > >
On 05/07/2017 08:23, Fam Zheng wrote: >> req->vq = vq; >> req->dev = s; >> - qemu_sglist_init(&req->qsgl, DEVICE(s), 8, &address_space_memory); >> + qemu_sglist_init(&req->qsgl, DEVICE(s), 8, vdev->dma_as); > Not related to this patch, but is our vIOMMU iotlb implementation safe for data > plane (both virtio-blk and virtio-scsi)? There we don't have BQL to synchronize > between IOVA lookup and update, which means threads can race when accessing the > iotlb GHashTable. > > Did I overlook a synchronization mechanism? Or is it missing? Paolo, MST? I think it's missing. One way to fix it could be to finish the MemoryRegionCache work and have an IOMMU->virtio notifier, similar to how vhost works. Paolo
diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c index f46f06d..d076fe7 100644 --- a/hw/scsi/virtio-scsi.c +++ b/hw/scsi/virtio-scsi.c @@ -43,12 +43,13 @@ static inline SCSIDevice *virtio_scsi_device_find(VirtIOSCSI *s, uint8_t *lun) void virtio_scsi_init_req(VirtIOSCSI *s, VirtQueue *vq, VirtIOSCSIReq *req) { + VirtIODevice *vdev = VIRTIO_DEVICE(s); const size_t zero_skip = offsetof(VirtIOSCSIReq, resp_iov) + sizeof(req->resp_iov); req->vq = vq; req->dev = s; - qemu_sglist_init(&req->qsgl, DEVICE(s), 8, &address_space_memory); + qemu_sglist_init(&req->qsgl, DEVICE(s), 8, vdev->dma_as); qemu_iovec_init(&req->resp_iov, 1); memset((uint8_t *)req + zero_skip, 0, sizeof(*req) - zero_skip); }
After converting to use DMA api for virtio devices, we should use dma_as instead of address_space_memory. Otherwise it won't work if IOMMU is enabled. Fixes: commit 8607f5c3072c ("virtio: convert to use DMA api") Cc: qemu-stable@nongnu.org Signed-off-by: Jason Wang <jasowang@redhat.com> --- hw/scsi/virtio-scsi.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)